Incompatible (u)intptr_t type and PRIxPTR definitions

[stephanosio]

Describe the bug
For some platforms, the intptr_t and uintptr_t types do not match the PRIxPTR types.

To Reproduce
On an x86-64 host, build the following code for native_posix board using LLVM (ZEPHYR_TOOLCHAIN_VARIANT=llvm).

uintptr_t val = 1234;
printk("%" PRIuPTR "\n", val);

See the following error:

error: format specifies type 'unsigned int' but the argument has type 'uintptr_t' (aka 'unsigned long') [-Werror,-Wformat]

Expected behavior
The types of intptr_t and uintptr_t and the PRIxPTR match.

For instance:

• If intptr_t is int, PRIdPTR should be %d.
• If intptr_t is long, PRIdPTR should be %ld.

Impact
Makes PRIxPTR useless on the affected platforms (in particular, native_posix on x86-64 with Clang/LLVM).

Environment (please complete the following information):

• OS: Ubuntu 20.04 x86-64
• Toolchain: Clang 10.0 (also tested on Clang 12 and the problem persists)
• Commit SHA: feb0e9f

Additional context
This happens because zephyr_stdint.h re-defines __INTPTR_TYPE__ and __UINTPTR_TYPE__ as long int and long unsigned int, respectively, while the PRIxPTR definitions stay %x with no size prefix.

zephyr/include/toolchain/zephyr_stdint.h

Lines 47 to 50 in feb0e9f

	#undef __INTPTR_TYPE__
	#undef __UINTPTR_TYPE__
	#define __INTPTR_TYPE__ long int
	#define __UINTPTR_TYPE__ long unsigned int

See #37712 (comment).
See also #16645.

[stephanosio]

TODO: Further investigate all the affected platforms/toolchains and make a list of them.
NOTE: Also improve test coverage so that we do not end up with things like this ...

p.s. This should really be fixed before we release LTS.

[stephanosio]

cc @npitre @andyross @pfalcon @galak @nashif @carlescufi @mnkp @pabigot @dcpleung

[npitre]

LLVM is wrong. On a 64-bits platform, a pointer is wider than an unsigned int.

In lib/libc/minimal/include/inttypes.h we have:

#define PRIuPTR                 "lu"

What lib are you using?

[stephanosio]

LLVM is wrong. On a 64-bits platform, a pointer is wider than an unsigned int.

@npitre Building native_posix is with -m32, so pointer should be/is by default int.

For native_posix_64 (which builds with -m64), it will be long.

What lib are you using?

In case of native_posix, it builds with the host Clang (with -m32).

[stephanosio]

This might give better a overview of the issue seen on native_posix with LLVM:
https://gist.github.com/stephanosio/01d70f08046dc979ff7c1de5aab9ea4e

p.s. It looks like, in case of native_posix with host GCC, it does not use the __INTPTR_TYPE__ definition and just typedefs intptr_t to int.

[npitre]

Building native_posix is with -m32, so pointer should be/is by default int.
For native_posix_64 (which builds with -m64), it will be long.

A while ago, we had many problems like that. Commit f32330b fixed
most of them. Zephyr is a young hosting environment so it is in a position
to dictate its types and avoids such legacy shortcomings.

So int32_t is always an int, intptr_t is always a long, and PRIuPTR is
always "lu", for all supported architectures.

Now the native_posix target is a special cases as it runs inside another
environment. Still, the native_posix environment should be isolated as
much as possible so to preserve the Zephyr conventions for Zephyr specific
code.

[stephanosio]

The scope of this issue might be limited to the POSIX arch boards, but I somehow doubt that because we have other third-party toolchains like Arm Compiler and ARC MWDT with their own headers (by the way, cc @tejlmand @abrodkin @evgeniy-paltsev).

I don't have time to do an in-depth analysis on this at the moment. I will revisit this later and do full analysis.

[de-nordic]

There is also problem of definitions like __UINTPTR_MAX__ that do not get override.
The zephyr/include/toolchain/zephyr_stdint.h actually overrides all types by #undef and #define but does nothing to do the same with *_MAX *_MIN definitions, so you may end up with having those mismatched with type.

[pfalcon]

Now the native_posix target is a special cases as it runs inside another
environment. Still, the native_posix environment should be isolated as
much as possible so to preserve the Zephyr conventions for Zephyr specific
code.

I definitely second the sentiment that native_posix breaks a lot the fun. I had a lot of issues with it while working on POSIX subsys: #13054 . And I also eyeballed "radical" solutions like saying that POSIX subsys simply isn't supported on native_posix. Of course, I never went forward with that and exactly tried to "isolate" native_posix stuff from Zephyr's POSIX, but that was quite a swamp (and never finished). Of course, here it's much more focused matter, and maybe it will be possible to make it reach the sweet spot. But given concerns of supporting additional toolchain types raised above, maybe it would just make sense to (re)embrace the fact that these definitions belong to a toolchain, not Zephyr...

(Just thinking aloud really, my 2 cents.)

[npitre]

I think it really depends on how important the native_posix target is, and how much it is worth affecting all other targets to accommodate it. Having a coherent set of definitions for all (real) target is a big plus. And this particular issue is valid only for native_posix (the 32-bit one). There are no such conflicts with native_posix_64.

[stephanosio]

TODO: Check how sane the type system is in armclang with both STDLIB and MICROLIB (cc @tejlmand).

[stephanosio]

For 2.7, we will leave the Arm Compiler and ARC MWDT out of question since they are very much experimental right now.

The scope of fix for 2.7 will be limited to the native_posix with Clang as identified in this issue.

In the future, we could look into reverting #16645:

1. stdint.h: streamline type definitions #16645 should not have been necessary if we actually made use of the PRIx and SCNx definitions from inttypes.h in the code -- we have no reason not to since we are effectively on C99 and there is no point in supporting both ancient and highly obsolete C standards that predate C99.
2. Strange targets such as baremetal x86-32 that define INT32_TYPE as long int can be reworked to use int for INT32_TYPE, once we have the infrastructure to do that -- Add Zephyr-specific target for toolchain components sdk-ng#350 will add Zephyr OS native targets for the GNU toolchains (i.e. aarch64-*-zephyr instead of aarch64-*-elf), so we can easily do that.
3. stdint.h: streamline type definitions #16645 is really an ugly hack than a proper solution (though, I do note that it may have been inevitable at the time).