fix(nrf52): LTO was dropping the board variant's weak hook overrides (#11415)
* fix(nrf52): LTO was dropping the board variant's weak hook overrides Whole-image LTO (enabled arch-wide for nrf52840 in #10655) inlines the empty weak body of earlyInitVariant()/lateInitVariant()/variant_shutdown()/ variant_nrf52LoopHook()/variantDefault*Config() at the call site, because the weak default and the call site live in the SAME translation unit. The strong override in variants/<arch>/<board>/variant.cpp is then never linked, and the board's hardware setup silently does not run. nrf52_lto.py's -fno-lto variant recompile does not help here: the caller is the problem, not the variant object. Needs both ingredients, so this only affects 2.8: the earlyInitVariant() indirection landed in #9438 and is present in v2.7.26 too, but v2.7.26 has no -flto, so the override linked normally. Found on the muzi R1 Neo, whose earlyInitVariant() drives DCDC_EN_HOLD (P0.13, the DC-DC hold after the user button) and NRF_ON (P0.29, "tells IO controller device is on"). Both were dropped from the image, so the companion MCU never saw the nRF application come up and stayed in its DFU indication (purple LED). Verified in the ELF: pre-fix setup() runs straight from waitUntilPowerLevelSafe() to the LED_NOTIFICATION block with no earlyInitVariant symbol in the binary and no pinMode/digitalWrite on P0.13 or P0.29 anywhere; post-fix it calls the real override. HW-confirmed on an R1 Neo. Also affected on nrf52840: earlyInitVariant() on 10 variants (incl. t-echo-card, which sequences its RT9080 3V3 rail there), variant_shutdown() on 18 variants (t114, t-echo, ThinkNode M1-M8, meshlink, wio-tracker-L1 ... - sleep pin parking, so deep-sleep leakage), variant_nrf52LoopHook() on 3 RAK variants. Confirmed dropped on heltec-mesh-node-t114 by build, not just by inspection. Fix is __attribute__((noinline)) on both the weak declaration and definition - the same guard already carried by loopCanSleep(), preFSBegin(), PowerHAL and variant_enableBatteryLpcompWake(), whose comment in main-nrf52.cpp already documents this exact failure mode. Also extend _VARIANT_OVERRIDES in extra_scripts/nrf52_lto.py from just _Z11initVariantv to all eight hooks. That post-link guard already had the right logic and would have caught this on every PR - it simply was not listing Meshtastic's own weak variant hooks, only the core's. With the list extended it goes red on both r1-neo and heltec-mesh-node-t114 when the noinline is reverted, and green with it. Its failure message now names both possible causes. * review: trim the noinline rationale comments to two lines Per AGENTS.md ("keep code comments minimal - one or two lines, max"), the incident detail and extended background belong in the PR description, not the source. Keeps the LTO/noinline rationale and the pointer to the guard. --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
This commit is contained in:
co-authored by
Jonathan Bennett
parent
204f88ddfe
commit
87a009da63
4 files changed
+47
-20
No files matched your search
@@ -233,9 +233,26 @@ def _assert_isr_handlers_survived(source, target, env):
|
||||
# and the build stays green. Turn that into a red build:
|
||||
# 1. the linked variant.cpp.o must not be an LTO object (proves the -fno-lto recompile fired);
|
||||
# 2. any override the object defines strong must resolve strong in the ELF.
|
||||
#
|
||||
# The list must also cover Meshtastic's OWN weak variant hooks, not just the core's. Those have
|
||||
# a second, independent way to vanish: their weak default AND their call site sit in the same
|
||||
# LTO'd translation unit (src/main.cpp, src/platform/nrf52/main-nrf52.cpp), so GCC inlines the
|
||||
# empty body at the call site and never reaches for the variant's strong override -- the
|
||||
# -fno-lto middleware above cannot help, the caller is the problem. The definitions carry
|
||||
# __attribute__((noinline)) to prevent it; this guard is what catches a future one that forgets.
|
||||
# Regression that motivated the extension: 2.8 dropped earlyInitVariant() on the muzi R1 Neo, so
|
||||
# DCDC_EN_HOLD/NRF_ON were never driven and the IO controller read the nRF as stuck in DFU
|
||||
# (purple LED). The build stayed green because only _Z11initVariantv was listed here.
|
||||
_VARIANT_OVERRIDES = (
|
||||
"_Z11initVariantv",
|
||||
) # extend if the core grows more weak variant hooks
|
||||
"_Z11initVariantv", # core hook (cores/nRF5/main.cpp)
|
||||
"_Z16earlyInitVariantv", # src/main.cpp -- pre-peripheral board bring-up
|
||||
"_Z15lateInitVariantv", # src/main.cpp -- post-radio board bring-up
|
||||
"_Z16variant_shutdownv", # main-nrf52.cpp -- pin parking before System OFF
|
||||
"_Z21variant_nrf52LoopHookv", # main-nrf52.cpp -- per-loop variant hook
|
||||
"_Z31variant_enableBatteryLpcompWakev", # main-nrf52.cpp -- LPCOMP wake opt-out
|
||||
"_Z20variantDefaultConfigv", # NodeDB.cpp -- per-board config defaults
|
||||
"_Z26variantDefaultModuleConfigv", # NodeDB.cpp -- per-board module defaults
|
||||
) # extend if the core (or Meshtastic) grows more weak variant hooks
|
||||
|
||||
|
||||
def _assert_variant_survived(source, target, env):
|
||||
@@ -289,15 +306,19 @@ def _assert_variant_survived(source, target, env):
|
||||
):
|
||||
problems.append(
|
||||
"%s is strong in variant.cpp.o but weak/absent in the ELF "
|
||||
"(LTO resolved the core's call to the empty weak stub)" % sym
|
||||
"(LTO resolved the call to the empty weak stub)" % sym
|
||||
)
|
||||
if problems:
|
||||
sys.stderr.write(
|
||||
"\n*** nrf52 LTO guard: board variant DROPPED from the image ***\n%s\n"
|
||||
"The variant's early hardware setup (initVariant) will never run on this board.\n"
|
||||
"Check _is_board_variant() in extra_scripts/nrf52_lto.py -- middleware nodes are\n"
|
||||
"$BUILD_DIR-mirrored; match srcnode() paths, not node.get_abspath().\n\n"
|
||||
% "\n".join(" - " + p for p in problems)
|
||||
"\n*** nrf52 LTO guard: board variant override DROPPED from the image ***\n%s\n"
|
||||
"That board hardware setup silently will not run. Two possible causes:\n"
|
||||
" 1. The weak default and its CALL SITE share one LTO'd translation unit\n"
|
||||
" (src/main.cpp, src/platform/nrf52/main-nrf52.cpp, src/mesh/NodeDB.cpp), so GCC\n"
|
||||
" inlined the empty body and never reached the override. Fix: mark BOTH the weak\n"
|
||||
" declaration and definition __attribute__((noinline)) -- see earlyInitVariant().\n"
|
||||
" 2. The -fno-lto middleware stopped matching the variant. Check _is_board_variant()\n"
|
||||
" below -- middleware nodes are $BUILD_DIR-mirrored, so match srcnode() paths,\n"
|
||||
" not node.get_abspath().\n\n" % "\n".join(" - " + p for p in problems)
|
||||
)
|
||||
from SCons.Script import Exit
|
||||
|
||||
|
||||
+6
-4
@@ -316,11 +316,13 @@ __attribute__((weak, noinline)) bool loopCanSleep()
|
||||
|
||||
// Weak empty variant initialization function.
|
||||
// May be redefined by variant files.
|
||||
void lateInitVariant() __attribute__((weak));
|
||||
void lateInitVariant() {}
|
||||
// noinline: weak default and call site share this TU, so LTO would inline the empty body and
|
||||
// never link the variant's strong override. nrf52_lto.py's _VARIANT_OVERRIDES guards this.
|
||||
__attribute__((noinline)) void lateInitVariant() __attribute__((weak));
|
||||
__attribute__((noinline)) void lateInitVariant() {}
|
||||
|
||||
void earlyInitVariant() __attribute__((weak));
|
||||
void earlyInitVariant() {}
|
||||
__attribute__((noinline)) void earlyInitVariant() __attribute__((weak));
|
||||
__attribute__((noinline)) void earlyInitVariant() {}
|
||||
|
||||
// NRF52 (and probably other platforms) can report when system is in power failure mode
|
||||
// (eg. too low battery voltage) and operating it is unsafe (data corruption, bootloops, etc).
|
||||
|
||||
+6
-4
@@ -98,11 +98,13 @@ static unsigned char userprefs_admin_key_2[] = USERPREFS_USE_ADMIN_KEY_2;
|
||||
|
||||
// Weak empty variant initialization function.
|
||||
// May be redefined by variant files.
|
||||
void variantDefaultConfig() __attribute__((weak));
|
||||
void variantDefaultConfig() {}
|
||||
// noinline: weak default and call site share this TU, so LTO would inline the empty body and
|
||||
// never link the variant's strong override. Same guard as earlyInitVariant() in main.cpp.
|
||||
__attribute__((noinline)) void variantDefaultConfig() __attribute__((weak));
|
||||
__attribute__((noinline)) void variantDefaultConfig() {}
|
||||
|
||||
void variantDefaultModuleConfig() __attribute__((weak));
|
||||
void variantDefaultModuleConfig() {}
|
||||
__attribute__((noinline)) void variantDefaultModuleConfig() __attribute__((weak));
|
||||
__attribute__((noinline)) void variantDefaultModuleConfig() {}
|
||||
|
||||
#ifdef HELTEC_MESH_NODE_T114
|
||||
|
||||
|
||||
@@ -51,12 +51,14 @@ uint16_t getVDDVoltage();
|
||||
|
||||
// Weak empty variant shutdown prep function.
|
||||
// May be redefined by variant files.
|
||||
void variant_shutdown() __attribute__((weak));
|
||||
void variant_shutdown() {}
|
||||
// noinline: same reason as variant_enableBatteryLpcompWake() below -- weak default and call
|
||||
// site are in this file, so LTO would inline the empty body and drop the variant's override.
|
||||
__attribute__((noinline)) void variant_shutdown() __attribute__((weak));
|
||||
__attribute__((noinline)) void variant_shutdown() {}
|
||||
|
||||
// Optional variant hook called each nrf52Loop(); e.g. for low-VDD System OFF.
|
||||
void variant_nrf52LoopHook(void) __attribute__((weak));
|
||||
void variant_nrf52LoopHook(void) {}
|
||||
__attribute__((noinline)) void variant_nrf52LoopHook(void) __attribute__((weak));
|
||||
__attribute__((noinline)) void variant_nrf52LoopHook(void) {}
|
||||
|
||||
// Return false to skip LPCOMP wake when entering System OFF (e.g. user CLI shutdown).
|
||||
// noinline: weak default and call site are in this file; without it GCC may inline the
|
||||
|
||||
Reference in New Issue
Block a user