llvm dev - Nov 2013 - [LLVMdev] bit code file incompatibility due to debug info changes

On Mon, Nov 18, 2013 at 10:55 AM, David Blaikie <dblaikie at gmail.com>
wrote:
> It depends a bit, also, on what kind of guarantees we need to offer. If
> the guarantee when reading IR from disk is "will not crash" then
there's
> nothing for it but to run full debug info verification.
>
> On the other hand, if we can assume that some specific metadata implies
> the correctness of some other metadata, then all we need to do is check a
> known debug info version number. If it's the current number, do nothing
> (assume the rest of the debug info is up to date and well formed),
> otherwise do all the work to find the debug info and dump it (no need to
> verify it - we just have to do the work to find it, so we don't go
> following bad links later on - that should be as easy as dropping the
> llvm.dbg.cu named node and removing all debug intrinsics and the
> instruction metadata line references). But this latter scheme isn't
robust
> against arbitrary metadata (that could, coincidentally, have the right
> version number and arbitrary metadata that breaks all our debug info
> metadata assumptions)
>
> If the latter is sufficient for everyone's needs/principles, great.
>
This makes sense to me, but I see Eric's fundamental concern with upgrading
test cases.


One other possible idea: we could artificially force coarseness on metadata
while things are still iterating rapidly by just having a version number
that rotates every "release". (Even non-open-source-releases could be
handled by letting anyone, any time they need to rev it, update what the
current version is and update every test case to reference that version.)
If the version is nicely factored, it should at least be an obvious diff if
still huge.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131118/4c20cf71/attachment.html>

Eric, why did you move the PR back to resolved/won't-fix again after I
reopened it?  We really need to do *something* here.  I'm  open to
discussing any of the possible solutions that have been offered but just letting
the compiler crash does not seem acceptable.

On Nov 18, 2013, at 11:00 AM, Chandler Carruth <chandlerc at google.com>
wrote:
> 
> On Mon, Nov 18, 2013 at 10:55 AM, David Blaikie <dblaikie at
gmail.com> wrote:
> It depends a bit, also, on what kind of guarantees we need to offer. If the
guarantee when reading IR from disk is "will not crash" then
there's nothing for it but to run full debug info verification.
> 
> On the other hand, if we can assume that some specific metadata implies the
correctness of some other metadata, then all we need to do is check a known
debug info version number. If it's the current number, do nothing (assume
the rest of the debug info is up to date and well formed), otherwise do all the
work to find the debug info and dump it (no need to verify it - we just have to
do the work to find it, so we don't go following bad links later on - that
should be as easy as dropping the llvm.dbg.cu named node and removing all debug
intrinsics and the instruction metadata line references). But this latter scheme
isn't robust against arbitrary metadata (that could, coincidentally, have
the right version number and arbitrary metadata that breaks all our debug info
metadata assumptions)
> 
> If the latter is sufficient for everyone's needs/principles, great.
> 
> This makes sense to me, but I see Eric's fundamental concern with
upgrading test cases.
> 
> 
> One other possible idea: we could artificially force coarseness on metadata
while things are still iterating rapidly by just having a version number that
rotates every "release". (Even non-open-source-releases could be
handled by letting anyone, any time they need to rev it, update what the current
version is and update every test case to reference that version.) If the version
is nicely factored, it should at least be an obvious diff if still huge.
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131118/70b13031/attachment.html>

For the same reason that you asked me in two threads: because it's
splitting the discussion up and making it more complicated. Let's keep
it to one thread and then open a bug at the end when/if we have
something that we can take action on.

-eric

On Mon, Nov 18, 2013 at 11:05 AM, Bob Wilson <bob.wilson at apple.com>
wrote:
> Eric, why did you move the PR back to resolved/won't-fix again after I
> reopened it?  We really need to do *something* here.  I'm  open to
> discussing any of the possible solutions that have been offered but just
> letting the compiler crash does not seem acceptable.
>
> On Nov 18, 2013, at 11:00 AM, Chandler Carruth <chandlerc at
google.com> wrote:
>
>
> On Mon, Nov 18, 2013 at 10:55 AM, David Blaikie <dblaikie at
gmail.com> wrote:
>>
>> It depends a bit, also, on what kind of guarantees we need to offer. If
>> the guarantee when reading IR from disk is "will not crash"
then there's
>> nothing for it but to run full debug info verification.
>>
>> On the other hand, if we can assume that some specific metadata implies
>> the correctness of some other metadata, then all we need to do is check
a
>> known debug info version number. If it's the current number, do
nothing
>> (assume the rest of the debug info is up to date and well formed),
otherwise
>> do all the work to find the debug info and dump it (no need to verify
it -
>> we just have to do the work to find it, so we don't go following
bad links
>> later on - that should be as easy as dropping the llvm.dbg.cu named
node and
>> removing all debug intrinsics and the instruction metadata line
references).
>> But this latter scheme isn't robust against arbitrary metadata
(that could,
>> coincidentally, have the right version number and arbitrary metadata
that
>> breaks all our debug info metadata assumptions)
>>
>> If the latter is sufficient for everyone's needs/principles, great.
>
>
> This makes sense to me, but I see Eric's fundamental concern with
upgrading
> test cases.
>
>
> One other possible idea: we could artificially force coarseness on metadata
> while things are still iterating rapidly by just having a version number
> that rotates every "release". (Even non-open-source-releases
could be
> handled by letting anyone, any time they need to rev it, update what the
> current version is and update every test case to reference that version.)
If
> the version is nicely factored, it should at least be an obvious diff if
> still huge.
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>

On Mon, Nov 18, 2013 at 11:00 AM, Chandler Carruth <chandlerc at
google.com> wrote:
>
> On Mon, Nov 18, 2013 at 10:55 AM, David Blaikie <dblaikie at
gmail.com> wrote:
>>
>> It depends a bit, also, on what kind of guarantees we need to offer. If
>> the guarantee when reading IR from disk is "will not crash"
then there's
>> nothing for it but to run full debug info verification.
>>
>> On the other hand, if we can assume that some specific metadata implies
>> the correctness of some other metadata, then all we need to do is check
a
>> known debug info version number. If it's the current number, do
nothing
>> (assume the rest of the debug info is up to date and well formed),
otherwise
>> do all the work to find the debug info and dump it (no need to verify
it -
>> we just have to do the work to find it, so we don't go following
bad links
>> later on - that should be as easy as dropping the llvm.dbg.cu named
node and
>> removing all debug intrinsics and the instruction metadata line
references).
>> But this latter scheme isn't robust against arbitrary metadata
(that could,
>> coincidentally, have the right version number and arbitrary metadata
that
>> breaks all our debug info metadata assumptions)
>>
>> If the latter is sufficient for everyone's needs/principles, great.
>
>
> This makes sense to me, but I see Eric's fundamental concern with
upgrading
> test cases.
>
>
> One other possible idea: we could artificially force coarseness on metadata
> while things are still iterating rapidly by just having a version number
> that rotates every "release". (Even non-open-source-releases
could be
> handled by letting anyone, any time they need to rev it, update what the
> current version is and update every test case to reference that version.)
If
> the version is nicely factored, it should at least be an obvious diff if
> still huge.
That could work, it'd be a simple script to say "update all of the
debug info testcases to move to the new version" between versions but
it probably wouldn't be too bad. Part of the branch cut.

-eric

On Mon, Nov 18, 2013 at 11:00 AM, Chandler Carruth <chandlerc at
google.com>wrote:
>
> On Mon, Nov 18, 2013 at 10:55 AM, David Blaikie <dblaikie at
gmail.com>wrote:
>
>> It depends a bit, also, on what kind of guarantees we need to offer. If
>> the guarantee when reading IR from disk is "will not crash"
then there's
>> nothing for it but to run full debug info verification.
>>
>> On the other hand, if we can assume that some specific metadata implies
>> the correctness of some other metadata, then all we need to do is check
a
>> known debug info version number. If it's the current number, do
nothing
>> (assume the rest of the debug info is up to date and well formed),
>> otherwise do all the work to find the debug info and dump it (no need
to
>> verify it - we just have to do the work to find it, so we don't go
>> following bad links later on - that should be as easy as dropping the
>> llvm.dbg.cu named node and removing all debug intrinsics and the
>> instruction metadata line references). But this latter scheme isn't
robust
>> against arbitrary metadata (that could, coincidentally, have the right
>> version number and arbitrary metadata that breaks all our debug info
>> metadata assumptions)
>>
>> If the latter is sufficient for everyone's needs/principles, great.
>>
>
> This makes sense to me, but I see Eric's fundamental concern with
> upgrading test cases.
>
>
> One other possible idea: we could artificially force coarseness on
> metadata while things are still iterating rapidly by just having a version
> number that rotates every "release". (Even
non-open-source-releases could
> be handled by letting anyone, any time they need to rev it, update what the
> current version is and update every test case to reference that version.)
> If the version is nicely factored, it should at least be an obvious diff if
> still huge.
>
Adding a debug info version number and ignoring the debug info MDNodes when
version does not match makes sense. And changing the version number with
every release sounds reasonable.

One implementation is
1> completely get rid of version number in the tag
  LLVMDebugVersion is only used in two places and we should be able to
remove it.
    include/llvm/DebugInfo.h:    return getUnsignedField(0) &
~LLVMDebugVersionMask;
    lib/IR/DIBuilder.cpp:  assert((Tag & LLVMDebugVersionMask) == 0
&&
    lib/IR/DIBuilder.cpp:  return
ConstantInt::get(Type::getInt32Ty(VMContext), Tag | LLVMDebugVersion);
  I remember there was something that blocks David from completely getting
rid of version number, but it seems the blocker is gone now.
  This can be done separately as well.

2> introduce a debug info version in module flags similar to "dwarf
version" module flag
     LTO can correctly handle the module flag during linking and we only
need to change one line in the debug info testing cases when we update
debug info version number.
     One issue is that we don't have a mode that emits a warning and
outputs the largest value (we have a mode that emits a warning and outputs
the value from the first module, but we can definitely add another mode if
necessary)

3> when to drop the debug info MDNodes?
     We could do this in the bit code loader but it seems a violation of
layers. One possibility is to run a module pass right after loading the bit
code, to remove debug intrinsics and debug tags on instructions.

4> how to report warning?
    Is errs() << "WARNING: " good enough? BTW this is how we
emit warning
when linking module flags.

Any objection to the above? Any other suggestion?

Thanks,
Manman

> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131119/24b97e5e/attachment.html>

On Tue, Nov 19, 2013 at 5:26 PM, Manman Ren <manman.ren at gmail.com>
wrote:
>
>
>
> On Mon, Nov 18, 2013 at 11:00 AM, Chandler Carruth <chandlerc at
google.com>wrote:
>
>>
>> On Mon, Nov 18, 2013 at 10:55 AM, David Blaikie <dblaikie at
gmail.com>wrote:
>>
>>> It depends a bit, also, on what kind of guarantees we need to
offer. If
>>> the guarantee when reading IR from disk is "will not
crash" then there's
>>> nothing for it but to run full debug info verification.
>>>
>>> On the other hand, if we can assume that some specific metadata
implies
>>> the correctness of some other metadata, then all we need to do is
check a
>>> known debug info version number. If it's the current number, do
nothing
>>> (assume the rest of the debug info is up to date and well formed),
>>> otherwise do all the work to find the debug info and dump it (no
need to
>>> verify it - we just have to do the work to find it, so we don't
go
>>> following bad links later on - that should be as easy as dropping
the
>>> llvm.dbg.cu named node and removing all debug intrinsics and the
>>> instruction metadata line references). But this latter scheme
isn't robust
>>> against arbitrary metadata (that could, coincidentally, have the
right
>>> version number and arbitrary metadata that breaks all our debug
info
>>> metadata assumptions)
>>>
>>> If the latter is sufficient for everyone's needs/principles,
great.
>>>
>>
>> This makes sense to me, but I see Eric's fundamental concern with
>> upgrading test cases.
>>
>>
>> One other possible idea: we could artificially force coarseness on
>> metadata while things are still iterating rapidly by just having a
version
>> number that rotates every "release". (Even
non-open-source-releases could
>> be handled by letting anyone, any time they need to rev it, update what
the
>> current version is and update every test case to reference that
version.)
>> If the version is nicely factored, it should at least be an obvious
diff if
>> still huge.
>>
>
> Adding a debug info version number and ignoring the debug info MDNodes
> when version does not match makes sense. And changing the version number
> with every release sounds reasonable.
>
> One implementation is
> 1> completely get rid of version number in the tag
>   LLVMDebugVersion is only used in two places and we should be able to
> remove it.
>     include/llvm/DebugInfo.h:    return getUnsignedField(0) &
> ~LLVMDebugVersionMask;
>     lib/IR/DIBuilder.cpp:  assert((Tag & LLVMDebugVersionMask) == 0
&&
>     lib/IR/DIBuilder.cpp:  return
> ConstantInt::get(Type::getInt32Ty(VMContext), Tag | LLVMDebugVersion);
>   I remember there was something that blocks David from completely getting
> rid of version number, but it seems the blocker is gone now.
>   This can be done separately as well.
>
> 2> introduce a debug info version in module flags similar to "dwarf
> version" module flag
>      LTO can correctly handle the module flag during linking and we only
> need to change one line in the debug info testing cases when we update
> debug info version number.
>      One issue is that we don't have a mode that emits a warning and
> outputs the largest value (we have a mode that emits a warning and outputs
> the value from the first module, but we can definitely add another mode if
> necessary)
>
> 3> when to drop the debug info MDNodes?
>      We could do this in the bit code loader but it seems a violation of
> layers. One possibility is to run a module pass right after loading the bit
> code, to remove debug intrinsics and debug tags on instructions.
>
We have an existing pass StripSymbols that can strip debug info in the
module.
So it is just a matter of adding that pass after bit code loader if the
debug info version number does not match.

Manman

> 4> how to report warning?
>     Is errs() << "WARNING: " good enough? BTW this is how
we emit warning
> when linking module flags.
>
> Any objection to the above? Any other suggestion?
>
> Thanks,
> Manman
>
>
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131119/22100bce/attachment.html>