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

OSMemCreateAssumes 32bit pointers #7

Closed
heyneman opened this issue Mar 4, 2021 · 4 comments
Closed

OSMemCreateAssumes 32bit pointers #7

heyneman opened this issue Mar 4, 2021 · 4 comments
Assignees
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@heyneman
Copy link

heyneman commented Mar 4, 2021

In the call to OSMemCreate(), if arg checking is enabled, the pointer to the start of the partition is cast to an integer in order to verify its aligned to store pointers.

When I compile this on a 64bit system I get compiler warnings about the cast to a smaller type. uC-CPU defines an OS_ADDR type, which would be ideal to use here, but uC-OS2 isn't required to use it. My workaround so far has been to add a similar typedef in os_cpu.h (along with the others like INT32U currently used in the cast).

To reproduce; compile os_mem.c on Ubuntu 18.04 (64bit) using gcc 7.5.0.

if (((INT32U)addr & (sizeof(void *) - 1u)) != 0u){ /* Must be pointer size aligned */

@wes-jmagasrevy wes-jmagasrevy added the bug Something isn't working label Apr 27, 2021
@forg0ne forg0ne added the wontfix This will not be worked on label May 4, 2021
@forg0ne
Copy link
Member

forg0ne commented May 4, 2021

Thank you for reporting this.
Unfortunately, there is no easy way for us to resolve it.
We are reluctant to include a dependency on uC-CPU for legacy support reasons.
Neither can we use INT64U because it is only defined for a couple of ports.

Because this is an edge-case, the best approach in our opinion is to document it as an errata with a workaround for those who encounter it.

@forg0ne
Copy link
Member

forg0ne commented May 4, 2021

Errata for uCOS-III can be found here.

@forg0ne forg0ne closed this as completed May 4, 2021
@heyneman
Copy link
Author

@forg0ne Just for my understanding, why would adding a typedef <...> OS_ADDR line in os_cpu.h (per port) not be an easy way to resolve it?

Then OSMemCreate() can use that type internally rather than assuming 32b.

@forg0ne
Copy link
Member

forg0ne commented May 17, 2021

@heyneman There are a couple of issues that hold us back from adding this change:

  1. The sheer number of ports makes it infeasible to test the change for each port.
  2. We would be changing a function signature for a product that has a long history of use in safety-critical applications. This is something we would prefer to avoid if possible due to the impact it may have on certification.

The vast majority of architectures we target are 32-bit. For the exceptions, such as ARM64, the workaround can be applied on a case-by-case basis by the end-user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants