[Bug tdep/25096] amd64_push_arguments places values in wrong register

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25096

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d10eccaa723e31689cdeadaea6bebabe2f63de34

commit d10eccaa723e31689cdeadaea6bebabe2f63de34
Author: Tom de Vries <tdevries@suse.de>
Date:   Wed Oct 16 17:11:56 2019 +0200

    [gdb/tdep] Fix inferior call arg passing for amd64

    We currently have 12 KFAILS in gdb.base/infcall-nested-structs.exp for
    PR tdep/25096.

    A minimal version of the failure looks like this.  Consider test.c:
    ...
    struct s { int c; struct { int a; float b; } s1; };
    struct s ref = { 0, { 'a', 'b' } };

    int __attribute__((noinline,noclone)) check (struct s arg)
    { return arg.s1.a == 'a' && arg.s1.b == 'b' && arg.c == 0; }

    int main (void)
    { return check (ref); }
    ...

    When calling 'check (ref)' from main, we have '1' as expected:
    ...
    $ g++ test.c -g ; ./a.out ; echo $?
    1
    ...

    But when calling 'check (ref)' from the gdb prompt, we get '0':
    ...
    $ gdb a.out -batch -ex start -ex "p check (ref)"
    Temporary breakpoint 1 at 0x400518: file test.c, line 8.

    Temporary breakpoint 1, main () at test.c:8
    8       { return check (ref); }
    $1 = 0
    ...

    The layout of struct s is this:
    - the field c occupies 4 bytes at offset 0,
    - the s1.a field occupies 4 bytes at offset 4, and
    - the s1.b field occupies 4 bytes at offset 8.

    When compiling at -O2, we can see from the disassembly of main:
    ...
      4003f0:       48 8b 3d 31 0c 20 00    mov    0x200c31(%rip),%rdi \
                                                   # 601028 <ref>
      4003f7:       f3 0f 10 05 31 0c 20    movss  0x200c31(%rip),%xmm0 \
                                                   # 601030 <ref+0x8>
      4003fe:       00
      4003ff:       e9 ec 00 00 00          jmpq   4004f0 <_Z5check1s>
    ...
    that check is called with fields c and s1.a passed in %rdi, and s1.b passed
    in %xmm0.

    However, the classification in theclass (a variable representing the first
and
    second eightbytes, to put it in SYSV X86_64 psABI terms) in
    amd64_push_arguments is incorrect:
    ...
    (gdb) p theclass
    $1 = {AMD64_INTEGER, AMD64_INTEGER}
    ...
    and therefore the struct is passed using %rdi and %rsi instead of using
%rdi
    and %xmm0, which explains the failure.

    The reason that we're misclassifying the argument in
amd64_classify_aggregate
    has to do with how nested struct are handled.

    Rather than using fields c and s1.a for the first eightbyte, and using
field
    s1.b for the second eightbyte, instead field c is used for the first
    eightbyte, and fields s1.a and s1.b are classified together in an
intermediate
    eightbyte, which is then used to merge with both the first and second
    eightbyte.

    Fix this by factoring out a new function amd64_classify_aggregate_field,
and
    letting it recursively handle fields of nested structs.

    Tested on x86_64-linux.

    Tested with g++ 4.8.5, 7.4.1, 8.3.1, 9.2.1.

    Tested with clang++ 5.0.2 (which requires removing
additional_flags=-Wno-psabi
    and adding additional_flags=-Wno-deprecated).

    gdb/ChangeLog:

    2019-10-16  Tom de Vries  <tdevries@suse.de>

        PR tdep/25096
        * amd64-tdep.c (amd64_classify_aggregate_field): Factor out of ...
        (amd64_classify_aggregate): ... here.
        (amd64_classify_aggregate_field): Handled fiels of nested structs
        recursively.

    gdb/testsuite/ChangeLog:

    2019-10-16  Tom de Vries  <tdevries@suse.de>

        PR tdep/25096
        * gdb.base/infcall-nested-structs.exp: Remove PR25096 KFAILs.

    Change-Id: Id55c74755f0a431ce31223acc86865718ae0c123

-- 
You are receiving this mail because:
You are on the CC list for the bug.