Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upServo crashes on launch with Rust nightly ≥ 2017-11-21 #19519
Comments
|
macOS is fine. |
|
Same symptom (maybe different issue) on Linux x86_64. I attached glxinfo, gdb backtrace, and servo standard output. |
|
I'm also seeing this on Linux. It only occurs on a release build, debug builds are fine. The backtrace on Linux is in a GLX specific function, which suggests that either the Windows crash is different, or probably more likely, there is something else bad going on which happens to cause a crash in that location. |
|
A bit more information: gdb claims the function at the top of the stack is: |
|
git bisect says: Assuming I bisected correctly, that is. The crash only seems to occur when opening a real window - it runs fine in headless mode, which would explain why it got through CI OK. cc @SimonSapin @jdm |
|
That commit updates the compiler from |
|
I'm seeing this too. |
|
rustc 2017-12-01 doesn't build: |
|
This is likely rust-lang/rust#46519, which was introduced in rust-lang/rust#45225 ( |
|
Confirming that current master built with nightly 2017-11-14 doesn't have this problem. |
|
We should try building with LLVM asserts: https://rust-lang-ci2.s3.amazonaws.com/rustc-builds-alt/5a2465e2b44ecdd3b5d835c0abe29e9a4c9dcfe4/rustc-nightly-x86_64-unknown-linux-gnu.tar.gz |
|
Unfortunately that didn’t help, I just built servo in release mode with the "alt" build of |
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes work around #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes work around #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes work around #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes work around #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
Downgrade to rustc nightly-2017-11-14 <!-- Please describe your changes on the following line: --> This is causing nightly servo to crash with hardware acceleration. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes work around #19519 - [X] These changes do not require tests because testing requires AWS instances with GPUs. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19574) <!-- Reviewable:end -->
|
This was fixed. |
|
No it is not. We just reverted to an earlier version of the compiler. We still need to isolate the bug and file it upstream so that it can be properly fixed. |
SimonSapin
changed the title
atouchet
changed the title
|
I didn’t manage to reproduce this on Linux with Intel GPU. Servo master compiled with nightly-2017-12-07 in release mode loaded a couple URLs without crashing. |
[Do not merge] Upgrade to rustc 1.24.0-nightly (0a3761e63 2018-01-03) Do not merge (yet), as this might bring #19519 back. Also blocked on rust-lang/rust#46906 (comment). <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19683) <!-- Reviewable:end -->
[Do not merge] Upgrade to rustc 1.24.0-nightly (0a3761e63 2018-01-03) Do not merge (yet), as this might bring #19519 back. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19683) <!-- Reviewable:end -->
[Do not merge] Upgrade to rustc 1.24.0-nightly (0a3761e63 2018-01-03) Do not merge (yet), as this might bring #19519 back. Fixes #19635 Fixes #19637 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19683) <!-- Reviewable:end -->
|
I’ve managed to reproduce on my desktop machine. With nightly-2018-01-04 (#19683), in release mode with debug symbols enabled, gdb says that the stack at SIGSEGV as below. This is on Ubuntu 17.10 with a #0 0x0000000000000000 in ?? ()
#1 0x00005555578ef697 in glutin::api::x11::ffi::glx_extra::Glx::SwapIntervalEXT (interval=1, self=<optimized out>,
dpy=<optimized out>, drawable=<optimized out>)
at /home/simon/servo/target/release/build/servo-glutin-3060eb57c723a788/out/glx_extra_bindings.rs:593
#2 glutin::api::glx::ContextPrototype::finish (self=ContextPrototype = {...}, window=27262978)
at /home/simon/.cargo/registry/src/github.com-1ecc6299db9ec823/servo-glutin-0.13.1/src/api/glx/mod.rs:223
#3 0x00005555578f6ca0 in glutin::api::x11::window::Window::new (display=<optimized out>,
window_attrs=<optimized out>, pf_reqs=<optimized out>, opengl=<optimized out>)
at /home/simon/.cargo/registry/src/github.com-1ecc6299db9ec823/servo-glutin-0.13.1/src/api/x11/window.rs:642
#4 0x00005555578fbeac in glutin::platform::platform::api_dispatch::Window::new (window=<optimized out>,
pf_reqs=<optimized out>, opengl=...)
at /home/simon/.cargo/registry/src/github.com-1ecc6299db9ec823/servo-glutin-0.13.1/src/platform/linux/api_dispatch.rs:129
#5 glutin::window::<impl glutin::WindowBuilder<'a>>::build (self=WindowBuilder = {...})
at /home/simon/.cargo/registry/src/github.com-1ecc6299db9ec823/servo-glutin-0.13.1/src/window.rs:232
#6 0x00005555558a237e in glutin_app::window::Window::new (is_foreground=<optimized out>,
window_size=TypedSize2D<u32, servo_geometry::DeviceIndependentPixel> = {...}, parent=core::option::Option::None)
at ports/glutin/window.rs:273
#7 0x00005555558a0707 in glutin_app::create_window (parent=core::option::Option::None) at ports/glutin/lib.rs:48
#8 0x0000555555874757 in servo::main () at ports/servo/main.rs:147 |
|
With some whitespace added, the function for the #[inline]
pub unsafe fn SwapIntervalEXT(&self, dpy: *mut types::Display, drawable: types::GLXDrawable,
interval: __gl_imports::raw::c_int) -> () {
__gl_imports::mem::transmute::<
_,
extern "system" fn(*mut types::Display, types::GLXDrawable, __gl_imports::raw::c_int) -> ()
>(self.SwapIntervalEXT.f)(dpy, drawable, interval)
}Since the #[allow(dead_code, missing_copy_implementations)]
#[derive(Clone)]
pub struct FnPtr {
/// The function pointer that will be used when calling the function.
f: *const __gl_imports::raw::c_void,
/// True if the pointer points to a real function, false if points to a `panic!` fn.
is_loaded: bool,
}
impl FnPtr {
/// Creates a `FnPtr` from a load attempt.
fn new(ptr: *const __gl_imports::raw::c_void) -> FnPtr {
if ptr.is_null() {
FnPtr {
f: missing_fn_panic as *const __gl_imports::raw::c_void,
is_loaded: false
}
} else {
FnPtr { f: ptr, is_loaded: true }
}
}After some gdb wrangling and squinting as disassembled code with nbp (thanks), the memory area that is probably I’ve also established that this bug is introduced or uncovered by rust-lang/rust#45225 [1], but [1] This crash does not occur with nightly-2017-11-20, which is immediately before that Rust PR was merged. Servo doesn’t build at that PR, but if I compile Rust at the merge commit of rust-lang/rust#45225 with rust-lang/rust#46521 (the build error fix) backported, I can then reproduce this crash. |
SimonSapin
changed the title
|
@eddyb Any idea how your memory layout changes in rust-lang/rust#45225 could cause |
|
@SimonSapin From what I've seen in the past, there's potentially some layout optimization going on "around" the struct in question, i.e. a type containing Another possibility is the signature of |
|
This #[derive(Clone)]
pub struct Glx {
pub ChooseFBConfig: FnPtr,
// 36 other FnPtr fields
pub SwapIntervalEXT: FnPtr,
pub SwapIntervalSGI: FnPtr,
pub UseXFont: FnPtr,
pub WaitGL: FnPtr,
pub WaitX: FnPtr,
_priv: ()
}At one point that struct cloned out of an The IR contains !3882 = !DIDerivedType(tag: DW_TAG_member, name: "ChooseFBConfig", scope: !3878, file: !2, baseType: !3883, size: 128, align: 64)
!3883 = !DICompositeType(tag: DW_TAG_structure_type, name: "FnPtr", scope: !3879, file: !2, size: 128, align: 64, elements: !3884, identifier: "d6efa958cbdab5062f3008eefb6f530f")
!3884 = !{!3885, !3887}
!3885 = !DIDerivedType(tag: DW_TAG_member, name: "f", scope: !3883, file: !2, baseType: !3886, size: 64, align: 64)
!3886 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "*const std::os::raw::c_void", baseType: !160, size: 64, align: 64)
!3887 = !DIDerivedType(tag: DW_TAG_member, name: "is_loaded", scope: !3883, file: !2, baseType: !3888, size: 8, align: 8, offset: 64)
!3888 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
!3889 = !DIDerivedType(tag: DW_TAG_member, name: "ChooseVisual", scope: !3878, file: !2, baseType: !3883, size: 128, align: 64, offset: 128)and later !21353 = !DIDerivedType(tag: DW_TAG_member, name: "ChooseFBConfig", scope: !21350, file: !2, baseType: !21354, size: 128, align: 64)
!21354 = !DICompositeType(tag: DW_TAG_structure_type, name: "FnPtr", scope: !21351, file: !2, size: 128, align: 64, elements: !21355, identifier: "93bb23b996d87ed937224ca12cf3a54d")
!21355 = !{!21356, !21357}
!21356 = !DIDerivedType(tag: DW_TAG_member, name: "f", scope: !21354, file: !2, baseType: !3886, size: 64, align: 64)
!21357 = !DIDerivedType(tag: DW_TAG_member, name: "is_loaded", scope: !21354, file: !2, baseType: !3888, size: 8, align: 8, offset: 64)
!21358 = !DIDerivedType(tag: DW_TAG_member, name: "ChooseVisual", scope: !21350, file: !2, baseType: !21354, size: 128, align: 64, offset: 128)
// […]
!21394 = !DIDerivedType(tag: DW_TAG_member, name: "SwapIntervalEXT", scope: !21350, file: !2, baseType: !21354, size: 128, align: 64, offset: 4736) |
|
@SimonSapin Sorry, I meant to write "the signature of |
; glutin::api::egl::ffi::egl::FnPtr::new
; Function Attrs: nounwind readnone uwtable
define { i8*, i1 } @_ZN6glutin3api3egl3ffi3egl5FnPtr3new17h381a0c6cab4ff37bE(i8*) unnamed_addr #2 !dbg !35222 {
start:
tail call void @llvm.dbg.value(metadata i8* %0, i64 0, metadata !35221, metadata !4955), !dbg !37651
tail call void @llvm.dbg.value(metadata i8* %0, i64 0, metadata !15186, metadata !4955), !dbg !37652
%1 = icmp eq i8* %0, null, !dbg !37654
%.sink3 = select i1 %1, i8* bitcast (void ()* @_ZN6glutin3api3egl3ffi3egl16missing_fn_panic17he9e478322295adbdE to i8*), i8* %0, !dbg !37655
%not. = xor i1 %1, true, !dbg !37655
%2 = insertvalue { i8*, i1 } undef, i8* %.sink3, 0, !dbg !37656
%3 = insertvalue { i8*, i1 } %2, i1 %not., 1, !dbg !37656
ret { i8*, i1 } %3, !dbg !37656
} |
|
We’re interested in |
|
Well, that IR looks just fine to me, so it's probably some outer/earlier code to the point where it becomes NULL. What happens if you replace the |
|
I’ll try patching the build-dependency crate that generates this code to do that. Won’t there still be a niche in the padding (one pointer + one byte rounded up to two words) that |
|
@SimonSapin Using padding for niches is not necessarily cheaper because now all padding everywhere must be always initialized (similar problem with |
|
Ok, I see (re padding). Changing the |
|
Partly-minimized test case: extern crate glutin;
fn main() {
glutin::WindowBuilder::new().with_vsync().build().unwrap();
}With our https://crates.io/crates/servo-glutin fork. |
|
The segfault disappears if I add |
|
Or if I make the |
[Do not merge] Upgrade to rustc 1.24.0-nightly (0a3761e63 2018-01-03) Do not merge (yet), as this might bring #19519 back. Fixes #19635 Fixes #19637 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19683) <!-- Reviewable:end -->
[Do not merge] Upgrade to rustc 1.24.0-nightly (0a3761e63 2018-01-03) Do not merge (yet), as this might bring #19519 back. Fixes #19635 Fixes #19637 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19683) <!-- Reviewable:end -->
|
Pinging @aturon so that it's on the Rust team's radar. |
[Do not merge] Upgrade to rustc 1.24.0-nightly (0a3761e63 2018-01-03) Do not merge (yet), as this might bring #19519 back. Fixes #19635 Fixes #19637 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19683) <!-- Reviewable:end -->
|
This is brendanzab/gl-rs#438, a library bug. |
|
Thanks to eddyb’s pointed questions we found the root cause. https://botbot.me/mozilla/rustc/2018-01-10/?msg=95583307&page=2 TL;DR: it’s a bug in The function pointer that is incorrectly null is earlier returned by: pub unsafe fn GetProcAddress(&self, procName: *const types::GLubyte) -> types::__GLXextFuncPtrwith pub type __GLXextFuncPtr = extern "system" fn();This type definition is incorrect because The fix is defining |
Upgrade to rustc 1.24.0-nightly (0a3761e63 2018-01-03) Fixes #19635 Fixes #19637 Fixes #19735 Fixes #19519 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19683) <!-- Reviewable:end -->
atouchet
changed the title
Steps to reproduce:
Tested in Windows 10 with Browser.html.