EC: Do some stack size analysis of each task |
|||||||
Issue descriptionEach EC task has a limited stack size, and stack overflow may only happen under rare circumstances (e.g. rarely used code path, with an interrupt coming just at the wrong time). We should be able to do some build-time analysis of the stack size, at the very least in a semi-automated way. In https://code.google.com/p/chrome-os-partner/issues/detail?id=57544, I've had some success using -fstack-usage gcc flag and avstack.pl (http://www.dlbeer.co.nz/oss/avstack.html), however, this has a few issues: 1. avstack is not aware of EC tasks, and is not really presenting information in a useful manner (I had to hack it to show stack usage for some specific task) 2. avstack reports the worst possible path, e.g.: - any printf is assumed to use some %lld format, which would require an expensive 64-bit integer division. - some code path are never used (e.g. on elm, the accelerometer uses SPI, but I2C code path was shown in the tree) 3. Most of the code paths that uses a lot of stack are children of panic_assert_fail. It's probably ok to overflow the stack if we're going to panic anyway. 4. avstack is not aware of function pointers, we need a way to manually feed that into the analysis.
,
Aug 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/64ecddfd861002cb5cff04b22211d23cf5fbcb5d commit 64ecddfd861002cb5cff04b22211d23cf5fbcb5d Author: Che-yu Wu <cheyuw@google.com> Date: Tue Aug 15 07:25:20 2017 ec: Add a task information library for the stack analyzer. Add a shared library to export task information. Modified the stack analyzer to get information from the shared library. Show allocated stack sizes of tasks in the stack analyzer. To get the all task information (including the allocated stack size), A shared library is added and compiled with the board to export all configurations of the tasklist. BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm && extra/stack_analyzer/stack_analyzer.py \ --objdump=arm-none-eabi-objdump \ --addr2line=arm-none-eabi-addr2line \ --export_taskinfo=./build/elm/util/export_taskinfo.so \ --section=RW \ ./build/elm/RW/ec.RW.elf make BOARD=${BOARD} SECTION=${SECTION} analyzestack Change-Id: I72f01424142bb0a99c6776a55735557308e2cab6 Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/611693 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> [modify] https://crrev.com/64ecddfd861002cb5cff04b22211d23cf5fbcb5d/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/64ecddfd861002cb5cff04b22211d23cf5fbcb5d/util/build.mk [modify] https://crrev.com/64ecddfd861002cb5cff04b22211d23cf5fbcb5d/include/task_filter.h [add] https://crrev.com/64ecddfd861002cb5cff04b22211d23cf5fbcb5d/util/export_taskinfo.c [modify] https://crrev.com/64ecddfd861002cb5cff04b22211d23cf5fbcb5d/Makefile.rules [modify] https://crrev.com/64ecddfd861002cb5cff04b22211d23cf5fbcb5d/extra/stack_analyzer/stack_analyzer.py
,
Aug 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/ef09835e1941a2388e15b410791bed6ff5d84339 commit ef09835e1941a2388e15b410791bed6ff5d84339 Author: Che-yu Wu <cheyuw@google.com> Date: Tue Aug 22 06:48:14 2017 ec: Add annotation feature to the stack analyzer. Get stack analyzer supported to read annotation file and do basic annotation on the callgraph. The basic annotation includes: 1. Add missing calls to the callgraph 2. Ignore functions on the callgraph BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm && extra/stack_analyzer/stack_analyzer.py \ --objdump=arm-none-eabi-objdump \ --addr2line=arm-none-eabi-addr2line \ --export_taskinfo=./build/elm/util/export_taskinfo.so \ --section=RW \ --annotation=./extra/stack_analyzer/example_annotation.yaml \ ./build/elm/RW/ec.RW.elf make BOARD=elm SECTION=RW \ ANNOTATION=./extra/stack_analyzer/example_annotation.yaml \ analyzestack Change-Id: I4cc7c34f422655708a7312db3f6b4416e1af917f Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/614825 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [add] https://crrev.com/ef09835e1941a2388e15b410791bed6ff5d84339/extra/stack_analyzer/example_annotation.yaml [modify] https://crrev.com/ef09835e1941a2388e15b410791bed6ff5d84339/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/ef09835e1941a2388e15b410791bed6ff5d84339/extra/stack_analyzer/README.md [modify] https://crrev.com/ef09835e1941a2388e15b410791bed6ff5d84339/Makefile.rules [modify] https://crrev.com/ef09835e1941a2388e15b410791bed6ff5d84339/extra/stack_analyzer/stack_analyzer.py
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/eeeee803b731d292c81c3ec013641181bff4f4bd commit eeeee803b731d292c81c3ec013641181bff4f4bd Author: Che-yu Wu <cheyuw@google.com> Date: Thu Aug 24 06:13:38 2017 extra/stack_analyzer: Show callsite information. Show callsite details in the call trace. Handle another addr2line failure output. BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm && extra/stack_analyzer/stack_analyzer.py \ --objdump=arm-none-eabi-objdump \ --addr2line=arm-none-eabi-addr2line \ --export_taskinfo=./build/elm/util/export_taskinfo.so \ --section=RW \ --annotation=./extra/stack_analyzer/example_annotation.yaml \ ./build/elm/RW/ec.RW.elf make BOARD=elm SECTION=RW \ ANNOTATION=./extra/stack_analyzer/example_annotation.yaml \ analyzestack Change-Id: I3f36584af85f578f1d298bcd06622ba8e7e5262d Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/628000 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> [modify] https://crrev.com/eeeee803b731d292c81c3ec013641181bff4f4bd/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/eeeee803b731d292c81c3ec013641181bff4f4bd/extra/stack_analyzer/README.md [modify] https://crrev.com/eeeee803b731d292c81c3ec013641181bff4f4bd/extra/stack_analyzer/stack_analyzer.py
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/4f21ee309c63db2084dd0c0122c5311636a3627e commit 4f21ee309c63db2084dd0c0122c5311636a3627e Author: Che-yu Wu <cheyuw@google.com> Date: Thu Aug 24 08:25:51 2017 extra/stack_analyzer: Show indirect calls. Show the indirect calls found in disassembly. BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm && extra/stack_analyzer/stack_analyzer.py \ --objdump=arm-none-eabi-objdump \ --addr2line=arm-none-eabi-addr2line \ --export_taskinfo=./build/elm/util/export_taskinfo.so \ --section=RW \ --annotation=./extra/stack_analyzer/example_annotation.yaml \ ./build/elm/RW/ec.RW.elf make BOARD=elm SECTION=RW \ ANNOTATION=./extra/stack_analyzer/example_annotation.yaml \ analyzestack Change-Id: Ib82e68e0bc6c4be56ab679c297f256cbfe94bbb7 Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/628048 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> [modify] https://crrev.com/4f21ee309c63db2084dd0c0122c5311636a3627e/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/4f21ee309c63db2084dd0c0122c5311636a3627e/extra/stack_analyzer/README.md [modify] https://crrev.com/4f21ee309c63db2084dd0c0122c5311636a3627e/extra/stack_analyzer/stack_analyzer.py
,
Aug 24 2017
You could make the annotation json parsing a bit more robust, when commenting out stuffs in the file during testing I hit a few exceptions e.g. :
Traceback (most recent call last):
File "extra/stack_analyzer/stack_analyzer.py", line 1253, in <module>
main()
File "extra/stack_analyzer/stack_analyzer.py", line 1247, in main
analyzer.Analyze()
File "extra/stack_analyzer/stack_analyzer.py", line 1040, in Analyze
(add_set, remove_set, failed_sigtxts) = self.ResolveAnnotation(function_map)
File "extra/stack_analyzer/stack_analyzer.py", line 792, in ResolveAnnotation
(add_rules, remove_rules, invaild_sigtxts) = self.LoadAnnotation()
File "extra/stack_analyzer/stack_analyzer.py", line 759, in LoadAnnotation
for src_sigtxt, dst_sigtxts in self.annotation['add'].items():
AttributeError: 'NoneType' object has no attribute 'items'
That's the 'add:' with nothing inside (all commented out while testing)
Possible simple fix:
--- a/extra/stack_analyzer/stack_analyzer.py
+++ b/extra/stack_analyzer/stack_analyzer.py
@@ -755,7 +755,7 @@ class StackAnalyzer(object):
remove_rules = set()
invaild_sigtxts = set()
- if 'add' in self.annotation:
+ if 'add' in self.annotation and self.annotation['add']:
for src_sigtxt, dst_sigtxts in self.annotation['add'].items():
src_sig = NormalizeSignature(src_sigtxt)
if src_sig is None:
ditto when adding functions with no calls:
Traceback (most recent call last):
File "extra/stack_analyzer/stack_analyzer.py", line 1253, in <module>
main()
File "extra/stack_analyzer/stack_analyzer.py", line 1247, in main
analyzer.Analyze()
File "extra/stack_analyzer/stack_analyzer.py", line 1040, in Analyze
(add_set, remove_set, failed_sigtxts) = self.ResolveAnnotation(function_map)
File "extra/stack_analyzer/stack_analyzer.py", line 792, in ResolveAnnotation
(add_rules, remove_rules, invaild_sigtxts) = self.LoadAnnotation()
File "extra/stack_analyzer/stack_analyzer.py", line 765, in LoadAnnotation
for dst_sigtxt in dst_sigtxts:
TypeError: 'NoneType' object is not iterable
,
Aug 24 2017
FYI, when running it inside my chroot, I'm getting annoying paths like this:
timer_arm (24) [/mnt/host/source/src/platform/ec/common/timer.c:117] 5502c
-> timer_arm [/mnt/host/source/src/platform/ec/common/timer.c:118] 55040
I want the nice relative paths shown in your examples !
Please note that in my case, addr2line does return "/home/vpalatin/trunk/src/platform/ec/common/timer.c" then os.path.realpath() converts it to "/mnt/host/source/src/platform/ec/common/timer.c"
,
Aug 24 2017
My idea is remove the prefix of your current working directory from the path. And fallback to full path if I can't do that (you are in the totally different directory)
,
Aug 24 2017
> And fallback to full path if I can't do that (you are in the totally different directory) No, I'm not in a totally different path I'm executing it from src/platform/ec and "/home/vpalatin/trunk/src/platform/ec" / "/mnt/host/source/src/platform/ec" are the same thing in the chroot.
,
Aug 24 2017
Sorry I didn't explain that clearly. I'm preparing to remove the prefix of path like that, not implemented yet. > You could make the annotation json parsing a bit more robust I updated the CL: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/631082/2
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/af32f8918e119022f5a02e930edcd6e4de75bee1 commit af32f8918e119022f5a02e930edcd6e4de75bee1 Author: Che-yu Wu <cheyuw@google.com> Date: Tue Aug 29 11:45:51 2017 extra/stack_analyzer: Eliminate annotated indirect calls. Indirect calls can be eliminated by adding corresponding annotations. BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm && extra/stack_analyzer/stack_analyzer.py \ --objdump=arm-none-eabi-objdump \ --addr2line=arm-none-eabi-addr2line \ --export_taskinfo=./build/elm/util/export_taskinfo.so \ --section=RW \ --annotation=./extra/stack_analyzer/example_annotation.yaml \ ./build/elm/RW/ec.RW.elf make BOARD=elm SECTION=RW \ ANNOTATION=./extra/stack_analyzer/example_annotation.yaml \ analyzestack Change-Id: I18c317f9c6478b5b431ee04d882453791df27891 Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/631082 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/af32f8918e119022f5a02e930edcd6e4de75bee1/extra/stack_analyzer/example_annotation.yaml [modify] https://crrev.com/af32f8918e119022f5a02e930edcd6e4de75bee1/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/af32f8918e119022f5a02e930edcd6e4de75bee1/extra/stack_analyzer/README.md [modify] https://crrev.com/af32f8918e119022f5a02e930edcd6e4de75bee1/extra/stack_analyzer/stack_analyzer.py
,
Sep 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/816a8d87cd43a3ebb2d454c9e1e02e12581788fc commit 816a8d87cd43a3ebb2d454c9e1e02e12581788fc Author: Che-yu Wu <cheyuw@google.com> Date: Fri Sep 01 07:44:32 2017 extra/stack_analyzer: Support to remove invalid paths. Invalid paths (with arbitrary length) can be annotated and removed. Report set of possible function cycles. Sort the callsite outputs by filename and line number. BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm && extra/stack_analyzer/stack_analyzer.py \ --objdump=arm-none-eabi-objdump \ --addr2line=arm-none-eabi-addr2line \ --export_taskinfo=./build/elm/util/export_taskinfo.so \ --section=RW \ --annotation=./extra/stack_analyzer/example_annotation.yaml \ ./build/elm/RW/ec.RW.elf make BOARD=elm SECTION=RW \ ANNOTATION=./extra/stack_analyzer/example_annotation.yaml \ analyzestack Change-Id: I9d443df6439b55d5b92a7624bdd93cb6e18494e2 Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/640393 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> [modify] https://crrev.com/816a8d87cd43a3ebb2d454c9e1e02e12581788fc/extra/stack_analyzer/example_annotation.yaml [modify] https://crrev.com/816a8d87cd43a3ebb2d454c9e1e02e12581788fc/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/816a8d87cd43a3ebb2d454c9e1e02e12581788fc/extra/stack_analyzer/stack_analyzer.py
,
Sep 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/ce15362e895bac60f18d7fa000029be6e7879e9f commit ce15362e895bac60f18d7fa000029be6e7879e9f Author: Che-yu Wu <cheyuw@google.com> Date: Wed Sep 06 06:01:09 2017 extra/stack_analyzer: Support function sets in invalid paths. There are lots of invalid paths have common segments. In this patch, each vertex of an invalid path can be a function set. Examples can be found in extra/stack_analyzer/example_annotation.yaml BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm SECTION=RW \ ANNOTATION=./extra/stack_analyzer/example_annotation.yaml \ analyzestack Change-Id: Ib10d68edd04725af4d803f54f7e208e55d574c7d Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/649453 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> [modify] https://crrev.com/ce15362e895bac60f18d7fa000029be6e7879e9f/extra/stack_analyzer/example_annotation.yaml [modify] https://crrev.com/ce15362e895bac60f18d7fa000029be6e7879e9f/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/ce15362e895bac60f18d7fa000029be6e7879e9f/extra/stack_analyzer/stack_analyzer.py
,
Sep 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/4a98e9c9847e391d57095bcb1ccea52b676fc0dd commit 4a98e9c9847e391d57095bcb1ccea52b676fc0dd Author: Che-yu Wu <cheyuw@google.com> Date: Wed Sep 06 06:01:10 2017 extra/stack_analyzer: Configurable exception frame size. Make the size of extra stack needed by exceptions configurable. It can be set in the annotation file. BUG= chromium:648840 BRANCH=none TEST=extra/stack_analyzer/stack_analyzer_unittest.py make BOARD=elm SECTION=RW \ ANNOTATION=./extra/stack_analyzer/example_annotation.yaml \ analyzestack Change-Id: Idf2a59650dd20257a0291f89d788c0c83b91a7c9 Signed-off-by: Che-yu Wu <cheyuw@google.com> Reviewed-on: https://chromium-review.googlesource.com/649454 Reviewed-by: Nicolas Boichat <drinkcat@chromium.org> [modify] https://crrev.com/4a98e9c9847e391d57095bcb1ccea52b676fc0dd/extra/stack_analyzer/example_annotation.yaml [modify] https://crrev.com/4a98e9c9847e391d57095bcb1ccea52b676fc0dd/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/4a98e9c9847e391d57095bcb1ccea52b676fc0dd/extra/stack_analyzer/README.md [modify] https://crrev.com/4a98e9c9847e391d57095bcb1ccea52b676fc0dd/extra/stack_analyzer/stack_analyzer.py
,
Sep 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/588320d4b840e4466775815496f266e003586f9d commit 588320d4b840e4466775815496f266e003586f9d Author: Nicolas Boichat <drinkcat@chromium.org> Date: Thu Sep 07 19:56:29 2017 stack_analyzer: Use board/$BOARD/analyzestack.yaml by default BRANCH=none BUG= chromium:648840 TEST=make BOARD=hammer analyzestack Change-Id: Id05fee7e085a02dd4c2d36880f6891c3eb86b404 Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/637550 Reviewed-by: Che-yu Wu <cheyuw@google.com> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/588320d4b840e4466775815496f266e003586f9d/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/588320d4b840e4466775815496f266e003586f9d/extra/stack_analyzer/README.md [modify] https://crrev.com/588320d4b840e4466775815496f266e003586f9d/Makefile.rules [modify] https://crrev.com/588320d4b840e4466775815496f266e003586f9d/extra/stack_analyzer/stack_analyzer.py
,
Sep 13 2017
The assigned owner "cheyuw@google.com" is not able to receive e-mails, please re-triage. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 13 2017
Looks mostly done, thanks to cheyuw@ and drinkcat@. https://www.chromium.org/chromium-os/ec-development/stack-size-analyzer
,
Jan 22 2018
,
Jan 23 2018
,
Mar 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/0cbb4b6f9dc71f6c6d90fd628f466bb61bae9dca commit 0cbb4b6f9dc71f6c6d90fd628f466bb61bae9dca Author: Nicolas Boichat <drinkcat@chromium.org> Date: Mon Mar 19 12:43:45 2018 stack_analyzer: Add new syntax for function pointer arrays Makes it far simpler to support hooks, console commands, host commands. BRANCH=poppy,fizz BUG= chromium:648840 TEST=Add new array annotation, run stack_analyzer Change-Id: I8ed074ba5534661ed59f4f713bb4ba194e712f4e Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/966042 Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/0cbb4b6f9dc71f6c6d90fd628f466bb61bae9dca/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/0cbb4b6f9dc71f6c6d90fd628f466bb61bae9dca/extra/stack_analyzer/README.md [modify] https://crrev.com/0cbb4b6f9dc71f6c6d90fd628f466bb61bae9dca/extra/stack_analyzer/stack_analyzer.py
,
Mar 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/82c9d435d052d3399ed81f33ef6c9d2366c53519 commit 82c9d435d052d3399ed81f33ef6c9d2366c53519 Author: Nicolas Boichat <drinkcat@chromium.org> Date: Mon Mar 19 23:11:07 2018 stack_analyzer: Add new syntax for function pointer arrays Makes it far simpler to support hooks, console commands, host commands. BRANCH=poppy,fizz BUG= chromium:648840 TEST=Add new array annotation, run stack_analyzer Change-Id: I8ed074ba5534661ed59f4f713bb4ba194e712f4e Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/966042 Reviewed-by: Vincent Palatin <vpalatin@chromium.org> (cherry picked from commit 0cbb4b6f9dc71f6c6d90fd628f466bb61bae9dca) Reviewed-on: https://chromium-review.googlesource.com/969721 [modify] https://crrev.com/82c9d435d052d3399ed81f33ef6c9d2366c53519/extra/stack_analyzer/stack_analyzer_unittest.py [modify] https://crrev.com/82c9d435d052d3399ed81f33ef6c9d2366c53519/extra/stack_analyzer/README.md [modify] https://crrev.com/82c9d435d052d3399ed81f33ef6c9d2366c53519/extra/stack_analyzer/stack_analyzer.py |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by bugdroid1@chromium.org
, Aug 14 2017