Re: [PATCH] xen/arm: mark handle_linux_pci_domain() __init

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 10/14/22 04:22, Julien Grall wrote:
> Hi,
>
> On 14/10/2022 08:16, Jan Beulich wrote:
> > On 14.10.2022 04:53, Stewart Hildebrand wrote:
> > > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
> > I guess a non-empty description and a Fixes: tag would be nice.
Okay, I will send a v2 with the following description:
All functions in domain_build.c should be marked __init. This was 
spotted when building the hypervisor with -Og.
Fixes: 1050a7b91c xen/arm: add pci-domain for disabled devices
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>

> +1. I am actually quite interested to understand how this was spotted.
>
> The build system should check that any function/data in domain_build.c
> are part of the __init section. So I guess the compiler you are using
> doesn't inline the function?
>
> If so, I am actually surprised you are the first one spotted this... We
> are building on various distribution without any issues (?). I would be
> interested to know the compiler version and maybe we could add it in the
> CI.
I added -Og to the make command line so it takes precedence over the 
default -O1/-O2:
$ make EXTRA_CFLAGS_XEN_CORE="-Og" XEN_TARGET_ARCH=arm64 
CROSS_COMPILE=aarch64-none-linux-gnu- dist-xen -j $(nproc)
Indeed, I did observe the build error:
Error: size of arch/arm/domain_build.o:.text is 0x00000008

I used this rune to reveal the culprit:

$ aarch64-none-linux-gnu-objdump -d xen/arch/arm/domain_build.o | head

xen/arch/arm/domain_build.o:     file format elf64-littleaarch64


Disassembly of section .text:

0000000000000000 <handle_linux_pci_domain>:
    0:   52800000        mov     w0, #0x0                        // #0
    4:   d65f03c0        ret

I am using this toolchain: 
https://developer.arm.com/-/media/Files/downloads/gnu/11.3.rel1/binrel/arm-gnu-toolchain-11.3.rel1-x86_64-aarch64-none-linux-gnu.tar.xz
Further, there were two more build errors observed when building with -Og:
arch/arm/domain_build.c: In function ‘make_cpus_node’:
arch/arm/domain_build.c:2013:12: error: ‘clock_valid’ may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
 2013 |         if ( clock_valid )
      |            ^

arch/arm/efi/boot.c: In function ‘efi_start’:
arch/arm/efi/boot.c:1464:9: error: ‘argc’ may be used uninitialized in 
this function [-Werror=maybe-uninitialized]
 1464 |         efi_arch_handle_cmdline(argc ? *argv : NULL, options, 
name.s);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I assume these uninitialized use errors can simply be fixed by 
initializing the respective variables to false/0, but a second opinion 
would certainly be helpful.