This looks fine to me. Assuming I'm reading the code correctly, this change is only for targets where int64_t is long, so there should be no change in any C++ / ObjC name mangling as a result of this. It's a bit curious that we explicitly define it as a fixed-width type for 32-bit targets but long for 64-bit prior to this change, did I do that?

12 is getting close to EOL, 14 is close to release, so it's too late for a back-port to 12 to matter to me.

I've rebased this, addressed reviewer comments (except the one that wants me to change some whitespace in a way that I don't fully understand), and tested it again with the Verona process-sandbox code.

• Address some reviewer comments.

LGTM. I thought we had those functions, but I've never seen any code in the wild using them, so if we don't then we should get rid of any reference to them.

The goal is to have tcmalloc natively working with full capacity and without further patching.

I would still like to see a design document and some design review before this is added. This feature in Linux has been quite controversial. I can see a case for copying a Linux feature that is widely used, or for adding a feature in the Linuxulator that is needed by certain workloads, but this doesn't seem to meet either of those requirements (not widely used, not being added to the Linux ABI layer here, only as a native syscall).

Fantastic, thanks!

This is because the controlling expression always undergoes lvalue
conversion first, dropping any cv-qualifiers

In D33248#764167, @kib wrote:

Don't you want to have core dumped for unhandled SIGCAP?

Ah, sorry, I thought these were additional changes. Looking more carefully, these all seem to be things that we've done upstream already.

Please can you raise a PR upstream that adds a macro to select between the two ABIs, so that we're not carrying a diff in the FreeBSD version?

Are there any other blockers to committing this? @markj, please can you approve if not?

It's still not clear what the purpose of this is. It's not added to the Linux ABI. If we're adding a new FreeBSD syscall, there should be some design review or at least motivating use cases. Linux' rseq is mostly useless (far less useful than a lightweight userspace interrupt delivery mechanism or a resume-from-context-switch handler) and is *only* vaguely useful on Linux in combination with the fact that Linux has a lightweight get-CPU system call implemented in the VDSO that is cheaper than a CPUID (which is a serialising instruction and generally costs more than the saving of most of the win from doing per-CPU instead of per-thread things).

• Fix comma splice.

In D33248#752692, @markj wrote:

FWIW, I tried the tests for your process_sandbox (with this patch applied of course) and one test consistently fails:

markj@biggie> ./test-onebitsem-child

• Address code review comments.

• Fix copy-and-paste fail.

Again, this is clearly outside the design space of rseq.

In D32505#733678, @kib wrote:

Lets limit the discussion to rseq(2) and not to some future hypothetical design needed for CheriBSD (which is not FreeBSD).

In D32505#733659, @kib wrote:

In D32505#733654, @theraven wrote:

• For allocator hardening, we'd like to ensure that signals delivered while executing in the allocator don't expose internal allocator state, so we'd like a mechanism somewhat closer to Windows structured exception handling that allows signals to be redirected based on a specific IP range.

Why should this be done in kernel? More, I believe that Windows does not do it in kernel either. Last time I looked (I admit it was very long time ago) they have a single upcall from kernel to userspace for all that stuff. It is usermode duty to interpret signal source + find the corresponding entry in the exceptions ranges table and do the unwind.

More, I think that there is a strong reason why kernel should not do that. You probably need to distinguish between sync and async signals, and further classify them based on si_code before even taking the look at the unwinding, so that only the events you are prepared for, like GC barriers or whatever you know about, started your specific actions. Kernel should not know about all that details.

@brooks, can you take a look at this from a CheriABI perspective? The current version uses the Linux convention of assuming that uint64_t is a sensible type for memory addresses. We probably can't do that in CheriABI because it would allow you to register a jump address that would make other code jump to your handler (or, conversely, it may prevent you from setting a range in some situations where there's a PCC change in between the rseq setup and the destination).

Thanks for doing this, it will make porting our work to FreeBSD easier. There are some missing man pages (both for the new pmap interfaces and for the syscall).

Abandoning: This is probably not the right approach and @jhb is working on something similar so I'll wait for his version.

@brooks, this should also be useful for combining CHERI coprocesses with Capsicum (coprocesses are currently created with vfork + coexecve).

• Propagate the field on fork.

• Add missing siginfo(3) documentation.
• Propagate the field on fork.

• Add missing siginfo(3) documentation.

Let's try option 2 and see if it works :-)

I've never used git send-mail, how does it interact with Phabricator? If I do a rebase / squash to produce a single commit, is that sufficient? You should be able to then arc patch and git push.

Thanks. Please can you commit if you're happy with it?

• Use less-ambiguous terminology.

• Use less-ambiguous terminology.

Can rfork (or pdfork / vfork / whatever) both fail with a capsicum violation *and* create a new struct proc and return in the child context with a signal? That should be the only case where you need to duplicate the original_code field into the child.

12-STABLE version is D30635.

Thanks. I think I'm happy for this version to land in main if reviewers are and I'll work on a version for 12- and 13-stable.

• Reorder original_code to fit in padding for 64-bit archs.

In D29185#655877, @kib wrote:

In D29185#655872, @theraven wrote:

By themself, syscall_args are not used by anything but syscall entry code (and some ptrace bits). But the structure is embedded into struct thread, and layout of struct thread must be stable.

Are the syscall_args structures part of the stable KBI? This won't quite cleanly apply to older versions of FreeBSD because at least on x86 the system call entry code has been cleaned up to remove some code duplication, but I am running on 12-STABLE with a version of this that doesn't correctly handle the syscall / __syscall syscalls and it would be great to be able to MFC as much as possible, even if we can't get full support without the KBI change.

• Add missing architecture support, address reviewer comments.

Sorry, this version does work, I had only updated one of the places in the code using it to check si_syscall and so the test was failing yesterday.

Add amd64 paths to preserve the original system call number.

When do you need this? Isn't this kind of introspection warrants a use of real debugger which can deduce all the data from debugging information?

I appear to be setting the field in the siginfo structure incorrectly in the new version, but it's now the end of the day for me so I will try to debug this tomorrow.

@emaste, sorry, I don't think I understand this comment.

Updated based on @kib's feedback.

@kib, thank you, that also mirrors what Linux does for seccomp violation signals. I will update the diff to reflect that.

Thanks, I'll see if I can chase this down later. My commit bit lapsed, please can you land it?

Yes it should be strdup'ed somewhere but I am surprised that it works. Look at the start of load_object(): if name != NULL, it searches for existing loaded object with the specified name.

Thanks. I can confirm that this change also fixes this problem:

For snmalloc, we use this call in the Linux PAL. On FreeBSD, we use mmap to map a new anonymous memory object over the old one, which has the same behaviour. We don't do this on shared memory mappings, but for sandboxing use cases we do need some equivalent mechanism (i.e. something that can zero a large range of shared memory, without requiring the OS to actually allocate physical pages for all of the zeroes). On Linux, we can do this with fallocate on the underlying memory object (I think we can do it with mem_fd objects, we can definitely do it in tmpfs). I am not sure what the correct mechanism for doing this in FreeBSD is.

This looks fine, but please submit it as a GitHub PR so that it goes through CI (I can't imagine it failing).

Unless I'm missing something, this leaves GPL'd dtc's sources in the tree but removes the ability to build them. Would it make sense to remove the code at the same time as removing the build system goo?

I've now tried doing this and have finally managed to get it to work. A couple of things:

In D19566#422302, @jhb wrote:

I had labored under the incorrect assumption that libcxxrt was somewhat abandoned and that all other libc++ consumers used libc++abi, however, it seems that it's only Apple and OpenBSD that use libc++abi. NetBSD still uses libcxxrt. Also, when I compared the repositories both libcxxrt and libcxxabi had basically no real activity in the last two years. libcxxabi had more commits, but they were all due to relicensing and moving to monorepo.

I'd be very happy to replace the libelftc demangler with the one from libc++abi, but last time this was discussed there were some complaints about the significant increase in binary size. One solution that I'd considered is to replace the demangler in libc++abi with a stub that doesn't actually do any demangling and then import the libc++abi demangler into libc++, possibly excluding it from release builds of the static library (dynamically linked programs probably don't mind paying the size overhead).

Please do not do this. libc++abi implements the Itanium ABI specification, but *not* GNU extensions. This is fine for Apple, because their Objective-C exceptions are thin wrappers around C++ exceptions, but other languages on non-Apple platforms (Objective-C definitely, I think Ada and a few others) rely on GNU extensions. These are supported by libsupc++ and libcxxrt. There's a reason that the product groups at $WORK picked libcxxrt for things that ship libc++...

In D17529#405556, @kib wrote:

I am going to commit this shortly, with 'Submitted by: theraven'.

In D17529#403296, @kib wrote:

Yes, I agree from reading of the code. I suspect that I left the chunk because I wanted to keep dlopen() handling as is, but later added the initlist_objects_ifunc() call in dlopen_object().

If you can test that the latest amd64 head boots multiuser with your change applied, please commit and MFC to 12+11.

In D17529#403239, @kib wrote:

In D17529#403212, @theraven wrote:

In D17529#374132, @kib wrote:

I think you somehow failed to provide the explanation of your case. Esp. the last paragraph of the description is confusing, please recheck the order of dependencies and order of resolution and correct it so I can understand. Better yet, provide the minimal example of the self-contained code which demonstrates the issue.

Sorry for the delay in getting back to this. I hoped that D18400 would fix it, but now that I've upgraded to 12, I'm still seeing the issue. We have a library that exposes a function with an ifunc resolve. The ifunc resolver calls another function in the same library, via the PLT. When a second library links against our library and includes a non-PLT relocation for our ifunc symbol, our ifunc resolver invokes a function via a PLT stub, but the PLT stub is not yet initialised and so this dies.

The problem is in libexec/rtld-elf/amd64/reloc.c where, in reloc_non_plt, it calls rtld_resolve_ifunc without checking that defobj is relocated. Adding this before the call is almost certainly the wrong solution, but does address the problem:

if (!defobj->relocated)
    relocate_object(__DECONST(Obj_Entry *, defobj), false, obj_rtld, flags & ~SYMLOOK_IFUNC, lockstate);

I think the correct fix most likely involved doing something a little bit closer to this, and deferring ifunc resolution until after the library implementing the ifunc has had its non-ifunc relocations applied.

This is exactly how the current rtld ifunc resolution works:

• first all objects gets their non-plt non-ifunc relocations resolved.
• second all objects gets their ifuncs resolved, including plt resolution if LD_BIND_NOW.
• third pass where init functions are called.

The second pass (for ifuncs) is performed in the reverse-init order, same as the third pass.

In D17529#374132, @kib wrote:

I think you somehow failed to provide the explanation of your case. Esp. the last paragraph of the description is confusing, please recheck the order of dependencies and order of resolution and correct it so I can understand. Better yet, provide the minimal example of the self-contained code which demonstrates the issue.

There may be a few more changes coming. I can now build all of the gnustep-app packages with the new ABI, but sope2 fails to link with some interesting errors.

These changes look fine, but please send a PR on GitHub and then merge to FreeBSD from upstream.

I wonder how we prevent these from getting out of sync (or check their correctness in the first place). How feasible is it to add a dynamic check along the lines of WITNESS that would record these annotations for each block of memory and then check that copyin / copyout were not done past the declared length, or in the wrong direction? I suspect a static analysis would be too hard unless we are willing to propagate these annotations all of the way down into the kernel, unless the copyin / copyout are always done near the system call layer.

I saw something similar. This upgrade first failed because of a samba44 samba46 conflict. Running it again silently removed samba44 (nothing in my scrollback indicates that 4.4 will be removed) and installed 4.6. Not a great user experience.

I don't have an exhaustive list, and even if we fix all of them in the base system, there will be others in ports. This is why we have things like $PAGER - to avoid needing to hard-code defaults. If the project believes that less is a better default pager than more then we should modify /etc/profile or something to set PAGER=less, not go and modify all of the tools by hand to use less (and if there are any tools that don't respect $PAGER, $EDITOR and so on then they are buggy and should be fixed).

I can't help feeling that this poll is really asking the wrong question. There are a bunch of things like this, for example:

What are the changes? I've only ever used less on GNU systems where the more install is ancient and doesn't support scrolling up. I have no opinion if I'm not going to notice the difference, but without knowing what the 'few things less does differently' are I'd vote no to avoid surprises.

I don't have strong opinions about this - hopefully this code can be fixed by some judicious application of svn rm in the near future...

This probably means that we no longer need to expose _DefaultRuneLocale in the header.