Skip to content

tests/cortexm_common_ldscript: update code section for kinetis#11588

Merged
cladmi merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/cortexm_common_ldscripts/prepare_kinetis
May 27, 2019
Merged

tests/cortexm_common_ldscript: update code section for kinetis#11588
cladmi merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/cortexm_common_ldscripts/prepare_kinetis

Conversation

@cladmi
Copy link
Contributor

@cladmi cladmi commented May 27, 2019

Contribution description

Update the 'code' section detection to also work on kinetis.

The boards using 'cortexm.ld' have the code section starting with
'.text'. For the 'cpu/kinetis/kinetis.ls' the first section is '.vector'.

Update the 'awk' matching pattern to correctly detect the kinetis boards.
It is a dependency to allow testing upcoming offset support with kinetis.

I am not 100% sure about the pattern for awk.

Testing procedure

All the currently supported boards still work. It will be tested by CI during compilation (no need to run tests).
You can also run the test for samr21-xpro or iotlab-m3 if you want.

RIOT_CI_BUILD=1 BOARD=iotlab-m3 make
Building application "tests_cortexm_common_ldscript" for "iotlab-m3" with MCU "stm32f1".

   text    data     bss     dec     hex filename
   7888     120    2556   10564    2944 /home/harter/work/git/RIOT/tests/cortexm_common_ldscript/bin/iotlab-m3/tests_cortexm_common_ldscript.elf
Test rom offset 1 byte overflow detection: [OK]
Test rom offset substracted from rom length in elffile: [SKIP](Reason: board does not have a ROM_OFFSET configured)
Test compilation with offset 0x1000: [OK]
Test compilation with offset 0x2000: [OK]
Test compilation with half ROM length: [OK]
Test ROM overflow detection (too_big_for_rom): [OK]
Test ROM overflow detection (offset_and_romlen): [OK]

For the frdm-kw41z the test fails with only this pull request but works with #11562 . However the address of 0x0000 is currently detected and fails as expected.

RIOT_CI_BUILD=1 BOARD=frdm-kw41z BOARD_WHITELIST=frdm-kw41z make
Building application "tests_cortexm_common_ldscript" for "frdm-kw41z" with MCU "kinetis".

   text    data     bss     dec     hex filename
   8928     116    2544   11588    2d44 /home/harter/work/git/RIOT/tests/cortexm_common_ldscript/bin/frdm-kw41z/tests_cortexm_common_ldscript.elf
Test rom offset 1 byte overflow detection: [OK]
Test rom offset substracted from rom length in elffile: [SKIP](Reason: board does not have a ROM_OFFSET configured)
Test compilation with offset 0x1000: [ERROR] Linker offset not used 0x00000000 != 0x00001000
Makefile:95: recipe for target 'test-offset_0x1000' failed
make: *** [test-offset_0x1000] Error 1

With master it was detecting 0x410 which was not the base of the whole code section as it starts with .vector/.fcfield/.text.

RIOT_CI_BUILD=1 BOARD=frdm-kw41z BOARD_WHITELIST=frdm-kw41z make
Building application "tests_cortexm_common_ldscript" for "frdm-kw41z" with MCU "kinetis".

   text    data     bss     dec     hex filename
   8928     116    2544   11588    2d44 /home/harter/work/git/RIOT/tests/cortexm_common_ldscript/bin/frdm-kw41z/tests_cortexm_common_ldscript.elf
Test rom offset 1 byte overflow detection: [OK]
Test rom offset substracted from rom length in elffile: [SKIP](Reason: board does not have a ROM_OFFSET configured)
Test compilation with offset 0x1000: [ERROR] Linker offset not used 0x00000410 != 0x00001000
Makefile:90: recipe for target 'test-offset_0x1000' failed
make: *** [test-offset_0x1000] Error 1

Issues/PRs references

Part of #11562

Update the 'code' section detection to also work on kinetis.

The boards using 'cortexm.ld' have the code section starting with
'.text'. For the 'cpu/kinetis/kinetis.ls' the first section is '.vector'.

Update the 'awk' matching pattern to correctly detect the kinetis boards.
It is a dependency to allow testing upcoming offset support with kinetis.

I am not 100% sure about the pattern for awk.
@cladmi cladmi added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: OTA Area: Over-the-air updates labels May 27, 2019
@cladmi cladmi added this to the Release 2019.07 milestone May 27, 2019
@cladmi cladmi requested a review from fjmolinas May 27, 2019 13:53
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested for some kinetis boards: frdm-kw41z pba-d-01-kw2x.

Can you add them to the whitelist? at least frdm-kw41z since riotboot should be soon supported. Also l1 and l0 boards pass the test but aren't white listed.

@cladmi
Copy link
Contributor Author

cladmi commented May 27, 2019

@fjmolinas it currently does not work for frdm-kw41z and pba-d-01-kw2x with only this PR as the offset handling is done in your pull request :)

It is also shown in my testing procedure.

@fjmolinas
Copy link
Contributor

@cladmi you are right, should be in the next one. I was jumping a step.

@fjmolinas
Copy link
Contributor

I am not 100% sure about the pattern for awk. do you want this in the commit history? Can you elaborate on why you are not 100% sure, as i understand it the code section will be the first one starting with PROGBITS, before that you would get NULL, right?

@cladmi
Copy link
Contributor Author

cladmi commented May 27, 2019

@fjmolinas Because I do not know about how to write patterns for awk :D
It may match thing we do not want. By saying "I am not sure" means, if you find a mistake, there are a lot of reasons that it is my fault.

But I can remove it, I wrote it so the pattern gets a second look.

# * [ 1] .text PROGBITS 08001000 001000 001ef8 ...
# * [ 1] .vector PROGBITS 00001000 001000 000400 ...
# So match with the section [ 1] being `PROGBITS`. Adapt if new cases appear.
CODE_SECTION = \[ 1\] \.[a-zA-Z_.]* *PROGBITS
Copy link
Contributor

@fjmolinas fjmolinas May 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[a-zA-Z_.] Do sections have "." in the middle of their names? you are already matching the one leading the section name.

Copy link
Contributor Author

@cladmi cladmi May 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the output of for an iotlab-m3 as example:

And .ARM.attributes has one.

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        08001000 001000 001ed0 00  AX  0   0  4
  [ 2] .stack            NOBITS          20000000 020000 000200 00  WA  0   0  1
readelf: Warning: [ 3]: Link field (0) should index a symtab section.
  [ 3] .relocate         REL             20000200 010200 000078 08  WA  0   0  4
  [ 4] .noinit           PROGBITS        20000278 010278 000000 00   W  0   0  1
  [ 5] .bss              NOBITS          20000278 010278 0007fc 00  WA  0   0  4
  [ 6] .debug_info       PROGBITS        00000000 010278 016389 00      0   0  1
  [ 7] .debug_abbrev     PROGBITS        00000000 026601 00378e 00      0   0  1
  [ 8] .debug_aranges    PROGBITS        00000000 029d8f 000558 00      0   0  1
  [ 9] .debug_ranges     PROGBITS        00000000 02a2e7 0005e8 00      0   0  1
  [10] .debug_macro      PROGBITS        00000000 02a8cf 01e43a 00      0   0  1
  [11] .debug_line       PROGBITS        00000000 048d09 009a70 00      0   0  1
  [12] .debug_str        PROGBITS        00000000 052779 03e32e 01  MS  0   0  1
  [13] .comment          PROGBITS        00000000 090aa7 00007f 01  MS  0   0  1
  [14] .ARM.attributes   ARM_ATTRIBUTES  00000000 090b26 000033 00      0   0  1
  [15] .debug_frame      PROGBITS        00000000 090b5c 00105c 00      0   0  4
  [16] .debug_loc        PROGBITS        00000000 091bb8 001a2d 00      0   0  1
  [17] .symtab           SYMTAB          00000000 0935e8 001d90 10     18 271  4
  [18] .strtab           STRTAB          00000000 095378 000c0a 00      0   0  1
  [19] .shstrtab         STRTAB          00000000 095f82 0000cb 00      0   0  1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect :)

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code matching looks good to me. Regarding the commit message I leave it up to you :) ACK, you cant change it or merge you PR!

@cladmi
Copy link
Contributor Author

cladmi commented May 27, 2019

Ok, I keep my warning then :)

@cladmi cladmi merged commit 1c4ab53 into RIOT-OS:master May 27, 2019
@cladmi
Copy link
Contributor Author

cladmi commented May 27, 2019

Thank you for the review.

@cladmi cladmi deleted the pr/cortexm_common_ldscripts/prepare_kinetis branch May 27, 2019 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: OTA Area: Over-the-air updates Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants