llvm dev - Jul 2016 - 'invalid subroutine type ref' when linking custom metadata

With 3.9, llvm-link tells me 'invalid subroutine type ref' when
linking the two code pieces below, and I don't quite understand why.
It looks like it merges the debug information with the custom
metadata. I've filed a ticket already [1] but as I'm not sure if this
is indeed a bug or if I'm misunderstanding something, I thought I'd
ask here.

Any ideas? Thanks,

Robin

    > cat a.ll
    define void @a() {
      ret void
    }
    
    !bar = !{!3}
    
    !3 = !{void ()* @a}
    
    > cat b.ll
    define void @foo() !dbg !20 {
      ret void
    }
    
    !llvm.module.flags = !{!17, !18}
    
    !0 = distinct !DICompileUnit(language: DW_LANG_C89, file: !1)
    !1 = !DIFile(filename: "x.c", directory: "/tmp")
    !17 = !{i32 2, !"Dwarf Version", i32 4}
    !18 = !{i32 2, !"Debug Info Version", i32 3}
    !20 = distinct !DISubprogram(name: "foo", scope: !1, file: !1,
line: 92, type: !21, unit: !0)
    !21 = !DISubroutineType(types: !22)
    !22 = !{null}
    !23 = !{}
    
    > llvm-link -o out a.ll b.ll
    invalid subroutine type ref
    !5 = !DISubroutineType(types: !0)
    !0 = !{void ()* @a}
    void ()* @a
    All DICompileUnits must be listed in llvm.dbg.cu
    llvm-link: error: linked module is broken!

(The *.ll files are stripped down from much larger files to find a
minimal example. I suppose the message about llvm.dbg.cu is just
because I removed a bit too much).

Robin

[1] https://llvm.org/bugs/show_bug.cgi?id=28697

-- 
Robin Sommer * ICSI/LBNL * robin at icir.org * www.icir.org/robin

+CC Duncan``` 
> On Jul 25, 2016, at 8:53 PM, Robin Sommer via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 
> With 3.9, llvm-link tells me 'invalid subroutine type ref' when
> linking the two code pieces below, and I don't quite understand why.
> It looks like it merges the debug information with the custom
> metadata. I've filed a ticket already [1] but as I'm not sure if
this
> is indeed a bug or if I'm misunderstanding something, I thought I'd
> ask here.
> 
> Any ideas?
This is a bug, and I believe it is caused by r266579.

It seems that:

1) The module for a.ll is created.
2) "!3 = !{void ()* @a}" is a MDTuple, and MDTuple nodes are leaking
to the context and survive module deletion.
3) "void ()* @a” which is the only element in this MDTuple is a
“ConstantAsMetadata”, this does not leak and will be deleted with the module.
4) a.ll is linked into an empty module, using an IRMover. This process will
involved populating a map of Metadata -> mapped Metadata. One entry will be
created with this MDTuple !3.
5) a.ll is destroyed, including the “ConstantAsMetadata”.
6) b.ll is parsed, the “null” constant will get the same pointer as the
previously deleted constant from a.ll.
7) "!22 = !{null}” is parsed. Since this is a “uniqued” node, we are
looking in the context for a MDTuple with the null constant as a single operand.
We find the previously "!3 = !{void ()* @a}”, and this is fine since it
nows point to the new “null” constant”.
8) We reuse the map of metadata to link b.ll, however this map is no longer
valid since the MDTuple entry for !3 is outdated and will map to the moved
constant "void ()* @a”.

Duncan: can you confirm that this analysis makes sense to you?
Is it an oversight of your commit in r266579?

Thanks,

— 
Mehdi

> 
>> cat a.ll
>    define void @a() {
>      ret void
>    }
> 
>    !bar = !{!3}
> 
>    !3 = !{void ()* @a}
> 
>> cat b.ll
>    define void @foo() !dbg !20 {
>      ret void
>    }
> 
>    !llvm.module.flags = !{!17, !18}
> 
>    !0 = distinct !DICompileUnit(language: DW_LANG_C89, file: !1)
>    !1 = !DIFile(filename: "x.c", directory: "/tmp")
>    !17 = !{i32 2, !"Dwarf Version", i32 4}
>    !18 = !{i32 2, !"Debug Info Version", i32 3}
>    !20 = distinct !DISubprogram(name: "foo", scope: !1, file: !1,
line: 92, type: !21, unit: !0)
>    !21 = !DISubroutineType(types: !22)
>    !22 = !{null}
>    !23 = !{}
> 
>> llvm-link -o out a.ll b.ll
>    invalid subroutine type ref
>    !5 = !DISubroutineType(types: !0)
>    !0 = !{void ()* @a}
>    void ()* @a
>    All DICompileUnits must be listed in llvm.dbg.cu
>    llvm-link: error: linked module is broken!
> 
> (The *.ll files are stripped down from much larger files to find a
> minimal example. I suppose the message about llvm.dbg.cu is just
> because I removed a bit too much).
Just adding:


— 
Mehdi
> 
> Robin
> 
> [1] https://llvm.org/bugs/show_bug.cgi?id=28697
> 
> -- 
> Robin Sommer * ICSI/LBNL * robin at icir.org * www.icir.org/robin
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Mon, Jul 25, 2016 at 23:22 -0700, Mehdi Amini wrote:
> This is a bug, and I believe it is caused by r266579.
Thanks, Medi, I can confirm that if I disable the cache with the hack
below, it works fine.

Robin

diff --git a/lib/Linker/IRMover.cpp b/lib/Linker/IRMover.cpp
index 4935868..7d3d0b1 100644
--- a/lib/Linker/IRMover.cpp
+++ b/lib/Linker/IRMover.cpp
@@ -490,11 +490,11 @@ public:
                &GValMaterializer),
         AliasMCID(Mapper.registerAlternateMappingContext(AliasValueMap,
                                                         
&LValMaterializer)) {
-    ValueMap.getMDMap() = std::move(SharedMDs);
+    /* ValueMap.getMDMap() = std::move(SharedMDs); */
     for (GlobalValue *GV : ValuesToLink)
       maybeAdd(GV);
   }
-  ~IRLinker() { SharedMDs = std::move(*ValueMap.getMDMap()); }
+  ~IRLinker() { /* SharedMDs = std::move(*ValueMap.getMDMap()); */ }

   Error run();
   Value *materialize(Value *V, bool ForAlias);


-- 
Robin Sommer * ICSI/LBNL * robin at icir.org * www.icir.org/robin

> On 2016-Jul-26, at 02:22, Mehdi Amini <mehdi.amini at apple.com>
wrote:
> 
> +CC Duncan``` 
> 
>> On Jul 25, 2016, at 8:53 PM, Robin Sommer via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>> 
>> 
>> With 3.9, llvm-link tells me 'invalid subroutine type ref' when
>> linking the two code pieces below, and I don't quite understand
why.
>> It looks like it merges the debug information with the custom
>> metadata. I've filed a ticket already [1] but as I'm not sure
if this
>> is indeed a bug or if I'm misunderstanding something, I thought
I'd
>> ask here.
>> 
>> Any ideas?
> 
> This is a bug, and I believe it is caused by r266579.
> 
> It seems that:
> 
> 1) The module for a.ll is created.
> 2) "!3 = !{void ()* @a}" is a MDTuple, and MDTuple nodes are
leaking to the context and survive module deletion.
> 3) "void ()* @a” which is the only element in this MDTuple is a
“ConstantAsMetadata”, this does not leak and will be deleted with the module.
> 4) a.ll is linked into an empty module, using an IRMover. This process will
involved populating a map of Metadata -> mapped Metadata. One entry will be
created with this MDTuple !3.
> 5) a.ll is destroyed, including the “ConstantAsMetadata”.
> 6) b.ll is parsed, the “null” constant will get the same pointer as the
previously deleted constant from a.ll.
Ugh.  This behaviour is ugly.
> 7) "!22 = !{null}” is parsed. Since this is a “uniqued” node, we are
looking in the context for a MDTuple with the null constant as a single operand.
We find the previously "!3 = !{void ()* @a}”, and this is fine since it
nows point to the new “null” constant”.
> 8) We reuse the map of metadata to link b.ll, however this map is no longer
valid since the MDTuple entry for !3 is outdated and will map to the moved
constant "void ()* @a”.
> 
> Duncan: can you confirm that this analysis makes sense to you?
> Is it an oversight of your commit in r266579?
This sounds correct to me :(.  Unfortunately the shared map is quite important
for performance.

Once possible fix is to make the mapped-value a TrackingMDNodeRef, but that
seems like a hack.  Is there a way to invalidate just the right parts of the
cache?

Another possible solution, that starts with a history lesson: before I split
Metadata from the Value hierarchy, when an MDNode operand changed to null, the
MDNode would drop uniquing.  I didn't understand why that would be useful
outside of speeding up teardown.  Since it was actively harmful in some cases, I
removed the behaviour, and tried to optimize teardown in other ways.

But I can see how it would make sense here.  Maybe, when an MDNode operand that
is a ConstantAsMetadata gets RAUW'ed to "null" and/or deleted, we
should drop uniquing from the MDNode.  This is a more targeted version of the
historical behaviour.  It would prevent the SharedMDs entry from being reused in
this case.
> 
> Thanks,
> 
> — 
> Mehdi
> 
> 
>> 
>>> cat a.ll
>>   define void @a() {
>>     ret void
>>   }
>> 
>>   !bar = !{!3}
>> 
>>   !3 = !{void ()* @a}
>> 
>>> cat b.ll
>>   define void @foo() !dbg !20 {
>>     ret void
>>   }
>> 
>>   !llvm.module.flags = !{!17, !18}
>> 
>>   !0 = distinct !DICompileUnit(language: DW_LANG_C89, file: !1)
>>   !1 = !DIFile(filename: "x.c", directory: "/tmp")
>>   !17 = !{i32 2, !"Dwarf Version", i32 4}
>>   !18 = !{i32 2, !"Debug Info Version", i32 3}
>>   !20 = distinct !DISubprogram(name: "foo", scope: !1, file:
!1, line: 92, type: !21, unit: !0)
>>   !21 = !DISubroutineType(types: !22)
>>   !22 = !{null}
>>   !23 = !{}
>> 
>>> llvm-link -o out a.ll b.ll
>>   invalid subroutine type ref
>>   !5 = !DISubroutineType(types: !0)
>>   !0 = !{void ()* @a}
>>   void ()* @a
>>   All DICompileUnits must be listed in llvm.dbg.cu
>>   llvm-link: error: linked module is broken!
>> 
>> (The *.ll files are stripped down from much larger files to find a
>> minimal example. I suppose the message about llvm.dbg.cu is just
>> because I removed a bit too much).
> 
> Just adding:
> 
> 
> — 
> Mehdi
> 
>> 
>> Robin
>> 
>> [1] https://llvm.org/bugs/show_bug.cgi?id=28697
>> 
>> -- 
>> Robin Sommer * ICSI/LBNL * robin at icir.org * www.icir.org/robin
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

The language reference says: "identified [structure] types are never
uniqued". But llvm-link seems to do just that at link-time:

    > cat x.ll

    %A = type {}
    %B = type {}
    %C = type { %A*, %B* }

    define void @foo(%C* %c) {
      ret void
    }

    >  llvm-link x.ll | llvm-dis
    ; ModuleID = '<stdin>'
    source_filename = "llvm-link"

    %C = type { %A*, %A* }
    %A = type {}

    define void @foo(%C* %c) {
        ret void
    }

Does the statement in the language reference not apply to linking?

(This is with a recent 3.9).

Robin

-- 
Robin Sommer * ICSI/LBNL * robin at icir.org * www.icir.org/robin