[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
MaskRay added a comment. Hi! The added lines have some spaces at line ends. I think using `clang-format-diff.py` would remove them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
mib added a comment. LGTM! Thanks Jim! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2b0c88654212: Refine the reporting mechanism for
interruption. (authored by jingham).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154542/new/
https://reviews.llvm.org/D154542
Files:
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Target/TargetList.h
lldb/source/API/SBFrame.cpp
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/Module.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetList.cpp
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
return error;
}
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+ debugger.GetTargetList().RegisterInProcessTarget(target_sp);
target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
if (user_exe_path_is_bundle)
exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
// No file was specified, just create an empty target with any arch if a
// valid arch was specified
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp);
}
if (!target_sp)
@@ -513,6 +515,7 @@
void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
"target already exists it the list");
+ UnregisterInProcessTarget(target_sp);
m_target_list.push_back(std::move(target_sp));
if (do_select)
SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
m_selected_target_idx = 0;
return GetTargetAtIndex(m_selected_target_idx);
}
+
+bool TargetList::AnyTargetContainsModule(Module &module) {
+ std::lock_guard guard(m_target_list_mutex);
+ for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ for (const auto &target_sp: m_in_process_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ return false;
+}
+
+ void TargetList::RegisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target_sp);
+assert(was_added && "Target pointer was left in the in-process map");
+ }
+
+ void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target_sp);
+assert(was_present && "Target pointer being removed was not registered");
+ }
+
+ bool TargetList::IsTargetInProcess(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target_sp) == 1;
+ }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
// each library in parallel.
if (GetPreloadSymbols())
module_sp->PreloadSymbols();
-
llvm::SmallVector replaced_modules;
for (ModuleSP &old_module_sp : old_modules) {
if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
}
bool TargetProperties::GetPreloadSymbols() const {
+ if (INTERRUPT_REQUESTED(m_target->GetDebugger(),
+ "Interrupted checking preload symbols")) {
+return false;
+ }
const uint32_t idx = ePropertyPreloadSymbols;
return GetPropertyAtIndexAs(
idx, g_target_properties[idx].default_uint_value != 0);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -509,11 +509,11 @@
} else {
// Check for interruption when building the frames.
// Do the check in idx > 0 so that we'll always create a 0th frame.
- if (allow_interrupt && dbg.InterruptRequested()) {
-Log *log = GetLog(LLDBLog::Host);
-LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
-was_interrupted = true;
-break;
+ if (allow_interrupt
+ && INTERRUPT_REQUESTED(dbg, "Interrupted having fetched {0} frames",
+ m_frames.size())) {
+ was_interrupted = true;
+ break;
}
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham updated this revision to Diff 537908.
jingham added a comment.
Add some std;:move's, Jonas agrees all the cool kids are doing that these days.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154542/new/
https://reviews.llvm.org/D154542
Files:
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Target/TargetList.h
lldb/source/API/SBFrame.cpp
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/Module.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetList.cpp
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
return error;
}
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+ debugger.GetTargetList().RegisterInProcessTarget(target_sp);
target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
if (user_exe_path_is_bundle)
exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
// No file was specified, just create an empty target with any arch if a
// valid arch was specified
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp);
}
if (!target_sp)
@@ -513,6 +515,7 @@
void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
"target already exists it the list");
+ UnregisterInProcessTarget(target_sp);
m_target_list.push_back(std::move(target_sp));
if (do_select)
SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
m_selected_target_idx = 0;
return GetTargetAtIndex(m_selected_target_idx);
}
+
+bool TargetList::AnyTargetContainsModule(Module &module) {
+ std::lock_guard guard(m_target_list_mutex);
+ for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ for (const auto &target_sp: m_in_process_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ return false;
+}
+
+ void TargetList::RegisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target_sp);
+assert(was_added && "Target pointer was left in the in-process map");
+ }
+
+ void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target_sp);
+assert(was_present && "Target pointer being removed was not registered");
+ }
+
+ bool TargetList::IsTargetInProcess(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target_sp) == 1;
+ }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
// each library in parallel.
if (GetPreloadSymbols())
module_sp->PreloadSymbols();
-
llvm::SmallVector replaced_modules;
for (ModuleSP &old_module_sp : old_modules) {
if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
}
bool TargetProperties::GetPreloadSymbols() const {
+ if (INTERRUPT_REQUESTED(m_target->GetDebugger(),
+ "Interrupted checking preload symbols")) {
+return false;
+ }
const uint32_t idx = ePropertyPreloadSymbols;
return GetPropertyAtIndexAs(
idx, g_target_properties[idx].default_uint_value != 0);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -509,11 +509,11 @@
} else {
// Check for interruption when building the frames.
// Do the check in idx > 0 so that we'll always create a 0th frame.
- if (allow_interrupt && dbg.InterruptRequested()) {
-Log *log = GetLog(LLDBLog::Host);
-LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
-was_interrupted = true;
-break;
+ if (allow_interrupt
+ && INTERRUPT_REQUESTED(dbg, "Interrupted having fetched {0} frames",
+ m_frames.size())) {
+ was_interrupted = true;
+ break;
}
const bool success
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
bulbazord added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:455-456 +InterruptionReport(std::string function_name, std::string description) : +m_function_name(function_name), +m_description(description), +m_interrupt_time(std::chrono::system_clock::now()), jingham wrote: > bulbazord wrote: > > jingham wrote: > > > bulbazord wrote: > > > > To avoid some unnecessary copies > > > > > > > > Could also do what Ismail is suggesting. > > > This is a local that is copied to an ivar and never used again. Do I > > > really have to put move in there explicitly for the optimizer to know it > > > can reuse the value? > > Yes. I created a simple example on Godbolt: https://godbolt.org/z/G4ej76Enn > > > > Observe that the constructor in the example invokes the `std::string` copy > > constructor. Add a `std::move` and it then invokes the move constructor. > Thanks for the example, that was interesting. > > Note however that in your example, you the flags you specified were > `-std=c++17` so that was an unoptimized build. That's expected, unoptimized > builds are supposed to be as literal as they can be. If you instead use the > same example with `-std=c++17 -O3` you won't see any copy constructors. I > don't 100% mind putting this std::move in, but it seems noisy to decorate up > your code with hints that the optimizer can figure out on its own. You're right that I did not consider higher optimization levels there. That being said, I believe this is a matter of language semantics and not optimization level. I've created a slightly more complex example to illustrate: https://godbolt.org/z/naMTcd7Ev In this other example, I have passed `-O3`. You'll notice that in the emitted constructor code there are 2 calls to `operator new`. If you add `std::move` for both of the parameters of the constructor, both of these allocations go away and the generated code becomes ~60% smaller. Even with optimization involved, the compiler can only optimize so much without breaking semantics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:455-456 +InterruptionReport(std::string function_name, std::string description) : +m_function_name(function_name), +m_description(description), +m_interrupt_time(std::chrono::system_clock::now()), bulbazord wrote: > jingham wrote: > > bulbazord wrote: > > > To avoid some unnecessary copies > > > > > > Could also do what Ismail is suggesting. > > This is a local that is copied to an ivar and never used again. Do I > > really have to put move in there explicitly for the optimizer to know it > > can reuse the value? > Yes. I created a simple example on Godbolt: https://godbolt.org/z/G4ej76Enn > > Observe that the constructor in the example invokes the `std::string` copy > constructor. Add a `std::move` and it then invokes the move constructor. Thanks for the example, that was interesting. Note however that in your example, you the flags you specified were `-std=c++17` so that was an unoptimized build. That's expected, unoptimized builds are supposed to be as literal as they can be. If you instead use the same example with `-std=c++17 -O3` you won't see any copy constructors. I don't 100% mind putting this std::move in, but it seems noisy to decorate up your code with hints that the optimizer can figure out on its own. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
bulbazord added inline comments.
Comment at: lldb/include/lldb/Core/Debugger.h:430-431
+ template
+ bool InterruptRequested(const char *cur_func,
+ const char *formatv, Args &&... args) {
+bool ret_val = InterruptRequested();
jingham wrote:
> bulbazord wrote:
> > I think it would make more sense to have `cur_func` and `formatv` be of
> > type `llvm::StringRef`. Concretely, if you construct a `std::string` from
> > `nullptr` (like you're doing below when you make an InterruptionReport
> > object), you will crash.
> >
> > I know the recommendation is to use `INTERRUPT_REQUESTED` instead of
> > filling this manually, but inevitably somebody will go against the advice
> > and make a mistake.
> > I think it would make more sense to have `cur_func` and `formatv` be of
> > type `llvm::StringRef`. Concretely, if you construct a `std::string` from
> > `nullptr` (like you're doing below when you make an InterruptionReport
> > object), you will crash.
> >
> > I know the recommendation is to use `INTERRUPT_REQUESTED` instead of
> > filling this manually, but inevitably somebody will go against the advice
> > and make a mistake.
>
> I don't see how I can make the formatv option a StringRef. The llvm::formatv
> API only offers a version that takes a `const char *`. Anyway, these are
> formatv strings, they are almost universally going to be const strings.
> Turning them into llvm::StringRef's and back out again to use in
> llvm::formatv seems odd.
>
> But you are right, we should protect against someone passing in a null
> pointer for the function or format to InterruptRequested. Since this is
> just logging, an assert seems overkill, I'll just add null pointer checks
> here and turn them into "UNKNOWN FUNCTION" and "Unknown message".
Oh... so `llvm::formatv` does take a `const char *`... My bad there, that makes
sense, no need to change the type of `format` then.
Otherwise, I'm satisfied with the safety checks.
Comment at: lldb/include/lldb/Core/Debugger.h:455-456
+InterruptionReport(std::string function_name, std::string description) :
+m_function_name(function_name),
+m_description(description),
+m_interrupt_time(std::chrono::system_clock::now()),
jingham wrote:
> bulbazord wrote:
> > To avoid some unnecessary copies
> >
> > Could also do what Ismail is suggesting.
> This is a local that is copied to an ivar and never used again. Do I really
> have to put move in there explicitly for the optimizer to know it can reuse
> the value?
Yes. I created a simple example on Godbolt: https://godbolt.org/z/G4ej76Enn
Observe that the constructor in the example invokes the `std::string` copy
constructor. Add a `std::move` and it then invokes the move constructor.
Comment at: lldb/include/lldb/Core/Debugger.h:465
+ InterruptionReport(std::string function_name,
+ const char *format, Args &&... args) :
+InterruptionReport(function_name, llvm::formatv(format,
std::forward(args)...)) {}
bulbazord wrote:
> since we're passing `format` to `formatv`, I think it would make sense for
> its type to be `llvm::StringRef` up-front here.
As you've pointed out above, `formatv` takes a `const char *` so ignore this.
Comment at: lldb/source/Core/Debugger.cpp:1280
+const llvm::formatv_object_base &payload) :
+m_function_name(function_name),
+m_interrupt_time(std::chrono::system_clock::now()),
You'll also want to `std::move(function_name)` here too, I think.
Comment at: lldb/source/Target/TargetList.cpp:518
"target already exists it the list");
+ UnregisterInProcessTarget(target_sp.get());
m_target_list.push_back(std::move(target_sp));
jingham wrote:
> bulbazord wrote:
> > I'm somewhat puzzled by this. Why are we unregistering the target in
> > `AddTargetInternal` when we're registering it in `CreateTargetInternal`?
> > Maybe I don't understand the intent behind
> > `{Register,Unregister}InProcessTarget`.
> The point behind this is someone says "make me a target" and then in the
> process of making the target, the Target code does something slow (e.g. look
> for external debug files) that we want to interrupt. So we need a way for
> the debugger to find the "in the process of being made" Targets.
>
> "AddTargetInternal" is the API where the target gets added to the Debugger's
> TargetList, so after that method has run we will find the target in the
> regular TargetList. But between CreateTargetInternal and AddTargetInternal,
> that target isn't stored anywhere that the Debugger knows how to query. I
> added this list to hold the targets that are in the process of getting made,
> before they get added to the Debugger's TargetList.
Gotcha, that makes sense. Thanks for explaining!
Repository:
rG LLVM
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:455-456 +InterruptionReport(std::string function_name, std::string description) : +m_function_name(function_name), +m_description(description), +m_interrupt_time(std::chrono::system_clock::now()), bulbazord wrote: > To avoid some unnecessary copies > > Could also do what Ismail is suggesting. This is a local that is copied to an ivar and never used again. Do I really have to put move in there explicitly for the optimizer to know it can reuse the value? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham updated this revision to Diff 537551.
jingham added a comment.
Protect InterruptRequested from null function & format strings.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154542/new/
https://reviews.llvm.org/D154542
Files:
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Target/TargetList.h
lldb/source/API/SBFrame.cpp
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/Module.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetList.cpp
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
return error;
}
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+ debugger.GetTargetList().RegisterInProcessTarget(target_sp);
target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
if (user_exe_path_is_bundle)
exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
// No file was specified, just create an empty target with any arch if a
// valid arch was specified
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp);
}
if (!target_sp)
@@ -513,6 +515,7 @@
void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
"target already exists it the list");
+ UnregisterInProcessTarget(target_sp);
m_target_list.push_back(std::move(target_sp));
if (do_select)
SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
m_selected_target_idx = 0;
return GetTargetAtIndex(m_selected_target_idx);
}
+
+bool TargetList::AnyTargetContainsModule(Module &module) {
+ std::lock_guard guard(m_target_list_mutex);
+ for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ for (const auto &target_sp: m_in_process_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ return false;
+}
+
+ void TargetList::RegisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target_sp);
+assert(was_added && "Target pointer was left in the in-process map");
+ }
+
+ void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target_sp);
+assert(was_present && "Target pointer being removed was not registered");
+ }
+
+ bool TargetList::IsTargetInProcess(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target_sp) == 1;
+ }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
// each library in parallel.
if (GetPreloadSymbols())
module_sp->PreloadSymbols();
-
llvm::SmallVector replaced_modules;
for (ModuleSP &old_module_sp : old_modules) {
if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
}
bool TargetProperties::GetPreloadSymbols() const {
+ if (INTERRUPT_REQUESTED(m_target->GetDebugger(),
+ "Interrupted checking preload symbols")) {
+return false;
+ }
const uint32_t idx = ePropertyPreloadSymbols;
return GetPropertyAtIndexAs(
idx, g_target_properties[idx].default_uint_value != 0);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -509,11 +509,11 @@
} else {
// Check for interruption when building the frames.
// Do the check in idx > 0 so that we'll always create a 0th frame.
- if (allow_interrupt && dbg.InterruptRequested()) {
-Log *log = GetLog(LLDBLog::Host);
-LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
-was_interrupted = true;
-break;
+ if (allow_interrupt
+ && INTERRUPT_REQUESTED(dbg, "Interrupted having fetched {0} frames",
+ m_frames.size())) {
+ was_interrupted = true;
+ break;
}
const bool success =
@@ -965,11 +9
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham added inline comments.
Comment at: lldb/include/lldb/Core/Debugger.h:430-431
+ template
+ bool InterruptRequested(const char *cur_func,
+ const char *formatv, Args &&... args) {
+bool ret_val = InterruptRequested();
bulbazord wrote:
> I think it would make more sense to have `cur_func` and `formatv` be of type
> `llvm::StringRef`. Concretely, if you construct a `std::string` from
> `nullptr` (like you're doing below when you make an InterruptionReport
> object), you will crash.
>
> I know the recommendation is to use `INTERRUPT_REQUESTED` instead of filling
> this manually, but inevitably somebody will go against the advice and make a
> mistake.
> I think it would make more sense to have `cur_func` and `formatv` be of type
> `llvm::StringRef`. Concretely, if you construct a `std::string` from
> `nullptr` (like you're doing below when you make an InterruptionReport
> object), you will crash.
>
> I know the recommendation is to use `INTERRUPT_REQUESTED` instead of filling
> this manually, but inevitably somebody will go against the advice and make a
> mistake.
I don't see how I can make the formatv option a StringRef. The llvm::formatv
API only offers a version that takes a `const char *`. Anyway, these are
formatv strings, they are almost universally going to be const strings.
Turning them into llvm::StringRef's and back out again to use in llvm::formatv
seems odd.
But you are right, we should protect against someone passing in a null pointer
for the function or format to InterruptRequested. Since this is just logging,
an assert seems overkill, I'll just add null pointer checks here and turn them
into "UNKNOWN FUNCTION" and "Unknown message".
Comment at: lldb/source/API/SBFrame.cpp:57-58
+#include
+
using namespace lldb;
mib wrote:
> bulbazord wrote:
> > What is this used for?
> Is this still necessary ?
Not sure what you are asking here. We use StackFrameSP w/o saying
lldb::StackFrameSP and we use VariableList for instance rather than
lldb_private::VariableList. We could remove the using statements here but
that's not how we do it in general. In some cases we don't do `using namespace
lldb` but that's mostly in files that use clang API's heavily since those
conflict with some clang type names and it was good to be explicit there. But
I don't think we want to be verbose like that in the SB files.
Comment at: lldb/source/Target/StackFrameList.cpp:31
#include
+#include
bulbazord wrote:
> Where is this used?
Dunno, but this seems more like an orthogonal cleanup unrelated to the current
patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154542/new/
https://reviews.llvm.org/D154542
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham updated this revision to Diff 537544.
jingham added a comment.
Address review comments:
Made the in_process target list shared pointers.
Removed sstream includes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154542/new/
https://reviews.llvm.org/D154542
Files:
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Target/TargetList.h
lldb/source/API/SBFrame.cpp
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/Module.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetList.cpp
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
return error;
}
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+ debugger.GetTargetList().RegisterInProcessTarget(target_sp);
target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
if (user_exe_path_is_bundle)
exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
// No file was specified, just create an empty target with any arch if a
// valid arch was specified
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp);
}
if (!target_sp)
@@ -513,6 +515,7 @@
void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
"target already exists it the list");
+ UnregisterInProcessTarget(target_sp);
m_target_list.push_back(std::move(target_sp));
if (do_select)
SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
m_selected_target_idx = 0;
return GetTargetAtIndex(m_selected_target_idx);
}
+
+bool TargetList::AnyTargetContainsModule(Module &module) {
+ std::lock_guard guard(m_target_list_mutex);
+ for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ for (const auto &target_sp: m_in_process_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+ return true;
+ }
+ return false;
+}
+
+ void TargetList::RegisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target_sp);
+assert(was_added && "Target pointer was left in the in-process map");
+ }
+
+ void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target_sp);
+assert(was_present && "Target pointer being removed was not registered");
+ }
+
+ bool TargetList::IsTargetInProcess(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target_sp) == 1;
+ }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
// each library in parallel.
if (GetPreloadSymbols())
module_sp->PreloadSymbols();
-
llvm::SmallVector replaced_modules;
for (ModuleSP &old_module_sp : old_modules) {
if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
}
bool TargetProperties::GetPreloadSymbols() const {
+ if (INTERRUPT_REQUESTED(m_target->GetDebugger(),
+ "Interrupted checking preload symbols")) {
+return false;
+ }
const uint32_t idx = ePropertyPreloadSymbols;
return GetPropertyAtIndexAs(
idx, g_target_properties[idx].default_uint_value != 0);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -509,11 +509,11 @@
} else {
// Check for interruption when building the frames.
// Do the check in idx > 0 so that we'll always create a 0th frame.
- if (allow_interrupt && dbg.InterruptRequested()) {
-Log *log = GetLog(LLDBLog::Host);
-LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
-was_interrupted = true;
-break;
+ if (allow_interrupt
+ && INTERRUPT_REQUESTED(dbg, "Interrupted having fetched {0} frames",
+ m_frames.size())) {
+ was_interrupted = true;
+ break;
}
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham added inline comments.
Comment at: lldb/include/lldb/Core/Debugger.h:435-436
+ ReportInterruption(InterruptionReport(cur_func,
+llvm::formatv(formatv,
+std::forward(args)...)));
+}
mib wrote:
> If you use `LLVM_PRETTY_FUNCTION` you won't need this.
I'm not sure I understand how you want to use LLVM_PRETTY_FUNCTION. From what
I can tell, LLVM_PRETTY_FUNCTION prints something that looks like the function
invocation as written. But ReportInterruption and the associated macros
provide a formatv API.
Comment at: lldb/source/Target/TargetList.cpp:518
"target already exists it the list");
+ UnregisterInProcessTarget(target_sp.get());
m_target_list.push_back(std::move(target_sp));
bulbazord wrote:
> I'm somewhat puzzled by this. Why are we unregistering the target in
> `AddTargetInternal` when we're registering it in `CreateTargetInternal`?
> Maybe I don't understand the intent behind
> `{Register,Unregister}InProcessTarget`.
The point behind this is someone says "make me a target" and then in the
process of making the target, the Target code does something slow (e.g. look
for external debug files) that we want to interrupt. So we need a way for the
debugger to find the "in the process of being made" Targets.
"AddTargetInternal" is the API where the target gets added to the Debugger's
TargetList, so after that method has run we will find the target in the regular
TargetList. But between CreateTargetInternal and AddTargetInternal, that
target isn't stored anywhere that the Debugger knows how to query. I added
this list to hold the targets that are in the process of getting made, before
they get added to the Debugger's TargetList.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154542/new/
https://reviews.llvm.org/D154542
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
mib added a comment. I had an offline chat with Jim and I misunderstood originally, what he was trying to achieve with the `InterruptionReport:: m_function_name` so my suggestion of using `LLVM_PRETTY_FUNCTION` doesn't actually work here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
bulbazord added a comment.
I really like the change to the interface, I especially like that it can report
what was interrupted and how much work actually done. I had a lot of the same
feedback as Ismail but also had some questions to help me understand all the
details.
Comment at: lldb/include/lldb/Core/Debugger.h:430-431
+ template
+ bool InterruptRequested(const char *cur_func,
+ const char *formatv, Args &&... args) {
+bool ret_val = InterruptRequested();
I think it would make more sense to have `cur_func` and `formatv` be of type
`llvm::StringRef`. Concretely, if you construct a `std::string` from `nullptr`
(like you're doing below when you make an InterruptionReport object), you will
crash.
I know the recommendation is to use `INTERRUPT_REQUESTED` instead of filling
this manually, but inevitably somebody will go against the advice and make a
mistake.
Comment at: lldb/include/lldb/Core/Debugger.h:446
+#define INTERRUPT_REQUESTED(debugger, ...) \
+(debugger).InterruptRequested(__func__, __VA_ARGS__)
+
mib wrote:
> You could use `LLVM_PRETTY_FUNCTION` which will give you both the function
> signature and the argument well formatted.
+1
Comment at: lldb/include/lldb/Core/Debugger.h:455-456
+InterruptionReport(std::string function_name, std::string description) :
+m_function_name(function_name),
+m_description(description),
+m_interrupt_time(std::chrono::system_clock::now()),
To avoid some unnecessary copies
Could also do what Ismail is suggesting.
Comment at: lldb/include/lldb/Core/Debugger.h:465
+ InterruptionReport(std::string function_name,
+ const char *format, Args &&... args) :
+InterruptionReport(function_name, llvm::formatv(format,
std::forward(args)...)) {}
since we're passing `format` to `formatv`, I think it would make sense for its
type to be `llvm::StringRef` up-front here.
Comment at: lldb/include/lldb/Target/TargetList.h:191
+ /// these not yet added target, but for interruption purposes, we might
+ /// need to ask whetehr this target contains this module.
+ bool AnyTargetContainsModule(Module *module);
Comment at: lldb/include/lldb/Target/TargetList.h:192
+ /// need to ask whetehr this target contains this module.
+ bool AnyTargetContainsModule(Module *module);
mib wrote:
> Can the module be null ? If not, can we pass a reference instead of a raw
> pointer here ?
+1
Alternatively, if these are being pulled out of shared pointers, pass the
shared pointer directly so we can avoid the issue of dangling raw pointers if
the module ever is freed by the shared pointer.
Comment at: lldb/include/lldb/Target/TargetList.h:200
collection m_target_list;
+ std::unordered_set m_in_process_target_list;
mutable std::recursive_mutex m_target_list_mutex;
mib wrote:
> Same question as above, can any of the `in_process_target` be null ? If not,
> lets make it a `Target&`
+1
Same as above, let's not circumvent shared pointer semantics since we're
storing references/pointers to these objects.
Comment at: lldb/include/lldb/Target/TargetList.h:216-221
+ void RegisterInProcessTarget(Target *target);
+
+ void UnregisterInProcessTarget(Target *target);
+
+ bool IsTargetInProcess(Target *target);
+
mib wrote:
> ditto
Same as my comment above.
Comment at: lldb/source/API/SBFrame.cpp:57
+#include
+
What is this used for?
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:67-68
+#include
+
mib wrote:
> Is this still necessary ?
Where is this used?
Comment at: lldb/source/Core/Debugger.cpp:1266-1267
-bool Debugger::InterruptRequested() {
+bool Debugger::InterruptRequested()
+{
// This is the one we should call internally. This will return true either
Did clang-format give this to you?
Comment at: lldb/source/Core/Debugger.cpp:1291-1292
+// For now, just log the description:
+ Log *log = GetLog(LLDBLog::Host);
+ LLDB_LOG(log, "Interruption: {0}", report.m_description);
+}
Comment at: lldb/source/Core/Module.cpp:1075
+for (auto debugger_sp : interruptors) {
+ REPORT_INTERRUPTION(*(debugger_sp.get()),
+ "Interrupted fetching symbols for module {0}",
Comment at: lldb/source/Core/Module.cpp:1077
+ "Interrupted fetching symbols for module {0}",
+ this->GetFileSpec());
+}
I don't think you need to specify `this->`?
Comment at: lldb/source/Ta
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
mib added a comment.
This looks very cool! I left "a few" comments on how I think we can simplify
this patch, and also had some remarks regarding the `in_process_target` sanity
checks.
Comment at: lldb/include/lldb/Core/Debugger.h:435-436
+ ReportInterruption(InterruptionReport(cur_func,
+llvm::formatv(formatv,
+std::forward(args)...)));
+}
If you use `LLVM_PRETTY_FUNCTION` you won't need this.
Comment at: lldb/include/lldb/Core/Debugger.h:446
+#define INTERRUPT_REQUESTED(debugger, ...) \
+(debugger).InterruptRequested(__func__, __VA_ARGS__)
+
You could use `LLVM_PRETTY_FUNCTION` which will give you both the function
signature and the argument well formatted.
Comment at: lldb/include/lldb/Core/Debugger.h:454
+ public:
+InterruptionReport(std::string function_name, std::string description) :
+m_function_name(function_name),
Comment at: lldb/include/lldb/Core/Debugger.h:460-468
+InterruptionReport(std::string function_name,
+const llvm::formatv_object_base &payload);
+
+ template
+ InterruptionReport(std::string function_name,
+ const char *format, Args &&... args) :
+InterruptionReport(function_name, llvm::formatv(format,
std::forward(args)...)) {}
Using `LLVM_PRETTY_FUNCTION` we can simplify the class by removing the variadic
constructors and replace to `std::string` with a `llvm::StringLiteral`
Comment at: lldb/include/lldb/Target/TargetList.h:192
+ /// need to ask whetehr this target contains this module.
+ bool AnyTargetContainsModule(Module *module);
Can the module be null ? If not, can we pass a reference instead of a raw
pointer here ?
Comment at: lldb/include/lldb/Target/TargetList.h:200
collection m_target_list;
+ std::unordered_set m_in_process_target_list;
mutable std::recursive_mutex m_target_list_mutex;
Same question as above, can any of the `in_process_target` be null ? If not,
lets make it a `Target&`
Comment at: lldb/include/lldb/Target/TargetList.h:216
+ void RegisterInProcessTarget(Target *target);
+
ditto
Comment at: lldb/include/lldb/Target/TargetList.h:218
+
+ void UnregisterInProcessTarget(Target *target);
+
ditto
Comment at: lldb/include/lldb/Target/TargetList.h:220
+
+ bool IsTargetInProcess(Target *target);
+
ditto
Comment at: lldb/source/API/SBFrame.cpp:57-58
+#include
+
using namespace lldb;
Is this still necessary ?
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:67-68
+#include
+
Is this still necessary ?
Comment at: lldb/source/Core/Debugger.cpp:1279-1287
+Debugger::InterruptionReport::InterruptionReport(std::string function_name,
+const llvm::formatv_object_base &payload) :
+m_function_name(function_name),
+m_interrupt_time(std::chrono::system_clock::now()),
+m_thread_id(llvm::get_threadid())
+{
+ llvm::raw_string_ostream desc(m_description);
You can get rid of this is you use `LLVM_PRETTY_FUNCTION`.
Comment at: lldb/source/Core/Debugger.cpp:1289-1293
+void Debugger::ReportInterruption(const InterruptionReport &report) {
+// For now, just log the description:
+ Log *log = GetLog(LLDBLog::Host);
+ LLDB_LOG(log, "Interruption: {0}", report.m_description);
+}
It would be nice if `InterruptionReport` had a `Dump` method.
Comment at: lldb/source/Core/Module.cpp:1071-1072
if (!m_did_load_symfile.load() && can_create) {
+ Debugger::DebuggerList interruptors
+ = DebuggersOwningModuleRequestingInterruption(this);
+ if (!interruptors.empty()) {
You can get the Module reference by dereferencing `this`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154542/new/
https://reviews.llvm.org/D154542
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb
jingham created this revision.
jingham added reviewers: JDevlieghere, mib, bulbazord, labath.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This patch enhances the interruption features I added a little while back.
There are two main changes. The main one here is that I made a nicer interface
to reporting interruption events. I also added a way to ask a debugger if it
was using a Module, so that we can query for interruption in Module code where
we don't have access to a debugger. I used that to support interrupting debug
info reading on a per-module basis - since that's a long-running but - at least
on a module boundary - resumable operation.
The current method of checking "Does this module one belong to a debugger
requesting interruption" is quick and dirty here. IMO, a better solution would
be to have Modules keep a list of the Debuggers (or maybe Targets) using them.
But we should figure out all the ways we want to use that and design something
nice for the purpose.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154542
Files:
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Target/TargetList.h
lldb/source/API/SBFrame.cpp
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/Module.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetList.cpp
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
return error;
}
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+ debugger.GetTargetList().RegisterInProcessTarget(target_sp.get());
target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
if (user_exe_path_is_bundle)
exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
// No file was specified, just create an empty target with any arch if a
// valid arch was specified
target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp.get());
}
if (!target_sp)
@@ -513,6 +515,7 @@
void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
"target already exists it the list");
+ UnregisterInProcessTarget(target_sp.get());
m_target_list.push_back(std::move(target_sp));
if (do_select)
SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
m_selected_target_idx = 0;
return GetTargetAtIndex(m_selected_target_idx);
}
+
+bool TargetList::AnyTargetContainsModule(Module *module) {
+ std::lock_guard guard(m_target_list_mutex);
+ for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(module))
+ return true;
+ }
+ for (const auto target: m_in_process_target_list) {
+if (target->GetImages().FindModule(module))
+ return true;
+ }
+ return false;
+}
+
+ void TargetList::RegisterInProcessTarget(Target *target) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target);
+assert(was_added && "Target pointer was left in the in-process map");
+ }
+
+ void TargetList::UnregisterInProcessTarget(Target *target) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target);
+assert(was_present && "Target pointer being removed was not registered");
+ }
+
+ bool TargetList::IsTargetInProcess(Target *target) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target) == 1;
+ }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
// each library in parallel.
if (GetPreloadSymbols())
module_sp->PreloadSymbols();
-
llvm::SmallVector replaced_modules;
for (ModuleSP &old_module_sp : old_modules) {
if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
}
bool TargetProperties::GetPreloadSymbols() const {
+ if (INTERRUPT_REQUESTED(m_target->GetDebugger(),
+ "Interrupted checking preload symbols")) {
+return false;
+ }
const uint32_t idx = ePropertyPreloadSymbols;
return GetPropertyAtIndexAs(
idx, g_tar
