fix(lookup_kallsyms): 修复符号表查询问题

[goldwind-ting]

原有逻辑addr刚好等于kallsyms_adress_list[i+1]时，索引index值会是i，但是实际上是i+1；现在修改为通过二分查找符号
Fixes #1402

[fslongjin]

@claude review this pr in Chinese.
Review this pull request and provide feedback on:
- Code quality and best practices
- Potential bugs or issues
- Performance considerations
- Security concerns
- Test coverage

        After reviewing, leave a constructive comment on the PR.

[dragonos-community-ai-pr-reviewer]

Claude finished @fslongjin's task —— View job

---

Claude 代码审查任务

我已审查了PR #1443关于修复符号表查询问题的代码变更。

待办事项清单：

• 创建任务清单并开始审查
• 查看代码变更和上下文
• 分析修复的逻辑问题
• 检查代码质量和最佳实践
• 评估潜在的性能和安全性问题
• 提供详细的审查反馈
• 总结建议并更新评论

审查总结

正面反馈

1. 正确的性能改进：从O(n)的线性搜索改为O(log n)的二分查找是显著的性能提升，特别是对于大型符号表。
2. 修复了原始逻辑错误：原始代码在addr == kallsyms_address_list[i+1]时会错误地返回i而不是i+1，新实现通过binary_search的Ok(i)路径正确解决了这个问题。
3. 代码结构清晰：使用match表达式处理二分查找结果使意图更明确。

发现的问题

1. 边界情况处理有缺陷

问题位置: kernel/src/debug/traceback/mod.rs:26-36

当前实现：

let index = match kallsyms_address_list.binary_search(&addr) {
    Ok(i) => i,
    Err(i) => {
        // 符号不存在
        if i == sym_num {
            i
        } else {
            i - 1  // 这里有bug！
        }
    },
};

问题：

• 当binary_search返回Err(0)时（地址小于所有符号地址），执行i - 1会导致无符号整数下溢（0usize - 1），结果是usize::MAX。
• 后续的if index < sym_num检查可能为真（如果usize::MAX < sym_num），导致越界访问。
• 实际上，当i == 0时，表示addr < kallsyms_address_list[0]，没有匹配的符号，应该返回sym_num（无效索引）。

2. 空符号表处理

问题位置: kernel/src/debug/traceback/mod.rs:56

addr - kallsyms_address_list[sym_num - 1]

如果sym_num == 0（空符号表）：

1. kallsyms_address_list是空切片
2. binary_search(&addr)返回Err(0)
3. 根据当前逻辑，i == sym_num为真（0 == 0），返回i = 0
4. index < sym_num为假（0 < 0）
5. 执行else分支，访问kallsyms_address_list[sym_num - 1]即kallsyms_address_list[0usize - 1]，导致panic

3. 逻辑不一致性

当binary_search返回Err(sym_num)时（地址大于所有符号地址），当前代码返回i（即sym_num）。这虽然不会导致index < sym_num为真，但index值实际上是无效的。更好的做法是返回sym_num - 1，与函数语义（找到地址≤目标地址的最大符号）一致。

改进建议

1. 修复边界情况处理

let index = match kallsyms_address_list.binary_search(&addr) {
    Ok(i) => i,  // 精确匹配
    Err(i) => {
        // i 是插入位置
        match i {
            0 => sym_num,        // 地址小于所有符号：无匹配
            _ => i - 1,          // 返回前一个符号的索引
        }
    },
};

或者更清晰地：

let index = match kallsyms_address_list.binary_search(&addr) {
    Ok(i) => i,
    Err(0) => sym_num,           // 地址小于第一个符号
    Err(i) if i == sym_num => i - 1,  // 地址大于最后一个符号
    Err(i) => i - 1,             // 地址在两个符号之间
};

2. 添加空符号表检查

在函数开始处添加：

if sym_num == 0 {
    println!("[{}] function:unknown \t(+) {:04} address:{:#018x}", level, 0, addr);
    return false;
}

3. 改进错误处理

考虑将unwrap()改为更安全的处理，虽然在内核环境中panic可能是可接受的：

let sym_name = match CStr::from_ptr(sym_names.add(sym_names_index[index] as usize) as _)
    .to_str() 
{
    Ok(name) => name,
    Err(_) => "<invalid-utf8>",
};

4. 小改进

• 第60行：使用表达式is_kernel_main而不是return is_kernel_main;
• 考虑为sym_num == 0的情况添加单元测试

性能考虑

• 二分查找比线性搜索有显著性能优势（O(log n) vs O(n)）
• 符号表假设已排序，注释说明"由于符号表使用nm -n生成，因此是按照地址升序排列的"，这个假设是合理的
• 如果未来符号表可能未排序，需要添加验证或回退到线性搜索

安全性

• 函数标记为unsafe是恰当的，因为使用了原始指针和from_raw_parts
• 边界检查index < sym_num是必要的，但当前实现在边界情况下可能失效

建议的修改

基于以上分析，建议修复边界情况处理，特别是Err(0)的情况。修复后，代码将正确处理所有边界情况，同时保持性能改进。

---

注意：以上是代码审查反馈。如果需要我帮助实现这些修复，请告知。

[fslongjin]

@goldwind-ting ping~ hello, there are some issue in this PR~

[goldwind-ting]

@goldwind-ting ping~ hello, there are some issue in this PR~

重新修改了

[fslongjin]

@Godones hey哥们，可以麻烦帮忙看看这里的吗~

[Godones]

@goldwind-ting 如果地址大于最后一个符号，也返回sym_num比较合理

[fslongjin]

• lookup_kallsyms 现在对“地址大于最后一个符号”的情况会返回最后一个符号并打印偏移（Err(sym_num) => sym_num - 1），之前是落入未知分支。这会把超出符号范围的地址误标成最后一个符号，可能掩盖缺失的符号或溢出问题。建议在二分后显式判断 addr > kallsyms_address_list[sym_num - 1] 时仍走未知分支。
• 既有问题仍在：当 addr 小于首符号时，未知分支里的偏移打印使用 addr - kallsyms_address_list[sym_num - 1]，会发生大幅度的无符号下溢，日志偏移值不可读。

[goldwind-ting]

• lookup_kallsyms 现在对“地址大于最后一个符号”的情况会返回最后一个符号并打印偏移（Err(sym_num) => sym_num - 1），之前是落入未知分支。这会把超出符号范围的地址误标成最后一个符号，可能掩盖缺失的符号或溢出问题。建议在二分后显式判断 addr > kallsyms_address_list[sym_num - 1] 时仍走未知分支。
• 既有问题仍在：当 addr 小于首符号时，未知分支里的偏移打印使用 addr - kallsyms_address_list[sym_num - 1]，会发生大幅度的无符号下溢，日志偏移值不可读。

1. kallsyms_address_list[sym_num - 1] 不是最后一个符号开始的地址吗？严格说得获取到最后一个符号对应函数结尾的地址
2. 可以加个判断是不是小于首符号地址

如果可以获取最后一个符号对应函数结尾的地址sym_end_addr，可以在函数开始判断下：

if addr < kallsyms_address_list[sym_num - 1] || addr > sym_end_addr {
        println!("function:unknown.....");
        return false;
}

[fslongjin]

• lookup_kallsyms 现在对“地址大于最后一个符号”的情况会返回最后一个符号并打印偏移（Err(sym_num) => sym_num - 1），之前是落入未知分支。这会把超出符号范围的地址误标成最后一个符号，可能掩盖缺失的符号或溢出问题。建议在二分后显式判断 addr > kallsyms_address_list[sym_num - 1] 时仍走未知分支。
• 既有问题仍在：当 addr 小于首符号时，未知分支里的偏移打印使用 addr - kallsyms_address_list[sym_num - 1]，会发生大幅度的无符号下溢，日志偏移值不可读。

1. kallsyms_address_list[sym_num - 1] 不是最后一个符号开始的地址吗？严格说得获取到最后一个符号对应函数结尾的地址
2. 可以加个判断是不是小于首符号地址

如果可以获取最后一个符号对应函数结尾的地址sym_end_addr，可以在函数开始判断下：

if addr < kallsyms_address_list[sym_num - 1] || addr > sym_end_addr {
        println!("function:unknown.....");
        return false;
}

qs应该是可行的

[Godones]

• lookup_kallsyms 现在对“地址大于最后一个符号”的情况会返回最后一个符号并打印偏移（Err(sym_num) => sym_num - 1），之前是落入未知分支。这会把超出符号范围的地址误标成最后一个符号，可能掩盖缺失的符号或溢出问题。建议在二分后显式判断 addr > kallsyms_address_list[sym_num - 1] 时仍走未知分支。
• 既有问题仍在：当 addr 小于首符号时，未知分支里的偏移打印使用 addr - kallsyms_address_list[sym_num - 1]，会发生大幅度的无符号下溢，日志偏移值不可读。

1. kallsyms_address_list[sym_num - 1] 不是最后一个符号开始的地址吗？严格说得获取到最后一个符号对应函数结尾的地址
2. 可以加个判断是不是小于首符号地址

如果可以获取最后一个符号对应函数结尾的地址sym_end_addr，可以在函数开始判断下：

if addr < kallsyms_address_list[sym_num - 1] || addr > sym_end_addr {
        println!("function:unknown.....");
        return false;
}

实际上最后一个符号是__etext，kallsyms_address_list[sym_num - 1] 是__etext的起始地址，也是结束地址

[fslongjin]

LGTM

[fslongjin]

你好！感谢你的贡献！该PR看起来解决了问题！不过在合入之前，需要您对您的github账号做一个小更改：

dragonos社区需要commit可追溯（以便联系到开发者），因此需要麻烦您关闭匿名邮箱功能，方法请见：
https://community.dragonos.org/contributors/cheat-sheet.html

一旦您关闭了这个功能，我会尽快把PR合入～感谢

[goldwind-ting]

你好！感谢你的贡献！该PR看起来解决了问题！不过在合入之前，需要您对您的github账号做一个小更改：

dragonos社区需要commit可追溯（以便联系到开发者），因此需要麻烦您关闭匿名邮箱功能，方法请见： https://community.dragonos.org/contributors/cheat-sheet.html

一旦您关闭了这个功能，我会尽快把PR合入～感谢

ok