Vedant, thank you! I had meant to ask if any of this reminded you all
of something else that I could emulate. I'll look into uses of
'replaceDbgDeclare' in SafeStack/ASan. - Brian

On Wed, Feb 26, 2020 at 5:08 PM Vedant Kumar <vedant_ku...@apple.com> wrote:
>
> I haven't fully parsed this thread (sorry!), but I wanted to briefly mention 
> that the SafeStack & ASan passes both do something similar (I think): move 
> local variables backed by allocas onto a separate stack. These passes use 
> replaceDbgDeclare to rewrite dbg.declares s.t. they point into the new stack. 
> After that, llvm presumably runs LowerDbgDeclare (usually via InstCombine), 
> but all the inserted dbg.values have useful locations relative to the new 
> stack.
>
> It could be worth investigating whether replaceDbgDeclare is a good fit for 
> the coro-split pass.
>
> vedant
>
> On Feb 26, 2020, at 1:32 PM, Brian Gesiak via llvm-dev 
> <llvm-...@lists.llvm.org> wrote:
>
> Awesome, thanks Jeremy.
>
> On Wed, Feb 26, 2020 at 11:02 AM Jeremy Morse
> <jeremy.morse.l...@gmail.com> wrote:
>
>
> Hi Brian,
>
> On Tue, Feb 25, 2020 at 7:43 PM Brian Gesiak <modoca...@gmail.com> wrote:
>
> In other words, the value of %i is stored on the frame object, on the
> heap, at an offset of 7 into the frame. I'm beginning to think a
> fundamental fix for this issue would be to stop replacing
> llvm.dbg.declare with llvm.dbg.value, and instead replace the
> llvm.dbg.declare with llvm.dbg.addr that points the debugger to the %i
> variable's new permanent location as an offset into the coroutine
> frame object. Does this approach make sense to people on this mailing
> list, who probably know more about how these intrinsics work than I
> do?
>
>
> This matches a few similar use cases that I'm aware of -- certain
> kinds of struct that are passed-by-value according to the language,
> but passed-by-reference according to ABI, are treated in that way. In
> general, the downside is that the debugger can only observe variable
> values when they get written to memory, not when they're computed, as
> dbg.values and dbg.declares aren't supposed to be mixed. Observing
> variable values slightly later might be an improvement over the
> current situation.
>
>
> This is very reassuring, thank you!
>
> Although, I don't think this will work immediately, see below,
>
> I tried multiple approaches to manually inserting an llvm.dbg.addr
> after the store instruction, as per your suggestion, Jeremy. I used
> llc to compile the IR into an object file that I then linked, and
> inspected the DWARF generated for the file. Unfortunately, inserting
> dbg.addr that operated on the reloaded values didn't lead to any
> change in the DWARF that was produced --  specifically, this didn't
> make a difference:
>
> call void @llvm.dbg.addr(metadata i32* %i.reload.addr62, metadata
> !873, metadata !DIExpression()), !dbg !884
>
>
> Ouch, I tried this myself, and ran into the same difficulty. I'd
> missed that all your functions are marked "optnone" / -O0, which means
> a different instruction-selection pass (FastISel) runs, and it turns
> out FastISel isn't aware of dbg.addrs existence. Even better, FastISel
> doesn't manage to lower any debug intrinsic (including dbg.declare)
> that refers to a GEP, because it doesn't have a register location (the
> GEP gets folded into a memory addressing mode).
>
> I've hacked together some support in [0], that allows dbg.addr's of
> GEPs to be handled. A single dbg.addr at the start of the function
> (and no dbg.values) should get you the same behaviour as a
> dbg.declare.
>
>
> Maybe I'm running into the limitations of how far I can get here by
> hacking up the LLVM IR directly, but I've made a couple of tries here
> without any luck. I put the IR files and the DWARF dumps of
> executables built from that IR in another GitHub gist,
> https://gist.github.com/modocache/8a7b12eb42012990ba534787c4a47275.
> Here's a summary of the attempts I've made -- all of them with your
> patch included in my llvm-project tree:
>
> 1. Don't lower llvm.dbg.declare to llvm.dbg.value at all, no other
> changes -- doesn't work, in the way I expected it to not work
>
> I tried removing the call to llvm::LowerDbgDeclare that's being made
> by the coroutine passes. The IR and DWARF dump after having done so
> are in the files 'repro.declare.ll' and 'repro.declare.dwarf.txt'.
>
> Using an executable built from this IR, I can use lldb to break at the
> line where the declare exists,
> https://gist.github.com/modocache/8a7b12eb42012990ba534787c4a47275#file-repro-declare-ll-L167,
> using the command 'b repro.cpp:24'. Running 'e i' at this point prints
> an incorrect value, '(int) $0 = 24742', but at least 'i' is found.
>
> Then, continuing past a coroutine "suspend point" (a point after which
> the value of 'i' is stored into an offset of the coroutine frame
> object, an outlined coroutine function 'foo.resume' is invoked, 'i' is
> loaded out of an offset on the coroutine frame object, and normally a
> llvm.dbg.value call would have been generated for that load), and then
> breaking at 'b repro.cpp:32', executing 'e i' results in 'error: <user
> expression 1>:1:1: use of undeclared identifier 'i''. This fits my
> mental model, I think: 'i' was declared in the function 'foo', and its
> DWARF information
> https://gist.github.com/modocache/8a7b12eb42012990ba534787c4a47275#file-repro-declare-dwarf-txt-L3711
> only specifies its offset from the 'foo' frame. After the suspend
> point, lldb is stopped in 'foo.resume', and the 'foo' frame is no
> longer active, so lldb determines 'i' is out of scope. Makes sense!
>
> 2. Don't lower llvm.dbg.declare at all, sed to automatically replace
> all llvm.dbg.declare with llvm.dbg.addr -- doesn't work, in an
> unexpected way
>
> The outputs from this are in the files repro.addr.ll and
> repro.addr.dwarf.txt. My understanding based on
> https://llvm.org/docs/SourceLevelDebugging.html#llvm-dbg-addr is that
> llvm.dbg.addr should work at least as well as llvm.dbg.declare does.
> However, when I use it and break at
> https://gist.github.com/modocache/8a7b12eb42012990ba534787c4a47275#file-repro-addr-ll-L167
> ('b repro.cpp:24'), executing 'e i' in lldb outputs 'error: <user
> expression 1>:1:1: use of undeclared identifier 'i''. This threw me
> for a loop -- maybe I'm misunderstanding the relationship between
> llvm.dbg.addr and llvm.dbg.declare?
>
> - Brian Gesiak
>
> I suspect the reason why this problem hasn't shown up in
> the past is because the coroutine code being generated hits a gap
> between "optimised" and "not optimised": I believe all variables in
> code that isn't optimised get their own storage (and so will always
> have a stack or register location). Wheras in the coroutine code
> you're generating the variable address doesn't get storage.
>
> If [0] is useful for you I can get that landed; it'd be good to hear
> whether this resolves the dbg.addr intrinsics not having an affect on
> the output.
>
> [0] 
> https://github.com/jmorse/llvm-project/commit/40927e6c2b71ec914d937287a0c2ca6c52c01f6b
>
> --
> Thanks,
> Jeremy
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Reply via email to