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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

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 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've must 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.



================
Comment at: lldb/source/Symbol/CompileUnit.cpp:336
     } 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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136207