Skip to content

arch: arm: include: cortex_m: tz: Patch cmse_nsfptr_create for gcc 8 #19182

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

Closed

Conversation

sigvartmh
Copy link
Collaborator

arm-none-eabi-gcc 8 has a bug in the cmse_nsfptr_create macro. This
patch undefs the symbol and patches it with the corrected version which
is present in version 7.2 and 9.2.

Signed-off-by: Sigvart Hovland [email protected]

arm-none-eabi-gcc 8 has a bug in the `cmse_nsfptr_create` macro. This
patch undefs the symbol and patches it with the corrected version which
is present in version 7.2 and 9.2.

Signed-off-by: Sigvart Hovland <[email protected]>
@zephyrbot zephyrbot added the area: ARM ARM (32-bit) Architecture label Sep 16, 2019
@sigvartmh
Copy link
Collaborator Author

When trying to use cmse_nsfptr_create the following error is given by the compiler

zephyr/arch/arm/include/cortex_m/tz.h:319:22: error: called object is not a function or function pointer
  ((tz_ns_func_ptr_t)(cmse_nsfptr_create(fptr)))
                      ^~~~~~~~~~~~~~~~~~
<external subsys>/non_secure_jump.c:429:13 note: in expansion of macro 'TZ_NONSECURE_FUNC_PTR_CREATE'
  reset_ns = TZ_NONSECURE_FUNC_PTR_CREATE(vtor_ns[1]);

This is due to the header of arm-none-eabi-gcc-8/arm_cmse.h defines the macro as #define cmse_nsfptr_create(p) ((typeof ((p))) ((intptr_t) (p) & ~1)) while in previous and newer version it is defined as #define cmse_nsfptr_create(p) ((__typeof__ ((p))) ((__INTPTR_TYPE__) (p) & ~1))

The actual root cause of the bug is typeofvs __typeof__ when the macro is expanded.

@ioannisg
Copy link
Member

Thanks for the patch @sigvartmh .
So this made me think that we actually removed the typeof re-definitions, some time ago, here:
01a71ea#diff-56d470cd9436c3647aa88791690483f3

Since our SDK makes it work, I am not sure we want to have these work-around
But I would ask @galak to comment here.

@ioannisg
Copy link
Member

#16199

@sigvartmh
Copy link
Collaborator Author

It's only related to 8.0-8.2 gcc so I guess just stating it in the docs would be sufficient.

@sigvartmh sigvartmh closed this Sep 16, 2019
@galak
Copy link
Collaborator

galak commented Sep 16, 2019

It's only related to 8.0-8.2 gcc so I guess just stating it in the docs would be sufficient.

Curious which GCC you might have hit this with?

Its probably not a terrible "fix" to have for toolchains out of our control.

@sigvartmh
Copy link
Collaborator Author

I hit it with gcc 8.2 didn't realize 8.3 was released in july so I was a bit slow to update it seems. Used 7.2 as a workaround.

@ioannisg
Copy link
Member

It's only related to 8.0-8.2 gcc so I guess just stating it in the docs would be sufficient.

Curious which GCC you might have hit this with?

Its probably not a terrible "fix" to have for toolchains out of our control.

Problem is, if we open this possibility for fixes, just to support existing bugs, we might end up with a lot of noise in the codebase @galak

@SebastianBoe
Copy link
Collaborator

This affects the latest released GNU ARM Embedded toolchain.

@SebastianBoe
Copy link
Collaborator

Not sure where the problem is.

We add workarounds to bugs that affect systems outside of our control all the time.

@SebastianBoe SebastianBoe reopened this Sep 16, 2019
@SebastianBoe
Copy link
Collaborator

Could we revert #16199 ?

That seems to me like a more general fix.

@SebastianBoe
Copy link
Collaborator

We will revert #16199 instead.

@sigvartmh
Copy link
Collaborator Author

My fault had the patch in my zephyr branch when i tried 8.3 yeah still broken.

@SebastianBoe
Copy link
Collaborator

So 8.0-8.3 broken, 7.x and 9.x OK?

@sigvartmh
Copy link
Collaborator Author

That's my experience at least

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants