Page MenuHomeFreeBSD

Allow sbrk on arm64 and riscv
AcceptedPublic

Authored by kib on Nov 30 2023, 7:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 29, 10:16 AM
Unknown Object (File)
Mon, Apr 22, 9:06 AM
Unknown Object (File)
Sat, Apr 20, 8:35 PM
Unknown Object (File)
Jan 18 2024, 1:22 PM
Unknown Object (File)
Jan 5 2024, 10:38 PM
Unknown Object (File)
Dec 29 2023, 9:49 PM
Unknown Object (File)
Dec 24 2023, 9:47 PM
Unknown Object (File)
Dec 23 2023, 5:25 AM
Subscribers

Details

Summary
break(2): do execute everywhere

stop pretending that arm64 and riscv cannot provide the syscall.  So far
it only annoyed porters.
libc: export brk and sbrk symbols on all architectures

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Nov 30 2023, 7:40 PM

I'm glad that commit 9f9c9b22ecfd9e87cbe9e1becf946faeb5fbcac6 allowed some time to be saved here. :)

lib/libc/tests/sys/Makefile should be updated too, so that brk_test is installed unconditionally. (I verified that those tests pass on arm64 with this patch applied.)

I'm glad that commit 9f9c9b22ecfd9e87cbe9e1becf946faeb5fbcac6 allowed some time to be saved here. :)

Yes, that commit was of course the main bits to allow this. But I do not see why it was not finished then.

In D42853#977457, @kib wrote:

I'm glad that commit 9f9c9b22ecfd9e87cbe9e1becf946faeb5fbcac6 allowed some time to be saved here. :)

Yes, that commit was of course the main bits to allow this. But I do not see why it was not finished then.

I'm not sure what you mean - what was not finished? At the time of that commit, arm64 and riscv already had removed break(2) support.

This revision is now accepted and ready to land.Nov 30 2023, 9:07 PM
In D42853#977457, @kib wrote:

I'm glad that commit 9f9c9b22ecfd9e87cbe9e1becf946faeb5fbcac6 allowed some time to be saved here. :)

Yes, that commit was of course the main bits to allow this. But I do not see why it was not finished then.

I'm not sure what you mean - what was not finished? At the time of that commit, arm64 and riscv already had removed break(2) support.

I mean that (re)enabling break() on the arches should be right after the commit.

I won't attempt to block this change, but I don't like it. break/sbrk is an awful interface and can't be fixed. IMO we're better off without it.

Which ports (or possible ports) are having issues because of a lack of sbrk?

I know emacs had issues in the past, however our lack of sbrk helped them finally fix their broken use of it (and fix emacs when aslr is enabled). Recently vlang [1] was mentioned, however it appears to be an easy fix to switch to mmap as the mmap support is already there, it's just not enabled by default.

[1] https://github.com/vlang/v

Which ports (or possible ports) are having issues because of a lack of sbrk?

I know emacs had issues in the past, however our lack of sbrk helped them finally fix their broken use of it (and fix emacs when aslr is enabled). Recently vlang [1] was mentioned, however it appears to be an easy fix to switch to mmap as the mmap support is already there, it's just not enabled by default.

[1] https://github.com/vlang/v

I do not remember exactly, just that the mentioning of sbrk(2) as an annoying missed feature happens.

I do not see a point keeping the interface disabled while it is implemented.

D42862 is my preferred alternative (plus a patch I've not published to fix the tests)

I *will* object to this. We've had multiple releases without this syscall. sbrk is an absolutely awful interface in this day and age. Eradicating its use is a worthwhile goal, and the primary culprits have been fixed. As time goes on it will become even less relevant. It seems wrong to go and add it back now for the minuscule set of irrelevant packages that still believe it's 1989 and risk new bogus uses of the functions creeping in. We should be striving to remove it from all architectures (with ABI compat for old releases), not adding it in more places.

That is, I believe @brooks's approach is the one we should be striving for, i.e. taking steps forwards and eradicating it in 15, not going backwards like this patch proposes.