[gcc/cabi_cortex] pin noreturn contract on _exit/abort (fixes #1298)

[adityachilka1]

Closes #1298.

The warning

ext/gcc/cabi_cortex.c: In function '_exit':
ext/gcc/cabi_cortex.c:20:1: warning: 'noreturn' function does return
   20 | }
ext/gcc/cabi_cortex.c: In function 'abort':
ext/gcc/cabi_cortex.c:26:1: warning: 'noreturn' function does return
   26 | }

_exit() and abort() are declared __noreturn__ in newlib's <stdlib.h>. The modm definitions are weak overrides of those declarations, so the compiler enforces the noreturn contract.

Why GCC is right

Inside the modm assertion macro:

modm_assert_report(&info);
if (behavior & 4) __builtin_unreachable();

For modm_assert(false, ...) the behaviour is 0x04, so __builtin_unreachable() does fire — but only when modm_assert_report() actually returns. modm_assert_report is a public override point (modm_extern_c void modm_assert_report(_modm_assertion_info *info);), so a user can install a handler that returns normally (debug logging, fault injection, soft assertions, etc.). When they do, the function falls through past __builtin_unreachable() and returns — violating the noreturn contract that newlib promised.

That's what's happening on the reporter's build. _exit() and abort() need to be unconditionally non-returning.

Fix

Add a for(;;){} after the assert call in each function. It's unreachable when the assertion handler halts as designed (zero overhead, dead code), and it's the safety net that pins the noreturn contract when the handler chooses to continue. Either way the function never returns.

 modm_weak void _exit(int status)
 {
        modm_assert(false, "libc.exit",
                        "The libc exit(status) function was called!", status);
+       for (;;) { }
 }

 modm_weak void abort(void)
 {
        modm_assert(false, "libc.abort",
                        "The libc abort() function was called!");
+       for (;;) { }
 }

Also added a comment block above the functions documenting why the loop is there (so it doesn't get optimised out by a well-meaning future cleanup).

Why not [[noreturn]] / modm_noreturn?

Adding a noreturn attribute to the definitions doesn't fix the underlying analysis problem — GCC would still warn that a noreturn-marked function falls through. And it risks attribute-mismatch errors against newlib's declaration. The infinite loop is the canonical fix and is what the language-lawyer interpretation expects.

Answer to @salkinium's question on the issue thread

"What happens if you remove the return statements?"

There aren't any return statements to remove — the functions just end after the assert. The right move is to add a noreturn-satisfying construct, which is what this PR does.

No behavioural change

When the assertion handler halts as intended (default modm behaviour), the loop is unreachable and the warning goes away. When a user-supplied handler returns, the loop now correctly pins noreturn instead of falling through into undefined behaviour.

Credit

Thanks to @Tecnologic for filing the report with the exact warning output and to @salkinium for pointing the investigation in the right direction.

[salkinium]

Shouldn't we fix the modm_assert handler instead?

[adityachilka1]

Good question — I weighed both directions before opening this PR. Sharing my reasoning so you can pick.

Why I landed on the _exit/abort site rather than modm_assert:

_exit and abort are declared __noreturn__ by newlib, not by modm. That's the contract the compiler is checking when it emits the warning — and it stays an external contract regardless of what we do inside modm. The compiler can only assume modm_assert(...) is non-returning if it can prove it at the call site. Today it can't, because modm_assert_report is an intentional override point:

modm_extern_c void modm_assert_report(_modm_assertion_info *info);

A user-supplied handler is allowed to return — that's the whole point of soft asserts, fault injection, post-mortem logging, hostile-test rigs, etc. So a noreturn attribute on the declaration would either silently mislead the optimiser (UB if a user override returns) or attribute-clash with handlers that already build successfully against the current declaration.

Could we fix it inside the assert macro?

We could push a for(;;){} into the modm_assert macro after __builtin_unreachable() for behaviour-bit-4 calls. That would catch every hard-assert site, not just these two. But it changes the semantics for everyone who relies on the existing soft-fall-through path; the warning we're fixing here is specifically newlib's noreturn promise, which only applies at the _exit/abort boundary.

That's the asymmetry — the boundary is the right place to pin the contract.

Happy to flip the approach if you'd rather:

If you'd prefer the fix inside the assert plumbing, I can rework it three ways:

1. Add for(;;){} only for behaviour-bit-4 in the macro (narrow; same intent as this PR but generalised across all modm_assert(false, ...) sites).
2. Mark modm_assert_report [[noreturn]] for the behaviour-bit-4 dispatch (heavier — has the attribute-mismatch risk for user overrides).
3. Leave this PR as is.

Your call — it's your handler design and I don't want to push a change you'd rather make differently. If (1) sounds right I'll close this and open a follow-up against the macro.

[salkinium]

This LLM doesn't understand the problem nor the fix sadly :-(