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

[cmsis_gcc.h] nested extern declaration #617

Open
salkinium opened this issue Jun 25, 2019 · 29 comments
Open

[cmsis_gcc.h] nested extern declaration #617

salkinium opened this issue Jun 25, 2019 · 29 comments
Assignees

Comments

@salkinium
Copy link
Contributor

When compiling with -Wnested-externs, these warnings appear:

modm/ext/cmsis/core/cmsis_gcc.h: In function '__cmsis_start':
modm/ext/cmsis/core/cmsis_gcc.h:133:15: warning: nested extern declaration of '_start' [-Wnested-externs]
   extern void _start(void) __NO_RETURN;
               ^~~~~~
modm/ext/cmsis/core/cmsis_gcc.h:146:31: warning: nested extern declaration of '__copy_table_start__' [-Wnested-externs]
   extern const __copy_table_t __copy_table_start__;
                               ^~~~~~~~~~~~~~~~~~~~
modm/ext/cmsis/core/cmsis_gcc.h:147:31: warning: nested extern declaration of '__copy_table_end__' [-Wnested-externs]
   extern const __copy_table_t __copy_table_end__;
                               ^~~~~~~~~~~~~~~~~~
modm/ext/cmsis/core/cmsis_gcc.h:148:31: warning: nested extern declaration of '__zero_table_start__' [-Wnested-externs]
   extern const __zero_table_t __zero_table_start__;
                               ^~~~~~~~~~~~~~~~~~~~
modm/ext/cmsis/core/cmsis_gcc.h:149:31: warning: nested extern declaration of '__zero_table_end__' [-Wnested-externs]
   extern const __zero_table_t __zero_table_end__;
                               ^~~~~~~~~~~~~~~~~~

I'm currently working around this by #define __PROGRAM_START 1, but that's pretty brute force.

@salkinium
Copy link
Contributor Author

Uses the latest version of these files.

@JonatanAntoni
Copy link
Member

Hi @salkinium,

thanks for pointing this out. These warnings are caused by the reworked C startup code (compiler agnostic). I'll check what's behind and how we can change it.

As long as you are using the former startup routines (compiler specific) you are fine with defining __PROGRAM_START yourself. The new startup code would fail if __PROGRAM_START doesn't name a proper library (or application) start routine.

Cheers,
Jonatan

@salkinium
Copy link
Contributor Author

Yeah, redefining __PROGRAM_STARTis very hacky and fragile. Unfortunately, since it‘s a header file, it‘s a bit difficult to not leak the extern variables everywhere… So I understand why they were extern declared inside the function.

@JonatanAntoni
Copy link
Member

Hi @salkinium,

I checked with our GCC experts and the code should just be fine. Do you have any concerns if we simply suppress that warning in that location? I.e. by adding #pragma GCC diagnostic ignored "-Wnested-externs"?

Cheers,
Jonatan

@salkinium
Copy link
Contributor Author

salkinium commented Jun 26, 2019

No, suppressing the warning locally is fine for me.

@JonatanAntoni
Copy link
Member

May I ask you for what reason you enabled that warning? There seems nothing wrong with using this kind of extern declaration. Hence I am now struggling between adding the pragmas to suppress the warning locally versus keeping the code clean. As I don't see any problems with that code I tend to conclude: That warning should not be used.

@salkinium
Copy link
Contributor Author

cc @dergraaf Why was -Wnested-externs added?

Is there a complete list of compile flags to use to compile your code?
Then I can check what the differences are and adapt our flags to that.
Or since we only include the header file once, I can also wrap the include in the #pragma.

@salkinium
Copy link
Contributor Author

Or since we only include the header file once, I can also wrap the include in the #pragma.

I've solved it this way. I think this is best since -Wnested-externs is a style choice, not a technical one.

Thanks for you help!

@zejiang0jason
Copy link

Hi @JonatanAntoni , I got the similar error while including the "arm_math.h" in *.cpp file, the gcc error is:

const __cmsis_start()::__copy_table_t __copy_table_start__', declared using local type 'const __cmsis_start()::__copy_table_t', is used but never defined [-fpermissive]
   extern const __copy_table_t __copy_table_start__;

The flag -fpermissive could convert the error to warning, but this is not a good solution for our customers. What is your suggestions for this? Is there a plan to update for this?

By the way, when I changed the cmsis_gcc.h, moving

  extern void _start(void) __NO_RETURN;

  typedef struct {
    uint32_t const* src;
    uint32_t* dest;
    uint32_t  wlen;
  } __copy_table_t;

  typedef struct {
    uint32_t* dest;
    uint32_t  wlen;
  } __zero_table_t;

  extern const __copy_table_t __copy_table_start__;
  extern const __copy_table_t __copy_table_end__;
  extern const __zero_table_t __zero_table_start__;
  extern const __zero_table_t __zero_table_end__;

out of the function __cmsis_start, then the error disappears.

@JonatanAntoni
Copy link
Member

Hi @zejiangfish,

I guess this happens because the symbols in question (e.g. __copy_table_start__) are linker generated. Hence the compiler doesn't see the definition.

We could of course move the type defines and extern declarations into global scope. I added them to local scope in order not to clutter the global scope.

Cheers,
Jonatan

@JonatanAntoni JonatanAntoni reopened this Jan 10, 2020
@zejiang0jason
Copy link

Hi @JonatanAntoni ,

Thank you very much for analyze, is there plan for this error? Or some suggestions? thanks.

@noxuz
Copy link

noxuz commented Mar 5, 2020

Facing the same problem here, moving the type defines out of the function clears the compiler error.

error: 'const __cmsis_start()::__copy_table_t __copy_table_start__', declared using local type 'const __cmsis_start()::__copy_table_t', is used but never defined [-fpermissive]

@JonatanAntoni
Copy link
Member

Hi @noxuz,

Thanks for getting in touch. I had clarified with our Compiler experts back in June last year that these nested declarations should just be fine.

Why is this causing errors for you?

Cheers,
Jonatan

@noxuz
Copy link

noxuz commented Mar 29, 2020

@JonatanAntoni Thank you for the follow-up.

I was trying to compile a project using directly the CMSIS library from this repo, although I was not using that particular function, the compiler did popped the same error from the top of this post.

I'm currently working with NXP mcu's, and I ended up using the CMSIS-like header that its provided from within the IDE for it. You can take a look at it in this Link.

yan12125 added a commit to EMCLab-Sinica/ARM-CMSIS_5 that referenced this issue Jul 30, 2020
@yan12125
Copy link

I got the same error as @zejiangfish, and I managed to get a patch with only the struct declarations moved: EMCLab-Sinica@f37a114. extern symbol declarations are kept local. That should reduce the risk of symbol conflicts.

@JonatanAntoni
Copy link
Member

@yan12125,

which version of GCC are you using? As stated above, the nested declarations should be just fine from C standard point of view.
Is this caused by a shortcoming in certain compiler versions?

Cheers,
Jonatan

@yan12125
Copy link

I'm using GCC 10.1. I guess GCC does not like it as I'm compiling C++ code. I remember CMSIS worked fine with one of my another project with only C code.

@yan12125
Copy link

Apologies - it turns out the problem is from dependency management of our projects. The "declared using local type" error appears only with CMSIS 5.6.0. CMSIS 5.7.0 works fine with C++ source files (our code base uses C++ 11, if that is relevant).

As a side note, the key change in CMSIS between 5.6.0 and 5.7.0 appears to be 3b2a0ee#diff-b9d8110b101a50848c56c0c9c90fc936. As cmsis_gcc.h, which is included from cmsis_compiler.h, is protected by the extern "C" block since that commit, GCC no longer complains about local types.

@CheMax-Tag
Copy link

CheMax-Tag commented Mar 10, 2021

Hello, today I meet this error too. I use Segger Embedded Studio with GCC compiler (standarts: c++17, c17).
For fast "bugFix", I add missing "extern C" guard in "cmsis_gcc.h" file.

But patching package files supplied by the IDE developer is bad form. Therefore, I cover all the including of this file with guards.
Maybe you should add guards "extern C" to all header files?

@JonatanAntoni
Copy link
Member

Hi @CheMax-Tag,

The extern C declaration is in all the core_cmX.h files. So including the cmsis_gcc.h file the regular way (through the device header including the required core header) should not cause any issues.

Do you have specific requirements why you'd want to include internal stuff of CMSIS directly from user application?

Cheers,
Jonatan

@CheMax-Tag
Copy link

Hi @JonatanAntoni ,

I include this file to access assembly wrappers, such as __SEV(); __WFE(); __BKPT() and other.

Since the file in which I make the connection is used for different MCUs, I wanted to make a universal solution. Therefore, I connected a "universal" file.

For the test, I tried to include "core_cm4.h", but got another error:
IRQn_Type, defined in "stm32l4r9xx.h" is not visible in "core_cm4.h".

Regards,
Max

@ilg-ul
Copy link
Contributor

ilg-ul commented Mar 10, 2021

I had a similar experience.

Anyway, it is good practice for each header file to include the extern "C" braces.

@JonatanAntoni
Copy link
Member

I agree, if done properly the extern "C" should not harm, either.
May one of you find the time to raise the required changes as a PR? Would be quite helpful.

Thanks,
Jonatan

@CheMax-Tag
Copy link

@JonatanAntoni , Ok. I try do it.

@dzid26
Copy link

dzid26 commented Aug 22, 2021

Hi @CheMax-Tag,

The extern C declaration is in all the core_cmX.h files. So including the cmsis_gcc.h file the regular way (through the device header including the required core header) should not cause any issues.

In my case I am including cmsis_gcc regular way via core_cm4.h, and with gcc the error doesn't show up.

But the project I am using was configured to Clang 12 and I was getting following error:

In file included from ../../board/inc/stm32f4xx.h:189:
In file included from ../../board/inc/stm32f413xx.h:197:
In file included from ../../board/inc/core_cm4.h:162:
In file included from ../../board/inc/cmsis_compiler.h:54:
../../board/inc/cmsis_gcc.h:135:17: error: unsupported: anonymous type given name for linkage purposes by typedef declaration after its linkage was computed; add a tag name here to establish linkage prior to definition
  typedef struct {
                ^
../../board/inc/cmsis_gcc.h:139:5: note: type is given name '__copy_table_t' for linkage purposes by this typedef declaration
  } __copy_table_t;
    ^
../../board/inc/cmsis_gcc.h:141:17: error: unsupported: anonymous type given name for linkage purposes by typedef declaration after its linkage was computed; add a tag name here to establish linkage prior to definition
  typedef struct {
                ^
../../board/inc/cmsis_gcc.h:144:5: note: type is given name '__zero_table_t' for linkage purposes by this typedef declaration
  } __zero_table_t;
    ^

Should Clang use a different header?

Interestingly this ,global define fixes it even for Clang.

@JonatanAntoni
Copy link
Member

Hi @dzid26,

I can only speculate because using vanilla Clang is currently not in scope.

The "problem" might be the declaration of extern variables with non-external types. Strictly speaking no extern module would ever be able to define a variable with that function-local type.

In this specific case the extern variables are linker defined trough the used linker script. The type's memory layout is respected without actually using the type delaration from the c module, of course.

The compiler cannot know this. By moving the type declaration to global scope the warning disappears. A global type could be used from an external module to define the variable. If it is actually used doesn't matter, here.

The reason for having the type declaration local to the function is to prevent cluttering global scope. In fact, this type should not be required/used in any other place.

Can you try to modify the code as proposed by the error message, please? I.e.

[..] add a tag name here to establish linkage [..]

so the code should become something like

  typedef struct copy_table_t {
    uint32_t const* src;
    uint32_t* dest;
    uint32_t  wlen;
  } __copy_table_t;

Does this solve the issue for you? If yes, would you mind to raise this change as a pull-request?`

Thanks,
Jonatan

@puzrin
Copy link

puzrin commented Oct 20, 2021

In .cpp file, if i include like this:

extern "C" {
    #include "arm_math.h"
}

them errors disappear.

@syousufimam
Copy link

syousufimam commented Feb 17, 2022

Hi @JonatanAntoni , I got the similar error while including the "arm_math.h" in *.cpp file, the gcc error is:

const __cmsis_start()::__copy_table_t __copy_table_start__', declared using local type 'const __cmsis_start()::__copy_table_t', is used but never defined [-fpermissive]
   extern const __copy_table_t __copy_table_start__;

The flag -fpermissive could convert the error to warning, but this is not a good solution for our customers. What is your suggestions for this? Is there a plan to update for this?

By the way, when I changed the cmsis_gcc.h, moving

  extern void _start(void) __NO_RETURN;

  typedef struct {
    uint32_t const* src;
    uint32_t* dest;
    uint32_t  wlen;
  } __copy_table_t;

  typedef struct {
    uint32_t* dest;
    uint32_t  wlen;
  } __zero_table_t;

  extern const __copy_table_t __copy_table_start__;
  extern const __copy_table_t __copy_table_end__;
  extern const __zero_table_t __zero_table_start__;
  extern const __zero_table_t __zero_table_end__;

out of the function __cmsis_start, then the error disappears.

Hi @zejiang0jason

Thanks for your suggestions. Finally it worked.

 extern void _start(void) __NO_RETURN;
  
  typedef struct {
    uint32_t const* src;
    uint32_t* dest;
    uint32_t  wlen;
  } __copy_table_t;
  
  typedef struct {
    uint32_t* dest;
    uint32_t  wlen;
  } __zero_table_t;
  
  extern const __copy_table_t __copy_table_start__;
  extern const __copy_table_t __copy_table_end__;
  extern const __zero_table_t __zero_table_start__;
  extern const __zero_table_t __zero_table_end__;

__STATIC_FORCEINLINE __NO_RETURN void __cmsis_start(void)
{
 
  for (__copy_table_t const* pTable = &__copy_table_start__; pTable < &__copy_table_end__; ++pTable) {
    for(uint32_t i=0u; i<pTable->wlen; ++i) {
      pTable->dest[i] = pTable->src[i];
    }
  }
 
  for (__zero_table_t const* pTable = &__zero_table_start__; pTable < &__zero_table_end__; ++pTable) {
    for(uint32_t i=0u; i<pTable->wlen; ++i) {
      pTable->dest[i] = 0u;
    }
  }
 
  _start();
}

@Cromagnon31
Copy link

Hi,

I encounter same issue. The main reason If I include cmsis_compiler.h directly it's to access cortex_m macros/types/inlines for sources common to different cores (m0, m0+, m3, m4...).
There is an other ways to correct the issue. Simply encapsulate this code with an extern "C", since this code is only workable in "C".
Extern "C" statement exists for that case.

But I also think that encapsulating #include files with extern "C" statement like in core_cm?.h files is definitely not a good idea and may have unexpected effects for C++ development. We are at the age where Linux kernel is migrating to C++ !

Olivier

ya-mouse added a commit to ya-mouse/CMSIS_5 that referenced this issue Dec 17, 2022
ya-mouse added a commit to ya-mouse/CMSIS_5 that referenced this issue Dec 17, 2022
ya-mouse added a commit to ya-mouse/CMSIS_5 that referenced this issue Dec 17, 2022
ya-mouse added a commit to ya-mouse/CMSIS_5 that referenced this issue Dec 19, 2022
ya-mouse added a commit to ya-mouse/CMSIS_5 that referenced this issue Dec 19, 2022
ya-mouse added a commit to ya-mouse/CMSIS_5 that referenced this issue Dec 20, 2022
JiafeiPan pushed a commit to nxp-mcuxpresso/CMSIS_5 that referenced this issue Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests