On Thursday, 2020-09-10 at 12:36:52 -04, Alexander Bulekov wrote: > On 200910 1645, Darren Kenny wrote: >> 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 <alx...@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')], >> > > Hi Darren, > >> 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. > > Good point. I tried that out, and everything still works. > Functionality-wise, I don't think it makes a big difference (for example > look at the other calls to add_project*arguments, which all pass in > arrays returned by split()), but its probably better to stick with what > is written in the official docs. I ran oss-fuzz builds with this change > and, unfortunately, the order of the calls to add_project_link_arguments > still makes a difference. > >> >> 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? >> > > I tried that when I was attempting to fix the original build-issue, but > to no avail... I don't have a good explanation. On one hand, the problem > was related to the fact that we were linking in libqos.a. Renaming it to > libqos.fa was part of the fix. On the other hand, we also needed to > specify the arguments as part of global project link arguments, rather > than the proper way with executable() link_args. > > I think Paolo had a better understanding of the actual issue, and some > ideas about how to fix this within meson, rather than with the hacks > exemplified by this patch.
Fair enough, guess we'll see if Paolo has any better suggestions then :) Thanks, Darren.
