r324991 - Fix for PR32992. Static const classes not exported.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 20 01:03:59 PST 2018
The problem is that your patch caused the reproducer to trigger an
assert, which it didn't do before, causing our builds to break.
Your patch seems like it does fix the PR, but it can't break currently
working builds.
$ build.release/bin/clang -cc1 -triple i386-pc-windows-msvc19.11.0
-emit-pch -fms-extensions -fms-compatibility
-fms-compatibility-version=19.11 -std=c++14 -fdelayed-template-parsing
-x c++-header /tmp/a.cc -o /dev/null
clang: /work/llvm.combined/llvm/tools/clang/lib/Serialization/ASTWriter.cpp:4723:
clang::ASTFileSignature clang::ASTWriter::WriteASTCore(clang::Sema&,
llvm::StringRef, const string&, clang::Module*): Assertion
`SemaRef.PendingLocalImplicitInstantiations.empty() && "There are
local ones at end of translation unit!"' failed.
build.release/bin/clang(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x55c535576aaa]
build.release/bin/clang(_ZN4llvm3sys17RunSignalHandlersEv+0x3e)[0x55c53557492e]
build.release/bin/clang(+0x2424a7c)[0x55c535574a7c]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x110c0)[0x7f4aeeb9d0c0]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcf)[0x7f4aed732fcf]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x16a)[0x7f4aed7343fa]
/lib/x86_64-linux-gnu/libc.so.6(+0x2be37)[0x7f4aed72be37]
/lib/x86_64-linux-gnu/libc.so.6(+0x2bee2)[0x7f4aed72bee2]
build.release/bin/clang(_ZN5clang9ASTWriter12WriteASTCoreERNS_4SemaEN4llvm9StringRefERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPNS_6ModuleE+0x29fa)[0x55c536444cea]
build.release/bin/clang(_ZN5clang9ASTWriter8WriteASTERNS_4SemaERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPNS_6ModuleEN4llvm9StringRefEb+0xad)[0x55c536444dfd]
build.release/bin/clang(_ZN5clang12PCHGenerator21HandleTranslationUnitERNS_10ASTContextE+0x80)[0x55c536465fe0]
build.release/bin/clang(_ZN5clang17MultiplexConsumer21HandleTranslationUnitERNS_10ASTContextE+0x28)[0x55c535b53488]
build.release/bin/clang(_ZN5clang8ParseASTERNS_4SemaEbb+0x350)[0x55c5362c5d30]
build.release/bin/clang(_ZN5clang14FrontendAction7ExecuteEv+0x11e)[0x55c535b2958e]
build.release/bin/clang(_ZN5clang16CompilerInstance13ExecuteActionERNS_14FrontendActionE+0x176)[0x55c535af54d6]
build.release/bin/clang(_ZN5clang25ExecuteCompilerInvocationEPNS_16CompilerInstanceE+0x981)[0x55c535bc2561]
build.release/bin/clang(_Z8cc1_mainN4llvm8ArrayRefIPKcEES2_Pv+0xc50)[0x55c533e1a890]
build.release/bin/clang(main+0x1295)[0x55c533d9b585]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7f4aed7202b1]
build.release/bin/clang(_start+0x2a)[0x55c533e1684a]
Stack dump:
0. Program arguments: build.release/bin/clang -cc1 -triple
i386-pc-windows-msvc19.11.0 -emit-pch -fms-extensions
-fms-compatibility -fms-compatibility-version=19.11 -std=c++14
-fdelayed-template-parsing -x c++-header /tmp/a.cc -o /dev/null
1. <eof> parser at end of file
Aborted
On Mon, Feb 19, 2018 at 10:18 PM, Ammarguellat, Zahira
<zahira.ammarguellat at intel.com> wrote:
> Hans,
>
>
>
> The reproducer below generates wrong export symbols compared to MSVC but I
> am not sure it is related to my change.
>
> Compiling this with MSVC generates these symbols:
>
>
>
> ksh-3.2$ dumpbin /directives t3.obj | grep EXPORT
>
> /EXPORT:??4?$d at H@@QEAAAEAV0 at AEBV0@@Z
>
> /EXPORT:??4?$d at H@@QEAAAEAV0@$$QEAV0@@Z
>
> /EXPORT:??4?$h at H@f@@QEAAAEAV01 at AEBV01@@Z
>
> /EXPORT:??4?$h at H@f@@QEAAAEAV01@$$QEAV01@@Z
>
> /EXPORT:??4i@@QEAAAEAV0 at AEBV0@@Z
>
> /EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z
>
>
>
>
>
> Compiling with clang (my patches no included) generate these symbols:
>
> ksh-3.2$ dumpbin /directives t3.o | grep EXPORT
>
> /EXPORT:??4?$d at H@@QEAAAEAV0 at AEBV0@@Z
>
> /EXPORT:??4?$d at H@@QEAAAEAV0@$$QEAV0@@Z
>
> /EXPORT:??4?$h at H@f@@QEAAAEAV01 at AEBV01@@Z
>
> /EXPORT:??4?$h at H@f@@QEAAAEAV01@$$QEAV01@@Z
>
> /EXPORT:??4i@@QEAAAEAV0 at AEBV0@@Z
>
> /EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z
>
> /EXPORT:?e@?$d at H@@0W4b at a@@B,DATA รง This is not generated in MSVC.
>
>
>
> Although this is a bug that needs to be fixes, it is in my opinion
> un-related to the patches I have proposed.
>
>
>
> Your thoughts?
>
>
>
> Thanks.
>
> -Zahira
>
>
>
>
>
> -----Original Message-----
> From: hwennborg at google.com [mailto:hwennborg at google.com] On Behalf Of Hans
> Wennborg
> Sent: Monday, February 19, 2018 5:11 AM
> To: cfe-commits <cfe-commits at lists.llvm.org>; Ammarguellat, Zahira
> <zahira.ammarguellat at intel.com>
> Subject: Re: r324991 - Fix for PR32992. Static const classes not exported.
>
>
>
> Reduced repro:
>
>
>
> $ clang -cc1 -triple i386-pc-windows-msvc19.11.0 -emit-pch -fms-extensions
> -fms-compatibility -fms-compatibility-version=19.11
>
> -std=c++14 -fdelayed-template-parsing -x c++-header a.ii -o /dev/null
>
>
>
> a.ii:
>
>
>
> namespace a {
>
> enum b { c };
>
> }
>
> template <typename> class d { static constexpr a::b e = a::c; }; namespace f
> { template <typename g = int> class h : d<g> {}; } using f::h; class
> __declspec(dllexport) i : h<> {};
>
>
>
> On Wed, Feb 14, 2018 at 4:22 PM, Hans Wennborg <hans at chromium.org> wrote:
>
>> I ended up having to revert this in r325133 as it broke the Chromium
>
>> build. Please see
>
>> https://bugs.chromium.org/p/chromium/issues/detail?id=812231#c1 for a
>
>> reproducer.
>
>>
>
>> On Tue, Feb 13, 2018 at 10:19 AM, Hans Wennborg via cfe-commits
>
>> <cfe-commits at lists.llvm.org> wrote:
>
>>> Author: hans
>
>>> Date: Tue Feb 13 01:19:43 2018
>
>>> New Revision: 324991
>
>>>
>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=324991&view=rev
>
>>> Log:
>
>>> Fix for PR32992. Static const classes not exported.
>
>>>
>
>>> Patch by zahiraam!
>
>>>
>
>>> Differential Revision: https://reviews.llvm.org/D42968
>
>>>
>
>>> Modified:
>
>>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>
>>> cfe/trunk/test/CodeGenCXX/dllexport.cpp
>
>>>
>
>>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>
>>> URL:
>
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cp
>
>>> p?rev=324991&r1=324990&r2=324991&view=diff
>
>>> =====================================================================
>
>>> =========
>
>>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>
>>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Feb 13 01:19:43 2018
>
>>> @@ -5476,7 +5476,7 @@ static void CheckAbstractClassUsage(Abst
>
>>> }
>
>>> }
>
>>>
>
>>> -static void ReferenceDllExportedMethods(Sema &S, CXXRecordDecl
>
>>> *Class) {
>
>>> +static void ReferenceDllExportedMembers(Sema &S, CXXRecordDecl
>
>>> +*Class) {
>
>>> Attr *ClassAttr = getDLLAttr(Class);
>
>>> if (!ClassAttr)
>
>>> return;
>
>>> @@ -5491,6 +5491,16 @@ static void ReferenceDllExportedMethods(
>
>>> return;
>
>>>
>
>>> for (Decl *Member : Class->decls()) {
>
>>> + // Defined static variables that are members of an exported base
>
>>> + // class must be marked export too. Push them to implicit
>>> instantiation
>
>>> + // queue.
>
>>> + auto *VD = dyn_cast<VarDecl>(Member);
>
>>> + if (VD && Member->getAttr<DLLExportAttr>() &&
>
>>> + VD->getStorageClass() == SC_Static &&
>
>>> + TSK == TSK_ImplicitInstantiation)
>
>>> + S.PendingLocalImplicitInstantiations.push_back(
>
>>> + std::make_pair(VD, VD->getLocation()));
>
>>> +
>
>>> auto *MD = dyn_cast<CXXMethodDecl>(Member);
>
>>> if (!MD)
>
>>> continue;
>
>>> @@ -10902,12 +10912,12 @@ void Sema::ActOnFinishCXXNonNestedClass(
>
>>>
>
>>> void Sema::referenceDLLExportedClassMethods() {
>
>>> if (!DelayedDllExportClasses.empty()) {
>
>>> - // Calling ReferenceDllExportedMethods might cause the current
>>> function to
>
>>> + // Calling ReferenceDllExportedMembers might cause the current
>
>>> + function to
>
>>> // be called again, so use a local copy of DelayedDllExportClasses.
>
>>> SmallVector<CXXRecordDecl *, 4> WorkList;
>
>>> std::swap(DelayedDllExportClasses, WorkList);
>
>>> for (CXXRecordDecl *Class : WorkList)
>
>>> - ReferenceDllExportedMethods(*this, Class);
>
>>> + ReferenceDllExportedMembers(*this, Class);
>
>>> }
>
>>> }
>
>>>
>
>>>
>
>>> Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
>
>>> URL:
>
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexpo
>
>>> rt.cpp?rev=324991&r1=324990&r2=324991&view=diff
>
>>> =====================================================================
>
>>> =========
>
>>> --- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
>
>>> +++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Tue Feb 13 01:19:43 2018
>
>>> @@ -28,6 +28,7 @@ struct External { int v; };
>
>>>
>
>>> // The vftable for struct W is comdat largest because we have RTTI.
>
>>> // M32-DAG: $"\01??_7W@@6B@" = comdat largest
>
>>> +// M32-DAG: $"\01?smember@?$Base at H@PR32992@@0HA" = comdat any
>
>>>
>
>>>
>
>>>
>
>>> //===----------------------------------------------------------------
>
>>> ------===// @@ -977,3 +978,21 @@ class __declspec(dllexport)
>
>>> ACE_Service_ // MSVC2015-DAG: define weak_odr dllexport
>
>>> {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
>
>>> // The declarations should not be exported.
>
>>> // MSVC2013-NOT: define weak_odr dllexport
>
>>> {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
>
>>> +
>
>>> +namespace PR32992 {
>
>>> +// Static data members of a instantiated base class should be exported.
>
>>> +template <class T>
>
>>> +class Base {
>
>>> + virtual void myfunc() {}
>
>>> + static int smember;
>
>>> +};
>
>>> +// MS-DAG: @"\01?smember@?$Base at H@PR32992@@0HA" = weak_odr
>
>>> +dllexport global i32 77, comdat, align 4 template <class T> int
>
>>> +Base<T>::smember = 77; template <class T> class
>
>>> +__declspec(dllexport) Derived2 : Base<T> {
>
>>> + void myfunc() {}
>
>>> +};
>
>>> +class Derived : public Derived2<int> {
>
>>> + void myfunc() {}
>
>>> +};
>
>>> +} // namespace PR32992
>
>>>
>
>>>
>
>>> _______________________________________________
>
>>> cfe-commits mailing list
>
>>> cfe-commits at lists.llvm.org
>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list