1. Patchew
  2. Xen
  3. View series

[PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32 building

Wei Chen posted 1 patch 1 year, 9 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20220705035405.1283032-1-wei.chen@arm.com
xen/arch/arm/efi/Makefile | 3 +++
1 file changed, 3 insertions(+)
[PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32 building
Posted by Wei Chen 1 year, 9 months ago 
Xen uses "-fshort-wchar" in CFLAGS for EFI common code. Arm32
is using stub.c of EFI common code for EFI stub functions. But
"-fshort-wchar" CFLAG will cause a warning when build stub.c
for Arm32:
"arm-linux-gnueabihf-ld: warning: arch/arm/efi/built_in.o uses
2-byte wchar_t yet the output is to use 4-byte wchar_t; use of
wchar_t values across objects may fail"

This is because the "-fshort-wchar" flag causes GCC to generate
code that is not binary compatible with code generated without
that flag. Why this warning hasn't been triggered in Arm64 is
because Arm64 does not use wchar type directly in any code for
parameters, variables and return values. And in EFI code, wchar
has been replaced by CHAR16 (the UEFI "abstraction" of wchar_t).
CHAR16 has been specified as unsigned short type in typedef, the
"-fshort-wchar" flag will not affect CHAR16. So Arm64 object
files are exactly the same with "-fshort-wchar" and without
"-fshort-wchar".

We are also not using wchar in Arm32 codes, but Arm32 will embed
ABI information in ".ARM.attributes" section. This section stores
some object file attributes, like ABI version, CPU arch and etc.
And wchar size is described in this section by "Tag_ABI_PCS_wchar_t"
too. Tag_ABI_PCS_wchar_t is 2 for object files with "-fshort-wchar",
but for object files without "-fshort-wchar" is 4. Arm32 GCC
ld will check this tag, and throw above warning when it finds
the object files have different Tag_ABI_PCS_wchar_t values.

As gnu-efi-3.0 use the GCC option "-fshort-wchar" to force wchar
to use short integers (2 bytes) instead of integers (4 bytes).
So keep "-fshort-wchar" for Xen EFI code is reasonable. In this
patch, we add "-fno-short-wchar" to override "-fshort-wchar" for Arm
architecutres without EFI enabled to remove above warning.

Reported-and-Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Wei Chen <wei.chen@arm.com>
---
v1 -> v2:
1. Use CONFIG_ARM_EFI to replace CONFIG_ARM_32 to gate
   "-fno-short-whar".
---
 xen/arch/arm/efi/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
index dffe72e589..bd954a3b2d 100644
--- a/xen/arch/arm/efi/Makefile
+++ b/xen/arch/arm/efi/Makefile
@@ -9,4 +9,7 @@ else
 # will not be cleaned in "make clean".
 EFIOBJ-y += stub.o
 obj-y += stub.o
+
+$(obj)/stub.o: CFLAGS-y += -fno-short-wchar
+
 endif
-- 
2.25.1
Re: [PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32 building
Posted by Julien Grall 1 year, 9 months ago 
Hi Wei,

On 05/07/2022 04:54, Wei Chen wrote:
> Xen uses "-fshort-wchar" in CFLAGS for EFI common code. Arm32
> is using stub.c of EFI common code for EFI stub functions. But
> "-fshort-wchar" CFLAG will cause a warning when build stub.c
> for Arm32:
> "arm-linux-gnueabihf-ld: warning: arch/arm/efi/built_in.o uses
> 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of
> wchar_t values across objects may fail"
> 
> This is because the "-fshort-wchar" flag causes GCC to generate
> code that is not binary compatible with code generated without
> that flag. Why this warning hasn't been triggered in Arm64 is
> because Arm64 does not use wchar type directly in any code for
> parameters, variables and return values. And in EFI code, wchar
> has been replaced by CHAR16 (the UEFI "abstraction" of wchar_t).
> CHAR16 has been specified as unsigned short type in typedef, the
> "-fshort-wchar" flag will not affect CHAR16. So Arm64 object
> files are exactly the same with "-fshort-wchar" and without
> "-fshort-wchar".
> 
> We are also not using wchar in Arm32 codes, but Arm32 will embed
> ABI information in ".ARM.attributes" section. This section stores
> some object file attributes, like ABI version, CPU arch and etc.
> And wchar size is described in this section by "Tag_ABI_PCS_wchar_t"
> too. Tag_ABI_PCS_wchar_t is 2 for object files with "-fshort-wchar",
> but for object files without "-fshort-wchar" is 4. Arm32 GCC
> ld will check this tag, and throw above warning when it finds
> the object files have different Tag_ABI_PCS_wchar_t values.
> 
> As gnu-efi-3.0 use the GCC option "-fshort-wchar" to force wchar
> to use short integers (2 bytes) instead of integers (4 bytes).
> So keep "-fshort-wchar" for Xen EFI code is reasonable. In this
> patch, we add "-fno-short-wchar" to override "-fshort-wchar" for Arm
> architecutres without EFI enabled to remove above warning.
> 
> Reported-and-Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Wei Chen <wei.chen@arm.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall
Re: [PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32 building
Posted by Jan Beulich 1 year, 9 months ago 
On 05.07.2022 05:54, Wei Chen wrote:
> Xen uses "-fshort-wchar" in CFLAGS for EFI common code. Arm32
> is using stub.c of EFI common code for EFI stub functions. But
> "-fshort-wchar" CFLAG will cause a warning when build stub.c
> for Arm32:
> "arm-linux-gnueabihf-ld: warning: arch/arm/efi/built_in.o uses
> 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of
> wchar_t values across objects may fail"
> 
> This is because the "-fshort-wchar" flag causes GCC to generate
> code that is not binary compatible with code generated without
> that flag. Why this warning hasn't been triggered in Arm64 is
> because Arm64 does not use wchar type directly in any code for
> parameters, variables and return values. And in EFI code, wchar
> has been replaced by CHAR16 (the UEFI "abstraction" of wchar_t).
> CHAR16 has been specified as unsigned short type in typedef, the
> "-fshort-wchar" flag will not affect CHAR16. So Arm64 object
> files are exactly the same with "-fshort-wchar" and without
> "-fshort-wchar".
> 
> We are also not using wchar in Arm32 codes, but Arm32 will embed
> ABI information in ".ARM.attributes" section. This section stores
> some object file attributes, like ABI version, CPU arch and etc.
> And wchar size is described in this section by "Tag_ABI_PCS_wchar_t"
> too. Tag_ABI_PCS_wchar_t is 2 for object files with "-fshort-wchar",
> but for object files without "-fshort-wchar" is 4. Arm32 GCC
> ld will check this tag, and throw above warning when it finds
> the object files have different Tag_ABI_PCS_wchar_t values.
> 
> As gnu-efi-3.0 use the GCC option "-fshort-wchar" to force wchar
> to use short integers (2 bytes) instead of integers (4 bytes).
> So keep "-fshort-wchar" for Xen EFI code is reasonable. In this
> patch, we add "-fno-short-wchar" to override "-fshort-wchar" for Arm
> architecutres without EFI enabled to remove above warning.
> 
> Reported-and-Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Wei Chen <wei.chen@arm.com>

Tested-by: Jan Beulich <jbeulich@suse.com>

As to the description: Why the reference to gnu-efi? I don't think
it matters how they build their code? All that's important is what
we do, I suppose.

As to the title: I think the prefix would better be Arm32: (or
alike). Hence how about "Arm32: avoid EFI stub wchar_t size linker
warning"?

Preferably with respective adjustments also:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
RE: [PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32 building
Posted by Wei Chen 1 year, 9 months ago 
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年7月5日 14:35
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>; Volodymyr
> Babchuk <Volodymyr_Babchuk@epam.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32
> building
> 
> On 05.07.2022 05:54, Wei Chen wrote:
> > Xen uses "-fshort-wchar" in CFLAGS for EFI common code. Arm32
> > is using stub.c of EFI common code for EFI stub functions. But
> > "-fshort-wchar" CFLAG will cause a warning when build stub.c
> > for Arm32:
> > "arm-linux-gnueabihf-ld: warning: arch/arm/efi/built_in.o uses
> > 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of
> > wchar_t values across objects may fail"
> >
> > This is because the "-fshort-wchar" flag causes GCC to generate
> > code that is not binary compatible with code generated without
> > that flag. Why this warning hasn't been triggered in Arm64 is
> > because Arm64 does not use wchar type directly in any code for
> > parameters, variables and return values. And in EFI code, wchar
> > has been replaced by CHAR16 (the UEFI "abstraction" of wchar_t).
> > CHAR16 has been specified as unsigned short type in typedef, the
> > "-fshort-wchar" flag will not affect CHAR16. So Arm64 object
> > files are exactly the same with "-fshort-wchar" and without
> > "-fshort-wchar".
> >
> > We are also not using wchar in Arm32 codes, but Arm32 will embed
> > ABI information in ".ARM.attributes" section. This section stores
> > some object file attributes, like ABI version, CPU arch and etc.
> > And wchar size is described in this section by "Tag_ABI_PCS_wchar_t"
> > too. Tag_ABI_PCS_wchar_t is 2 for object files with "-fshort-wchar",
> > but for object files without "-fshort-wchar" is 4. Arm32 GCC
> > ld will check this tag, and throw above warning when it finds
> > the object files have different Tag_ABI_PCS_wchar_t values.
> >
> > As gnu-efi-3.0 use the GCC option "-fshort-wchar" to force wchar
> > to use short integers (2 bytes) instead of integers (4 bytes).
> > So keep "-fshort-wchar" for Xen EFI code is reasonable. In this
> > patch, we add "-fno-short-wchar" to override "-fshort-wchar" for Arm
> > architecutres without EFI enabled to remove above warning.
> >
> > Reported-and-Suggested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Wei Chen <wei.chen@arm.com>
> 
> Tested-by: Jan Beulich <jbeulich@suse.com>
> 

Thanks for testing!

> As to the description: Why the reference to gnu-efi? I don't think
> it matters how they build their code? All that's important is what
> we do, I suppose.
> 

How about:
"Xen need to keep "-fshort-wchar" in EFI code to force wchar to use
short integers (2 bytes) instead of integers (4 bytes), but this is
unnecessary for code out of EFI. So in this patch, we add
"-fno-short-wchar" to override "-fshort-wchar" for Arm architectures
without EFI enabled to remove above warning."


> As to the title: I think the prefix would better be Arm32: (or
> alike). Hence how about "Arm32: avoid EFI stub wchar_t size linker
> warning"?
> 

I would send a new version to adjust the title and above description.

Thanks,
Wei Chen


> Preferably with respective adjustments also:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Jan
Re: [PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32 building
Posted by Jan Beulich 1 year, 9 months ago 
On 05.07.2022 09:18, Wei Chen wrote:
> Hi Jan,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 2022年7月5日 14:35
>> To: Wei Chen <Wei.Chen@arm.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
>> <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>; Volodymyr
>> Babchuk <Volodymyr_Babchuk@epam.com>; xen-devel@lists.xenproject.org
>> Subject: Re: [PATCH v2] Xen: fix EFI stub wchar_t size warning of arm32
>> building
>>
>> On 05.07.2022 05:54, Wei Chen wrote:
>>> Xen uses "-fshort-wchar" in CFLAGS for EFI common code. Arm32
>>> is using stub.c of EFI common code for EFI stub functions. But
>>> "-fshort-wchar" CFLAG will cause a warning when build stub.c
>>> for Arm32:
>>> "arm-linux-gnueabihf-ld: warning: arch/arm/efi/built_in.o uses
>>> 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of
>>> wchar_t values across objects may fail"
>>>
>>> This is because the "-fshort-wchar" flag causes GCC to generate
>>> code that is not binary compatible with code generated without
>>> that flag. Why this warning hasn't been triggered in Arm64 is
>>> because Arm64 does not use wchar type directly in any code for
>>> parameters, variables and return values. And in EFI code, wchar
>>> has been replaced by CHAR16 (the UEFI "abstraction" of wchar_t).
>>> CHAR16 has been specified as unsigned short type in typedef, the
>>> "-fshort-wchar" flag will not affect CHAR16. So Arm64 object
>>> files are exactly the same with "-fshort-wchar" and without
>>> "-fshort-wchar".
>>>
>>> We are also not using wchar in Arm32 codes, but Arm32 will embed
>>> ABI information in ".ARM.attributes" section. This section stores
>>> some object file attributes, like ABI version, CPU arch and etc.
>>> And wchar size is described in this section by "Tag_ABI_PCS_wchar_t"
>>> too. Tag_ABI_PCS_wchar_t is 2 for object files with "-fshort-wchar",
>>> but for object files without "-fshort-wchar" is 4. Arm32 GCC
>>> ld will check this tag, and throw above warning when it finds
>>> the object files have different Tag_ABI_PCS_wchar_t values.
>>>
>>> As gnu-efi-3.0 use the GCC option "-fshort-wchar" to force wchar
>>> to use short integers (2 bytes) instead of integers (4 bytes).
>>> So keep "-fshort-wchar" for Xen EFI code is reasonable. In this
>>> patch, we add "-fno-short-wchar" to override "-fshort-wchar" for Arm
>>> architecutres without EFI enabled to remove above warning.
>>>
>>> Reported-and-Suggested-by: Jan Beulich <jbeulich@suse.com>
>>> Signed-off-by: Wei Chen <wei.chen@arm.com>
>>
>> Tested-by: Jan Beulich <jbeulich@suse.com>
>>
> 
> Thanks for testing!
> 
>> As to the description: Why the reference to gnu-efi? I don't think
>> it matters how they build their code? All that's important is what
>> we do, I suppose.
>>
> 
> How about:
> "Xen need to keep "-fshort-wchar" in EFI code to force wchar to use
> short integers (2 bytes) instead of integers (4 bytes), but this is
> unnecessary for code out of EFI. So in this patch, we add
> "-fno-short-wchar" to override "-fshort-wchar" for Arm architectures
> without EFI enabled to remove above warning."

Reads better to me, thanks.

Jan

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.