-
Notifications
You must be signed in to change notification settings - Fork 18k
misc/cgo/test: 'signal arrived during external code execution' with updated windows/386 builder #53540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Cherry helped me with some debugging and was able to reduce the test case down to this:
By comparing external linking (works) with internal linking (crashes) she found that the C function printf generated from the C fragment above contains an indirect call to the data symbol "__imp____acrt_iob_func". Here's the code:
Note the call at _printf+0x2a: it loads the data symbol and then branches to the loaded value. However this data symbol is being renamed to "__acrt_iob_func" (a text symbol) as a result of CL 382837. Prior to CL 382837 the host object loader would always rename "__imp_XXX" to "XXX" except for the specific case of "__imp____acrt_iob_func"; after 382837 we always do the renaming except for when there is a local def in the same PE file. Because of the bad rename, the code loads up data from the function itself and then tries to branch to it (crash). I tried restoring the special case for "__imp____acrt_iob_func" that was present before CL 382837. With that change in place, there is no longer a crash at that spot, but a new crash with a very similar failure mode:
I inspected the resulting executable. The call to "0x4d8208" is happening from the mingw function
I tracked down the object file that this is coming from in libmingwex.a, and dumped out the function there with relocations:
I don't have a good idea as to what went wrong here. The relocation is to "__errno", but it looks as though we already have a SDYNIMPORT symbol for "_errno" set up... and I don't see any other bogus _errno definitions. The address in question (0x4d8208) is somewhere in the .data section apparently:
As to why the relocation targeting _errno is being resolved to some place in the data section, I am really not sure (scratching my head over that one). |
I looked at what the external linker is doing for the
As far as I know there isn't any code in the Go linker that creates these sorts of import thunks for x86. Hmm. |
It looks similar to generating PLT stubs for ELF, which the Go linker can do. Maybe we could do something similar? |
Agree. I am looking over the corresponding code in lld, e.g. https://github.com/llvm/llvm-project/blob/83d59e05b201760e3f364ff6316301d347cbad95/lld/COFF/InputFiles.cpp#L974 to see if I can use that as a model as well. |
Change https://go.dev/cl/417554 mentions this issue: |
Change https://go.dev/cl/417556 mentions this issue: |
Change https://go.dev/cl/451738 mentions this issue: |
Change https://go.dev/cl/451816 mentions this issue: |
Adapt the testcshared tests to handle the case where the path output by invoking gcc -print-prog-name=dlltool is a path lacking the final ".exe" suffix (this seems to be what clang is doing); tack it on before using if this is the case. Updates #35006. Updates #53540. Change-Id: I04fb7b9fc90677880b1ced4a4ad2a8867a3f5f86 Reviewed-on: https://go-review.googlesource.com/c/go/+/451816 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Bryan Mills <[email protected]> Run-TryBot: Than McIntosh <[email protected]> Reviewed-by: Cherry Mui <[email protected]>
This patch reworks the handling of DLL import symbols in the PE host object loader to ensure that the Go linker can deal with them properly during internal linking. Prior to this point the strategy was to immediately treat an import symbol reference of the form "__imp__XXX" as if it were a reference to the corresponding DYNIMPORT symbol XXX, except for certain special cases. This worked for the most part, but ran into problems in situations where the target ("XXX") wasn't a previously created DYNIMPORT symbol (and when these problems happened, the root cause was not always easy to see). The new strategy is to not do any renaming or forwarding immediately, but to delay handling until host object loading is complete. At that point we make a scan through the newly introduced text+data sections looking at the relocations that target import symbols, forwarding the references to the corresponding DYNIMPORT sym where appropriate and where there are direct refs to the DYNIMPORT syms, tagging them for stub generation later on. Updates #35006. Updates #53540. Change-Id: I2d42b39141ae150a9f82ecc334001749ae8a3b4a Reviewed-on: https://go-review.googlesource.com/c/go/+/451738 Reviewed-by: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: David Chase <[email protected]> Run-TryBot: Than McIntosh <[email protected]>
Change https://go.dev/cl/452675 mentions this issue: |
I accidentally reverted its edits with a bad cherry-pick in CL 452457. This should re-fix the windows-.*-newcc builders that regressed at that change. Updates #47257. Updates #35006. Updates #53540. Change-Id: I5818416af7c4c8c1593c36aa0198331b42b6c7d7 Reviewed-on: https://go-review.googlesource.com/c/go/+/452675 Run-TryBot: Bryan Mills <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Auto-Submit: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
Closing out this bug; CL 451738 takes care of it. |
What version of Go are you using (
go version
)?main branch tip
go version go1.19-pre4 cl/455575533 +12f49fe0ed
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?windows/386, specifically the new builder with update windows image and compilers, for example
windows-386-2012-newcc
.What did you do?
ran the misc/cgo/test as part of all.bat
What did you expect to see?
clean run
What did you see instead?
Representative slowbot failure:
https://storage.googleapis.com/go-build-log/6a2e5140/windows-386-2012-newcc_2be6fdb8.log
The test fails with this failure mode:
test run output with stack traces
I am not sure what the root cause is here, needs to be debugged.
The text was updated successfully, but these errors were encountered: