Re: [PATCH v2 3/3] io: Add FORTIFY_SOURCE check for fcntl arguments

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 29/05/23 17:49, Sergey Bugaev wrote:
> On Mon, May 29, 2023 at 11:14 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>> Now that you brought Rust black_box, we already something similar on
>> benchtests: DO_NOT_OPTIMIZE_OUT.
> 
> I see, thanks -- but that also seems different in that it doesn't mark
> the value as the output of the asm block, so the compiler could see
> that it's returned unmodified. So it wouldn't be suitable here.

It is returned unmodified, but the asm acts a compiler barrier which gcc
documentation also declares as 'strong memory barrier' [1] (which I think was
written before C11 memory semantic).

[1] https://gcc.gnu.org/onlinedocs/gcc/Volatiles.html

> 
> Rust's black_box is / was [0] instead implemented as
> 
> llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile");

What the 'volatile' constraint does for the llvm_asm? Is is to mimic a
'asm volatile' or is something else? 

> 
> i.e. it marks the mutable reference to the value as inline asm's
> input, not the value itself.
> 
> [0]: nowadays it's a compiler intrinsic that still gets codegenned to
> a similar asm block when using the LLVM codegen backend

Yeah, without a compiler intrinsic with proper semantic we need to rely on
hacks such this asm barrier to prevent code optimization.

> 
>> The LFS names are not considered a namespace pollution, so I think that's
>> why it always provided (just check tst-fortify.c LFS name usage, like
>> pread64).
> 
> They may be always provided when building tst-fortify.c, but they're
> certainly not always provided when building user code (not even
> provided by default):
> 
> $ gcc use-fcntl64.c -Werror
> use-fcntl64.c: In function ‘main’:
> use-fcntl64.c:6:3: error: implicit declaration of function ‘fcntl64’;
> did you mean ‘fcntl’? [-Werror=implicit-function-declaration]
>     6 |   fcntl64 (0, F_GETFD);
>       |   ^~~~~~~
>       |   fcntl
> cc1: all warnings being treated as errors
> $ gcc use-fcntl64.c -D _LARGEFILE64_SOURCE -Werror
> (builds successfully)
> 
> So this has to be supported, and ideally tested too.

It is exported because all tests are actually built with _GNU_SOURCE (done
by include/libc-symbols.h), so the test check is superfluous.  It also
leaks implementation details, such as internal defines.  Usually to check 
for internal implementation we use test-internal (which are built
statically).

But if you really want to check for _LARGEFILE64_SOURCE, you will need to
add *another* fortify test that undef _GNU_SOURCE (like stdlib/tst-strtol-binary-c11.c
for instance).