STMicroelectronics / STM32CubeU5

Full Firmware Package for the STM32U5 series: HAL+LL drivers, CMSIS, BSP, MW, plus a set of Projects (examples and demos) running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits).
Other
116 stars 61 forks source link

SBSFU app imgtool bug #46

Open tdjastrzebski opened 3 months ago

tdjastrzebski commented 3 months ago

STM32CubeU5 SBSFU app (latest revision 1.5) comes with probably custom imgtool, which has at least one additional flash command not found in the current original MCUboot implementation. Method description reads: modify bash /batch variable value with layout file content Essentially, this method allows to update shell script variable values based on macro-preprocessed C file like image_macros_to_preprocess_bl2.c or this test example:

enum image_attributes
{
    RE_IMAGE_FLASH_NON_SECURE_IMAGE_SIZE = (0x20000 - (0x10000 + 0x8000)) // yields 0x18000 instead of 0x8000
};

After running this command: ./Middlewares/mcuboot/scripts/dist/imgtool/imgtool.exe flash --layout ./<my_path>/test.c -b image_ns_size -m RE_IMAGE_FLASH_NON_SECURE_IMAGE_SIZE -s 0 ./test.sh image_ns_size variable value in test.sh script should get updated to 0x8000 value but it does not. Instead, 0x18000 value gets written. Test files attached. test.zip

The current implementation does not always correctly subtract values. The culprit is macro_parser.py where this problem is even mentioned but it is not a solution, it is a particularly nasty trap. I lost many hours before I found out what I described here.

I am not a Python expert but probably there are at least several way better approaches that could be used like py-expression, pyparsing or lark parser (sample code) - see how easy it is to use.

There are also other problems with this tool. 1) In case file path does not exist, misleading "local variable 'value' referenced before assignment" error message is displayed. 2) Unlike the output file path, input file path must be absolute.

Why command modifying scripts is named flash? Perhaps more adequate name could be used. Why is this function part of the imgtool at all? It has very little to do with image creation. A separate tool would be much better, especially if one wants/needs to use the latest original imgtool version.

ALABSTM commented 2 months ago

Hi @tdjastrzebski,

Issue forwarded to our development teams. I will keep you informed. Thank you for having reported.

With regards,

ALABSTM commented 2 months ago

ST Internal Reference: 179487