Skip to content

Intermittent with backtrace-debuginfo.rs #27619

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
Manishearth opened this issue Aug 9, 2015 · 7 comments
Closed

Intermittent with backtrace-debuginfo.rs #27619

Manishearth opened this issue Aug 9, 2015 · 7 comments

Comments

@Manishearth
Copy link
Member

http://buildbot.rust-lang.org/builders/auto-linux-32-opt/builds/6036/steps/test/logs/stdio from #27617


failures:

---- [run-pass] run-pass/backtrace-debuginfo.rs stdout ----

error: test run failed!
status: exit code: 101
command: i686-unknown-linux-gnu/test/run-pass/backtrace-debuginfo.stage2-i686-unknown-linux-gnu 
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
thread '<main>' panicked at 'trace does not match position list: test case 3
thread '<main>' panicked at 'explicit panic', /home/ubuntu/src/rust-buildbot/slave/auto-linux-32-opt/build/src/test/run-pass/backtrace-debuginfo.rs:79
stack backtrace:
   1: 0x55640dc0 - sys::backtrace::write::h24f1716ef3fafcd37As
   2: 0x5564a225 - panicking::on_panic::h5c416bec33d18845IFx
   3: 0x556065b1 - rt::unwind::begin_unwind_inner::he563e4ed74b0a915T6w
   4: 0x55556e6c - rt::unwind::begin_unwind::h11536406982305423188
                at ../src/libstd/rt/unwind/mod.rs:236
   5: 0x55556dfe - inner::hebc4c6975bd94e72uca
                at /home/ubuntu/src/rust-buildbot/slave/auto-linux-32-opt/build/obj/<std macros>:3
   6: 0x555570bd - outer::h56df4a17ad3d0ec6kla
                at /home/ubuntu/src/rust-buildbot/slave/auto-linux-32-opt/build/src/test/run-pass/backtrace-debuginfo.rs:104
   7: 0x55559a56 - main::hfba55d59fb2cc519Pqa
                at /home/ubuntu/src/rust-buildbot/slave/auto-linux-32-opt/build/src/test/run-pass/backtrace-debuginfo.rs:152
   8: 0x5564c2ea - rt::unwind::try::try_fn::h13372541063495895142
   9: 0x55649a6d - __rust_try
  10: 0x5564bfee - rt::lang_start::h1d090ba3935217b6DAx
  11: 0x5555a512 - main
  12: 0x559b5a82 - __libc_start_main
  13: 0x55556850 - <unknown>

---
backtrace-debuginfo-aux.rs:20
backtrace-debuginfo.rs:78
backtrace-debuginfo.rs:104
backtrace-debuginfo.rs:152
', /home/ubuntu/src/rust-buildbot/slave/auto-linux-32-opt/build/src/test/run-pass/backtrace-debuginfo.rs:118

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

thread '[run-pass] run-pass/backtrace-debuginfo.rs' panicked at 'explicit panic', /home/ubuntu/src/rust-buildbot/slave/auto-linux-32-opt/build/src/compiletest/runtest.rs:1490



failures:
    [run-pass] run-pass/backtrace-debuginfo.rs
@TimNN
Copy link
Contributor

TimNN commented Aug 9, 2015

I think this needs to be reopened:

cc #27617, #27612, #27611

@TimNN
Copy link
Contributor

TimNN commented Aug 10, 2015

I just tried building the master branch locally and it fails with the same error (compiling i686-unknown-linux-gnu on 64bit ubuntu).

@TimNN
Copy link
Contributor

TimNN commented Aug 10, 2015

Looks like this was introduced in febdc3b (a5d33d8 tests successfully for me) when @arielb1 accidentally pushed to master (see https://botbot.me/mozilla/rust-internals/2015-08-09/?msg=46787325&page=2; because of the minor change the consensus was to leave the commit as is).

bors added a commit that referenced this issue Aug 10, 2015

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
I don't know how this single inline caused the breakage but it seems to be the cause of the issue (see #27619 (comment)).
@bluss
Copy link
Member

bluss commented Aug 10, 2015

I'm sorry that I didn't suggest more caution there, truly surprising breakage. Lesson learned.. everything must pass bors.

@barosl
Copy link
Contributor

barosl commented Aug 10, 2015

That's quite intriguing! The one-line change seemed truly innocent to me. Any idea on how the breakage happened?

@alexcrichton
Copy link
Member

Fixed in #27634

@dotdash
Copy link
Contributor

dotdash commented Aug 10, 2015

So, I have figured out what happened here (mostly) and it's pretty hilarious. For those who care here's the story...

The key fact here is that there a few calls to begin_unwind, and all of these that the string explicit panic. Usually LLVM recognizes this and specializes begin_unwind to have that value inline instead of getting it as an argument.

This means that the call to begin_unwind is very cheap, only the line number needs to be loaded into a register. This means that the common code for the calls to begin_unwind are just 2 instructions (a move and a call).

Now, for some reason the added inline attribute tickles LLVM in a bad way and it stops to eliminate the function argument. I don't have an explanation for this(, yet?). Anyway, because the argument is still there, the calls to begin_unwind end up being more expensive (3 moves and a call).

Now, the calls to begin_unwind don't return. So they are block tails. And because the calls are more expensive now, tail merging now kicks in. And tail merging simply can't preserve the debuginfo, so the test has to fail.

Took me quite a while to figure that out. Fun fact: I basically had this figured out before, because tail merging was also the reason why I had to remove a case from that test when I turned fat pointers into immediate function arguments.

Anyway, I'm going to file a PR that adds -Cllvm-args=-enable-tail-merge=0 for this testcase, that should make it less error prone.

dotdash added a commit to dotdash/rust that referenced this issue Aug 10, 2015

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
LLVM might perform tail merging on the calls that initiate the unwinding
process which breaks debuginfo and therefore this test. Since tail
merging is guaranteed to break debuginfo, it should be disabled for this
test.

This allows us to restore a testcase that I had to remove earlier
because of the same problem, because back then I didn't realize that
disabling tail merging was an option.

cc rust-lang#27619
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 12, 2015
LLVM might perform tail merging on the calls that initiate the unwinding
process which breaks debuginfo and therefore this test. Since tail
merging is guaranteed to break debuginfo, it should be disabled for this
test.

This allows us to restore a testcase that I had to remove earlier
because of the same problem, because back then I didn't realize that
disabling tail merging was an option.

cc rust-lang#27619
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

6 participants