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

Workaround for #24984 - Solve crashes when panicking / Hang Monitoring by using cargo's patch mechanism to work around a bug in libbacktrace #25248

Merged
merged 1 commit into from Dec 12, 2019

Conversation

@MeFisto94
Copy link
Contributor

MeFisto94 commented Dec 11, 2019

Temporarly patch backtrace-rs


libbacktrace contains an error leading to invalid pointers when trying to backtrace a stacktrace.
These invalid pointers led to a crash, whenever a panic or Hang Monitoring happened (which is where the stack is captured).
Since fixing the issue properly at gcc will require some time, I've created a small and patched version of backtrace-rs, which will be used as patch here.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #24984
  • These changes do not require tests because they fix a native crash which is a) really obvious and b) shouldn't be a regular case.
@highfive
Copy link

highfive commented Dec 11, 2019

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @ferjm (or someone else) soon.

@jdm
Copy link
Member

jdm commented Dec 11, 2019

@bors-servo r+
Thanks!

@bors-servo
Copy link
Contributor

bors-servo commented Dec 11, 2019

📌 Commit 9c6d6e5 has been approved by jdm

@highfive highfive assigned jdm and unassigned ferjm Dec 11, 2019
@MeFisto94 MeFisto94 force-pushed the MeFisto94:fix-linux-crash-libbacktrack branch from 47d0d8a to 93305fc Dec 11, 2019
@jdm
Copy link
Member

jdm commented Dec 11, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 11, 2019

📌 Commit 93305fc has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

Testing commit 93305fc with merge e119266...

bors-servo added a commit that referenced this pull request Dec 12, 2019
Workaround for #24984 - Solve crashes when panicking / Hang Monitoring by using cargo's patch mechanism to work around a bug in libbacktrace

Temporarly patch backtrace-rs

---
libbacktrace contains an error leading to invalid pointers when trying to backtrace a stacktrace.
These invalid pointers led to a crash, whenever a panic or Hang Monitoring happened (which is where the stack is captured).
Since fixing the issue properly at gcc will require some time, I've created a small and patched version of backtrace-rs, which will be used as patch here.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24984
- [X] These changes do not require tests because they fix a native crash which is a) really obvious and b) shouldn't be a regular case.
@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 12, 2019

Both macos builds failed while building backtrace-sys:

running: "clang" "clang" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=x86_64-apple-darwin" "-I" "src/libbacktrace" "-I" "/Users/worker/tasks/task_1576113888/repo/target/debug/build/backtrace-sys-e396ae41fd7f9ab3/out" "-fvisibility=hidden" "-DBACKTRACE_SUPPORTED=1" "-DBACKTRACE_USES_MALLOC=1" "-DBACKTRACE_SUPPORTS_THREADS=0" "-DBACKTRACE_SUPPORTS_DATA=0" "-DHAVE_DL_ITERATE_PHDR=1" "-D_GNU_SOURCE=1" "-D_LARGE_FILES=1" "-Dbacktrace_full=__rbt_backtrace_full" "-Dbacktrace_dwarf_add=__rbt_backtrace_dwarf_add" "-Dbacktrace_initialize=__rbt_backtrace_initialize" "-Dbacktrace_pcinfo=__rbt_backtrace_pcinfo" "-Dbacktrace_syminfo=__rbt_backtrace_syminfo" "-Dbacktrace_get_view=__rbt_backtrace_get_view" "-Dbacktrace_release_view=__rbt_backtrace_release_view" "-Dbacktrace_alloc=__rbt_backtrace_alloc" "-Dbacktrace_free=__rbt_backtrace_free" "-Dbacktrace_vector_finish=__rbt_backtrace_vector_finish" "-Dbacktrace_vector_grow=__rbt_backtrace_vector_grow" "-Dbacktrace_vector_release=__rbt_backtrace_vector_release" "-Dbacktrace_close=__rbt_backtrace_close" "-Dbacktrace_open=__rbt_backtrace_open" "-Dbacktrace_print=__rbt_backtrace_print" "-Dbacktrace_simple=__rbt_backtrace_simple" "-Dbacktrace_qsort=__rbt_backtrace_qsort" "-Dbacktrace_create_state=__rbt_backtrace_create_state" "-Dbacktrace_uncompress_zdebug=__rbt_backtrace_uncompress_zdebug" "-Dmacho_get_view=__rbt_macho_get_view" "-Dmacho_symbol_type_relevant=__rbt_macho_symbol_type_relevant" "-Dmacho_get_commands=__rbt_macho_get_commands" "-Dmacho_try_dsym=__rbt_macho_try_dsym" "-Dmacho_try_dwarf=__rbt_macho_try_dwarf" "-Dmacho_get_addr_range=__rbt_macho_get_addr_range" "-Dmacho_get_uuid=__rbt_macho_get_uuid" "-Dmacho_add=__rbt_macho_add" "-Dmacho_add_symtab=__rbt_macho_add_symtab" "-Dmacho_file_to_host_u64=__rbt_macho_file_to_host_u64" "-Dmacho_file_to_host_u32=__rbt_macho_file_to_host_u32" "-Dmacho_file_to_host_u16=__rbt_macho_file_to_host_u16" "-o" "/Users/worker/tasks/task_1576113888/repo/target/debug/build/backtrace-sys-e396ae41fd7f9ab3/out/src/libbacktrace/macho.o" "-c" "src/libbacktrace/macho.c"
cargo:warning=clang: error: no such file or directory: 'src/libbacktrace/macho.c'
cargo:warning=clang: error: no input files
exit code: 1
@MeFisto94
Copy link
Contributor Author

MeFisto94 commented Dec 12, 2019

This happened, because I based my patch on master, but it seems there already were some rust specific fixes in branch https://github.com/MeFisto94/libbacktrace/commits/rust-snapshot-2018-05-22
So I'm going to rebase the branch, and then a retry should be enough.

@MeFisto94
Copy link
Contributor Author

MeFisto94 commented Dec 12, 2019

@bors-servo retry
(If I am not authorized, you at least now know that it should be fixed :P)

@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

@MeFisto94: 🔑 Insufficient privileges: not in try users

@jdm
Copy link
Member

jdm commented Dec 12, 2019

There will need to be updated Cargo.lock changes in this PR.

@jdm
Copy link
Member

jdm commented Dec 12, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

Testing commit c27b4e2 with merge b43cc1d...

bors-servo added a commit that referenced this pull request Dec 12, 2019
Workaround for #24984 - Solve crashes when panicking / Hang Monitoring by using cargo's patch mechanism to work around a bug in libbacktrace

Temporarly patch backtrace-rs

---
libbacktrace contains an error leading to invalid pointers when trying to backtrace a stacktrace.
These invalid pointers led to a crash, whenever a panic or Hang Monitoring happened (which is where the stack is captured).
Since fixing the issue properly at gcc will require some time, I've created a small and patched version of backtrace-rs, which will be used as patch here.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24984
- [X] These changes do not require tests because they fix a native crash which is a) really obvious and b) shouldn't be a regular case.
@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 12, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

Testing commit c27b4e2 with merge 16906d5...

bors-servo added a commit that referenced this pull request Dec 12, 2019
Workaround for #24984 - Solve crashes when panicking / Hang Monitoring by using cargo's patch mechanism to work around a bug in libbacktrace

Temporarly patch backtrace-rs

---
libbacktrace contains an error leading to invalid pointers when trying to backtrace a stacktrace.
These invalid pointers led to a crash, whenever a panic or Hang Monitoring happened (which is where the stack is captured).
Since fixing the issue properly at gcc will require some time, I've created a small and patched version of backtrace-rs, which will be used as patch here.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24984
- [X] These changes do not require tests because they fix a native crash which is a) really obvious and b) shouldn't be a regular case.
@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 12, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

Testing commit c27b4e2 with merge a5c368f...

bors-servo added a commit that referenced this pull request Dec 12, 2019
Workaround for #24984 - Solve crashes when panicking / Hang Monitoring by using cargo's patch mechanism to work around a bug in libbacktrace

Temporarly patch backtrace-rs

---
libbacktrace contains an error leading to invalid pointers when trying to backtrace a stacktrace.
These invalid pointers led to a crash, whenever a panic or Hang Monitoring happened (which is where the stack is captured).
Since fixing the issue properly at gcc will require some time, I've created a small and patched version of backtrace-rs, which will be used as patch here.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24984
- [X] These changes do not require tests because they fix a native crash which is a) really obvious and b) shouldn't be a regular case.
@bors-servo
Copy link
Contributor

bors-servo commented Dec 12, 2019

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing a5c368f to master...

@bors-servo bors-servo merged commit c27b4e2 into servo:master Dec 12, 2019
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@SimonSapin
Copy link
Member

SimonSapin commented Dec 13, 2019

What’s the path to going back to upstream backtrace-rs? Is there a PR or an issue open with them?

@MeFisto94
Copy link
Contributor Author

MeFisto94 commented Dec 13, 2019

Yep, just look at the tracking issue #24984
There I've linked the ultimate-upstream for gcc and the actual libbacktrace in my last comment. Once those are solved, it will trickle down to backtrace-rs (I openend an issue, it should be linked in the very first comments/links in the issue).

The problem mainly is: This crash (invalid strings) is only a symptom of symbols loading, which fails early (basically due to a wrong assumption and servos sheer size of over 2GiB (thus it cannot be read with one syscall). So there is more involved to solve things properly

@MeFisto94 MeFisto94 deleted the MeFisto94:fix-linux-crash-libbacktrack branch Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants
You can’t perform that action at this time.