Remove unreachable statements warnings #11439

hugueskamba · 2019-09-09T08:52:55Z

Description

Remove unreachable statements warnings reported when building with the IAR toolchain.

Warnings observed when building the mbed-os-example-blinky example project with the following command:
mbed compile -t IAR -m K64F

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

ciarmcom · 2019-09-09T09:00:22Z

@hugueskamba, thank you for your changes.
@ARMmbed/mbed-os-hal @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

hal/mbed_pinmap_common.c

drivers/source/usb/USBDevice.cpp

kjbracey · 2019-09-10T07:32:53Z

Note that some environments would actually treat an ASSERT as a guarantee in a release build - they might optimise the rest of the code on the assumption that the condition wasn't met.

For example:

 CUSTOM_ASSERT(x > 3);
 if (x <= 3) {
      return;
 }

When NDEBUG is off, the compiler must check for !(x > 3) in the assert, and if it knows an assert failure doesn't return, then it knows x > 3 if it reaches the if, so it doesn't need to actually put the if (x <=3) return; code in, and probably won't.

But conceivably in a release build with NDEBUG, it could leave out the assert failure check, but still believe the assertion - assume that x > 3 as we claim, and hence still leave out the if. (This would be achieved by using a __builtin_unreachable() directive in the NDEBUG assert implementation).

We (and the standard C library assert) don't do that - instead both guarantee that our macro does nothing if NDEBUG is set, rather than believing the assertion. But it is a possible different assert implementation.

I guess my point is to consider what would happen if the compiler believed our assertions. Would the rest of the code make sense? If not, you probably want to have more explicit "if (condition) error or return" code.

hugueskamba · 2019-09-11T09:27:38Z

This force-push uses the "if (condition) error or return" code instead of assertions.

kjbracey

Seems reasonable to me.

drivers/source/usb/USBDevice.cpp

0xc0170 · 2019-09-11T12:50:50Z

CI started

mbed-ci · 2019-09-11T14:29:26Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2019-09-12T05:51:09Z

CI restarted

mbed-ci · 2019-09-12T07:28:17Z

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test
jenkins-ci/mbed-os-ci_cloud-client-test

mbed-ci · 2019-09-12T08:16:56Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

hugueskamba · 2019-09-12T08:48:42Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

jenkins-ci/mbed-os-ci_build-GCC_ARM

jenkins-ci/mbed-os-ci_build-IAR

@0xc0170,
Is this an internal error? I am sure which log file to look at.

0xc0170 · 2019-09-12T09:59:48Z

Will restart after rc2 generation, need 2 more patches to get in

0xc0170 · 2019-09-12T10:08:05Z

Aborted, rc2 jobs are in now, will be restarted

mbed-ci · 2019-09-12T10:08:55Z

Test run: FAILED

Summary: 6 of 11 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_mbed2-build-IAR
jenkins-ci/mbed-os-ci_dynamic-memory-usage
jenkins-ci/mbed-os-ci_greentea-test
jenkins-ci/mbed-os-ci_exporter

0xc0170 · 2019-09-12T16:03:28Z

Ci restarted

mbed-ci · 2019-09-12T20:52:05Z

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2019-09-13T10:32:40Z

tests restarted

fixed

0xc0170 · 2019-09-16T12:10:29Z

Ci restarted again

0xc0170 · 2019-09-18T06:18:35Z

CI started

mbed-ci · 2019-09-18T11:08:03Z

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2019-09-18T11:13:59Z

Test restarted

0xc0170 · 2019-09-19T08:21:39Z

CI restarted again

0xc0170 · 2019-09-20T07:25:46Z

Restarted entire pipeline

mbed-ci · 2019-09-20T14:55:03Z

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 7
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2019-09-23T10:04:56Z

hal/mbed_ticker_api.c

@@ -41,8 +41,9 @@ static void initialize(const ticker_data_t *ticker)

    const ticker_info_t *info = ticker->interface->get_info();
    uint32_t frequency = info->frequency;
+
+    MBED_ASSERT(info->frequency == 0);


Please review latest failures. Looks like we are hitting this assert in the tests. ERROR reported for all platforms there.

#11439 (comment)

The warnings are observed when building with the IAR toolchain.

hugueskamba · 2019-09-23T11:00:21Z

This force-push uses the "if (condition) error or return" code instead of assertions for mbed-os/hal/mbed_ticker_api.c.

0xc0170 · 2019-09-24T16:39:44Z

CI restarted

mbed-ci · 2019-09-24T19:29:59Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 8
Build artifacts

ciarmcom requested review from a team September 9, 2019 09:00

ciarmcom added the needs: review label Sep 9, 2019

evedon previously requested changes Sep 9, 2019

View reviewed changes

hal/mbed_pinmap_common.c Outdated Show resolved Hide resolved

hugueskamba force-pushed the hk-remove-unreachable-statement-warnings branch from 1041181 to f02a4f1 Compare September 10, 2019 06:28

kjbracey suggested changes Sep 10, 2019

View reviewed changes

drivers/source/usb/USBDevice.cpp Outdated Show resolved Hide resolved

hugueskamba force-pushed the hk-remove-unreachable-statement-warnings branch from f02a4f1 to 682e2ef Compare September 10, 2019 07:02

0xc0170 requested a review from evedon September 10, 2019 09:17

hugueskamba force-pushed the hk-remove-unreachable-statement-warnings branch from 682e2ef to eb5574d Compare September 11, 2019 09:26

kjbracey approved these changes Sep 11, 2019

View reviewed changes

drivers/source/usb/USBDevice.cpp Outdated Show resolved Hide resolved

hugueskamba force-pushed the hk-remove-unreachable-statement-warnings branch from eb5574d to e58f40b Compare September 11, 2019 10:57

kjbracey approved these changes Sep 11, 2019

View reviewed changes

0xc0170 approved these changes Sep 11, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Sep 11, 2019

0xc0170 requested changes Sep 23, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: CI labels Sep 23, 2019

Remove unreachable statements warnings

5724c4c

The warnings are observed when building with the IAR toolchain.

hugueskamba force-pushed the hk-remove-unreachable-statement-warnings branch from e58f40b to 5724c4c Compare September 23, 2019 10:58

0xc0170 self-requested a review September 25, 2019 10:24

0xc0170 added needs: CI and removed needs: work needs: CI labels Sep 25, 2019

0xc0170 approved these changes Sep 25, 2019

View reviewed changes

0xc0170 added the ready for merge label Sep 26, 2019

0xc0170 merged commit ba7b479 into ARMmbed:master Sep 26, 2019

0xc0170 added release-version: 5.14.1 and removed ready for merge labels Sep 26, 2019

hugueskamba deleted the hk-remove-unreachable-statement-warnings branch December 22, 2019 22:12

Remove unreachable statements warnings #11439

Remove unreachable statements warnings #11439

Conversation

hugueskamba commented Sep 9, 2019

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Sep 9, 2019

Uh oh!

Uh oh!

Uh oh!

kjbracey commented Sep 10, 2019

Uh oh!

hugueskamba commented Sep 11, 2019

Uh oh!

kjbracey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xc0170 commented Sep 11, 2019

Uh oh!

mbed-ci commented Sep 11, 2019

Uh oh!

0xc0170 commented Sep 12, 2019

Uh oh!

mbed-ci commented Sep 12, 2019

Uh oh!

mbed-ci commented Sep 12, 2019

Uh oh!

hugueskamba commented Sep 12, 2019

Uh oh!

0xc0170 commented Sep 12, 2019

Uh oh!

0xc0170 commented Sep 12, 2019

Uh oh!

mbed-ci commented Sep 12, 2019

Uh oh!

0xc0170 commented Sep 12, 2019

Uh oh!

mbed-ci commented Sep 12, 2019

Uh oh!

0xc0170 commented Sep 13, 2019

Uh oh!

0xc0170 commented Sep 16, 2019

Uh oh!

0xc0170 commented Sep 18, 2019

Uh oh!

mbed-ci commented Sep 18, 2019

Uh oh!

0xc0170 commented Sep 18, 2019

Uh oh!

0xc0170 commented Sep 19, 2019

Uh oh!

0xc0170 commented Sep 20, 2019

Uh oh!

mbed-ci commented Sep 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugueskamba commented Sep 23, 2019

Uh oh!

0xc0170 commented Sep 24, 2019

Uh oh!

mbed-ci commented Sep 24, 2019

Uh oh!

Uh oh!