MS ABI: Give thunks linkonce_odr linkage

[hans]

Summary

Reid commented on http://reviews.llvm.org/D3877?id=9937#inline-32885 that WeakAny didn't seem like the right linkage.

[rnk]

I think it's handling this corner case:

struct Incomplete;
struct A { int a; virtual A *bar(); };
struct B { int b; virtual B *foo(Incomplete); };
struct C : A, B { int c; virtual C *foo(Incomplete); };
C c;

MSVC gives this error:

t.cpp(5) : error C2664: 'C *C::foo(Incomplete)' : cannot convert argument 1 from 'Incomplete' to 'Incomplete'
      Source or target has incomplete type
      This diagnostic occurred in the compiler generated function 'B *C::foo(Incomplete)'

Clang on the other hand relies on the TU defining the virtual function to emit code for the thunk. Therefore I think the correct linkage is weak_odr. This should also be applicable to the Itanium C++ ABI.

[hans]

Switch to weak_odr.

[rnk]

It occurs to me that the linkage is still wrong for classes with thunks in anonymous namespaces. It'd also be nice to have the thunks be discardable (i.e. linkonce_odr) in the common cases.

I think this is the right linkage computation:

• class has GVA_Internal linkage -> thunk is internal
• the thunk has a return adjustment -> thunk is weak_odr, to handle the evil corner case
• all other normal methods -> thunk is linkonce_odr

[hans]

Calculate the thunk linkage as suggested by Reid.

[hans]

Ping?

[rnk]

lgtm

[hans]

Closed by commit rL210368 (authored by @hans).

cfe/trunk/lib/CodeGen/CGCXXABI.h

virtual void adjustCallArgsForDestructorThunk(CodeGenFunction &CGF,
                                                 GlobalDecl GD,
                                                 CallArgList &CallArgs) {}
 
   /// Emit any tables needed to implement virtual inheritance.  For Itanium,
   /// this emits virtual table tables.  For the MSVC++ ABI, this emits virtual
   /// base tables.
   virtual void emitVirtualInheritanceTables(const CXXRecordDecl *RD) = 0;
 
-  virtual void setThunkLinkage(llvm::Function *Thunk, bool ForVTable) = 0;
+  virtual void setThunkLinkage(llvm::Function *Thunk, bool ForVTable,
+                               GlobalDecl GD, bool ReturnAdjustment) = 0;
 
   virtual llvm::Value *performThisAdjustment(CodeGenFunction &CGF,
                                              llvm::Value *This,
                                              const ThisAdjustment &TA) = 0;
 
   virtual llvm::Value *performReturnAdjustment(CodeGenFunction &CGF,
                                                llvm::Value *Ret,
                                                const ReturnAdjustment &RA) = 0;

cfe/trunk/lib/CodeGen/CGVTables.cpp

void CodeGenVTables::emitThunk(GlobalDecl GD, const ThunkInfo &Thunk,
   if (ThunkFn->isVarArg()) {
     // Varargs thunks are special; we can't just generate a call because
     // we can't copy the varargs.  Our implementation is rather
     // expensive/sucky at the moment, so don't generate the thunk unless
     // we have to.
     // FIXME: Do something better here; GenerateVarArgsThunk is extremely ugly.
     if (!UseAvailableExternallyLinkage) {
       CodeGenFunction(CGM).GenerateVarArgsThunk(ThunkFn, FnInfo, GD, Thunk);
-      CGM.getCXXABI().setThunkLinkage(ThunkFn, ForVTable);
+      CGM.getCXXABI().setThunkLinkage(ThunkFn, ForVTable, GD,
+                                      !Thunk.Return.isEmpty());
     }
   } else {
     // Normal thunk body generation.
     CodeGenFunction(CGM).GenerateThunk(ThunkFn, FnInfo, GD, Thunk);
-    CGM.getCXXABI().setThunkLinkage(ThunkFn, ForVTable);
+    CGM.getCXXABI().setThunkLinkage(ThunkFn, ForVTable, GD,
+                                    !Thunk.Return.isEmpty());
   }
 }
 
 void CodeGenVTables::maybeEmitThunkForVTable(GlobalDecl GD,
                                              const ThunkInfo &Thunk) {
   // If the ABI has key functions, only the TU with the key function should emit
   // the thunk. However, we can allow inlining of thunks if we emit them with
   // available_externally linkage together with vtables when optimizations are

cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp

public:
 
   void EmitVirtualDestructorCall(CodeGenFunction &CGF,
                                  const CXXDestructorDecl *Dtor,
                                  CXXDtorType DtorType, SourceLocation CallLoc,
                                  llvm::Value *This) override;
 
   void emitVirtualInheritanceTables(const CXXRecordDecl *RD) override;
 
-  void setThunkLinkage(llvm::Function *Thunk, bool ForVTable) override {
+  void setThunkLinkage(llvm::Function *Thunk, bool ForVTable, GlobalDecl GD,
+                       bool ReturnAdjustment) override {
     // Allow inlining of thunks by emitting them with available_externally
     // linkage together with vtables when needed.
     if (ForVTable)
       Thunk->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);
   }
 
   llvm::Value *performThisAdjustment(CodeGenFunction &CGF, llvm::Value *This,
                                      const ThisAdjustment &TA) override;

cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

public:
 
   llvm::GlobalVariable *
   getAddrOfVBTable(const VPtrInfo &VBT, const CXXRecordDecl *RD,
                    llvm::GlobalVariable::LinkageTypes Linkage);
 
   void emitVBTableDefinition(const VPtrInfo &VBT, const CXXRecordDecl *RD,
                              llvm::GlobalVariable *GV) const;
 
-  void setThunkLinkage(llvm::Function *Thunk, bool ForVTable) override {
-    Thunk->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
-
+  void setThunkLinkage(llvm::Function *Thunk, bool ForVTable,
+                       GlobalDecl GD, bool ReturnAdjustment) override {
     // Never dllimport/dllexport thunks.
     Thunk->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
+
+    GVALinkage Linkage =
+        getContext().GetGVALinkageForFunction(cast<FunctionDecl>(GD.getDecl()));
+
+    if (Linkage == GVA_Internal)
+      Thunk->setLinkage(llvm::GlobalValue::InternalLinkage);
+    else if (ReturnAdjustment)
+      Thunk->setLinkage(llvm::GlobalValue::WeakODRLinkage);
+    else
+      Thunk->setLinkage(llvm::GlobalValue::LinkOnceODRLinkage);
   }
 
   llvm::Value *performThisAdjustment(CodeGenFunction &CGF, llvm::Value *This,
                                      const ThisAdjustment &TA) override;
 
   llvm::Value *performReturnAdjustment(CodeGenFunction &CGF, llvm::Value *Ret,
                                        const ReturnAdjustment &RA) override;
 

cfe/trunk/test/CodeGenCXX/dllimport.cpp

namespace DontUseDtorAlias {
   inline B::~B() { }
   // Emit a real definition of B's constructor; don't alias it to A's.
   // MO1-DAG: available_externally dllimport x86_thiscallcc void @"\01??1B@DontUseDtorAlias@@QAE@XZ"
   USECLASS(B)
 }
 
 namespace Vtordisp {
   // Don't dllimport the vtordisp.
-  // MO1-DAG: define weak x86_thiscallcc void @"\01?f@?$C@D@Vtordisp@@$4PPPPPPPM@A@AEXXZ"
+  // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@D@Vtordisp@@$4PPPPPPPM@A@AEXXZ"
 
   class Base {
     virtual void f() {}
   };
   template <typename T>
   class __declspec(dllimport) C : virtual public Base {
   public:
     C() {}

cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp

 // CHECK:   call x86_thiscallcc void @"\01??1B@dtor_in_second_nvbase@@UAE@XZ"
 // CHECK:       (%"struct.dtor_in_second_nvbase::B"* %{{.*}})
 // CHECK:   ret void
 }
 
 void foo() {
   C c;
 }
-// DTORS2-LABEL: define weak x86_thiscallcc void @"\01??_EC@dtor_in_second_nvbase@@W3AEPAXI@Z"
+// DTORS2-LABEL: define linkonce_odr x86_thiscallcc void @"\01??_EC@dtor_in_second_nvbase@@W3AEPAXI@Z"
 // DTORS2:       (%"struct.dtor_in_second_nvbase::C"* %this, i32 %should_call_delete)
 //      Do an adjustment from B* to C*.
 // DTORS2:   getelementptr i8* %{{.*}}, i32 -4
 // DTORS2:   bitcast i8* %{{.*}} to %"struct.dtor_in_second_nvbase::C"*
 // DTORS2:   call x86_thiscallcc void @"\01??_GC@dtor_in_second_nvbase@@UAEPAXI@Z"
 // DTORS2:   ret void
 
 }

cfe/trunk/test/CodeGenCXX/microsoft-abi-thunks.cpp

private:
   // MANGLING-DAG: @"\01?private_f@C@@EAEXXZ"
   // MANGLING-DAG: @"\01?private_f@C@@G3AEXXZ"
   // MANGLING-X64-DAG: @"\01?private_f@C@@EEAAXXZ"
   // MANGLING-X64-DAG: @"\01?private_f@C@@G7EAAXXZ"
 };
 
 C::C() {}  // Emits vftable and forces thunk generation.
 
-// CODEGEN-LABEL: define weak x86_thiscallcc void @"\01??_EC@@W3AEPAXI@Z"(%struct.C* %this, i32 %should_call_delete)
+// CODEGEN-LABEL: define linkonce_odr x86_thiscallcc void @"\01??_EC@@W3AEPAXI@Z"(%struct.C* %this, i32 %should_call_delete)
 // CODEGEN:   getelementptr i8* {{.*}}, i32 -4
 // FIXME: should actually call _EC, not _GC.
 // CODEGEN:   call x86_thiscallcc void @"\01??_GC@@UAEPAXI@Z"
 // CODEGEN: ret
 
-// CODEGEN-LABEL: define weak x86_thiscallcc void @"\01?public_f@C@@W3AEXXZ"(%struct.C*
+// CODEGEN-LABEL: define linkonce_odr x86_thiscallcc void @"\01?public_f@C@@W3AEXXZ"(%struct.C*
 // CODEGEN:   getelementptr i8* {{.*}}, i32 -4
 // CODEGEN:   call x86_thiscallcc void @"\01?public_f@C@@UAEXXZ"(%struct.C*
 // CODEGEN: ret
 
 void zoo(C* obj) {
   delete obj;
 }
 
 struct D {
   virtual B* goo();
 };
 
 struct E : D {
   E();
   virtual C* goo();
   // MANGLING-DAG: @"\01?goo@E@@UAEPAUC@@XZ"
   // MANGLING-DAG: @"\01?goo@E@@QAEPAUB@@XZ"
   // MANGLING-X64-DAG: @"\01?goo@E@@UEAAPEAUC@@XZ"
   // MANGLING-X64-DAG: @"\01?goo@E@@QEAAPEAUB@@XZ"
 };
 
 E::E() {}  // Emits vftable and forces thunk generation.
 
-// CODEGEN-LABEL: define weak x86_thiscallcc %struct.C* @"\01?goo@E@@QAEPAUB@@XZ"
+// CODEGEN-LABEL: define weak_odr x86_thiscallcc %struct.C* @"\01?goo@E@@QAEPAUB@@XZ"
 // CODEGEN:   call x86_thiscallcc %struct.C* @"\01?goo@E@@UAEPAUC@@XZ"
 // CODEGEN:   getelementptr inbounds i8* {{.*}}, i32 4
 // CODEGEN: ret
 
 struct F : virtual A, virtual B {
   virtual void own_method();
   virtual ~F();
 };

 
 struct I : D {
   I();
   virtual F* goo();
 };
 
 I::I() {}  // Emits vftable and forces thunk generation.
 
-// CODEGEN-LABEL: define weak x86_thiscallcc %struct.{{[BF]}}* @"\01?goo@I@@QAEPAUB@@XZ"
+// CODEGEN-LABEL: define weak_odr x86_thiscallcc %struct.{{[BF]}}* @"\01?goo@I@@QAEPAUB@@XZ"
 // CODEGEN: %[[ORIG_RET:.*]] = call x86_thiscallcc %struct.F* @"\01?goo@I@@UAEPAUF@@XZ"
 // CODEGEN: %[[ORIG_RET_i8:.*]] = bitcast %struct.F* %[[ORIG_RET]] to i8*
 // CODEGEN: %[[VBPTR_i8:.*]] = getelementptr inbounds i8* %[[ORIG_RET_i8]], i32 4
 // CODEGEN: %[[VBPTR:.*]] = bitcast i8* %[[VBPTR_i8]] to i8**
 // CODEGEN: %[[VBTABLE:.*]] = load i8** %[[VBPTR]]
 // CODEGEN: %[[VBASE_OFFSET_PTR_i8:.*]] = getelementptr inbounds i8* %[[VBTABLE]], i32 8
 // CODEGEN: %[[VBASE_OFFSET_PTR:.*]] = bitcast i8* %[[VBASE_OFFSET_PTR_i8]] to i32*
 // CODEGEN: %[[VBASE_OFFSET:.*]] = load i32* %[[VBASE_OFFSET_PTR]]

 struct B {
   virtual void __stdcall foo();
 };
 struct C : A, B {
   virtual void __stdcall foo();
 };
 C c;
 }
+
+namespace {
+struct E : D {
+  E();
+  virtual C* goo();
+};
+E::E() {}
+E e;
+// Class with internal linkage has internal linkage thunks.
+// CODEGEN: define internal x86_thiscallcc %struct.C* @"\01?goo@E@?A@@QAEPAUB@@XZ"
+}

cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp

struct D : virtual C {
   ~D();
   virtual void f();
   void g();
   int xxx;
 };
 
 D::D() {}  // Forces vftable emission.
 
-// CHECK-LABEL: define weak x86_thiscallcc void @"\01?f@D@@$4PPPPPPPM@A@AEXXZ"
+// CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@D@@$4PPPPPPPM@A@AEXXZ"
 // CHECK: %[[ECX:.*]] = load %struct.D** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.D* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr i8* %[[ECX_i8]], i32 -4
 // CHECK: %[[VTORDISP_PTR:.*]] = bitcast i8* %[[VTORDISP_PTR_i8]] to i32*
 // CHECK: %[[VTORDISP:.*]] = load i32* %[[VTORDISP_PTR]]
 // CHECK: %[[VTORDISP_NEG:.*]] = sub i32 0, %[[VTORDISP]]
 // CHECK: %[[ADJUSTED_i8:.*]] = getelementptr i8* %[[ECX_i8]], i32 %[[VTORDISP_NEG]]
 // CHECK: call x86_thiscallcc void @"\01?f@D@@UAEXXZ"(i8* %[[ADJUSTED_i8]])
 // CHECK: ret void
 
-// CHECK-LABEL: define weak x86_thiscallcc void @"\01?f@D@@$4PPPPPPPI@3AEXXZ"
+// CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@D@@$4PPPPPPPI@3AEXXZ"
 // CHECK: %[[ECX:.*]] = load %struct.D** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.D* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr i8* %[[ECX_i8]], i32 -8
 // CHECK: %[[VTORDISP_PTR:.*]] = bitcast i8* %[[VTORDISP_PTR_i8]] to i32*
 // CHECK: %[[VTORDISP:.*]] = load i32* %[[VTORDISP_PTR]]
 // CHECK: %[[VTORDISP_NEG:.*]] = sub i32 0, %[[VTORDISP]]
 // CHECK: %[[VTORDISP_ADJUSTED_i8:.*]] = getelementptr i8* %[[ECX_i8]], i32 %[[VTORDISP_NEG]]
 // CHECK: %[[ADJUSTED_i8:.*]] = getelementptr i8* %[[VTORDISP_ADJUSTED_i8]], i32 -4

 struct G : virtual F, virtual E {
   int ggg;
   G();
   ~G();
 };
 
 G::G() {}  // Forces vftable emission.
 
-// CHECK-LABEL: define weak x86_thiscallcc void @"\01?f@E@@$R4BA@M@PPPPPPPM@7AEXXZ"(i8*)
+// CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@E@@$R4BA@M@PPPPPPPM@7AEXXZ"(i8*)
 // CHECK: %[[ECX:.*]] = load %struct.E** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.E* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr i8* %[[ECX_i8]], i32 -4
 // CHECK: %[[VTORDISP_PTR:.*]] = bitcast i8* %[[VTORDISP_PTR_i8]] to i32*
 // CHECK: %[[VTORDISP:.*]] = load i32* %[[VTORDISP_PTR]]
 // CHECK: %[[VTORDISP_NEG:.*]] = sub i32 0, %[[VTORDISP]]
 // CHECK: %[[VTORDISP_ADJUSTED_i8:.*]] = getelementptr i8* %[[ECX_i8]], i32 %[[VTORDISP_NEG]]
 // CHECK: %[[VBPTR_i8:.*]] = getelementptr inbounds i8* %[[VTORDISP_ADJUSTED_i8]], i32 -16

cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp

struct X: virtual C {
   // CHECK-LABEL: Thunks for 'void C::f()' (1 entry).
   // CHECK-NEXT: 0 | [this adjustment: 8 non-virtual]
 
   // CHECK-NOT: VFTable indices for 'Test4::X'
 
   // MANGLING-DAG: @"\01??_7X@Test4@@6B@"
 
   // Also check the mangling of the thunk.
-  // MANGLING-DAG: define weak x86_thiscallcc void @"\01?f@C@@WPPPPPPPI@AEXXZ"
+  // MANGLING-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@C@@WPPPPPPPI@AEXXZ"
 };
 
 X::X() {}
 }
 
 namespace Test5 {
 
 // New methods are added to the base's vftable.