This revision was automatically updated to reflect the committed changes.
Closed by commit rGeae2d4b8520c: [Windows Itanium][PS4] handle dllimport/export
w.r.t vtables/rtti (authored by Ben Dunbobbin <ben.dunbob...@sony.com>).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D93203?vs=318527&id=337091#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93203/new/
https://reviews.llvm.org/D93203
Files:
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/ps4-dllstorage-vtable-rtti.cpp
Index: clang/test/CodeGenCXX/ps4-dllstorage-vtable-rtti.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/ps4-dllstorage-vtable-rtti.cpp
@@ -0,0 +1,210 @@
+// For a class that has a vtable (and hence, also has a typeinfo symbol for
+// RTTI), if a user marks either:
+//
+// (a) the entire class as dllexport (dllimport), or
+// (b) all non-inline virtual methods of the class as dllexport (dllimport)
+//
+// then Clang must export the vtable and typeinfo symbol from the TU where they
+// are defined (the TU containing the definition of the Itanium C++ ABI "key
+// function"), and must import them in other modules where they are referenced.
+//
+// Conversely to point (b), if some (but not all) of the non-inline virtual
+// methods of a class are marked as dllexport (dllimport), then the vtable and
+// typeinfo symbols must not be exported (imported). This will result in a
+// link-time failure when linking the importing module. This link-time failure
+// is the desired behavior, because the Microsoft toolchain also gets a
+// link-time failure in these cases (and since __declspec(dllexport)
+// (__declspec(dllimport)) is a Microsoft extension, our intention is to mimic
+// that Microsoft behavior).
+//
+// Side note: It is within the bodies of constructors (and in some cases,
+// destructors) that the vtable is explicitly referenced. In case (a) above,
+// where the entire class is exported (imported), then all constructors (among
+// other things) are exported (imported). So for that situation, an importing
+// module for a well-formed program will not actually reference the vtable,
+// since constructor calls will all be to functions external to that module
+// (and imported into it, from the exporting module). I.e., all vtable
+// references will be in that module where the constructor and destructor
+// bodies are, therefore, there will not be a need to import the vtable in
+// that case.
+//
+// This test contains 6 test classes:
+// 2 for point (a),
+// 2 for point (b),
+// and 2 negative tests for the converse of point (b).
+//
+// The two tests for each of these points are one for importing, and one for
+// exporting.
+
+// RUN: %clang_cc1 -I%S -fdeclspec -triple x86_64-unknown-windows-itanium -emit-llvm -o - %s -fhalf-no-semantic-interposition | FileCheck %s -check-prefix=WI
+// RUN: %clang_cc1 -I%S -fdeclspec -triple x86_64-scei-windows-itanium -emit-llvm -o - %s -fhalf-no-semantic-interposition | FileCheck %s --check-prefixes=PS4,SCEI_WI
+// RUN: %clang_cc1 -I%S -fdeclspec -triple x86_64-scei-ps4 -emit-llvm -o - %s -fhalf-no-semantic-interposition | FileCheck %s --check-prefixes=PS4,SCEI_PS4
+
+#include <typeinfo>
+
+// Case (a) -- Import Aspect
+// The entire class is imported. The typeinfo symbol must also be imported,
+// but the vtable will not be referenced, and so does not need to be imported
+// (as described in the "Side note", above).
+//
+// PS4-DAG: @_ZTI10FullImport = {{.*}}dllimport
+// WI-DAG: @_ZTI10FullImport = external dllimport constant i8*
+struct __declspec(dllimport) FullImport
+{
+ virtual void getId() {}
+ virtual void Bump();
+ virtual void Decrement();
+};
+
+// 'FullImport::Bump()' is the key function, so the vtable and typeinfo symbol
+// of 'FullImport' will be defined in the TU that contains the definition of
+// 'Bump()' (and they must be exported from there).
+void FullImportTest()
+{
+ typeid(FullImport).name();
+}
+
+///////////////////////////////////////////////////////////////////
+
+// Case (a) -- Export Aspect
+// The entire class is exported. The vtable and typeinfo symbols must also be
+// exported,
+//
+// PS4-DAG: @_ZTV10FullExport ={{.*}}dllexport
+// WI-DAG: @_ZTV10FullExport ={{.*}}dllexport
+// PS4-DAG: @_ZTI10FullExport ={{.*}}dllexport
+// WI-DAG: @_ZTI10FullExport = dso_local dllexport constant {
+struct __declspec(dllexport) FullExport // Easy case: Entire class is exported.
+{
+ virtual void getId() {}
+ virtual void Bump();
+ virtual void Decrement();
+};
+
+// This is the key function of the class 'FullExport', so the vtable and
+// typeinfo symbols of 'FullExport' will be defined in this TU, and so they
+// must be exported from this TU.
+void FullExport::Bump()
+{
+ typeid(FullExport).name();
+}
+
+///////////////////////////////////////////////////////////////////
+
+// Case (b) -- Import Aspect
+// The class as a whole is not imported, but all non-inline virtual methods of
+// the class are, so the vtable and typeinfo symbol must be imported.
+//
+// PS4-DAG: @_ZTV9FooImport ={{.*}}dllimport
+// WI-DAG: @_ZTV9FooImport = linkonce_odr dso_local unnamed_addr constant {
+// PS4-DAG: @_ZTI9FooImport ={{.*}}dllimport
+// WI-DAG: @_ZTI9FooImport = linkonce_odr dso_local constant {
+
+
+struct FooImport
+{
+ virtual void getId() const {}
+ __declspec(dllimport) virtual void Bump();
+ __declspec(dllimport) virtual void Decrement();
+};
+
+// 'FooImport::Bump()' is the key function, so the vtable and typeinfo symbol
+// of 'FooImport' will be defined in the TU that contains the definition of
+// 'Bump()' (and they must be exported from there). Here, we will reference
+// the vtable and typeinfo symbol, so we must also import them.
+void importTest()
+{
+ typeid(FooImport).name();
+}
+
+///////////////////////////////////////////////////////////////////
+
+// Case (b) -- Export Aspect
+// The class as a whole is not exported, but all non-inline virtual methods of
+// the class are, so the vtable and typeinfo symbol must be exported.
+//
+// PS4-DAG: @_ZTV9FooExport ={{.*}}dllexport
+// WI-DAG: @_ZTV9FooExport = dso_local unnamed_addr constant {
+// PS4-DAG: @_ZTI9FooExport ={{.*}}dllexport
+// WI-DAG: @_ZTI9FooExport = dso_local constant {
+struct FooExport
+{
+ virtual void getId() const {}
+ __declspec(dllexport) virtual void Bump();
+ __declspec(dllexport) virtual void Decrement();
+};
+
+// This is the key function of the class 'FooExport', so the vtable and
+// typeinfo symbol of 'FooExport' will be defined in this TU, and so they must
+// be exported from this TU.
+void FooExport::Bump()
+{
+ FooImport f;
+ typeid(FooExport).name();
+}
+
+///////////////////////////////////////////////////////////////////
+
+// The tests below verify that the associated vtable and typeinfo symbols are
+// not imported/exported. These are the converse of case (b).
+//
+// Note that ultimately, if the module doing the importing calls a constructor
+// of the class with the vtable, or makes a reference to the typeinfo symbol of
+// the class, then this will result in an unresolved reference (to the vtable
+// or typeinfo symbol) when linking the importing module, and thus a link-time
+// failure.
+//
+// Note that with the Microsoft toolchain there will also be a link-time
+// failure when linking the module doing the importing. With the Microsoft
+// toolchain, it will be an unresolved reference to the method 'Decrement()'
+// of the approriate class, rather than to the vtable or typeinfo symbol of
+// the class, because Microsoft defines the vtable and typeinfo symbol (weakly)
+// everywhere they are used.
+
+// Converse of case (b) -- Import Aspect
+// The class as a whole is not imported, and not all non-inline virtual methods
+// are imported, so the vtable and typeinfo symbol are not to be imported.
+//
+// CHECK-PS4: @_ZTV11FooNoImport = external dso_local unnamed_addr constant {
+// CHECK-WI: @_ZTV11FooNoImport = linkonce_odr dso_local unnamed_addr constant {
+// CHECK-PS4: @_ZTI11FooNoImport = external dso_local constant i8*{{$}}
+// CHECK-WI: @_ZTI11FooNoImport = linkonce_odr dso_local constant {
+struct FooNoImport
+{
+ virtual void getId() const {}
+ __declspec(dllimport) virtual void Bump();
+ virtual void Decrement(); // Not imported.
+ int mCounter;
+};
+
+void importNegativeTest()
+{
+ FooNoImport f;
+ typeid(FooNoImport).name();
+}
+
+///////////////////////////////////////////////////////////////////
+
+// Converse of case (b) -- Export Aspect
+// The class as a whole is not exported, and not all non-inline virtual methods
+// are exported, so the vtable and typeinfo symbol are not to be exported.
+//
+// SCEI_PS4-DAG: @_ZTV11FooNoImport = external unnamed_addr constant {
+// SCEI_WI-DAG: @_ZTV11FooNoExport = dso_local unnamed_addr constant {
+
+// WI-DAG: @_ZTV11FooNoExport = dso_local unnamed_addr constant {
+// SCEI_PS4-DAG: @_ZTI11FooNoExport = constant {
+// SCEI_WI-DAG: @_ZTI11FooNoExport = dso_local constant {
+// WI-DAG: @_ZTI11FooNoExport = dso_local constant {
+struct FooNoExport
+{
+ virtual void getId() const {}
+ __declspec(dllexport) virtual void Bump();
+ virtual void Decrement(); // Not exported.
+ int mCounter;
+};
+
+void FooNoExport::Bump()
+{
+ typeid(FooNoExport).name();
+}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1835,6 +1835,29 @@
/*InRangeIndex=*/1);
}
+// Check whether all the non-inline virtual methods for the class have the
+// specified attribute.
+template <typename T>
+static bool CXXRecordAllNonInlineVirtualsHaveAttr(const CXXRecordDecl *RD) {
+ bool FoundNonInlineVirtualMethodWithAttr = false;
+ for (const auto *D : RD->noload_decls()) {
+ if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+ if (!FD->isVirtualAsWritten() || FD->isInlineSpecified() ||
+ FD->doesThisDeclarationHaveABody())
+ continue;
+ if (!D->hasAttr<T>())
+ return false;
+ FoundNonInlineVirtualMethodWithAttr = true;
+ }
+ }
+
+ // We didn't find any non-inline virtual methods missing the attribute. We
+ // will return true when we found at least one non-inline virtual with the
+ // attribute. (This lets our caller know that the attribute needs to be
+ // propagated up to the vtable.)
+ return FoundNonInlineVirtualMethodWithAttr;
+}
+
llvm::Value *ItaniumCXXABI::getVTableAddressPointInStructorWithVTT(
CodeGenFunction &CGF, const CXXRecordDecl *VTableClass, BaseSubobject Base,
const CXXRecordDecl *NearestVBase) {
@@ -1891,6 +1914,24 @@
getContext().toCharUnitsFromBits(PAlign).getQuantity());
VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+ // In MS C++ if you have a class with virtual functions in which you are using
+ // selective member import/export, then all virtual functions must be exported
+ // unless they are inline, otherwise a link error will result. To match this
+ // behavior, for such classes, we dllimport the vtable if it is defined
+ // externally and all the non-inline virtual methods are marked dllimport, and
+ // we dllexport the vtable if it is defined in this TU and all the non-inline
+ // virtual methods are marked dllexport.
+ if (CGM.getTarget().hasPS4DLLImportExport()) {
+ if ((!RD->hasAttr<DLLImportAttr>()) && (!RD->hasAttr<DLLExportAttr>())) {
+ if (CGM.getVTables().isVTableExternal(RD)) {
+ if (CXXRecordAllNonInlineVirtualsHaveAttr<DLLImportAttr>(RD))
+ VTable->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+ } else {
+ if (CXXRecordAllNonInlineVirtualsHaveAttr<DLLExportAttr>(RD))
+ VTable->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
+ }
+ }
+ }
CGM.setGVProperties(VTable, RD);
return VTable;
@@ -3139,6 +3180,14 @@
Name);
const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
CGM.setGVProperties(GV, RD);
+ // Import the typeinfo symbol when all non-inline virtual methods are
+ // imported.
+ if (CGM.getTarget().hasPS4DLLImportExport()) {
+ if (RD && CXXRecordAllNonInlineVirtualsHaveAttr<DLLImportAttr>(RD)) {
+ GV->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
+ CGM.setDSOLocal(GV);
+ }
+ }
}
return llvm::ConstantExpr::getBitCast(GV, CGM.Int8PtrTy);
@@ -3315,11 +3364,14 @@
if (CGM.getTriple().isWindowsGNUEnvironment())
return false;
- if (CGM.getVTables().isVTableExternal(RD))
+ if (CGM.getVTables().isVTableExternal(RD)) {
+ if (CGM.getTarget().hasPS4DLLImportExport())
+ return true;
+
return IsDLLImport && !CGM.getTriple().isWindowsItaniumEnvironment()
? false
: true;
-
+ }
if (IsDLLImport)
return true;
}
@@ -3771,6 +3823,18 @@
new llvm::GlobalVariable(M, Init->getType(),
/*isConstant=*/true, Linkage, Init, Name);
+ // Export the typeinfo in the same circumstances as the vtable is exported.
+ auto GVDLLStorageClass = DLLStorageClass;
+ if (CGM.getTarget().hasPS4DLLImportExport()) {
+ if (const RecordType *RecordTy = dyn_cast<RecordType>(Ty)) {
+ const CXXRecordDecl *RD = cast<CXXRecordDecl>(RecordTy->getDecl());
+ if (RD->hasAttr<DLLExportAttr>() ||
+ CXXRecordAllNonInlineVirtualsHaveAttr<DLLExportAttr>(RD)) {
+ GVDLLStorageClass = llvm::GlobalVariable::DLLExportStorageClass;
+ }
+ }
+ }
+
// If there's already an old global variable, replace it with the new one.
if (OldGV) {
GV->takeName(OldGV);
@@ -3809,7 +3873,9 @@
CGM.setDSOLocal(GV);
TypeName->setDLLStorageClass(DLLStorageClass);
- GV->setDLLStorageClass(DLLStorageClass);
+ GV->setDLLStorageClass(CGM.getTarget().hasPS4DLLImportExport()
+ ? GVDLLStorageClass
+ : DLLStorageClass);
TypeName->setPartition(CGM.getCodeGenOpts().SymbolPartition);
GV->setPartition(CGM.getCodeGenOpts().SymbolPartition);
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2293,7 +2293,8 @@
// If the key function is dllimport but the class isn't, then the class has
// no key function. The DLL that exports the key function won't export the
// vtable in this case.
- if (MD->hasAttr<DLLImportAttr>() && !RD->hasAttr<DLLImportAttr>())
+ if (MD->hasAttr<DLLImportAttr>() && !RD->hasAttr<DLLImportAttr>() &&
+ !Context.getTargetInfo().hasPS4DLLImportExport())
return nullptr;
// We found it.
Index: clang/include/clang/Basic/TargetInfo.h
===================================================================
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1130,6 +1130,15 @@
getTriple().isWindowsItaniumEnvironment() || getTriple().isPS4CPU();
}
+ // Does this target have PS4 specific dllimport/export handling?
+ virtual bool hasPS4DLLImportExport() const {
+ return getTriple().isPS4CPU() ||
+ // Windows Itanium support allows for testing the SCEI flavour of
+ // dllimport/export handling on a Windows system.
+ (getTriple().isWindowsItaniumEnvironment() &&
+ getTriple().getVendor() == llvm::Triple::SCEI);
+ }
+
/// An optional hook that targets can implement to perform semantic
/// checking on attribute((section("foo"))) specifiers.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
