Skip to content

Commit 75021b9

Browse files
author
ci_lynx
committed
[BugFix][Debugger] Fix incorrect handling of sourceURL in script parsing.
1. In `DebuggerParseScript`, always create a new script instead of reusing one by matching the sourceURL with `LEPUSScriptSource.url`. 2. Find sourceURL only when necessary. 3. Remove useless code.
1 parent 5deb977 commit 75021b9

File tree

2 files changed

+128
-41
lines changed

2 files changed

+128
-41
lines changed

src/inspector/debugger/debugger.cc

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -591,14 +591,21 @@ QJS_HIDE char *DebuggerSetScriptHash(LEPUSContext *ctx, const char *src,
591591
}
592592

593593
QJS_STATIC void SetScriptUrl(LEPUSContext *ctx, const char *filename,
594-
LEPUSScriptSource *script, char *source_url) {
594+
LEPUSScriptSource *script) {
595595
script->url = NULL;
596596
if (*filename) {
597597
script->url = lepus_strdup(ctx, filename, ALLOC_TAG_WITHOUT_PTR);
598598
} else if (script->source) {
599-
script->url = source_url
600-
? lepus_strdup(ctx, source_url, ALLOC_TAG_WITHOUT_PTR)
601-
: lepus_strdup(ctx, "", ALLOC_TAG_WITHOUT_PTR);
599+
char *source_url = FindDebuggerMagicContent(ctx, (char *)script->source,
600+
(char *)"sourceURL", 0);
601+
if (source_url) {
602+
script->url = lepus_strdup(ctx, source_url, ALLOC_TAG_WITHOUT_PTR);
603+
if (!ctx->gc_enable) {
604+
lepus_free(ctx, source_url);
605+
}
606+
} else {
607+
script->url = lepus_strdup(ctx, "", ALLOC_TAG_WITHOUT_PTR);
608+
}
602609
}
603610
return;
604611
}
@@ -635,19 +642,6 @@ QJS_STATIC bool IsDebuggerFile(const char *filename) {
635642
return filename && strcmp(filename, "quickjsTriggerTimer.js") != 0;
636643
}
637644

638-
QJS_STATIC LEPUSScriptSource *FindDebuggerScript(LEPUSContext *ctx,
639-
char *source_url) {
640-
struct list_head *el, *el1;
641-
list_for_each_safe(el, el1, &ctx->debugger_info->script_list) {
642-
LEPUSScriptSource *script = list_entry(el, LEPUSScriptSource, link);
643-
if (script && script->url && source_url &&
644-
strcmp(script->url, source_url) == 0) {
645-
return script;
646-
}
647-
}
648-
return NULL;
649-
}
650-
651645
// get view id from filename
652646
// filename format: file://viewx/app-service.js
653647
QJS_STATIC int32_t GetViewID(const char *filename) {
@@ -689,22 +683,10 @@ QJS_HIDE void DebuggerParseScript(LEPUSContext *ctx, const char *input,
689683
int32_t err, int start_line_number) {
690684
auto *debug_info = ctx->debugger_info;
691685
if (!debug_info) return;
692-
char *source_url = NULL;
693686
LEPUSScriptSource *script = NULL;
694687
HandleScope func_scope{ctx};
695-
if (input) {
696-
source_url =
697-
FindDebuggerMagicContent(ctx, (char *)input, (char *)"sourceURL", 0);
698688

699-
func_scope.PushHandle(source_url, HANDLE_TYPE_DIR_HEAP_OBJ);
700-
if (source_url) {
701-
// if the script is already in script list, do not need to create new
702-
// script
703-
script = FindDebuggerScript(ctx, source_url);
704-
}
705-
}
706-
707-
if (!script && IsDebuggerFile(filename)) {
689+
if (IsDebuggerFile(filename)) {
708690
script = static_cast<LEPUSScriptSource *>(lepus_mallocz(
709691
ctx, sizeof(LEPUSScriptSource), ALLOC_TAG_LEPUSScriptSource));
710692
func_scope.PushHandle(script, HANDLE_TYPE_DIR_HEAP_OBJ);
@@ -718,7 +700,7 @@ QJS_HIDE void DebuggerParseScript(LEPUSContext *ctx, const char *input,
718700
memcpy(script->source, input, input_len + 1);
719701
}
720702
script->end_line = end_line_num + start_line_number;
721-
SetScriptUrl(ctx, filename, script, source_url);
703+
SetScriptUrl(ctx, filename, script);
722704
SetScriptSourceMappingUrl(ctx, script);
723705
SetScriptHash(ctx, script);
724706
debug_info->script_num++;
@@ -741,9 +723,6 @@ QJS_HIDE void DebuggerParseScript(LEPUSContext *ctx, const char *input,
741723
SendParseScriptNotification(ctx, script, err, view_id);
742724
}
743725
}
744-
if (!ctx->gc_enable) {
745-
lepus_free(ctx, source_url);
746-
}
747726
return;
748727
}
749728

testing/quickjs/compiler/test_debug_common.cc

Lines changed: 115 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2038,14 +2038,25 @@ TEST_F(QjsDebugMethods, TestFailToParse) {
20382038

20392039
TEST_F(QjsDebugMethods, TestFindDebuggerMagicContent) {
20402040
std::string source =
2041-
"test js.map //## source sourceURL\n"
2041+
"test js.map"
2042+
"// # sourceURL=error.js\n"
2043+
"//# sourceURL=test_source_url.js\n"
2044+
"// # sourceMappingURL=error.js.map"
20422045
"//# sourceMappingURL=7778.4f4d5141.js.map";
2043-
char* result = FindDebuggerMagicContent(ctx_, (char*)source.c_str(),
2044-
(char*)"sourceMappingURL", 0);
2045-
std::string result_str(result);
2046-
if (!ctx_->rt->gc_enable) lepus_free(ctx_, result);
2047-
std::cout << "result: " << result_str << std::endl;
2048-
ASSERT_TRUE(result_str == "7778.4f4d5141.js.map");
2046+
2047+
char* source_map_url = FindDebuggerMagicContent(ctx_, (char*)source.c_str(),
2048+
(char*)"sourceMappingURL", 0);
2049+
std::string source_map_str(source_map_url);
2050+
if (!ctx_->rt->gc_enable) lepus_free(ctx_, source_map_url);
2051+
std::cout << "sourceMappingURL: " << source_map_str << std::endl;
2052+
ASSERT_TRUE(source_map_str == "7778.4f4d5141.js.map");
2053+
2054+
char* source_url = FindDebuggerMagicContent(ctx_, (char*)source.c_str(),
2055+
(char*)"sourceURL", 0);
2056+
std::string source_url_str(source_url);
2057+
if (!ctx_->rt->gc_enable) lepus_free(ctx_, source_url);
2058+
std::cout << "sourceURL: " << source_url_str << std::endl;
2059+
ASSERT_TRUE(source_url_str == "test_source_url.js");
20492060
}
20502061

20512062
static void CheckStatementPause(LEPUSContext* ctx, int32_t line_number_gt,
@@ -2458,4 +2469,101 @@ TEST_F(QjsDebugMethods, TestPauseOnNextStatement) {
24582469
}
24592470
}
24602471

2472+
TEST_F(QjsDebugMethods, TestScriptUrl) {
2473+
auto funcs = GetQJSCallbackFuncs();
2474+
RegisterQJSDebuggerCallbacks(rt_, funcs.data(), funcs.size());
2475+
2476+
std::string debugger_enable =
2477+
"{\"id\":0,\"method\":\"Debugger.enable\",\"params\":{"
2478+
"\"maxScriptsCacheSize\":100000000}}";
2479+
const char* source1 = R"(function test() {
2480+
let a = 1;
2481+
console.log(a++);
2482+
}
2483+
test();
2484+
//# sourceURL=test_source_url.js
2485+
)";
2486+
const char* source2 = R"(function test() {
2487+
let a = 1;
2488+
console.log(a++);
2489+
}
2490+
test();
2491+
//# sourceURL=source1.js
2492+
)";
2493+
2494+
// Test case 1: Check URL when filename is specified
2495+
QjsDebugQueue::GetSendMessageQueue().push(debugger_enable);
2496+
LEPUSValue ret = LEPUS_Eval(ctx_, source1, strlen(source1), "source1.js",
2497+
LEPUS_EVAL_TYPE_GLOBAL);
2498+
if (!ctx_->rt->gc_enable) LEPUS_FreeValue(ctx_, ret);
2499+
// Pop response of Debugger.enable
2500+
QjsDebugQueue::GetReceiveMessageQueue().pop();
2501+
std::string script_parsed_msg =
2502+
QjsDebugQueue::GetReceiveMessageQueue().front();
2503+
QjsDebugQueue::GetReceiveMessageQueue().pop();
2504+
{
2505+
LEPUSValue json = LEPUS_ParseJSON(ctx_, script_parsed_msg.c_str(),
2506+
script_parsed_msg.length(), "");
2507+
HandleScope func_scope(ctx_, &json, HANDLE_TYPE_LEPUS_VALUE);
2508+
LEPUSValue params = LEPUS_GetPropertyStr(ctx_, json, "params");
2509+
LEPUSValue url = LEPUS_GetPropertyStr(ctx_, params, "url");
2510+
const char* url_str = LEPUS_ToCString(ctx_, url);
2511+
std::cout << "lqy: case1: " << url_str << std::endl;
2512+
ASSERT_TRUE(std::string(url_str) == "source1.js");
2513+
if (!ctx_->rt->gc_enable) {
2514+
LEPUS_FreeCString(ctx_, url_str);
2515+
LEPUS_FreeValue(ctx_, url);
2516+
LEPUS_FreeValue(ctx_, params);
2517+
LEPUS_FreeValue(ctx_, json);
2518+
}
2519+
}
2520+
2521+
// Test case 2: Check URL when no filename is specified, but sourceURL is
2522+
// present in the script
2523+
ret = LEPUS_Eval(ctx_, source1, strlen(source1), "", LEPUS_EVAL_TYPE_GLOBAL);
2524+
if (!ctx_->rt->gc_enable) LEPUS_FreeValue(ctx_, ret);
2525+
script_parsed_msg = QjsDebugQueue::GetReceiveMessageQueue().front();
2526+
QjsDebugQueue::GetReceiveMessageQueue().pop();
2527+
{
2528+
LEPUSValue json = LEPUS_ParseJSON(ctx_, script_parsed_msg.c_str(),
2529+
script_parsed_msg.length(), "");
2530+
HandleScope func_scope(ctx_, &json, HANDLE_TYPE_LEPUS_VALUE);
2531+
LEPUSValue params = LEPUS_GetPropertyStr(ctx_, json, "params");
2532+
LEPUSValue url = LEPUS_GetPropertyStr(ctx_, params, "url");
2533+
const char* url_str = LEPUS_ToCString(ctx_, url);
2534+
std::cout << "lqy: case2: " << url_str << std::endl;
2535+
ASSERT_TRUE(std::string(url_str) == "test_source_url.js");
2536+
if (!ctx_->rt->gc_enable) {
2537+
LEPUS_FreeCString(ctx_, url_str);
2538+
LEPUS_FreeValue(ctx_, url);
2539+
LEPUS_FreeValue(ctx_, params);
2540+
LEPUS_FreeValue(ctx_, json);
2541+
}
2542+
}
2543+
2544+
// Test case 3: Check URL when filename is specified as a parameter and
2545+
// sourceURL is present in the script; filename should take precedence
2546+
ret = LEPUS_Eval(ctx_, source2, strlen(source2), "source2.js",
2547+
LEPUS_EVAL_TYPE_GLOBAL);
2548+
if (!ctx_->rt->gc_enable) LEPUS_FreeValue(ctx_, ret);
2549+
script_parsed_msg = QjsDebugQueue::GetReceiveMessageQueue().front();
2550+
QjsDebugQueue::GetReceiveMessageQueue().pop();
2551+
{
2552+
LEPUSValue json = LEPUS_ParseJSON(ctx_, script_parsed_msg.c_str(),
2553+
script_parsed_msg.length(), "");
2554+
HandleScope func_scope(ctx_, &json, HANDLE_TYPE_LEPUS_VALUE);
2555+
LEPUSValue params = LEPUS_GetPropertyStr(ctx_, json, "params");
2556+
LEPUSValue url = LEPUS_GetPropertyStr(ctx_, params, "url");
2557+
const char* url_str = LEPUS_ToCString(ctx_, url);
2558+
std::cout << "lqy: case3: " << url_str << std::endl;
2559+
ASSERT_TRUE(std::string(url_str) == "source2.js");
2560+
if (!ctx_->rt->gc_enable) {
2561+
LEPUS_FreeCString(ctx_, url_str);
2562+
LEPUS_FreeValue(ctx_, url);
2563+
LEPUS_FreeValue(ctx_, params);
2564+
LEPUS_FreeValue(ctx_, json);
2565+
}
2566+
}
2567+
}
2568+
24612569
} // namespace qjs_debug_test

0 commit comments

Comments
 (0)