raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.66k stars 911 forks source link

bootrom.h requires a __force_inline on rom_hwords_as_ptr ? #1700

Closed smithps closed 1 month ago

smithps commented 4 months ago

I have just been debugging some code which I required to be running from RAM as I was overwriting code in FLASH (Yes, I know I shouldn't).

All was well until the function rom_func_lookup_inline was called whilst programming the flash. This in turn called rom_hword_as_ptr, which although it was inlined the compiler thought best to leave in FLASH.....except that happened to be the bit I was overwriting.

My "fix" was to edit bootrom.h in the Pico SDK and change the inline declaration of rom_hword_as_ptr to

static __force_inline void *rom_hword_as_ptr(uint16_t rom_address) {

so that the code was inlined into RAM along with the caller (rom_func_lookup_inline) as I believe is the intention of making rom_func_lookup_inline a forced inline.

Note the compiler I am using is GCC 12

my suggestion is to put the __force_inline declaration into bootromh of the SDK as so;

#if PICO_C_COMPILER_IS_GNU && (__GNUC__ >= 12)
// Convert a 16 bit pointer stored at the given rom address into a 32 bit pointer
static __force_inline void *rom_hword_as_ptr(uint16_t rom_address) {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
    return (void *)(uintptr_t)*(uint16_t *)(uintptr_t)rom_address;
#pragma GCC diagnostic pop
}
#else
// Convert a 16 bit pointer stored at the given rom address into a 32 bit pointer
#define rom_hword_as_ptr(rom_address) (void *)(uintptr_t)(*(uint16_t *)(uintptr_t)(rom_address))
#endif
kilograham commented 1 month ago

fixed