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.

Sign up for GitHub

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

Jump to bottom

Update libc and some fixes for x86_64-unknown-linux-gnux32 #45391

Merged
merged 2 commits into from
Oct 21, 2017
Merged

Update libc and some fixes for x86_64-unknown-linux-gnux32 #45391

merged 2 commits into from
Oct 21, 2017

Conversation

malbarbo
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@malbarbo
Copy link
Contributor Author

The libstd tests fails in my machine, let's see on travis.

alexcrichton
alexcrichton reviewed Oct 19, 2017
View reviewed changes
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

src/libcore/tests/hash/sip.rs Outdated
@@ -249,12 +249,21 @@ fn test_hash_usize() {
assert!(hash(&(val as u64)) != hash(&(val as usize)));
assert_eq!(hash(&(val as u32)), hash(&(val as usize)));
}
#[test] #[cfg(target_arch = "x86_64")]

#[test] #[cfg(all(target_arch = "x86_64", target_pointer_width = "32"))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've got existing tests for x86/x86_64, perhaps those could just switch to being two tests based on target_pointer_width?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/libstd/sys/unix/condvar.rs Outdated
@@ -92,7 +92,16 @@ impl Condvar {
assert_eq!(r, 0);

// Nanosecond calculations can't overflow because both values are below 1e9.
let nsec = dur.subsec_nanos() as libc::c_long + now.tv_nsec as libc::c_long;
// HACK: add support to linux x32, the original code was
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary b/c the values are always < 1e9? Doesn't that mean we can basically cast to "anything reasonable" here and expect it to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use i32?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that could work? It just seemed like the extra casting wasn't necessary here given the various constraints.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As subsec_nanos is u32, I opted to use u32. Both i32 and u32 are fine.

src/ci/docker/x86_64-gnu/Dockerfile Outdated
ENV HOSTS=x86_64-unknown-linux-gnu

ENV TARGETS=x86_64-unknown-linux-gnu
ENV TARGETS=$TARGETS,x86_64-unknown-linux-gnux32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you plan on leaving this permanently? It looks like we don't have a lot of extra time on this builder unfortunately (the last one clocked in at 1h40m)...

I'd imagine though that once we get the test suite passing once it's probably pretty unlikely to regress?

@alexcrichton
Copy link
Member

Looks like maybe something related to stack walking is odd?

[01:12:28]     [run-pass] run-pass/backtrace-debuginfo.rs

[01:12:28]     [run-pass] run-pass/backtrace.rs

[01:12:28]     [run-pass] run-pass/command-before-exec.rs

[01:12:28]     [run-pass] run-pass/command-exec.rs

[01:12:28]     [run-pass] run-pass/out-of-stack.rs

[01:12:28]     [run-pass] run-pass/panic-runtime/lto-unwind.rs

[01:12:28]     [run-pass] run-pass/stack-probes-lto.rs

[01:12:28]     [run-pass] run-pass/stack-probes.rs

[01:12:28]     [run-pass] run-pass/wait-forked-but-failed-child.rs

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 20, 2017
@malbarbo malbarbo mentioned this pull request Oct 20, 2017
Open
@malbarbo
Copy link
Contributor Author

@alexcrichton I squashed the commits and disable the gnux32 tests. I also opened the issue #45416 to track the failures.

@alexcrichton
Copy link
Member

Ok! Is this ready to go? (did you mean to add ignore annotations to all affected tests?)

@malbarbo malbarbo mentioned this pull request Oct 20, 2017
Closed
@malbarbo
Copy link
Contributor Author

I think this is ready to go. This will allows others to test the target with the fixed libc.

I think you are correct about the stack walk odds. I will be out for some time now, but it would be great if someone could take a look at it.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 20, 2017
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Oct 20, 2017

📌 Commit e57ee3d has been approved by alexcrichton

@kennytm kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 20, 2017
@Mark-Simulacrum
Copy link
Member

Is this intended to fix #45417, #45416 and perhaps others?

@bors
Copy link
Collaborator

bors commented Oct 21, 2017

⌛ Testing commit e57ee3d with merge d532ba7...

bors added a commit that referenced this pull request Oct 21, 2017
@bors
Auto merge of #45391 - malbarbo:x32-1, r=alexcrichton
d532ba7 
Update libc and some fixes for x86_64-unknown-linux-gnux32
@bors
Copy link
Collaborator

bors commented Oct 21, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing d532ba7 to master...

@bors bors merged commit e57ee3d into rust-lang:master Oct 21, 2017
@bors bors mentioned this pull request Oct 21, 2017
Closed
@malbarbo
Copy link
Contributor Author

Is this intended to fix #45417, #45416 and perhaps others?

No. 😞

@malbarbo malbarbo deleted the x32-1 branch October 25, 2017 16:37
@malbarbo malbarbo restored the x32-1 branch October 25, 2017 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@malbarbo @rust-highfive @alexcrichton @bors @Mark-Simulacrum @kennytm