[llvm] r187285 - Debug Info Verifier: verify SPs in llvm.dbg.sp.

[David Blaikie]

On Wed, Jul 31, 2013 at 3:32 PM, Manman Ren <manman.ren at gmail.com> wrote:
> On 7/26/13, 11:05 PM, David Blaikie wrote:
>>
>> On Fri, Jul 26, 2013 at 10:45 PM, Manman <manman.ren at gmail.com> wrote:
>>>
>>>
>>>> On Jul 26, 2013, at 8:31 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>>
>>>>> On Fri, Jul 26, 2013 at 6:26 PM, Manman Ren <manman.ren at gmail.com>
>>>>> wrote:
>>>>> Author: mren
>>>>> Date: Fri Jul 26 20:26:08 2013
>>>>> New Revision: 187285
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=187285&view=rev
>>>>> Log:
>>>>> Debug Info Verifier: verify SPs in llvm.dbg.sp.
>>>>
>>>> Wait do we still even do anything with that named metadata anymore? I
>>>> don't think we do - we killed off that kind of debug info a while ago,
>>>> I thought. (the only named metadata should be llvm.dbg.cu, right?)
>>>
>>> That sounds true. But it is still used in the testing cases. After we
>>> replace those with dbg.cu, we can probably remove checking for dbg.sp.
>>
>> It may still be /present/ in the testing cases, but not actually used,
>> since the debug info code never walks from that root node...
>
> "llvm.dbg.sp" is only used in IPO/StripSymbols.cpp.
>
> The verifier currently does not check the debug location attached to an
> instruction. I had a patch ready, but it requires updating 30+ testing cases
> and I didn't get a chance to update the testing cases yet.
>
> For testing cases that do not have dbg.cu,

Any test case that doesn't have dbg.cu shouldn't be considered to be a
debug info testcase, just bogus/malformed debug info.

> the only way to reach some of the
> SPs is through the debug locations or through dbg.sp.

But if the rest of LLVM doesn't access them through dbg.sp, why are we
verifying them? They're not actually used.

I'd like to ensure that the verifier, ideally, only accesses debug
info metadata through the same mechanisms as the actual LLVM
production code (& that it accesses them through all such mechanisms).
That way the verifier ensures that the debug info that matters is in
the right form, and that any other pseudo debug info (such as entries
that look like line entries but aren't, because they aren't referenced
from the debugloc attribute in an Instruction) is ignored.

> For now, we can check the SPs listed in dbg.sp to provide more coverage for the verifier.

It's false/irrelevant coverage though, isn't it?

> Once the verifier verifies DbgLoc, we can remove dbg.sp from the testing
> cases and also remove checking them from the verifier.

OK. I'd probably be happy enough not to check them until we check them
correctly, rather than check incorrect stuff.

But so long as you've got a plan, I can live with that.

- David