Re: QOM crash via soundhw_init()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 12/10/23 18:26, Peng Liang wrote:
> On 10/12/2023 18:53, Philippe Mathieu-Daudé wrote:
>> Hi Martin, Paolo, Markus, Marc-André,
>>
>> With the following changes:
>>
>> -- >8 --
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 137276bcb9..291495f798 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
>> @@ -245,6 +245,7 @@ static void ibm_40p_init(MachineState *machine)
>>       uint32_t kernel_base = 0, initrd_base = 0;
>>       long kernel_size = 0, initrd_size = 0;
>>       char boot_device;
>> +    MemoryRegion rom;
>>
>>       if (kvm_enabled()) {
>>           error_report("machine %s does not support the KVM accelerator",
>> @@ -277,6 +278,9 @@ static void ibm_40p_init(MachineState *machine)
>>           exit(1);
>>       }
>>
>> +    memory_region_init_rom_nomigrate(&rom, OBJECT(machine), "test",
>> +                                     4 * KiB, &error_fatal);
>> +
>>       /* PCI -> ISA bridge */
>>       i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
>>       qdev_connect_gpio_out(i82378_dev, 0,
>>
>> ---
> 
> I think it can be fixed by changing the type of rom to MemoryRegion*, such as:
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 137276bcb9..b5c2345ec8 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -245,6 +245,7 @@ static void ibm_40p_init(MachineState *machine)
>       uint32_t kernel_base = 0, initrd_base = 0;
>       long kernel_size = 0, initrd_size = 0;
>       char boot_device;
> +    MemoryRegion *rom = g_new0(MemoryRegion, 1);
> 
>       if (kvm_enabled()) {
>           error_report("machine %s does not support the KVM accelerator",
> @@ -277,6 +278,9 @@ static void ibm_40p_init(MachineState *machine)
>           exit(1);
>       }
> 
> +    memory_region_init_rom_nomigrate(rom, OBJECT(machine), "test", 4 * KiB,
> +                                     &error_fatal);
> +
>       /* PCI -> ISA bridge */
>       i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
>       qdev_connect_gpio_out(i82378_dev, 0,
> ---
> 
> In the original patch, rom is an object on stack and machine will save a reference
> to rom in its properties after memory_region_init_rom_nomigrate. When the function
> returns, the stack frame is freed and the data in rom becomes to garbage. After that,
> when we call object_resolve_path_type, the properties of machine will be used to
> match the specific path and type, then we will use some garbage in rom (which is on
> stack).

YES! Stupid mistake... Thank you Peng, you saved my day :)