std::threads: revisit stack address calculation on netbsd.

[devnexen]

like older linux glibc versions, we need to get the guard size and increasing the stack's bottom address accordingly.

[rustbot]

r? @workingjubilee

rustbot has assigned @workingjubilee. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[workingjubilee]

cc @he32 It's not clear to me which versions of NetBSD we support based on our platform support docs for NetBSD. Can you offer some guidance on what we should do here, in light of things like https://gnats.netbsd.org/57721

[he32]

cc @he32 It's not clear to me which versions of NetBSD we support based on our platform support docs for NetBSD. Can you offer some guidance on what we should do here, in light of things like https://gnats.netbsd.org/57721

The TL;DR; version is "NetBSD 9.0 or newer".

We used to support rust on NetBSD/8.0, but recent-ish bumps of LLVM have upped the requirement for the C++ compiler to such an extent that supporting 8.0 was no longer feasible using the "native" C++ compiler.

There are platform-dependent exceptions, e.g. the riscv64 target is so new that it requires ~10.0 or newer -- the port was only recently added, and I think it still only runs in an emulator (qemu).

That said, I have sadly not yet been able to translate Taylor's investigations in the quoted NetBSD PR back into a workable platform-dependent patch for rust. However, rust 1.75.0 on NetBSD/amd64 builds a working firefox and thunderbird, and I have no reason to believe 1.76.0 is any worse in that regard.

[riastradh]

Is there an explanation somewhere of why Rust is querying the stack parameters, and what Rust does with them? Programs don't normally need to do this, so it would be helpful to know what Rust is trying to achieve.

The layout of the stack in NetBSD is documented here: https://man.NetBSD.org/stack.7

Note that it is different for the main thread and for threads created with pthread_create. Also, the usable size of the main thread's stack -- and thus the exact least/greatest address of the stack, depending on whether it grows down or up -- may vary at runtime depending on the stack size rlimit. So if you ask for the least/greatest usable stack address at two different times, you may get two different answers, if the process called setrlimit in the middle. That's why it would be helpful to know what Rust wants this for before committing to a way to do it.

[workingjubilee]

@riastradh This information is used to make decisions on how to set up our guard pages, if we need to set them up, which itself is a very OS-specific decision. It is also used to inform error reporting on whether the reason we hit a signal handler is stack overflow or what. This function will be called at program startup:

https://github.com/rust-lang/rust/blob/b77e0184a95d60080f0d0605e2a3b337e904c21e/library/std/src/sys/pal/unix/thread.rs#L856-L866

[workingjubilee]

Programs don't normally need to do this, so it would be helpful to know what Rust is trying to achieve.

Programs, strictly speaking, do not "need" to do anything useful, yet we demand they be useful anyways. int main() { while (true) {} } is a valid C program. So I'd ask that we focus on what we can do, rather than jump to assumptions about necessity, as asking too many questions about necessity quickly lands in rather existentialist territory about why anything instead of nothing? And because I am happy to entertain "anything" instead of "nothing", I would suggest that detecting and reporting stack overflow, or indeed any useful error data at all, instead of doing nothing and contenting ourselves with Segmentation fault (core dumped), is in fact preferable.

Anyways, I don't think anything in std calls pthread_attr_setstack, only pthread_attr_setstacksize upon spawning a thread.

[workingjubilee]

@devnexen Hmm. Is this fallthrough implementation correct for NetBSD? We don't have to also fix this in the same PR, but I'm feeling somewhat doubtful that the code becomes meaningfully more correct if we don't touch both places. https://github.com/rust-lang/rust/blob/b77e0184a95d60080f0d0605e2a3b337e904c21e/library/std/src/sys/pal/unix/thread.rs#L924-L954

[devnexen]

@devnexen Hmm. Is this fallthrough implementation correct for NetBSD? We don't have to also fix this in the same PR, but I'm feeling somewhat doubtful that the code becomes meaningfully more correct if we don't touch both places.

I m not sure it s incorrect but more do we really to do this with NetBSD ? I m fine reverting back this change if needed.

[riastradh]

So I'd ask that we focus on what we can do, rather than jump to assumptions about necessity, as asking too many questions about necessity quickly lands in rather existentialist territory about why anything instead of nothing?

Sorry, didn't mean to get philosophical here, just wanted to know what this query is in the service of. I only said that to explain it's such an unusual thing to do I didn't have a guess about why Rust might be doing it.

Anyways, I don't think anything in std calls pthread_attr_setstack, only pthread_attr_setstacksize upon spawning a thread.

Great, so that means you don't have to worry about the pthread_attr_setstack bug with the guard region.

@riastradh This information is used to make decisions on how to set up our guard pages, if we need to set them up, which itself is a very OS-specific decision.

If you want to guarantee there is a guard region of at least a particular size in threads created with pthread_create, you can do that portably with pthread_attr_setguardsize, so there's no need to query it for that purpose.

For the main thread, it may be a different story on other operating systems. But on NetBSD, there is always a guard region of a number of bytes given by sysctl vm.guard_size -- default one megabyte (1048576 bytes), and I don't think there's any supported way to change it. So unless you want a guard region larger than that, there's no need to query or do anything. (That's separate from the pages allocated for the difference between the soft and hard stack size rlimits on exec, which is what https://man.netbsd.org/stack.7 calls the inaccessible pages.)

It is also used to inform error reporting on whether the reason we hit a signal handler is stack overflow or what.

In that case:

• For the main thread, any SIGSEGV with .si_code = SEGV_ACCERR and .si_addr anywhere between the process's stack base (obtained from the ELF auxiliary vector AT_STACKBASE entry) and the hard rlimit, up or down depending on the direction of stack growth, is probably a stack overflow. (It could also be an attempt to execute code in the stack, though, and I'm not sure offhand if there's an easy way to distinguish that case.)

  I think you may also be able to narrow it down to stack overflow vs some other abuse of stack address space by examining getrusage .ru_isrss to see whether .si_addr has gone beyond the actual stack utilization of the process.

  (One can recover the stack base from pthread_attr_getstack in the main thread, but it's a little circuitous: if the stack grows up, you get the stack base directly; if the stack grows down, you have to add the (process-initial) soft rlimit to the address returned by pthread_attr_getstack to get the stack base. If you never call setrlimit(RLIMIT_STACK) to change the soft limit, you can just use the main thread's pthread_attr_getstack region directly for this, but I would guess Rust does provide some way to call setrlimit(RLIMIT_STACK) so you can't rely on it.)

• For a thread created with pthread_create, any SIGSEGV with .si_code = SEGV_ACCERR and .si_addr in the region returned by pthread_attr_getstack is probably a stack overflow (with the same caveat about distinguishing execute access).

This function will be called at program startup:

https://github.com/rust-lang/rust/blob/b77e0184a95d60080f0d0605e2a3b337e904c21e/library/std/src/sys/pal/unix/thread.rs#L856-L866

So I gathered, but what uses the results of that function?

[workingjubilee]

(One can recover the stack base from pthread_attr_getstack in the main thread, but it's a little circuitous: if the stack grows up, you get the stack base directly; if the stack grows down, you have to add the (process-initial) soft rlimit to the address returned by pthread_attr_getstack to get the stack base. If you never call setrlimit(RLIMIT_STACK) to change the soft limit, you can just use the main thread's pthread_attr_getstack region directly for this, but I would guess Rust does provide some way to call setrlimit(RLIMIT_STACK) so you can't rely on it.)

We do not "provide some way to call setrlimit" per se, however I should note that it is incorrect to reason about what std does as definitive for a process, because we do "provide a way" to call arbitrary extern "C" fn, by allowing people to write

extern "C" {
    fn setrlimit(resource: i32, rlim: *const rlimit) -> i32;
}

And a Rust-defined extern "C" fn can be called by a C program. All this is why I was thinking about the pthread_attr_setstack bug anyways.

[riastradh]

We do not "provide some way to call setrlimit" per se, however I should note that it is incorrect to reason about what std does as definitive for a process, because we do "provide a way" to call arbitrary extern "C" fn, by allowing people to write

extern "C" {
    fn setrlimit(resource: i32, rlim: *const rlimit) -> i32;
}

And a Rust-defined extern "C" fn can be called by a C program. All this is why I was thinking about the pthread_attr_setstack bug anyways.

There's a difference between these two issues:

• This logic doesn't need to care if some other Rust code calls pthread_attr_setstack and triggers the netbsd<=9 bug. (The other Rust code needs to do pthread_attr_setguardsize(A, 0) and it'll be fine.)
• This logic does need to care if some other Rust code changes the soft stack rlimit. If this logic caches the results of pthread_attr_getstack without any adjustment for the main thread, the other code may invalidate that cache and break whatever relies on the cache. (And I don't think this is peculiar to NetBSD -- I expect it's the same deal on most operating systems.)

[workingjubilee]

So I gathered, but what uses the results of that function?

To answer this, it is called in the main runtime startup:

https://github.com/rust-lang/rust/blob/1c580bcb703d2ba288c380d2e283451a34d4eb1c/library/std/src/rt.rs#L71-L107

Which then uses it to set these:

https://github.com/rust-lang/rust/blob/1c580bcb703d2ba288c380d2e283451a34d4eb1c/library/std/src/sys_common/thread_info.rs#L13-L18

Which finally becomes an answer for a question way over here: https://github.com/rust-lang/rust/blob/735f7589e3d7d28c2e1bc5c241750ee069d0d65a/library/std/src/sys/pal/unix/stack_overflow.rs#L82-L87

Wow that's a lot of hops to trace this logic.

[riastradh]

Wow that's a lot of hops to trace this logic.

OK, cool, thanks for tracking this down!

Is there another call for threads created with pthread_create, or is it only called for the main thread?

If the code you quoted covers all uses of the outputs, then it looks like this logic is used only to determine a region of memory in which a SIGSEGV probably indicates a stack overflow.

In that case: I suggest you just use pthread_attr_getstack to get p and n, and pthread_attr_getguardsize to get g (or use g = PAGE_SIZE), and use [p - g, p) on machines where the stack grows down, or [p + n, p + n + g) on machines where the stack grows up, as the guard region for the SIGSEGV handler's stack overflow detection.

This is not quite accurate for the main thread, but the harm of the inaccuracy is just that some SIGSEGVs which actually are stack overflows -- in programs that have raised the soft rlimit -- may not be reported as stack overflows. Nothing else bad will happen.

So these existing fragments are probably fine to use for NetBSD, on machines where stacks grow down:

https://github.com/rust-lang/rust/blob/79d246112dc95bbd67848f7546f3fd1aca516b82/library/std/src/sys/pal/unix/thread.rs#L806-L833 https://github.com/rust-lang/rust/blob/79d246112dc95bbd67848f7546f3fd1aca516b82/library/std/src/sys/pal/unix/thread.rs#L860-L873

Or the OpenBSD case below, which is identical:

https://github.com/rust-lang/rust/blob/79d246112dc95bbd67848f7546f3fd1aca516b82/library/std/src/sys/pal/unix/thread.rs#L914-L923

Side notes about this code:

• You don't need to mmap or mprotect guard pages. NetBSD already does this for you.
• You don't need to use pthread_attr_setguardsize. It's only relevant when creating threads with pthread_create; it doesn't serve any purpose in querying the stack parameters of existing threads.

---

The above assumes you don't create pthreads with pthread_attr_setstack. If you do, then it is your responsibility to allocate guard pages and tell the signal handler what guard pages you allocated for it. (That's true for all operating systems; it's not NetBSD-specific.)

[riastradh]

@devnexen Why are you still defining a NetBSD-specific get_stack_start, when it looks like the existing one that is currently shared with android/freebsd/hurd/linux/l4re should work just fine (or, no worse than it already works for those other platforms, anyway)?

You seem to be trying to work around the pthread_attr_setstack bug, but that bug doesn't apply here (and adjusting the stack address you get out of pthread_attr_getstack doesn't help with that bug). The pthread_attr_setstack bug applies only when you create a thread with pthread_attr_setstack, which this logic is not doing; it doesn't apply when you query an existing thread, which this logic is doing.

[workingjubilee]

Wow that's a lot of hops to trace this logic.

OK, cool, thanks for tracking this down!

Is there another call for threads created with pthread_create, or is it only called for the main thread?

If the code you quoted covers all uses of the outputs, then it looks like this logic is used only to determine a region of memory in which a SIGSEGV probably indicates a stack overflow.

Almost. The sys::thread::guard::init fn is only called on the first-time startup there, which does whatever guard mapping it needs. There is one other place that calls thread_info::set, which takes the stack guard argument, and it is thread::Builder::spawn_unchecked:

https://github.com/rust-lang/rust/blob/8401645716b26a8b4c6974dc0680e55e81e9e8a1/library/std/src/thread/mod.rs#L459-L541

From guard::current(), which is like this for NetBSD:

https://github.com/rust-lang/rust/blob/79d246112dc95bbd67848f7546f3fd1aca516b82/library/std/src/sys/pal/unix/thread.rs#L964-L1021

We may wish to adjust how we set up threads, in light of this information (this has been quite illuminating, thank you! I wish the man pages for the pthread headers were as clear...) but I believe it's currently harmless, because this information still is only consumed by the signal handler.

Side notes about this code:

* You don't need to mmap or mprotect guard pages.  NetBSD already does this for you.

Right, it sounds like for sys::thread::guard::init the OpenBSD branch here is the one that NetBSD should be using? Though it sounds like the stack isn't immutable, exactly, on NetBSD, but we "don't care" because, thinking about it more clearly now, anyone running pthread_attr_setstack has to be running it after sys::thread::guard::init, in which case we can't exactly interdict their syscalls, as far as I know, and they're taking their life into their own hands, OR we never get to run this function anyways.

https://github.com/rust-lang/rust/blob/79d246112dc95bbd67848f7546f3fd1aca516b82/library/std/src/sys/pal/unix/thread.rs#L914-L923

[workingjubilee]

Based on my understanding of how things are so far, I think this is the correct change, and nothing in my last message actually changes things, so we can ship this, assuming @riastradh or @he32 also agrees. Thank you for bearing with us as we hashed this out, @devnexen. You do a lot of work for the BSD targets and it's nice to see.

@bors delegate=@riastradh rollup=always

[bors]

:v: @riastradh, you can now approve this pull request!

If @workingjubilee told you to "r=me" after making some further change, please make that change, then do @bors r=@workingjubilee

[riastradh]

Is there another call for threads created with pthread_create, or is it only called for the main thread? If the code you quoted covers all uses of the outputs, then it looks like this logic is used only to determine a region of memory in which a SIGSEGV probably indicates a stack overflow.

Almost. The sys::thread::guard::init fn is only called on the first-time startup there, which does whatever guard mapping it needs.

OK, so it sounds like there is nothing else that uses sys::thread::guard::init, then? In other words: this is used exclusively to find the main thread's stack guard region, in order to recognize stack overflows in the SIGSEGV handler?

There is one other place that calls thread_info::set, which takes the stack guard argument, and it is thread::Builder::spawn_unchecked: [...] From guard::current(), which is like this for NetBSD:

Interesting, so there's separate logic to find the stack guards of non-main threads? That logic looks fine too.

For NetBSD (as well as Linux and probably every other system) it might be worthwhile, in the main thread's stack overflow recognition on SIGSEGV, to use the combination of what is called the guard and the inaccessible pages at https://man.NetBSD.org/stack.7 -- that is, the region between the soft rlimit and the hard rlimit, plus the guard.

But that's a little more work -- and it's not really NetBSD-specific because other OSes also grow the stack automatically, so there's no need to put it in this PR, which, as I understand it, is really only needed to omit Rust's attempt to mmap/mprotect its own guard page at the process's minimum possible stack size when NetBSD already put one at the maximum possible stack size.

(this has been quite illuminating, thank you! I wish the man pages for the pthread headers were as clear...)

If you can point out any parts you found unclear in the pthread man pages in NetBSD, I'd be happy to take a look and see if we can improve them!

Side notes about this code:

• You don't need to mmap or mprotect guard pages. NetBSD already does this for you.

Right, it sounds like for sys::thread::guard::init the OpenBSD branch here is the one that NetBSD should be using?

Yes, that looks good.

Though it sounds like the stack isn't immutable, exactly, on NetBSD,

On most systems, I expect the main thread's stack will grow automatically up to the soft rlimit (which can, in turn, be grown up to the hard rlimit). So if that's what you mean by 'the stack isn't immutable', that's not NetBSD-specific.

but we "don't care" because, thinking about it more clearly now, anyone running pthread_attr_setstack has to be running it after sys::thread::guard::init, in which case we can't exactly interdict their syscalls, as far as I know, and they're taking their life into their own hands, OR we never get to run this function anyways.

Note that pthread_attr_setstack doesn't change any existing threads. It only sets up configuration (pthread attributes) that can be passed to pthread_create when creating new threads. If you do pthread_getattr_np(T, &A) and then pthread_attr_setstack(&A, ...), that doesn't change anything about the existing thread T -- pthread_getattr_np just makes a copy of T's configuration; the resulting pthread_attr_t A has no lingering connection to T.

And if someone does write Rust code to create a pthread with a custom stack using pthread_attr_setstack and pthread_create, they will have to arrange their own guard pages (mapping them and informing the SIGSEGV handler of them) on every OS, not just NetBSD.

[workingjubilee]

honestly no idea what the comment that says "the stack is immutable" really means.

And while I said that OpenBSD and NetBSD should share the logic in question for guard::init(), that was in fact introduced a revision ago and I was just looking at the wrong commit. So yeah! Good to go.

@bors rollup=always r=workingjubilee,riastradh

[bors]

:pushpin: Commit ffdd97f79178f16c5306a84af933aa31f223ffb4 has been approved by workingjubilee,riastradh

It is now in the queue for this repository.