From 42c3ad9a4f10e8a3b65a53ee68e5a3bf540df73c Mon Sep 17 00:00:00 2001 From: Gymee Date: Tue, 27 Sep 2022 09:17:05 +0800 Subject: [PATCH 1/2] debugger support hot reload Issue: #I5TWT0 Signed-off-by: Gymee Change-Id: Ia7d2a0dedc4dbb6dbf8c6e5577696faaa176b40f --- tooling/agent/debugger_impl.cpp | 45 ++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/tooling/agent/debugger_impl.cpp b/tooling/agent/debugger_impl.cpp index 6b030f45..11ee643c 100644 --- a/tooling/agent/debugger_impl.cpp +++ b/tooling/agent/debugger_impl.cpp @@ -152,12 +152,16 @@ bool DebuggerImpl::NotifySingleStep(const JSPtLocation &location) bool DebuggerImpl::IsSkipLine(const JSPtLocation &location) { DebugInfoExtractor *extractor = nullptr; - auto scriptFunc = [this, &extractor](PtScript *script) -> bool { - extractor = GetExtractor(script->GetUrl()); + const auto *jsPandaFile = location.GetJsPandaFile(); + auto scriptFunc = [this, &extractor, jsPandaFile](PtScript *) -> bool { + extractor = GetExtractor(jsPandaFile); return true; }; - if (!MatchScripts(scriptFunc, location.GetPandaFile(), ScriptMatchType::FILE_NAME) || extractor == nullptr) { - LOG_DEBUGGER(INFO) << "StepComplete: skip unknown file"; + + // In hot reload scenario, use the base js panda file instead + const auto &fileName = DebuggerApi::GetBaseJSPandaFile(vm_, jsPandaFile)->GetJSPandaFileDesc(); + if (!MatchScripts(scriptFunc, fileName.c_str(), ScriptMatchType::FILE_NAME) || extractor == nullptr) { + LOG_DEBUGGER(INFO) << "StepComplete: skip unknown file " << fileName.c_str(); return true; } @@ -199,9 +203,9 @@ void DebuggerImpl::NotifyPaused(std::optional location, PauseReaso if (location.has_value()) { BreakpointDetails detail; DebugInfoExtractor *extractor = nullptr; - auto scriptFunc = [this, &extractor, &detail](PtScript *script) -> bool { + auto scriptFunc = [this, &location, &detail, &extractor](PtScript *script) -> bool { detail.url_ = script->GetUrl(); - extractor = GetExtractor(detail.url_); + extractor = GetExtractor(location->GetJsPandaFile()); return true; }; auto callbackLineFunc = [&detail](int32_t line) -> bool { @@ -214,10 +218,11 @@ void DebuggerImpl::NotifyPaused(std::optional location, PauseReaso }; File::EntityId methodId = location->GetMethodId(); uint32_t offset = location->GetBytecodeOffset(); - if (!MatchScripts(scriptFunc, location->GetPandaFile(), ScriptMatchType::FILE_NAME) || + // In merge abc scenario, need to use the source file to match to get right url + if (!MatchScripts(scriptFunc, location->GetSourceFile(), ScriptMatchType::URL) || extractor == nullptr || !extractor->MatchLineWithOffset(callbackLineFunc, methodId, offset) || !extractor->MatchColumnWithOffset(callbackColumnFunc, methodId, offset)) { - LOG_DEBUGGER(ERROR) << "NotifyPaused: unknown " << location->GetPandaFile(); + LOG_DEBUGGER(ERROR) << "NotifyPaused: unknown file " << location->GetSourceFile(); return; } hitBreakpoints.emplace_back(BreakpointDetails::ToString(detail)); @@ -628,7 +633,7 @@ DispatchResponse DebuggerImpl::GetPossibleBreakpoints(const GetPossibleBreakpoin int32_t line = start->GetLine(); int32_t column = start->GetColumn(); - auto callbackFunc = []([[maybe_unused]] File::EntityId id, [[maybe_unused]] uint32_t offset) -> bool { + auto callbackFunc = [](const JSPtLocation &) -> bool { return true; }; if (extractor->MatchWithLocation(callbackFunc, line, column, url)) { @@ -672,9 +677,7 @@ DispatchResponse DebuggerImpl::RemoveBreakpoint(const RemoveBreakpointParams &pa return DispatchResponse::Fail("Unknown file name."); } - std::string fileName; - auto scriptFunc = [&fileName](PtScript *script) -> bool { - fileName = script->GetFileName(); + auto scriptFunc = [](PtScript *) -> bool { return true; }; if (!MatchScripts(scriptFunc, metaData.url_, ScriptMatchType::URL)) { @@ -682,12 +685,12 @@ DispatchResponse DebuggerImpl::RemoveBreakpoint(const RemoveBreakpointParams &pa return DispatchResponse::Fail("Unknown file name."); } - auto callbackFunc = [this, fileName](File::EntityId id, uint32_t offset) -> bool { - JSPtLocation location {fileName.c_str(), id, offset}; + auto callbackFunc = [this](const JSPtLocation &location) -> bool { + LOG_DEBUGGER(INFO) << "remove breakpoint location: " << location.ToString(); return DebuggerApi::RemoveBreakpoint(jsDebugger_, location); }; if (!extractor->MatchWithLocation(callbackFunc, metaData.line_, metaData.column_, metaData.url_)) { - LOG_DEBUGGER(ERROR) << "failed to set breakpoint location number: " + LOG_DEBUGGER(ERROR) << "failed to remove breakpoint location number: " << metaData.line_ << ":" << metaData.column_; return DispatchResponse::Fail("Breakpoint not found."); } @@ -735,8 +738,8 @@ DispatchResponse DebuggerImpl::SetBreakpointByUrl(const SetBreakpointByUrlParams return DispatchResponse::Fail("Unknown file name."); } - auto callbackFunc = [this, fileName, &condition](File::EntityId id, uint32_t offset) -> bool { - JSPtLocation location {fileName.c_str(), id, offset}; + auto callbackFunc = [this, &condition](const JSPtLocation &location) -> bool { + LOG_DEBUGGER(INFO) << "set breakpoint location: " << location.ToString(); Local condFuncRef = FunctionRef::Undefined(vm_); if (condition.has_value() && !condition.value().empty()) { std::string dest; @@ -851,8 +854,16 @@ DebugInfoExtractor *DebuggerImpl::GetExtractor(const JSPandaFile *jsPandaFile) return JSPandaFileManager::GetInstance()->GetJSPtExtractor(jsPandaFile); } +// mainly used for breakpoints to match location DebugInfoExtractor *DebuggerImpl::GetExtractor(const std::string &url) { + // match patch file first if it contains diff for the url, and currently only support the file + // specified by the url change as a whole + auto *extractor = DebuggerApi::GetPatchExtractor(vm_, url); + if (extractor != nullptr) { + return extractor; + } + auto iter = extractors_.find(url); if (iter == extractors_.end()) { return nullptr; -- Gitee From 7673d361307fda17ac5b63d81694adc06fe9c5c3 Mon Sep 17 00:00:00 2001 From: Gymee Date: Mon, 10 Oct 2022 20:01:54 +0800 Subject: [PATCH 2/2] debugger support hot reload(fix testcases) Issue: #I5TWT0 Signed-off-by: Gymee Change-Id: I55e49c9d719b69cee9def34623a0de8dd845ed20 --- tooling/test/js_pt_hooks_test.cpp | 9 +++------ tooling/test/utils/test_extractor.cpp | 6 +++--- tooling/test/utils/test_util.cpp | 2 +- tooling/test/utils/test_util.h | 16 ++++++++-------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/tooling/test/js_pt_hooks_test.cpp b/tooling/test/js_pt_hooks_test.cpp index cbae5c64..90527e0a 100644 --- a/tooling/test/js_pt_hooks_test.cpp +++ b/tooling/test/js_pt_hooks_test.cpp @@ -62,10 +62,9 @@ HWTEST_F_L0(JSPtHooksTest, BreakpointTest) { auto debugger = std::make_unique(ecmaVm, nullptr, nullptr); std::unique_ptr jspthooks = std::make_unique(debugger.get()); - const char *pandaFile = " "; EntityId methodId(0); uint32_t bytecodeOffset = 0; - JSPtLocation ptLocation1(pandaFile, methodId, bytecodeOffset); + JSPtLocation ptLocation1(nullptr, methodId, bytecodeOffset); jspthooks->Breakpoint(ptLocation1); ASSERT_NE(jspthooks, nullptr); } @@ -82,10 +81,9 @@ HWTEST_F_L0(JSPtHooksTest, ExceptionTest) { auto debugger = std::make_unique(ecmaVm, nullptr, nullptr); std::unique_ptr jspthooks = std::make_unique(debugger.get()); - const char *pandaFile = " "; EntityId methodId(0); uint32_t bytecodeOffset = 0; - JSPtLocation ptLocation2(pandaFile, methodId, bytecodeOffset); + JSPtLocation ptLocation2(nullptr, methodId, bytecodeOffset); jspthooks->Exception(ptLocation2); ASSERT_NE(jspthooks, nullptr); } @@ -94,10 +92,9 @@ HWTEST_F_L0(JSPtHooksTest, SingleStepTest) { auto debugger = std::make_unique(ecmaVm, nullptr, nullptr); std::unique_ptr jspthooks = std::make_unique(debugger.get()); - const char *pandaFile = " "; EntityId methodId(0); uint32_t bytecodeOffset = 0; - JSPtLocation ptLocation4(pandaFile, methodId, bytecodeOffset); + JSPtLocation ptLocation4(nullptr, methodId, bytecodeOffset); ASSERT_NE(jspthooks, nullptr); } diff --git a/tooling/test/utils/test_extractor.cpp b/tooling/test/utils/test_extractor.cpp index 21295c8e..c1d1412c 100644 --- a/tooling/test/utils/test_extractor.cpp +++ b/tooling/test/utils/test_extractor.cpp @@ -30,9 +30,9 @@ std::pair TestExtractor::GetBreakpointAddress(const SourceLo { EntityId retId = EntityId(); uint32_t retOffset = 0; - auto callbackFunc = [&retId, &retOffset](panda_file::File::EntityId id, uint32_t offset) -> bool { - retId = id; - retOffset = offset; + auto callbackFunc = [&retId, &retOffset](const JSPtLocation &jsLocation) -> bool { + retId = jsLocation.GetMethodId(); + retOffset = jsLocation.GetBytecodeOffset(); return true; }; MatchWithLocation(callbackFunc, sourceLocation.line, sourceLocation.column, sourceLocation.path); diff --git a/tooling/test/utils/test_util.cpp b/tooling/test/utils/test_util.cpp index d993558d..19ef855b 100644 --- a/tooling/test/utils/test_util.cpp +++ b/tooling/test/utils/test_util.cpp @@ -24,7 +24,7 @@ bool TestUtil::initialized_ = false; os::memory::Mutex TestUtil::suspendMutex_; os::memory::ConditionVariable TestUtil::suspendCv_; bool TestUtil::suspended_ = false; -JSPtLocation TestUtil::lastEventLocation_("", EntityId(0), 0); +JSPtLocation TestUtil::lastEventLocation_(nullptr, EntityId(0), 0); std::ostream &operator<<(std::ostream &out, DebugEvent value) { diff --git a/tooling/test/utils/test_util.h b/tooling/test/utils/test_util.h index 5086601a..893e04d3 100644 --- a/tooling/test/utils/test_util.h +++ b/tooling/test/utils/test_util.h @@ -54,7 +54,7 @@ public: auto predicate = [&location]() REQUIRES(eventMutex_) { return lastEventLocation_ == location; }; auto onSuccess = []() REQUIRES(eventMutex_) { // Need to reset location, because we might want to stop at the same point - lastEventLocation_ = JSPtLocation("", EntityId(0), 0); + lastEventLocation_ = JSPtLocation(nullptr, EntityId(0), 0); }; WaitForEvent(DebugEvent::BREAKPOINT, predicate, onSuccess); @@ -73,7 +73,7 @@ public: auto predicate = [&location]() REQUIRES(eventMutex_) { return lastEventLocation_ == location; }; auto onSuccess = []() REQUIRES(eventMutex_) { // Need to reset location, because we might want to stop at the same point - lastEventLocation_ = JSPtLocation("", EntityId(0), 0); + lastEventLocation_ = JSPtLocation(nullptr, EntityId(0), 0); }; WaitForEvent(DebugEvent::STEP_COMPLETE, predicate, onSuccess); @@ -99,7 +99,7 @@ public: return WaitForEvent(DebugEvent::LOAD_MODULE, predicate, [] {}); } - static void Event(DebugEvent event, JSPtLocation location = JSPtLocation("", EntityId(0), 0)) + static void Event(DebugEvent event, JSPtLocation location = JSPtLocation(nullptr, EntityId(0), 0)) { LOG_DEBUGGER(DEBUG) << "Occurred event " << event; os::memory::LockHolder holder(eventMutex_); @@ -133,11 +133,11 @@ public: { auto jsPandaFile = ::panda::ecmascript::JSPandaFileManager::GetInstance()->OpenJSPandaFile(pandaFile); if (jsPandaFile == nullptr) { - return JSPtLocation("", EntityId(0), 0); + return JSPtLocation(nullptr, EntityId(0), 0); } TestExtractor extractor(jsPandaFile); - auto [id, offset] = extractor.GetBreakpointAddress({"", line, column}); - return JSPtLocation(pandaFile, id, offset); + auto [id, offset] = extractor.GetBreakpointAddress({nullptr, line, column}); + return JSPtLocation(jsPandaFile, id, offset); } static SourceLocation GetSourceLocation(const JSPtLocation &location, const char *pandaFile) @@ -150,7 +150,7 @@ public: return extractor.GetSourceLocation(location.GetMethodId(), location.GetBytecodeOffset()); } - static bool SuspendUntilContinue(DebugEvent reason, JSPtLocation location = JSPtLocation("", EntityId(0), 0)) + static bool SuspendUntilContinue(DebugEvent reason, JSPtLocation location = JSPtLocation(nullptr, EntityId(0), 0)) { os::memory::LockHolder lock(suspendMutex_); suspended_ = true; @@ -282,7 +282,7 @@ std::ostream &operator<<(std::ostream &out, std::nullptr_t); #define ASSERT_LOCATION_EQ(lhs, rhs) \ do { \ - ASSERT_STREQ((lhs).GetPandaFile(), (rhs).GetPandaFile()); \ + ASSERT_EQ((lhs).GetJsPandaFile(), (rhs).GetJsPandaFile()); \ ASSERT_EQ((lhs).GetMethodId().GetOffset(), (rhs).GetMethodId().GetOffset()); \ ASSERT_EQ((lhs).GetBytecodeOffset(), (rhs).GetBytecodeOffset()); \ } while (0) -- Gitee