In D17367#376752, @alc wrote:

I *really* wish that vm_fault() knew whether the copy-on-write fault was caused by a protection violation or an invalid mapping.  I think that a better heuristic for determining whether to call vm_fault_prefault() would be based on that knowledge.  Specifically, I would call vm_fault_prefault() when the mapping was invalid.

We can try to approximately calculate this information, but it is not reliable even on x86. For instance, we get a nonsensical error code for spurious page faults. But still, translating the bits from x86 exceptional error code into some additional information to vm_fault() is easy.

This looks like a hack, and deserves a comment at least, if applied.

Can you use --shortstat instead of parsing sha's ?

Looks fine from PoV of libc integration. I did not looked at the algorithms.

Reword man pages changes.

In D17547#376430, @jonathan wrote:

On a more substantive point, do we set errno in the same way as Linux when openat(... O_BENEATH) fails? It might be helpful to porters if our behaviour is consistent, i.e., we don't set ENOTCAPABLE when people are expecting something like EINVAL.

Patch all manpages.

So both you and mjg mentioned that there are a lot of consumers which do not check the result from release_if_not_last. How is it handled ?

Disallow absolute paths for BENEATH.

Remove no longer needed addition of timo argument to vm_wait().

In D17615#376018, @mjg wrote:

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#x86Operandmodifiers

In D17615#376015, @kib wrote:

Wnere is the 'P' constraint documented ? I see that in gcc 8 manual, 'P' is listed and machine-specific constraint, but x86 section does not mention it.

Wnere is the 'P' constraint documented ? I see that in gcc 8 manual, 'P' is listed and machine-specific constraint, but x86 section does not mention it.

In D17587#375788, @emaste wrote:

Not supporting .ctors could be a problem when linking objects created with different compilers, e.g. a static library built with gcc, but linked through clang.

This is precisely the reason for maintaining .ctors/.dtors support; even if GCC uses .init_array/.fini_array on all platforms on FreeBSD now users may have existing .o files which do not.

In D17604#375787, @jtl wrote:

In D17604#375736, @kib wrote:

I definitely agree with the genoffset.sh changes.

On the other hand, I am not sure that we need to restore volatile qualifiers in the thread_lite, instead of removing them in the struct thread. The manipulations of the critnest level and pin count should use barriers, and I believe that the thread_lite patch and some of its follow-ups just did that. Volatile relied on the compiler-specific semantic to get similar effect.

Thanks for the review!

Here are my thoughts:

1. struct thread and struct thread_lite should use the same qualifiers. (Actually, I think they should use exactly the same types.) That way, we end up with the same behavior regardless of which is used.
2. I do think there is a use for the volatile keyword, even with compiler barriers.
   
   I could be wrong, but my understanding is that the volatile keyword keeps the compiler from assuming it can cache memory values in registers. Therefore, to increment a volatile variable, it would need to do something like this:
   
   ` inc 0x48(%rbx) ` Without the volatile qualifier, I think the following would be legal: ` mov 0x48(%rbx), %rax inc %rax mov %rax, 0x48(%rbx) ` If the value in memory changes between the load and store, it would be lost.

I do not believe that volatile makes such guarantees. I think that gcc and clang only do not allow the accesses to vanish, i.e. to optimize them based on cached values. But the load/inc/store code generation is legal for volatile x++;. In fact, the standard states that the value in the externally visible storage must be correct on the sequence point, thats all.
See the handling of amd64 pcb_flags where we do care about the interrupt safety of the value and use inline assembly instead of relying on the code generation.

Can we drop the operation, leaving the kernel structures in consistent state ?

Can you extract the helpers functions into separate review ?

Again, please upoad the context.

After some investigation, I see that crtbegin/crtend.o are shipped with compilers, at least it does with gcc. So I do not see a reason to provide the .ctor fallback for e.g. amd64 or i386: in-tree compiler does not need it, and out-of-tree gcc should cope on its own.

Can you reupload the diff with full context ?

I definitely agree with the genoffset.sh changes.

In D17547#375474, @jilles wrote:

Unconditionally allowing absolute paths with O_BENEATH seems a bit strange, as I expect this to be used for partially untrusted paths. An application can easily check for absolute paths and cause either behaviour (allowing or disallowing).

This was one of my questions. But, it is not clear to me what change, if any, you are proposing.

Fix asserted issues on i386.

Simplify retry case as suggested.

Update comments.

In D17550#374670, @yuripv wrote:

In D17550#374655, @kib wrote:

So we apparently have Y2K bug ?

If you mean the negative values for tm_year, then it likely should be called Y1.9K bug. Otherwise, we seem to have followed the standard, just didn't care about %C and %y both appearing in the format string in arbitrary order.

You probably need to add the workaround to Makefile.inc1.

So we apparently have Y2K bug ?

In D17547#374591, @emaste wrote:

Compare with D2808

Wording for stat(2).

Reword O_BENEATH explanation.

I accept absolute paths for O_BENEATH. This can be tweaked, I am not sure what is the useful approach there.

Commit _retval() change now, I agree with it.

Commit candidate, passed tinderbox.

I think that all hand-rolled string->number conversions in this file should check for overflow. Otherwise we call for undefined behavior and allow compiler authors to bully us.

Add suppression for !x86 arches.