Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IAR: Suppress C "bypasses initialization" warning #7261

Merged
merged 1 commit into from Jun 20, 2018

Conversation

kjbracey
Copy link
Contributor

Description

By default IAR generates "transfer of control bypasses initialization"
warnings for C code - it's a legal construct that frequently occurs when
doing Linux-style "goto error". Many occurrences in Nanostack.

Suppress the warning for C only, to align with GCC and ARMCC. Have to
take care not to put it in the "common" section, as this would suppress
it for C++, where it actually is illegal.

Resolves #7253 and #7254.

Pull request type

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

By default IAR generates "transfer of control bypasses initialization"
warnings for C code - it's a legal construct that frequently occurs when
doing Linux-style "goto error". Many occurrences in Nanostack.

Suppress the warning for C only, to align with GCC and ARMCC. Have to
take care not to put it in the "common" section, as this would suppress
it for C++, where it actually is illegal.
@0xc0170 0xc0170 requested review from a team June 19, 2018 13:04
@cmonr
Copy link
Contributor

cmonr commented Jun 19, 2018

Interesting. Good to know that other tools don't normally do this.

@cmonr
Copy link
Contributor

cmonr commented Jun 19, 2018

/morph build

@deepikabhavnani
Copy link

Instead of suppressing warnings for all C files, shouldn;t we suppress them for relevant files only as done for other compilers here

#pragma diag_suppress 546 // transfer of control bypasses initialization

I would suggest change as below:

#ifdef __CC_ARM
#pragma diag_suppress 546 // transfer of control bypasses initialization
+#elif defined (__ICCARM__)
+#pragma diag_suppress=Pe546 // transfer of control bypasses initialization
#endif

Also please check if we have warnings for GNU ARM compiler as well?

@mbed-ci
Copy link

mbed-ci commented Jun 19, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Jun 19, 2018

@mbed-ci
Copy link

mbed-ci commented Jun 20, 2018

@cmonr cmonr merged commit f11d0f3 into ARMmbed:master Jun 20, 2018
@kjbracey kjbracey deleted the iar-jump-warning branch June 20, 2018 06:21
@kjbracey
Copy link
Contributor Author

@deepikabhavnani - thanks for that - I'd forgotten we did already have a suppression inside Nanostack for ARMCC. GCC does not warn about this, so there's no suppression needed.

Had I remembered, I might have added it in there, but I think the goto error form is so generally useful, and the fact that attempts to avoid this warning usually make code worse makes it worth a global disable.

Possible warning workarounds are:

  1. moving the declaration+initialiser higher up, at a potential runtime/compilation size cost, unnecessarily increasing the scope of the variable
  2. actually split the declaration and initialisation, leaving it uninitialised for some time
  3. putting in { } to end the variable lifetime before the label
  4. avoiding the goto error idiom altogether.

Of those, only 3 is actually an improvement to the code, but can make a mess of your source layout, to the extent it's usually not worthwhile, except if it's switch cases. People hitting the warning usually do 2, in my experience :( That is counter-productive.

This does mean we're now inconsistent - the warning will be present outside Nanostack for ARMCC only. We should resolve that either by limiting this suppression to Nanostack, or adding it to ARMCC globally. I vote for the latter.

@deepikabhavnani
Copy link

We should resolve that either by limiting this suppression to Nanostack, or adding it to ARMCC globally. I vote for the latter.

I am fine with either of the option, as long as it is consistent across compilers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants