In a ARM multicore system, write_secondary_boot() only initializes fixups for
FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, while smpboot[] also uses FIXUP_DSB.
This results in write_bootloader() using uninitialized fixupcontext[FIXUP_DSB]
instruction in the bootloader code...
Zero-initialize fixupcontext[] to avoid this issue.

hw/arm/boot.c:157:18: warning: Assigned value is garbage or undefined
            insn = fixupcontext[fixup];
                 ^ ~~~~~~~~~~~~~~~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index c2720c8046..fb21f113c2 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -170,7 +170,7 @@ static void write_bootloader(const char *name, hwaddr addr,
 static void default_write_secondary(ARMCPU *cpu,
                                     const struct arm_boot_info *info)
 {
-    uint32_t fixupcontext[FIXUP_MAX];
+    uint32_t fixupcontext[FIXUP_MAX] = {};
 
     fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
     fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
-- 
2.13.3

On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> In a ARM multicore system, write_secondary_boot() only initializes fixups for
> FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, while smpboot[] also uses FIXUP_DSB.
> This results in write_bootloader() using uninitialized fixupcontext[FIXUP_DSB]
> instruction in the bootloader code...

Hmm? The code does:

    if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
        fixupcontext[FIXUP_DSB] = DSB_INSN;
    } else {
        fixupcontext[FIXUP_DSB] = CP15_DSB_INSN;
    }

so fixupcontext[FIXUP_DSB] is guaranteed initialized,
as are FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, which are
the only fixups that the smpboot[] code uses.

thanks
-- PMM

On 07/24/2017 06:06 PM, Peter Maydell wrote:
> On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> In a ARM multicore system, write_secondary_boot() only initializes fixups for
>> FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, while smpboot[] also uses FIXUP_DSB.
>> This results in write_bootloader() using uninitialized fixupcontext[FIXUP_DSB]
>> instruction in the bootloader code...
> 
> Hmm? The code does:
> 
>      if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
>          fixupcontext[FIXUP_DSB] = DSB_INSN;
>      } else {
>          fixupcontext[FIXUP_DSB] = CP15_DSB_INSN;
>      }
> 
> so fixupcontext[FIXUP_DSB] is guaranteed initialized,
> as are FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, which are
> the only fixups that the smpboot[] code uses.

Indeed :)

Sorry for the noise, I'll add few hints to the analyzer.