Rework max-stack analysis crate, address some limitations

[jamesmunns]

Note: this builds on top of #2587, which broke out the relevant functions into a standalone crate.

This PR consists of four main parts:

1. A direct fix for Current max stack analysis code is (significantly) underestimating necessary stack space for tasks #2588 "Program Counter Relative Branching" defect. This is a ~1 line change when filtering the potential instructions for branching
2. An attempt to address Current max stack analysis code is (significantly) underestimating necessary stack space for tasks #2588 "Missing sanity cross-checks", which looks for any functions not present in the call graph, and adds the largest call stack of those functions to the evaluated call graph, as a potential "fudge factor" for indirect branching/dyn dispatch we were unable to follow
3. Updating of manifests to increase stack sizes based on 1 + 2
4. A major refactor of the current max stack analysis code

The 4th part isn't strictly necessary, however while investigating and making myself familiar with how our current code works, it was useful to me to split things out so I could follow what was going on, and restructure the analysis to allow for some more interactive debugging/diffing. I personally think the code is a bit more accessible now, but that could be more personal preference than anything else.

The 1st and 3rd parts are achievable with a much smaller (a few lines) diff. If this PR is too difficult to review, happy to bail on this or at least defer it.

The 2nd part would be more difficult (but still possible) to achieve without the refactorings made in the 4th part, as the new call graph analysis lets us more easily keep track of which functions have been visiting when traversing the graph.

If we merge this PR, I'd consider the "urgent" part of #2588 resolved, though there are still some "known defects", and we might want to increase the baseline stack requirements to handle register stacking (this would bump most task stack sizes in most manifests by 104 bytes, and +36 for gimletlet).

[jamesmunns]

Issues I've found so far:

At least some code does meaningfully have recursion cycles

$ cargo xtask dist test/tests-stm32g0/app-g070.toml
thread 'main' (17436042) panicked at build/stack/src/lib.rs:285:27:
unable to resove 0x08004BD8: Refusing to handle recursion: [0x08004BD8, 0x08004BE8] -> 0x08004BD8
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

 08004bd8 <core::str::slice_error_fail>:
 8004bd8: b580          push    {r7, lr}
 8004bda: af00          add     r7, sp, #0x0
 8004bdc: b082          sub     sp, #0x8
 8004bde: 68bc          ldr     r4, [r7, #0x8]
 8004be0: 9400          str     r4, [sp]
 8004be2: f000 f801     bl      0x8004be8 <core::str::slice_error_fail_rt> @ imm = #0x2
 8004be6: d4d4          bmi     0x8004b92 <<u32 as core::fmt::Display>::fmt+0xaa> @ imm = #-0x58

08004be8 <core::str::slice_error_fail_rt>:
 8004be8: b580          push    {r7, lr}
 8004bea: af00          add     r7, sp, #0x0
...
 8004c92: 9200          str     r2, [sp]
 8004c94: 4622          mov     r2, r4
 8004c96: f7ff ff9f     bl      0x8004bd8 <core::str::slice_error_fail> @ imm = #-0xc2

At least some code is missing proper symbol table metadata

objdump -xSC ../../target/oxide-rot-1-selfsigned/dist/attest.tmp | grep '161f4'
000161f4 l       .text	00000000 $t
000161f4 g     F .text	00000000 fe25519_add_asm
                        ^^^^^^^^ -> size = 0

the function gets filtered out because of this

thread 'main' (17388305) panicked at build/stack/src/main.rs:43:48:
called `Result::unwrap()` on an `Err` value: 000161f4: no function data
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

// The original .a doesn't have info for this object. Some functions do, this one doesn't.

$ objdump -xSC ~/Downloads/salty-asm.a
SYMBOL TABLE:
00000000 l    d  .text  00000000 .text
00000000 l    d  .data  00000000 .data
00000000 l    d  .bss   00000000 .bss
00000000 l       .text  00000000 $t
00000000 l    d  .ARM.attributes        00000000 .ARM.attributes
00000000 g     F .text  00000000 fe25519_add_asm

Disassembly of section .text:

00000000 <fe25519_add_asm>:
       0: b4f0          push    {r4, r5, r6, r7}
       2: f04f 0701     mov.w   r7, #0x1

[hawkw]

this is mostly style nitpicking because i didn't really understand the old code and i am not sure if i fully understand the new code either.

[labbott]

I think this is a good approach, I'll take another pass later

[jamesmunns]

@hawkw I will likely punt on switching over to iqqdq, I agree with you it's a good fit, but I probably need to stop working on this for a bit, and would like to get it landed.

[jamesmunns]

For the sake of verification, I coerced a couple different generations of the stack analysis to output similar reports, and the easiest way to multi-file-diff (edit: and share it, locally I use bcompare) I could think of was making a git repo: https://github.com/jamesmunns/stack-compare/commits/main/

largely: they change in the way we expect:

• Commit 1/4, which is 2f318dc, just before the "fire drill"
• Commit 2/4, which is after Increase stack limits, check relative branches #2590, adds a whole bunch of missing function calls
• Commit 3/4, which is after Rework max-stack analysis crate, address some limitations #2589, now includes functions that aren't "reachable" with recursive analysis
• Commit 4/4, which is after 6b4e417, doesn't really change anything, but that's good, because the "what" is the same but the "how" is different (this isn't part of 2589 yet)

Commit 2->3 has some meaningful code changes (I tried to back these out, but it was annoying and I gave up), so there are some changed addresses, but 1->2 and 3->4 are relatively the same code, just with analysis changed.

[labbott]

I think this is a really nice improvement

[labbott]

Does this comment need to be updated to match our current approach with "blessed recursion"?

[jamesmunns]

Yes! Updated this comment.