[lldb] Fix breakpoint resolver serialization bug

[bulbazord]

BreakpointResolverAddress optionally can include the module name related to the address that gets resolved. Currently this will never work because it sets the name to itself (which is empty).

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)

Changes

BreakpointResolverAddress optionally can include the module name related to the address that gets resolved. Currently this will never work because it sets the name to itself (which is empty).

---

Full diff: https://github.com/llvm/llvm-project/pull/76766.diff

1 Files Affected:

• (modified) lldb/source/Breakpoint/BreakpointResolverAddress.cpp (+3-7)

diff --git a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
index a0c628a8e299ce..dcdcea101045f7 100644
--- a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -65,13 +65,9 @@ BreakpointResolverAddress::SerializeToStructuredData() {
       new StructuredData::Dictionary());
   SectionSP section_sp = m_addr.GetSection();
   if (section_sp) {
-    ModuleSP module_sp = section_sp->GetModule();
-    ConstString module_name;
-    if (module_sp)
-      module_name.SetCString(module_name.GetCString());
-
-    options_dict_sp->AddStringItem(GetKey(OptionNames::ModuleName),
-                                   module_name.GetCString());
+    if (ModuleSP module_sp = section_sp->GetModule())
+      options_dict_sp->AddStringItem(GetKey(OptionNames::ModuleName),
+                                     module_sp->GetObjectName().GetCString());
     options_dict_sp->AddIntegerItem(GetKey(OptionNames::AddressOffset),
                                     m_addr.GetOffset());
   } else {

[clayborg]

Can this be tested?

[bulbazord]

Can this be tested?

I took a look at existing tests, but I'm not sure how to best test this. I think we would need to create a breakpoint in a module by address and make sure the resolver correctly serializes the name (in statistics dump or otherwise). Does that sound appropriate to you?

[clayborg]

Can this be tested?

I took a look at existing tests, but I'm not sure how to best test this. I think we would need to create a breakpoint in a module by address and make sure the resolver correctly serializes the name (in statistics dump or otherwise). Does that sound appropriate to you?

That can work. Another idea is to create a test that sets a breakpoint by function name where we will find one match. After setting this breakpoint, get the one and only SBBreakpointLocation from this breakpoint and get the SBAddress from the location. Then delete the breakpoint by function name and create one using the SBAddress using:

  lldb::SBBreakpoint lldb::SBTarget::BreakpointCreateBySBAddress(SBAddress &address);

You can then serialize the breakpoints to a JSON file using:

(lldb) breakpoint write -f <path>

Then you can use the JSON to verify the bug is fixed?

[bulbazord]

@clayborg I was able to write a test similar to what you suggested (without needing to set two breakpoints). I also changed the actual fix since a module's object name isn't quite what I wanted in the first place.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[clayborg]

One quick fix suggested, but looks good.

[clayborg]

Looks good!

[bulbazord]

Looks like this broke on aarch64-windows. Taking a look now.

https://lab.llvm.org/buildbot/#/builders/219/builds/7961/steps/6/logs/stdio

[bulbazord]

======================================================================
ERROR: test_resolver_serialization (TestBreakpointSerialization.BreakpointSerialization)
   Test that breakpoint resolvers contain the expected information
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\API\functionalities\breakpoint\serialize\TestBreakpointSerialization.py", line 60, in test_resolver_serialization
    exe_module.IsValid(), "Failed to find the executable module in target"
    ^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'IsValid'
Config=aarch64-C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe
----------------------------------------------------------------------

@clayborg Any idea why the executable module wouldn't be found here? Looks like exe_module is None.

[clayborg]

======================================================================
ERROR: test_resolver_serialization (TestBreakpointSerialization.BreakpointSerialization)
   Test that breakpoint resolvers contain the expected information
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\API\functionalities\breakpoint\serialize\TestBreakpointSerialization.py", line 60, in test_resolver_serialization
    exe_module.IsValid(), "Failed to find the executable module in target"
    ^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'IsValid'
Config=aarch64-C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe
----------------------------------------------------------------------

@clayborg Any idea why the executable module wouldn't be found here? Looks like exe_module is None.

You can just use "a.out" when doing the lookup, do this:

exe_module = self.orig_target.module["a.out"]

The full path isn't needed. I would guess there might be some sort of path issue with the value returned from self.getBuildArtifact("a.out") that doesn't match exactly with the windows path from the module list?

[bulbazord]

Speculative fix in bdaedff