In D50716#1158047, @kib wrote:

You need at least sys/types.h there as well.

Move endif

Trim and sort CFLAGS

In D50683#1157180, @markj wrote:

In D50683#1157061, @jrtc27 wrote:

What if there is no symbol (which presumably shows up as off != 0, using whatever symbol happens to be before it)? Or it's in another file? Those are legitimate cases (the former in particular, that's for when the implementation is static, and the latter is weird but there's no reason you can't do it).

If there's no symbol, what should we do? I don't think it's really correct to keep passing the resolver symbol's size, so should we just set it to 0 instead? In practice this case currently doesn't arise AFAICT, and so I'm rather inclined to signal the failure to the consumer.

What if there is no symbol (which presumably shows up as off != 0, using whatever symbol happens to be before it)? Or it's in another file? Those are legitimate cases (the former in particular, that's for when the implementation is static, and the latter is weird but there's no reason you can't do it).

As mentioned on IRC but summarised here for reference: do we want to support PKGCONFBRANCH=latest being set in a user's src.conf on release branches? 0b18e008ccf74ee87d38ca16c9c4d9bb0b174bec deliberately made this an overridable variable, and so it we want to preserve that we probably should have a latest-release file too.

Extend existing comment to note different implementation to NetBSD

In D50563#1154663, @kib wrote:

Indeed this is not how things are supposed to work. At least on x86, this is how things are. There are vendor libs linked with static TLS model, which happens to be dynamically loaded, and people really want that to work. The libs cannot be recompiled (they come from nvidia).

If you want to keep risc-v "pure", it is up to you.

I've gone digging and this looks like confusion importing NetBSD's code. This was applying https://github.com/NetBSD/src/commit/cc08a85a2529815b95799c6500b40a2c596c0c93 to our tree, just as 91880e07f605edb90339685bc934699a4344de3b was https://github.com/NetBSD/src/commit/3caa8dc7351c8383b2c3832b2610624b347e9065. The key point is that FreeBSD's tlsoffset is always relative to the TCB, whereas for Variant I TLS on NetBSD it's relative to the end of the TCB. So the original code was wrong for NetBSD but right for FreeBSD, and the updated code fixed it for NetBSD but broke FreeBSD by importing it. Will update the commit message with a properly-written summary tomorrow.

NB: I do intend to do that cleanup, just not tonight

In D50459#1151997, @emaste wrote:

would it not be even faster, and sidestep the issue entirely, if we just used bsdtar's -J flag

Good point, although that might be GNU tar on other systems? If so -J should be fine but is --options xz:threads=0 handled?

macOS's /bin/sh is governed by /private/var/select/sh. On my system it's /bin/bash and supposedly new macOS installs will default it to /bin/sh (at least according to tools/build/Makefile, which links /bin/bash into the tmp path as sh). Is there a reason to use the pipe though; would it not be even faster, and sidestep the issue entirely, if we just used bsdtar's -J flag? (You can set xz:compression-level and xz:threads on the command line)

Unhook from build and clean up

Oh right yes these are actually-installed files (even more ugh), not something in tools/, so need to be unhooked from the build too. One moment...

Hm, is this because of rtld using the nossppico files from libc? I see several problems:

1. The nossppico files really shouldn't have the syscall bodies either
2. On CheriBSD we don't have nossppico for CHERI, since we don't have/need SSP, so rtld pulls from the regular pico files

Obligatory style review:

• I see commented out code; either delete it or use #if(def) to gate it with an internal debug define
• FreeBSD uses /* C-style comments */ not // C++-style comments but I see a few instances of the latter
• Continuation lines aren't being consistently indented (I'm struggling to figure out what rule you've followed, even); they should be indented by four spaces relative to the block-level indent
• For combinations of bitwise operations etc where the operator precedence can be unintuitive we generally include redundant parentheses to avoid the need to think about whether it's correct without them (both for reading and writing)

What shell are we using here, the default system sh? dash doesn't support pipefail, so I'm concerned this will break on Linux.

Or, like WITNESS or MALLOC_PRODUCTION, on in HEAD and off in stable branches?

Seems sensible to me

Hm, binutils should still be defaulting to -znostart-stop-gc even today in their master branch. So this really should be a no-op, this shouldn't be the cause of any issues. (Unless sjg is building with some modified environment that overrides that default, I suppose, but that's shooting oneself in the foot...)

In D50226#1145897, @kib wrote:

In D50226#1145856, @jrtc27 wrote:

In D50226#1145838, @kib wrote:

In D50226#1145834, @jrtc27 wrote:

In D50226#1145753, @kib wrote:

For me, Elf_Addr is more natural type synonym to use in the rtld code. Also, it gives us the luxury of Elf32/64_Addr typedefs, so if needed, multilib-like rtld has an easier way to express itself.

Well, Elf_Addr is about the format of addresses within the ELF file, matching the default ELF class for the current ABI. Technically that does not have to match the format of addresses at run time (LP64 IA-64 used both ELFCLASS32 and ELFCLASS64; LP46 vs ILP32 was an orthogonal EF_IA_64_ABI64), though you probably shouldn't build a system that does that these days, and of course it definitely doesn't match the format of pointers for CHERI. The values here are not addresses in ELF files, they are pointers at run time, so they should use pointer types, and using anything other than (u)intptr_t or T * does not work on CHERI. We've already had to change this code downstream and whilst refactoring things upstream I'd like to take the time to use CHERI-compatible types rather than write new(ish) code that I know does not work for CHERI.

I think this opinion should be stated in the change' summary.

Thanks. Are you happy with the updated description?

Is it LP46 or LP64?
Yes, I think that the updated text provides better motivation for the change. Thank you.

In D50226#1145838, @kib wrote:

In D50226#1145834, @jrtc27 wrote:

In D50226#1145753, @kib wrote:

For me, Elf_Addr is more natural type synonym to use in the rtld code. Also, it gives us the luxury of Elf32/64_Addr typedefs, so if needed, multilib-like rtld has an easier way to express itself.

Well, Elf_Addr is about the format of addresses within the ELF file, matching the default ELF class for the current ABI. Technically that does not have to match the format of addresses at run time (LP64 IA-64 used both ELFCLASS32 and ELFCLASS64; LP46 vs ILP32 was an orthogonal EF_IA_64_ABI64), though you probably shouldn't build a system that does that these days, and of course it definitely doesn't match the format of pointers for CHERI. The values here are not addresses in ELF files, they are pointers at run time, so they should use pointer types, and using anything other than (u)intptr_t or T * does not work on CHERI. We've already had to change this code downstream and whilst refactoring things upstream I'd like to take the time to use CHERI-compatible types rather than write new(ish) code that I know does not work for CHERI.

I think this opinion should be stated in the change' summary.

In D50226#1145753, @kib wrote:

For me, Elf_Addr is more natural type synonym to use in the rtld code. Also, it gives us the luxury of Elf32/64_Addr typedefs, so if needed, multilib-like rtld has an easier way to express itself.

Only include libc changes