llvm dev - Dec 2016 - Debug Locations for Optimized Code

Hi Kostya,

So... a bunch of folks (not all CC'd - feel free to add anyone who seems
relevant, I haven't gone back over the commits to check who's made what
changes) have been making improvements to optimized code debug information
- much of it around making sure sample profiles are accurate.

This mostly entails removing debug locations from instructions that are
moved between basic blocks.

The justification for this has usually been backed up by "it'll make
users
debug experience better too" because the debugger won't jump around so
much, or give users misleading situations like "why am I stepped into a
line of code in the 'if' block when the condition was clearly
false"
(because we raised a load that was done in both blocks up to before the
branch and had to pick one of the two locations for it)

This may lead to reduction in quality of debug info for other uses - in
particular in ASan and MSan at least.

ASan and MSan care a lot about loads and stores and the location where
those came from - less so about whether the load or store was sunk into or
out of a loop, etc. Dropping the locations entirely means the user has far
less information about which load or store caused the invalid memory
access. (The 'if' case is still tricky and would be confusing to a *San
user as it was to a debug user.)

I don't know what the right, if any, solution to this is - but I thought I
should bring it up in case you or anyone else wanted to puzzle it over &
see if the competing needs/desires might need to be considered.

- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161207/60f68366/attachment.html>

----- Original Message -----
> From: "David Blaikie via llvm-dev" <llvm-dev at
lists.llvm.org>
> To: "llvm-dev" <llvm-dev at lists.llvm.org>, "Kostya
Serebryany"
> <kcc at google.com>
> Sent: Tuesday, December 6, 2016 10:38:50 PM
> Subject: [llvm-dev] Debug Locations for Optimized Code
> Hi Kostya,
> So... a bunch of folks (not all CC'd - feel free to add anyone who
> seems relevant, I haven't gone back over the commits to check who's
> made what changes) have been making improvements to optimized code
> debug information - much of it around making sure sample profiles
> are accurate.
> This mostly entails removing debug locations from instructions that
> are moved between basic blocks.
> The justification for this has usually been backed up by "it'll
make
> users debug experience better too" because the debugger won't jump
> around so much, or give users misleading situations like "why am I
> stepped into a line of code in the 'if' block when the condition
was
> clearly false" (because we raised a load that was done in both
> blocks up to before the branch and had to pick one of the two
> locations for it)
> This may lead to reduction in quality of debug info for other uses -
> in particular in ASan and MSan at least.
> ASan and MSan care a lot about loads and stores and the location
> where those came from - less so about whether the load or store was
> sunk into or out of a loop, etc. Dropping the locations entirely
> means the user has far less information about which load or store
> caused the invalid memory access. (The 'if' case is still tricky
and
> would be confusing to a *San user as it was to a debug user.)
I think that it is always potentially confusing: Having ASAN complain about a
memory access from a loop that the code hasn't entered yet or has already
completed is also difficult to decipher.
> I don't know what the right, if any, solution to this is - but I
> thought I should bring it up in case you or anyone else wanted to
> puzzle it over & see if the competing needs/desires might need to be
> considered.
One thing that I recall being discussed was changing the way that we set the
is_stmt flag in the DWARF line-table information. As I understand it, we
currently set this flag for the first instruction in any sequence that is on the
same line. This is, in part, why the debugger appears to jump around when
stepping through code with speculated instructions, etc. If we did not do this
for out-of-place instructions, then we might be able to keep for debugging
information for tools while still providing a reasonable debugging experience.

-Hal 
> - Dave
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-- 

Hal Finkel 
Lead, Compiler Technology and Programming Languages 
Leadership Computing Facility 
Argonne National Laboratory 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161207/d8185e64/attachment.html>

>> I don't know what the right, if any, solution to this is - but I
>> thought I should bring it up in case you or anyone else wanted to
>> puzzle it over & see if the competing needs/desires might need to
be
>> considered.
> One thing that I recall being discussed was changing the way that we
> set the is_stmt flag in the DWARF line-table information. As I
> understand it, we currently set this flag for the first instruction in
> any sequence that is on the same line. This is, in part, why the
> debugger appears to jump around when stepping through code with
> speculated instructions, etc. If we did not do this for out-of-place
> instructions, then we might be able to keep for debugging information
> for tools while still providing a reasonable debugging experience.
When we are looking at a situation where an instruction is merely *moved*
from one place to another, retaining the source location and having a 
less naïve statement-marking tactic could help the debugging experience 
without perturbing other consumers (although one still wonders whether 
profiles will get messed up in cases where e.g. a loop invariant gets 
hoisted out of a cold loop into a hot predecessor).

When we are looking at a situation where two instructions are *merged* or
*combined* into one, and the original two instructions had different 
source locations, that's a separate problem.  In that case there is no 
single correct source location for the new instruction, and typically 
erasing the source location will give a better debugging experience (also 
a less misleading profile).

My personal opinion is that having sanitizers *rely* on debug info for 
accurate source attribution is just asking for trouble.  It happens to 
work at –O0 but cannot be considered reliable in the face of optimization.
IMO this is a fundamental design flaw; debug info is best-effort and full 
of ambiguities, as shown above. Sanitizers need a more reliable 
source-of-truth, i.e. they should encode source info into their own 
instrumentation.

--paulr

Should it turn out that the needs of instrumentation versus debugging are truly
irreconcilable (and I hope they aren't) we could add an instrumentation
debugger tuning option as a last resort.

-- adrian

Ultimately I agree with Adrian here. I'd also need a compelling argument
that not only are these things irreconcilable, but that there's nothing we
can do as far as information or even dwarf changes to ameliorate the
problems.

Thanks.

-eric

On Wed, Dec 7, 2016 at 9:08 AM Adrian Prantl <aprantl at apple.com> wrote:
> Should it turn out that the needs of instrumentation versus debugging are
> truly irreconcilable (and I hope they aren't) we could add an
> instrumentation debugger tuning option as a last resort.
>
> -- adrian
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161210/ddadd659/attachment.html>