CODE AND STORAGE OVERLAP while building firmware for NRF52

Posted on
of 2
/ 2
  • Seems to be online, but for example selecting the MDBT42Q just hangs there....

  • tried today and it was slow but produced something with MDBT42Q config, however this board has 2600 variables by default and when clicking in various tabs I saw VARIABLES: 0 somewhere.

  • I tried to mess with location and size of variables and currently I have at least small improvement that they are automatically sized in free space between end of heap and beginning of stack. Linker provides following symbols: __HeapStart,__HeapLimit,__StackLimit so if stack is sized properly variables can be between __HeapLimit and __StackLimit (heap is in fact zero so __HeapStart =__HeapLimit but if we'd like to have some heap in future HeapLimit works better).

    I have added stack painting with DEADBEEF to nrf startup and initially set stack size to 0x2600 which gives similar size of variables than before (2650) but it looks that when running my typical code in DS-D6 the stack could be much less.
    @Gordon would you be interested to go in this direction? sizing just stack and autosizing variables according to that? And BTW I had to move variable initialization sooner in main as they are probably used without proper jsvInit call but with static allocation it probably doesn't break.

    here is small patch to see the changes­f42d866c046e5ee243fb72e3 for SDK11 based build

    Stack sizing and those symbols are used here­b/master/targetlibs/nrf5x_12/components/­toolchain/gcc/gcc_startup_nrf52.S#L39 and I have put stack painting here after clearing BSS­b/master/targetlibs/nrf5x_12/components/­toolchain/gcc/gcc_startup_nrf52.S#L397 with loop taken from this­q-a/3650/nrf51822---exception-on-stack-o­verflow/13253#13253

    and BTW when unzipping Nordic sdk11 zip the file gcc_startup_nrf52.s was named with lower case .s suffix and looks like when it does not end with .S the preprocessor run is skipped so at first I had a lot of head scratching why -D__STACK_SIZE=x had no effect.

    Also it looks like default stack size is 4096 in SDK11 and was raised to 8192 in SDK12 so maybe 8KB 0x2000 should be enough? How is C stack used in JS interpreter with relation to nested js functions, does it use just JS variables or C stack too?

    BTW, Just checked stack in DS-D6 over bluetooth with my SDK11 build

    >for (i=0x20010000-0x2600;i<0x20010000;i+=4) { if (peek32(i)!=0xdeadbeef) break; } ; print(i.toString(16))

    so basically only 0x0D68=3432bytes of stack was ever used. Is there some specific espruino code I can try that uses lot of stack? (eg. regexes?)

  • That's interesting - it does look pretty tidy, although it doesn't solve the MTU issue unless you also swap the order of heap and stack? IIRC there's something (_bss?) that you can actually modify at runtime such that if someone tries to use malloc (which was done in a recent bit of code you posted I think) you don't end up allocating over the top of it.

    Honestly right now it's not a big priority to add something like that though....

    Did you find where variables are used without initialisation? That sounds like a pretty worrying bug.

    How is C stack used in JS interpreter with relation to nested js functions, does it use just JS variables or C stack too?

    It's a bit of both. Because it's recursive descent it actually uses the stack pretty heavily so you need quite a bit.

  • Thats just first step to move it out of static array. And worth it even alone since the variable size is just a guesswork now (?) and also depends on size of globals, i.e you add some globals over time (or only with some specific build options - like e.g. some display driver with some buffer) and need to think to shrink variables. With this they are sized automatically to fill all available space.

    The only guesswork that remains is stack, which can be checked/tuned by stack painting and monitoring its size. Is there some stack check now at runtime or it will silently overwrite variables now (and other random globals a bit earlier since in lst file jsVars array is somewhere between globals)? If you don't set stack size explicitly now at build time and compensate for it by shrinking variables by trial and error, that's not ideal.

    As for resizing I agree this is not priority because of MTU allocation and is definitely separate step. In fact it would make more sense for stability and safety first - on stack side. It could try to e.g. shrink variables when stack grows over some safe threshold instead of crashing or corrupting data.

    Overall agree changing size at runtime is not trivial however this first step (= optimal static allocation outside of static variable array declaration) is easy and removes some uncertainty and guesswork.

    As for dynamic heap, that's between __HeapStart,__HeapLimit now and is zero (so default malloc should fail?). As for that long writes code using malloc I linked, I don't know why or if that worked, maybe the guy increased heap at build time with -D__HEAP_SIZE=, that would work for him and would work also with this patch (and shrink variables automatically).

    Was just asking to know if it makes sense to clean it up a bit and send PR for this. However it needs some first default stack size guess (possibly for each SDK,platform) - fortunately this feature can be gradually enabled per board.

    BTW, already found one bug/feature related to this - process.memory().stackEndAddress returns LINKER_END_VAR instead of __StackLimit which is the real end of stack. And with this patch without fix it currently points to beginning of variables, not end. So the fix is to change it to __StackLimit.

    However from the comments­b/master/src/jswrap_process.c#L139 it looks like it is on purpose even if it really means end of globals and not end of stack. Not sure what was the reason to provide this feature exactly - some form of small and dangerous dynamic heap?

  • Did you find where variables are used without initialisation? That sounds like a pretty worrying bug.

    Perhaps yes - main runs jshInit before jsvInit­b/master/targets/nrf5x/main.c#L21
    near end of jshInit is jsble_init­b/master/targets/nrf5x/jshardware.c#L641­
    so I am guessing this­b/master/targets/nrf5x/bluetooth.c#L2408­
    at least it hangs in HardFault_Handler after stepping over jsble_init()

  • Is there some stack check now at runtime

    There is, yes - basically whenever it recurses it checks, and tries to leave 256 bytes free.

    process.memory().stackEndAddress returns LINKER_END_VAR

    LINKER_END_VAR is end, which is realistically the end of the stack. Unless I'm wrong (which is very possible), since we have no malloc the stack runs from the top of memory, all the way down to the last allocated variable.

    __StackLimit is something specific to Nordic's SDK (and no other platforms) so I don't tend to use it. If I did, we'd just end up wasting RAM though.

    Not sure what was the reason to provide this feature exactly

    Someone wanted to store a bunch of data in 'free' RAM. IMO it should be removed - there are definitely more helpful things that could be added instead.

    variables are used without initialisation

    Thanks! I've just filed an issue for this:­ues/1696

    That's a bug in the BLE code really - it's expecting to run after code has been loaded into the variables (so there's something there to read). Just moving the jsvar init earlier for that platform will fix the crash but the BLE code won't work because it's going to be looking at a bunch of empty JsVars.

    JsVar sizing

    ... I do like the idea of changing the amount of variables based maybe on a define for the amount of stack we want, but it'd be nice to have something that worked nicely multiplatform (at least on ARM). The original Espruino is based on STM32F103RCT6 (256k/48k), but in literally every board I've tested the chip was actually a STM32F103RDT6 (384k/64k). I never took advantage of that just in case, but it'd be great to have something that could take advantage of it at runtime.

    I'm a bit concered that Linux has a way of altering memory size, as does ESP32, and now this. And they're all IFDEF'd at the top of jsvInit. IMO it'd be nice to just pass in a pointer as well as size and handle it in the platform-specific main (although the Linux stuff would have to stay as that does some weird 'magic').

  • Post a reply
    • Bold
    • Italics
    • Link
    • Image
    • List
    • Quote
    • code
    • Preview

CODE AND STORAGE OVERLAP while building firmware for NRF52

Posted by Avatar for binux @binux