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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.07.2022 12:13, 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. So in
> this patch, we add "-fno-short-wchar" to override "-fshort-wchar"
> for Arm32 to remove above warning.
> 
> Reported-and-Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>

Interesting new tag (but why not).

> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> ---
>  xen/arch/arm/efi/Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
> index dffe72e589..b06fb96c1f 100644
> --- a/xen/arch/arm/efi/Makefile
> +++ b/xen/arch/arm/efi/Makefile
> @@ -1,5 +1,9 @@
>  include $(srctree)/common/efi/efi-common.mk
>  
> +ifeq ($(CONFIG_ARM_32),y)
> +CFLAGS-y += -fno-short-wchar
> +endif

Simply

CFLAGS-$(CONFIG_ARM_32) += -fno-short-wchar

? But, as suggested, perhaps further

$(obj)/stub.o: CFLAGS-$(CONFIG_ARM_32) += -fno-short-wchar

to make sure we'd notice any other uses / issues here. After all it
is - at least in theory - possible that Arm32 would also gain EFI
support, and then it would be a problem if the other ("real") files
were compiled that way (albeit I think the issue would be easily
noticeable, as I don't think things would build that way).

Jan