From a28507e0b117c76cddfddd7cdab636882d02db8a Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Wed, 7 Apr 2021 21:26:16 -0700 Subject: [PATCH] Apply new CentOS fix for strdup null-test bug (re: 7afb30e) (#255) This is an update to one of Red Hat's patches. The strdup change is from CentOS: https://git.centos.org/rpms/ksh/blob/c8s/f/SOURCES/ksh-20120801-annocheck.patch The reason why gcc (and also clang) optimize out the null check is because the glibc string.h header gives 's' a nonnull attribute (in other words, this is a glibc compatibility bug, not a compiler bug). Clang gives the following informative warning when compiling strdup: > /home/johno/GitRepos/KornShell/ksh/src/lib/libast/string/strdup.c:66:10: warning: nonnull parameter 's' will evaluate to 'true' on > return (s && (t = oldof(0, char, n = strlen(s) + 1, 0))) ? (char*)memcpy(t, s, n) : (char*)0; > ^ ~~ > /usr/include/string.h:172:35: note: declared 'nonnull' here > __THROW __attribute_malloc__ __nonnull ((1)); > ^ > /usr/include/sys/cdefs.h:303:44: note: expanded from macro '__nonnull' > # define __nonnull(params) __attribute__ ((__nonnull__ params)) The proper fix is to rename the function in strdup.c to '_ast_strdup'. This avoids the string.h conflict and fixes the Red Hat bug. I've also made a similar change to getopt.c, since clang was throwing a nonnull warning there as well. src/lib/libast/features/map.c (which generates FEATURE/map which is indirectly included by everything) is updated to always map getopt to _ast_getopt and strdup to _ast_strdup. --- src/lib/libast/comp/getopt.c | 6 +++++- src/lib/libast/features/map.c | 12 ++++++++---- src/lib/libast/string/strdup.c | 10 +++------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/lib/libast/comp/getopt.c b/src/lib/libast/comp/getopt.c index 586947217..01a05202e 100644 --- a/src/lib/libast/comp/getopt.c +++ b/src/lib/libast/comp/getopt.c @@ -43,8 +43,12 @@ char* optarg = 0; static int lastoptind; +/* + * Avoid a null-test optimization bug caused by glibc's headers + * by naming this function '_ast_getopt' instead of 'getopt'. + */ extern int -getopt(int argc, char* const* argv, const char* optstring) +_ast_getopt(int argc, char* const* argv, const char* optstring) { int n; diff --git a/src/lib/libast/features/map.c b/src/lib/libast/features/map.c index 3d9e74c63..660d4d5cb 100644 --- a/src/lib/libast/features/map.c +++ b/src/lib/libast/features/map.c @@ -113,9 +113,12 @@ main() #endif printf("#undef getdate\n"); printf("#define getdate _ast_getdate\n"); -#if _lib_getopt || _lib_getsubopt || _lib_getopt_long || _lib_getopt_long_only +#endif + /* libast always provides its own getopt implementation */ printf("#undef getopt\n"); printf("#define getopt _ast_getopt\n"); +#if _map_libc +#if _lib_getopt || _lib_getsubopt || _lib_getopt_long || _lib_getopt_long_only printf("#undef getsubopt\n"); printf("#define getsubopt _ast_getsubopt\n"); printf("#undef getopt_long\n"); @@ -447,9 +450,6 @@ main() printf("#undef realloc\n"); printf("#define realloc _ast_realloc\n"); printf("extern void* realloc(void*, size_t);\n"); - printf("#undef strdup\n"); - printf("#define strdup _ast_strdup\n"); - printf("extern char* strdup(const char*);\n"); #if _lib_valloc printf("#undef valloc\n"); printf("#define valloc _ast_valloc\n"); @@ -457,6 +457,10 @@ main() #endif #endif #endif + /* we always use the libast strdup implementation */ + printf("#undef strdup\n"); + printf("#define strdup _ast_strdup\n"); + printf("extern char* strdup(const char*);\n"); /* * overriding strto*() is problematic to say the least diff --git a/src/lib/libast/string/strdup.c b/src/lib/libast/string/strdup.c index e2a8be5b6..7a17b45d2 100644 --- a/src/lib/libast/string/strdup.c +++ b/src/lib/libast/string/strdup.c @@ -51,19 +51,15 @@ __STDPP__directive pragma pp:nohide strdup #endif /* - * Work around a null-test optimization bug in GCC. + * Avoid a null-test optimization bug caused by glibc's headers + * by naming this function '_ast_strdup' instead of 'strdup'. * https://bugzilla.redhat.com/1221766 */ -#pragma GCC push_options -#pragma GCC optimize ("O0") - extern char* -strdup(register const char* s) +_ast_strdup(const char* s) { register char* t; register int n; return (s && (t = oldof(0, char, n = strlen(s) + 1, 0))) ? (char*)memcpy(t, s, n) : (char*)0; } - -#pragma GCC pop_options