[PATCH] fortify: further improve Clang style fortify

[Adhemerval Zanella Netto]

On 27/03/25 14:43, Adrian Ratiu wrote:
> Hi Adhemerval and thank you for the prompt response!
> 
> On Thu, 27 Mar 2025, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote:
>>
>> These should be considered real bugs and have proper regression testcases.  Do you have any examples of which fortify wrappers are not being triggered correctly? 
> 
> Yes, all the ones affected by the objsize vs objsize0 mismatch which I'm touching here.
> 
> In version 2, which will be patch series for better clarity, I will provide an explicit list. From the top of my head the memset/memcpy/mempcpy/memmove & co, including their wcsmbs counterparts.

Thanks.

> 
>>
>> Unfortunately we don't have tests for the compiler warning itself, just for the runtime behavior.  And I recently updated my clang branch [1] and I don't see any regression with debug fortify tests with clang 18/19/20/main.
>> I also don't see any issue withg clang 18/19/20/main when used just as test  compiler (TEST_CC/TEST_CXX).
>> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/clang
> 
> I'm testing this by building glibc with GCC (currently 10.2, in the process of upgrading to 14.2.1 from Gentoo), then building and running the tests linked in the patch description [1] with clang 20 against those glibc headers on amd64.
> 
> I can create minimal reproducing C testcases, to avoid for e.g. the CPP GTest suite we use in CrOS.

If you could add then either on debug/tst-fortify.c or in an extra extra it would
be helpful.  The tst-fortify.c is the simplest because we test with a lot of
permutation (_FORTIFY_SOURCE equal 1,2,3, C/C++, etc.)

> 
> Also these are not quite regressions due to compiler versions or glibc itself, they just never worked and are the last missing pieces. :)

We do have tests that check for invalid mem* and wmem* on tst-fortify.c, so
they catch missing *_chk calls from compiler/fortify header and also missing
abort exceptions due invalid calls. We don't test invalid calls where the
fortify wrappers will thrown a compile warning.

> For some fun context, with glibc 2.41 the ChromiumOS downstream clang fortify patch went from:
> 17 files changed, 1114 insertions(+), 621 deletions(-) to 7 files changed, 67 insertions(+), 36 deletions(-)
> 
> The remaining 67(+), 36(-) is exactly this patch, so sweet! :)

Nice.

> 
>>
>> This seems not really related to the described issues in commit message and not really clang-specific since gcc also supports __format__ attribute.  So I think we should be a different change.
> 
> Yes, I'll split this into separate smaller patches, each with their own separate reasoning, because indeed there are multiple separate issues here.
> 
> Thanks again for everything!