From 553c1a767c868e87f3736b1c70c96da1111c1e6b Mon Sep 17 00:00:00 2001 From: zhangsizheng Date: Tue, 20 May 2025 16:45:31 +0800 Subject: [PATCH] Bug fix for scopeinfo Signed-off-by: zhangsizheng --- src/js_native_api_v8.cpp | 38 +++++++++++++++++++---- src/js_native_api_v8.h | 46 +++++++++++++++++----------- src/jsvm_dfx.h | 60 +++++++++++++++++++++---------------- test/unittest/test_jsvm.cpp | 32 +++++++++++++++++--- 4 files changed, 123 insertions(+), 53 deletions(-) diff --git a/src/js_native_api_v8.cpp b/src/js_native_api_v8.cpp index da0d1de..d7b6aa2 100644 --- a/src/js_native_api_v8.cpp +++ b/src/js_native_api_v8.cpp @@ -2855,9 +2855,13 @@ JSVM_Status OH_JSVM_GetCbInfo(JSVM_Env env, // [in] JSVM environment *data = info->Data(); } - if (argv != nullptr) { - for (int i = 0; i <= *argc; i++) { - ADD_VAL_TO_SCOPE_CHECK(env, argv[i]); + if (UNLIKELY(env->debugFlags)) { + if (UNLIKELY(env->debugFlags & (1 << JSVM_SCOPE_CHECK))) { + if (argv != nullptr) { + for (int i = 0; i <= *argc; i++) { + ADD_VAL_TO_SCOPE_CHECK(env, argv[i]); + } + } } } @@ -2902,6 +2906,16 @@ JSVM_Status OH_JSVM_CallFunction(JSVM_Env env, auto maybe = v8func->Call(context, v8recv, argc, reinterpret_cast*>(const_cast(argv))); + if (UNLIKELY(env->debugFlags)) { + if (UNLIKELY(env->debugFlags & (1 << JSVM_SCOPE_CHECK))) { + if (argv != nullptr) { + for (int i = 0; i <= argc; i++) { + ADD_VAL_TO_SCOPE_CHECK(env, argv[i]); + } + } + } + } + RETURN_IF_EXCEPTION_HAS_CAUGHT(env); if (result != nullptr) { @@ -3620,6 +3634,13 @@ JSVM_Status OH_JSVM_OpenEscapableHandleScope(JSVM_Env env, JSVM_EscapableHandleS *result = v8impl::JsEscapableHandleScopeFromV8EscapableHandleScope(new v8impl::EscapableHandleScopeWrapper(env->isolate)); env->openHandleScopes++; + + if (UNLIKELY(env->debugFlags)) { + if (UNLIKELY(env->debugFlags & (1 << JSVM_SCOPE_CHECK))) { + env->GetScopeTracker()->IncHandleScopeDepth(); + } + } + return ClearLastError(env); } @@ -3635,6 +3656,14 @@ JSVM_Status OH_JSVM_CloseEscapableHandleScope(JSVM_Env env, JSVM_EscapableHandle delete v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope); env->openHandleScopes--; + + if (UNLIKELY(env->debugFlags)) { + if (UNLIKELY(env->debugFlags & (1 << JSVM_SCOPE_CHECK))) { + env->GetScopeTracker()->ReleaseJSVMVals(); + env->GetScopeTracker()->DecHandleScopeDepth(); + } + } + return ClearLastError(env); } @@ -3662,10 +3691,9 @@ JSVM_Status OH_JSVM_EscapeHandle(JSVM_Env env, JSVM_EscapableHandleScope scope, v8impl::EscapableHandleScopeWrapper* s = v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope); if (!s->IsEscapeCalled()) { *result = v8impl::JsValueFromV8LocalValue(s->Escape(v8impl::V8LocalValueFromJsValue(escapee))); - ADD_VAL_TO_SCOPE_CHECK(env, *result); + ADD_VAL_TO_ESCAPE_SCOPE_CHECK(env, *result); return ClearLastError(env); } - ADD_VAL_TO_SCOPE_CHECK(env, *result); return SetLastError(env, JSVM_ESCAPE_CALLED_TWICE); } diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index af738f2..8e3f165 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -190,26 +190,36 @@ inline JSVM_Status SetLastError(JSVM_Env env, #define JSVM_PRIVATE_KEY(isolate, suffix) (v8impl::GetIsolateData(isolate)->suffix##Key.Get(isolate)) -#define ADD_VAL_TO_SCOPE_CHECK(env, val) \ - do { \ - if (UNLIKELY((env)->debugFlags)) { \ - if (UNLIKELY((env)->debugFlags & (1 << JSVM_SCOPE_CHECK)) && (val)) { \ - LOG(Info) << "ADD_VAL_TO_SCOPE_CHECK in function: " << __func__; \ - (env)->GetScopeTracker()->AddJSVMVal(val); \ - } \ - } \ +#define ADD_VAL_TO_SCOPE_CHECK(env, val) \ + do { \ + if (UNLIKELY((env)->debugFlags)) { \ + if (UNLIKELY((env)->debugFlags & (1 << JSVM_SCOPE_CHECK)) && (val)) { \ + LOG(Info) << "ADD_VAL_TO_SCOPE_CHECK in function: " << __func__; \ + (env)->GetScopeTracker()->AddJSVMVal(val); \ + } \ + } \ } while (0) -#define CHECK_SCOPE(env, val) \ - do { \ - if (UNLIKELY((env)->debugFlags)) { \ - if (UNLIKELY((env)->debugFlags & (1 << JSVM_SCOPE_CHECK)) && (val)) { \ - LOG(Info) << "CHECK_SCOPE in function: " << __func__; \ - if (!(env)->GetScopeTracker()->CheckJSVMVal(val)) { \ - JSVM_FATAL("Run in wrong HandleScope"); \ - } \ - } \ - } \ +#define ADD_VAL_TO_ESCAPE_SCOPE_CHECK(env, val) \ + do { \ + if (UNLIKELY((env)->debugFlags)) { \ + if (UNLIKELY((env)->debugFlags & (1 << JSVM_SCOPE_CHECK)) && (val)) { \ + LOG(Info) << "ADD_VAL_TO_SCOPE_CHECK in function: " << __func__; \ + (env)->GetScopeTracker()->AddJSVMVal(val, true); \ + } \ + } \ + } while (0) + +#define CHECK_SCOPE(env, val) \ + do { \ + if (UNLIKELY((env)->debugFlags)) { \ + if (UNLIKELY((env)->debugFlags & (1 << JSVM_SCOPE_CHECK)) && (val)) { \ + LOG(Info) << "CHECK_SCOPE in function: " << __func__; \ + if (!(env)->GetScopeTracker()->CheckJSVMVal(val)) { \ + JSVM_FATAL("Run in wrong HandleScope"); \ + } \ + } \ + } \ } while (0) #define STATUS_CALL(call) \ diff --git a/src/jsvm_dfx.h b/src/jsvm_dfx.h index b45efd2..e4ce8b2 100644 --- a/src/jsvm_dfx.h +++ b/src/jsvm_dfx.h @@ -17,7 +17,7 @@ #define JSVM_DFX_H #include -#include +#include #include #include "jsvm_log.h" @@ -28,6 +28,13 @@ // v8 header #include "v8.h" +#define JSVM_FATAL(message) \ + do { \ + /* Make sure that this struct does not end up in inline code, but */ \ + /* rather in a read-only data section when modifying this code. */ \ + jsvm::OnFatalError(STRINGIFY(__FILE__) ":" STRINGIFY(__LINE__) " ", STRINGIFY(message)); \ + } while (0) + namespace jsvm { [[noreturn]] inline void OnFatalError(const char* location, const char* message) { @@ -56,43 +63,51 @@ class ScopeLifecycleTracker { public: uint32_t GetCurrentScopeDepth() const { - return currentScopeDepth; + return scopeDepthToVal.size(); } void IncHandleScopeDepth() { - currentScopeDepth++; + scopeDepthToVal.push_back(std::vector()); } void DecHandleScopeDepth() { - currentScopeDepth--; + scopeDepthToVal.pop_back(); } void ReleaseJSVMVals() { - if (scopeDepthToVal.find(currentScopeDepth) != scopeDepthToVal.end()) { - for (auto item : scopeDepthToVal[currentScopeDepth]) { - valToScopeDepth.erase(item); - } + if (scopeDepthToVal.size() == 0) { + JSVM_FATAL("Unpaired HandleScope detected after scope check is enabled!"); } - - if (currentScopeDepth > 0) { - scopeDepthToVal.erase(currentScopeDepth); + for (auto item : scopeDepthToVal[scopeDepthToVal.size() - 1]) { + addedVal.erase(item); } } - void AddJSVMVal(JSVM_Value val) + void AddJSVMVal(JSVM_Value val, bool isEscape = false) { - valToScopeDepth[val] = currentScopeDepth; - scopeDepthToVal[currentScopeDepth].push_back(val); + if (scopeDepthToVal.size() == 0) { + JSVM_FATAL("Unpaired HandleScope detected after scope check is enabled!"); + } + addedVal.insert(val); + if (!isEscape) { + // Add JSVM value to current depth + scopeDepthToVal[scopeDepthToVal.size() - 1].push_back(val); + } else { + // Add JSVM value to parent depth + if (scopeDepthToVal.size() - 2 < 0) { + JSVM_FATAL("Not in any scope!"); + } + scopeDepthToVal[scopeDepthToVal.size() - 2].push_back(val); + } } bool CheckJSVMVal(JSVM_Value val) { - auto values = scopeDepthToVal[currentScopeDepth]; - auto it = std::find(values.begin(), values.end(), val); - if (it != values.end()) { + auto it = addedVal.find(val); + if (it != addedVal.end()) { return true; } else { return false; @@ -100,19 +115,12 @@ public: } private: - uint32_t currentScopeDepth = 0; - std::unordered_map valToScopeDepth; - std::unordered_map> scopeDepthToVal; + std::unordered_set addedVal; + std::vector> scopeDepthToVal; }; } // namespace jsvm -#define JSVM_FATAL(message) \ - do { \ - /* Make sure that this struct does not end up in inline code, but */ \ - /* rather in a read-only data section when modifying this code. */ \ - jsvm::OnFatalError(STRINGIFY(__FILE__) ":" STRINGIFY(__LINE__) " ", STRINGIFY(message)); \ - } while (0) #define UNREACHABLE(...) JSVM_FATAL("Unreachable code reached" __VA_OPT__(": ") __VA_ARGS__) diff --git a/test/unittest/test_jsvm.cpp b/test/unittest/test_jsvm.cpp index 107de67..bcbc25f 100644 --- a/test/unittest/test_jsvm.cpp +++ b/test/unittest/test_jsvm.cpp @@ -1532,7 +1532,8 @@ HWTEST_F(JSVMTest, test_set_debug_option1, TestSize.Level1) int num = 100; bool boolValue = false; JSVM_Value array1[num], array2[num], array3[num]; - JSVMTEST_CALL(OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, true)); + auto status1 = OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, true); + ASSERT_TRUE(status1 == JSVM_OK); OH_JSVM_OpenHandleScope(env, &handleScope1); for (int i = 0; i < num; i++) { OH_JSVM_GetBoolean(env, false, &array1[i]); @@ -1547,11 +1548,13 @@ HWTEST_F(JSVMTest, test_set_debug_option1, TestSize.Level1) for (int i = 0; i < num; i++) { OH_JSVM_GetBoolean(env, false, &array3[i]); OH_JSVM_IsBoolean(env, array3[i], &boolValue); + OH_JSVM_IsBoolean(env, array2[i], &boolValue); } OH_JSVM_CloseHandleScope(env, handleScope3); OH_JSVM_CloseHandleScope(env, handleScope2); OH_JSVM_CloseHandleScope(env, handleScope1); - JSVMTEST_CALL(OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, false)); + auto status2 = OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, false); + ASSERT_TRUE(status2 == JSVM_OK); } static bool g_fatalErrorFinished = false; @@ -1567,7 +1570,8 @@ HWTEST_F(JSVMTest, test_set_debug_option2, TestSize.Level1) JSVM_HandleScope handleScope; JSVM_Value result; bool boolValue = false; - JSVMTEST_CALL(OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, true)); + auto status1 = OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, true); + ASSERT_TRUE(status1 == JSVM_OK); OH_JSVM_OpenHandleScope(env, &handleScope); OH_JSVM_GetBoolean(env, true, &result); OH_JSVM_CloseHandleScope(env, handleScope); @@ -1578,6 +1582,26 @@ HWTEST_F(JSVMTest, test_set_debug_option2, TestSize.Level1) fataled = true; OH_JSVM_IsBoolean(env, result, &boolValue); } - JSVMTEST_CALL(OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, false)); + auto status2 = OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, false); + ASSERT_TRUE(status2 == JSVM_OK); ASSERT_TRUE(g_fatalErrorFinished); +} + +HWTEST_F(JSVMTest, test_set_debug_option3, TestSize.Level1) +{ + JSVM_HandleScope handleScope; + JSVM_Value val, escapeVal; + bool boolValue = false; + auto status1 = OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, true); + ASSERT_TRUE(status1 == JSVM_OK); + OH_JSVM_OpenHandleScope(env, &handleScope); + JSVM_EscapableHandleScope scope = nullptr; + OH_JSVM_OpenEscapableHandleScope(env, &scope); + OH_JSVM_GetBoolean(env, true, &val); + OH_JSVM_EscapeHandle(env, scope, val, &escapeVal); + OH_JSVM_CloseEscapableHandleScope(env, scope); + OH_JSVM_IsBoolean(env, escapeVal, &boolValue); + OH_JSVM_CloseHandleScope(env, handleScope); + auto status2 = OH_JSVM_SetDebugOption(env, JSVM_SCOPE_CHECK, false); + ASSERT_TRUE(status2 == JSVM_OK); } \ No newline at end of file -- Gitee