已开启
Stack parsing code optimization and UT additions #20313
Stack parsing code optimization and UT additions #20313
已开启
rentangyu创建于 2 天前
rentangyu
rentangyu
2 天前

IssueNo:
https://gitcode.com/openharmony/arkcompiler_ets_runtime/issues/13612?ref=&did=4282824#tid-4282824
Description:
Stack parsing code optimization and UT additions
稳定性自检:

自检项 自检结果
涉及跨进程调用的相关操作需要抛至主线程或加锁防止并发
成员变量进行赋值或创建需要排查并发
谨慎在lambda表达式中使用引用捕获
谨慎在未经拷贝的情况下使用外部传入的string、C字符串
map\vector\list\set等stl模板类使用时需要排查并发
谨慎考虑加锁范围
在IPC通信中谨慎使用同步通信方式
禁止传递this指针至其他模块或线程(特别是eventhandler任务)
禁止将外部传入的裸指针在内部直接构造智能指针
禁止多个独立创建的智能指针管理同一地址
禁止在析构函数中抛异步任务
禁止js对象在非js线程(例如在IPC线程)创建、使用或销毁
禁止在对外接口中未经判空直接使用外部传入的指针
禁止接口返回局部变量引用
禁止在信号函数中加锁
禁止在关键流程(SA启动、应用启动等主流程)执行耗时的操作
禁止将同一个cpp编译在不同的so中

安全编码自检:

自检项 自检结果
裸指针避免通过隐式转换构造为sptr
json对象在取值之前必须先判断类型,避免类型不匹配
序列化时必须对传入的数组大小进行校验,避免出现超大数组
避免使用未明确位宽的整型,选择使用int8_t、uint8_t等类型
外部传入的路径要做规范化校验,对路径中的.、..、../等特殊字符严格校验
指针变量、表示资源描述符的变量、bool变量必须赋初值
readParcelable获取的对象使用前需要判空
分配和释放内存的函数需要成对出现
申请内存后异常退出前需要及时进行内存释放
内存申请前必须对内存大小进行合法性校验
内存分配后必须判断是否成功
禁止使用realloc、alloca函数
禁止打印文件路径、口令等敏感信息,如有需要,使用private修饰
禁止打印内存地址
整数之间运算时必须严格检查,确保不会出现溢出、反转、除0
禁止对有符号整数进行位操作符运算
禁止对指针进行逻辑或位运算
循环次数如果收外部数据控制,需要检验其合法性
禁止使用内存操作类危险函数,需要使用安全函数
谨慎使用不可重入函数
必须检查安全函数的返回值,并进行正确处理
禁止仅通过TokenType类型判断绕过权限校验

TDD Result:

XTS Result:

是否已执行L0用例

AI检视评分(使用本地代码检视skills扫描):

Change-Id: I5f73699f93427756d7e2c739504a19b81fc4fdaa

likedislike
合并受阻
rentangyurentangyu
2 天前 关联了issue:[Bug]: 堆栈解析代码优化和ut补充
afwk_helper成员
2 天前 评论:

开始进行AI检视!

AI review has been started, please wait...

likedislike
afwk_helper成员
2 天前 评论:
check type result report
start ai_review pass -
likedislike
openharmony_ciopenharmony_ci成员
2 天前 添加了label:waiting_on_author
openharmony_ci
openharmony_ci成员
2 天前 评论:

感谢提交 Pull Requests!如果您提交的PR已经开发完毕,请评论 "start build" 触发门禁,更多交互操作,请访问OpenHarmony社区支持命令清单。如果需要调整订阅PR、Issue的变更状态,请访问订阅链接


Thanks for submitting the pull request. If your Pull Request has already been developed, you can leave a "start build" comment to trigger the gated system. For more commands, please visit OpenHarmony Command List. If you need to change the subscription of a Pull Request or Issue, please visit the link.

likedislike
openharmony_ciopenharmony_ci成员
2 天前 添加了label:dco检查成功
afwk_helper成员
2 天前 评论:

⚠️ 🤖 AI 代码检视报告 ⚠️

总体评估: NEEDS_ATTENTION

问题统计:

  • 总问题数: 3
  • 严重问题: 0
  • 高危问题: 2

摘要:
代码在堆栈解析和状态管理上做了优化并补充了UT,但存在全局状态在循环内赋值导致的逻辑隐患,以及测试用例中使用sleep_for同步异步任务的可靠性问题,需要进一步修复。

📊 详细报告
查看完整的审查详情,包括具体的问题描述、建议和代码位置:
🔗 查看详细报告


此评论由 OpenHarmony Insight 代码审查系统自动生成

likedislike
afwk_helper成员2 天前进行代码检视1
frameworks/native/runtime/js_runtime.cpp
@@ -1019,3 +1018,1 @@
1019- std::string sourceMapData;
1020- JsRuntime::ReadSourceMapData(hapInfo, MERGE_SOURCE_MAP_PATH, sourceMapData);
1021- DFXJSNApi::SourceMapSplitSourceMap(sourceMapData);
1018+ if (hapInfo.empty()) {
afwk_helper2 天前评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🟠 全局状态 IsRelease 在循环内被反复覆盖,存在逻辑隐患

位置: L1018-L1027 | 严重程度: High

❓ 问题描述

在遍历 hapList 的循环内部,代码根据 ReadSourceMapData 的结果调用 DFXJSNApi::SourceMapSetIsRelease 修改全局状态。如果列表中包含多个 HAP(例如一个调试应用依赖了一个发布模式的共享包),全局标志的最终值将完全取决于 HAP 的迭代顺序,最后一个处理的 HAP 会覆盖之前的状态。此外,如果 hapList 为空,IsRelease 将保持旧值不变,可能导致使用了过期的全局状态。

💡 修复建议

修改建议:避免在循环内部修改全局的 IsRelease 状态。应在循环外部确定当前 bundle 的发布状态,或者仅在循环结束后根据综合判断结果设置一次全局标志。

1018: if (hapInfo.empty()) {
1019: continue;
1020: }
1021: std::string sourceMapData;
1022: if (JsRuntime::ReadSourceMapData(hapInfo, MERGE_SOURCE_MAP_PATH, sourceMapData)) {
1023: // Process source map data
1024: DFXJSNApi::SourceMapSplitSourceMap(sourceMapData);
1025: }
1026: // Removed global state modification inside the loop


likedislike
afwk_helper成员2 天前进行代码检视1
test/unittest/runtime_test/js_runtime_test.cpp
@@ -2455,0 +2468,4 @@
2468+ DFXJSNApi::SourceMapSetInitStatus(panda::ecmascript::InitStatus::NOT_EXECUTED);
2469+ DFXJSNApi::SourceMapSetIsRelease(false);
2470+ 
2471+ jsRuntime->InitSourceMap(bundleName);
afwk_helper2 天前评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🟠 使用 sleep_for 同步异步任务存在可靠性风险

位置: L2471-L2487 | 严重程度: High

❓ 问题描述

测试用例通过 std::this_thread::sleep_for 固定睡眠时间来等待 ffrt 异步任务完成。这是一种典型的测试反模式:在性能较弱的 CI 机器上任务可能未完成,导致误报(false negative);在性能强的机器上又会浪费时间。且在 jsRuntime.reset() 后再次 sleep_for 试图等待对象销毁,属于不可靠的同步方式。

💡 修复建议

修改建议:不要使用 sleep_for 等待异步任务,应使用 ffrt 提供的任务等待机制,或通过 Mock 框架将异步调用转为同步执行,确保测试的确定性和高效性。

2471: jsRuntime->InitSourceMap(bundleName);
2472: // Wait for ffrt task to complete synchronously or via ffrt::wait
2473: // std::this_thread::sleep_for(std::chrono::milliseconds(500));
2474:
2475: // In test env, GetHapPathList returns empty list -> for-loop does not execute
2476: // -> IsRelease remains false (never set in the loop)
2477: // -> TranslateBySourceMap does NOT return NOT_FOUNDMAP prefix
2478: std::string stackStr = "at func (entry/src/main/ets/pages/Index.ts:10:5)";
2479: std::string result = DFXJSNApi::SourceMapTranslateBySourceMap(stackStr);
2480: EXPECT_EQ(result.find("Cannot get SourceMap info"), std::string::npos);
2481:
2482: // Cleanup
2483: DFXJSNApi::SourceMapSetInitStatus(panda::ecmascript::InitStatus::NOT_EXECUTED);
2484: DFXJSNApi::SourceMapSetIsRelease(false);
2485: jsRuntime.reset();
2486: // std::this_thread::sleep_for(std::chrono::milliseconds(200));
2487: TAG_LOGI(AAFwkTag::TEST, "InitSourceMap_0400 end");


likedislike
afwk_helper成员2 天前进行代码检视1
test/unittest/runtime_test/js_runtime_test.cpp
@@ -2455,0 +2461,4 @@
2461+ 
2462+ auto jsRuntime = std::make_unique<JsRuntime>();
2463+ ASSERT_NE(jsRuntime, nullptr);
2464+ jsRuntime->jsEnv_ = std::make_shared<JsEnv::JsEnvironment>();
afwk_helper2 天前评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🟡 直接操作内部成员变量 jsEnv_ 破坏了封装性

位置: L2464-L2465 | 严重程度: Medium

❓ 问题描述

测试代码直接赋值 jsRuntime->jsEnv_,如果该成员是 privateprotected,这通常意味着测试代码使用了 #define private public 之类的宏 hack,这会导致未定义行为(UB);如果该成员是 public,则说明 JsRuntime 的设计缺乏封装,暴露了不必要的内部实现细节。

💡 修复建议

修改建议:避免直接操作内部成员变量,应通过 JsRuntime 提供的公共接口(如初始化函数或 Mock 注入方式)来设置环境,保持良好的封装性。

2464: // Use public API to initialize JsEnvironment instead of direct member access
2465: // jsRuntime->jsEnv_ = std::make_sharedJsEnv::JsEnvironment();


likedislike
rentangyurentangyu
2 天前 修改了pull request 的描述