llvm dev - Nov 2019 - Question about TypeMapper::addTypeMapping in IRMover.cpp

I have a question about the following code in TypeMapper::addTypeMapping in
IRMover.cpp:

if (!areTypesIsomorphic(DstTy, SrcTy)) {
   :
   :
} else {
    // SrcTy and DstTy are recursively ismorphic. We clear names of SrcTy
    // and all its descendants to lower amount of renaming in LLVM context
    // Renaming occurs because we load all source modules to the same context
    // and declaration with existing name gets renamed (i.e Foo -> Foo.42).
    // As a result we may get several different types in the destination
    // module, which are in fact the same.
    for (Type *Ty : SpeculativeTypes)
      if (auto *STy = dyn_cast<StructType>(Ty))
        if (STy->hasName())
          STy->setName("");

The above doesn't account for the possibility SrcTy and DstTy can be the
same pointer. When they are the same, effectively, the above ends up clearing
the name of DstTy.

Is there an assumption that addTypeMapping should never be called with the same
types? The code for areTypesIsomorphic does specifically handle the case where
DstTy == SrcTy.

Background for the above question...

I am running into this problem where we have two modules sharing the same types
and we are trying to link one module into the other. In our application, we have
a common module with inline functions and several other modules call inline
functions from this common module. This code was originally written with LLVM
3.4.2. At that time, the linker API had a PreserveSource mode which allowed for
the common module to be preserved so that it can be linked into every other
module. As we are now upgrading our LLVM version, we found that the current API
takes ownership of the source module and destroys it after linking. We worked
around this problem by cloning the common module before calling
Linker::linkInModule. However the problem is that it still doesn't work due:

Consider this function in the destination module:

declare void @foo(%struct.bar * %this1, i8 zeroext %s1) #0

and the corresponding function in the source module:

define linkonce_odr void @foo(%struct.bar* %this1, i8 zeroext %s1) #0 {
  :
}

The linking doesn't succeed (definition of foo doesn't get copied into
the destination module) because of the following code in
IRLinker::linkGlobalValueProto:

Constant *C = NewGV;
// Only create a bitcast if necessary. In particular, with
// DebugTypeODRUniquing we may reach metadata in the destination module
// containing a GV from the source module, in which case SGV will be
// the same as DGV and NewGV, and TypeMap.get() will assert since it
// assumes it is being invoked on a type in the source module.
if (DGV && NewGV != SGV) {
  C = ConstantExpr::getPointerBitCastOrAddrSpaceCast(
    NewGV, TypeMap.get(SGV->getType()));
}

The above results in C to become something like (I suspect that the problem is
with the TypeMap which causes a bit-cast to be created). C->dump() shows...

     void (%"type 0x5d6c9c0"*, i8)* bitcast (void (%"type
0x5d6bf50"*, i8)* @foo to void (%"type 0x5d6c9c0"*, i8)*)

This does not cast to a GlobalValue and IRLinker::materialize returns without
copying the body of foo:

GlobalValue *New = dyn_cast<GlobalValue>(*NewProto);
  if (!New)
    return *NewProto;

llvm::verifyModule of the destination module then fails with the following
error:

Global is external, but doesn't have external or weak linkage!
void (%0*, i32)* @foo
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191110/f0142639/attachment-0001.html>

Any comments on this? I think this issue is different from the one that was
discussed in https://lists.llvm.org/pipermail/llvm-dev/2016-November/106949.html

/Riyaz

From: Riyaz Puthiyapurayil
Sent: Sunday, November 10, 2019 10:42 AM
To: 'llvm-dev' <llvm-dev at lists.llvm.org>
Subject: Question about TypeMapper::addTypeMapping in IRMover.cpp

I have a question about the following code in TypeMapper::addTypeMapping in
IRMover.cpp:

if (!areTypesIsomorphic(DstTy, SrcTy)) {
   :
   :
} else {
    // SrcTy and DstTy are recursively ismorphic. We clear names of SrcTy
    // and all its descendants to lower amount of renaming in LLVM context
    // Renaming occurs because we load all source modules to the same context
    // and declaration with existing name gets renamed (i.e Foo -> Foo.42).
    // As a result we may get several different types in the destination
    // module, which are in fact the same.
    for (Type *Ty : SpeculativeTypes)
      if (auto *STy = dyn_cast<StructType>(Ty))
        if (STy->hasName())
          STy->setName("");

The above doesn't account for the possibility SrcTy and DstTy can be the
same pointer. When they are the same, effectively, the above ends up clearing
the name of DstTy.

Is there an assumption that addTypeMapping should never be called with the same
types? The code for areTypesIsomorphic does specifically handle the case where
DstTy == SrcTy.

Background for the above question...

I am running into this problem where we have two modules sharing the same types
and we are trying to link one module into the other. In our application, we have
a common module with inline functions and several other modules call inline
functions from this common module. This code was originally written with LLVM
3.4.2. At that time, the linker API had a PreserveSource mode which allowed for
the common module to be preserved so that it can be linked into every other
module. As we are now upgrading our LLVM version, we found that the current API
takes ownership of the source module and destroys it after linking. We worked
around this problem by cloning the common module before calling
Linker::linkInModule. However the problem is that it still doesn't work due:

Consider this function in the destination module:

declare void @foo(%struct.bar * %this1, i8 zeroext %s1) #0

and the corresponding function in the source module:

define linkonce_odr void @foo(%struct.bar* %this1, i8 zeroext %s1) #0 {
  :
}

The linking doesn't succeed (definition of foo doesn't get copied into
the destination module) because of the following code in
IRLinker::linkGlobalValueProto:

Constant *C = NewGV;
// Only create a bitcast if necessary. In particular, with
// DebugTypeODRUniquing we may reach metadata in the destination module
// containing a GV from the source module, in which case SGV will be
// the same as DGV and NewGV, and TypeMap.get() will assert since it
// assumes it is being invoked on a type in the source module.
if (DGV && NewGV != SGV) {
  C = ConstantExpr::getPointerBitCastOrAddrSpaceCast(
    NewGV, TypeMap.get(SGV->getType()));
}

The above results in C to become something like (I suspect that the problem is
with the TypeMap which causes a bit-cast to be created). C->dump() shows...

     void (%"type 0x5d6c9c0"*, i8)* bitcast (void (%"type
0x5d6bf50"*, i8)* @foo to void (%"type 0x5d6c9c0"*, i8)*)

This does not cast to a GlobalValue and IRLinker::materialize returns without
copying the body of foo:

GlobalValue *New = dyn_cast<GlobalValue>(*NewProto);
  if (!New)
    return *NewProto;

llvm::verifyModule of the destination module then fails with the following
error:

Global is external, but doesn't have external or weak linkage!
void (%0*, i32)* @foo
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191112/7e54dbfa/attachment.html>