Hi Masahiro,
On 15 December 2014 at 18:47, Masahiro YAMADA yamada.m@jp.panasonic.com wrote:
Simon,
2014-12-02 5:06 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 26 November 2014 at 00:45, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Tom, Simon, other developers,
Since commit 0d296cc2d3b8 (Provide option to avoid defining a custom version of uintptr_t) and commit 4166ecb247a1 (Add some standard headers external code might need), I have been wondering if they were right decisions.
As arch/arm/include/asm/types.h of Linux Kernel says, using 'stdint.h' is not feasible.
-------------------------------------->8------------------------------------- /*
- The C99 types uintXX_t that are usually defined in 'stdint.h' are not as
- unambiguous on ARM as you would expect. For the types below, there is a
- difference on ARM between GCC built for bare metal ARM, GCC built for glibc
- and the kernel itself, which results in build errors if you try to build with
- -ffreestanding and include 'stdint.h' (such as when you include 'arm_neon.h'
- in order to use NEON intrinsics)
- As the typedefs for these types in 'stdint.h' are based on builtin defines
- supplied by GCC, we can tweak these to align with the kernel's idea of those
- types, so 'linux/types.h' and 'stdint.h' can be safely included from the same
- source file (provided that -ffreestanding is used).
int32_t uint32_t uintptr_t- bare metal GCC long unsigned long unsigned int
- glibc GCC int unsigned int unsigned int
- kernel int unsigned int unsigned long
*/ --------------------------------------8<----------------------------------------
To me this doesn't matter. I feel that int32_t should probably be int on 32-bit ARM, but actually so long as it is consistent then it is fine if it is long. In fact so long as these types are defined consistently, everything works.
Gabe Black and you broke the consistency. See my explanation below.
Actually, the kernel never includes <stdint.h> except host programs.
Commit 0d296cc2d3b8 introduced "USE_STDINT", but it causes type conflicts depending on which GCC is used.
With ARM bare metal GCC,
yamada@beagle:~/workspace/u-boot$ make omap3_beagle_defconfig all CROSS_COMPILE=arm-none-eabi- USE_STDINT=1 # # configuration written to .config # # # configuration written to spl/.config # scripts/kconfig/conf --silentoldconfig Kconfig scripts/kconfig/conf --silentoldconfig Kconfig CHK include/config.h GEN include/autoconf.mk GEN include/autoconf.mk.dep GEN spl/include/autoconf.mk CHK include/config/uboot.release CHK include/generated/version_autogenerated.h CHK include/generated/timestamp_autogenerated.h UPD include/generated/timestamp_autogenerated.h CC lib/asm-offsets.s In file included from /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint.h:5:0, from include/compiler.h:117, from include/image.h:19, from include/common.h:85, from lib/asm-offsets.c:15: /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint-gcc.h:40:24: error: conflicting types for 'int32_t' include/linux/types.h:99:17: note: previous declaration of 'int32_t' was here /opt/arm-2011.03/bin/../lib/gcc/arm-none-eabi/4.5.2/include/stdint-gcc.h:52:25: error: conflicting types for 'uint32_t' include/linux/types.h:105:17: note: previous declaration of 'uint32_t' was here make[2]: *** [lib/asm-offsets.s] Error 1 make[1]: *** [prepare0] Error 2 make: *** [__build_one_by_one] Error 2
While toolchain is this? I don't see this problem - it seems broken.
Not broken at all. I downloaded it from Mentor Graphics http://www.mentor.com/embedded-software/codesourcery
This is a bare-metal compiler. If you do not understand, you should read arch/arm/include/asm/types.h of Linux Kernel once again.
OK, so perhaps for that compiler you cannot use USE_STDINT=1? What does it define for the conflicting type?
Until commit 0d296cc2d, U-Boot has used the hard-coded defines like Linux.
Well it still does, only that it also now has the *option* of using stdint.h.
In my understaing, we should only use ILP32 and LP64 compilers.
short int long longlong pointerILP32 (32bit system) 16 32 32 64 32 LP64 (64bit Unix-like system) 16 32 64 64 64
Whether it is 32bit or 64bit system, we can hard-code
u32/uint32_t as unsigned int u64/uint64_t as unsigned long long uintptr_t as unsigned long
We do not need to refer to compiler-provided headers.
Moreover, we __should not__ refer to compiler-provided <stdint.h>.
Including <stdint.h> means that we use uint32_t defined by the compiler. It is "unsigned int" on some compilers, and "unsigned long" on some compilers.
I don't seem to have that problem, or at least I have not noticed it with printf().
We still have the hard-coded uint32_t define in <linux/types.h> This causes the type conflict error I showed above.
OK, we can fix it, but horrible problems still remain.
If we depends on <stdint.h>, we do not know if uint32_t is defined as "unsigned int" or "unsigned long".
To print out 32bit variables, we would always have to use PRId32. Do you want to modify all the printf() printing 32bit variables just to make them unreadable?
Ick, I have not seen this. Can you given an example of where this happens?
OK, we can fix "int32_t" and "uint32_t", but it still seems strange to see that "uint32_t" is defined as "unsigned long", whereas "u32" is defined as "unsigned int".
If so, must we fix "u32", "s32", ... all the fixed-width typedefs ?
I notice including <linux/types.h> in the U-Boot source tree and <stdint.h> provided by your compiler at the same time is a nightmare.
If we lean toward <stdint.h>, we must ban <linux/types.h>, but <stdint.h> is not available all the time. For example, kernel.org tool-chains do not provide <stdint.h>.
Maybe we can drastically re-write <linux/types.h> and friends to resolve the type conflicts, but I do not think we should not drift apart from the kernel because we have borrowed many source files from Linux.
So far I don't see a big problem.
Horrible problem!
I feel that, were stdint.h available earlier, then types.h might not have been written and we would just use stdint.h. Presumably stdint.h has been created to fix these sorts of problems, and in fact for new projects, they would not define their own types.
What is missing is how to pass the variable width nicely to printf()/scanf().
printf("foo =" PRId32 "\n", foo_32); printf("bar =" PRId64 "\n", bar_64);
This is ridiculous. Extremely unreadable.
I want something like this: printf("foo = %32x \n", foo_32); printf("bar =" %64x \n", bar_64);
If this had been provided, Linux and U-Boot might have adopted <stdint.h>, but in fact, PRI* is an awful workaround.
Agreed it's not very readable but so long as it only applies to 64-bit values and only to printf() it doesn't see bad to me. Provided we resolve what you have brought up above.
IMO in time the kernel and U-Boot might move to stdint.h, and having it as an option at the moment helps us understand the issues. It does not break any builds.
I double it. Using <stdint.h> is a misjudge. It will mess up printf() everywhere.
Linux Kernel has guideline how to print each variable type. Documentation/printk-formats.txt
I could be wrong though, time will tell. We should keep an eye on it.
If we keep wrong code in the code base, someone might continue development base on it, which is also wrong.
In order not to lose our time, wrong code should be immediately removed.
How do you explain stdint.h? So many projects use it - it is now part of the C99 standard. Has the world gone mad?
Regards, Simon