make: update stack-analysis format#2712
Conversation
| printf "main stack frame: \n" | ||
| $(find $(rustc --print sysroot) -name llvm-readobj) --elf-output-style GNU --stack-sizes $1 | grep 'main' | ||
| bold=$(tput bold) | ||
| normal=$(tput sgr0) |
There was a problem hiding this comment.
Just a heads-up this adds a dependency on ncurses. It's shipped with my distro and seems to be on many, but it's worth keeping in mind - for instance I'm not quite sure whether Cygwin would have it by default.
There was a problem hiding this comment.
We've had tput around in the tools and build system for a long time; not a new dependency here, so I think it's fine.
|
We need to be more careful and cautious about changing the output of tools. It can be that other tools depend on their output, such that changing it breaks those tools. This is an example of a churn/incremental improvement change which can be frustrating to people downstream. |
|
This tool has only existed for 138 days. Are you aware of any downstream scripts that process the output? |
Co-authored-by: Pat Pannuto <pat.pannuto@gmail.com>
ff4a429
|
I am not aware of anyone processing the output of this script downstream, and downstream users are welcome to vendor a copy of the old version if they want the old output format. I do not think that every tool committed to the Tock repository should be locked into its original format, when I published this originally it was just a quick way to let others experiment with solutions to our stack size woes. |
|
ping |
|
bors r+ Phil's point about output stability is well-taken, particularly for things that are 'promoted' (i.e. this isn't just a script in the |
|
Build failed (retrying...): |
|
Build failed: |
|
bors retry |
2712: make: update stack-analysis format r=ppannuto a=bradjc
### Pull Request Overview
Two things:
1. Makes the `make allstack` easier to read:
```
$ make stack-analysis
imix
----------------------
main stack frame:
2048 main
5 largest stack frames:
2048 main
720 _ZN104_$LT$capsules..process_console..ProcessConsole$LT$C$GT$$u20$as$u20$kernel..hil..uart..TransmitClient$GT$18transmitted_buffer17h4c3fef6f6a95d58fE
632 _ZN8capsules15process_console23ProcessConsole$LT$C$GT$11write_state17hee14a9c17699800aE
608 _ZN7cortexm24kernel_hardfault_arm_v7m17h8c7b48b27a33c005E
600 _ZN98_$LT$capsules..ieee802154..framer..Framer$LT$M$C$A$GT$$u20$as$u20$kernel..hil..radio..RxClient$GT$7receive17h8a8322dd16823f26E
**WARNING! main is the largest stack frame!**
```
2. Enables `make stack-analysis V=1` to work fully.
### Testing Strategy
Running `make stack-analysis`.
### TODO or Help Wanted
n/a
### Documentation Updated
- [x] Updated the relevant files in `/docs`, or no updates are required.
### Formatting
- [x] Ran `make prepush`.
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Pull Request Overview
Two things:
make allstackeasier to read:make stack-analysis V=1to work fully.Testing Strategy
Running
make stack-analysis.TODO or Help Wanted
n/a
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.