Open Quuxplusone opened 7 years ago
Bugzilla Link | PR33285 |
Status | NEW |
Importance | P enhancement |
Reported by | Piotr Padlewski (piotr.padlewski@gmail.com) |
Reported on | 2017-06-02 13:18:13 -0700 |
Last modified on | 2018-10-25 20:11:57 -0700 |
Version | trunk |
Hardware | PC Linux |
CC | dgregor@apple.com, ditaliano@apple.com, llvm-bugs@lists.llvm.org, martin@martin.st, rafael@espindo.la, richard-llvm@metafoo.co.uk, rnk@google.com |
Fixed by commit(s) | |
Attachments | |
Blocks | |
Blocked by | |
See also |
I think declspec(selectany) should be treated as COMDATs.
I tried to reduce this case but as I build clang with assertions, I found the
frontend crashing in mangling. I'll open another bug and take a look at that
issue first.
clang-5.0: ../tools/clang/lib/AST/ItaniumMangle.cpp:4518: void
{anonymous}::CXXNameMangler::addSubstitution(uintptr_t): Assertion
`!Substitutions.count(Ptr) && "Substitution alread$
exists!"' failed.
Are you trying the most recent trunk? There was a patch today solving crashes in
Itanium (mangling of __unaligned), so I hope it is this one, because I had
similar problem before it was submited.
Yep, it is exactly the one I had: https://bugs.llvm.org/show_bug.cgi?id=33178
It was fixed in this patch https://reviews.llvm.org/D33398
(In reply to Piotr Padlewski from comment #3)
> Yep, it is exactly the one I had: https://bugs.llvm.org/show_bug.cgi?id=33178
> It was fixed in this patch https://reviews.llvm.org/D33398
Same here.
I can reproduce with just these two files:
ld.lld -shared
./out/Release/bin/GCStress/CMakeFiles/GCStress.dir/GCStress.cpp.o
./out/Release/bin/GCStress/CMakeFiles/GCStress.dir/RecyclerTestObject.cpp.o
Now, trying to reduce.
If you have:
$ clang++ --version clang version 5.0.0 (trunk 304592) (llvm/trunk 304594)
$ cat 1.cpp __declspec(selectany) int TinkyWinky = false;
$ clang++ 1.cpp -o 1.o -c -fms-extensions 1.cpp:1:12: warning: declspec attribute 'selectany' is not supported [-Wignored-attributes] declspec(selectany) int TinkyWinky = false; ^
So I don't think this is going to work.
(and if you actually look at the IR emitted by clang, the GV is not marked as comdat
http://llvm.org/docs/LangRef.html#comdats, so MC won't emit a COMDAT section and the linker is completely oblivious about the fact it has to discard one of the two as this information is lost in the frontend).
If you have:
$ clang++ --version clang version 5.0.0 (trunk 304592) (llvm/trunk 304594)
$ cat 1.cpp __declspec(selectany) int TinkyWinky = false;
$ clang++ 1.cpp -o 1.o -c -fms-extensions 1.cpp:1:12: warning: declspec attribute 'selectany' is not supported [-Wignored-attributes] declspec(selectany) int TinkyWinky = false; ^
So I don't think this is going to work.
(and if you actually look at the IR emitted by clang, the GV is not marked as comdat
http://llvm.org/docs/LangRef.html#comdats, so MC won't emit a COMDAT section and the linker is completely oblivious about the fact it has to discard one of the two as this information is lost in the frontend).
And if you dump the IR for any of the two files, at -O3, you see:
@_ZN7BVUnitTIjE10ShiftValueE = local_unnamed_addr constant i32 5, align 4
@_ZN7BVUnitTImE10ShiftValueE = local_unnamed_addr constant i32 6, align 4
or similarly, at -O0:
@_ZN7BVUnitTIjE10ShiftValueE = constant i32 5, align 4
@_ZN7BVUnitTImE10ShiftValueE = constant i32 6, align 4
(so, no comdat ;)
Did we eve support selectany?
prazek@0x01:~ $ clang++-3.8 --version
clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
prazek@0x01:~ $clang++-3.8 1.cpp -fms-extensions -c
prazek@0x01:~ $ clang++-3.8 1.cpp -fms-extensions -S -emit-llvm
prazek@0x01:~ $ cat 1.ll
; ModuleID = '1.cpp'
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-linux-gnu"
$TinkyWinky = comdat any
@TinkyWinky = weak_odr global i32 0, comdat, align 4
!llvm.ident = !{!0}
!0 = !{!"clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)"}
This is my suspect:
https://reviews.llvm.org/D32083
We support that, when targeting Windows. In fact, if you pass -target x86_64-pc-
win32 you get
[davide@cupiditate decl]$ ../clang++ 1.cpp -o - -c -fms-extensions -target
x86_64-pc-
win32 -emit-llvm -S |grep Tinky
$"\01?TinkyWinky@@3HA" = comdat any
@"\01?TinkyWinky@@3HA" = weak_odr global i32 0, comdat, align 4
While on Windows:
[davide@cupidatate decl]$ ../clang++ 1.cpp -o - -c -fms-extensions -target
x86_64-
unknown-linux -emit-llvm -S |grep Tinky
1.cpp:1:12: warning: __declspec attribute 'selectany' is not supported [-
Wignored-attributes]
__declspec(selectany) int TinkyWinky = false;
^
1 warning generated.
@TinkyWinky = global i32 0, align 4
I'm not entirely sure why this used to work, it could've been by accident
(originally enabled anywhere, then disabled on ELF targets, e.g.).
Instead of keep trying to guess, I'd bisect in order to understand the first
commit where we don't mark the GV as comdat. My devstation is going to be
fairly busy for the next couple of hours, as I'm building something else, but
I'll start a bisection after that.
Thanks,
--
Davide
I think I fixed that.
Just a heads up - as I saw that the fix got merged after 5.0 was released; if you want to try to get this fix into 5.0.1 you've still got a little time (see http://llvm.org/docs/HowToReleaseLLVM.html#merge-requests for the procedure on how to request it to be merged), but 5.0.1 is not far away.
I tried following the instructions, but it fails with
error: invalid stable version
for command:
utils/release/merge-request.sh -stable-version 5.0.1 -r 313278 -user
piotr.padlewski@gmail.com