Re: [PATCH] oss-fuzz: move linker arg to fix coverage-build

[Darren Kenny]

Hi Alex,

I'm certainly not an expert in meson, but have some questions below...

On Wednesday, 2020-09-09 at 18:05:16 -04, Alexander Bulekov wrote:
> The order of the add_project_link_arguments calls impacts which
> arguments are placed between --start-group and --end-group.
> OSS-Fuzz coverage builds seem to just add these to CFLAGS:
> -fprofile-instr-generate -fcoverage-mapping pthread -Wl,--no-as-needed
> --Wl,-ldl -Wl,-lm Wno-unused-command-line-argument
>
> for some reason that is enough to shift the fork_fuzz.ld linker-script
> back into the linker group. Move the linker-script meson call before the
> other calls to mitigate this.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  meson.build | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> Good news! Standard oss-fuzz builds are working again:
> https://oss-fuzz-build-logs.storage.googleapis.com/log-2fa5122f-c98c-4e46-b3ff-e6835d9ecda6.txt
>
> Bad news: Coverage builds are still-broken:
> https://oss-fuzz-build-logs.storage.googleapis.com/log-dafece55-81f2-4d1d-a686-c5197cdd15c1.txt
>
> For some reason, just switching around the order of the
> add_project_arguments fixes this (i.e. the order of the calls impacts
> which arguments are placed between --start-group --end-group). I don't
> really like this because it makes this linker-script block even more
> visible in meson.build, by placing it directly beneath the "Compiler
> flags" heading. Paolo, do you have a better suggestion?
>
> diff --git a/meson.build b/meson.build
> index 5421eca66a..2ba1823ca3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -49,6 +49,14 @@ configure_file(input: files('scripts/ninjatool.py'),
>  # Compiler flags #
>  ##################
>  
> +# Specify linker-script with add_project_link_arguments so that it is not 
> placed
> +# within a linker --start-group/--end-group pair
> +if 'CONFIG_FUZZ' in config_host
> +   add_project_link_arguments(['-Wl,-T,',
> +                               (meson.current_source_dir() / 
> 'tests/qtest/fuzz/fork_fuzz.ld')],

Why do you use an array here rather than a string concatenation? Looking
at the documentation, it suggests that each arg to
add_project_link_arguments() should be specified separately - and
doesn't mention anything about an argument being a list and what would
happen here.

What I'm wondering is how the array might be handled, as in would it be
like the Python equivalent of:

  "".join(['a', b'])   => 'ab'

or

  " ".join(['a', b'])   => 'a b'

It's not honestly clear, or at least I couldn't find anything that says
clearly what the result would be.

So, would it be more correct as either:

  add_project_link_arguments('-Wl,-T,' + (meson.current_source_dir() / 
'tests/qtest/fuzz/fork_fuzz.ld'),

or

  add_project_link_arguments('-Wl,-T,', (meson.current_source_dir() / 
'tests/qtest/fuzz/fork_fuzz.ld'),

I'm also wondering if this in any way would affect how meson moves the
linker arguments around later.

Alternatively, there is a link_args argument to the executable()
function, which is being used for adding @qemu.syms and @block.syms
around line 1017.

Would it work to add this linker-script at this point, in a conditional
block for CONFIG_FUZZ here instead?

Thanks,

Darren.

> +                              native: false, language: ['c', 'cpp', 'objc'])
> +endif
> +
>  add_project_arguments(config_host['QEMU_CFLAGS'].split(),
>                        native: false, language: ['c', 'objc'])
>  add_project_arguments(config_host['QEMU_CXXFLAGS'].split(),
> @@ -58,13 +66,6 @@ 
> add_project_link_arguments(config_host['QEMU_LDFLAGS'].split(),
>  add_project_arguments(config_host['QEMU_INCLUDES'].split(),
>                        language: ['c', 'cpp', 'objc'])
>  
> -# Specify linker-script with add_project_link_arguments so that it is not 
> placed
> -# within a linker --start-group/--end-group pair
> -if 'CONFIG_FUZZ' in config_host
> -   add_project_link_arguments(['-Wl,-T,',
> -                               (meson.current_source_dir() / 
> 'tests/qtest/fuzz/fork_fuzz.ld')],
> -                              native: false, language: ['c', 'cpp', 'objc'])
> -endif
>  
>  link_language = meson.get_external_property('link_language', 'cpp')
>  if link_language == 'cpp'
> -- 
> 2.28.0