Code Coverage using gcov #2629

mazimkhan · 2016-09-06T14:37:52Z

Code Coverage (GCOV)

GCOV based code coverage can be enabled by compiling and linking code with flags -fprofile-arcs -ftest-coverage. Optimisation should be disabled with flag -O0 and debug symbols added with -g flags. An additional coverage macro -DMBED_CFG_DEBUG_OPTIONS_COVERAGE is added to code the instrumentation required for moving coverage data from embedded platform to the host machine under it.

GCOV Impact on application

Enabling gcov profiling has an impact on binary size and stack requirement. Stack size has been increased in rtos/rtx/TARGET_CORTEX_M/cmsis_os.h under coverage macro.

It has not been possible to compile whole of mbed-os with coverage flags as it increases static data part of the binary making heap and stack overlap. Typically giving following error:

[DEBUG] Errors: c:/program files (x86)/gnu tools arm embedded/4.9 2015q3/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld.exe: region m_data_2 overflowed with stack and
 heap

or

[DEBUG] Errors: c:/program files (x86)/gnu tools arm embedded/4.9 2015q3/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld.exe: C:/Users/azikha01/Documents/GitHub/mazimkhan/mbed
-os/.build/tests/K64F/GCC_ARM/./TESTS/integration/basic/basic.elf section `.bss' will not fit in region `m_data_2'

Heap and stack size are reduced a little in below file to fit individual modules: hal/targets/cmsis/TARGET_Freescale/TARGET_MCU_K64F/TOOLCHAIN_GCC_ARM/MK64FN1M0xxx12.ld.

Still whole mbed-os does not fit the memory. For this reason below build tool changes are made to selectively turn On coverage on parts mbed-os source.

Build tools changes

A new option is passed down test.py to class GCC that adds coverage flags to gcc compiler:
Option synopsis:
--coverage-filter <regex to match source path> [--coverage-filter <regex to match source path>]
This new option collects all supplied filters (regex) into a list. Coverage is turned on if list is not empty.

test.py also enables coverage on test case sources when coverage filter is supplied. This is because for gcov to initialise it should also be enabled on the file containing main().

Code changes

gcov flags make the application write profiling data on disk per source file profiled. Embedded applications that do not have file-system require instrumentation to print coverage report on serial console that can be read and consolidated by a host application (greentea).
Mainly open, write and close wrappers in retartget.cpp are modified to channel gcov file data to serial console.

mazimkhan · 2016-09-06T14:38:57Z

@c1728p9 @0xc0170 @bridadan @adbridge Please review

c1728p9 · 2016-09-06T14:52:01Z

rtos/rtx/TARGET_CORTEX_M/cmsis_os.h

@@ -82,7 +82,12 @@

 #endif

+#if defined (MBED_CFG_DEBUG_OPTIONS_COVERAGE)
+/* GCOV works with stack size approx. 32K. */
+#define DEFAULT_STACK_SIZE         (WORDS_STACK_SIZE*4*8)


It might be worth checking stack usage with a debugger to see how big this stack needs to be.

I have two problems with this:

if "gcov works with stack size approx. 32k", that should be stated explicitly in the definition of DEFAULT_STACK_SIZE, rather than depending on the definition of another macro (WORDS_STACK_SIZE) that can change unexpectedly. In other words: #define DEFAULT_STACK_SIZE 32768.

that said, that's a pretty huge stack. Where is the "gcov works with stack size approx. 32k" coming from? I was not able to find this information with google.

mazimkhan · 2016-09-07T16:24:58Z

@bogdanm please review.

bogdanm · 2016-09-08T14:14:31Z

hal/common/retarget.cpp

+/* Coverage sync flag used between greentea_client\source\test_env.cpp:greentea_notify_completion()
+ * and _open() to indicate that file is being open by gcov.
+ */
+bool coverage_report = false;


Doesn't greentea also need a change to work with this flag? Or is this covered in a separate PR (or maybe it already works with this flag)?

Since greentea-client was taken from the yotta module used with mbed-os 3.0 this flag is already implemented in mbed-os/features/frameworks/greentea-client/source/test_env.cpp

bogdanm · 2016-09-08T14:44:01Z

Not yet ready to merge I think:

address @c1728p9's comments about the changed in the .ld file (since they don't seem to do what you expected them to do, is that affecting this PR in any way?)
figure out if the 32k stack size requirement for coverage is indeed a requirement
how does this work in combination with greentea? Does greentea use the same macro to figure out if coverage is enabled, or something else?

mazimkhan · 2016-09-08T14:47:53Z

@bogdanm Yes not until all the comment here are satisfied.

…le and link. Removed flag -fprofile-dir=. as it is ineffactive in our unconventional build system

…er module

mazimkhan · 2016-10-20T21:43:55Z

retest uvisor

bogdanm · 2016-11-01T12:06:33Z

@mazimkhan, are you still looking at this? We need a way to enable code coverage.

mazimkhan · 2016-11-01T13:07:10Z

Yes. This PR enables it. However you can't enable coverage on whole code. Hence looking into fast models.

bogdanm · 2016-11-01T13:15:05Z

Sorry, what do you mean by "you can't enable coverage on whole code"? Why?

mazimkhan · 2016-11-01T13:22:44Z

Ok so we only compile a part of the source files with coverage flags to include coverage code introduced by gcov. This code inserted by gcov is extra to mbed-os code. When enabled on whole source the image size exceeds to fit K64F target.

bogdanm · 2016-11-01T13:24:21Z

Ah OK, I see now. Using --coverage-filter you can enable code coverage only on some parts of the code. That's perfect, thank you.

bogdanm · 2016-11-01T13:29:53Z

I can see some errors related to the "coverage filter" argument, for example https://travis-ci.org/ARMmbed/mbed-os/builds/169266037#L469. Does this need a retest?

mazimkhan · 2016-11-01T14:23:31Z

Some of the failing jobs are not run any more. Those checks will stay. But two checks towards the bottom are passing.
Please try locally. It should build fine.

sg- · 2016-11-01T14:36:37Z

@mazimkhan Can we close this and re-open so current CI is only viewable and tested?

mazimkhan · 2016-11-01T15:04:29Z

Sure. I will recreate it.

…..4a3c5c5 4a3c5c5 Merge remote-tracking branch 'origin/release_internal' into release_external 2b8d2e1 Do not reset radio when MAC data request timeouts (ARMmbed#2647) 95c506a Frame counters for nw keys are now stored to NVM only after send key is set (ARMmbed#2641) 3b3010a Adjusted stagger random to [min,min+max] and for small nw set the stagger value to 10 seconds 02bc33a Adjusted security protocol (EAP-TLS,4WH,2WH) retry timers eb26726 High Priority timestamp compare overflow support fix. 928723a FHSS WS: Initialize broadcast channel count when enabling FHSS (ARMmbed#2642) 6040d70 Updated change log 667b191 Changed initial EAPOL-key retries from trickle to exponential backup d925145 Add RTT calculation for DHCP Time calculation 0b82953 Traceroute bug fix. 04de6e2 Merge pull request ARMmbed#2638 from PelionIoT/mbed_os_fix_ufsi_calculation 2012347 Fixed FHSS UFSI calculation unit tests 436f16e Handle timer rollover in calculate_ufsi 411cf5c coding style d6f4421 Correct ufsi timing calculation 560619d Add network time vendor data element to DHCPv6 reply message 6d290dc System time read/write callbacks (ARMmbed#2637) 7905df6 Restart or remove transmission when MAC data request timeouts (ARMmbed#2636) c97695c Bug fix: EAPOL parent compare fix e283e62 Fixed channel mask usage with OFDM configurations (ARMmbed#2633) 24168f8 Do not send too old packets (ARMmbed#2632) dbd83be Fix copyrights (ARMmbed#2631) 7f0cffd Merge pull request ARMmbed#2630 from PelionIoT/use_pelion_copyright 511bd5a Corrected coding style 57ec028 Corrected comparison 7d853de When EAPOL waiting queue is full oldest entry is removed acf580f Update copyright in changed MDNS files 933c0bb Update copyright 3aeb2af Statistics for data request latencies (ARMmbed#2629) 3f7eae6 EAPOL FHSS temp entry discover 5200b66 DHCP time elapsed time write fix. 0536874 Removed empty EAPOL-key message send after 4WH completion to nodes on relay 8a2a683 Fixed DHCP wrong time elapsed value write. 283f2ee DHCPv6 update: 99be778 EAPOL temp neighbour update 4f9e3d1 Adaptation layer to remove oldest packet first 20f1f64 Added ignoring of retry messages from RADIUS server when waiting EAP-TLS 8a8b407 Add RSL check for ETX Calculation for RPL parent selection c05e1da Fix DHCP server Uninitialized memory read 77229ee Fix CPP error from domain prefix check 7e47889 support filtering of EAPOL parents based device-min-sens configuration 618a191 Wi-SUN Expedite forward state update 4371462 Fix NULL read from RPL header addition 7802c7e Update CHANGELOG.md b2c8104 CHANGELOG for Nanostack v13.0.0 (ARMmbed#2615) git-subtree-dir: features/nanostack/sal-stack-nanostack git-subtree-split: 4a3c5c5

…903b81..4a3c5c5 4a3c5c5 Merge remote-tracking branch 'origin/release_internal' into release_external 2b8d2e1 Do not reset radio when MAC data request timeouts (ARMmbed#2647) 95c506a Frame counters for nw keys are now stored to NVM only after send key is set (ARMmbed#2641) 3b3010a Adjusted stagger random to [min,min+max] and for small nw set the stagger value to 10 seconds 02bc33a Adjusted security protocol (EAP-TLS,4WH,2WH) retry timers eb26726 High Priority timestamp compare overflow support fix. 928723a FHSS WS: Initialize broadcast channel count when enabling FHSS (ARMmbed#2642) 6040d70 Updated change log 667b191 Changed initial EAPOL-key retries from trickle to exponential backup d925145 Add RTT calculation for DHCP Time calculation 0b82953 Traceroute bug fix. 04de6e2 Merge pull request ARMmbed#2638 from PelionIoT/mbed_os_fix_ufsi_calculation 2012347 Fixed FHSS UFSI calculation unit tests 436f16e Handle timer rollover in calculate_ufsi 411cf5c coding style d6f4421 Correct ufsi timing calculation 560619d Add network time vendor data element to DHCPv6 reply message 6d290dc System time read/write callbacks (ARMmbed#2637) 7905df6 Restart or remove transmission when MAC data request timeouts (ARMmbed#2636) c97695c Bug fix: EAPOL parent compare fix e283e62 Fixed channel mask usage with OFDM configurations (ARMmbed#2633) 24168f8 Do not send too old packets (ARMmbed#2632) dbd83be Fix copyrights (ARMmbed#2631) 7f0cffd Merge pull request ARMmbed#2630 from PelionIoT/use_pelion_copyright 511bd5a Corrected coding style 57ec028 Corrected comparison 7d853de When EAPOL waiting queue is full oldest entry is removed acf580f Update copyright in changed MDNS files 933c0bb Update copyright 3aeb2af Statistics for data request latencies (ARMmbed#2629) 3f7eae6 EAPOL FHSS temp entry discover 5200b66 DHCP time elapsed time write fix. 0536874 Removed empty EAPOL-key message send after 4WH completion to nodes on relay 8a2a683 Fixed DHCP wrong time elapsed value write. 283f2ee DHCPv6 update: 99be778 EAPOL temp neighbour update 4f9e3d1 Adaptation layer to remove oldest packet first 20f1f64 Added ignoring of retry messages from RADIUS server when waiting EAP-TLS 8a8b407 Add RSL check for ETX Calculation for RPL parent selection c05e1da Fix DHCP server Uninitialized memory read 77229ee Fix CPP error from domain prefix check 7e47889 support filtering of EAPOL parents based device-min-sens configuration 618a191 Wi-SUN Expedite forward state update 4371462 Fix NULL read from RPL header addition 7802c7e Update CHANGELOG.md b2c8104 CHANGELOG for Nanostack v13.0.0 (ARMmbed#2615) git-subtree-dir: connectivity/nanostack/sal-stack-nanostack git-subtree-split: 4a3c5c5

…..4a3c5c5 4a3c5c5 Merge remote-tracking branch 'origin/release_internal' into release_external 2b8d2e1 Do not reset radio when MAC data request timeouts (ARMmbed#2647) 95c506a Frame counters for nw keys are now stored to NVM only after send key is set (ARMmbed#2641) 3b3010a Adjusted stagger random to [min,min+max] and for small nw set the stagger value to 10 seconds 02bc33a Adjusted security protocol (EAP-TLS,4WH,2WH) retry timers eb26726 High Priority timestamp compare overflow support fix. 928723a FHSS WS: Initialize broadcast channel count when enabling FHSS (ARMmbed#2642) 6040d70 Updated change log 667b191 Changed initial EAPOL-key retries from trickle to exponential backup d925145 Add RTT calculation for DHCP Time calculation 0b82953 Traceroute bug fix. 04de6e2 Merge pull request ARMmbed#2638 from PelionIoT/mbed_os_fix_ufsi_calculation 2012347 Fixed FHSS UFSI calculation unit tests 436f16e Handle timer rollover in calculate_ufsi 411cf5c coding style d6f4421 Correct ufsi timing calculation 560619d Add network time vendor data element to DHCPv6 reply message 6d290dc System time read/write callbacks (ARMmbed#2637) 7905df6 Restart or remove transmission when MAC data request timeouts (ARMmbed#2636) c97695c Bug fix: EAPOL parent compare fix e283e62 Fixed channel mask usage with OFDM configurations (ARMmbed#2633) 24168f8 Do not send too old packets (ARMmbed#2632) dbd83be Fix copyrights (ARMmbed#2631) 7f0cffd Merge pull request ARMmbed#2630 from PelionIoT/use_pelion_copyright 511bd5a Corrected coding style 57ec028 Corrected comparison 7d853de When EAPOL waiting queue is full oldest entry is removed acf580f Update copyright in changed MDNS files 933c0bb Update copyright 3aeb2af Statistics for data request latencies (ARMmbed#2629) 3f7eae6 EAPOL FHSS temp entry discover 5200b66 DHCP time elapsed time write fix. 0536874 Removed empty EAPOL-key message send after 4WH completion to nodes on relay 8a2a683 Fixed DHCP wrong time elapsed value write. 283f2ee DHCPv6 update: 99be778 EAPOL temp neighbour update 4f9e3d1 Adaptation layer to remove oldest packet first 20f1f64 Added ignoring of retry messages from RADIUS server when waiting EAP-TLS 8a8b407 Add RSL check for ETX Calculation for RPL parent selection c05e1da Fix DHCP server Uninitialized memory read 77229ee Fix CPP error from domain prefix check 7e47889 support filtering of EAPOL parents based device-min-sens configuration 618a191 Wi-SUN Expedite forward state update 4371462 Fix NULL read from RPL header addition 7802c7e Update CHANGELOG.md b2c8104 CHANGELOG for Nanostack v13.0.0 (ARMmbed#2615) git-subtree-dir: features/nanostack/sal-stack-nanostack git-subtree-split: 4a3c5c5

c1728p9 reviewed Sep 6, 2016
View reviewed changes

bogdanm reviewed Sep 8, 2016
View reviewed changes

bogdanm added the needs: work label Sep 9, 2016

Mohammad Azim Khan added 11 commits October 20, 2016 11:01

Rebased with master

5651b9b

Increasing stack size for coverage builds

a1e1eff

adjust stack and heap to allow more static created by gcc coverage

d494bfe

Changed --coverage-filter to accept a list of filter

5578749

Code cleanup and debugs removed

0189283

Tuned stack size to be 32K

924b8d8

Consolidate coverage flags in one variable as they are same for compi…

205a970

…le and link. Removed flag -fprofile-dir=. as it is ineffactive in our unconventional build system

Automatically enabling coverage on test when it is enabled on any oth…

ef1f902

…er module

Added code comments

8a7f83e

Only pass coverage filter for GCC_ARM compiler selection

5b5022c

Fixed rebasing errors

f2e8cb8

mazimkhan force-pushed the coverage branch from 8db04cf to f2e8cb8 Compare October 20, 2016 15:34

mazimkhan closed this Nov 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code Coverage using gcov #2629

Code Coverage using gcov #2629

mazimkhan commented Sep 6, 2016

mazimkhan commented Sep 6, 2016

c1728p9 Sep 6, 2016

bogdanm Sep 8, 2016

mazimkhan commented Sep 7, 2016

bogdanm Sep 8, 2016 •

edited

Loading

mazimkhan Sep 8, 2016

bogdanm commented Sep 8, 2016

mazimkhan commented Sep 8, 2016

mazimkhan commented Oct 20, 2016

bogdanm commented Nov 1, 2016

mazimkhan commented Nov 1, 2016 •

edited

Loading

bogdanm commented Nov 1, 2016

mazimkhan commented Nov 1, 2016

bogdanm commented Nov 1, 2016

bogdanm commented Nov 1, 2016

mazimkhan commented Nov 1, 2016

sg- commented Nov 1, 2016

mazimkhan commented Nov 1, 2016

Code Coverage using gcov #2629

Code Coverage using gcov #2629

Conversation

mazimkhan commented Sep 6, 2016

Code Coverage (GCOV)

GCOV Impact on application

Build tools changes

Code changes

mazimkhan commented Sep 6, 2016

c1728p9 Sep 6, 2016

Choose a reason for hiding this comment

bogdanm Sep 8, 2016

Choose a reason for hiding this comment

mazimkhan commented Sep 7, 2016

bogdanm Sep 8, 2016 • edited Loading

Choose a reason for hiding this comment

mazimkhan Sep 8, 2016

Choose a reason for hiding this comment

bogdanm commented Sep 8, 2016

mazimkhan commented Sep 8, 2016

mazimkhan commented Oct 20, 2016

bogdanm commented Nov 1, 2016

mazimkhan commented Nov 1, 2016 • edited Loading

bogdanm commented Nov 1, 2016

mazimkhan commented Nov 1, 2016

bogdanm commented Nov 1, 2016

bogdanm commented Nov 1, 2016

mazimkhan commented Nov 1, 2016

sg- commented Nov 1, 2016

mazimkhan commented Nov 1, 2016

bogdanm Sep 8, 2016 •

edited

Loading

mazimkhan commented Nov 1, 2016 •

edited

Loading