sys: build core headers under bounds-safety

[abhijeetsharma2002_gmail.com]

Summary

Spell out existing idioms the checker rejects: allocator and
register-read locals as __single, per-CPU and td_sched accessors kept
as pointer arithmetic rather than integer round-trips, real array
bounds on the libkern tables. No behaviour or layout change.

Sponsored by: The FreeBSD Foundation

[nickbanks_netflix.com]

sys/sys/fnv_hash.h

50

Sorry to bother, but may I ask for the reasoning behind adding this new local variable n instead of using len directly? Just curious. Also, I didn't realize FreeBSD had these types of annotations. Reminds me of SAL on Windows.

[abhijeetsharma2002_gmail.com]

sys/sys/fnv_hash.h

50

No bother at all.

buf is __sized_by(len), so len is not an independent parameter anymore. It is the recorded size of buf, and the compiler requires the pointer and its count to be assigned together, so decrementing len by itself is rejected. The local keeps len, meaning "size of buf", for the whole function and leaves the counter as an ordinary variable.

You can look up the docs for fbounds-safety here.

I am not familiar with SAL so cannot compare them properly. This is a FreeBSD Foundation project to adopt -fbounds-safety in the kernel network stack, proposal can be found here.

[imp]

So you are using the raw compiler commands, unconditionally. This will likely break gcc badly.
Normally we define 'standard' macros in cdefs.h and use those everywhere we can (eg, we'd use
them in clang, but maybe not gcc if there's no support for the concept). I don't have a good
design, but I can't see this not breaking things.

So that needs to be addressed before these can be committed.

sys/sys/fnv_hash.h

50

We use SAL markings in the syscall.master files. We don't use it elsewhere.

[imp]

I actually missed the other review that did exactly what I requested.

[abhijeetsharma2002_gmail.com]

In D58985#1353091, @imp wrote:

So you are using the raw compiler commands, unconditionally. This will likely break gcc badly.
Normally we define 'standard' macros in cdefs.h and use those everywhere we can (eg, we'd use
them in clang, but maybe not gcc if there's no support for the concept). I don't have a good
design, but I can't see this not breaking things.

So that needs to be addressed before these can be committed.

This part of a stack starting at D58983 which contains definitions for annotations in cdefs.h. I had 5 commits; thus, there are 5 differentials. Please let me know if it is preferable to squash the commits to have only one large differential to review.

[rpaulo]

sys/compat/linuxkpi/common/include/linux/compiler.h

49

I think this would not be needed anymore? Meaning we can remove lines 50 to 55?

sys/sys/pcpu.h

262

Maybe we should do this under #if ptrcheck ?

sys/sys/proc.h

1349

Do you need the unsafe cast?

[abhijeetsharma2002_gmail.com]

sys/compat/linuxkpi/common/include/linux/compiler.h

49

Hey, while this block is not needed as written, we do still want __counted_by defined here. Linux only uses it on FAM, which gcc and mainline clang both support, whereas cdefs.h only gives the real attribute under -fbounds-safety. So, as written the driver annotations become no-ops on a normal build.

sys/sys/pcpu.h

262

Sure

sys/sys/proc.h

1349

Yes. td is _single, so any plain arithmetic on it is rejected. _unsafe_indexable is what makes the arithmetic legal, and the forge converts the result back to a td_sched *.

[abhijeetsharma2002_gmail.com]

Address code review.