Skip to content

/rust

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

Implement basic NetBSD/amd64 support #28543

Merged
merged 2 commits into from Sep 22, 2015

Conversation

Reviewers
No reviews
Assignees

@huonw huonw

Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@gandro @rust-highfive @eefriedman @alexcrichton @bors @huonw
@gandro
Copy link
Contributor

gandro commented Sep 20, 2015

These changes introduce the ability to cross-compile working binaries for NetBSD/amd64. Previous support added in PR #26682 shared all its code with the OpenBSD implementation, and was therefore never functional (e.g. linking against non-existing symbols and using wrong type definitions). Nonetheless, the previous patches were a great starting point and made my work significantly easier. 😃

Because there are no stage0 snapshots for NetBSD (yet), I used a cross-compiler for NetBSD 7.0 RC3 and only tested some toy programs (threading and channels, stack guards, a small TCP/IP echo server and some other platform dependent bits). If someone could point me to documentation on how to generate a stage0 snapshot from a cross-compiler I'm happy to run the full test suite.

A few other notes regarding Rust on NetBSD/amd64:

  • To preserve binary compatibility, NetBSD introduces new symbols for system call wrappers on breaking ABI changes and keeps the old (legacy) symbols around, see this documentation for some details. I went ahead and modified the libc and std crate to use the current (renamed) symbols instead of the legacy ones where I found them, but I might have missed some. Notably using the sigaction symbol (deprecated in 1998) instead of __sigaction14 even triggers SIGSYS (bad syscall) on my amd64 setup. I also changed the type definitions to use the most recent version.
  • NetBSD's gdb doesn't really support position independent executables, so you might want to turn that off for debugging, see NetBSD Problem Report #48250.
  • For binaries invoked using a relative path, NetBSD supports $ORIGIN only for short rpaths (~64 chars or so, I'm told). If running an executable fails with execname not specified in AUX vector: No such file or directory, consider invoking the binary using its full absolute path.

@rust-highfive rust-highfive assigned huonw Sep 20, 2015

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Sep 20, 2015

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.
@eefriedman

This comment has been minimized.

Sign in to view
Copy link
Contributor

eefriedman commented Sep 20, 2015

Not precisely part of this change, but it would probably be in the best interest of the NetBSD port to move the functions marked with link_name out of libstd into the libc crate, so crates outside of rustc have access to the correct declarations.
src/libstd/sys/unix/time.rs Outdated
extern {
fn clock_gettime(clk_id: libc::c_int, tp: *mut libc::timespec) -> libc::c_int;
}
#[cfg(target_os = "netbsd")]
extern {
#[link_name = "__clock_gettime50"]

This comment has been minimized.

Sign in to view
Copy link
@alexcrichton

alexcrichton Sep 21, 2015

Member

These sorts of #[cfg] attributes are probably best done via:

#[cfg_attr(target_os = "netbsd", link_name = "__clock_gettime50")]

Could this happen for the modifications to liblibc as well?
src/libstd/sys/unix/thread.rs Outdated
arg: *mut libc::c_void) -> libc::c_int;
}
let cname = CString::new(&b"%s"[..]).unwrap();
let carg = CString::new(name).unwrap().as_ptr() as *mut libc::c_void;

This comment has been minimized.

Sign in to view
Copy link
@alexcrichton

alexcrichton Sep 21, 2015

Member

This'll end up being a use-after-free because the CString needs to live over the call of pthread_setname_np

This comment has been minimized.

Sign in to view
Copy link
@gandro

gandro Sep 21, 2015

Author Contributor

Oh wow, I didn't know this. Thanks, will fix!
@gandro
Various fixes for NetBSD/amd64
Loading status checks…
318cd84

@gandro gandro force-pushed the gandro:netbsd branch from 2ff9ec5 to 318cd84 Sep 21, 2015

@gandro

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

gandro commented Sep 21, 2015

Rebased. Found some more functions using the wrong linker symbol, hopefully these should be all.

@alexcrichton I addressed your comments, now uses cfg_attr for the link_name, even for two OSX specific declarations. Looks much cleaner!

@eefriedman Good point. Do I understand it correctly that you want to move those functions into the libc create for all platforms, and not just for NetBSD?
@eefriedman

This comment has been minimized.

Sign in to view
Copy link
Contributor

eefriedman commented Sep 21, 2015

Yes, I mean for all platforms.
src/libstd/sys/unix/sync.rs Outdated
__magic: libc::c_uint,
__opaque: [u8; __PTHREAD_COND_SIZE__],
}
#[repr(C, packed)]

This comment has been minimized.

Sign in to view
Copy link
@alexcrichton

alexcrichton Sep 22, 2015

Member

Is the packed annotation her needed for alignment/size? It looks like there's no padding here so it may not be needed?

This comment has been minimized.

Sign in to view
Copy link
@gandro

gandro Sep 22, 2015

Author Contributor

Oh dear, you are right, this is completely wrong. I added packed to avoid padding, which is indeed not needed. But as a side effect it completely messed up alignment - even without packed the alignment was wrong for some types, I pushed a fix. Rust and C agree now on size and alignment for these four types.
@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member

alexcrichton commented Sep 22, 2015

@bors: r+ 428bb16

Thanks!
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 22, 2015

⌛️ Testing commit 428bb16 with merge 9c1aaeb...

bors added a commit that referenced this pull request Sep 22, 2015

@bors
Auto merge of #28543 - gandro:netbsd, r=alexcrichton 
These changes introduce the ability to cross-compile working binaries for NetBSD/amd64. Previous support added in PR #26682 shared all its code with the OpenBSD implementation, and was therefore never functional (e.g. linking against non-existing symbols and using wrong type definitions). Nonetheless, the previous patches were a great starting point and made my work significantly easier. 😃 

Because there are no stage0 snapshots for NetBSD (yet), I used a cross-compiler for NetBSD 7.0 RC3 and only tested some toy programs (threading and channels, stack guards, a small TCP/IP echo server and some other platform dependent bits). If someone could point me to documentation on how to generate a stage0 snapshot from a cross-compiler I'm happy to run the full test suite.

A few other notes regarding Rust on NetBSD/amd64:
- To preserve binary compatibility, NetBSD introduces new symbols for system call wrappers on breaking ABI changes and keeps the old (legacy) symbols around, see [this documentation](https://www.netbsd.org/docs/internals/en/chap-processes.html#syscalls_master) for some details. I went ahead and modified the `libc` and `std` crate to use the current (renamed) symbols instead of the legacy ones where I found them, but I might have missed some. Notably using the `sigaction` symbol (deprecated in 1998) instead of `__sigaction14` even triggers SIGSYS (bad syscall) on my amd64 setup. I also changed the type definitions to use the most recent version.
- NetBSD's gdb doesn't really support position independent executables, so you might want to turn that off for debugging, see [NetBSD Problem Report #48250](https://gnats.netbsd.org/48250).
- For binaries invoked using a relative path, NetBSD supports `$ORIGIN` only for short `rpath`s (~64 chars or so, I'm told). If running an executable fails with `execname not specified in AUX vector: No such file or directory`, consider invoking the binary using its full absolute path.
Loading status checks…
9c1aaeb
@bors

@bors bors merged commit 428bb16 into rust-lang:master Sep 22, 2015
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

This was referenced Sep 23, 2015

Merged
Closed

bors added a commit that referenced this pull request Sep 24, 2015

@bors
Auto merge of #28598 - semarie:openbsd-unbreak, r=alexcrichton 
separate use code between openbsd/netbsd

netbsd use c_int and c_uint, but openbsd not, resulting a unused_import
error.

r? @alexcrichton 

problem introduced by #28543
Loading status checks…
f0666b4

@gandro gandro referenced this pull request Nov 26, 2015

Closed

Add rumprun CI #76

@gandro gandro referenced this pull request Feb 17, 2016

Merged

this fixes the siginfo_t compilation errors #181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.