Skip to content

fix(syscall-stat): stop skipping syscall key 0 during stats traversal and no top mode - #119

Open
yuKing123-king wants to merge 1 commit into
DKapture:mainfrom
yuKing123-king:fix/syscall-stat-first-key
Open

yuKing123-king wants to merge 1 commit into
DKapture:mainfrom
yuKing123-king:fix/syscall-stat-first-key

Conversation

@yuKing123-king

@yuKing123-king yuKing123-king commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

修复了两个测试过程中发现的 Bug

  • Bug 1: 修复系统调用号 0 (read) 统计丢失问题

    • 原因:原逻辑中 key 初始化为 0,且使用 bpf_map_get_next_key 查找下一个 key。第一个 key 就是 0,get_next_key 会跳过它直接找下一个,后续使用nxt_key作为真正第一个开始查找的key,导致 系统调用号为0的read syscall 永远不被统计。
    • 修复:调整遍历逻辑,确保从第一个 key 开始完整遍历。
    • 代码如下
      u32 key = 0, nxt_key;
      u32 total = 0;
      std::vector<std::pair<u32, info>> stats; 
      
      // 先尝试获取第一个 key
      if (bpf_map_get_next_key(stats_fd, nullptr, &key) != 0) {
          return; // Map is empty
      }
      
      do {
          info sys_stat;
          if (bpf_map_lookup_elem(stats_fd, &key, &sys_stat) != 0) {
              key = nxt_key;
              continue;
          }
      
          if (key >= sizeof(sys_tbl) / sizeof(sys_tbl[0])) {
              break; 
          }
      
          if (sys_stat.cnt > 0) {
              stats.push_back({key, sys_stat});
              total += sys_stat.cnt;
              
              // Clear the stat after reading
              memset(&sys_stat, 0, sizeof(sys_stat));
              bpf_map_update_elem(stats_fd, &key, &sys_stat, BPF_ANY);
          }
          
          // Get next key for the next iteration
          if (bpf_map_get_next_key(stats_fd, &key, &nxt_key) != 0) {
              break; // No more keys
          }
          key = nxt_key;
      } while (true);
  • Bug 2: 修复 --top / -t 动态刷新功能无效问题

    • 原因:虽然底层动态刷新逻辑已实现,但在 parse_args 参数解析函数中遗漏了对 -t--top 选项的处理。
    • 修复:在 parse_args 中添加了对应的 case 分支,使终端能正确识别并开启动态刷新模式。

@yuKing123-king
yuKing123-king force-pushed the fix/syscall-stat-first-key branch from 3b8787c to 2d9bbe7 Compare August 4, 2026 08:24
@yuKing123-king yuKing123-king changed the title fix(syscall-stat): stop skipping syscall key 0 during stats traversal fix(syscall-stat): stop skipping syscall key 0 during stats traversal and no top mode Aug 4, 2026
@yuKing123-king
yuKing123-king force-pushed the fix/syscall-stat-first-key branch 2 times, most recently from f411ec9 to e379b4d Compare August 12, 2026 08:27
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 5dabdbb)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

… and no top mode

Signed-off-by: Wang Yu <wangyu6@uniontech.com>
@yuKing123-king
yuKing123-king force-pushed the fix/syscall-stat-first-key branch from e379b4d to 5dabdbb Compare August 12, 2026 08:34
@yus-cpu

yus-cpu commented Aug 12, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
Recommended focus areas for review

Loop Termination Bug

在成功处理一条统计记录后,第 321 行将 bpf_map_get_next_key 的返回值赋给 lret,而 while 循环条件检查的是 retret 在该路径上从未被更新,导致循环无法通过正常路径检测到"没有更多 key"的情况。当前依赖一个间接兜底路径终止循环:重新访问上一个 key(其 cnt 已被清零),走 sys_stat.cnt == 0 分支时再次调用 bpf_map_get_next_key 并更新 ret 才能退出。如果 BPF 程序在两次迭代之间又对该 key 的计数器进行了递增,兜底路径将失效,同一 key 会被重复处理,造成统计数据重复累加。应将第 321 行的 lret 改为 ret

已更改

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 5dabdbb

@dkapture-ci-bot

Copy link
Copy Markdown
Contributor

你好,这是对 libdkapture#119 fix(syscall-stat): stop skipping syscall key 0 during stats 的评审。

本轮为 DKapture 两仓(libdkapture / dkapture-bpf)全部以 fix 开头 open PR 的批量评审,共 17 个;汇总表如下,本 PR 加粗。逐项详评见分隔线下方。

PR 标题 作者 规模 结论 主要风险
#149 fix(net-traffic): initialize rules before loadin yuKing123-king +19/-9 可合并 规则安装顺序与其它工具约定不一致;本仓改动无正确性缺陷
#147 fix(net-filter): abort rule loading on invalid c yuKing123-king +9/-3 需修改 失败时 clear_rules() 波及 add_rule() 装入的规则;空白注释行导致整个加载失败
#141 fix(dkapture): honor parsed pid in read(vector<p JoeSergen +23/-2 需修改 unsafe_find 单记录语义使 /proc//fd 只回调一次;返回值语义漂移
#140 fix(dkapture): return bytes read, not remaining JoeSergen +10/-4 可合并 无阻塞风险;与文档契约对齐,仓内无调用方依赖旧语义
#114 fix: replace manual destructor calls in construc JoeSergen +28/-7 需修改 捆绑了与 open PR #112 逐字节相同的 fs_watch 修复,跨 PR 重复
#125 fix(lsof): start ringbuf consumer before iterato yuKing123-king +17/-3 需修改 线程启动后错误路径仍 goto err_out → 释放运行中 rb(UAF)+ 线程泄漏
#112 fix: fs_watch calls trace_file_init instead of m JoeSergen +1/-1 可合并 无;仅修一处复制粘贴错误
#103 fix: make power-snoop internal symbols static fo yuKing123-king +3/-2 可合并 改动无害;但 PR 描述的 multiple definition 在当前构建配置下无法复现
#119 fix(syscall-stat): stop skipping syscall key 0 d yuKing123-king +18/-6 可合并 循环终止四条路径已逐一核实;缩进与提交拆分小问题
#115 fix(syscall-stat): improve builtin flow yuKing123-king +204/-41 需修改 三处 bpf_get_map_fd 错误路径未设 ret,最终 return ret 误报成功
#117 fix(trace-signal): validate invalid command line yuKing123-king +190/-29 需修改 BUILTIN 测试入口未接入 test/Makefile,不可达;register_signal 残留
#102 fix(trace-exec): reject invalid command line arg yuKing123-king +135/-22 需修改 BUILTIN 入口同样未接入构建;-h 退出码 0→1 属未说明的行为变更
#97 fix(so): reuse pinned dkapture bpf objects corre yuKing123-king +1/-1 需修改 test mock 仍按 map- 前缀命名 pin,合并后 gtest FindMap 用例失败
#35 fix(pagefault): add max_entries and value_size f yuKing123-king +7/-3 可合并 修复真实,但已被 main 上等效修复 1e90486 取代,建议确认后关闭
#32 fix(peek-fd): correct args field name from mvlen yuKing123-king +1/-1 可合并 无功能风险;标题/描述与实际改动方向不符
#30 fix(trace-signal): avoid inflight event key coll yuKing123-king +36/-22 需修改 sys_exit_kill 的 !rule 早退路径仍泄漏 inflight 条目
#29 fix(syscall-stat): replace exec fexit with kprob yuKing123-king +93/-13 阻塞 exec 路径从 struct filename* 本身读字符串,-f 过滤将完全失效

本 PR 评审详情

总体结论: 两项修复均成立。key 0 语义核实:include/syscall-tbl.h:8 为 __SYSCALL(0, read),0..453 编号连续无空洞(实测无 GAP),syscall 0 是有效键;旧代码 observe/syscall-stat.cpp:291 以 key=0 调 get_next_key 从键 1 起遍历,键 0(read)统计永远读不到;新代码先以 NULL 取首键(observe/syscall-stat.cpp:294),libbpf 语义正确。“t”选项在基线已注册于 lopts(:80)但 parse_args 无 case,落入 default 使 -t 直接 Usage+exit(-1),新增 case 't'(:187-188)补齐 top 模式。中期 commit 的 lret/ret 循环终止 bug(bot 评论 5264347748)在最终 commit 已修复:四条路径(:299-303、:305-309、:311-316、:319-320)均更新 ret。

主要问题:

  • (P2) observe/syscall-stat.cpp:305-313 (新) — 两个新增分支体用 5 tab 缩进,与 :299-303 的 4 tab 不一致,违反 .clang-format。

次要建议:

  • :294 (新) int ret = 双空格;
  • :299 (新) if (lret < 0) { K&R 风格与 .clang-format Allman 不符;一个 commit 混两个不相关修复,建议拆分;commit message "and no top mode" 措辞含混。

亮点: 顺带给 bpf_map_lookup_elem 加返回值检查(:298-303),避免基线未初始化 info 被使用。

commit message: 符合规范(conventional commits + scope + DCO),仅措辞与拆分见上。

已有讨论: 无人工评论;bot 曾报 Loop Termination Bug,最终 diff 已不存在,视为已解决。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants