Created attachment 253899 [details] stdio.h: don't expose rsize_t unless __EXT1_VISIBLE <stdio.h> should not unconditionally declare rsize_t; this patch fixes that by wrapping the typedef inside an `#if __EXT1_VISIBLE`, which is how it's done in <stddef.h>, <stdlib.h>, and <string.h>. In C99, rsize_t is not a reserved identifier. (POSIX reserves *_t, but in C99, there's no global *_t reservation; only int*_t and uint*_t in <stdint.h>.) As such, technically speaking this should compile: ``` $ cat conflict.c #include <stdio.h> typedef int rsize_t; $ c99 -c conflict.c conflict.c:2:13: error: typedef redefinition with different types ('int' vs 'size_t' (aka 'unsigned long')) 2 | typedef int rsize_t; | ^ /usr/include/stdio.h:55:16: note: previous definition is here 55 | typedef size_t rsize_t; | ^ 1 error generated. ``` Related history: - Problematic commit: https://github.com/freebsd/freebsd-src/commit/c13559d31e90a8c405771be36ab9ccfa41d4ebd6 - Good example: adding `typedef size_t rsize_t;` to <stddef.h> and <stdlib.h> as part of memset_s addition https://reviews.freebsd.org/D9903
I'll review and commit if needed.
It looks good to me and matches all the other places we do similar things. cy@ I'll let you land this.
Agreed. Looks good and tested out ok.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=b35f0aa4952cf03f1b2093b110607c13dd2ec991 commit b35f0aa4952cf03f1b2093b110607c13dd2ec991 Author: Graham Percival <gperciva@tarsnap.com> AuthorDate: 2024-09-30 16:37:59 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2024-10-01 03:07:43 +0000 stdio.h: don't expose rsize_t unless __EXT1_VISIBLE PR: 281768 Fixes: c13559d31e90 MFC after: 1 week include/stdio.h | 3 +++ 1 file changed, 3 insertions(+)
While I'm upgrading the multimedia/handbrake port, I've got the following error on my FreeBSD current box. This source is build with '-D_FORTIFY_SOURCE=2' and defines '#define _XOPEN_SOURCE 600' at the top of the source. '_XOPEN_SOURCE 600' won't define '__EXT1_VISIBLE' so that 'rsize_t' is hidden. '-D_FORTIFY_SOURCE=2' pulls ssp/stdio.h and 'rsize_t' is used in the 'gets_s' prototype. So please hide away 'gets_s' prototype unless __EXT1_VISIBLE is set. ``` /usr/bin/cc -I. -I./ -I/home/yuichiro/FreeBSD/ports/multimedia/handbrake/work/Ha ndBrake-1.8.2/build/contrib/include -DLIBICONV_PLUG -D_ISOC11_SOURCE -D_FILE_OFF SET_BITS=64 -D_LARGEFILE_SOURCE -DZLIB_CONST -DHAVE_AV_CONFIG_H -DBUILDING_avuti l -I/usr/local/include -DLIBICONV_PLUG -isystem /usr/local/include -O2 -pipe -m arch=skylake -DLIBICONV_PLUG -fstack-protector-strong -isystem /usr/local/includ e -fno-strict-aliasing -fstack-protector-strong -D_FORTIFY_SOURCE=2 -I/home/yuic hiro/FreeBSD/ports/multimedia/handbrake/work/HandBrake-1.8.2/build/contrib/inclu de -DNDEBUG -std=c17 -fomit-frame-pointer -I/usr/local/include -pthread -I/home /yuichiro/FreeBSD/ports/multimedia/handbrake/work/HandBrake-1.8.2/build/contrib/ include -pthread -I/usr/local/include -I/usr/local/include/libdrm -I/home/yuichi ro/FreeBSD/ports/multimedia/handbrake/work/HandBrake-1.8.2/build/contrib/include -I/usr/local/libdata/pkgconfig/../../include -I/usr/local/libdata/pkgconfig/../ ../include/vpl -DMFX_DEPRECATED_OFF -I/usr/local/include/opus -I/usr/local/inclu de/opus -I/usr/local/include -I/usr/local/include -I/usr/local/include -I/home/y uichiro/FreeBSD/ports/multimedia/handbrake/work/HandBrake-1.8.2/build/contrib/in clude -I/usr/local/include -I/usr/local/include -I/usr/local/include -I/usr/loca l/include -I/usr/local/include -I/usr/local/include -I/usr/local/include -I/usr/ local/include/libdrm -I/usr/local/include -I/usr/local/include -Wdeclaration-aft er-statement -Wall -Wdisabled-optimization -Wpointer-arith -Wredundant-decls -Ww rite-strings -Wtype-limits -Wundef -Wmissing-prototypes -Wstrict-prototypes -Wem pty-body -Wno-parentheses -Wno-switch -Wno-format-zero-length -Wno-pointer-sign -Wno-unused-const-variable -Wno-bool-operation -Wno-char-subscripts -O3 -fno-mat h-errno -fno-signed-zeros -mstack-alignment=16 -Qunused-arguments -Werror=implic it-function-declaration -Werror=missing-prototypes -Werror=return-type -MMD -M F libavutil/error.d -MT libavutil/error.o -c -o libavutil/error.o libavutil/erro r.c In file included from libavutil/error.c:21: In file included from /usr/include/stdio.h:256: /usr/include/ssp/stdio.h:53:46: error: unknown type name 'rsize_t'; did you mean 'size_t'? 53 | __ssp_redirect(char *, gets_s, (char *__buf, rsize_t __len), (__buf, __l en)); | ^ /usr/include/stdio.h:47:18: note: 'size_t' declared here 47 | typedef __size_t size_t; | ^ In file included from libavutil/error.c:21: In file included from /usr/include/stdio.h:256: /usr/include/ssp/stdio.h:53:46: error: unknown type name 'rsize_t'; did you mean 'size_t'? 53 | __ssp_redirect(char *, gets_s, (char *__buf, rsize_t __len), (__buf, __len)); | ^ /usr/include/stdio.h:47:18: note: 'size_t' declared here 47 | typedef __size_t size_t; | ^ In file included from libavutil/error.c:21: In file included from /usr/include/stdio.h:256: /usr/include/ssp/stdio.h:53:46: error: unknown type name 'rsize_t'; did you mean 'size_t'? 53 | __ssp_redirect(char *, gets_s, (char *__buf, rsize_t __len), (__buf, __len)); | ^ /usr/include/stdio.h:47:18: note: 'size_t' declared here 47 | typedef __size_t size_t; | ^ 3 errors generated. ```
finance/gnucash is also regressed: FAILED: common/test-core/CMakeFiles/unittest_support.dir/swig-unittest-support-python.c.o /ccache/libexec/ccache/cc -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_56 -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_56 -DHAVE_CONFIG_H -DHAVE_GUILE30 -D_GNU_SOURCE -Dunittest_support_EXPORTS -I/wrkdirs/usr/ports/finance/gnucash/work/gnucash-5.9/common -I/wrkdirs/usr/ports/finance/gnucash/work/gnucash-5.9/libgnucash/engine -I/wrkdirs/usr/ports/finance/gnucash/work/gnucash-5.9/common/test-core -I/usr/local/include/guile/3.0 -I/usr/local -I/usr/local/include/python3.11 -I/wrkdirs/usr/ports/finance/gnucash/work/gnucash-5.9/borrowed/libc -I/wrkdirs/usr/ports/finance/gnucash/work/gnucash-5.9/libgnucash/core-utils -isystem /usr/local/include/glib-2.0 -isystem /usr/local/lib/glib-2.0/include -Werror -Wall -Wmissing-prototypes -Wmissing-declarations -O2 -pipe -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -O3 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Werror -Wall -Wmissing-prototypes -Wmissing-declarations -O2 -pipe -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -std=gnu11 -fPIC -MD -MT common/test-core/CMakeFiles/unittest_support.dir/swig-unittest-support-python.c.o -MF common/test-core/CMakeFiles/unittest_support.dir/swig-unittest-support-python.c.o.d -o common/test-core/CMakeFiles/unittest_support.dir/swig-unittest-support-python.c.o -c /wrkdirs/usr/ports/finance/gnucash/work/gnucash-5.9/common/test-core/swig-unittest-support-python.c In file included from /wrkdirs/usr/ports/finance/gnucash/work/gnucash-5.9/common/test-core/swig-unittest-support-python.c:198: In file included from /usr/local/include/python3.11/Python.h:24: In file included from /usr/include/stdio.h:256: /usr/include/ssp/stdio.h:53:46: error: unknown type name 'rsize_t' 53 | __ssp_redirect(char *, gets_s, (char *__buf, rsize_t __len), (__buf, __len)); | ^ /usr/include/ssp/stdio.h:53:46: error: unknown type name 'rsize_t' /usr/include/ssp/stdio.h:53:46: error: unknown type name 'rsize_t' 3 errors generated.
I have reverted this commit as it broke a number of port builds and assigned the PR to portmgr@ for an exp-run.
Right. The problematic commit for /usr/include/ssp/stdio.h is https://reviews.freebsd.org/D45679 from 2024-06-21; in particular, the __ssp_redirect(char *, gets_s, (char *__buf, rsize_t __len), (__buf, __len)); line (which is not hidden by __EXT1_VISIBLE). That's the cause of the two reports in this bugzilla issue, at least. My tentative suggestion is to fix that, wait a week or two to see if anything breaks, then revisit the rsize_t issue? I can make a patch and check with a few ports, but maybe there's a better way of doing it?
(In reply to Graham Percival from comment #8) Sure, submit a phabricator review and add kevans@ as a reviewer.
(In reply to Cy Schubert from comment #9) Yeah, sorry, this was reported to me by HardenedBSD as well. I'll fix that here soon-ish- the redirect should've been gated, but I overlooked it when I was looking for other definitions that should've been made conditional.
(In reply to Kyle Evans from comment #10) I'll re-commit the attached patch then. Do we still need an exp-run? IMO possibly.
(In reply to Cy Schubert from comment #11) I fixed the FORTIFY header in c25e55bcf80b7fc5384c34948404ae9d65c29bab now. re: exp-run, it's probably a good idea since we're limiting the visibility of a type that's been exposed for at least a couple of releases (2018 would put its debut around... 12.0 and maybe 11.x > 0?)- you never know what might have crept in.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=eb84c129d6d6b82238575946ba3552c9310f24f9 commit eb84c129d6d6b82238575946ba3552c9310f24f9 Author: Graham Percival <gperciva@tarsnap.com> AuthorDate: 2024-09-30 00:06:29 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2024-10-02 18:01:39 +0000 stdio.h: don't expose rsize_t unless __EXT1_VISIBLE This is how the other typedefs for rsize_t handle it (in <stddef.h>, <stdlib.h>, and <string.h>). In particular, we shouldn't have any rsize_t if a C environment earlier C11 was requested. This reapplies b35f0aa4952c, chasing c25e55bcf80b, fixing ports build failures following b35f0aa4952c without c25e55bcf80b. PR: 281768 Sponsored by: Tarsnap Backup Inc. Signed-off-by: Graham Percival include/stdio.h | 3 +++ 1 file changed, 3 insertions(+)
Thanks for the quick fix. I confirmed multimedia/handbrake build finished successfully. Probably the 'gets_s' manual page needs to be updated as same as 'qsort_s' requires "#define __STDC_WANT_LIB_EXT1__ 1".
(In reply to Yuichiro NAITO from comment #14) Can you open a new PR for this, please? This is a different issue; helps us keep track of it. Thanks.
(In reply to Cy Schubert from comment #15) Done in #281828.
Requesting an exp-run after the commit does not make sense.