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

Nightly 2020-02-12 causes segmentation fault in glib testsuite #69102

Closed
sdroege opened this issue Feb 12, 2020 · 4 comments
Closed

Nightly 2020-02-12 causes segmentation fault in glib testsuite #69102

sdroege opened this issue Feb 12, 2020 · 4 comments
Labels
C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sdroege
Copy link
Contributor

sdroege commented Feb 12, 2020

cargo rustc-bisect blames fc23a81 from #68491 (CC @pnkfelix)

Running cargo test -- subclass::object::test::test_create in the glib bindings crate is enough to reproduce it.

I'll look in more detail later / in the next days, just wanted to get this out there for now.

Backtrace looks as follows

Process 425537 stopped
* thread #2, name = 'subclass::objec', stop reason = signal SIGSEGV: invalid address (fault address: 0x1)
    frame #0: 0x00007ffff7f6f30d libgobject-2.0.so.0`g_type_check_instance_is_fundamentally_a + 13
libgobject-2.0.so.0`g_type_check_instance_is_fundamentally_a:
->  0x7ffff7f6f30d <+13>: movq   (%rax), %rax
    0x7ffff7f6f310 <+16>: cmpq   $0x3fc, %rax              ; imm = 0x3FC 
    0x7ffff7f6f316 <+22>: jbe    0x7ffff7f6f330            ; <+48>
    0x7ffff7f6f318 <+24>: andq   $-0x4, %rax
(lldb) bt
error: need to add support for DW_TAG_base_type '()' encoded with DW_ATE = 0x7, bit_size = 0
* thread #2, name = 'subclass::objec', stop reason = signal SIGSEGV: invalid address (fault address: 0x1)
  * frame #0: 0x00007ffff7f6f30d libgobject-2.0.so.0`g_type_check_instance_is_fundamentally_a + 13
    frame #1: 0x00007ffff7f4fbde libgobject-2.0.so.0`g_object_ref + 14
    frame #2: 0x00005555555b2a9f glib-b0a38ca36f7dfe57`glib::subclass::types::ObjectSubclass::get_instance::hc977522c194b04a8(self=0x00005555557e7b08) at types.rs:304:12
    frame #3: 0x000055555566065a glib-b0a38ca36f7dfe57`_$LT$glib..subclass..object..test..SimpleObject$u20$as$u20$glib..subclass..object..ObjectImpl$GT$::constructed::h03ebf75be9b3b22b(self=0x00005555557e7b08, obj=0x00007ffff7a38b40) at object.rs:507:29
    frame #4: 0x0000555555646f16 glib-b0a38ca36f7dfe57`glib::subclass::object::constructed::h429c59a68160c4c0(obj=0x00005555557e7b40) at object.rs:114:4
    frame #5: 0x00007ffff7f508e6 libgobject-2.0.so.0`g_object_new_internal + 1270
    frame #6: 0x00007ffff7f5210d libgobject-2.0.so.0`g_object_newv + 685
    frame #7: 0x0000555555670820 glib-b0a38ca36f7dfe57`glib::object::Object::new::he71cb0db0aae219e(type_=Type @ 0x00007ffff7a38dd0, properties=&[(&str, &ToValue)] @ 0x00007ffff7a38ec0) at object.rs:1243:22
    frame #8: 0x000055555566111e glib-b0a38ca36f7dfe57`glib::subclass::object::test::test_create::hdf1c7b63350582d7 at object.rs:550:18
    frame #9: 0x00005555555b385a glib-b0a38ca36f7dfe57`glib::subclass::object::test::test_create::_$u7b$$u7b$closure$u7d$$u7d$::hd75406da7aa6e765((null)=0x00007ffff7a391b8) at object.rs:548:4
    frame #10: 0x00005555555d6e4e glib-b0a38ca36f7dfe57`core::ops::function::FnOnce::call_once::h6f8e1ccc8777885d((null)=closure-0 @ 0x00007ffff7a391b8, (null)=<unavailable>) at function.rs:232:4
    frame #11: 0x00005555556883df glib-b0a38ca36f7dfe57`_$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::hd59eaf1aa3e39474 at boxed.rs:1016:8
    frame #12: 0x00005555557344e7 glib-b0a38ca36f7dfe57`__rust_maybe_catch_panic at lib.rs:86:7
    frame #13: 0x00005555556a40f2 glib-b0a38ca36f7dfe57`test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::h42b032c21cc1ae65 [inlined] std::panicking::try::h3d3113d4174c87ff at panicking.rs:281:12
    frame #14: 0x00005555556a40a5 glib-b0a38ca36f7dfe57`test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::h42b032c21cc1ae65 [inlined] std::panic::catch_unwind::hab9bbd3b53a9e45a at panic.rs:394
    frame #15: 0x00005555556a40a5 glib-b0a38ca36f7dfe57`test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::h42b032c21cc1ae65 [inlined] test::run_test_in_process::h9b6ad3f84760e7c5 at lib.rs:542
    frame #16: 0x00005555556a4089 glib-b0a38ca36f7dfe57`test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::h42b032c21cc1ae65 at lib.rs:451
    frame #17: 0x000055555567b416 glib-b0a38ca36f7dfe57`std::sys_common::backtrace::__rust_begin_short_backtrace::h07b0076fdebe9589 at backtrace.rs:129:4
    frame #18: 0x00005555556800c6 glib-b0a38ca36f7dfe57`std::panicking::try::do_call::ha7aa02412f37f691 [inlined] std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::hc9f3cc384fd567c9 at mod.rs:475:16
    frame #19: 0x00005555556800b0 glib-b0a38ca36f7dfe57`std::panicking::try::do_call::ha7aa02412f37f691 [inlined] _$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h8c27547e77ca53c5 at panic.rs:318
    frame #20: 0x00005555556800b0 glib-b0a38ca36f7dfe57`std::panicking::try::do_call::ha7aa02412f37f691 at panicking.rs:303
    frame #21: 0x00005555557344e7 glib-b0a38ca36f7dfe57`__rust_maybe_catch_panic at lib.rs:86:7
    frame #22: 0x0000555555680c66 glib-b0a38ca36f7dfe57`core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h9241a82a54370334 [inlined] std::panicking::try::h8332343e01e2d282 at panicking.rs:281:12
    frame #23: 0x0000555555680c27 glib-b0a38ca36f7dfe57`core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h9241a82a54370334 [inlined] std::panic::catch_unwind::h20e99e197cc70de9 at panic.rs:394
    frame #24: 0x0000555555680c27 glib-b0a38ca36f7dfe57`core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h9241a82a54370334 [inlined] std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::h70832712d9677cb3 at mod.rs:474
    frame #25: 0x0000555555680bf3 glib-b0a38ca36f7dfe57`core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h9241a82a54370334 at function.rs:232
    frame #26: 0x000055555571e69f glib-b0a38ca36f7dfe57`_$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::hb394f6fd8b564f12 at boxed.rs:1016:8
    frame #27: 0x0000555555733600 glib-b0a38ca36f7dfe57`std::sys::unix::thread::Thread::new::thread_start::hf3c41fcf7f48ce57 [inlined] _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h6708b9fca8e9fd7d at boxed.rs:1016:8
    frame #28: 0x00005555557335f4 glib-b0a38ca36f7dfe57`std::sys::unix::thread::Thread::new::thread_start::hf3c41fcf7f48ce57 [inlined] std::sys_common::thread::start_thread::h50fcd2bcae595c39 at thread.rs:13
    frame #29: 0x000055555573357a glib-b0a38ca36f7dfe57`std::sys::unix::thread::Thread::new::thread_start::hf3c41fcf7f48ce57 at thread.rs:80
    frame #30: 0x00007ffff7de9fb7 libpthread.so.0`start_thread(arg=<unavailable>) at pthread_create.c:486:8
    frame #31: 0x00007ffff7cff2cf libc.so.6`__GI___clone at clone.S:95

valgrind says

==425930== Thread 2 subclass::object:
==425930== Invalid read of size 8
==425930==    at 0x48B830D: g_type_check_instance_is_fundamentally_a (gtype.c:4028)
==425930==    by 0x4898BDD: g_object_ref (gobject.c:3248)
==425930==    by 0x166A9E: glib::subclass::types::ObjectSubclass::get_instance (types.rs:304)
==425930==    by 0x214659: <glib::subclass::object::test::SimpleObject as glib::subclass::object::ObjectImpl>::constructed (object.rs:507)
==425930==    by 0x1FAF15: glib::subclass::object::constructed (object.rs:114)
==425930==    by 0x48998E5: g_object_new_internal (gobject.c:1867)
==425930==    by 0x489B10C: g_object_newv (gobject.c:2065)
==425930==    by 0x22481F: glib::object::Object::new (object.rs:1243)
==425930==    by 0x21511D: glib::subclass::object::test::test_create (object.rs:550)
==425930==    by 0x167859: glib::subclass::object::test::test_create::{{closure}} (object.rs:548)
==425930==    by 0x18AE4D: core::ops::function::FnOnce::call_once (function.rs:232)
==425930==    by 0x23C3DE: <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once (boxed.rs:1016)
==425930==  Address 0x1 is not stack'd, malloc'd or (recently) free'd
==425930== 
==425930== 
==425930== Process terminating with default action of signal 11 (SIGSEGV)
==425930==  Access not within mapped region at address 0x1
==425930==    at 0x48B830D: g_type_check_instance_is_fundamentally_a (gtype.c:4028)
==425930==    by 0x4898BDD: g_object_ref (gobject.c:3248)
==425930==    by 0x166A9E: glib::subclass::types::ObjectSubclass::get_instance (types.rs:304)
==425930==    by 0x214659: <glib::subclass::object::test::SimpleObject as glib::subclass::object::ObjectImpl>::constructed (object.rs:507)
==425930==    by 0x1FAF15: glib::subclass::object::constructed (object.rs:114)
==425930==    by 0x48998E5: g_object_new_internal (gobject.c:1867)
==425930==    by 0x489B10C: g_object_newv (gobject.c:2065)
==425930==    by 0x22481F: glib::object::Object::new (object.rs:1243)
==425930==    by 0x21511D: glib::subclass::object::test::test_create (object.rs:550)
==425930==    by 0x167859: glib::subclass::object::test::test_create::{{closure}} (object.rs:548)
==425930==    by 0x18AE4D: core::ops::function::FnOnce::call_once (function.rs:232)
==425930==    by 0x23C3DE: <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once (boxed.rs:1016)
@jonas-schievink jonas-schievink added C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 12, 2020
@sdroege
Copy link
Contributor Author

sdroege commented Feb 12, 2020

Needless to say that it might of course also be unsound unsafe code that is involved here, but this was working fine for years :)

@sdroege
Copy link
Contributor Author

sdroege commented Feb 12, 2020

This is actually a bug in the GLib bindings, I believe. Debugging a bit further now.

@sdroege
Copy link
Contributor Author

sdroege commented Feb 12, 2020

So basically what happened is that in one place we allocate memory for an Option<T> and in another place we use a pointer to that memory as a *const T and dereference it as such.

For reasons that I don't understand this worked before (I assume because the discriminant of the Option was stored at the end of the struct or moved into a niche inside one of the RefCells, see below?) but not anymore after the above change.

T in this testcase is

pub struct SimpleObject {
    name: RefCell<Option<String>>,
    constructed: RefCell<bool>,
}

@sdroege
Copy link
Contributor Author

sdroege commented Feb 12, 2020

See gtk-rs/glib#589 for the fix.

GuillaumeGomez pushed a commit to gtk-rs/gtk3-rs that referenced this issue Oct 28, 2020
We don't know the exact memory layout of the Option<T> in relation to T
so can't easily go from a *const T to the surrounding *const Option<T>.

Since nightly from 2020-02-12 using both pointer types interchangeably
causes segmentation faults as their memory layouts are not actually
compatible and never were, but due to some compiler changes this
actually causes crashes at runtime now.

See rust-lang/rust#69102 for details.

As an Option<_> was only used for some paranoid runtime checks that are
not really needed, simply store T directly as impl/private data of
subclasses.

This has the side-effect of having zero-sized impl/private data types to
actually occupy zero bytes of memory, and in some other cases to save a
few bytes of the allocation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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

2 participants