[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function
This revision was automatically updated to reflect the committed changes.
Closed by commit rL349256: [MinGW] Produce a vtable and RTTI for dllexported
classes without a key function (authored by mstorsjo, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D55698?vs=178275=178343#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55698/new/
https://reviews.llvm.org/D55698
Files:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/dllexport-missing-key.cpp
Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -5528,6 +5528,9 @@
// declaration.
return;
+ if (S.Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+S.MarkVTableUsed(Class->getLocation(), Class, true);
+
for (Decl *Member : Class->decls()) {
// Defined static variables that are members of an exported base
// class must be marked export too.
Index: cfe/trunk/test/CodeGenCXX/dllexport-missing-key.cpp
===
--- cfe/trunk/test/CodeGenCXX/dllexport-missing-key.cpp
+++ cfe/trunk/test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s |
FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -5528,6 +5528,9 @@
// declaration.
return;
+ if (S.Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+S.MarkVTableUsed(Class->getLocation(), Class, true);
+
for (Decl *Member : Class->decls()) {
// Defined static variables that are members of an exported base
// class must be marked export too.
Index: cfe/trunk/test/CodeGenCXX/dllexport-missing-key.cpp
===
--- cfe/trunk/test/CodeGenCXX/dllexport-missing-key.cpp
+++ cfe/trunk/test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s | FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55698/new/ https://reviews.llvm.org/D55698 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function
mstorsjo updated this revision to Diff 178275.
mstorsjo added a comment.
Moved the code to work on DelayedDllExportClasses instead, as suggested, which
still works for the testcase. (I've yet to test that approach on a larger
codebase though.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55698/new/
https://reviews.llvm.org/D55698
Files:
lib/Sema/SemaDeclCXX.cpp
test/CodeGenCXX/dllexport-missing-key.cpp
Index: test/CodeGenCXX/dllexport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s |
FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5493,6 +5493,9 @@
// declaration.
return;
+ if (S.Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+S.MarkVTableUsed(Class->getLocation(), Class, true);
+
for (Decl *Member : Class->decls()) {
// Defined static variables that are members of an exported base
// class must be marked export too.
Index: test/CodeGenCXX/dllexport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s | FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5493,6 +5493,9 @@
// declaration.
return;
+ if (S.Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+S.MarkVTableUsed(Class->getLocation(), Class, true);
+
for (Decl *Member : Class->decls()) {
// Defined static variables that are members of an exported base
// class must be marked export too.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function
mstorsjo marked 2 inline comments as done. mstorsjo added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:5754-5756 + if (ClassExported && + Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) +MarkVTableUsed(Class->getLocation(), Class, true); rnk wrote: > This may be too early, you can get into situations like this if you start > emitting the vtable (which will emit inline methods) before we get to the end > of the outermost class. See this bug for example: > https://bugs.llvm.org/show_bug.cgi?id=40006 > > Maybe if you have a dllexported nested class with a virtual method that > references the constructor of the outer class which has a late-parsed member > initializer... you can get things to go wrong as in the bug above. > > I think the fix will be to touch the vtable when we process delayed > dllexported classes from the list just above this line. Ok, will upload a new version of the patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55698/new/ https://reviews.llvm.org/D55698 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function
rnk added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:5754-5756 + if (ClassExported && + Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) +MarkVTableUsed(Class->getLocation(), Class, true); This may be too early, you can get into situations like this if you start emitting the vtable (which will emit inline methods) before we get to the end of the outermost class. See this bug for example: https://bugs.llvm.org/show_bug.cgi?id=40006 Maybe if you have a dllexported nested class with a virtual method that references the constructor of the outer class which has a late-parsed member initializer... you can get things to go wrong as in the bug above. I think the fix will be to touch the vtable when we process delayed dllexported classes from the list just above this line. Comment at: test/CodeGenCXX/dllexport-missing-key.cpp:20 + +// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport +// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr OK, good, I was going to ask if it became weak_odr, but looks like it all works. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55698/new/ https://reviews.llvm.org/D55698 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function
mstorsjo updated this revision to Diff 178222.
mstorsjo added a comment.
Fixed wrapping of the RUN line in the test.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55698/new/
https://reviews.llvm.org/D55698
Files:
lib/Sema/SemaDeclCXX.cpp
test/CodeGenCXX/dllexport-missing-key.cpp
Index: test/CodeGenCXX/dllexport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s |
FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5750,6 +5750,10 @@
if (ClassExported)
DelayedDllExportClasses.push_back(Class);
+
+ if (ClassExported &&
+ Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+MarkVTableUsed(Class->getLocation(), Class, true);
}
/// Perform propagation of DLL attributes from a derived class to a
Index: test/CodeGenCXX/dllexport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s | FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5750,6 +5750,10 @@
if (ClassExported)
DelayedDllExportClasses.push_back(Class);
+
+ if (ClassExported &&
+ Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+MarkVTableUsed(Class->getLocation(), Class, true);
}
/// Perform propagation of DLL attributes from a derived class to a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function
mstorsjo created this revision.
mstorsjo added reviewers: rnk, majnemer.
This matches what GCC does in these situations.
This fixes compiling Qt in debug mode. In release mode, references to the
vtable of this particular class ends up optimized away, but in debug mode, the
compiler creates references to the vtable, which is expected to be dllexported
from a different DLL. Make sure the dllexported version actually ends up
emitted.
Repository:
rC Clang
https://reviews.llvm.org/D55698
Files:
lib/Sema/SemaDeclCXX.cpp
test/CodeGenCXX/dllexport-missing-key.cpp
Index: test/CodeGenCXX/dllexport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s |
+// FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5750,6 +5750,10 @@
if (ClassExported)
DelayedDllExportClasses.push_back(Class);
+
+ if (ClassExported &&
+ Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+MarkVTableUsed(Class->getLocation(), Class, true);
}
/// Perform propagation of DLL attributes from a derived class to a
Index: test/CodeGenCXX/dllexport-missing-key.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-missing-key.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -std=c++11 -o - %s |
+// FileCheck --check-prefix=GNU %s
+
+class __declspec(dllexport) QAbstractLayoutStyleInfo {
+public:
+ QAbstractLayoutStyleInfo() : m_isWindow(false) {}
+ virtual ~QAbstractLayoutStyleInfo() {}
+
+ virtual bool hasChangedCore() const { return false; }
+
+ virtual void invalidate() {}
+
+ virtual double windowMargin(bool orientation) const = 0;
+
+ bool isWindow() const { return m_isWindow; }
+
+protected:
+ bool m_isWindow;
+};
+
+// GNU-DAG: @_ZTV24QAbstractLayoutStyleInfo = weak_odr dso_local dllexport
+// GNU-DAG: @_ZTS24QAbstractLayoutStyleInfo = linkonce_odr
+// GNU-DAG: @_ZTI24QAbstractLayoutStyleInfo = linkonce_odr
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5750,6 +5750,10 @@
if (ClassExported)
DelayedDllExportClasses.push_back(Class);
+
+ if (ClassExported &&
+ Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+MarkVTableUsed(Class->getLocation(), Class, true);
}
/// Perform propagation of DLL attributes from a derived class to a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
