[PATCH] D21818: Add artificial debug information to avoid compiler crash

[Adrian Prantl via llvm-commits]

> On Aug 26, 2016, at 4:53 PM, Gao, Yunzhong <Yunzhong.Gao at sony.com> wrote:
> 
> Hi Adrian,
> I did the following experiment.
> Step#1, check out from trunk r279786
> Step#2, revert the following commits: r267370, r267904 and r274263
> Step#3, build and run regression test => all tests pass with no failure
> 
> In addition,
> I tested the compiler built above with this test added in r267370, llvm/test/Verifier/callsite-dbgloc.ll,
> the compilation succeeds with no assertion failure. This test has a comment at the top of the file
> which mentions a C example that was expected to get an assertion failure with "-O2 -g", but upon
> my experiment, no assertion failure was triggered either.
> 
> I tested the compiler built above with the test change introduced in r274263, and again, the test
> passed with no assertion failure.
> 
> What do you think of this experiment? Can you draw meaningful conclusions from this,
> or am I missing some steps?

The dangerous pattern is this:

define void @foo !dbg !1 {
  ...
  call @__stack_chk_fail() ; Missing !dbg location here!
  ...
}

define void @__stack_chk_fail() !dbg !2 {
  some_insn, !dbg !3
  ...
}

When some_insn gets inlined into @foo its debug location will still point to @__stack_chk_fail, instead of being proper inlined location rooted in @foo.

Does your test exercise this case?

-- adrian

> 
> - Gao
> 
> 
> 
> ________________________________________
> From: aprantl at apple.com [aprantl at apple.com]
> Sent: Thursday, August 25, 2016 10:52 AM
> To: Robinson, Paul
> Cc: Gao, Yunzhong; David Blaikie; reviews+D21818+public+b40f0be86340e0e5 at reviews.llvm.org; llvm-commits at lists.llvm.org
> Subject: Re: [PATCH] D21818: Add artificial debug information to avoid compiler crash
> 
>> On Aug 25, 2016, at 10:49 AM, Robinson, Paul <paul.robinson at sony.com> wrote:
>> 
>> (re-adding llvm-commits which got lost from my previous)
>> 
>>> -----Original Message-----
>>> From: aprantl at apple.com [mailto:aprantl at apple.com]
>>> Sent: Thursday, August 25, 2016 10:42 AM
>>> To: Robinson, Paul
>>> Cc: Gao, Yunzhong; David Blaikie;
>>> reviews+D21818+public+b40f0be86340e0e5 at reviews.llvm.org
>>> Subject: Re: [PATCH] D21818: Add artificial debug information to avoid
>>> compiler crash
>>> 
>>> 
>>>> On Aug 25, 2016, at 10:37 AM, Robinson, Paul <paul.robinson at sony.com>
>>> wrote:
>>>>> -----Original Message-----
>>>>> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
>>> Behalf
>>>>> Of Gao, Yunzhong via llvm-commits
>>>>> Hi Adrian,
>>>>> I would not say that attaching an artificial debug location should be
>>>>> ruled out as
>>>>> an option, and the original commit r274263 used that approach. But I
>>> think
>>>>> David
>>>>> was suggesting that the verifier could be modified to consider this IR
>>> as
>>>>> valid
>>>>> without the extra debug info (that was my interpretation of what David
>>>>> said,
>>>>> but I could be wrong) and therefore I did some investigation here,
>>>> 
>>>> Right, the line-0 tactic increases debug-info size but the original
>>>> complaint was because of a verifier failure.  If we can mark the
>>>> offending artificial calls so that the verifier will allow them,
>>>> then we don't pay the debug-info size cost in the object file.
>>>> 
>>>> I'd be happier with a tactic that would automatically deal with this
>>>> for all cases of artificial calls to hard-coded function names, but
>>>> it's not immediately clear how to make that work.
>>> 
>>> I added the verifier check because having an inlineable function call
>>> without a debug location to a function with debug info inside a function
>>> with debug info causes an assertion failure when the backend attempts to
>>> construct the inline scopes.
>>> Do we know that it is safe to relax the verifier here?
>>> 
>>> -- adrian
>>> 
>>> 
>> Well, maybe not, and that would be a good experiment for Gao to try--
>> undo the original patch, so it doesn't mark the call with line-0, and
>> make the called function inlineable, and see if it dies when generating
>> the inlined scopes.
> 
> Gao, I think the assertion we're looking for is:
> 
> LexicalScopes::getOrCreateRegularScope()
>  assert(cast<DISubprogram>(Scope)->describes(MF->getFunction()));
> 
>> --paulr
>> 
> 
>