[Lldb-commits] [PATCH] D115313: [lldb/Target] Slide source display for artificial locations at function start

Wed Dec 8 12:13:50 PST 2021

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

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

================
Comment at: lldb/source/Target/StackFrame.cpp:1898
             have_source = true;
-          // TODO: Give here a one time warning if source file is missing.
+          if (!num_lines || !m_sc.line_entry.line ||
+              m_sc.line_entry.line == LLDB_INVALID_LINE_NUMBER) {
----------------
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.

================
Comment at: lldb/source/Target/StackFrame.cpp:1907
+            else
+              strm.Printf("Warning: the current PC is an artificial location "
+                          "unassociated with a particular line in %s.",
----------------
This seems a confusing way to say "but we can't tell what function it is in..."

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115313/new/

https://reviews.llvm.org/D115313