-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
debug/pe: TestDefaultLinkerDWARF and TestExternalLinkerDWARF fail on windows/arm64 builder #46406
Comments
It appears the problem here is decoding Dwarf section. I am not familiar with the code that decodes Dwarf. I also don't have windows-arm64 hardware so I cannot even try to reproduce it here. I also don't use windows-arm64 myself. Perhaps @zx2c4 will be interested investigating this.
This test uses external linker, not internal linker. And Windows does not come with C compiler and linker installed. So whatever C tools you installed on that builder could be relevant to this bug. I hope it helps. Alex |
I poked at this a bit. I changed the error in question to a panic; here's the stack trace at the point of the error:
What this says is that things are going bad at a very early stage in the process of reading the DWARF. The reader is counting the number of compilation units in the .debug_info section; it does this by reading the unit length, then skipping over to the next unit using that value. This suggests some sort of very basic problem in the DWARF generation, like getting the unit lengths wrong, or incorrectly adding padding between the units. |
I did a bit more hand testing with the gomote. I think this is mainly a problem with external linking. When I force internal linkage for my small testcase, the DWARF looks ok (at least it is not hitting the problem in this issue). |
Is R_DWARFSECREF handled in the right way in pereloc1 at https://tip.golang.org/src/cmd/link/internal/arm64/asm.go#L633 ? |
Thanks -- I'll give that a try. I note that when I compile this program:
I get:
which also seems to point the finger at R_DWARFSECREF. |
I tried hacking arm64/asm.go to treat R_DWARFSECREF the same as R_ADDR, e.g.
but this did not seem to do the trick (same error from the debug/pe test). For the errors I posted in the previous comment, I think they are coming from which suggests some sort of problem with gensymslate / label symbols (e.g. to get around the 21 bit limit). More investigation needed. Not sure if the two problems are related though. |
What cgo toolchain is the builder using? Honestly the simplest thing to do is probably to make sure the builder uses 20201020. I am trying to confirm that that's the version I used but I am having trouble getting the tablet to turn on. (!) |
From above, we know the test is passing with 20201020, but failing with the newer 20210423. It should work with a newer stable llvm-mingw release, so this will need investigation anyway, it can just happen later. We can add a skip for the test and keep the builder as is, both to avoid the relatively slow process of rebuilding the image and to have the existing builder with latest stable llvm-mingw available for testing. (Thanks @heschi for the suggestion.) |
Change https://golang.org/cl/323990 mentions this issue: |
@mstorsjo - seems like there might be a regression in your latest toolchain. Interested in taking a look? |
Always interested in resolving regressions, but I'm not really familiar with what I'm looking at and I'm (in a 2 minute readthrough) seeing various issues being discussed here. Given llvm-mingw as is, how do I minimally reproduce the issue at hand? (Is it a new build time warning that wasn't there before?) I'd love to bisect down the upstream llvm change that caused it. |
@mstorsjo If you're able to build Go tip from source for windows/arm64, it should be possible to reproduce this with |
I don't think we have conclusive evidence at all that it's a regression in LLVM-Mingw, only an incompatibility between Go and LLVM-Mingw with those particular versions. It could just as easily be a problem on Go's side, although our DWARF parser does handle regular LLVM and GCC just fine. |
A smaller standalone repro would be ideal yeah - but if someone can provide instructions for how to build go from source with llvm-mingw I could try the full case, at least for pinpointing where the regression came from. Can I cross build Go from unix, transfer the build to a windows/arm64 machine and then use that to run tests, or do I need to build it natively (and how do I do that)? |
Go programs can be build on any OS for any OS. So you can build windows-arm64 binaries on UNIX (including test binaries). Unfortunately this test uses mix of Go and C - called Cgo. So you also need to have your C compiler on UNIX to be able to cross-compile to Windows. And you would need to run test on Windows. And that test executes GCC on Windows. I would say it is easier to install Go from source and C compiler on Windows. And then you could run This is https://golang.org/doc/install/source how to install Go from source. All Go tools are written in Go. So before getting Go source, you should download another version of Go with already pre-built binaries. Most users would use this section https://golang.org/doc/install/source#bootstrapFromSource Unfortunately this will not work for you because windows-arm64 is very new and pre-built binaries are not available for download. So you should use a UNIX system with any version of Go installed, and build windows-arm64 toolchain https://golang.org/doc/install/source#bootstrapFromCrosscompiledSource and then copy the files onto Windows and use that toolchain to built Go from source there. I hope it helps. Alex |
@mstorsjo Apologies for you getting dragged in here. I think you should wait until we have a clearer repro case to hand you. Chances are decent still that the problem is on our side. |
Ok, no problem! Holding off of it for now. |
I have awakened my Surface Pro X from its slumber, but despite valiant efforts I have not succeeded in convincing it that today is not April 26, 2021. It insists that all the other computers on the internet serving certificates from the future must be the ones who are insecurely configured. After a reboot, even less works on the machine, not even trying to use the start menu and certainly not the "Adjust date/time" menu entry. I assume it is doing background network connections all of which are failing during TLS. Perhaps if I let it sit here for a few hours it will reconsider. A remarkably secure operating system. |
I spent a bit more time debugging. I think I have a fix for part of the problem-- the DWARF sections aren't being given the correct alignment. I'll send a CL shortly (will also re-enable the test). The other problem (mentioned above in #46406 (comment)) is still there however so I don't think we are completely out of the woods on DWARF. |
Change https://golang.org/cl/324763 mentions this issue: |
Set the correct section flags to insure that .debug_* sections are using 1-byte alignment instead of the default. This seems to be important for later versions of LLVM-mingw on windows (shows up on the windows/arm64 builder). Updates #46406. Change-Id: I023d5208374f867552ba68b45011f7990159868f Reviewed-on: https://go-review.googlesource.com/c/go/+/324763 Trust: Than McIntosh <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Run-TryBot: Than McIntosh <[email protected]> TryBot-Result: Go Bot <[email protected]>
I've submitted https://golang.org/cl/324763 and manually verified that the test runs correctly on the windows-arm64-aws builder. @dmitshur , ok to close this issue? |
Yes, thank you for fixing this problem and verifying @thanm. Let's close the tracking issue. I'll also close CL 323990 (test skip) since it's no longer necessary to skip the test. |
Change https://golang.org/cl/345129 mentions this issue: |
By now, all outstanding issues preventing release tests for windows-arm64 from being turned on by default are resolved: - Issues golang/go#46406 and golang/go#46502 are fixed. - The other remaining blocker golang/go#47017 is determined to be a builder-specific issue (not reproducible on physical Windows ARM64 hardware) that by now happens infrequently enough that automated retries should be able to cover for future occurrences. Issue golang/go#47965 is opened to track progress on this. - The builder performance is good and allows release tests to complete quickly, not adding any bottlenecks to the release process. If something changes, we can revisit this, but information available so far suggests it's a good time to start running release tests for windows/arm64 by default. Fixes golang/go#47017. Updates golang/go#47965. Change-Id: Ie96164821a2f8e795a22ac2d36b7292587a3e117 Reviewed-on: https://go-review.googlesource.com/c/build/+/345129 Reviewed-by: Alexander Rakoczy <[email protected]> Trust: Dmitri Shuralyov <[email protected]>
Spotted on the new
windows-arm64-aws
builder on a recent Go tip commit a62c087:(Source.)
That builder has an open known issue (#42604) so there's a possibility of problem with the builder, but the test failure looks like it may be a problem in
debug/pe
on windows/arm64 specifically. It may be relevant that testing of internal linking on windows/arm64 was intentionally skipped in CL 312045 (CC @rsc).CC @alexbrainman via owners. Also CC @rsc and @golang/release.
The text was updated successfully, but these errors were encountered: