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

LLVM 5 fails debuginfo/by-value-self-argument-in-trait-impl.rs #47611

Closed
cuviper opened this issue Jan 20, 2018 · 8 comments
Closed

LLVM 5 fails debuginfo/by-value-self-argument-in-trait-impl.rs #47611

cuviper opened this issue Jan 20, 2018 · 8 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Jan 20, 2018

rustc compiled with LLVM 5 fails debuginfo/by-value-self-argument-in-trait-impl.rs on x86_64-unknown-linux-gnu. GDB fails to read self for the following impl in the test:

impl Trait for (f64, isize, isize, f64) {
    fn method(self) -> (f64, isize, isize, f64) {
        zzz(); // #break
        self
    }
}
Cannot access memory at address 0x40b15c8000000000

It's called with (4444.5, 5555, 6666, 7777.5).method(), and it just so happens that 4444.5 transmuted to a u64 is 0x40b15c8000000000, so there seems to be an indirection/deref problem in the debuginfo.

Full test output:

---- [debuginfo-gdb] debuginfo/by-value-self-argument-in-trait-impl.rs stdout ----
        NOTE: compiletest thinks it is using GDB with native rust support
NOTE: compiletest thinks it is using GDB version 8000001

error: line not found in debugger output: $3 = (4444.5, 5555, 6666, 7777.5)
status: exit code: 0
command: "/usr/bin/gdb" "-quiet" "-batch" "-nx" "-command=/home/jistone/rust/rust/build/x86_64-unknown-linux-gnu/test/debuginfo/by-value-self-argument-in-trait-impl.debugger.script"
stdout:
------------------------------------------
GNU gdb (GDB) Fedora 8.0.1-33.fc27
Copyright (C) 2017 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 "x86_64-redhat-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 0xf9c: file /home/jistone/rust/rust/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs, line 59.
Breakpoint 2 at 0xfc0: file /home/jistone/rust/rust/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs, line 71.
Breakpoint 3 at 0xfeb: file /home/jistone/rust/rust/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs, line 78.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, <isize as by_value_self_argument_in_trait_impl::Trait>::method (self=1111) at /home/jistone/rust/rust/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 /home/jistone/rust/rust/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=<error reading variable: Cannot access memory at address 0x40b15c8000000000>) at /home/jistone/rust/rust/src/test/debuginfo/by-value-self-argument-in-trait-impl.rs:78
78              zzz(); // #break

------------------------------------------
stderr:
------------------------------------------
/home/jistone/rust/rust/build/x86_64-unknown-linux-gnu/test/debuginfo/by-value-self-argument-in-trait-impl.debugger.script:16: Error in sourced command file:
Cannot access memory at address 0x40b15c8000000000

------------------------------------------

thread '[debuginfo-gdb] debuginfo/by-value-self-argument-in-trait-impl.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2884:9
@cuviper cuviper mentioned this issue Jan 20, 2018
43 tasks
@cuviper
Copy link
Member Author

cuviper commented Jan 20, 2018

Here's the .debug_loc in the working version compiled with LLVM 4:

    Offset   Begin            End              Expression
    00000000 0000000000000fe0 0000000000000ff3 (DW_OP_breg4 (rsi): 0)
    00000014 0000000000000ff3 000000000000104c (DW_OP_breg6 (rbp): -48; DW_OP_deref)
    00000029 <End of list>

And here it is compiled with LLVM 5 -- note the extra DW_OP_deref:

    Offset   Begin            End              Expression
    00000000 0000000000000fe0 0000000000000ff3 (DW_OP_breg4 (rsi): 0; DW_OP_deref)
    00000015 0000000000000ff3 000000000000104c (DW_OP_breg6 (rbp): -48; DW_OP_deref; DW_OP_deref)
    0000002b <End of list>

@alexcrichton
Copy link
Member

Nice investigation @cuviper! Grepping the LLVM history may mean that llvm-mirror/llvm@ca18763 is a point of interest.

@cuviper
Copy link
Member Author

cuviper commented Jan 23, 2018

@alexcrichton AFAICS that commit is after LLVM 5, present in the release_60 and master branches -- so are you looking at that commit that it might have fixed this?

In release_40..release_50, this one looks suspicious to me: llvm-mirror/llvm@b560ea7

I had to fix the semantics of dbg.declare for describing function
arguments. After this patch a dbg.declare always takes the address
of a variable as the first argument, even if the argument is not an
alloca.

Which might mean that our own deref logic is out of date.

@alexcrichton
Copy link
Member

Oh heh, good point!

I actually saw that LLVM 6 was tagged with an rc today so I was kicking the tires tonight, and unfortunately this failure is still here after updating to LLVM 6. (that's how I saw it)

In that case you're probably right! @eddyb or @michaelwoerister would y'all be able to help out with this?

@kennytm kennytm added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jan 23, 2018
@cuviper
Copy link
Member Author

cuviper commented Jan 23, 2018

To test, I tried simply masking that manual deref op:

diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs
index b367eb6548d0..56e545cf8c3a 100644
--- a/src/librustc_trans/mir/mod.rs
+++ b/src/librustc_trans/mir/mod.rs
@@ -492,7 +492,7 @@ fn arg_local_refs<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
                 };

                 if let PassMode::Indirect(ref attrs) = arg.mode {
-                    if !attrs.contains(ArgAttribute::ByVal) {
+                    if !attrs.contains(ArgAttribute::ByVal) && false {
                         variable_access = VariableAccess::IndirectVariable {
                             alloca: place.llval,
                             address_operations: &deref_op,

This passes all debuginfo tests with LLVM 5! But (predictably) it fails on LLVM 4 and 3.9. So I guess we could add a conditional flag from rustllvm to tell us whether a manual deref should be added, depending on LLVM version. Or is there a better way to accomplish this?

@alexcrichton
Copy link
Member

@cuviper heh I was just reading over the commit you linked (which seems like the correct diagnosis) and the rustc code and concluded to test the same thing, glad it actually works!

I'd personally be fine with a "do this on LLVM 4 and below but not 5 and up" conditional (with a comment), and @michaelwoerister could do the final sign off to make sure it's not completely crazy.

cuviper added a commit to cuviper/rust that referenced this issue Jan 23, 2018
We needed to manually added the `DW_OP_deref` ourselves in earlier LLVM,
but starting with [D31439] in LLVM 5, it appears that LLVM will always
handle this itself.  When we were still adding this manually, the
resulting `.debug_loc` had too many derefs, and this failed test
`debuginfo/by-value-self-argument-in-trait-impl.rs`.

[D31439]: https://reviews.llvm.org/D31439

Fixes rust-lang#47611.
cc @alexcrichton
r? @michaelwoerister
alexcrichton pushed a commit that referenced this issue Jan 24, 2018
We needed to manually added the `DW_OP_deref` ourselves in earlier LLVM,
but starting with [D31439] in LLVM 5, it appears that LLVM will always
handle this itself.  When we were still adding this manually, the
resulting `.debug_loc` had too many derefs, and this failed test
`debuginfo/by-value-self-argument-in-trait-impl.rs`.

[D31439]: https://reviews.llvm.org/D31439

Fixes #47611.
cc @alexcrichton
r? @michaelwoerister
alexcrichton pushed a commit that referenced this issue Jan 25, 2018
We needed to manually added the `DW_OP_deref` ourselves in earlier LLVM,
but starting with [D31439] in LLVM 5, it appears that LLVM will always
handle this itself.  When we were still adding this manually, the
resulting `.debug_loc` had too many derefs, and this failed test
`debuginfo/by-value-self-argument-in-trait-impl.rs`.

[D31439]: https://reviews.llvm.org/D31439

Fixes #47611.
cc @alexcrichton
r? @michaelwoerister
@dotdash
Copy link
Contributor

dotdash commented Jan 26, 2018

If I'm not mistaken, I originally added that check, and looking through the referenced LLVM commit, doing this conditionally based on the LLVM version seems fine to me.

@alexcrichton
Copy link
Member

Thanks for the confirmation @dotdash!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. 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

4 participants