[Lldb-commits] [PATCH] D136207: Fix breakpoint setting so it always works when there is a line entry in a compile unit's line table.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, aadsm, yinghuitan.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

Prior to this fix, if the compile unit function:

  void CompileUnit::ResolveSymbolContext(const SourceLocationSpec &src_location_spec, SymbolContextItem resolve_scope, SymbolContextList &sc_list);

was called with a resolve scope that wasn't just eSymbolContextLineEntry, we would end up calling:

  line_entry.range.GetBaseAddress().CalculateSymbolContext(&sc, resolve_scope);

This is ok as long as the line entry's base address is able to be resolved back to the same information, but there were problems when it didn't. The example I found was we have a file with a bad .debug_aranges section where the address to compile unit mapping was incomplete. When this happens, the above function call to calculate the symbol context would end up matching the module and it would NULL out the compile unit and line entry, which means we would fail to set this breakpoint. We have many other clients that ask for eSymbolContextEverything as the resolve_scope, so all other locations could end up failing as well.

The solutions is to make sure the compile unit matches the current compile unit after calling the calculate symbol context. If the compile unit is NULL, then we report an error via the module/debugger as this indicates an entry in the line table fails to resolve back to any compile unit. If the compile unit is not NULL and it differs from the current compile unit, we restore the current compile unit and line entry to ensure the call to .CalculateSymbolContext doesn't match something completely different, as can easily happen if LTO or other link time optimizations are enabled that could end up outlining or merging functions.

This patch allows breakpoint succeeding to work as expected and not get short circuited by our address lookup logic failing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136207

Files:
  lldb/source/Symbol/CompileUnit.cpp
  lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py


Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===================================================================
--- lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -106,6 +106,44 @@
                 'Incorrectly resolved a breakpoint using full path "%s" with '
                 'debug info that has relative path with matching suffix' % (path))
 
+    @skipIf(oslist=["windows"])
+    @no_debug_info_test
+    def test_breakpoints_with_bad_aranges(self):
+        """
+            Test that we can set breakpoints in a file that has an invalid
+            .debug_aranges. Older versions of LLDB would find a line entry
+            in the line table and then would use the start address of the line
+            entry to do an address lookup on the entry from the line table. If
+            this address to symbol context lookup would fail, due to a bad
+            .debug_aranges, it would cause the breakpoint to not get resolved.
+            Verify that even in these conditions we are able to resolve a
+            breakpoint.
+
+            The "bad_aranges.yaml" contains a line table that is:
+
+            Line table for /tmp/ab/main.cpp in `a.out
+            0x0000000100003f94: /tmp/ab/main.cpp:1
+            0x0000000100003fb0: /tmp/ab/main.cpp:2:3
+            0x0000000100003fb8: /tmp/ab/main.cpp:2:3
+
+            The .debug_aranges has one range for this compile unit that is
+            invalid: [0x0000000200003f94-0x0000000200003fb8). This will cause
+            the resolving of the addresses to fail.
+        """
+        src_dir = self.getSourceDir()
+        yaml_path = os.path.join(src_dir, "bad_aranges.yaml")
+        yaml_base, ext = os.path.splitext(yaml_path)
+        obj_path = self.getBuildArtifact("a.out")
+        self.yaml2obj(yaml_path, obj_path)
+
+        # Create a target with the object file we just created from YAML
+        target = self.dbg.CreateTarget(obj_path)
+        src_path = '/tmp/ab/main.cpp'
+        bkpt = target.BreakpointCreateByLocation(src_path, 2)
+        self.assertTrue(bkpt.GetNumLocations() > 0,
+            'Couldn\'t resolve breakpoint using "%s" in executate "%s" with '
+            'debug info that has a bad .debug_aranges section' % (src_path, self.getBuildArtifact("a.out")))
+
 
     def setUp(self):
         # Call super's setUp().
Index: lldb/source/Symbol/CompileUnit.cpp
===================================================================
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -333,8 +333,38 @@
     if (resolve_scope == eSymbolContextLineEntry) {
       sc.line_entry = line_entry;
     } else {
+      // Doing a reverse address lookup based on the line entry might not be
+      // what we want to do. Why? We might have an address that appears in
+      // multiple compile units due to function outlining or LTO passes and we
+      // could end up with a completely different line entry...
       line_entry.range.GetBaseAddress().CalculateSymbolContext(&sc,
                                                                resolve_scope);
+      // Sometimes debug info is bad and isn't able to resolve the line entry's
+      // address back to the same compile unit and/or line entry. If the compile
+      // unit changed, then revert back to just the compile unit and line entry.
+      // Prior to this fix, the above code might end up not being able to lookup
+      // the address, and then it would clear compile unit and the line entry in
+      // the symbol context and the breakpoint would fail to get set even though
+      // we have a valid line table entry in this compile unit. The address
+      // lookup can also end up finding another function if the DWARF has
+      // overlappging address ranges. If we end up resolving to another compile
+      // unit, revert to the info from this compile unit only.
+      if (sc.comp_unit != this) {
+        if (sc.comp_unit == nullptr && sc.module_sp) {
+          // Only report an error if we don't map back to any compile unit. With
+          // link time optimizations, the debug info might have many compile
+          // units that have the same address range due to function outlining
+          // or other linke time optimizations.
+          sc.module_sp->ReportError(
+              "unable to resolve a line table file address 0x%" PRIx64 " back "
+              "to a compile unit, please file a bug and attach the address "
+              "and file.", line_entry.range.GetBaseAddress().GetFileAddress());
+        }
+        sc.comp_unit = this;
+        sc.function = nullptr;
+        sc.block = nullptr;
+        sc.line_entry = line_entry;
+      }
     }
 
     sc_list.Append(sc);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136207.468719.patch
Type: text/x-patch
Size: 4889 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20221018/ed3effbd/attachment-0001.bin>