fix: prevent LTO from breaking settingGet/settingGetIndex round-trip#11577
Conversation
Under INTERPROCEDURAL_OPTIMIZATION, the compiler inlines settingGet and settingGetIndex with divergent static settingsTable base addresses, causing MSPV2_SETTING to return wrong data for some settings while MSP2_COMMON_SETTING_INFO (name lookup) is correct.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Test firmware build ready — commit Download firmware for PR #11577 234 targets built. Find your board's
|
- Improve comment accuracy: describe observed behavior rather than the inferred mechanism; note why raw attribute is used instead of NOINLINE macro (NOINLINE is empty on non-F7/H7 targets but bug affects all LTO release builds) - Add noinline to settingGetPgn which has identical pointer arithmetic against settingsTable and is called from settingGetValuePointer - Add brief comments above settingGetIndex and settingGetPgn pointing back to the main explanation on settingGet
|
This is the bugfix I was trying to recall. My friend could not set the Idle Throttle using the gui on his Matekf405 wing but his other boards work. |
2 participants
Summary
Under
INTERPROCEDURAL_OPTIMIZATION(LTO), the compiler inlinessettingGetandsettingGetIndexat their respective call sites with potentially divergent views ofsettingsTable's base address. The result is thatsettingGetIndex(ptr)returns index X, butsettingGet(X)does not return the same entry — breaking the round-trip.Symptom:
MSPV2_SETTINGreturns the wrong value size (e.g. 1 byte) for some settings, whileMSP2_COMMON_SETTING_INFOfor the same setting name correctly returns the expected size and value. SITL builds are not affected because SITL is excluded from LTO.Fix: Mark
settingGetandsettingGetIndexasnoinline, forcing LTO to use a single, consistent reference tosettingsTableacross all call sites.Changes
src/main/fc/settings.c: add__attribute__((noinline))tosettingGetandsettingGetIndexTesting