i686 mingw-w64 comdat functions use wrong .text section name

[rnk]

Bugzilla Link	39218
Resolution	FIXED
Resolved on	Nov 29, 2018 16:09
Version	trunk
OS	Windows NT
Blocks	#38454
CC	@tstellar
Fixed by commit(s)	r347431 r347931

Extended Description

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.

[llvmbot]

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.

[llvmbot]

Please consider for 7.0.1 release. This prevents using Clang 7 on i686 Windows NT. Thank you.

[llvmbot]

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."

[tstellar]

Has this patch been submitted to phabricator?

[llvmbot]

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

[rnk]

Tom, feel free to merge r347431 to 7.0.1, although you might want to let it bake in tree for a few days.

[llvmbot]

Added bug 39781 to track the merge.