patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 7 Packages Projects Releases Wiki Activity

sys/cbprintf_internal.h: Silence spurious warnings with -save-temps

Browse source 
Because Z_CBPRINTF_STATIC_PACKAGE_GENERIC()'s first argument can be a
static memory location and we have `-Wall`, which implicitly enables
`-Waddress`, we get the following warnings when you enable
`EXTRA_CFLAGS=-save-temps=obj`.

    $ west build -b qemu_cortex_m3 samples/hello_world -- \
          -DEXTRA_CFLAGS=-save-temps=obj
       :
       :
    zephyr/kernel/fatal.c: In function 'k_sys_fatal_error_handler':
    zephyr/kernel/fatal.c:45:1125: warning: the comparison will
    always evaluate as 'true' for the address of 'data' will never be NULL
    [-Waddress]
       45 |         LOG_ERR("Halting system");
          |
    In file included from zephyr/include/zephyr/logging/log_backend.h:9,
                     from zephyr/include/zephyr/logging/log_ctrl.h:10,
                     from zephyr/kernel/fatal.c:13:
    .../include/zephyr/logging/log_msg.h:94:17: note: 'data' declared here
       94 |         uint8_t data[];
          |                 ^~~~

The reason why you don't see this warning without the flag is that GCC
tracks tokens and suppress the warning if it's from a macro expansion.  You
can disable this feature by adding `-ftrack-macro-expansion=0`:

    west build -b qemu_cortex_m3 samples/hello_world -- \
          -DEXTRA_CFLAGS=-ftrack-macro-expansion=0

Because `-save-temps` generates .i files, all macros have been expanded and
the information has already been lost.  All GCC sees at the compilation
stage are the comparisons of static memory locations.

This commit replaces `buf != NULL` with a static inline function
`___is_null()` to silence the compiler.  By passing a static memory
location to a function, the compiler doesn't see the comparisons of a
static memory locations against literal values.  But it's still able to
optimize out in the later stage.

There is another way to silence it; By ignoring `-Waddress` with a pragma.
But its effect is the macro wide and it's too wide IMHO.  Thus, I've
decided to go with the inline function.

To add one more note: The name `___is_null()` is obviously too generic.
But let's have it here until someone finds it problematic.

This closes #51528.

Signed-off-by: Yasushi SHOJI <yashi@spacecubics.com>
This commit is contained in:
Yasushi SHOJI 2023-02-01 18:19:29 +09:00 committed by Carles Cufí
parent 575dfbf5d6
commit d51f874158
1 changed files with 15 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
include/zephyr/sys/cbprintf_internal.h
View file

@ -382,6 +382,20 @@ do { \
_name##_buf32)))
#endif
/* When the first argument of Z_CBPRINTF_STATIC_PACKAGE_GENERIC() is a
* static memory location, some compiler warns you if you compare the
* location against NULL. ___is_null() is used to kill this warning.
*
* The warnings would be visible when you built with -save-temps=obj,
* our standard debugging tip for macro problems.
*
* https://github.com/zephyrproject-rtos/zephyr/issues/51528
*/
static ALWAYS_INLINE bool ___is_null(void *p)
{
return p == NULL;
}
/** @brief Statically package a formatted string with arguments.
*
* @param buf buffer. If null then only length is calculated.
@ -430,7 +444,7 @@ do { \
Z_CBPRINTF_ON_STACK_ALLOC(_ros_pos_buf, _ros_cnt); \
uint8_t *_rws_buffer; \
Z_CBPRINTF_ON_STACK_ALLOC(_rws_buffer, 2 * _rws_cnt); \
size_t _pmax = (buf != NULL) ? _inlen : INT32_MAX; \
size_t _pmax = !___is_null(buf) ? _inlen : INT32_MAX; \
int _pkg_len = 0; \
int _total_len = 0; \
int _pkg_offset = _align_offset; \