Skip to content

Test by-value-self-argument-in-trait-impl.rs fails on aarch64 #49807

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

Closed
bkchr opened this issue Apr 9, 2018 · 14 comments
Closed

Test by-value-self-argument-in-trait-impl.rs fails on aarch64 #49807

bkchr opened this issue Apr 9, 2018 · 14 comments
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bkchr
Copy link
Contributor

bkchr commented Apr 9, 2018

Hi,
the by-value-self-argument-in-trait-impl.rs test fails for NixOS when compiling for aarch64 with the following output:

error: line not found in debugger output: $3 = (4444.5, 5555, 6666, 7777.5)
status: exit code: 0
command: "/nix/store/9wp8kivp1yp8x9gkzx7r4mry9gksn0ak-gdb-8.1/bin/gdb" "-quiet" "-batch" "-nx" "-command=/build/rustc-1.25.0-src/build/aarch64-unknown-linux-gnu/test/debuginfo/by-value-self-argument-in-trait-impl.debugger.script"
stdout:
------------------------------------------
GNU gdb (GDB) 8.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "aarch64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
Breakpoint 1 at 0x1464: file /build/rustc-1.25.0-src/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs, line 59.
Breakpoint 2 at 0x1490: file /build/rustc-1.25.0-src/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs, line 71.
Breakpoint 3 at 0x14c0: file /build/rustc-1.25.0-src/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs, line 78.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/nix/store/p59wbsv95bqapd9mpilcrdpcfv4bxldg-glibc-2.26-131/lib/libthread_db.so.1".

Breakpoint 1, <isize as by_value_self_argument_in_trait_impl::Trait>::method (self=1111) at /build/rustc-1.25.0-src/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs:59
59              zzz(); // #break
$1 = 1111

Breakpoint 2, <by_value_self_argument_in_trait_impl::Struct as by_value_self_argument_in_trait_impl::Trait>::method (self=...) at /build/rustc-1.25.0-src/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs:71
71              zzz(); // #break
$2 = by_value_self_argument_in_trait_impl::Struct {x: 2222, y: 3333}

Breakpoint 3, <(f64, isize, isize, f64) as by_value_self_argument_in_trait_impl::Trait>::method (self=...) at /build/rustc-1.25.0-src/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs:78
78              zzz(); // #break
$3 = (1.3906711614819425e-309, 3333, 281474976693728, 1.390671161483405e-309)
[Inferior 1 (process 87577) exited normally]

The full output can be found here (error is at the end).
The third gdb command prints:
$3 = (1.3906711614819425e-309, 3333, 281474976693728, 1.390671161483405e-309)
while the following is expected:
$3 = (4444.5, 5555, 6666, 7777.5)

There is clearly something going on, but I don't have any idea what that could be. I also don't have any access to an aarch64 build machine to debug this problem.

@bkchr
Copy link
Contributor Author

bkchr commented Apr 12, 2018

@alexcrichton could that be related to updating to LLVM6?

@alexcrichton
Copy link
Member

Perhaps! Especially if it's the 24 -> 25 release

@bkchr
Copy link
Contributor Author

bkchr commented Apr 12, 2018

Did you see something similar before?

@alexcrichton
Copy link
Member

Our CI does not run aarch64 tests, so this would not have been caught. This is likely an LLVM bug or a rustc bug

@Mic92
Copy link
Contributor

Mic92 commented Apr 12, 2018

@Mic92
Copy link
Contributor

Mic92 commented Apr 12, 2018

We also use the bundled llvm that comes with rust.

@alexcrichton
Copy link
Member

@Mic92 we're not running tests for aarch64, just building binaries

@vcunat
Copy link

vcunat commented Apr 16, 2018

Off-topic/tangent: I agree it seems better to disable all tests on aarch64 in nixpkgs. I think such test-fixing efforts should "start upstream". Nixpkgs probably doesn't have enough knowledge (human resources) to spearhead this, and even if we did, it would better be approached in a different way, e.g. testing and fixing before rustc is released, etc. (No blaming meant, just stating.) More thoughts on this?

@bkchr
Copy link
Contributor Author

bkchr commented Apr 16, 2018

No, I'm also for disabling the tests on aarch64 in nixpkgs :)

@Mic92
Copy link
Contributor

Mic92 commented Apr 16, 2018

done in NixOS/nixpkgs#39000

@chrisccoulson
Copy link
Contributor

chrisccoulson commented Apr 25, 2018

With a breakpoint at by-value-self-argument-in-trait-impl.rs:82:

(gdb) bt                                      
#0  <(f64, isize, isize, f64) as by_value_self_argument_in_trait_impl::Trait>::method (self=...)
    at /home/ubuntu/src/rustc/bionic-arm64/rustc-1.25.0+dfsg1+llvm/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs:82
#1  0x0000aaaaaaaab320 in by_value_self_argument_in_trait_impl::main () at /home/ubuntu/src/rustc/bionic-arm64/rustc-1.25.0+dfsg1+llvm/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs:90
(gdb) p self                                                                                                                                                                                             
$5 = (1.3906711615485821e-309, 3333, 281474976707216, 1.3906711615500445e-309)
(gdb) info registers                                
x0             0xfffffffff170   281474976706928     
x1             0xd05    3333                                                                             
x2             0xfffffffff290   281474976707216     
x3             0xfffffffff298   281474976707224     
x4             0xffffb7fd1258   281473768559192     
x5             0x0      0                           
x6             0x1      1                           
x7             0x0      0                                                                                                                                                                                          
x8             0xfffffffff150   281474976706896     
x9             0x1a0a   6666                        
x10            0x15b3   5555                        
x11            0x40b15c8000000000       4661608794130743296
x12            0x1      1
x13            0x270f   9999
x14            0x2      2
x15            0x0      0
x16            0xffffb7fd0ac8   281473768557256
x17            0xffffb7f5acbc   281473768074428
x18            0xffffb7ea7a70   281473767340656
x19            0xfffffffff250   281474976707152
x20            0xfffffffff290   281474976707216
x21            0xffffb7fd1258   281473768559192
x22            0x0      0
x23            0x0      0
x24            0x0      0
x25            0x0      0
x26            0x0      0
x27            0x0      0
x28            0x0      0
x29            0xfffffffff130   281474976706864
x30            0xaaaaaaaab320   187649984475936
sp             0xfffffffff0f0   0xfffffffff0f0
pc             0xaaaaaaaab268   0xaaaaaaaab268 <<(f64, isize, isize, f64) as by_value_self_argument_in_trait_impl::Trait>::method+12>
cpsr           0x60000000       [ EL=0 C Z ]
fpsr           0x0      0
fpcr           0x0      0

Observe that running "print self" dumps what appears to be the contents of x0 - x3. The procedure call standard for AArch64 says that in this case, the first parameter (self) will be passed as a pointer to caller allocated memory in x0, rather than being passed by value in registers. We can verify that this is actually the case:

(gdb) x/4xg $x0                                     
0xfffffffff170: 0x40b15c8000000000      0x00000000000015b3
0xfffffffff180: 0x0000000000001a0a      0x40be618000000000

(These are the values expected - 0x40b15c8000000000 = 4444.5, 0x15b3 = 5555, 0x1a0a = 6666, 0x40be618000000000 = 7777.5).

As expected, register x0 appears to be pointing to |self|, so this suggests there's a bug in the DWARF symbols. Looking out the output of objdump -e, you can see:

In .debug_info:

 <3><2ae>: Abbrev Number: 3 (DW_TAG_subprogram)
    <2af>   DW_AT_low_pc      : 0x125c
    <2b7>   DW_AT_high_pc     : 0x54
    <2bb>   DW_AT_frame_base  : 1 byte block: 6d        (DW_OP_reg29 (x29))
    <2bd>   DW_AT_linkage_name: (indirect string, offset: 0x3b9): _ZN112_$LT$$LP$f64$C$$u20$isize$C$$u20$isize$C$$u20$f64$RP$$u20$as$u20$by_value_self_argument_in_trait_impl..Trait$GT$6method17h59f5b2e7e1da7cd2E
    <2c1>   DW_AT_name        : (indirect string, offset: 0x313): method
    <2c5>   DW_AT_decl_file   : 1
    <2c6>   DW_AT_decl_line   : 81
    <2c7>   DW_AT_type        : <0x33b>
 <4><2cb>: Abbrev Number: 5 (DW_TAG_formal_parameter)
    <2cc>   DW_AT_location    : 0x0 (location list)
    <2d0>   DW_AT_name        : (indirect string, offset: 0x4fd): self
    <2d4>   DW_AT_decl_file   : 1
    <2d5>   DW_AT_decl_line   : 1
    <2d6>   DW_AT_type        : <0x33b>

DW_AT_type is the correct type:

 <1><33b>: Abbrev Number: 8 (DW_TAG_structure_type)
    <33c>   DW_AT_name        : (indirect string, offset: 0x45b): (f64, isize, isize, f64)
    <340>   DW_AT_byte_size   : 32
    <341>   DW_AT_alignment   : 8

But DW_AT_location doesn't look quite right. From the .debug_loc section:

    Offset   Begin            End              Expression
    00000000 000000000000125c 0000000000001270 (DW_OP_reg0 (x0))
    00000013 0000000000001270 00000000000012ac (DW_OP_breg31 (sp): 16)
    00000027 <End of list>

DW_OP_reg0 says that the location of |self| is x0, but x0 contains a pointer to the location. Shouldn't the "DW_OP_reg0" be "DW_OP_breg0: 0" instead?

@XAMPPRocky XAMPPRocky added A-trait-system Area: Trait system O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jul 7, 2018
@cuviper
Copy link
Member

cuviper commented Sep 5, 2018

I've done some investigation on this for Red Hat bugs, and found it to be an issue with GlobalISel.

If you add -Cllvm-args=-fast-isel, or enable optimization so -fast-isel is the default, then it works.

@cuviper
Copy link
Member

cuviper commented Sep 5, 2018

This also reminds me of #47611, where we needed to stop adding an explicit DW_OP_deref for LLVM5+ -- maybe GlobalISel didn't get that same behavior change?

@cuviper
Copy link
Member

cuviper commented Sep 6, 2018

I have a potential LLVM fix here: https://bugzilla.redhat.com/show_bug.cgi?id=1625768#c2

cuviper added a commit to cuviper/llvm that referenced this issue Sep 11, 2018
Summary:
D31439 changed the semantics of dbg.declare to take the address of a
variable as the first argument, making it indirect.  It specifically
updated FastISel for this change here:

https://reviews.llvm.org/D31439#change-WVArzi177jPl

GlobalISel needs to follow suit, or else it will be missing a level of
indirection in the generated debuginfo.  This problem was seen in a Rust
debuginfo test on aarch64, since GlobalISel is used at -O0 for aarch64.

rust-lang/rust#49807
https://bugzilla.redhat.com/show_bug.cgi?id=1611597
https://bugzilla.redhat.com/show_bug.cgi?id=1625768

Reviewers: dblaikie, aprantl, t.p.northover, javed.absar, rnk

Reviewed By: rnk

Subscribers: #debug-info, rovka, kristof.beyls, JDevlieghere, llvm-commits, tstellar

Differential Revision: https://reviews.llvm.org/D51749

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@341969 91177308-0d34-0410-b5e6-96231b3b80d8
cuviper added a commit to cuviper/rust that referenced this issue Sep 11, 2018
earl pushed a commit to earl/llvm-mirror that referenced this issue Sep 11, 2018
Summary:
D31439 changed the semantics of dbg.declare to take the address of a
variable as the first argument, making it indirect.  It specifically
updated FastISel for this change here:

https://reviews.llvm.org/D31439#change-WVArzi177jPl

GlobalISel needs to follow suit, or else it will be missing a level of
indirection in the generated debuginfo.  This problem was seen in a Rust
debuginfo test on aarch64, since GlobalISel is used at -O0 for aarch64.

rust-lang/rust#49807
https://bugzilla.redhat.com/show_bug.cgi?id=1611597
https://bugzilla.redhat.com/show_bug.cgi?id=1625768

Reviewers: dblaikie, aprantl, t.p.northover, javed.absar, rnk

Reviewed By: rnk

Subscribers: #debug-info, rovka, kristof.beyls, JDevlieghere, llvm-commits, tstellar

Differential Revision: https://reviews.llvm.org/D51749

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@341969 91177308-0d34-0410-b5e6-96231b3b80d8
kennytm added a commit to kennytm/rust that referenced this issue Sep 12, 2018
…alexcrichton

Update LLVM to fix GlobalISel dbg.declare

Fixes rust-lang#49807.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants