[lldb] Fix breakpoint setting so it always works when there is a line entry in a compile unit's line table.

[clayborg]

Summary

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.

[jingham]

Seems reasonable that when resolving the start address of a line entry from CU A we get back a line entry in CU B, then there's just something wrong in the debug info. Trying to recover whatever info is recoverable seems worthwhile: essentially by falling back to a resolve_scope of eSymbolContextLineEntry.

However, that does produce a fairly odd SymbolContext. Did we ever have a case where you can have aSymbolContext with a valid CU & LineEntry that is in no function and no block for realz? I wouldn't be altogether surprised if we had code that asks to resolve everything in ResolveSymbolContext and if we get a CU & LineEntry back, assumes you must also have a block or function, and uses them w/o checking. I can't think of a good way to defend against this except maybe saying explicitly in SymbolContext.h that because of potential bad debug info you can't make any assumptions about which entities will get filled in in a symbol context.

I also found the first of the two comments you added confusing.

lldb/source/Symbol/CompileUnit.cpp

336

This comment seems odd to me. You say "X might not be something we want to do for reason Y" right before you actually do do X.

The following comment make it seem like you more mean "The address lookup might be problematic for reasons A & B which we're going to detect in way C & fix by setting the SC by hand, which doesn't really jibe with what the first comment said.

The first comment makes me think we shouldn't be calling CalculateSymbolContext at all and do something else - it's not clear what that would be, however. So I think it is just confusing.

[clayborg]

Added missing test case file and clarified comments to be more consistent.

[clayborg]

In D136207#3866729, @jingham wrote:

Seems reasonable that when resolving the start address of a line entry from CU A we get back a line entry in CU B, then there's just something wrong in the debug info. Trying to recover whatever info is recoverable seems worthwhile: essentially by falling back to a resolve_scope of eSymbolContextLineEntry.

However, that does produce a fairly odd SymbolContext. Did we ever have a case where you can have aSymbolContext with a valid CU & LineEntry that is in no function and no block for realz? I wouldn't be altogether surprised if we had code that asks to resolve everything in ResolveSymbolContext and if we get a CU & LineEntry back, assumes you must also have a block or function, and uses them w/o checking. I can't think of a good way to defend against this except maybe saying explicitly in SymbolContext.h that because of potential bad debug info you can't make any assumptions about which entities will get filled in in a symbol context.

I also found the first of the two comments you added confusing.

I didn't mean to leave the first comment it actually. When I first figured out the root cause I put that in there. Let me know if the current state of the comments makes more sense

[clayborg]

Added missing file for real this time.

[labath]

lldb/source/Symbol/CompileUnit.cpp

336

I think this would be more robustly written as something like

line_entry.range.GetBaseAddress().CalculateSymbolContext(&new_sc, resolve_scope);
if (new_sc.comp_unit == this)
  sc_list.Append(new_sc);
else
  sc_list.Append(sc);

• basically using a new symbol context for the resolution, instead of twiddling the old one back and forth.

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

109

I would hope that the yaml-based test can run on windows as well.

[clayborg]

Create a new "resolved_sc" that gets used to resolve the line_entry's address. If the resolving goes to plan, append that symbol context, else append the original one.

[clayborg]

Also test on windows.

lldb/source/Symbol/CompileUnit.cpp

void CompileUnit::ResolveSymbolContext(
 
   while (line_idx != UINT32_MAX) {
     // If they only asked for the line entry, then we're done, we can
     // just copy that over. But if they wanted more than just the line
     // number, fill it in.
     if (resolve_scope == eSymbolContextLineEntry) {
       sc.line_entry = line_entry;
     } else {
       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 in another compiler
+      // unit if the DWARF has overlappging address ranges. So if we end up with
+      // no compile unit or a different one after the above function call,
+      // revert back to the same results as if resolve_scope was set exactly to
+      // eSymbolContextLineEntry.
+      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 link time optimizations. If the compile unit is NULL, then
+          // address resolving is completely failing and more deserving of an
+          // error message the user can see.
+          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());
+        }
+        // Revert to equivalent of resolve_scope == eSymbolContextLineEntry.
+        sc.comp_unit = this;
+        sc.function = nullptr;
+        sc.block = nullptr;
+        sc.line_entry = line_entry;
+      }
     }
 
     sc_list.Append(sc);
     if (num_file_indexes == 1)
       line_idx = line_table->FindLineEntryIndexByFileIndex(
           line_idx + 1, file_indexes.front(), found_entry, &line_entry);
     else
       line_idx = line_table->FindLineEntryIndexByFileIndex(

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

def test_breakpoints_with_relative_path_line_tables(self):
         # Reset source map.
         self.runCmd("settings clear target.source-map")
         for path in invalid_paths:
             bkpt = target.BreakpointCreateByLocation(path, 2)
             self.assertTrue(bkpt.GetNumLocations() == 0,
                 '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().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.line = line_number('main.c', '// Set break point at this line.')
         # disable "There is a running process, kill it and restart?" prompt
         self.runCmd("settings set auto-confirm true")