[PATCH] ninjatool: quote dollars in variables

[PATCH] ninjatool: quote dollars in variables

[Paolo Bonzini]

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

[Peter Maydell]

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

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

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

>

[-- Attachment #2: Type: text/html, Size: 1379 bytes --]

Re: [PATCH] ninjatool: quote dollars in variables

[Alexander Bulekov]

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

[Laurent Vivier]

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

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

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
>
>

[-- Attachment #2: Type: text/html, Size: 1192 bytes --]

Re: [PATCH] ninjatool: quote dollars in variables

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

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
>>
>>

[-- Attachment #2: Type: text/html, Size: 1937 bytes --]

Re: [PATCH] ninjatool: quote dollars in variables

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

... 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
>>>
>>>

[-- Attachment #2: Type: text/html, Size: 2548 bytes --]

Re: [PATCH] ninjatool: quote dollars in variables

[Peter Maydell]

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

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

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
>
>

[-- Attachment #2: Type: text/html, Size: 1309 bytes --]

Re: [PATCH] ninjatool: quote dollars in variables

[Laurent Vivier]

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