History

fix a subtle x86-64 calling bug

I ran into an issue playing with tinycc, and tracked it down to a rather weird assumption in the function calling code. This breaks only when varargs and float/double arguments are combined, I think, and only when calling GCC-generated (or non-TinyCC, at least) code. The problem is we sometimes generate code like this: 804a468: 4c 89 d9 mov %r11,%rcx 804a46b: b8 01 00 00 00 mov $0x1,%eax 804a470: 48 8b 45 c0 mov -0x40(%rbp),%rax 804a474: 4c 8b 18 mov (%rax),%r11 804a477: 41 ff d3 callq *%r11 for a function call. Note how $eax is first set to the correct value, then clobbered when we try to load the function pointer into R11. With the patch, the code generated is: 804a468: 4c 89 d9 mov %r11,%rcx 804a46b: b8 01 00 00 00 mov $0x1,%eax 804a470: 4c 8b 5d c0 mov -0x40(%rbp),%r11 804a474: 4d 8b 1b mov (%r11),%r11 804a477: 41 ff d3 callq *%r11 which is correct. This becomes an issue when get_reg(RC_INT) is modified not always to return %rax after a save_regs(0), because then another register (%ecx, say) is clobbered, and the function passed an invalid argument. A rather convoluted test case that generates the above code is included. Please note that the test will not cause a failure because TinyCC code ignores the %rax argument, but it will cause incorrect behavior when combined with GCC code, which might wrongly fail to save XMM registers and cause data corruption.

Philip committed 11 years ago

Commit 059aea51 parent aacf65b2 files · +38 −1Browse files

Unified · Split

2 files
  1. tests/abitest.c +37 −0
  2. x86_64-gen.c +1 −1
tests/abitest.c+37−0
@@ -315,6 +315,42 @@ static int many_struct_test_2(void) { · 314 unmodified lines
315 return run_callback(src, many_struct_test_2_callback);315 return run_callback(src, many_struct_test_2_callback);
316}316}
317 317
318/*
319 * Win64 calling convention test.
320 */
321
322typedef struct many_struct_test_3_type_s {int a, b;} many_struct_test_3_type;
323typedef many_struct_test_3_type (*many_struct_test_3_function_type) (many_struct_test_3_type,many_struct_test_3_type,many_struct_test_3_type,many_struct_test_3_type,many_struct_test_3_type,many_struct_test_3_type, ...);
324typedef struct many_struct_test_3_struct_type { many_struct_test_3_function_type f; many_struct_test_3_function_type *f2; } many_struct_test_3_struct_type;
325
326static void many_struct_test_3_dummy(double d, ...)
327{
328 volatile double x = d;
329}
330
331static int many_struct_test_3_callback(void *ptr) {
332 many_struct_test_3_struct_type s = { ptr, };
333 many_struct_test_3_struct_type *s2 = &s;
334 s2->f2 = &s2->f;
335 many_struct_test_3_dummy(1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, &s2);
336 many_struct_test_3_function_type f = *(s2->f2);
337 many_struct_test_3_type v = {1,2};
338 many_struct_test_3_type r = (*((s2->f2=&f)+0))(v,v,v,v,v,v,1.0);
339 return ((r.a == 6) && (r.b == 12))?0:-1;
340}
341
342static int many_struct_test_3(void) {
343 const char *src =
344 "typedef struct many_struct_test_3_type_s {int a, b;} many_struct_test_3_type;\n"
345 "many_struct_test_3_type f(many_struct_test_3_type x1, many_struct_test_3_type x2, many_struct_test_3_type x3, many_struct_test_3_type x4, many_struct_test_3_type x5, many_struct_test_3_type x6, ...) {\n"
346 " many_struct_test_3_type y;\n"
347 " y.a = x1.a + x2.a + x3.a + x4.a + x5.a + x6.a;\n"
348 " y.b = x1.b + x2.b + x3.b + x4.b + x5.b + x6.b;\n"
349 " return y;\n"
350 "}\n";
351 return run_callback(src, many_struct_test_3_callback);
352}
353
318/*354/*
319 * stdarg_test: Test variable argument list ABI355 * stdarg_test: Test variable argument list ABI
320 */356 */
@@ -448,6 +484,7 @@ int main(int argc, char **argv) { · 127 unmodified lines
448 RUN_TEST(two_member_union_test);484 RUN_TEST(two_member_union_test);
449 RUN_TEST(many_struct_test);485 RUN_TEST(many_struct_test);
450 RUN_TEST(many_struct_test_2);486 RUN_TEST(many_struct_test_2);
487 RUN_TEST(many_struct_test_3);
451 RUN_TEST(stdarg_test);488 RUN_TEST(stdarg_test);
452 RUN_TEST(stdarg_struct_test);489 RUN_TEST(stdarg_struct_test);
453 RUN_TEST(arg_align_test);490 RUN_TEST(arg_align_test);
x86_64-gen.c+1−1
@@ -395,7 +395,7 @@ void load(int r, SValue *sv) · 394 unmodified lines
395 v1.r = VT_LOCAL | VT_LVAL;395 v1.r = VT_LOCAL | VT_LVAL;
396 v1.c.ul = fc;396 v1.c.ul = fc;
397 fr = r;397 fr = r;
398 if (!(reg_classes[fr] & RC_INT))398 if (!(reg_classes[fr] & (RC_INT|RC_R11)))
399 fr = get_reg(RC_INT);399 fr = get_reg(RC_INT);
400 load(fr, &v1);400 load(fr, &v1);
401 }401 }

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t