If you simply build and run the example from the original mingw-w64 comdat ABI compatibility bug (https://github.com/Alexpux/MINGW-packages/issues/1677#issuecomment-394906508, also below) for i686, it shows we don't use the right section names: $ cat a.cpp inline int foo(int a, int b) { return a + b; } int bar(void); int main() { return foo(1, 2) + bar(); } $ cat b.cpp inline int foo(int a, int b) { return a + b; } int bar() { return foo(3, 4); } $ gcc -m32 a.cpp -c -o a.o && clang --target=i686-w64-windows-gnu -c b.cpp -o b.o $ dumpbin a.o | grep text 34 .text 10 .text$_Z3fooii $ dumpbin b.o | grep text 1F .text 1F .text$__Z3fooii Clang adds the extra '_' for Windows C symbol mangling, but GCC does not. A user emailed me directly to report that they were getting linker errors, but I have not been able to observe any problems caused by this mismatch.
Created attachment 21109 [details] Potential fix The attached patch seems to fix this issue. However, from a code standpoint, I'm not sure it is up to standards. I am submitting this to see if it is directionally correct, and seeking feedback for a better way to accomplish this. Questions I have: 1. Are the modifications to the test correct? The code seems to function (it passes `check-llvm`, `check-clang`, and manual testing of the example with GCC). I am not very familiar with comdats or COFF, and therefore I am not 100% confident in the test case. 2. Is GO->getComdat()->getName() the correct place to get the name, or is it a coincidence that it contains the right data? Any feedback is welcome, and I am happy to submit another patch incorporating any suggested changes.
Please consider for 7.0.1 release. This prevents using Clang 7 on i686 Windows NT. Thank you.
(In reply to Andrew Yohn from comment #2) > Please consider for 7.0.1 release. This prevents using Clang 7 on i686 > Windows NT. Thank you. Sorry, that should have read, "This prevents using Clang 7 for i686 Windows NT with GCC/mingw-w64."
Has this patch been submitted to phabricator?
(In reply to Tom Stellard from comment #4) > Has this patch been submitted to phabricator? Sorry, I'm new to this process. I've created a review now. https://reviews.llvm.org/D54762
Tom, feel free to merge r347431 to 7.0.1, although you might want to let it bake in tree for a few days.
Added bug 39781 to track the merge.