For example building binutils HEAD get the following warning: ``` In file included from /usr/include/features.h:486, from /usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33, from /usr/include/stdio.h:27, from as.h:42, from read.c:33: In function ‘mbstowcs’, inlined from ‘read_symbol_name’ at read.c:1670:11: /usr/include/x86_64-linux-gnu/bits/stdlib.h:115:10: error: argument 1 is null but the corresponding size argument 3 value is [128, 9223372036854775807] [-Werror=nonnull] 115 | return __glibc_fortify_n (mbstowcs, __len, sizeof (wchar_t), | ^~~~~~~~~~~~~~~~~ In file included from /usr/include/stdlib.h:1027, from as.h:44: /usr/include/x86_64-linux-gnu/bits/stdlib.h: In function ‘read_symbol_name’: /usr/include/x86_64-linux-gnu/bits/stdlib.h:95:15: note: in a call to function ‘__mbstowcs_chk’ declared with attribute ‘access (read_only, 2)’ 95 | extern size_t __mbstowcs_chk (wchar_t *__restrict __dst, ``` Which is from: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gas/read.c;h=c6ce0345892284a48e5280ebb9cca111fab975f4;hb=HEAD#l1670 ``` 1670: if (mbstowcs (NULL, name, len) == (size_t) -1) ``` `mbstowcs` supports a NULL destination: https://en.cppreference.com/w/cpp/string/multibyte/mbstowcs#:~:text=POSIX%20specifies%20a%20common%20extension%3A%20if%20dst%20is%20a%20null%20pointer%2C%20this%20function%20returns%20the%20number%20of%20wide%20characters%20that%20would%20be%20written%20to%20dst%2C%20if%20converted.%20Similar%20behavior%20is%20standard%20for%20std%3A%3Ambsrtowcs. so that seems like a bug in the GLIBC attributes of `mbstowcs`.
Agreed, the __attr_access ((__write_only__, 1, 3)) is incorrect for mbstowcs, the former requires the size argument to accurately reflect the object argument (i.e. 0 for a NULL pointer) whereas the mbstowcs implementation is expected to ignore the size argument if the first argument is NULL. Would you like to write a patch to fix this up?
Sure(In reply to Siddhesh Poyarekar from comment #1) > Agreed, the __attr_access ((__write_only__, 1, 3)) is incorrect for > mbstowcs, the former requires the size argument to accurately reflect the > object argument (i.e. 0 for a NULL pointer) whereas the mbstowcs > implementation is expected to ignore the size argument if the first argument > is NULL. Would you like to write a patch to fix this up? Sure.
Created attachment 14157 [details] Potential Patch Not able to reproduce the issue in glibc test suite so not fully confident in this change. Siddhesh, any ideas for how to get this error to throw in testmb.c?
I'm not very picky about having a test for this FWIW, this seems very sensitive to optimizations and may not survive gcc updates. As for the fix, just remove the __attr_access ((__write_only__, 1, 3)), it's incorrect in the context of mbstowcs definition.
(In reply to Siddhesh Poyarekar from comment #4) > I'm not very picky about having a test for this FWIW, this seems very > sensitive to optimizations and may not survive gcc updates. > > As for the fix, just remove the __attr_access ((__write_only__, 1, 3)), > it's incorrect in the context of mbstowcs definition. You don't think its worth preserving if dst is non-null? In that case it is meaningful no?
(In reply to Noah Goldstein from comment #5) > (In reply to Siddhesh Poyarekar from comment #4) > > I'm not very picky about having a test for this FWIW, this seems very > > sensitive to optimizations and may not survive gcc updates. > > > > As for the fix, just remove the __attr_access ((__write_only__, 1, 3)), > > it's incorrect in the context of mbstowcs definition. > > You don't think its worth preserving if dst is non-null? In that case it is > meaningful no? It is meaningful, just that the check may trigger if the dest pointer has not yet been simplified to a const. Although, maybe I'm overthinking it. Lets go with what you have.
This has been fixed in glibc 2.36 with this commit: commit 464d189b9622932a75302290625de84931656ec0 Author: Noah Goldstein <goldstein.w.n@gmail.com> Date: Wed Jun 22 08:24:21 2022 -0700 stdlib: Remove attr_write from mbstows if dst is NULL [BZ: 29265] mbstows is defined if dst is NULL and is defined to special cased if dst is NULL so the fortify objsize check if incorrect in that case. Tested on x86-64 linux. Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>