[lldb/Target] Slide source display for artificial locations at function start

[mib]

Summary

It can happen that a line entry reports that some source code is located
at line 0. In DWARF, line 0 is a special location which indicates that
code has no 1-1 mapping with source.

When stopping in one of those artificial locations, lldb doesn't know which
line to display and shows the beginning of the file instead.

This patch mitigates this behaviour by checking if the current symbol context
of the line entry has a matching function, in which case, it slides the
source listing to the start of that function.

This patch also shows the user a warning explaining why lldb couldn't
show sources at that location.

rdar://83118425

Signed-off-by: Med Ismail Bennani <[e-mail]>

[dblaikie]

Side note: The src_file is not to be trusted/used either - once line 0 is specified, nothing else in that line entry is valid. LLVM lets the previous line entries file persist because this reduces encoding size (by not having to switch all the fields in the line table - only the line number - back and forth over a line number 0 area). eg: previous entry in the line table might be from a #include of some code (as in clang/llvm's use of .def files, for instance) into a function, or from some inlining above the line 0 region, etc.

So maybe "artificial location in function <X>" might be suitable? (the actual code at line 0 might still be from some inlining (LLVM does try to scope it - so the instruction should have the nearest common scope (in terms of lexical scopes or inlined functions) so if A has B inlined, B has C and D inlined into it and some code is commoned between C and D, it should be attributed to the inlined region of B - but if it's hoisted out of a basic block, I don't think we can properly attribute it to any scope, and so we'd have to attribute it to A in the scope DIE information (in all these cases it'd still have line 0, though).

[mib]

In D115313#3178578, @dblaikie wrote:

Side note: The src_file is not to be trusted/used either - once line 0 is specified, nothing else in that line entry is valid. LLVM lets the previous line entries file persist because this reduces encoding size (by not having to switch all the fields in the line table - only the line number - back and forth over a line number 0 area). eg: previous entry in the line table might be from a #include of some code (as in clang/llvm's use of .def files, for instance) into a function, or from some inlining above the line 0 region, etc.

So maybe "artificial location in function <X>" might be suitable? (the actual code at line 0 might still be from some inlining (LLVM does try to scope it - so the instruction should have the nearest common scope (in terms of lexical scopes or inlined functions) so if A has B inlined, B has C and D inlined into it and some code is commoned between C and D, it should be attributed to the inlined region of B - but if it's hoisted out of a basic block, I don't think we can properly attribute it to any scope, and so we'd have to attribute it to A in the scope DIE information (in all these cases it'd still have line 0, though).

Thanks for clarifying @DavidSpickett ! I'll change the warning accordingly.

[mib]

Change warning to specify function name instead of source file name when possible.

[jingham]

I don't think the num_lines check is handled correctly.

lldb/source/Target/StackFrame.cpp

1898–1912

I don't think it is correct to include !num_lines in this if. You get 0 back from DIsplaySourceLines... if you found a good line table entry but the source file was not available. But that doesn't mean that this is artificial code.

You also shouldn't remove the TO DO since you didn't do it.

1907

This seems a confusing way to say "but we can't tell what function it is in..."

[mib]

lldb/source/Target/StackFrame.cpp

1898–1912

Right (wrt to the condition).

I reverted the TODO and will take care of it in a separate patch.

[mib]

Address @jingham feedbacks.

[mib]

Update the condition for printing the warning.

[mib]

Dropping the LLDB_INVALID_LINE_NUMBER check as it should only occur when we have no debug info

[jingham]

That looks right.

[JDevlieghere]

lldb/test/API/source-manager/TestSourceManager.py

279

Should this print the "in function" part too? If so, why not check for it here?

[mib]

Address @JDevlieghere comment regarding test.

[DavidSpickett]

Thanks for clarifying @DavidSpickett ! I'll change the warning accordingly.

Too many Davids :)

[aprantl]

Thanks! I have some late wording suggestions.

lldb/source/Target/StackFrame.cpp

1904

This message might be confusing to developers. End-users won't know what "PC" and "artificial" means. What do you think about:

"Note: this address is compiler-generated code in function %s that has no source code associated with it."

?

1909

"Note: this address is compiler-generated code that has no source code associated with it."

[mib]

In D115313#3182258, @DavidSpickett wrote:

Thanks for clarifying @DavidSpickett ! I'll change the warning accordingly.

Too many Davids :)

😅😅

lldb/source/Target/StackFrame.cpp

if (target) {
       const uint32_t source_lines_after =
           debugger.GetStopSourceLineCount(false);
       disasm_display = debugger.GetStopDisassemblyDisplay();
 
       GetSymbolContext(eSymbolContextCompUnit | eSymbolContextLineEntry);
       if (m_sc.comp_unit && m_sc.line_entry.IsValid()) {
         have_debuginfo = true;
         if (source_lines_before > 0 || source_lines_after > 0) {
+          uint32_t start_line = m_sc.line_entry.line;
+          if (!start_line && m_sc.function) {
+            FileSpec source_file;
+            m_sc.function->GetStartLineSourceInfo(source_file, start_line);
+          }
+
           size_t num_lines =
               target->GetSourceManager().DisplaySourceLinesWithLineNumbers(
-                  m_sc.line_entry.file, m_sc.line_entry.line,
-                  m_sc.line_entry.column, source_lines_before,
-                  source_lines_after, "->", &strm);
+                  m_sc.line_entry.file, start_line, m_sc.line_entry.column,
+                  source_lines_before, source_lines_after, "->", &strm);
           if (num_lines != 0)
             have_source = true;
           // TODO: Give here a one time warning if source file is missing.
+          if (!m_sc.line_entry.line) {
+            ConstString fn_name = m_sc.GetFunctionName();
+
+            if (!fn_name.IsEmpty())
+              strm.Printf("Warning: the current PC is an artificial location "
+                          "in function %s.",
+                          fn_name.AsCString());
+            else
+              strm.Printf(
+                  "Warning: the current PC is an artificial location "
+                  "but lldb couldn't associate it with a function in %s.",
+                  m_sc.line_entry.file.GetFilename().GetCString());
+            strm.EOL();
+          }
         }
       }
       switch (disasm_display) {
       case Debugger::eStopDisassemblyTypeNever:
         break;
 
       case Debugger::eStopDisassemblyTypeNoDebugInfo:
         if (have_debuginfo)

lldb/test/API/source-manager/TestSourceManager.py

def test_modify_source_file_while_debugging(self):
 
         # Do the same thing with a file & line spec:
         self.expect(
             "source list -y main-copy.c:%d" %
             self.line,
             SOURCE_DISPLAYED_CORRECTLY,
             substrs=['Hello world'])
 
-        
         # The '-b' option shows the line table locations from the debug information
         # that indicates valid places to set source level breakpoints.
 
         # The file to display is implicit in this case.
         self.runCmd("source list -l %d -c 3 -b" % self.line)
         output = self.res.GetOutput().splitlines()[0]
 
         # If the breakpoint set command succeeded, we should expect a positive number
def test_set_breakpoint_with_absolute_path(self):
 
         self.runCmd("run", RUN_SUCCEEDED)
 
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'main-copy.c:%d' % self.line,
                              'stop reason = breakpoint'])
+
+    def test_artificial_source_location(self):
+        src_file = 'artificial_location.c'
+        d = {'C_SOURCES': src_file }
+        self.build(dictionary=d)
+
+        lldbutil.run_to_source_breakpoint(
+            self, 'main',
+            lldb.SBFileSpec(src_file, False))
+
+        self.expect("run", RUN_SUCCEEDED,
+                    substrs=['stop reason = breakpoint', '%s:%d' % (src_file,0),
+                             'Warning: the current PC is an artificial ',
+                             'location in function '
+                             ])
+

lldb/test/API/source-manager/artificial_location.c

+int foo() { return 42; }
+
+int main() {
+#line 0
+  return foo();
+}