Re: [PATCH v2] hw/acpi: fix breakage due to missing aml stub definitions when acpi is off

[Michael S. Tsirkin]

On Mon, Nov 07, 2022 at 06:08:49PM +0100, Philippe Mathieu-Daudé wrote:
> On 7/11/22 16:27, Ani Sinha wrote:
> > Some HW architectures do not support acpi and CONFIG_ACPI is off for them. 
> > For
> > those architectures, dummy stub function definitions help to resolve 
> > symbols.
> > This change adds couple of dummy stub definitions so that symbols for those 
> > can
> > be resolved and failures such as the following can be fixed for or1k 
> > targets.
> > 
> > Configuration:
> > qemu/build $ ../configure --enable-werror --disable-docs --disable-nettle \
> >               --enable-gcrypt --enable-fdt=system --enable-modules \
> >               --enable-trace-backends=dtrace --enable-docs \
> >          --enable-vfio-user-server \
> >               --target-list="ppc64-softmmu or1k-softmmu s390x-softmmu 
> > x86_64-softmmu
> >   rx-softmmu sh4-softmmu nios2-softmmu"
> > 
> > actual failure:
> > 
> > qemu/build $ QTEST_QEMU_BINARY=./qemu-system-or1k  ./tests/qtest/qos-test
> > 
> > failed to open module:
> > /build/qemu/qemu/build/qemu-bundle/usr/local/lib64/qemu/hw-display-virtio-vga.so:
> > undefined symbol: aml_return
> > qemu-system-or1k: ../util/error.c:59: error_setv: Assertion `*errp ==
> > NULL' failed.
> > Broken pipe
> > ../tests/qtest/libqtest.c:188: kill_qemu() detected QEMU death from
> > signal 6 (Aborted) (core dumped)
> > Aborted (core dumped)
> > 
> > CC: Bernhard Beschow <shentey@gmail.com>
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> >   hw/acpi/aml-build-stub.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > changelog:
> > v2: cosmetic commit description format update.
> > 
> > diff --git a/hw/acpi/aml-build-stub.c b/hw/acpi/aml-build-stub.c
> > index 8d8ad1a314..89a8fec4af 100644
> > --- a/hw/acpi/aml-build-stub.c
> > +++ b/hw/acpi/aml-build-stub.c
> > @@ -26,6 +26,16 @@ void aml_append(Aml *parent_ctx, Aml *child)
> >   {
> >   }
> > +Aml *aml_return(Aml *val)
> > +{
> > +    return NULL;
> 
> Can't return NULL, otherwise aml_append() will crash.

This is what rest of functions do.

> We just want the symbol to be defined, so instead:
> 
>       g_assert_not_reached();
> 
> > +}

NULL derefs are actually somewhat easier to debug than asserts.

-- 
MST