FlashIAP & NVStore tests: Skip test if overwriting code in flash #7212

davidsaada · 2018-06-13T16:44:04Z

Description

The FlashIAP test erases the last two sectors in some of its test cases. NVStore test does the same. However, with boards that have small flash components (and large last sectors), this test may overwrite the code section. This fix checks that scenario and skips the test if it is indeed the case.
Resolves #7149 and #7388.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

davidsaada · 2018-06-13T16:46:33Z

@jeromecoutant Would be glad if you could test this fix on your board, as I don't have it. If you can do it on all supported three toolchains (GCC_ARM, ARM, IAR), it would be best. Note that if it works, you should see a print telling the test was skipped.

0xc0170 · 2018-06-14T08:13:01Z

TESTS/mbed_drivers/flashiap/main.cpp

+#endif
+
+// This macro returns if flash test overwrites the code in flash.
+#define RETURN_IF_TEST_OVERWRITES_CODE(ADDRESS) \


this would be nicer if its just a function, no need for a macro?

I'm not a big macro fan myself. However, in this case, macro seems like the better solution, as it includes a return inside. Turning this into a function would make the code a bit more bloated, as it would make all three tests check the return value, and return if it indicates that the code is getting overwritten. No biggie, can change if required.

👍 Fine as it is

jeromecoutant · 2018-06-15T08:59:47Z

Hi
It looks OK with F410 and GCC,

But I couldn't compile for L475VG and ARM...

[DEBUG] Errors: Error: L6218E: Undefined symbol Load$$LR$$LR_m_text$$Limit (referred from BUILD/tests/DISCO_L475VG_IOT01A/ARM/TESTS/mbed_drivers/flashiap/main.o).

davidsaada · 2018-06-15T10:03:25Z

@jeromecoutant Yes I am able to reproduce this problem with the ARM compiler (the other toolchains seem to behave properly). Will probably need more time to investigate.

davidsaada · 2018-06-18T14:52:07Z

@jeromecoutant Please retest with the ARM compiler. Made a change that depends on another open PR, so even if it works, it'll be some time until it gets merged. You only need to pull TESTS/mbed_drivers/flashiap/main.cpp (other files aren't relevant for your target).

jeromecoutant · 2018-06-19T11:36:23Z

Hi

ARM: OK
GCC: OK
IAR: test crashes (it doesn't detect the end of code section ?)

davidsaada · 2018-06-20T12:34:48Z

IAR: test crashes (it doesn't detect the end of code section ?)

OK, I think I understand the problem (has to do with the beginning of the text section). Pushed a fix for it now. Keeps working on the boards I have.
@jeromecoutant Would appreciate retesting. I added a debug print in the beginning of the init test. Will be used if the test doesn't pass and removed if it does.

jeromecoutant

As discussed by mail, test seems OK with ARM, GCC and IAR now

davidsaada · 2018-06-21T08:00:18Z

Thanks @jeromecoutant. Just pushed a commit without the debug print.

0xc0170 · 2018-06-21T09:16:14Z

/morph build

mbed-ci · 2018-06-21T10:37:20Z

Build : FAILURE

Build number : 2412
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7212/

cmonr · 2018-06-21T13:08:41Z

@davidsaada Please review the build linker issues.

cmonr · 2018-08-15T15:28:26Z

@ARMmbed/mbed-os-storage Mind taking a quick look at the test changes? Should be fine.

offirko

Looks great to me

mbed-ci · 2018-08-16T07:56:03Z

Test : FAILURE

Build number : 2549
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7212/2549

davidsaada · 2018-08-16T12:19:03Z

Test needs to be rerun. Failure is on a timeout after flashing the image. Tried myself with the same board and it works OK.

cmonr · 2018-08-16T14:11:17Z

/morph test

Remove unnecessary manual inclusion of tmpm64b_fc object file in linker script

mbed-ci · 2018-08-17T00:48:03Z

Test : FAILURE

Build number : 2559
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7212/2559

cmonr · 2018-08-17T00:55:17Z

Welp, at least now the test that caused this PR to fail is not longer in the repo.

/morph test

cmonr · 2018-08-17T12:35:14Z

/morph test

davidsaada · 2018-08-17T13:14:21Z

@cmonr Rebased following merge of #7670 (as both modify the NVstore tests). Please rerun morph build when done.

cmonr · 2018-08-17T14:54:52Z

/morph build

mbed-ci · 2018-08-17T16:02:39Z

Build : SUCCESS

Build number : 2821
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7212/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

cmonr · 2018-08-17T16:09:37Z

/morph mbed2-build

mbed-ci · 2018-08-17T18:08:44Z

Exporter Build : SUCCESS

Build number : 2451
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7212/

mbed-ci · 2018-08-17T22:08:21Z

Test : SUCCESS

Build number : 2571
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7212/2571

davidsaada · 2018-08-19T07:42:25Z

@cmonr Don't know what this failure is about, probably not related to this PR. Regardless, please note that this PR is depending on #7791. As both PRs are on the verge of merging (after uVisor test failure gets sorted out), I don't think we should rebase this one and go through the entire morph build process again. Please just take this into consideration if and when both PRs get merged.

cmonr · 2018-08-19T23:39:10Z

It's possible that a commit that was merged caused the uVisor test to break in a strange way. We've been keeping an eye on it this weekend since it's blocking a whole slew of PRs from being merged :(

NirSonnenschein · 2018-08-21T08:24:18Z

/morph uvisor-test

…mall_flash FlashIAP & NVStore tests: Skip test if overwriting code in flash

cmonr · 2018-08-23T22:27:22Z

Retagging for 5.10 since FLASHIAP_ROM_END is not in a patch release.

cmonr added the needs: review label Jun 13, 2018

cmonr requested a review from jeromecoutant June 13, 2018 16:50

0xc0170 reviewed Jun 14, 2018

View reviewed changes

0xc0170 previously approved these changes Jun 14, 2018

View reviewed changes

davidsaada dismissed 0xc0170’s stale review via a7657df June 18, 2018 14:49

davidsaada force-pushed the david_flashiap_test_small_flash branch from 5900707 to a7657df Compare June 18, 2018 14:49

davidsaada force-pushed the david_flashiap_test_small_flash branch from a7657df to 3097d66 Compare June 18, 2018 15:20

0xc0170 added needs: work and removed needs: review labels Jun 20, 2018

davidsaada force-pushed the david_flashiap_test_small_flash branch 2 times, most recently from 908711d to dc5147f Compare June 20, 2018 12:31

davidsaada force-pushed the david_flashiap_test_small_flash branch from dc5147f to 102eeaa Compare June 20, 2018 15:45

jeromecoutant previously approved these changes Jun 21, 2018

View reviewed changes

davidsaada dismissed jeromecoutant’s stale review via 0e7f7f4 June 21, 2018 07:59

davidsaada force-pushed the david_flashiap_test_small_flash branch from 102eeaa to 0e7f7f4 Compare June 21, 2018 07:59

0xc0170 approved these changes Jun 21, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Jun 21, 2018

cmonr added needs: work and removed needs: CI labels Jun 21, 2018

offirko approved these changes Aug 15, 2018

View reviewed changes

David Saada added 2 commits August 16, 2018 21:59

Fix TMPM64B IAR linker file

62a70f4

Remove unnecessary manual inclusion of tmpm64b_fc object file in linker script

FlashIAP & NVStore tests: Skip test if overwriting code in flash.

876b5f7

davidsaada force-pushed the david_flashiap_test_small_flash branch from 7fddba1 to 876b5f7 Compare August 17, 2018 13:12

0xc0170 added ready for merge and removed needs: CI labels Aug 21, 2018

cmonr merged commit c10ad7f into ARMmbed:master Aug 21, 2018

0xc0170 removed the ready for merge label Aug 21, 2018

davidsaada deleted the david_flashiap_test_small_flash branch August 22, 2018 09:09

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Merge pull request ARMmbed#7212 from davidsaada/david_flashiap_test_s…

d52dd4b

…mall_flash FlashIAP & NVStore tests: Skip test if overwriting code in flash

cmonr added release-version: 5.10.0-rc1 and removed release-version: 5.9.6 labels Aug 23, 2018

jeromecoutant mentioned this pull request Oct 26, 2018

NVSTORE test issue #8555

Closed

FlashIAP & NVStore tests: Skip test if overwriting code in flash #7212

FlashIAP & NVStore tests: Skip test if overwriting code in flash #7212

Uh oh!

Conversation

davidsaada commented Jun 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

davidsaada commented Jun 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeromecoutant commented Jun 15, 2018

Uh oh!

davidsaada commented Jun 15, 2018

Uh oh!

davidsaada commented Jun 18, 2018

Uh oh!

jeromecoutant commented Jun 19, 2018

Uh oh!

davidsaada commented Jun 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeromecoutant left a comment

Choose a reason for hiding this comment

Uh oh!

davidsaada commented Jun 21, 2018

Uh oh!

0xc0170 commented Jun 21, 2018

Uh oh!

mbed-ci commented Jun 21, 2018

Uh oh!

cmonr commented Jun 21, 2018

Uh oh!

cmonr commented Aug 15, 2018

Uh oh!

offirko left a comment

Choose a reason for hiding this comment

Uh oh!

mbed-ci commented Aug 16, 2018

Uh oh!

davidsaada commented Aug 16, 2018

Uh oh!

cmonr commented Aug 16, 2018

Uh oh!

mbed-ci commented Aug 17, 2018

Uh oh!

cmonr commented Aug 17, 2018

Uh oh!

cmonr commented Aug 17, 2018

Uh oh!

davidsaada commented Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Aug 17, 2018

Uh oh!

mbed-ci commented Aug 17, 2018

Triggering tests

Uh oh!

cmonr commented Aug 17, 2018

Uh oh!

mbed-ci commented Aug 17, 2018

Uh oh!

mbed-ci commented Aug 17, 2018

Uh oh!

davidsaada commented Aug 19, 2018

Uh oh!

cmonr commented Aug 19, 2018

Uh oh!

NirSonnenschein commented Aug 21, 2018

Uh oh!

cmonr commented Aug 23, 2018

Uh oh!

Uh oh!

davidsaada commented Jun 13, 2018 •

edited

Loading

davidsaada commented Jun 20, 2018 •

edited

Loading

davidsaada commented Aug 17, 2018 •

edited

Loading