Re: [TF-A] ATF currently does not use proper printf format specifiers for fixed width types
Perhaps we ought to look at doing away with maintaining our own version of freestanding headers like stdint.h in the first place – they’re part of the freestanding portion of the C standard library for good reason (the implementations necessarily come directly from the compiler), and reimplementing them is really prone to portability errors like this (and can frequently confuse static analysers). If we are to continue using it, we should at least look into replacing the definitions with the builtin values provided by the compilers we use, e.g. typedef __UINT64_TYPE__ uint64_t;.
Using inttypes.h is the traditional wisdom for this particular specifier issue – `ll` for `long long`, `PRIu64` for `uint64_t. While it’s not particularly pleasant to read/write, it was the solution that the C standards committee came up with, so I approve of the principle of this change but I think a permanent solution would serve us better in the long run.
Chris
From: TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Joanna Farley via TF-A tf-a@lists.trustedfirmware.org Date: Wednesday, 3 March 2021 at 15:43 To: Joanna Farley via TF-A tf-a@lists.trustedfirmware.org Cc: Scott Branden scott.branden@broadcom.com Subject: [TF-A] ATF currently does not use proper printf format specifiers for fixed width types Hi All,
Back in September Scott posted a query to the group related to a patch he has created relating to printf format specifiers and some of the maintainers from Arm have reservations about and we asked him to get opinions from the broader project community as his patch changes a number of different platforms as well as core code.
I’m trying to help him reinvigorate the discussion so reposting his request with patch link below that had stalled.
Joanna
________________________________
Scott Branden scott.branden at broadcom.com mailto:tf-a%40lists.trustedfirmware.org?Subject=Re%3A%20%5BTF-A%5D%20ATF%20currently%20does%20not%20use%20proper%20printf%20format%20specifiers%0A%20for%20fixed%20width%20types&In-Reply-To=%3Cbd3b49f4-e8f9-9016-d11b-d08b81e6b43d%40broadcom.com%3E Mon Sep 14 18:34:45 UTC 2020
Hello,
ATF currently uses non-portable printf format specifiers for fixed width types defined in stdint.h
In addition, ATF redefines types defined in gcc for stdint.h with its own custom types causing additional issues.
This causes compilation issues when porting code to/from ATF.
AND, generates coverity parse errors as int64_t and uint64_t are incorrectly defined in ATF vs. gcc for aarch64.
The printf format specifiers in inttypes.h are to be used for the proper format specifiers.
And, uint64_t/int64_t should be defined the same as in gcc.
I tried fixing up all the instances of int64 printf format specifiers by introducing inttypes.h and redefined the stdint types correctly here:
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/5437
We have checked the change into our local tree so that everything compiles and runs in our system. Please accept change upstream.
Regards,
Scott
Hi Chris,
On 2021-03-03 8:13 a.m., Chris Kay wrote:
Perhaps we ought to look at doing away with maintaining our own version of freestanding headers like stdint.h in the first place – they’re part of the freestanding portion of the C standard library for good reason (the implementations necessarily come directly from the compiler), and reimplementing them is really prone to portability errors like this (and can frequently confuse static analysers). If we are to continue using it, we should at least look into replacing the definitions with the builtin values provided by the compilers we use, e.g. typedef __UINT64_TYPE__ uint64_t;.
Using inttypes.h is the traditional wisdom for this particular specifier issue – `ll` for `long long`, `PRIu64` for `uint64_t. While it’s not particularly pleasant to read/write, it was the solution that the C standards committee came up with, so I approve of the principle of this change but I think a permanent solution would serve us better in the long run.
Removing the custom stdint.h and inttypes.h could be done as a follow on if that makes sense to ATF. This patch gets it moving in the right direction to do so.
Chris
From: TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Joanna Farley via TF-A tf-a@lists.trustedfirmware.org Date: Wednesday, 3 March 2021 at 15:43 To: Joanna Farley via TF-A tf-a@lists.trustedfirmware.org Cc: Scott Branden scott.branden@broadcom.com Subject: [TF-A] ATF currently does not use proper printf format specifiers for fixed width types Hi All,
Back in September Scott posted a query to the group related to a patch he has created relating to printf format specifiers and some of the maintainers from Arm have reservations about and we asked him to get opinions from the broader project community as his patch changes a number of different platforms as well as core code.
I’m trying to help him reinvigorate the discussion so reposting his request with patch link below that had stalled.
Joanna
Scott Branden scott.branden at broadcom.com mailto:tf-a%40lists.trustedfirmware.org?Subject=Re%3A%20%5BTF-A%5D%20ATF%20currently%20does%20not%20use%20proper%20printf%20format%20specifiers%0A%20for%20fixed%20width%20types&In-Reply-To=%3Cbd3b49f4-e8f9-9016-d11b-d08b81e6b43d%40broadcom.com%3E Mon Sep 14 18:34:45 UTC 2020
Hello,
ATF currently uses non-portable printf format specifiers for fixed width types defined in stdint.h
In addition, ATF redefines types defined in gcc for stdint.h with its own custom types causing additional issues.
This causes compilation issues when porting code to/from ATF.
AND, generates coverity parse errors as int64_t and uint64_t are incorrectly defined in ATF vs. gcc for aarch64.
The printf format specifiers in inttypes.h are to be used for the proper format specifiers.
And, uint64_t/int64_t should be defined the same as in gcc.
I tried fixing up all the instances of int64 printf format specifiers by introducing inttypes.h and redefined the stdint types correctly here:
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/5437
We have checked the change into our local tree so that everything compiles and runs in our system. Please accept change upstream.
Regards,
Scott
Agreed, I think this change is necessary regardless of which direction we choose. I’ll try to find some time to do a proper review of the patchset shortly; I’ve nothing against the principle of it.
Chris
On 03/03/2021, 21:18, "Scott Branden" scott.branden@broadcom.com wrote: Hi Chris,
On 2021-03-03 8:13 a.m., Chris Kay wrote:
Perhaps we ought to look at doing away with maintaining our own version of freestanding headers like stdint.h in the first place � they�re part of the freestanding portion of the C standard library for good reason (the implementations necessarily come directly from the compiler), and reimplementing them is really prone to portability errors like this (and can frequently confuse static analysers). If we are to continue using it, we should at least look into replacing the definitions with the builtin values provided by the compilers we use, e.g. typedef __UINT64_TYPE__ uint64_t;.
Using inttypes.h is the traditional wisdom for this particular specifier issue � `ll` for `long long`, `PRIu64` for `uint64_t. While it�s not particularly pleasant to read/write, it was the solution that the C standards committee came up with, so I approve of the principle of this change but I think a permanent solution would serve us better in the long run.
Removing the custom stdint.h and inttypes.h could be done as a follow on if that makes sense to ATF. This patch gets it moving in the right direction to do so.
Chris
From: TF-A <tf-a-bounces@lists.trustedfirmware.orgmailto:tf-a-bounces@lists.trustedfirmware.org> on behalf of Joanna Farley via TF-A <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Date: Wednesday, 3 March 2021 at 15:43 To: Joanna Farley via TF-A <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Cc: Scott Branden <scott.branden@broadcom.commailto:scott.branden@broadcom.com> Subject: [TF-A] ATF currently does not use proper printf format specifiers for fixed width types Hi All,
Back in September Scott posted a query to the group related to a patch he has created relating to printf format specifiers and some of the maintainers from Arm have reservations about and we asked him to get opinions from the broader project community as his patch changes a number of different platforms as well as core code.
I�m trying to help him reinvigorate the discussion so reposting his request with patch link below that had stalled.
Joanna
Scott Branden scott.branden at broadcom.com mailto:tf-a%40lists.trustedfirmware.org?Subject=Re%3A%20%5BTF-A%5D%20ATF%20currently%20does%20not%20use%20proper%20printf%20format%20specifiers%0A%20for%20fixed%20width%20types&In-Reply-To=%3Cbd3b49f4-e8f9-9016-d11b-d08b81e6b43d%40broadcom.com%3E Mon Sep 14 18:34:45 UTC 2020
Hello,
ATF currently uses non-portable printf format specifiers for fixed width types defined in stdint.h
In addition, ATF redefines types defined in gcc for stdint.h with its own custom types causing additional issues.
This causes compilation issues when porting code to/from ATF.
AND, generates coverity parse errors as int64_t and uint64_t are incorrectly defined in ATF vs. gcc for aarch64.
The printf format specifiers in inttypes.h are to be used for the proper format specifiers.
And, uint64_t/int64_t should be defined the same as in gcc.
I tried fixing up all the instances of int64 printf format specifiers by introducing inttypes.h and redefined the stdint types correctly here:
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/5437
We have checked the change into our local tree so that everything compiles and runs in our system. Please accept change upstream.
Regards,
Scott
tf-a@lists.trustedfirmware.org
-
Chris Kay -
Scott Branden