Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiler warning: printf allows null string args... no (new/broken check?) #2109

Closed
desertwitch opened this issue Oct 16, 2023 · 12 comments · Fixed by #2111
Closed

compiler warning: printf allows null string args... no (new/broken check?) #2109

desertwitch opened this issue Oct 16, 2023 · 12 comments · Fixed by #2111
Labels
bug C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings
Milestone

Comments

@desertwitch
Copy link
Contributor

desertwitch commented Oct 16, 2023

Compiling on my usual (unchanged) build environment on Slackware 15.0, gcc 11.2.0:

last build from 3bf526e

checking for practical support to printf("%s", NULL)... yes
checking whether printf allows null string args... (null)ok

new build from current master (after 3bf526e)

checking for practical support to printf("%s", NULL)... yes
checking whether printf allows null string args... no
configure: WARNING: Your C library requires workarounds to print NULL values; if something crashes with a segmentation fault (especially with verbose debug) - that may be why

Attempt to re-create the check on my build system:

#include <stdlib.h>  
#include <stdio.h>
int main() 
{ 
   printf("%s", NULL); 
   return 0;
} 
(null)

I was trying to pin-point this from the commits since, but I couldn't figure out what has changed that could cause this.

@jimklimov
Copy link
Member

No, nothing was changed recently that should have impacted this...

Is it a cross-build or native (so executables can run locally)? Can something like an antivirus get in the way of running test programs, having their newly written binaries locked for its own checks - breaks a lot of active logic on Windows, for example.

The autotools config.log should have a detailed log of what it ran and what errors it got. It is usually helpful...

@desertwitch
Copy link
Contributor Author

No, nothing was changed recently that should have impacted this...

Is it a cross-build or native (so executables can run locally)? Can something like an antivirus get in the way of running test programs, having their newly written binaries locked for its own checks - breaks a lot of active logic on Windows, for example.

The autotools config.log should have a detailed log of what it ran and what errors it got. It is usually helpful...

It's a native build on Slackware 15.0 with gcc 11.2.0
No antivirus or the likes, the build environment is entirely unchanged.

Actually I've checked-out 3bf526e again just now and built it with the null arg check passing.
Afterwards I've checked-out the latest master state and built with the null arg check failing.

@desertwitch
Copy link
Contributor Author

desertwitch commented Oct 16, 2023

Here goes config.log building from current master just now:

configure:14136: checking whether printf allows null string args
configure:14214: x86_64-slackware-linux-gcc -o conftest -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration   conftest.c  >&5
conftest.c: In function 'main':
conftest.c:139:1: error: implicit declaration of function 'printf' [-Werror=implicit-function-declaration]
  139 | printf("%s", NULL)
      | ^~~~~~
conftest.c:136:1: note: include '<stdio.h>' or provide a declaration of 'printf'
  135 | #include <stdlib.h>
  +++ |+#include <stdio.h>
  136 | int
conftest.c:139:1: error: incompatible implicit declaration of built-in function 'printf' [-Werror=builtin-declaration-mismatch]
  139 | printf("%s", NULL)
      | ^~~~~~
conftest.c:139:1: note: include '<stdio.h>' or provide a declaration of 'printf'
conftest.c: At top level:
cc1: note: unrecognized command-line option '-Wno-unknown-warning-option' may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option '-Wno-reserved-identifier' may have been intended to silence earlier diagnostics
cc1: all warnings being treated as errors
configure:14214: $? = 1
configure: program exited with status 1
configure: failed program was:
| /* confdefs.h */

Here goes config.log building from 3bf526e just now:

configure:13949: checking whether printf allows null string args
configure:14023: x86_64-slackware-linux-gcc -o conftest -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99   conftest.c  >&5
conftest.c: In function 'main':
conftest.c:139:1: warning: implicit declaration of function 'printf' [-Wimplicit-function-declaration]
  139 | printf("%s", NULL)
      | ^~~~~~
conftest.c:136:1: note: include '<stdio.h>' or provide a declaration of 'printf'
  135 | #include <stdlib.h>
  +++ |+#include <stdio.h>
  136 | int
conftest.c:139:1: warning: incompatible implicit declaration of built-in function 'printf' [-Wbuiltin-declaration-mismatch]
  139 | printf("%s", NULL)
      | ^~~~~~
conftest.c:139:1: note: include '<stdio.h>' or provide a declaration of 'printf'
conftest.c: At top level:
cc1: note: unrecognized command-line option '-Wno-unknown-warning-option' may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option '-Wno-reserved-identifier' may have been intended to silence earlier diagnostics
configure:14023: $? = 0
configure:14023: ./conftest
configure:14023: $? = 0
configure:14025: result: ok

Seems the warning in the "working" build was raised to be an error in the newer master.
cc1: all warnings being treated as errors

@desertwitch
Copy link
Contributor Author

Seems I found it - error promotion comes from this change 5d1236b in #2096

@jimklimov
Copy link
Member

Interesting, seems the checking code snippet defined in configure.ac was not complete, then, thanks.

@jimklimov
Copy link
Member

Good catch - yes, that change was intentional as we had some other tests sort-of-passing when something was not really supported. But did not cross-check if some other new "errors" got discovered that way.

@desertwitch
Copy link
Contributor Author

I'm the one who's thankful for the effort you put into all of this - so, thanks ;-)

@jimklimov
Copy link
Member

So, after running some log collections with the likes of make distclean ; ./autogen.sh && ./configure --with-all 2>&1 | tee configure-9cf0811.log && cp -pf config{,-9cf0811}.log, the diffs are:

$ diff -bu configure-*.log | egrep '^[+-]'
--- configure-3bf526e.log       2023-10-16 10:06:05.955989071 +0200
+++ configure-9cf0811.log       2023-10-16 10:07:51.340478089 +0200
-Network UPS Tools version 2.8.0.1 (v2.8.0-signed-2608-g3bf526e72)
+Network UPS Tools version 2.8.0.1 (v2.8.0-signed-2667-g9cf0811d7)
+checking for pragma GCC diagnostic ignored "-Wcast-function-type-strict"... no
+checking for pragma GCC diagnostic ignored "-Wcast-function-type-strict" (outside functions)... no
-checking for practical support to printf("%s", NULL)... yes
+checking for practical support to printf("%s", NULL)... no
-checking whether printf allows null string args... (null)ok
+checking whether printf allows null string args... no
+configure: WARNING: Your C library requires workarounds to print NULL values; if something crashes with a segmentation fault (especially with verbose debug) - that may be why
+checking for inet_pton() with IPv4 and IPv6 support... 11yes
+checking for struct pollfd... yes
-checking if your systemd version supports Type=notify-reload... /home/jim/nut/systemd-analyze-ebCdQG.service:5: Failed to parse service type, ignoring: notify-reload
+checking if your systemd version supports Type=notify-reload... /home/jim/nut/systemd-analyze-gmrEmj.service:5: Failed to parse service type, ignoring: notify-reload
-checking if current toolkit can build and run cppunit tests (e.g. no ABI issues, related segfaults, etc.)... .
-
-yes
-checking for impact from --enable-cppunit option - should we build cppunit tests?... yes
-checking whether to build C++ tests with CPPUNIT... yes
+checking if current toolkit can build and run cppunit tests (e.g. no ABI issues, related segfaults, etc.)... no
+checking for impact from --enable-cppunit option - should we build cppunit tests?... no
+checking whether to build C++ tests with CPPUNIT... no
-* configured version:  2.8.0.1 (v2.8.0-signed-2608-g3bf526e72)
+* configured version:  2.8.0.1 (v2.8.0-signed-2667-g9cf0811d7)
-* build C++ tests with CPPUNIT:        yes
+* build C++ tests with CPPUNIT:        no
  • config.log changes are in thousands of lines, mostly due to line number changes, so won't show here.

So it apparently also "lost" cppunit tests. Thanks again :)

@desertwitch
Copy link
Contributor Author

desertwitch commented Oct 16, 2023

Can confirm this works as intended when I patch in an inclusion of stdio.h:

diff --git a/configure.ac b/configure.ac
index 66821dde4..3e9977828 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1030,7 +1030,8 @@ dnl The following code crashes on some libc implementations (e.g. Solaris 8)
 dnl TODO: We may need to update NUT codebase to use NUT_STRARG() macro more
 dnl often and consistently, or find a way to tweak upsdebugx() etc. varargs
 AX_RUN_OR_LINK_IFELSE([AC_LANG_PROGRAM(
-    [#include <stdlib.h>],
+    [#include <stdlib.h>
+#include <stdio.h>],
     [printf("%s", NULL)]
 )],
     [
configure:14136: checking whether printf allows null string args
configure:14216: x86_64-slackware-linux-gcc -o conftest -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration   conftest.c  >&5
configure:14216: $? = 0
configure:14216: ./conftest
configure:14216: $? = 0
configure:14218: result: ok

@jimklimov
Copy link
Member

Huh, it seems there are two tests historically - in the main configure.ac script and in m4:

  • configure.ac:
configure:15546: checking whether printf allows null string args
configure:15626: gcc -o conftest -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration   conftest.c  >&5
conftest.c: In function 'main':
conftest.c:149:1: error: implicit declaration of function 'printf' [-Werror=implicit-function-declaration]
  149 | printf("%s", NULL)
      | ^~~~~~
conftest.c:146:1: note: include '<stdio.h>' or provide a declaration of 'printf'
  145 | #include <stdlib.h>
  +++ |+#include <stdio.h>
  146 | int
conftest.c:149:1: error: incompatible implicit declaration of built-in function 'printf' [-Werror=builtin-declaration-mismatch]
  149 | printf("%s", NULL)
      | ^~~~~~
conftest.c:149:1: note: include '<stdio.h>' or provide a declaration of 'printf'
conftest.c:149:10: error: format '%s' expects argument of type 'char *', but argument 2 has type 'void *' [-Werror=format=]
  149 | printf("%s", NULL)
      |         ~^
      |          |
      |          char *
      |         %p
conftest.c:149:1: error: '%s' directive argument is null [-Werror=format-overflow=]
  149 | printf("%s", NULL)
      | ^~~~~~~~~~~~~~~~~~
conftest.c: At top level:
cc1: note: unrecognized command-line option '-Wno-unknown-warning-option' may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option '-Wno-reserved-identifier' may have been intended to silence earlier diagnostics
cc1: all warnings being treated as errors
configure:15626: $? = 1
configure: program exited with status 1
configure: failed program was:
...
| #include <stdlib.h>
| int
| main (void)
| {
| printf("%s", NULL)
|
|   ;
|   return 0;
| }
configure:15641: result: no
configure:15646: WARNING: Your C library requires workarounds to print NULL values; if something crashes with a segmentation fault (especially with verbose debug) - that may be why
  • m4/ax_c_pragmas.m4 AX_C_PRINTF_STRING_NULL:
configure:13545: checking for practical support to printf("%s", NULL)
configure:13641: gcc -o conftest -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration   conftest.c  >&5
In file included from /usr/include/stdio.h:894,
                 from conftest.c:93:
In function 'snprintf',
    inlined from 'main' at conftest.c:101:11:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: error: '%s' directive argument is null [-Werror=format-truncation=]
   71 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   72 |                                    __glibc_objsize (__s), __fmt,
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   73 |                                    __va_arg_pack ());
      |                                    ~~~~~~~~~~~~~~~~~
conftest.c: At top level:
cc1: note: unrecognized command-line option '-Wno-unknown-warning-option' may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option '-Wno-reserved-identifier' may have been intended to silence earlier diagnostics
cc1: all warnings being treated as errors
configure:13641: $? = 1
configure: program exited with status 1
configure: failed program was:
...
| /* end confdefs.h.  */
|
| #include <stdlib.h>
| #include <string.h>
| #include <stdio.h>
|
| int
| main (void)
| {
|
| char buf[128];
| char *s = NULL;
| int res = snprintf(buf, sizeof(buf), "%s", s);
| buf[sizeof(buf)-1] = '\0';
| if (res < 0) {
|     fprintf(stderr, "FAILED to snprintf() a NULL string argument\n");
|     return 1;
| }
| if (buf[0] == '\0') {
|     fprintf(stderr, "RETURNED empty string from snprintf() with a NULL string argument\n");
|     return 0;
| }
| if (strstr(buf, "null") == NULL) {
|     fprintf(stderr, "RETURNED some string from snprintf() with a NULL string argument: '%s'\n", buf);
|     return 0;
| }
| fprintf(stderr, "SUCCESS: RETURNED a string that contains something like 'null' from snprintf() with a NULL string argument: '%s'\n", buf);
| return 0;
|
|   ;
|   return 0;
| }
configure:13657: result: no

@jimklimov
Copy link
Member

jimklimov commented Oct 16, 2023

So with a fix like yours, it still rejects the change for my test system under hands (WSL2 Ubuntu 22.04, gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0):

configure:15546: checking whether printf allows null string args
configure:15632: gcc -o conftest -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration   conftest.c  >&5
conftest.c: In function 'main':
conftest.c:152:10: error: format '%s' expects argument of type 'char *', but argument 2 has type 'void *' [-Werror=format=]
  152 | printf("%s", NULL)
      |         ~^
      |          |
      |          char *
      |         %p
In file included from /usr/include/stdio.h:894,
                 from conftest.c:147:
In function 'printf',
    inlined from 'main' at conftest.c:152:1:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:112:10: error: '%s' directive argument is null [-Werror=format-overflow=]
  112 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
conftest.c: At top level:
cc1: note: unrecognized command-line option '-Wno-unknown-warning-option' may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option '-Wno-reserved-identifier' may have been intended to silence earlier diagnostics
cc1: all warnings being treated as errors
configure:15632: $? = 1
configure: program exited with status 1
configure: failed program was:
...
| /* end confdefs.h.  */
|
| #include <stdlib.h>
| #include <stdio.h>
|
| int
| main (void)
| {
| printf("%s", NULL)
|
|   ;
|   return 0;
| }
configure:15647: result: no
configure:15652: WARNING: Your C library requires workarounds to print NULL values; if something crashes with a segmentation fault (especially with verbose debug) - that may be why

Both tests seem to fail with master-branch codebase in fact:

-checking for practical support to printf("%s", NULL)... yes
+checking for practical support to printf("%s", NULL)... no
-checking whether printf allows null string args... (null)ok
+checking whether printf allows null string args... no

In the older version, the m4 test did complain (warnings about NULL) but did "succeed" as far as practice goes (despite complaints, current compiler+libc do provide a fallback):

configure:13418: checking for practical support to printf("%s", NULL)
configure:13506: gcc -o conftest -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99   conftest.c  >&5
In file included from /usr/include/stdio.h:894,
                 from conftest.c:90:
In function 'snprintf',
    inlined from 'main' at conftest.c:99:11:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: warning: '%s' directive argument is null [-Wformat-truncation=]
   71 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   72 |                                    __glibc_objsize (__s), __fmt,
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   73 |                                    __va_arg_pack ());
      |                                    ~~~~~~~~~~~~~~~~~
conftest.c: At top level:
cc1: note: unrecognized command-line option '-Wno-unknown-warning-option' may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option '-Wno-reserved-identifier' may have been intended to silence earlier diagnostics
configure:13506: $? = 0
configure:13506: ./conftest
SUCCESS: RETURNED a string that contains something like 'null' from snprintf() with a NULL string argument: '(null)'
configure:13506: $? = 0
configure:13520: result: yes

So I guess I need a way to pass -Wno-... into this link/run call to allow for NULL if it works practically, and to squash the two implementations :)

For context, this is part of the bugfix effort that introduced NUT_STRARG (on some systems, upsdebugx() printouts segfaulted due to NULL string parameters) but it is far from being used everywhere yet.

@desertwitch
Copy link
Contributor Author

desertwitch commented Oct 16, 2023

Interesting, the m4 test always passed for me, the configure.ac test now passes (but only with the include patched in).

jimklimov added a commit to jimklimov/nut that referenced this issue Oct 16, 2023
jimklimov added a commit to jimklimov/nut that referenced this issue Oct 16, 2023
…_OR_LINK_IFELSE option to ignore certain warnings [networkupstools#2109]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Oct 16, 2023
jimklimov added a commit to jimklimov/nut that referenced this issue Oct 16, 2023
jimklimov added a commit to jimklimov/nut that referenced this issue Oct 16, 2023
@jimklimov jimklimov added bug refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings labels Oct 16, 2023
@jimklimov jimklimov added this to the 2.8.1 milestone Oct 16, 2023
@jimklimov jimklimov added the C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants