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

long long int in va_arg is not passed correctly #18

Closed
fxcoudert opened this issue Aug 31, 2020 · 4 comments
Closed

long long int in va_arg is not passed correctly #18

fxcoudert opened this issue Aug 31, 2020 · 4 comments

Comments

@fxcoudert
Copy link
Contributor

$ cat va-arg-8.c 
#include <stdarg.h>
#include <stdio.h>

void
debug(int i1, int i2, int i3, int i4, int i5,
      int i6, int i7, int i8, int i9, ...)
{
  va_list ap;
  int i10;
  long long int ill;

  va_start (ap, i9);

  i10 = va_arg (ap,int);
  ill = va_arg (ap, long long int);
  //printf("%d\n", i10);
  //printf("%llx\n", ill);
  if (i10 != 10)
    __builtin_abort ();
  if (ill != 0x123400005678LL)
    __builtin_abort ();

  va_end (ap);
}

int
main(void)
{
  debug(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0x123400005678LL);
}
$ ./bin/gcc va-arg-8.c && ./a.out
zsh: abort      ./a.out

i10 gets passed correctly, but the long long int argument is passed as 0x567800000000LL instead of 0x123400005678LL.

@iains
Copy link
Owner

iains commented Aug 31, 2020

$ cat va-arg-8.c 
#include <stdarg.h>
#include <stdio.h>

void
debug(int i1, int i2, int i3, int i4, int i5,
      int i6, int i7, int i8, 

^ these will be passed in x0..x7

int i9, ...)

^^^ this will be passed as 4 bytes on the stack (leaving the stack misaligned)

int 10 - should force the stack to be aligned to 8 bytes (since it's unnamed at passing) it should also force the stack slot to be 8 bytes....
(which would mean that we'd expect the literal long long to be in the correct place...)

My guess is that the call is DTRT but the function lowering is failing to force-align the unnamed parts, somehow (but it could be the opposite - this is the messy part of the normal c.f. variadic).

@fxcoudert
Copy link
Contributor Author

I split caller and callee in two files, but compiling one with GCC and the other one with clang fails both ways:

$ clang caller.c function.c && ./a.out
$ ./bin/gcc caller.c function.c && ./a.out  
zsh: abort      ./a.out
$ ./bin/gcc -c function.c 
$ clang caller.c function.o && ./a.out   
zsh: abort      ./a.out
$ clang -c function.c 
$ ./bin/gcc caller.c function.o && ./a.out
zsh: abort      ./a.out

@iains
Copy link
Owner

iains commented Sep 1, 2020

yes, the implementation is broken - failing to pad the packed stack argument when we transition to the 'unnamed regime'. Not sure if it's going to be fixable with the current hacks, might need a proper solution to issue #4

@iains
Copy link
Owner

iains commented Sep 1, 2020

should be fixed by 91df520

NOTE: the real fix is to solve #4 - but it's useful to chase down the corner cases.

@iains iains closed this as completed Sep 1, 2020
iains pushed a commit that referenced this issue Sep 5, 2020
This patch moves the move-immediate splitter after the regular ones so
that it has lower precedence, and updates its constraints.

For
int f3 (void) { return 0x11000000; }
int f3_2 (void) { return 0x12345678; }

we now generate:
* with -O2 -mcpu=cortex-m0 -mpure-code:
f3:
	movs    r0, #136
	lsls    r0, r0, #21
	bx      lr
f3_2:
	movs    r0, #18
	lsls    r0, r0, #8
	adds    r0, r0, #52
	lsls    r0, r0, #8
	adds    r0, r0, #86
	lsls    r0, r0, #8
	adds    r0, r0, #121
	bx      lr

* with -O2 -mcpu=cortex-m23 -mpure-code:
f3:
	movs    r0, #136
	lsls    r0, r0, #21
	bx      lr
f3_2:
	movw    r0, #22136
	movt    r0, 4660
	bx      lr

2020-09-04  Christophe Lyon  <christophe.lyon@linaro.org>

	PR target/96769
	gcc/
	* config/arm/thumb1.md: Move movsi splitter for
	arm_disable_literal_pool after the other movsi splitters.

	gcc/testsuite/
	* gcc.target/arm/pure-code/pr96769.c: New test.
iains pushed a commit that referenced this issue Nov 7, 2020
Enable thumb1_gen_const_int to generate RTL or asm depending on the
context, so that we avoid duplicating code to handle constants in
Thumb-1 with -mpure-code.

Use a template so that the algorithm is effectively shared, and
rely on two classes to handle the actual emission as RTL or asm.

The generated sequence is improved to handle right-shiftable and small
values with less instructions. We now generate:

128:
        movs    r0, r0, #128
264:
        movs    r3, #33
        lsls    r3, #3
510:
        movs    r3, #255
        lsls    r3, #1
512:
        movs    r3, #1
        lsls    r3, #9
764:
        movs    r3, #191
        lsls    r3, #2
65536:
        movs    r3, #1
        lsls    r3, #16
0x123456:
        movs    r3, #18 ;0x12
        lsls    r3, #8
        adds    r3, #52 ;0x34
        lsls    r3, #8
        adds    r3, #86 ;0x56
0x1123456:
        movs    r3, #137 ;0x89
        lsls    r3, #8
        adds    r3, #26 ;0x1a
        lsls    r3, #8
        adds    r3, #43 ;0x2b
        lsls    r3, #1
0x1000010:
        movs    r3, #16
        lsls    r3, #16
        adds    r3, #1
        lsls    r3, #4
0x1000011:
        movs    r3, #1
        lsls    r3, #24
        adds    r3, #17
-8192:
	movs	r3, #1
	lsls	r3, #13
	rsbs	r3, #0

The patch adds a testcase which does not fully exercise
thumb1_gen_const_int, as other existing patterns already catch small
constants.  These parts of thumb1_gen_const_int are used by
arm_thumb1_mi_thunk.

2020-11-02  Christophe Lyon  <christophe.lyon@linaro.org>

	gcc/
	* config/arm/arm.c (thumb1_const_rtl, thumb1_const_print): New
	classes.
	(thumb1_gen_const_int): Rename to ...
	(thumb1_gen_const_int_1): ... New helper function. Add capability
	to emit either RTL or asm, improve generated code.
	(thumb1_gen_const_int_rtl): New function.
	* config/arm/arm-protos.h (thumb1_gen_const_int): Rename to
	thumb1_gen_const_int_rtl.
	* config/arm/thumb1.md: Call thumb1_gen_const_int_rtl instead
	of thumb1_gen_const_int.

	gcc/testsuite/
	* gcc.target/arm/pure-code/no-literal-pool-m0.c: New.
iains pushed a commit that referenced this issue Jun 18, 2021
The fixed error is:

==21166==ERROR: AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs operator delete) on 0x60300000d900
    #0 0x7367d7 in operator delete(void*, unsigned long) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/libsanitizer/asan/asan_new_delete.cpp:172
    #1 0x3b82e6e in pointer_equiv_analyzer::~pointer_equiv_analyzer() /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/gimple-ssa-evrp.c:161
    #2 0x3b83387 in hybrid_folder::~hybrid_folder() /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/gimple-ssa-evrp.c:517
    #3 0x3b83387 in execute_early_vrp /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/gimple-ssa-evrp.c:686
    #4 0x1790611 in execute_one_pass(opt_pass*) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2567
    #5 0x1792003 in execute_pass_list_1 /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2656
    #6 0x1792029 in execute_pass_list_1 /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2657
    #7 0x179209f in execute_pass_list(function*, opt_pass*) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2667
    #8 0x178a5f3 in do_per_function_toporder(void (*)(function*, void*), void*) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:1773
    #9 0x1792fac in do_per_function_toporder(void (*)(function*, void*), void*) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/plugin.h:191
    #10 0x1792fac in execute_ipa_pass_list(opt_pass*) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:3001
    #11 0xc525fc in ipa_passes /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:2154
    #12 0xc525fc in symbol_table::compile() /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:2289
    #13 0xc5a096 in symbol_table::compile() /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:2269
    #14 0xc5a096 in symbol_table::finalize_compilation_unit() /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:2537
    #15 0x1a7a17c in compile_file /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/toplev.c:482
    #16 0x69c758 in do_compile /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/toplev.c:2210
    #17 0x69c758 in toplev::main(int, char**) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/toplev.c:2349
    #18 0x6a932a in main /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/main.c:39
    #19 0x7ffff7820b34 in __libc_start_main ../csu/libc-start.c:332
    #20 0x6aa5fd in _start (/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/objdir/gcc/cc1+0x6aa5fd)

0x60300000d900 is located 0 bytes inside of 32-byte region [0x60300000d900,0x60300000d920)
allocated by thread T0 here:
    #0 0x735ab7 in operator new[](unsigned long) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x3b82dac in pointer_equiv_analyzer::pointer_equiv_analyzer(gimple_ranger*) /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/gimple-ssa-evrp.c:156

gcc/ChangeLog:

	* gimple-ssa-evrp.c (pointer_equiv_analyzer::~pointer_equiv_analyzer): Use delete[].
iains added a commit that referenced this issue May 26, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Jun 3, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Jun 10, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Jun 22, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Jun 25, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Jun 29, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Jul 27, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Jul 27, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Aug 10, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Aug 13, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Aug 15, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Aug 20, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Sep 4, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
iains added a commit that referenced this issue Sep 4, 2023
It also breaks cross compiler builds when the build compiler does
not support the -nodefaultpaths option (etc.)  This is issue #18 on
the arm64 gcc-12 branch.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libcc1/ChangeLog:

	* Makefile.am:
	* Makefile.in:
	* configure: Regenerate.
	* configure.ac:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants