dblaikie added inline comments.
================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:769-783
+ DataExtractor Data(Location.Expr, /*IsLittleEndian=*/true, 8);
+ uint64_t DataOffset = 0;
+ uint8_t Operation = Data.getU8(&DataOffset);
+ if (Operation == dwarf::DW_OP_addr) {
+ uint64_t Pointer = Data.getAddress(&DataOffset);
+ VariableDieMap[Pointer] = Die;
+ return;
----------------
hctim wrote:
> dblaikie wrote:
> > Are there any examples of global merge where this might not be correct?
> >
> > Like if a global was described as "addrx, 5, add" (eg: 5 beyond this
> > address) then only looking at the first operation might mis-conclude that
> > the variable is at this address when it's at 5 bytes past this address -
> > and some other variable is at this address)
> >
> > LLVM has a "global merge" optimization that might cause something like
> > this. Let's see if I can create an example.
> >
> > Ah, yeah, here we go:
> > ```
> > static int x;
> > static int y;
> > int *f(bool b) { return b ? &x : &y; }
> > ```
> > ```
> > $ clang++-tot -O3 merge.cpp -c -g -target aarch64-linux-gnuabi -mllvm
> > -global-merge-ignore-single-use && llvm-dwarfdump-tot merge.o | grep
> > DW_AT_location
> > DW_AT_location (DW_OP_addrx 0x0)
> > DW_AT_location (DW_OP_addrx 0x0, DW_OP_plus_uconst 0x4)
> > ```
> >
> > (the `-global-merge-ignore-single-use` is to simplify the test case -
> > without that it could still be tickled with a more complicated example -
> > seems we only enable global merge on ARM 32 and 64 bit due to the higher
> > register pressure there, by the sounds of it)
> >
> > I'm guessing this patch would overwrite the VariableDieMap entry for the
> > first global with the second one so queries for the first address would
> > result in the second DIE and queries for the second address wouldn't give
> > any result?
> >
> > Hmm, also - how does this handle queries that aren't at exactly the
> > starting address of the variable? Presumably the `DW_AT_type` of the
> > `DW_TAG_global_variable` would have to be inspected to find the size of the
> > variable starting at the address, and any query in that range should be
> > successful?
> I've added some handling for the `DW_OP_plus_uconst`. Unfortunately I didn't
> find any generic existing handling for the expression evaluation (which
> surprised me, maybe I just suck at looking), so I'm just making the
> assumption that global variable addresses aren't described using `DW_OP_plus`
> or anything else...
>
> Fixed it up to now provide line info when you provide an address halfway
> through a symbol. Good thing there wasn't a big impact in D123534 about
> keeping the string sizes around :). Worst case (if the type info sucks) we
> just only accept the precise start addresses of the symbols.
Probably worth maybe /only/ matching these expressions, then, rather than
currently it's matching any expression that starts with addr, or addr+plus?
Even if that's followed by any other garbage that would be ignored (& so the
expression would be misinterpreted)
There's probably no expression evaluation logic in LLVM's libDebugInfoDWARF -
there's no need to evaluate expressions when symbolizing, and dwarfdump just
wants to dump it, not compute the resulting value. (some similar code exists in
lldb that would do evaluation).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123538/new/
https://reviews.llvm.org/D123538
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
