Skip to content

tree-wide: fix compilation with GCC 15.1 and picolibc 1.8.10 #21443

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 28, 2025
Merged

tree-wide: fix compilation with GCC 15.1 and picolibc 1.8.10 #21443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
85233b3
tests/drivers/hm330x: use fmt_table
maribu Apr 26, 2025
914429c
core/lib/include/compiler_hints.h: fix style
maribu Apr 26, 2025
0b01fa0
core/compiler_hints: Add NONSTRING attribute
maribu Apr 26, 2025
1b70a59
tree-wide: add the NONSTRING attribute where needed
maribu Apr 26, 2025
59b1774
pkg: Disable more warnings
maribu Apr 26, 2025
0be58f6
sys/picolibc_syscalls_default: fix linking with 1.8.10+
maribu Apr 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 37 additions & 23 deletions core/lib/include/compiler_hints.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,25 @@ extern "C" {
* cannot return.
*/
#ifndef NORETURN
#ifdef __GNUC__
#define NORETURN __attribute__((noreturn))
#else
#define NORETURN
# ifdef __GNUC__
# define NORETURN __attribute__((noreturn))
# else
# define NORETURN
# endif
#endif

/**
* @def NONSTRING
* @brief The `NONSTRING` keyword tells the compiler to assume that a char array
* is not used as c string. (Specifically: It does not need a terminating
* zero byte.)
*/
#ifndef NONSTRING
# if (__GNUC__ >= 15)
# define NONSTRING __attribute__((nonstring))
# else
# define NONSTRING
# endif
#endif

/**
Expand All @@ -48,11 +62,11 @@ extern "C" {
* optimization just as an arithmetic operator would be.
*/
#ifndef PURE
#ifdef __GNUC__
#define PURE __attribute__((pure))
#else
#define PURE
#endif
# ifdef __GNUC__
# define PURE __attribute__((pure))
# else
# define PURE
# endif
#endif

/**
Expand All @@ -61,11 +75,11 @@ extern "C" {
* static functions, function arguments, local variables
*/
#ifndef MAYBE_UNUSED
#ifdef __GNUC__
#define MAYBE_UNUSED __attribute__((unused))
#else
#define MAYBE_UNUSED
#endif
# ifdef __GNUC__
# define MAYBE_UNUSED __attribute__((unused))
# else
# define MAYBE_UNUSED
# endif
#endif

/**
Expand All @@ -77,9 +91,9 @@ extern "C" {
* by llvm.
*/
#if defined(__llvm__) || defined(__clang__)
#define NO_SANITIZE_ARRAY __attribute__((no_sanitize("address")))
# define NO_SANITIZE_ARRAY __attribute__((no_sanitize("address")))
#else
#define NO_SANITIZE_ARRAY
# define NO_SANITIZE_ARRAY
#endif

/**
Expand All @@ -90,9 +104,9 @@ extern "C" {
* an assembler instruction causes a longjmp, or a write causes a reboot.
*/
#if ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 5)) || (__GNUC__ >= 5) || defined(__clang__)
#define UNREACHABLE() __builtin_unreachable()
# define UNREACHABLE() __builtin_unreachable()
#else
#define UNREACHABLE() do { /* nothing */ } while (1)
# define UNREACHABLE() do { /* nothing */ } while (1)
#endif

/**
Expand Down Expand Up @@ -138,12 +152,12 @@ extern "C" {
* This allows providing two different implementations in C, with one being
* more efficient if constant folding is used.
*/
#define IS_CT_CONSTANT(expr) <IMPLEMENTATION>
# define IS_CT_CONSTANT(expr) <IMPLEMENTATION>
#elif defined(__GNUC__)
/* both clang and gcc (which both define __GNUC__) support this */
#define IS_CT_CONSTANT(expr) __builtin_constant_p(expr)
# define IS_CT_CONSTANT(expr) __builtin_constant_p(expr)
#else
#define IS_CT_CONSTANT(expr) 0
# define IS_CT_CONSTANT(expr) 0
#endif

/**
Expand Down Expand Up @@ -175,9 +189,9 @@ extern "C" {
* @param[in] cond Condition that is guaranteed to be true
*/
#ifdef NDEBUG
#define assume(cond) ((cond) ? (void)0 : UNREACHABLE())
# define assume(cond) ((cond) ? (void)0 : UNREACHABLE())
#else
#define assume(cond) assert(cond)
# define assume(cond) assert(cond)
#endif

/**
Expand Down
1 change: 1 addition & 0 deletions makefiles/vars.inc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export NATIVEINCLUDES # The native include paths, set by the various nati

export GCC_C_INCLUDES # system include dirs implicitly used by GCC's c compiler, only defined with TOOLCHAIN=llvm
export GCC_CXX_INCLUDES # system include dirs implicitly used by GCC's c++ compiler, only defined with TOOLCHAIN=llvm
export GCC_VERSION # version of GCC if GCC is used, empty otherwise

export USEMODULE # Sys Module dependencies of the application. Set in the application's Makefile.
export BIN_USEMODULE # Modules specific to bindist (see bindist.ink.mk). Set in the application's Makefile.
Expand Down
10 changes: 10 additions & 0 deletions pkg/pkg.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,16 @@ distclean:: clean
endif

# Disabling some diagnostics: These issues needs to be fixed upstream
CFLAGS += -Wno-maybe-uninitialized
ifeq (llvm,$(TOOLCHAIN))
CFLAGS += -Wno-documentation
endif

include $(RIOTBASE)/makefiles/utils/strings.mk
Copy link
Member Author

@maribu maribu Apr 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting: When run with make -j, $(RIOTMAKE) (shorthand for $(RIOTBASE)/makefiles) is not defined for building the tools such as edbg. When build with make it is defined.

These kinds of race conditions really are no fun.

Copy link
Contributor

@crasbe crasbe Apr 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't look into the CI trace, but often packages are not fully defined and some assume certain variables are set which is not always the case. For example some packages don't build when you're in the folder of the package etc.

pkg/pkg.mk could use some love and the packages as well.

# Disabling -Wunterminated-string-initialization on toolchains that support this
# warning
ifeq (1,$(call version_is_greater_or_equal,$(GCC_VERSION),15))
CFLAGS += -Wno-unterminated-string-initialization
# this flag should not be passed to g++, only gcc:
CXXUWFLAGS += -Wno-unterminated-string-initialization
endif
2 changes: 2 additions & 0 deletions sys/fmt/fmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@
#include <time.h>
#include <unistd.h>

#include "compiler_hints.h"
#include "container.h"
#include "fmt.h"
#include "modules.h"

extern ssize_t stdio_write(const void* buffer, size_t len);

NONSTRING
static const char _hex_chars[16] = "0123456789ABCDEF";

static const uint32_t _tenmap[] = {
Expand Down
2 changes: 2 additions & 0 deletions sys/fmt/table.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
#include <unistd.h>
#include <string.h>

#include "compiler_hints.h"
#include "fmt.h"
#include "fmt_table.h"

NONSTRING
static const char fmt_table_spaces[16] = " ";

/**
Expand Down
4 changes: 3 additions & 1 deletion sys/picolibc_syscalls_default/syscalls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,9 @@ static int picolibc_get(FILE *file)
FILE picolibc_stdio =
FDEV_SETUP_STREAM(picolibc_put, picolibc_get, picolibc_flush, _FDEV_SETUP_RW);

#ifdef PICOLIBC_STDIO_GLOBALS
/* Since picolibc 1.8.10, PICOLIBC_STDIO_GLOBALS is prefixed with two leading
* underscores. We just test for both to remain backwards compatible */
#if defined(PICOLIBC_STDIO_GLOBALS) || defined(__PICOLIBC_STDIO_GLOBALS)
#ifdef __strong_reference
/* This saves two const pointers.
* See https://github.com/RIOT-OS/RIOT/pull/17001#issuecomment-945936918
Expand Down
5 changes: 5 additions & 0 deletions tests/bench/sys_base64/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,20 @@
#include <string.h>

#include "base64.h"
#include "compiler_hints.h"
#include "fmt.h"
#include "macros/utils.h"
#include "xtimer.h"

static char buf[128];

/* no need for the zero-termination here, base64_encode() gets the size of the
* string as explicit argument */
NONSTRING
static const char input[96] = "This is an extremely, enormously, greatly, "
"immensely, tremendously, remarkably lengthy "
"sentence!";
NONSTRING
static const char base64[128] =
"VGhpcyBpcyBhbiBleHRyZW1lbHksIGVub3Jtb3VzbHksIGdyZWF0bHksIGltbWVuc2VseSwgdHJl"
"bWVuZG91c2x5LCByZW1hcmthYmx5IGxlbmd0aHkgc2VudGVuY2Uh";
Expand Down
1 change: 1 addition & 0 deletions tests/drivers/hm330x/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ USEMODULE += $(DRIVER)
USEMODULE += ztimer_usec
USEMODULE += ztimer_msec
USEMODULE += fmt
USEMODULE += fmt_table

include $(RIOTBASE)/Makefile.include
25 changes: 2 additions & 23 deletions tests/drivers/hm330x/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,40 +13,19 @@
* @file
* @brief HM330X driver test application
*
* @author Marian Buschsieweke <[email protected]>
* @author Francisco Molina <[email protected]>
*
* @}
*/

#include <stdio.h>
#include <string.h>

#include "fmt.h"
#include "fmt_table.h"
#include "time_units.h"
#include "ztimer.h"
#include "timex.h"

#include "hm330x.h"
#include "hm330x_params.h"

static const char spaces[16] = " ";

static void print_col_u32_dec(uint32_t number, size_t width)
{
char sbuf[10]; /* "4294967295" */
size_t slen;

slen = fmt_u32_dec(sbuf, number);
if (width > slen) {
width -= slen;
while (width > sizeof(spaces)) {
print(spaces, sizeof(spaces));
}
print(spaces, width);
}
print(sbuf, slen);
}

int main(void)
{
hm330x_t dev;
Expand All @@ -61,13 +40,13 @@
return 1;
}


Check warning on line 43 in tests/drivers/hm330x/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

too many consecutive empty lines
#if IS_USED(MODULE_HM3302)
print_str(
"+------------------------+------------------------+----------------------------------------------+\n"

Check warning on line 46 in tests/drivers/hm330x/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
"| Standard concentration | Atmospheric Environment| # Particles in 0.1l air of diameter >= |\n"

Check warning on line 47 in tests/drivers/hm330x/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
"| PM1.0 | PM2.5 | PM10.0 | PM1.0 | PM2.5 | PM10.0 | 0.3µm | 0.5µm | 1.0µm | 2.5µm | 5.0µm | 10µm |\n"

Check warning on line 48 in tests/drivers/hm330x/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
"+-------+-------+--------+-------+-------+--------+-------+-------+-------+-------+-------+------+\n"

Check warning on line 49 in tests/drivers/hm330x/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
);
#else
print_str(
Expand Down
3 changes: 3 additions & 0 deletions tests/sys/psa_crypto/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <stdio.h>
#include "embUnit.h"
#include "compiler_hints.h"
#include "psa/crypto.h"

void addFailurePSA(const char *func, psa_status_t errcode, long line, const char *file)
Expand All @@ -31,9 +32,9 @@
addFailure(msg, line, file);
}

#define TEST_ASSERT_PSA(func_, do_) { psa_status_t ret = func_; if (ret != PSA_SUCCESS) { addFailurePSA(#func_, ret, __LINE__, __FILE__); do_; } }

Check warning on line 35 in tests/sys/psa_crypto/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
#define TEST_ASSERT_PSA_CLEANUP(func_) TEST_ASSERT_PSA(func_, goto cleanup)
#define TEST_ASSERT_PSA_RETURN(func_) TEST_ASSERT_PSA(func_, return)

Check warning on line 37 in tests/sys/psa_crypto/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

keyword 'return' not immediately followed by a semicolon or a single space
#define TEST_ASSERT_PSA_CONTINUE(func_) TEST_ASSERT_PSA(func_, )

/*
Expand Down Expand Up @@ -73,7 +74,9 @@
{
const psa_algorithm_t alg = PSA_ALG_SHA_256;

NONSTRING
const uint8_t in1[1] = "a";
NONSTRING
const uint8_t in2[1] = "b";
Comment on lines +77 to 80
Copy link
Contributor

@kfessel kfessel Apr 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ( first glace) like these should be

in1[1] = 'a';
in2[1] = 'b';

Copy link
Member Author

@maribu maribu Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that is invalid syntax.

You could do

const uint8_t in1[1] = { 'a' };

instead. But that would be the same result as before.


const uint8_t exp1[] = {
Expand Down Expand Up @@ -181,7 +184,7 @@
}

/**
* Exporting and re-importing a private Ed25519 key should result in the same public key and signature.

Check warning on line 187 in tests/sys/psa_crypto/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
*/
static void test_exported_key_is_identical_when_imported_again_ed25519(void)
{
Expand Down Expand Up @@ -219,7 +222,7 @@
TEST_ASSERT_PSA_CLEANUP(psa_generate_key(&key_attr, &key_id));

// sign msg with generated keypair
TEST_ASSERT_PSA_CLEANUP(psa_sign_message(key_id, key_alg, msg, sizeof(msg), sig, sizeof(sig), &sig_len));

Check warning on line 225 in tests/sys/psa_crypto/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters

// export public and private key, then free slot
TEST_ASSERT_PSA_CLEANUP(psa_export_public_key(key_id, pubkey, sizeof(pubkey), &pubkey_len));
Expand All @@ -233,7 +236,7 @@
TEST_ASSERT(pubkey_len == pubkey2_len && memcmp(pubkey, pubkey2, pubkey_len) == 0);

// sign msg with imported key and compare signatures
TEST_ASSERT_PSA_CLEANUP(psa_sign_message(key_id, key_alg, msg, sizeof(msg), sig2, sizeof(sig2), &sig2_len));

Check warning on line 239 in tests/sys/psa_crypto/main.c

View workflow job for this annotation

GitHub Actions / static-tests 

line is longer than 100 characters
TEST_ASSERT(sig_len == sig2_len && memcmp(sig, sig2, sig_len) == 0);

cleanup:
Expand Down
2 changes: 2 additions & 0 deletions tests/sys/psa_crypto_ecdsa/test_ecdsa_p256_vectors.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <stdio.h>
#include <stdint.h>

#include "compiler_hints.h"
#include "psa/crypto.h"

/*
Expand All @@ -42,6 +43,7 @@ static const uint8_t public_key[] = {0x04, 0x60, 0xFE, 0xD4, 0xBA, 0x25, 0x5A, 0

/* certain PSA backends require the data to be in RAM rather than ROM
* so these values cannot be `const` */
NONSTRING
static uint8_t message[6] = "sample";
static uint8_t signature[] = {0xEF, 0xD4, 0x8B, 0x2A, 0xAC, 0xB6, 0xA8, 0xFD, 0x11, 0x40,
0xDD, 0x9C, 0xD4, 0x5E, 0x81, 0xD6, 0x9D, 0x2C, 0x87, 0x7B, 0x56, 0xAA, 0xF9, 0x91, 0xC3, 0x4D,
Expand Down
4 changes: 4 additions & 0 deletions tests/sys/struct_tm_utility/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,21 @@
#include <stdlib.h>
#include <stdbool.h>

#include "compiler_hints.h"
#include "shell.h"
#include "tm.h"

NONSTRING
static const char MON_NAMES[12][3] = {
"JAN", "FEB", "MAR", "APR",
"MAY", "JUN", "JUL", "AUG",
"SEP", "OCT", "NOV", "DEC",
};
NONSTRING
static const char DAY_NAMES[7][3] = {
"SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT"
};
NONSTRING
static const char BOOL_NAMES[2][3] = { "NO", "YES" };

bool proper_atoi(const char *a, int *i)
Expand Down
9 changes: 9 additions & 0 deletions tests/unittests/tests-fmt/tests-fmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "embUnit/embUnit.h"

#include "compiler_hints.h"
#include "fmt.h"
#include "tests-fmt.h"

Expand Down Expand Up @@ -244,6 +245,7 @@ static void test_fmt_hex_bytes(void)

static void test_fmt_u16_hex(void)
{
NONSTRING
char out[8] = "zzzzzzzz";

/* Check return count with null buffer input */
Expand All @@ -260,6 +262,7 @@ static void test_fmt_u16_hex(void)

static void test_fmt_u32_hex(void)
{
NONSTRING
char out[12] = "zzzzzzzzzzzz";

/* Check return count with null buffer input */
Expand All @@ -276,6 +279,7 @@ static void test_fmt_u32_hex(void)

static void test_fmt_u64_hex(void)
{
NONSTRING
char out[20] = "zzzzzzzzzzzzzzzzzzzz";

/* Check return count with null buffer input */
Expand All @@ -292,6 +296,7 @@ static void test_fmt_u64_hex(void)

static void test_fmt_u16_dec(void)
{
NONSTRING
char out[8] = "zzzzzzzz";
uint8_t chars = 0;

Expand All @@ -310,6 +315,7 @@ static void test_fmt_u16_dec(void)

static void test_fmt_u32_dec(void)
{
NONSTRING
char out[16] = "zzzzzzzzzzzzzzzz";
uint8_t chars = 0;

Expand All @@ -329,6 +335,7 @@ static void test_fmt_u32_dec(void)

static void test_fmt_u64_dec(void)
{
NONSTRING
char out[24] = "zzzzzzzzzzzzzzzzzzzzzzzz";
uint8_t chars = 0;

Expand All @@ -345,6 +352,7 @@ static void test_fmt_u64_dec(void)

static void test_fmt_u64_dec_zero(void)
{
NONSTRING
char out[24] = "zzzzzzzzzzzzzzzzzzzzzzzz";
uint8_t chars = 0;

Expand All @@ -355,6 +363,7 @@ static void test_fmt_u64_dec_zero(void)

static void test_fmt_u64_dec_u64max(void)
{
NONSTRING
char out[24] = "zzzzzzzzzzzzzzzzzzzzzzzz";
uint8_t chars = 0;

Expand Down
Loading