Skip to content
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

coff: fix incorrect default image_base values and re-enable shared library tests on Windows #21767

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

kcbanner
Copy link
Contributor

Closes #16965
Closes #16959
Closes #16960
Closes #18427

As part of investigating a CI failure on aarch64-windows for #21758, I noticed that all the shared library tests were being skipped on either windows or aarch64-windows. I did some testing in an aarch64-windows VM, and noticed that the error happening here was a "Bad Image" error coming from the dll.

The way I figured this out was by removing each of the optional arguments from the linker line to see if I could get a different result. As it turned out, removing the -BASE:268435456 argument for the shared library caused the bug to go away. It turns out this value is incorrect for 64-bit targets, see: https://learn.microsoft.com/en-us/cpp/build/reference/base-base-address?view=msvc-170

Passing the correct value of 0x180000000 resolved the issue, which led me to the diff in this PR.

The regression that introduced this bug (as #18427 hints at) was this change in #18160:

-        if (self.base.options.image_base_override) |image_base| {
-            try argv.append(try std.fmt.allocPrint(arena, "-BASE:{d}", .{image_base}));
+        try argv.append(try std.fmt.allocPrint(arena, "-BASE:{d}", .{self.image_base}));

Instead of sending the optional override, self.image_base (which then contained the incorrect value for a 64-bit images) was now always being set on the linker line.

This didn't seem to cause an issue on x86_64, but it seems aarch64 Windows is more strict about this, and an image created with the wrong BASE would result in this "Bad Image" error there.

This PR fixes the image_base value as well as re-enables the disabled tests.

…library tests on Windows

This was the cause of aarch64-windows shared libraries causing  "bad image" errors
during load-time linking. I also re-enabled the tests that were surfacing this bug.
@andrewrk
Copy link
Member

Nice sleuthing, thank you.

@andrewrk andrewrk merged commit 85d87c9 into ziglang:master Oct 22, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants