Skip to content

Commit

Permalink
Fix win abi bug causing stack dealignment/crash
Browse files Browse the repository at this point in the history
The accounting for push_struct was wrong (double counted by both |stack|
and |by_ref_copies_size|) and the push of r11 wasn't being counted.
Sometimes they cancelled each other out, but only accidentally (because
the stack is always either 8 or 16 aligned, it's not too hard to get
accidentally right.)
  • Loading branch information
sgraham committed Jan 25, 2024
1 parent 068590a commit fdae45c
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/codegen.in.c
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,9 @@ static int push_args_win(Node* node, int* by_ref_copies_size) {
assert((*by_ref_copies_size == 0 && !has_by_ref_args) ||
(*by_ref_copies_size && has_by_ref_args));

if ((C(depth) + stack + (*by_ref_copies_size / 8)) % 2 == 1) {
// Realign the stack to 16 bytes if rsp fiddling mucked it up.
size_t r11_push_size = has_by_ref_args ? 1 : 0;
if ((C(depth) + stack + r11_push_size) % 2 == 1) {
///| sub rsp, 8
C(depth)++;
stack++;
Expand Down
44 changes: 44 additions & 0 deletions test/substructs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#include <stdio.h>

struct B {
int x;
int y;
int z;
};

struct C {
int x;
int y;
int z;
int w;
};

struct D {
int x;
int y;
int z;
int w;
int v;
};

void f(struct B b) {
printf("%d %d %d\n", b.x, b.y, b.z);
}

void g(struct C c) {
printf("%d %d %d %d\n", c.x, c.y, c.z, c.w);
}

void h(struct D d) {
printf("%d %d %d %d %d\n", d.x, d.y, d.z, d.w, d.v);
}

int main(void) {
struct B b = {10, 20, 30};
f(b);
struct C c = {10, 20, 30, 40};
g(c);
struct D d = {10, 20, 30, 40, 50};
h(d);
return 0;
}
1 change: 1 addition & 0 deletions test/test_helpers_for_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
.get_function_address = get_host_helper_func,
.output_function = NULL,
.use_ansi_codes = false,
.generate_debug_symbols = false,
};
DyibiccContext* ctx = dyibicc_set_environment(&env_data);
Expand Down
52 changes: 52 additions & 0 deletions test/update_structabi2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
from test_helpers_for_update import *

HOST = '''\
#include <stdio.h>
struct B {
int x;
int y;
int z;
};
void test_f(struct B b) {
printf("%d %d %d\\n", b.x, b.y, b.z);
}
'''

SRC = '''\
#include <stdio.h>
struct B {
int x;
int y;
int z;
};
extern void test_f(struct B b);
// This was causing stack un-alignment due to how struct copying was
// implemented in codegen.
int main(void) {
struct B b = {10, 20, 30};
test_f(b);
printf("OK\\n");
return 0;
}
'''

# This is not really an "update" test, but because the update tests happen to
# build a separate host binary rather than using the standard dyibicc.exe, it
# gives us a way to have C code to call that's test-specific and compiled by
# the host compiler, rather than by ours.

add_to_host(HOST);
add_host_helper_func("test_f")
include_path("../../test")

initial({'main.c': SRC})
update_ok()
expect(0)

done()

0 comments on commit fdae45c

Please sign in to comment.