ninjatool: quote dollars in variables

[PATCH] ninjatool: quote dollars in variables

Posted by Paolo Bonzini 3 years, 8 months ago

Otherwise, dollars (such as in the special $ORIGIN rpath) are
eaten by Make.

Reported-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/ninjatool.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index cc77d51aa8..c33eafb5a0 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -834,7 +834,8 @@ class Ninja2Make(NinjaParserEventsWithVars):
         self.print()
         for targets in self.build_vars:
             for name, value in self.build_vars[targets].items():
-                self.print('%s: private .var.%s := %s' % (targets, name, value))
+                self.print('%s: private .var.%s := %s' %
+                           (targets, name, value.replace('$', '$$')))
             self.print()
         if not self.seen_default:
             default_targets = sorted(self.all_outs - self.all_ins, key=natural_sort_key)
-- 
2.26.2

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Peter Maydell 3 years, 8 months ago

On Wed, 26 Aug 2020 at 20:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Otherwise, dollars (such as in the special $ORIGIN rpath) are
> eaten by Make.

Incidentally, why are we using rpath anyway? I'm pretty
sure the old build system didn't need it, and it's one of
those features I have mentally filed away under "liable
to confusing and non-portable weirdness"...

thanks
-- PMM

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Paolo Bonzini 3 years, 8 months ago

Il mer 26 ago 2020, 21:34 Peter Maydell <peter.maydell@linaro.org> ha
scritto:

> On Wed, 26 Aug 2020 at 20:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > Otherwise, dollars (such as in the special $ORIGIN rpath) are
> > eaten by Make.
>
> Incidentally, why are we using rpath anyway? I'm pretty
> sure the old build system didn't need it, and it's one of
> those features I have mentally filed away under "liable
> to confusing and non-portable weirdness"...
>

It's only done in the build tree, to allow running against uninstalled
shared_library. Installed binaries have no rpath (distros don't want it
anyway). QEMU doesn't need it since it has no shared library yet.

Paolo

>

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Alexander Bulekov 3 years, 8 months ago

On 200827 0613, Paolo Bonzini wrote:
> Il mer 26 ago 2020, 21:34 Peter Maydell <peter.maydell@linaro.org> ha
> scritto:
> 
> > On Wed, 26 Aug 2020 at 20:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >
> > > Otherwise, dollars (such as in the special $ORIGIN rpath) are
> > > eaten by Make.
> >
> > Incidentally, why are we using rpath anyway? I'm pretty
> > sure the old build system didn't need it, and it's one of
> > those features I have mentally filed away under "liable
> > to confusing and non-portable weirdness"...
> >
> 
> It's only done in the build tree, to allow running against uninstalled
> shared_library. Installed binaries have no rpath (distros don't want it
> anyway). QEMU doesn't need it since it has no shared library yet.
> 

Its an obscure example, but we use it in scripts/oss-fuzz/build.sh to
build a qemu-fuzz binary(qemu-system with some bells and whistles) that
is portable between containers and can be deployed on oss-fuzz. The
other option is to build a static binary, which AFAIK is not supported
for softmmu builds.

I guess this is a prime example of "confusing and non-portable
weirdness".
In defense, it wasn't our idea. The oss-fuzz documentation suggests it:
https://google.github.io/oss-fuzz/further-reading/fuzzer-environment/#runtime-dependencies

> Paolo
> 
> >

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Laurent Vivier 3 years, 8 months ago

Le 26/08/2020 à 21:01, Paolo Bonzini a écrit :
> Otherwise, dollars (such as in the special $ORIGIN rpath) are
> eaten by Make.
> 
> Reported-by: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/ninjatool.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
> index cc77d51aa8..c33eafb5a0 100755
> --- a/scripts/ninjatool.py
> +++ b/scripts/ninjatool.py
> @@ -834,7 +834,8 @@ class Ninja2Make(NinjaParserEventsWithVars):
>          self.print()
>          for targets in self.build_vars:
>              for name, value in self.build_vars[targets].items():
> -                self.print('%s: private .var.%s := %s' % (targets, name, value))
> +                self.print('%s: private .var.%s := %s' %
> +                           (targets, name, value.replace('$', '$$')))
>              self.print()
>          if not self.seen_default:
>              default_targets = sorted(self.all_outs - self.all_ins, key=natural_sort_key)
> 

This actually fixes the '-Wl,-rpath,$ORIGIN/', but doesn't fix the crash
with statically linked binaries.

Could we simply remove the the '-Wl,-rpath,$ORIGIN/' in the case of
"-static" build?

Thanks,
Laurent

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Paolo Bonzini 3 years, 8 months ago

Il gio 27 ago 2020, 09:33 Laurent Vivier <laurent@vivier.eu> ha scritto:

> This actually fixes the '-Wl,-rpath,$ORIGIN/', but doesn't fix the crash
> with statically linked binaries.
>

I will try to reproduce when I am back; it works for Peter so there must be
something different in the setup.

In any case, if needed we can both momentarily hack around it in Makefiles,
and fix it for good in Meson.

Paolo


> Could we simply remove the the '-Wl,-rpath,$ORIGIN/' in the case of
> "-static" build?
>
> Thanks,
> Laurent
>
>

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Paolo Bonzini 3 years, 8 months ago

Found; https://github.com/mesonbuild/Meson/issues/5191.

(With the fix there's no rpath at all in the QEMU build process).

Let's ask for a backport to 0.55.2.

Paolo

Il gio 27 ago 2020, 10:22 Paolo Bonzini <pbonzini@redhat.com> ha scritto:

>
>
> Il gio 27 ago 2020, 09:33 Laurent Vivier <laurent@vivier.eu> ha scritto:
>
>> This actually fixes the '-Wl,-rpath,$ORIGIN/', but doesn't fix the crash
>> with statically linked binaries.
>>
>
> I will try to reproduce when I am back; it works for Peter so there must
> be something different in the setup.
>
> In any case, if needed we can both momentarily hack around it in
> Makefiles, and fix it for good in Meson.
>
> Paolo
>
>
>> Could we simply remove the the '-Wl,-rpath,$ORIGIN/' in the case of
>> "-static" build?
>>
>> Thanks,
>> Laurent
>>
>>

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Paolo Bonzini 3 years, 8 months ago

... and actually it's fixed in 0.55.1. We can therefore just update the
submodule and declare 0.55.1 the minimum required version for QEMU.

Paolo

Il gio 27 ago 2020, 11:10 Paolo Bonzini <pbonzini@redhat.com> ha scritto:

> Found; https://github.com/mesonbuild/Meson/issues/5191.
>
> (With the fix there's no rpath at all in the QEMU build process).
>
> Let's ask for a backport to 0.55.2.
>
> Paolo
>
> Il gio 27 ago 2020, 10:22 Paolo Bonzini <pbonzini@redhat.com> ha scritto:
>
>>
>>
>> Il gio 27 ago 2020, 09:33 Laurent Vivier <laurent@vivier.eu> ha scritto:
>>
>>> This actually fixes the '-Wl,-rpath,$ORIGIN/', but doesn't fix the crash
>>> with statically linked binaries.
>>>
>>
>> I will try to reproduce when I am back; it works for Peter so there must
>> be something different in the setup.
>>
>> In any case, if needed we can both momentarily hack around it in
>> Makefiles, and fix it for good in Meson.
>>
>> Paolo
>>
>>
>>> Could we simply remove the the '-Wl,-rpath,$ORIGIN/' in the case of
>>> "-static" build?
>>>
>>> Thanks,
>>> Laurent
>>>
>>>

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Peter Maydell 3 years, 8 months ago

On Thu, 27 Aug 2020 at 10:20, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> ... and actually it's fixed in 0.55.1. We can therefore just update the submodule and declare 0.55.1 the minimum required version for QEMU.

Oh, I meant to ask -- if 0.56 is the one that gets rid of the
warnings about unstable-keyval backwards compatibility issues,
is there a reason our submodule version is 0.55 rather than 0.56 ?

thanks
-- PMM

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Paolo Bonzini 3 years, 8 months ago

Il gio 27 ago 2020, 12:08 Peter Maydell <peter.maydell@linaro.org> ha
scritto:

> On Thu, 27 Aug 2020 at 10:20, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > ... and actually it's fixed in 0.55.1. We can therefore just update the
> submodule and declare 0.55.1 the minimum required version for QEMU.
>
> Oh, I meant to ask -- if 0.56 is the one that gets rid of the
> warnings about unstable-keyval backwards compatibility issues,
> is there a reason our submodule version is 0.55 rather than 0.56 ?
>

Because 0.56 has not been released yet, it's what the master branch will be.

Paolo


> thanks
> -- PMM
>
>

Re: [PATCH] ninjatool: quote dollars in variables

Posted by Laurent Vivier 3 years, 8 months ago

Le 27/08/2020 à 11:14, Paolo Bonzini a écrit :
> ... and actually it's fixed in 0.55.1. We can therefore just update the
> submodule and declare 0.55.1 the minimum required version for QEMU.
> 

Updating the meson submodule to 0.55.1 has fixed the problem for me.

Thanks,
Laurent