Conversation
This fixes building with upcoming GCC 15 which defaults to -std=gnu23.
|
I’ve just taken over maintainership of the This PR works well for me, particularly in combination with #14, but I’m finding that in Fedora 42 and later, which have GCC 15, I encounter some regressions in the tests: (There are a couple of spots in the I’m not prepared to dig into this deeply, so for now, I’m planning to just skip these two tests and report the problem by email. I am curious whether you encountered the same problem and whether you chose to investigate it. |
|
Let me have a look. It might be a good task for one of my mentees as well. |
|
|
|
Thank you for investigating this and distilling it into a viable upstream bug report! |
The unifdef testsuite has been failing sometimes for years with _FORTIFY_SOURCE in a non-obvious way, where glibc's fgets will return \0 w/ n=1. I'd found marxin's PR for this years ago and apparently completely forgot about it (nor did I backport the patch into Gentoo at the time, as I didn't maintain it, and I was fairly new then). Backport marxin's patch from 2022 accordingly. Thanks to Ben Beasley poking me on the upstream PR (19) which made me look at all of this again. Bug: https://gcc.gnu.org/PR120205 Bug: fanf2/unifdef#15 Bug: fanf2/unifdef#19 Closes: https://bugs.gentoo.org/836698 Signed-off-by: Sam James <sam@gentoo.org>
|
Man, what a mess. Sorry for the runaround. I'm still not maintaining unifdef downstream but it's now marked as maintainer-needed (orphaned) so only started paying any attention to it recently. TL;DR: We need #15 and fgets has interesting behaviour with |
|
I've run into this as well, packaging unifdef with GCC 15.1.0. I'm not convinced that the change in #15 is right - it modifies the expected output for the whitespace tests so that they silently split a long line in two without emitting a diagnostic, which isn't correct behaviour. I think it's probable that there is a GCC bug here, since adding _FORTIFY_SOURCE shouldn't change behaviour unless the code had UB to start with (which I don't think is the case here, and ASan/UBSan doesn't find anything). In the long run, it may be easiest to get rid of the line length limitation - e.g. use getline rather than fgets, and make tline a pointer (using realloc if necessary to ensure there's still EDITSLOP space after the line). |
I don't think UBsan knows about POSIX constraints here (or indeed often behaviour of C standard library functions unless they can be expressed via some attribute we already have, like nonnull) and the argument is that the behaviour relied upon from
The relevant code is in glibc. In __fortify_function __wur __fortified_attr_access (__write_only__, 1, 2)
__nonnull ((3)) __attribute_overloadable__ char *
fgets (__fortify_clang_overload_arg (char *, __restrict, __s), int __n,
FILE *__restrict __stream)
__fortify_clang_warning (__fortify_clang_bos_static_lt (__n, __s) && __n > 0,
"fgets called with bigger size than length of "
"destination buffer")
{
size_t __sz = __glibc_objsize (__s);
if (__glibc_safe_or_unknown_len (__n, sizeof (char), __sz))
return __fgets_alias (__s, __n, __stream);
#if !__fortify_use_clang
if (__glibc_unsafe_len (__n, sizeof (char), __sz))
return __fgets_chk_warn (__s, __sz, __n, __stream);
#endif
return __fgets_chk (__s, __sz, __n, __stream);
}
|
3 participants
This fixes building with upcoming GCC 15 which defaults to -std=gnu23.