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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to