Skip to content

Conversation

@mrc0mmand
Copy link
Member

To appease gcc-14's -Wcalloc-transposed-args check.

Follow-up for 2a9ab09.


A couple of hours ago gcc-14 was pushed to Rawhide and some Packit jobs already picked it up and started failing. This PR should take the edge off, but there's still bunch of warnings when building with -O2 which will need further inspection.

@poettering
Copy link
Member

lgtm

@poettering poettering added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Jan 17, 2024
@poettering
Copy link
Member

i marked it green, but you marked it draft. just means: go ahead and merge it if you think this is ready

To appease gcc-14's -Wcalloc-transposed-args check.

Follow-up for 2a9ab09.
@mrc0mmand
Copy link
Member Author

Ugh, there's one warning which I can't really find a way around:

../src/basic/macro.h:395:90: warning: array subscript 1 is outside array bounds of ‘uint8_t[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds=]
  395 |              ((long)(_current_ - _entries_) < (long)ELEMENTSOF(_entries_)) && ({ entry = *_current_; true; }); \
../src/basic/macro.h:392:9: note: in expansion of macro ‘_VA_ARGS_FOREACH’
  392 |         _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__)
      |         ^~~~~~~~~~~~~~~~
../src/test/test-macro.c:328:9: note: in expansion of macro ‘VA_ARGS_FOREACH’
  328 |         VA_ARGS_FOREACH(u8, 0xff) {
      |         ^~~~~~~~~~~~~~~
../src/fundamental/macro-fundamental.h:163:37: note: at offset 1 into object ‘__unique_prefix__entries_183’ of size 1
  163 | #define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
      |                                     ^~~~~~~~~~~~~~~~
../src/basic/macro.h:394:28: note: in definition of macro ‘_VA_ARGS_FOREACH’
  394 |         for (typeof(entry) _entries_[] = { __VA_ARGS__ }, *_current_ = _entries_; \
      |                            ^~~~~~~~~
../src/fundamental/macro-fundamental.h:109:27: note: in expansion of macro ‘XCONCATENATE’
  109 | #define CONCATENATE(x, y) XCONCATENATE(x, y)
      |                           ^~~~~~~~~~~~
../src/fundamental/macro-fundamental.h:163:25: note: in expansion of macro ‘CONCATENATE’
  163 | #define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
      |                         ^~~~~~~~~~~
../src/basic/macro.h:392:33: note: in expansion of macro ‘UNIQ_T’
  392 |         _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__)
      |                                 ^~~~~~
../src/test/test-macro.c:328:9: note: in expansion of macro ‘VA_ARGS_FOREACH’
  328 |         VA_ARGS_FOREACH(u8, 0xff) {
      |         ^~~~~~~~~~~~~~~
[2414/2414] Generating export-dbus-interfaces with a custom command

In:

systemd/src/basic/macro.h

Lines 385 to 391 in 1cdd8b1

#define VA_ARGS_FOREACH(entry, ...) \
_VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__)
#define _VA_ARGS_FOREACH(entry, _entries_, _current_, ...) \
for (typeof(entry) _entries_[] = { __VA_ARGS__ }, *_current_ = _entries_; \
((long)(_current_ - _entries_) < (long)ELEMENTSOF(_entries_)) && ({ entry = *_current_; true; }); \
_current_++)

I tried a couple of solutions, like using the first value as a sentinel, but that feels icky (and the macro can be used even without arguments). @poettering @yuwata any idea here?

@mrc0mmand mrc0mmand removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Jan 17, 2024
@YHNdnzj YHNdnzj added this to the v256 milestone Jan 17, 2024
This gets enabled by default in gcc-14 and complains everywhere where we
use assert() on an expression that is always true (i.e. using
`int x[static 2]` in function declaration, etc.):

[153/2414] Compiling C object src/basic/libbasic.a.p/fs-util.c.o
In file included from ../src/basic/macro.h:13,
                 from ../src/basic/alloc-util.h:10,
                 from ../src/basic/fs-util.c:11:
../src/basic/fd-util.h: In function ‘format_proc_fd_path’:
../src/fundamental/macro-fundamental.h:74:41: warning: ‘nonnull’ argument ‘buf’ compared to NULL [-Wnonnull-compare]
   74 | #define _unlikely_(x) (__builtin_expect(!!(x), 0))
      |                                         ^~~~~
../src/basic/macro.h:150:21: note: in expansion of macro ‘_unlikely_’
  150 |                 if (_unlikely_(!(expr)))                                \
      |                     ^~~~~~~~~~
../src/basic/macro.h:167:22: note: in expansion of macro ‘assert_message_se’
  167 | #define assert(expr) assert_message_se(expr, #expr)
      |                      ^~~~~~~~~~~~~~~~~
../src/basic/fd-util.h:129:9: note: in expansion of macro ‘assert’
  129 |         assert(buf);
      |         ^~~~~~

Disabling this selectively only for asserts is a bit painful, since the
option is not available in all compilers, and it'd need to be handled in
the EFI stuff as well.
@mrc0mmand
Copy link
Member Author

mrc0mmand commented Jan 17, 2024

I pushed my latest attempt on making gcc-14 happy with the VA_ARGS_FOREACH() macro - it's not particularly pretty, but it makes gcc-14 shut up and the tests still pass.

Edit: oh well, computer says no again

So gcc-14 doesn't complain we're out of bounds on the last iteration:

[2092/2414] Compiling C object test-macro.p/src_test_test-macro.c.o
In file included from ../src/basic/list.h:209,
                 from ../src/basic/log.h:10,
                 from ../src/test/test-macro.c:5:
../src/test/test-macro.c: In function ‘test_FOREACH_VA_ARGS’:
../src/basic/macro.h:395:90: warning: array subscript 1 is outside array bounds of ‘uint8_t[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds=]
  395 |              ((long)(_current_ - _entries_) < (long)ELEMENTSOF(_entries_)) && ({ entry = *_current_; true; }); \
../src/basic/macro.h:392:9: note: in expansion of macro ‘_VA_ARGS_FOREACH’
  392 |         _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__)
      |         ^~~~~~~~~~~~~~~~
../src/test/test-macro.c:322:9: note: in expansion of macro ‘VA_ARGS_FOREACH’
  322 |         VA_ARGS_FOREACH(u8, 0) {
      |         ^~~~~~~~~~~~~~~
../src/fundamental/macro-fundamental.h:163:37: note: at offset 1 into object ‘__unique_prefix__entries_181’ of size 1
  163 | #define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
      |                                     ^~~~~~~~~~~~~~~~
../src/basic/macro.h:394:28: note: in definition of macro ‘_VA_ARGS_FOREACH’
  394 |         for (typeof(entry) _entries_[] = { __VA_ARGS__ }, *_current_ = _entries_; \
      |                            ^~~~~~~~~
../src/fundamental/macro-fundamental.h:109:27: note: in expansion of macro ‘XCONCATENATE’
  109 | #define CONCATENATE(x, y) XCONCATENATE(x, y)
      |                           ^~~~~~~~~~~~
../src/fundamental/macro-fundamental.h:163:25: note: in expansion of macro ‘CONCATENATE’
  163 | #define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
      |                         ^~~~~~~~~~~
../src/basic/macro.h:392:33: note: in expansion of macro ‘UNIQ_T’
  392 |         _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__)
      |                                 ^~~~~~
../src/test/test-macro.c:322:9: note: in expansion of macro ‘VA_ARGS_FOREACH’
  322 |         VA_ARGS_FOREACH(u8, 0) {
      |         ^~~~~~~~~~~~~~~
@mrc0mmand mrc0mmand marked this pull request as ready for review January 17, 2024 12:34
@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed build-system meson please-review PR is ready for (re-)review by a maintainer labels Jan 17, 2024
@poettering poettering merged commit e7f2eef into systemd:main Jan 18, 2024
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Jan 18, 2024
@fbuihuu
Copy link
Contributor

fbuihuu commented Feb 14, 2024

May I ask why it has been backported to the v254 stable tree ?

This bump the requirement on the version of gcc from 4.7 to 8.x which is not ok for a stable release.

@fbuihuu
Copy link
Contributor

fbuihuu commented Feb 14, 2024

May I ask why it has been backported to the v254 stable tree ?

This bump the requirement on the version of gcc from 4.7 to 8.x which is not ok for a stable release.

systemd/systemd-stable#368

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants