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

Upgrade rust to 58b83e7e740108611ad1e8286ee6b44ea5cbbb0f #5959

Closed

Conversation

@Manishearth
Copy link
Member

Manishearth commented May 6, 2015

Just edited the hash and ran ./mach update-cargo, plus some free warning fixes.

(Tue May 5 12:55:17 2015 +0000)

Review on Reviewable

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented May 6, 2015

Critic review: https://critic.hoppipolla.co.uk/r/4921

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@SimonSapin
Copy link
Member

SimonSapin commented May 6, 2015

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

📌 Commit a33aae0 has been approved by SimonSapin

@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

@jdm
Copy link
Member

jdm commented May 6, 2015

@Manishearth The libc removal will break the android build. There's a PR in the queue that does it properly.

@Manishearth Manishearth force-pushed the Manishearth:the_little_rustup_that_could branch from a33aae0 to 43a1bc8 May 6, 2015
@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

@bors-servo: r=SimonSapin

Undid the libc thingy

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

📌 Commit 43a1bc8 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

Testing commit 43a1bc8 with merge 407fe7a...

bors-servo pushed a commit that referenced this pull request May 6, 2015
…onSapin

Just edited the hash and ran `./mach update-cargo`, plus some free warning fixes.

(Tue May 5 12:55:17 2015 +0000)

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5959)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

💔 Test failed - mac1

@Manishearth Manishearth force-pushed the Manishearth:the_little_rustup_that_could branch from 43a1bc8 to 715a119 May 6, 2015
@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

@bors-servo: r=SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

📌 Commit 715a119 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

Testing commit 715a119 with merge 43d61ab...

bors-servo pushed a commit that referenced this pull request May 6, 2015
…onSapin

Just edited the hash and ran `./mach update-cargo`, plus some free warning fixes.

(Tue May 5 12:55:17 2015 +0000)

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5959)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

💔 Test failed - linux1

@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

@bors-servo: retry

@Ms2ger
Copy link
Contributor

Ms2ger commented May 6, 2015

@bors-servo: r-

../../tests/reftest.rs:88:9: 88:23 error: structure `test::TestOpts` has no field named `run_benchmarks`
../../tests/reftest.rs:88         run_benchmarks: false,
                                  ^~~~~~~~~~~~~~
../../tests/reftest.rs:88:9: 88:23 help: did you mean `bench_benchmarks`?
../../tests/reftest.rs:88         run_benchmarks: false,
                                  ^~~~~~~~~~~~~~
@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

Oh, I thought it had crashed -- I saw a purple

@Manishearth Manishearth force-pushed the Manishearth:the_little_rustup_that_could branch from 715a119 to 1cb1631 May 6, 2015
@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

@bors-servo: r=SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

📌 Commit 1cb1631 has been approved by SimonSapin

…2:55:17 2015 +0000)
@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

Testing commit 1cb1631 with merge 05213b9...

bors-servo pushed a commit that referenced this pull request May 6, 2015
…onSapin

Just edited the hash and ran `./mach update-cargo`, plus some free warning fixes.

(Tue May 5 12:55:17 2015 +0000)

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5959)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2015

💔 Test failed - linux1

@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

Opening wikipedia with gdb on gives me this:

#0  0x00005555571af3d8 in style::properties::cascade (viewport_size=..., applicable_declarations=..., 
    shareable=false, parent_style=..., cached_style=...)
    at target/debug/build/style-023f74d8196076da/out/properties.rs:16628
#1  0x00005555563b4637 in layout::css::matching::LayoutNode<'ln>.PrivateMatchMethods::cascade_node_pseudo_element
    (self=0x7fffcf3fc010, layout_context=0x7fffd53fc708, parent_style=..., applicable_declarations=..., 
    style=0x7fffd644a920, applicable_declarations_cache=0x7fffce838080, new_animations_sender=0x7fffd53fc880, 
    shareable=false) at /home/manishearth/Mozilla/voser/components/layout/css/matching.rs:457
#2  0x000055555639d2cd in layout::css::matching::LayoutNode<'ln>.MatchMethods::cascade_node (self=0x7fffcf3fc010, 
    layout_context=0x7fffd53fc708, parent=..., applicable_declarations=0x7fffcf3fbd60, 
    applicable_declarations_cache=0x7fffce838080, new_animations_sender=0x7fffd53fc880)
    at /home/manishearth/Mozilla/voser/components/layout/css/matching.rs:689
#3  0x000055555636fb21 in layout::traversal::RecalcStyleForNode<'a>.PreorderDomTraversal::process (
    self=0x7fffcf3fc220, node=...) at /home/manishearth/Mozilla/voser/components/layout/traversal.rs:181
#4  0x000055555636f3de in run_parallel_helper<layout::traversal::RecalcStyleForNode> (self=0x7fffcf3fc220, 
    unsafe_node=..., proxy=0x7fffcf3fc388, top_down_func=
    {void (struct (usize, usize), struct WorkerProxy<layout::context::SharedLayoutContextWrapper, (usize, usize)> *)} 0x7fffcf3fc1c0, bottom_up_func=
    {void (struct (usize, usize), struct WorkerProxy<layout::context::SharedLayoutContextWrapper, (usize, usize)> *)} 0x7fffcf3fc1b8) at /home/manishearth/Mozilla/voser/components/layout/parallel.rs:103
#5  layout::parallel::RecalcStyleForNode<'a>.ParallelPreorderDomTraversal::run_parallel (self=0x7fffcf3fc220, 
    unsafe_node=..., proxy=0x7fffcf3fc388) at /home/manishearth/Mozilla/voser/components/layout/parallel.rs:349
#6  0x000055555637027e in layout::parallel::recalc_style (unsafe_node=..., proxy=0x7fffcf3fc388)
    at /home/manishearth/Mozilla/voser/components/layout/parallel.rs:363
#7  0x0000555556303b8b in layout::workqueue::WorkerThread<QueueData, WorkData>::start (self=0x7fffcf3fc570)
    at /home/manishearth/Mozilla/voser/components/util/workqueue.rs:162
#8  0x00005555563033dc in fnfn () at /home/manishearth/Mozilla/voser/components/util/workqueue.rs:270
---Type <return> to continue, or q <return> to quit---
#9  0x000055555630321b in fnfn () at /home/manishearth/Mozilla/voser/components/util/task.rs:16
#10 0x0000555556303111 in layout::boxed::F.FnBox<A>::call_box (self=0x7fffd44283f0, args=0)
    at /home/larsberg/rust/src/liballoc/boxed.rs:374
#11 0x00005555562d8698 in layout::boxed::Box<FnBox<A, Output = R>+ Send + 'a>.FnOnce<A>::call_once (self=..., 
    args=0) at /home/larsberg/rust/src/liballoc/boxed.rs:390
#12 0x00005555562d7da4 in fnfn () at /home/larsberg/rust/src/libstd/thread/mod.rs:346
#13 0x00005555562d7d2f in layout::rt::unwind::try::try_fn<closure> (opt_closure=0x7fffcf3fc980)
    at /home/larsberg/rust/src/libstd/rt/unwind.rs:139
#14 0x00005555562d7cca in rt::unwind::try::try_fn::h7519854721141549484 ()
#15 0x0000555557840a19 in rust_try_inner ()
#16 0x0000555557840a06 in rust_try ()
#17 0x00005555562d71c3 in layout::rt::unwind::try<closure> (f={void (())} 0x7fffcf3fc9a0)
    at /home/larsberg/rust/src/libstd/rt/unwind.rs:125
#18 0x00005555562d6f91 in fnfn () at /home/larsberg/rust/src/libstd/thread/mod.rs:346
#19 0x00005555562d89be in layout::boxed::F.FnBox<A>::call_box (self=0x7fffd442f2d0, args=0)
    at /home/larsberg/rust/src/liballoc/boxed.rs:374
#20 0x000055555783a542 in sys::thread::Thread::new::thread_start::h4913e02ff25f93a3QIv ()
#21 0x00007ffff7bc4182 in start_thread (arg=0x7fffcf3fd700) at pthread_create.c:312
#22 0x00007ffff672147d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

gdb says that the program received a segfault, but when run directly I get "Illegal instruction (core dumped)". O.o

@SimonSapin
Copy link
Member

SimonSapin commented May 6, 2015

Line 16628 in the generated properties.rs is this one:

        let inherited_font_style = inherited_style.get_font();
        computed::Context {
            is_root_element: is_root_element,
            viewport_size: viewport_size,
            inherited_font_weight: inherited_font_style.font_weight,   #### Line 16628
            inherited_font_size: inherited_font_style.font_size,

In gdb, I get:

(gdb) p *inherited_style
$1 = {margin = {_ptr = {__0 = 0xec44300000000000}}, padding = {_ptr = {
      __0 = 0x7fff}}, border = {_ptr = {__0 = 0x0}}, outline = {_ptr = {
      __0 = 0x100000000}}, positionoffsets = {_ptr = {__0 = 0x0}}, box_ = {
    _ptr = {__0 = 0x0}}, inheritedbox = {_ptr = {__0 = 0x0}}, list = {_ptr = {
      __0 = 0x0}}, counters = {_ptr = {__0 = 0x0}}, background = {_ptr = {
      __0 = 0x100000000}}, color = {_ptr = {__0 = 0x0}}, font = {_ptr = {
      __0 = 0x0}}, inheritedtext = {_ptr = {__0 = 0x0}}, text = {_ptr = {
      __0 = 0x0}}, table = {_ptr = {__0 = 0x0}}, inheritedtable = {_ptr = {
      __0 = 0x0}}, pointing = {_ptr = {__0 = 0x0}}, column = {_ptr = {
      __0 = 0x0}}, effects = {_ptr = {__0 = 0x0}}, animation = {_ptr = {
      __0 = 0x0}}, shareable = false, writing_mode = {bits = 0 '\000'}, 
  root_font_size = {__0 = 0}}

All of these {_ptr = {__0 = …}} are of type Arc<_>, which is defined as:

pub struct Arc<T> {
    _ptr: NonZero<*mut ArcInner<T>>,
}

… so we have a NonZero<*mut _> which is zero. This sounds like bad news.

@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

@Manishearth
Copy link
Member Author

Manishearth commented May 6, 2015

If everything is zero (it isn't, it almost is) it might simply be an issue of gdb not doing line numbers correctly and the variable is being accessed before it is initialized.

We still have a segfault, though.

@jdm jdm added S-tests-failed and removed S-awaiting-review labels May 6, 2015
@SimonSapin
Copy link
Member

SimonSapin commented May 6, 2015

I’m skeptical that inherited_style is not initialized yet, as it’s either set from the INITIAL_VALUES lazy_static which uses a lock internally for initialization, or from the parent_style parameter. But I don’t know.

@Manishearth
Copy link
Member Author

Manishearth commented May 7, 2015

I get less zeroes

$2 = {margin = {_ptr = {__0 = 0x5633e73b00002aaa}}, padding = {_ptr = {__0 = 0x5555}}, border = {_ptr = {
      __0 = 0x200000000}}, outline = {_ptr = {__0 = 0x0}}, positionoffsets = {_ptr = {__0 = 0x0}}, box_ = {
    _ptr = {__0 = 0xe626416100000000}}, inheritedbox = {_ptr = {__0 = 0xed4444d4000002d8}}, list = {_ptr = {
      __0 = 0xed4444d400002aaa}}, counters = {_ptr = {__0 = 0x1d1d1d1d00002aaa}}, background = {_ptr = {
      __0 = 0xb7761a701d1d1d1d}}, color = {_ptr = {__0 = 0x2aaa}}, font = {_ptr = {__0 = 0xed4444b000000000}}, 
  inheritedtext = {_ptr = {__0 = 0xed4444a800002aaa}}, text = {_ptr = {__0 = 0x1d1d1d1d00002aaa}}, table = {
    _ptr = {__0 = 0x1d1d1d1d1d1d1d1d}}, inheritedtable = {_ptr = {__0 = 0x1d1d1d1d1d1d1d1d}}, pointing = {_ptr = {
      __0 = 0x1d1d1d1d1d1d1d1d}}, column = {_ptr = {__0 = 0x1d1d1d1d1d1d1d1d}}, effects = {_ptr = {
      __0 = 0xed4444a81d1d1d1d}}, animation = {_ptr = {__0 = 0xb776123000002aaa}}, shareable = 170, 
  writing_mode = {Python Exception <class 'gdb.error'> Argument to arithmetic operation not a number or boolean.: 
bits = 42 '*'}, root_font_size = }

This is with rr

@Manishearth
Copy link
Member Author

Manishearth commented May 7, 2015

On the other hand,

(gdb) p *inherited_font_style 
Cannot access memory at address 0xed4444b000000010
@Manishearth
Copy link
Member Author

Manishearth commented May 7, 2015

The first time around, inherited_font_style is fine, but the next time it hits that breakpoint I get the above error.

@bors-servo
Copy link
Contributor

bors-servo commented May 7, 2015

The latest upstream changes (presumably #5968) made this pull request unmergeable. Please resolve the merge conflicts.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented May 7, 2015

Do we need to find somebody on Rust to help us debug this? I'm not sure what our next steps are, but being blocked on an update because rustc is now generating invalid code seems bad.

@Manishearth
Copy link
Member Author

Manishearth commented May 7, 2015

At the moment; It's blocking me from finalizing the sizeof plugin, but that plugin isn't a blocking one; it just makes stuff easier for njn.

So low-pri to fix, but it's going to hit us eventually, so we should get it fixed :)

@michaelwu
Copy link
Contributor

michaelwu commented May 19, 2015

diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs
index 4615e0e..720a8b2 100644
--- a/components/script/dom/node.rs
+++ b/components/script/dom/node.rs
@@ -249,7 +249,7 @@ impl LayoutDataRef {
     #[inline]
     #[allow(unsafe_code)]
     pub unsafe fn borrow_unchecked(&self) -> *const Option<LayoutData> {
-        mem::transmute(&self.data_cell)
+        self.data_cell.as_unsafe_cell().get() as *const _
     }

     /// Borrows the layout data immutably. This function is *not* thread-safe.
@SimonSapin
Copy link
Member

SimonSapin commented May 19, 2015

@Ms2ger
Copy link
Contributor

Ms2ger commented May 20, 2015

FWIW, the issue was that the layout of RefCell changed to support unsized types.

@Manishearth
Copy link
Member Author

Manishearth commented May 20, 2015

Ohhhhhhhh

methinks we need to do another transmute audit

@mbrubeck
Copy link
Contributor

mbrubeck commented May 20, 2015

Moved to #6149.

@mbrubeck mbrubeck closed this May 20, 2015
@Manishearth Manishearth deleted the Manishearth:the_little_rustup_that_could branch Sep 3, 2015
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.

None yet

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