[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

Wed Nov 13 16:50:43 PST 2019

dblaikie added subscribers: probinson, echristo.
dblaikie added a comment.

OK - I believe the issue I'm seeing is an internal issue, a fragile/buggy test case. Please go ahead with this change if (by the sounds of it) all other concerns are addressed.

For more rambling context: A test case would call absl::GetStackTrace to get essentially the return address of the GetStackTrace call. It was doing this in an inlined function and expecting that the symbolized return address would reflect that. But the instruction immediately following the call to GetStackTrace (ie: the return address on the stack, the address that was being symbolized) was a spill due to something in the outer function.

This change caused that address (of the spill) to be labeled, to reference as the low_pc of a call_site.

Labeling the instruction caused a change in the line table due to use-unknown-locations (see r289468/ac7fe5e0c47a093b0fd8fd4972734ce143475c0e/D24180 <https://reviews.llvm.org/D24180>), giving the instruction a line zero location (since it was now at the start of a labeled region) making the stack trace not contain what was desired.

The test should probably be symbolizing 1 before the return address to get a more stable/intended location, and I'm working on an internal fix to that end.

/maybe/ the labels introduced by the call_site attribute (or any other label that is never used as a jump target) shouldn't trigger the line zero behavior from use-unknown-locations (@probinson in case he's got thoughts on that, though Sony might be using the "Always" option here, in which case this wouldn't've changed behavior for them)?

And/or maybe there are open questions about what location a spill should have? Whether we should back/forward propagate locations for those, since they can happen all over the place & maybe describing them as line zero isn't helpful? Not sure there.

(@echristo - just in case he's curious about what's going on here, though I'll write up more about the specific test on the internal bug)

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

https://reviews.llvm.org/D69970