search

Praqma / memory-map-plugin

A repository for the memory-map-plugin
13 stars 16 forks source link

Fixed the wordsize/radix mixup bug. #37

Closed AndersHoglund closed 6 years ago

AndersHoglund commented 7 years ago

There is a problem where wordSize is incorrectly used as radix when doing hex to dec conversion. Unfortunately the UnitTest only tested with wordsize 16, which also happens to be the radix for hexadecimal numbers, so the bug slipped by in all tests. So, I also added a few UnitTests for various word sizes.

Tested on Jenkins with the gcc-arm-none-eabi-6-2017-q2-update compiler on a STM32F303 project. Linker command file and map files used in the Jenkins test are attached. STM32_map_testfiles.zip

MadsNielsen commented 7 years ago

Hi @AndersHoglund ! Thanks for the pull request. We'll take a look at this when our guy is back from vacation! Thanks for the contribution!

AndersHoglund commented 7 years ago

Experimental sample runs to test this PR and show that it works, even on ARM, here: http://andwho.sytes.net:8080/job/AndWho_Betaflight/

However, we will not put this into production, as we have 100++ targets and map files to check. That is too much for this plugin to handle, would need a more statistical approach to condense that amount of data into a few graphs. Even if we select a few worst case candidates, that would still be too many graphs. However, for a more normal project, with a single executable and a single map file, it could be very useful.

Betaflight production runs with all targets and deliverables here (slow dl): http://andwho.sytes.net:8080/job/BorisB_BetaFlight/ and here: https://ci.betaflight.tech/job/Betaflight/

buep commented 6 years ago

See comment in #40 https://github.com/Praqma/memory-map-plugin/issues/40#issuecomment-357980579 as that change will contain this PR.

When #40 is closed, we close this PR.

buep commented 6 years ago

However, we will not put this into production, as we have 100++ targets and map files to check. That is too much for this plugin to handle, would need a more statistical approach to condense that amount of data into a few graphs. Even if we select a few worst case candidates, that would still be too many graphs.

@AndersHoglund then you might be interested in a little side project we have with some bachelor students that make parsing of memory map files using semantic analysis. The also revises the data structures, so the output will be more useful. I will probably be available in summer, and hope we can replace the current not so maintainable reg exp parsing setup in the plugin with a more common approach.

AndersHoglund commented 6 years ago

Thank you for accepting and merging this fix. Your side project sounds interesting.