[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

Fri Feb 18 02:31:28 PST 2022

labath added a comment.

In D119963#3329973 <https://reviews.llvm.org/D119963#3329973>, @jingham wrote:

> There was a question on the dev list a while ago about to print out all the valid ranges of a variable.  That's a useful bit of info if you're trying to figure out where you could break to actually see or change a variable's value in an optimized function.  I think that's the motivation for the change, so just showing the range that encompasses the address wouldn't satisfy that initial impetus for the change.

I'm pretty sure the immediate motivation is testing of D119508 <https://reviews.llvm.org/D119508>, but yes, if there was another feature request for something like this, then it makes sense to implement it more fully.

================
Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s:28-29
 # CHECK:     Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location =
 # CHECK-NEXT:  [0x0000000000000000, 0x0000000000000001): DW_OP_reg5 RDI
 # CHECK-NEXT:  [0x0000000000000001, 0x0000000000000006): DW_OP_reg0 RAX
 # CHECK:     Variable{{.*}}, name = "x1", {{.*}}, scope = parameter
----------------
zequanwu wrote:
> labath wrote:
> > zequanwu wrote:
> > > `image dump symfile` already prints valid ranges for variables along with where the value is at each range.
> > Are you sure it does?
> > 
> > I was under the impression that there are two distinct range concepts being combined here. One is the range list member of the Variable object (as given by `GetScopeRange` -- that's the one you're printing now), and the other is the list of ranges hidden in the DWARFExpression object, which come from the debug_loc(lists) section (that's the one we've been printing so far). And that the root cause of the confusion is the very existence of these two concepts.
> > 
> > If I got it wrong, then do let me know, cause it would make things a lot simpler if there is only one validity concept to think about.
> Dwarf plugin is supposed to construct the `m_scope_range` member of an Variable, but it doesn't. `scope_ranges` is empty at https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3468. 
> `image dump symfile` dumps the dwarf location list in `m_location` in `Variable`. 
> The dwarf location list has more information than `m_scope_range` as it contains info about where the value is during each range. (e.g. which register the variable lives in). 
> 
> So, I think we need to use similar logic to construct `m_scope_range` when creating `Variable` in dwarf plugin like this https://github.com/llvm/llvm-project/blob/main/lldb/source/Expression/DWARFExpression.cpp#L145.
Ok, I see where you're coming from. You're essentially saying that the fact that the dwarf plugin does not fill this out is a bug.

I don't think that's the case. My interpretation was (and [[ https://github.com/llvm/llvm-project/blob/main/lldb/source/Symbol/Variable.cpp#L313 | this comment]] confirms it) that an empty range here means the entire enclosing block. (Also, DWARF was for a long time the only symbol file plugin, so what it does is kinda "correct by definition").

I don't think we want to change that interpretation, as forcing a copy of the range in the location list would be wasteful (it would be different if this was an interface that one could query, and that the dwarf plugin could implement by consulting the location list). However, since the dwarf class does not actually make use of this functionality (it was [[ https://reviews.llvm.org/D17449 | added ]] to support DW_AT_start_scope, then broken at some point, and eventually [[ https://reviews.llvm.org/D62302 | removed ]]), we do have some freedom in defining the interactions of the two fields (if you still want to pursue this, that is).

So how about this: if the user passes the extra flag, then we print both the range field (if it exists), and the *full* location list (in that order, ideally). That way the output will be either `range = [a, b), [c, d), location = DW_OP_reg47` or `location = [a,b) -> DW_OP_reg4, [c,d) -> DW_OP_reg7`. If the dwarf plugin starts using the range field again then the output will contain both fields, which will be slightly confusing, but at least not misleading (and we can also change the format then).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963