Skip to content

fix(trace-exec): reject invalid command line arguments and add support builtin testing - #102

Open
yuKing123-king wants to merge 1 commit into
DKapture:mainfrom
yuKing123-king:test/add-mock-trace-exec-BUILTIN
Open

yuKing123-king wants to merge 1 commit into
DKapture:mainfrom
yuKing123-king:test/add-mock-trace-exec-BUILTIN

Conversation

@yuKing123-king

@yuKing123-king yuKing123-king commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

对本次trace-exec工具修改的内容

  1. 补齐 BUILTIN 测试入口,改为通过返回值处理参数解析结果
  2. 增加工具启动后的运行提示
  3. 增添用户传入异常参数时会报错反馈的功能,原来的代码如下:
void parse_args(int argc, char **argv)
{
	int opt, opt_idx;
	optind = 1;
	std::string sopts = long_opt2short_opt(lopts); // Convert long options to
												   // short options
	while ((opt = getopt_long(argc, argv, sopts.c_str(), lopts, &opt_idx)) > 0)
	{
		switch (opt)
		{
		case 'u': // UID option
			rule.uid = strtol(optarg, NULL, 10);
			break;
		case 'd': // Depth option
			rule.depth = strtol(optarg, NULL, 10);
			break;
		case 'h': // Help option
			Usage(argv[0]);
			exit(0);
			break;
		case 't': // Target path option
			strncpy(rule.target_path, optarg, PATH_MAX);
			rule.target_path[PATH_MAX - 1] = 0;
			break;
		default: // Invalid option
			Usage(argv[0]);
			exit(-1);
			break;
		}
	}
}

当用户传入-d -u的参数是负数或字母或极大值时,不会报错反馈给用户,导致工具状态异常,但是用户侧得不到反馈。现已增加参数异常报错反馈

@yuKing123-king
yuKing123-king force-pushed the test/add-mock-trace-exec-BUILTIN branch from e4ead48 to 129eca4 Compare July 21, 2026 02:37

@xu-lang xu-lang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

加这些为了做什么,解决什么问题

Comment thread observe/trace-exec.cpp Outdated
@yuKing123-king
yuKing123-king force-pushed the test/add-mock-trace-exec-BUILTIN branch from 129eca4 to 86353b9 Compare July 21, 2026 07:52
@yuKing123-king

Copy link
Copy Markdown
Contributor Author

加这些为了做什么,解决什么问题

  • 加 BUILTIN 模式,本质上是在给这些 eBPF 工具补一个“测试驱动入口”,让工具逻辑和真实内核/BPF 环境解耦,因为正常运行时,这些工具依赖: 真正的 BPF skeleton open/load/attach、真正的 ring buffer、真正的内核事件、真实 stdout/文件输出、信号、线程、阻塞轮询,但是这些东西在单测里都不稳定,很多还根本没法精确构造。

  • 所以BUILTIN 模式把“参数解析、规则装载、事件过滤、日志格式化、输出行为”这些核心逻辑保留下来,同时把“事件来源”替换成测试线程可注入的 mock 事件。把原本依赖真实内核事件的 eBPF 工具,改造成“主流程仍然真实、事件来源可控替换”的测试模式,加了 BUILTIN 后,可以让测试走的是xxx_main() 主路径,与正常工具不同的,只是事件不是来自内核,而是测试自己定义事件,然后主动投递。这样才能真正验证:帮助输出、非法参数处理、默认规则、自定义规则写入 filter map、事件是否按规则被过滤、日志是否最终被输出到 stdout 或 outfile等,依据不同工具的功能去测试不同的方面。

@yuKing123-king
yuKing123-king force-pushed the test/add-mock-trace-exec-BUILTIN branch 2 times, most recently from f1d78c1 to bfe1d80 Compare July 31, 2026 03:15
@yuKing123-king yuKing123-king changed the title test: add support builtin testing and improve runtime feedback fix(trace-exec): reject invalid command line arguments and add support builtin testing Jul 31, 2026
Signed-off-by: Wang Yu <wangyu6@uniontech.com>
@dkapture-ci-bot

Copy link
Copy Markdown
Contributor

你好,这是对 libdkapture#102 fix(trace-exec): reject invalid command line arguments and a 的评审。

本轮为 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 评审详情

作者: yuKing123-king | 规模: +135/-22 | 文件: 1
结论: 需修改
主要风险: BUILTIN 测试入口同样未接入构建/测试;另外 -h 的退出码从 0 变为 1,是描述中未说明的 CLI 行为变更。

总体结论: 与 #117 同一模式且对齐度更好:register_signal 正确移除 exit 改为 return(#117 应照此修改),optind=0/opterr=0 无条件设置与 frtp 一致,local_map_info 的 logs 1MB 与 trace-exec.bpf.c:69 一致。参数校验 uid 分支完整(非负、ERANGE、UINT32_MAX 上限),但 'd' depth 分支漏了 ERANGE,溢出输入会被 (uint32_t) 截断成错误值——恰是本 PR 要修的那类问题。同样没有任何测试消费新增入口。

主要问题:

  • (P1) observe/trace-exec.cpp:272-276 (新) — trace_exec_main/trace_exec_init/deinit 无调用方:test/Makefile TARGET 依赖不含 trace-exec.o,test/ 下无 trace-exec-test.cpp。"add support builtin testing" 交付不完整,BUILTIN 代码未被编译、无法验证。
  • (P2) observe/trace-exec.cpp:160-167 (新) — 'd' 校验为 end == optarg || *end != '\0' || val <= 0,缺 errno == ERANGE(同函数 'u' 分支:150 有)。传入 "99999999999999999999999" 时 strtol 返回 LONG_MAX 且通过检查,(uint32_t)val 截断为 4294967295;"4294967297" 静默变 1。
  • (P2) observe/trace-exec.cpp:170-172 (新) — 'h' 由 exit(0) 改为 return 1,main:284 直接 return rettrace-exec -h 退出码从 0 变 1,属未在描述中说明的行为变更,会破坏按退出码判断的脚本;且与本组 fix(trace-signal): validate invalid command line arguments and add tr… #117 的 -h→0 处理互相矛盾。建议 -h 返回 0(或至少两 PR 统一)。

次要建议:

  • observe/trace-exec.cpp:152 (新) — 参数错误信息 printf 到 stdout,frtp 同类用 pr_error,建议统一 stderr/pr_error。
  • observe/trace-exec.cpp:329 (新) — while(!exit_flag) 缺空格,与仓库其余代码/.clang-format 的 while (!exit_flag) 不一致。
  • observe/trace-exec.cpp:324 (新) — 新增 "Tracing exec events... Hit Ctrl-C to end." 提示好,但 fix(trace-signal): validate invalid command line arguments and add tr… #117 未给 trace-signal 同步,同模式 PR 应一致。

亮点:

  • register_signal 干净地以 return -1 替换 exit(EXIT_FAILURE)(frtp.cpp:672-675 同款),错误可上抛。
  • BUILTIN 契约与已合入 frtp.cpp 逐项对齐:init(FILE*, atomic, atomic**, int, int*) 签名、失败置 *condition=2、就绪置 *condition=1、BUILTIN 下 ring_buffer__poll 50ms、忙等退出循环,frtp-test.cpp 的 harness(test/frtp-test.cpp:267-321)可直接复用。
  • uid 校验完整(非负、ERANGE、UINT32_MAX);Usage/long_opt2short_opt static 化落实了行级讨论的符号冲突结论。

commit message: subject "reject invalid command line arguments" 只覆盖半个 PR,未提及 BUILTIN 测试支持与运行提示;与 #117 的 subject 覆盖度不一致。

已有讨论: 4 条行级评论 + 1 条 issue 评论。共识:maintainer (xu-lang) 质疑 #define BUILTIN_LOCAL 的必要性,作者解释 test/Makefile 把多个工具 .o 链接为同一可执行文件,Usage/long_opt2short_opt 等同名全局符号会冲突,最终 head 已改为直接 static;maintainer 另提醒"不要用 AI 回复"。issue 评论为作者自答,阐述 BUILTIN 模式设计意图(保留主流程、mock 事件源)。未决:maintainer 对最终实现无明确 approve。

(补充:两个 PR mergeable_state 均为 unstable [INFERENCE,来自 meta.json,本地无法核实 CI 详情]。本评审全程仅读本地 prdata/ 与仓库源码,无网络访问、无发布动作。)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants