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

Use new std::num::NonZero* types instead of deprecated core::nonzero::NonZero #20474

Merged
merged 2 commits into from
Apr 9, 2018

Conversation

SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Mar 29, 2018

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify script code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 29, 2018
@nox
Copy link
Contributor

nox commented Mar 29, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 970519f has been approved by nox

@highfive highfive assigned nox and unassigned pcwalton Mar 29, 2018
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 29, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 970519f with merge 51dbbbe...

bors-servo pushed a commit that referenced this pull request Mar 29, 2018
 Use new std::num::NonZero* types instead of deprecated core::nonzero::NonZero

<!-- 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/20474)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-dev

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 29, 2018
@CYBAI
Copy link
Member

CYBAI commented Mar 29, 2018

Checking files for tidiness...

./components/nonzero/lib.rs:31: derivable traits list is not in alphabetical order
	expected: Clone, Copy, Eq, Hash, Ord, PartialEq, PartialOrd
	found: Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash
error[E0425]: cannot find function `NonZero` in this scope
   --> components/nonzero/lib.rs:60:34
    |
60  |                           Some($Ty(NonZero(n)))
    |                                    ^^^^^^^ did you mean `NonZeroI8`?
...
106 | / nonzero_integers! {
107 | |     NonZeroU8(u8); NonZeroI8(i8);
108 | |     NonZeroU16(u16); NonZeroI16(i16);
109 | |     NonZeroU32(u32); NonZeroI32(i32);
110 | |     NonZeroU64(u64); NonZeroI64(i64);
111 | |     NonZeroUsize(usize); NonZeroIsize(isize);
112 | | }
    | |_- in this macro invocation

error: aborting due to previous error

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Mar 29, 2018
@SimonSapin
Copy link
Member Author

@bors-servo r=nox

@bors-servo
Copy link
Contributor

📌 Commit 7c73d98 has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 29, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 7c73d98 with merge a4896a3...

bors-servo pushed a commit that referenced this pull request Mar 29, 2018
 Use new std::num::NonZero* types instead of deprecated core::nonzero::NonZero

<!-- 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/20474)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-css

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 29, 2018
@CYBAI
Copy link
Member

CYBAI commented Mar 29, 2018

   Compiling servo_rand v0.0.1 (file:///home/servo/buildbot/slave/linux-rel-css/build/components/rand)
error: couldn't read "components/nonzero/lib.rs": No such file or directory (os error 2)

error: aborting due to previous error

@SimonSapin
Copy link
Member Author

In debug mode:

target/debug/servo http://127.0.0.1:8000/mozilla/bluetooth/getPrimaryService/service-found.html
H��@]�f (thread main, at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.11.2/src/platform/macos/window.rs:349)
Stack trace for thread "main"
stack backtrace:
   0:        0x10fafc0e4 - backtrace::backtrace::trace::h27367cd9586407d3
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.2/src/backtrace/mod.rs:42
   1:        0x10faf652c - backtrace::capture::Backtrace::new::he2d8ba3eb3fdde01
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.2/src/capture.rs:64
   2:        0x10951561c - servo::install_crash_handler::handler::h6675e611472db937
                        at ports/servo/main.rs:82
   3:     0x7fffb6db1b39 - _sigtramp
   4:     0x7fffb6db4342 - os_unfair_lock_lock
   5:        0x10cd97719 - je_malloc_mutex_lock
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/include/jemalloc/internal/mutex.h:97
   6:        0x10cd9d99b - je_arena_dalloc_large
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/src/arena.c:3075
   7:        0x10cd907af - isfree
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/src/jemalloc.c:1921
   8:        0x10cd9008f - _rjem_sdallocx
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/src/jemalloc.c:2667
   9:        0x10cd7bd98 - _$LT$$RF$$u27$a$u20$jemallocator..Jemalloc$u20$as$u20$core..heap..Alloc$GT$::dealloc::h2f133a6ba958cfee
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemallocator-0.1.4/src/lib.rs:163
  10:        0x10cd7b6d3 - __rg_dealloc
                        at components/allocator/lib.rs:12
  11:        0x10fabcf1d - _$LT$alloc..heap..Heap$u20$as$u20$core..heap..Alloc$GT$::dealloc::h78de2ab4fa4d4982
                        at /Users/travis/build/rust-lang/rust/src/liballoc/heap.rs:104
  12:        0x10fabbb11 - _$LT$alloc..raw_vec..RawVec$LT$T$C$$u20$A$GT$$GT$::dealloc_buffer::hf6c85c959458a4a3
                        at /Users/travis/build/rust-lang/rust/src/liballoc/raw_vec.rs:706
  13:        0x10fabc2f4 - _$LT$alloc..raw_vec..RawVec$LT$T$C$$u20$A$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::hdb0e899e35ead3df
                        at /Users/travis/build/rust-lang/rust/src/liballoc/raw_vec.rs:715
  14:        0x10faa3484 - core::ptr::drop_in_place::h07410edd4aff778c
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  15:        0x10faa3eb1 - core::ptr::drop_in_place::ha2abef70c0f2d71f
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  16:        0x10faa35f4 - core::ptr::drop_in_place::h22d72141708999a0
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  17:        0x10faa3e64 - core::ptr::drop_in_place::h9c84a53396c6010e
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  18:        0x10fa8c58a - winit::platform::platform::window::Window2::new::he77f2f7b30d50fcf
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.11.2/src/platform/macos/window.rs:349
  19:        0x10fa9f290 - winit::platform::platform::Window::new::h0b5e7ddefbafe0fa
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.11.2/src/platform/macos/mod.rs:32
  20:        0x10faa22bf - winit::window::_$LT$impl$u20$winit..WindowBuilder$GT$::build::h127ac10da84ab8d3
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.11.2/src/window.rs:119
  21:        0x10fa83673 - glutin::platform::platform::Context::new::hfeb241f3554e561e
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/glutin-0.13.1/src/platform/macos/mod.rs:48
  22:        0x10fa82a15 - glutin::GlWindow::new::h877de867d2c59403
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/glutin-0.13.1/src/lib.rs:313
  23:        0x10959c35f - servo::glutin_app::window::Window::new::hb85e1398e6182e7a
                        at ports/servo/glutin_app/window.rs:214
  24:        0x10956b367 - servo::glutin_app::create_window::h9181bd5d87834b8f
                        at ports/servo/glutin_app/mod.rs:19
  25:        0x109515ac9 - servo::main::h667fa9fabd8218de
                        at ports/servo/main.rs:157
  26:        0x10956f0f1 - std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::h307f18a20c6dd54b
                        at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  27:        0x10fb22597 - std::panicking::try::do_call::h2ff348095a84f7a4
                        at libstd/panicking.rs:305
  28:        0x10fb3a3be - __rust_maybe_catch_panic
                        at libpanic_unwind/lib.rs:102
  29:        0x10fb1e187 - std::rt::lang_start_internal::h065a6bf33dd96053
                        at libstd/panicking.rs:284
  30:        0x10956f0d1 - std::rt::lang_start::hcce6b761402d0017
                        at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  31:        0x109516434 - main
Stack trace for thread "main"
stack backtrace:
   0:        0x10fafc0e4 - backtrace::backtrace::trace::h27367cd9586407d3
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.2/src/backtrace/mod.rs:42
   1:        0x10faf652c - backtrace::capture::Backtrace::new::he2d8ba3eb3fdde01
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.2/src/capture.rs:64
   2:        0x10951561c - servo::install_crash_handler::handler::h6675e611472db937
                        at ports/servo/main.rs:82
   3:     0x7fffb6db1b39 - _sigtramp
   4:        0x10951572a - servo::install_crash_handler::handler::h6675e611472db937
                        at ports/servo/main.rs:86
   5:     0x7fffb6db1b39 - _sigtramp
   6:     0x7fffb6db4342 - os_unfair_lock_lock
   7:        0x10cd97719 - je_malloc_mutex_lock
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/include/jemalloc/internal/mutex.h:97
   8:        0x10cd9d99b - je_arena_dalloc_large
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/src/arena.c:3075
   9:        0x10cd907af - isfree
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/src/jemalloc.c:1921
  10:        0x10cd9008f - _rjem_sdallocx
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemalloc-sys-0.1.4/jemalloc/src/jemalloc.c:2667
  11:        0x10cd7bd98 - _$LT$$RF$$u27$a$u20$jemallocator..Jemalloc$u20$as$u20$core..heap..Alloc$GT$::dealloc::h2f133a6ba958cfee
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/jemallocator-0.1.4/src/lib.rs:163
  12:        0x10cd7b6d3 - __rg_dealloc
                        at components/allocator/lib.rs:12
  13:        0x10fabcf1d - _$LT$alloc..heap..Heap$u20$as$u20$core..heap..Alloc$GT$::dealloc::h78de2ab4fa4d4982
                        at /Users/travis/build/rust-lang/rust/src/liballoc/heap.rs:104
  14:        0x10fabbb11 - _$LT$alloc..raw_vec..RawVec$LT$T$C$$u20$A$GT$$GT$::dealloc_buffer::hf6c85c959458a4a3
                        at /Users/travis/build/rust-lang/rust/src/liballoc/raw_vec.rs:706
  15:        0x10fabc2f4 - _$LT$alloc..raw_vec..RawVec$LT$T$C$$u20$A$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::hdb0e899e35ead3df
                        at /Users/travis/build/rust-lang/rust/src/liballoc/raw_vec.rs:715
  16:        0x10faa3484 - core::ptr::drop_in_place::h07410edd4aff778c
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  17:        0x10faa3eb1 - core::ptr::drop_in_place::ha2abef70c0f2d71f
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  18:        0x10faa35f4 - core::ptr::drop_in_place::h22d72141708999a0
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  19:        0x10faa3e64 - core::ptr::drop_in_place::h9c84a53396c6010e
                        at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
  20:        0x10fa8c58a - winit::platform::platform::window::Window2::new::he77f2f7b30d50fcf
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.11.2/src/platform/macos/window.rs:349
  21:        0x10fa9f290 - winit::platform::platform::Window::new::h0b5e7ddefbafe0fa
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.11.2/src/platform/macos/mod.rs:32
  22:        0x10faa22bf - winit::window::_$LT$impl$u20$winit..WindowBuilder$GT$::build::h127ac10da84ab8d3
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.11.2/src/window.rs:119
  23:        0x10fa83673 - glutin::platform::platform::Context::new::hfeb241f3554e561e
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/glutin-0.13.1/src/platform/macos/mod.rs:48
  24:        0x10fa82a15 - glutin::GlWindow::new::h877de867d2c59403
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/glutin-0.13.1/src/lib.rs:313
  25:        0x10959c35f - servo::glutin_app::window::Window::new::hb85e1398e6182e7a
                        at ports/servo/glutin_app/window.rs:214
  26:        0x10956b367 - servo::glutin_app::create_window::h9181bd5d87834b8f
                        at ports/servo/glutin_app/mod.rs:19
  27:        0x109515ac9 - servo::main::h667fa9fabd8218de
                        at ports/servo/main.rs:157
  28:        0x10956f0f1 - std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::h307f18a20c6dd54b
                        at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  29:        0x10fb22597 - std::panicking::try::do_call::h2ff348095a84f7a4
                        at libstd/panicking.rs:305
  30:        0x10fb3a3be - __rust_maybe_catch_panic
                        at libpanic_unwind/lib.rs:102
  31:        0x10fb1e187 - std::rt::lang_start_internal::h065a6bf33dd96053
                        at libstd/panicking.rs:284
  32:        0x10956f0d1 - std::rt::lang_start::hcce6b761402d0017
                        at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  33:        0x109516434 - main

@SimonSapin
Copy link
Member Author

With ASAN:

RUSTFLAGS="-Z sanitizer=address" cargo build --target x86_64-apple-darwin
target/x86_64-apple-darwin/debug/servo http://127.0.0.1:8000/mozilla/bluetooth/getPrimaryService/service-found.html 
=================================================================
==96861==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00011c6b6c80 at pc 0x000127dd04e3 bp 0x7fff5d5217b0 sp 0x7fff5d520f50
READ of size 4337242689 at 0x00011c6b6c80 thread T0
    #0 0x127dd04e2 in wrap_memcpy (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x1c4e2)
    #1 0x11b4d57e8 in _$LT$core..fmt..Write..write_fmt..Adapter$LT$$u27$a$C$$u20$T$GT$$u20$as$u20$core..fmt..Write$GT$::write_str::h6bf4660ec65f883c mod.rs:214
    #2 0x11b3623d1 in _$LT$alloc..string..String$u20$as$u20$core..fmt..Display$GT$::fmt::h0bd2f935f36770e1 string.rs:1859
    #3 0x11b375157 in _$LT$objc..message..MessageError$u20$as$u20$core..fmt..Display$GT$::fmt::h4822abf5e3533712 mod.rs:157
    #4 0x11b4fcea8 in core::fmt::write::h46a35b62c3a6e274 mod.rs:1099
    #5 0x11b4d8763 in std::panicking::begin_panic_fmt::h29ede29c4b1288f2 mod.rs:226
    #6 0x11b23d539 in winit::platform::platform::window::Window2::new::hb879fdfe505bac55 window.rs:349
    #7 0x11b28d199 in winit::platform::platform::Window::new::h4d49af5e3b2259cd mod.rs:32
    #8 0x11b29a7c0 in winit::window::_$LT$impl$u20$winit..WindowBuilder$GT$::build::hfab1f6b86faf77ba window.rs:119
    #9 0x11b206c64 in glutin::platform::platform::Context::new::h155af04d779e2f8e mod.rs:48
    #10 0x11b20224a in glutin::GlWindow::new::h40d46f9b1b111ca8 lib.rs:313
    #11 0x10290f87f in servo::glutin_app::window::Window::new::h0107390f68ff169d window.rs:214
    #12 0x10283f55a in servo::glutin_app::create_window::hd287eb30fc2734ad mod.rs:19
    #13 0x1026d8ef3 in servo::main::h5c01dea7f3af6fed main.rs:157
    #14 0x1028512cd in std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::hcb1b98a46ea3a9e7 rt.rs:74
    #15 0x11b4d8647 in std::panicking::try::do_call::h2ff348095a84f7a4 panicking.rs:305
    #16 0x11b4f06ae in __rust_maybe_catch_panic lib.rs:102
    #17 0x11b4d4217 in std::rt::lang_start_internal::h065a6bf33dd96053 panicking.rs:284
    #18 0x102851240 in std::rt::lang_start::ha326545e93f432ab rt.rs:74
    #19 0x1026dadc4 in main (servo:x86_64+0x100005dc4)
    #20 0x7fffb6ba2234 in start (libdyld.dylib:x86_64+0x5234)

0x00011c6b6c80 is located 32 bytes to the left of global variable 'byte_str.1' defined in '1598q11f1nfieoq0-216cf653db547e4bd5a0e92dc632dff6.rs' (0x11c6b6ca0) of size 11
0x00011c6b6c80 is located 0 bytes to the right of global variable 'byte_str.0' defined in '1598q11f1nfieoq0-216cf653db547e4bd5a0e92dc632dff6.rs' (0x11c6b6c80) of size 0
  'byte_str.0' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x1c4e2) in wrap_memcpy
Shadow bytes around the buggy address:
  0x1000238d6d40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000238d6d50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000238d6d60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000238d6d70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000238d6d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x1000238d6d90:[f9]f9 f9 f9 00 03 f9 f9 f9 f9 f9 f9 01 f9 f9 f9
  0x1000238d6da0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000238d6db0: 00 00 00 00 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x1000238d6dc0: 00 00 00 02 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x1000238d6dd0: f9 f9 f9 f9 00 06 f9 f9 f9 f9 f9 f9 00 03 f9 f9
  0x1000238d6de0: f9 f9 f9 f9 00 00 03 f9 f9 f9 f9 f9 00 00 03 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==96861==ABORTING
Stack trace for thread "main"
stack backtrace:
   0:        0x11b479eb0 - backtrace::backtrace::trace::ha0855a09de2a3534
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.2/src/backtrace/mod.rs:42
   1:        0x11b45df2d - backtrace::capture::Backtrace::new::h0a60662aedf1b95d
                        at /Users/simon/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.2/src/capture.rs:64
   2:        0x1026d7a3b - servo::install_crash_handler::handler::heff262e055174113
                        at ports/servo/main.rs:82
   3:     0x7fffb6db1b39 - _sigtramp

@SimonSapin
Copy link
Member Author

    #6 0x11b23d539 in winit::platform::platform::window::Window2::new::hb879fdfe505bac55 window.rs:349

This is https://github.com/tomaka/winit/blob/51181b4347aed6f7c1a384c6810635bd6801b369/src/platform/macos/window.rs#L349-L350

            // register for drag and drop operations.
            msg_send![(*window as id),
registerForDraggedTypes:NSArray::arrayWithObject(nil, appkit::NSFilenamesPboardType)];

I couldn’t figure out how to make gdb work on OS X, so in lldb, breakpoint set --file window.rs --line 342 gets up close to before this line.

The msg_send! macro expands to: https://github.com/SSheldon/rust-objc/blob/0.2.2/src/macros.rs#L81-L85

        let sel = sel!($($name:)+);
        match $crate::__send_message(&*$obj, sel, ($($arg,)*)) {
            Err(s) => panic!("{}", s),
            Ok(r) => r,
        }

Note that the next frame in the ASAN stack trace is std::panicking::begin_panic_fmt, so it looks like we’re getting an Err(s) value here and panicking. That error comes from $crate::__send_message which is a reexport of https://github.com/SSheldon/rust-objc/blob/0.2.2/src/message/mod.rs#L194-L198

pub unsafe fn send_message<T, A, R>(obj: *const T, sel: Sel, args: A)
        -> Result<R, MessageError>
        where T: Message, A: MessageArguments, R: Any {
    send_unverified(obj, sel, args)
}

… which itself is based (in the same file) on https://github.com/SSheldon/rust-objc/blob/0.2.2/src/message/mod.rs#L167-L189

#[cfg(feature = "exception")]
macro_rules! objc_try {
    ($b:block) => (
        $crate::exception::try(|| $b).map_err(|exception| match exception {
            Some(exception) => MessageError(format!("Uncaught exception {:?}", &*exception)),
            None => MessageError("Uncaught exception nil".to_owned()),
        })
    )
}

#[cfg(not(feature = "exception"))]
macro_rules! objc_try {
    ($b:block) => (Ok($b))
}

unsafe fn send_unverified<T, A, R>(obj: *const T, sel: Sel, args: A)
        -> Result<R, MessageError>
        where T: Message, A: MessageArguments, R: Any {
    let (msg_send_fn, receiver) = msg_send_fn::<R>(obj as *mut T as *mut Object, sel);
    objc_try!({
        A::invoke(msg_send_fn, receiver, sel, args)
    })
}

I don’t see $crate::exception::try at all when stepping through lldb, so it looks like we’re in the no-exception case. So this function should always return Ok. Yet we established earlier that it returns Err!

I haven’t managed to reliably get lldb to print return values, but a few times I got things like:

Return value: (core::result::Result<!, objc::message::MessageError>) $0 = {
   = {
    __0 = {
      __0 = {
        vec = {
          buf = {
            ptr = {
              pointer = (__0 = "H\xffffff83\xffffffc4@]\xffffffc3f\x0f\x1fD")
            }
            cap = 140734799785904
          }
          len = 8
        }
      }
    }
  }
}

… which says that in our Result<R, MessageError> return type, R is the "never" type ! and should be impossible. Yet, through transmuting (elsewhere in this file) a function pointer to unsafe extern fn(*mut Object, Sel $(, $t)*) -> R we’re "creating" a value of this type anyway and presumably causing undefined behavior.

Now, why is R resolved to !? That value of type R ends up being "returned" by the msg_send! macro, but in this case it’s not used at all: msg_send![/*…*/];. The R generic type parameter is completely unspecified. I would expect this to cause a type inference error. It does in the simplest case:

fn main() {
    foo();
}

fn foo<R>() -> R {
    unimplemented!()
}
error[E0282]: type annotations needed
 --> a.rs:2:5
  |
2 |     foo();
  |     ^^^ cannot infer type for `R`

error: aborting due to previous error

… but not when the type parameter is used inside of a Result:

#![feature(core_intrinsics)]

fn main() {
    match send_message() {
        Ok(r) => r,
        Err(name) => println!("Inferred type: {}", name),
    };
}

fn send_message<R>() -> Result<R, &'static str> {
    unsafe {
        Err(std::intrinsics::type_name::<R>())
    }
}
Inferred type: ()

I don’t know why R is inferred to be ! in winit but () in my example.

Indeed, the doc-comment of the msg_send! macro has an example like that adds a type annotation https://github.com/SSheldon/rust-objc/blob/0.2.2/src/macros.rs#L52

let _: () = msg_send![obj, setArg1:1 arg2:2];

This is probably the correct fix, but rustc is still doing something surprising here…

@SimonSapin
Copy link
Member Author

Based on IRC chat with Eddyb the relevant rustc change is most likely the stabilization of the ! type which includes changing the default type used in unreachable code from () to !. If I change my example to panic like msg_send! does, I can reproduce R = ! being inferred:

#![feature(core_intrinsics)]

fn main() {
    match send_message() {
        Ok(r) => r,
        Err(name) => panic!("Inferred type: {}", name),
    };
}

fn send_message<R>() -> Result<R, &'static str> {
    unsafe {
        Err(std::intrinsics::type_name::<R>())
    }
}
thread 'main' panicked at 'Inferred type: !', a.rs:6:22
…

SimonSapin added a commit to SimonSapin/winit that referenced this pull request Apr 9, 2018
The error I was investigating servo/servo#20474 (comment) turned out to be already be fixed by rust-windowing#428, but there was a few more cases of the same problem.
SimonSapin added a commit to SimonSapin/winit that referenced this pull request Apr 9, 2018
The error I was investigating servo/servo#20474 (comment) turned out to be already be fixed by rust-windowing#428, but there was a few more cases of the same problem.
bors-servo pushed a commit to servo/devices that referenced this pull request Apr 9, 2018
Fix some unconstrained type parameters being inferred to `!`

Similar to rust-windowing/winit#428
CC servo/servo#20474 (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/devices/26)
<!-- Reviewable:end -->
@SimonSapin
Copy link
Member Author

The winit bug turned out to have been already fixed upstream rust-windowing/winit#428, but we had more of the same in blurmac: servo/devices#26

@bors-servo r=nox

@bors-servo
Copy link
Contributor

📌 Commit 52dceb3 has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 9, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 52dceb3 with merge bfb9fe6...

bors-servo pushed a commit that referenced this pull request Apr 9, 2018
 Use new std::num::NonZero* types instead of deprecated core::nonzero::NonZero

<!-- 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/20474)
<!-- Reviewable:end -->
francesca64 pushed a commit to rust-windowing/winit that referenced this pull request Apr 9, 2018
The error I was investigating servo/servo#20474 (comment) turned out to be already be fixed by #428, but there was a few more cases of the same problem.
@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: nox
Pushing bfb9fe6 to master...

@bors-servo bors-servo merged commit 52dceb3 into master Apr 9, 2018
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 9, 2018
@SimonSapin SimonSapin deleted the nonzero branch April 10, 2018 06:38
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

Successfully merging this pull request may close these issues.

8 participants