common.c/.h: warning: '%s' directive argument is null [-Wformat-overflow=] #2585

desertwitch · 2024-08-07T06:34:39Z

Getting a couple of compiler warnings while building recent master with gcc version 13.2.0 on Slackware 15.1 (-current)

In file included from common.c:21:
common.c: In function 'sendsignalpid':
common.c:1356:25: warning: '%s' directive argument is null [-Wformat-overflow=]
 1356 |                         "%s: ran at least one check, and all such checks "
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/common.h:423:69: note: in definition of macro 'upsdebugx'
  423 |         do { if (nut_debug_level >= (level)) { s_upsdebugx((level), __VA_ARGS__); } } while(0)
      |                                                                     ^~~~~~~~~~~
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

The text was updated successfully, but these errors were encountered:

jimklimov · 2024-08-07T06:59:44Z

That's odd: the first argument there (that the arrow points to) is __func__ and the pattern is used in a lot of places.

What it might more likely complain about is NUT_STRARG() in cases where its argument is definitely NULL in that situation (at compile-time). It is present at this print-out, but there are code-paths that could set the value so it is not always null here.

nut/common/common.c

Lines 1355 to 1367 in 171eb40

    
           upsdebugx(1, 
        
           	"%s: ran at least one check, and all such checks " 
        
           	"found a process name for PID %" PRIuMAX " and " 
        
           	"failed to match: " 
        
           	"found procname='%s', " 
        
           	"expected progname='%s' (res=%d%s), " 
        
           	"current progname='%s' (res=%d%s)", 
        
           	__func__, (uintmax_t)pid, 
        
           	NUT_STRARG(procname), 
        
           	NUT_STRARG(progname), cpn1, 
        
           	(cpn1 == -10 ? ": did not check" : ""), 
        
           	NUT_STRARG(current_progname), cpn2, 
        
           	(cpn2 == -10 ? ": did not check" : ""));

In a grander scheme of things, some compiler+libc combos:

print a string like (null) in case they get a %s format string and NULL as its value,
others complain like here, but do print in fact,
a few do crash with SIGSEGV

The macro is defined here:

nut/include/str.h

Lines 32 to 40 in 304d41e

    
           /* Some compilers and/or C libraries do not handle printf("%s", NULL) correctly */ 
        
           #ifndef NUT_STRARG 
        
           # if (defined REQUIRE_NUT_STRARG) && (REQUIRE_NUT_STRARG == 0) 
        
           #  define NUT_STRARG(x) x 
        
           # else 
        
           /* Is required, or not defined => err on safe side */ 
        
           #  define NUT_STRARG(x) (x?x:"(null)") 
        
           # endif 
        
           #endif

It aims to do nothing on platforms where configure tested the local printf() to do the safe thing correctly, or inject a (null) string on those that would crash. Occasionally a code path gets developed where the "do nothing" option yields a known NULL always, and static analysis in compilers finds that, which is helpful in its way.

desertwitch · 2024-08-07T07:21:24Z

Hi Jim, thanks for the express response.
I can confirm that a dirty patch (for sake of confirming your suspicion) of that code passage resolves the situation:

/* Some compilers and/or C libraries do not handle printf("%s", NULL) correctly */
#ifndef NUT_STRARG
# if (defined REQUIRE_NUT_STRARG) && (REQUIRE_NUT_STRARG == 0)
#  define NUT_STRARG(x) (x?x:"(null)")
# else
/* Is required, or not defined => err on safe side */
#  define NUT_STRARG(x) (x?x:"(null)")
# endif
#endif

Would it cause (portability) problems or significant overhead to always err on the safe side with:
define NUT_STRARG(x) (x?x:"(null)")
... as opposed to trying to detect what the compiler is capable of (yet may still complain about)?

jimklimov · 2024-08-07T07:37:44Z

Probably such big hammer would not hurt much, after all we have tests/nutlogtest.c to try and catch those systems where it would actually fail. It is a tiny bit of an efficiency loss, if we do such a check in vain (often in debug-logged loops) and it is not needed in the first place, so the first shot in this area was done like this in #904 and later in #2109 / #2111 (heh, due to your report too).

Ideally I'd probably wrap all string printouts into the macro for the peace of mind, but that is a far-away dream just due to sheer scale of the effort and lack of time (help would be welcome though).

TBH, here I am more puzzled about why it complains in the first place, and if this is the only spot - maybe I did miss some code path while walking it by eyesight, and something conspires to always have a NULL there indeed (so maybe a coding bug was caught)? Maybe some special ifdef combo? :)

After all, Slackware 15 is among tested systems, and is not the only one with a compiler that complains of such things, and there are no failures or even warnings on NUT CI...

desertwitch · 2024-08-07T07:43:03Z

Probably such big hammer would not hurt much, after all we have tests/nutlogtest.c to try and catch those systems where it would actually fail. It is a tiny bit of an efficiency loss, if we do such a check in vain (often in debug-logged loops) and it is not needed in the first place, so the first shot in this area was done like this in #904 and later in #2109 / #2111 (heh, due to your report too).

Ideally I'd probably wrap all string printouts into the macro for the peace of mind, but that is a far-away dream just due to sheer scale of the effort and lack of time (help would be welcome though).

TBH, here I am more puzzled about why it complains in the first place, and if this is the only spot - maybe I did miss some code path while walking it by eyesight, and something conspires to always have a NULL there indeed (so maybe a coding bug was caught)? Maybe some special ifdef combo? :)

After all, Slackware 15 is among tested systems, and is not the only one with a compiler that complains of such things, and there are no failures or even warnings on NUT CI...

FWIW it does compile without problems on Slackware 15.0 with older GCC version 11.2.0 (which, I think, is what we do on CI)

jimklimov · 2024-08-07T07:57:51Z

Is this the same type of build in both systems, or a cross-build etc.? Looking at m4/ax_c_pragmas.m4 I see that the test is done with AX_RUN_OR_LINK_IFELSE macro, which in cross-builds (where the building host can compile and link but not run the result) could proclaim support based on linking success (and ultimately REQUIRE_NUT_STRARG as 0/false), although the program might segfault on the target platform.

This is a bit of an opposite of your report though, the program does not crash.

Can you check in your config.log if there is anything interesting near checking for practical support to printf lines, e.g. does it only compile or also run, and what verdict comes up?

jimklimov · 2024-08-07T07:59:36Z

For additional information, I get three warnings (in total) throughout the compile process:

Thanks, they all seem to be in this same line. So it is less about the macro and more about some way for compiler static analysis to find the problem. Or, chances are, to hallucinate one.

desertwitch · 2024-08-07T08:01:59Z

configure:14801: checking for __attribute__((unused)) for function arguments
configure:14824: gcc -c -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99  conftest.c >&5
configure:14824: $? = 0
configure:14836: result: yes
configure:14839: checking for __attribute__((unused)) for functions
configure:14862: gcc -c -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99  conftest.c >&5
configure:14862: $? = 0
configure:14874: result: yes
configure:14877: checking for __attribute__((noreturn))
configure:14900: gcc -c -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99  conftest.c >&5
configure:14900: $? = 0
configure:14912: result: yes
configure:14921: checking for at least some __attribute__ support
configure:14938: result: yes
configure:14982: checking for practical support to printf("%s", NULL)
configure:15107: gcc -o conftest -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration -Wno-format-truncation -Wno-format-overflow
       conftest.c  >&5
configure:15107: $? = 0
configure:15107: ./conftest
SUCCESS: RETURNED a string that contains something like 'null' from snprintf() with a variable NULL string argument: '(null)'
configure:15107: $? = 0
configure:15127: result: yes
configure:15178: checking for cstdbool
configure:15178: gcc -c -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99  conftest.c >&5
conftest.c:135:10: fatal error: cstdbool: No such file or directory
  135 | #include <cstdbool>
      |          ^~~~~~~~~~
compilation terminated.
configure:15178: $? = 1

Exact same build process, configuration and no cross-building or anything else out of the ordinary.

For additional information, I get three warnings (in total) throughout the compile process:

* CC      	: gcc
* CFLAGS  	: -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -s -Wno-system-headers -Wall -Wextra -Wsign-compare -pedantic -Wno-error
* CXX     	: g++
* CXXFLAGS	: -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu++11 -Wno-system-headers -Wall -Wextra -Wno-error
* CPP     	: gcc -E
* CPPFLAGS	: 
* CONFIG_FLAGS	: --build=x86_64-slackware-linux --prefix=/usr --libdir=/usr/lib64 --sysconfdir=/etc/nut --localstatedir=/var --datadir=/usr/share/nut --mandir=/usr/man --enable-shared=yes --enable-static=no --enable-strip --enable-option-checking=fatal --with-all --with-linux-i2c=no --with-gpio=no --with-cgi=no --with-nut_monitor=no --with-docs=no --with-ssl --with-user=root --with-group=root --with-cgipath=/var/www/nut-cgi-bin --with-htmlpath=/var/www/nut --with-drvpath=/usr/libexec/nut --with-statepath=/var/run/nut --with-pidpath=/var/run/nut --with-altpidpath=/var/run/nut

make[2]: Entering directory '/usr/src/tmp/nut-2.8.2/common'
  CC       parseconf.lo
  CCLD     libparseconf.la
  CC       libcommon_la-state.lo
  CC       libcommon_la-str.lo
  CC       libcommon_la-upsconf.lo
  CC       libcommon_la-common.lo
In file included from common.c:21:
common.c: In function 'sendsignalpid':
common.c:1356:25: warning: '%s' directive argument is null [-Wformat-overflow=]
 1356 |                         "%s: ran at least one check, and all such checks "
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/common.h:423:69: note: in definition of macro 'upsdebugx'
  423 |         do { if (nut_debug_level >= (level)) { s_upsdebugx((level), __VA_ARGS__); } } while(0)
      |                                                                     ^~~~~~~~~~~
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
  CCLD     libcommon.la
  CC       libcommonclient_la-state.lo
  CC       libcommonclient_la-str.lo
  CC       libcommonclient_la-common.lo
In file included from common.c:21:
common.c: In function 'sendsignalpid':
common.c:1356:25: warning: '%s' directive argument is null [-Wformat-overflow=]
 1356 |                         "%s: ran at least one check, and all such checks "
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/common.h:423:69: note: in definition of macro 'upsdebugx'
  423 |         do { if (nut_debug_level >= (level)) { s_upsdebugx((level), __VA_ARGS__); } } while(0)
      |                                                                     ^~~~~~~~~~~
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
  CCLD     libcommonclient.la
  CXX      libnutconf_la-nutconf.lo
  CXX      libnutconf_la-nutstream.lo
  CXX      libnutconf_la-nutwriter.lo
  CXX      libnutconf_la-nutipc.lo
  CXXLD    libnutconf.la
  CC       libcommonstr_la-str.lo
  CC       libcommonstr_la-common.lo
In file included from common.c:21:
common.c: In function 'sendsignalpid':
common.c:1356:25: warning: '%s' directive argument is null [-Wformat-overflow=]
 1356 |                         "%s: ran at least one check, and all such checks "
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/common.h:423:69: note: in definition of macro 'upsdebugx'
  423 |         do { if (nut_debug_level >= (level)) { s_upsdebugx((level), __VA_ARGS__); } } while(0)
      |                                                                     ^~~~~~~~~~~
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
  CCLD     libcommonstr.la
make[2]: Leaving directory '/usr/src/tmp/nut-2.8.2/common'

jimklimov · 2024-08-07T08:07:22Z

Thanks, so configure does honestly run the built test, and the practical library code is capable of handling this situation:

configure:14982: checking for practical support to printf("%s", NULL)
configure:15107: gcc -o conftest -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration -Wno-format-truncation -Wno-format-overflow
       conftest.c  >&5
configure:15107: $? = 0
configure:15107: ./conftest
SUCCESS: RETURNED a string that contains something like 'null' from snprintf() with a variable NULL string argument: '(null)'
configure:15107: $? = 0
configure:15127: result: yes

I'm trying to remember more reasons why I wanted to keep the almost-noop part of the macro definition, but can't at the moment. Maybe consistency of how the system printf() would formulate the NULL in different messages vs. what we would inject in a portion but not all of them?..

Would you be open to a configure --enable-NUT_STRARG-always sort of toggle? Only today, only now, at bargain price? :D

desertwitch · 2024-08-07T08:11:49Z

Thanks, so configure does honestly run the built test, and the practical library code is capable of handling this situation:
configure:14982: checking for practical support to printf("%s", NULL)
configure:15107: gcc -o conftest -isystem /usr/local/include  -O2 -fPIC -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Werror -Werror=implicit-function-declaration -Wno-format-truncation -Wno-format-overflow
       conftest.c  >&5
configure:15107: $? = 0
configure:15107: ./conftest
SUCCESS: RETURNED a string that contains something like 'null' from snprintf() with a variable NULL string argument: '(null)'
configure:15107: $? = 0
configure:15127: result: yes
I'm trying to remember more reasons why I wanted to keep the almost-noop part of the macro definition, but can't at the moment. Maybe consistency of how the system printf() would formulate the NULL in different messages vs. what we would inject in a portion but not all of them?..

Would you be open to a configure --enable-NUT_STRARG-always sort of toggle? Only today, only now, at bargain price? :D

Sounds like a perfect deal 😉 I just stumbled upon this getting a testing package (of the proposed changes in #2565) shipped out to users affected by the APC BX problem, so wanted to check back if this is something that could cause additional problems or rather a compiler oddity that can be ignored safely for now.

jimklimov · 2024-08-09T23:40:58Z

Re-confirmed with gcc-13 on OpenIndiana after a recent update.

…: introduce `configure --enable-NUT_STRARG-always` setting [networkupstools#2585] Signed-off-by: Jim Klimov <[email protected]>

…ys` setting have `auto` detection by default [networkupstools#2585] Flips to "yes" for gcc-13.x (clang having issues is not proven at the moment) Signed-off-by: Jim Klimov <[email protected]>

…hing [networkupstools#2585] Signed-off-by: Jim Klimov <[email protected]>

desertwitch changed the title ~~common.h: warning: '%s' directive argument is null [-Wformat-overflow=]~~ common.c/.h: warning: '%s' directive argument is null [-Wformat-overflow=] Aug 7, 2024

jimklimov mentioned this issue Aug 9, 2024

CI: clang-18 complains about a lot of switch clauses without a default label #2588

Closed

jimklimov added a commit to jimklimov/nut that referenced this issue Aug 10, 2024

m4/ax_c_pragmas.m4, docs/configure.txt, UPGRADING.adoc, docs/nut.dict…

d27fa62

…: introduce `configure --enable-NUT_STRARG-always` setting [networkupstools#2585] Signed-off-by: Jim Klimov <[email protected]>

jimklimov added a commit to jimklimov/nut that referenced this issue Aug 10, 2024

include/str.h: use a more reliable definition of NUT_STRARG doing not…

beed06f

…hing [networkupstools#2585] Signed-off-by: Jim Klimov <[email protected]>

jimklimov mentioned this issue Aug 10, 2024

Fix compilation on newer gcc-13.x and clang-18, fix nut-scanner offering config values not applicable for some USB drivers #2589

Merged

jimklimov closed this as completed in #2589 Aug 11, 2024

jimklimov added this to the 2.8.3 milestone Aug 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

common.c/.h: warning: '%s' directive argument is null [-Wformat-overflow=] #2585

common.c/.h: warning: '%s' directive argument is null [-Wformat-overflow=] #2585

desertwitch commented Aug 7, 2024

jimklimov commented Aug 7, 2024 •

edited

Loading

desertwitch commented Aug 7, 2024 •

edited

Loading

jimklimov commented Aug 7, 2024 •

edited

Loading

desertwitch commented Aug 7, 2024

jimklimov commented Aug 7, 2024

jimklimov commented Aug 7, 2024

desertwitch commented Aug 7, 2024

jimklimov commented Aug 7, 2024 •

edited

Loading

desertwitch commented Aug 7, 2024 •

edited

Loading

jimklimov commented Aug 9, 2024

common.c/.h: warning: '%s' directive argument is null [-Wformat-overflow=] #2585

common.c/.h: warning: '%s' directive argument is null [-Wformat-overflow=] #2585

Comments

desertwitch commented Aug 7, 2024

jimklimov commented Aug 7, 2024 • edited Loading

desertwitch commented Aug 7, 2024 • edited Loading

jimklimov commented Aug 7, 2024 • edited Loading

desertwitch commented Aug 7, 2024

jimklimov commented Aug 7, 2024

jimklimov commented Aug 7, 2024

desertwitch commented Aug 7, 2024

jimklimov commented Aug 7, 2024 • edited Loading

desertwitch commented Aug 7, 2024 • edited Loading

jimklimov commented Aug 9, 2024

jimklimov commented Aug 7, 2024 •

edited

Loading

desertwitch commented Aug 7, 2024 •

edited

Loading

jimklimov commented Aug 7, 2024 •

edited

Loading

jimklimov commented Aug 7, 2024 •

edited

Loading

desertwitch commented Aug 7, 2024 •

edited

Loading