[PATCH] D104827: [DebugInfo] Enforce implicit constraints on `distinct` MDNodes

Thu Jul 8 21:41:32 PDT 2021

uabelho added a comment.

In D104827#2865292 <https://reviews.llvm.org/D104827#2865292>, @dexonsmith wrote:

> In D104827#2855970 <https://reviews.llvm.org/D104827#2855970>, @dexonsmith wrote:
>
>> That looks bad — best to revert while that's investigated.
>
> Oops... I had misread this comment:
>
> In D104827#2854825 <https://reviews.llvm.org/D104827#2854825>, @uabelho wrote:
>
>> We've noticed that this patch changes things so bcfiles created with the patch cannot be parsed with binaries built before it.
>
> I don't think we have any guarantees about reading new bitcode with old readers. There's some machinery for a high-level version check to get a nice error (checking `MODULE_CODE_VERSION`) but I think that's it. (I thought this was saying the new reader couldn't read the old bitcode, in which case there'd be a missing bitcode upgrade.)

Hm, I often do this and I can't remember it failing before. E.g. if I run into a crash in opt, I usually save the bc file and bisect into old versions to see with what opt version the crash starts appearing. This usually works very well so I was surprised and considered it a bug now when it didn't work.

> In D104827#2864582 <https://reviews.llvm.org/D104827#2864582>, @scott.linder wrote:
>
>> Any thoughts on where to go from here, and whether this patch should be blocked on these fixes?
>
> Assuming IIUC now, the patch can probably land as-is... but probably makes sense to investigate the LLDB issue first. @JDevlieghere, could the failing LLDB test somehow be reading a new bitcode file with an old reader, or is that something different?

I've seen the same crash in our testing and then there was no "new bitcode with old reader" stuff involved, it was just a normal run crashing in somewhere llc (for our out of tree target).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104827