Re: [PATCH] x86emul: rework wrapping of libc functions in test and fuzzing harnesses

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/08/2023 2:13 pm, Jan Beulich wrote:
> On 17.08.2023 14:58, Andrew Cooper wrote:
>> On 17/08/2023 12:47 pm, Jan Beulich wrote:
>>> Our present approach is working fully behind the compiler's back. This
>>> was found to not work with LTO. Employ ld's --wrap= option instead. Note
>>> that while this makes the build work at least with new enough gcc (it
>>> doesn't with gcc7, for example, due to tool chain side issues afaict),
>>> according to my testing things still won't work when building the
>>> fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as
>>> getting invoked, this does not happen with gcc13. Yet without using that
>>> assembler wrapper the resulting binary will look uninstrumented to
>>> afl-fuzz.
>>>
>>> While checking the resulting binaries I noticed that we've gained uses
>>> of snprintf() and strstr(), which only just so happen to not cause any
>>> problems. Add a wrappers for them as well.
>>>
>>> Since we don't have any actual uses of v{,sn}printf(), no definitions of
>>> their wrappers appear (just yet). But I think we want
>>> __wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away,
>>> which means we need delarations of the latter.
>>>
>>> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> This does resolve the build issue.  I do get a binary out of the end, so
>> Tested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>.
> Thanks.
>
>>   I presume that you've smoke tested the resulting binary?
> The fuzzer one? No. I didn't think it is of any use when not driven by afl.
> I did a proper test of the test harness one, albeit not with LTO in use (I
> focused on the fuzzer one with the LTO issue).

The fuzzer is only built because it's active-by-default in tools/, not
because it's used.

The test harness is the one that gets used routinely in testing.

>
>> However, I do see something else in the logs which is concerning. 
>> Likely unrelated.
>>
>> make[6]: Entering directory
>> '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator'
>> gcc -m32 -march=i686 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall
>> -Wstrict-prototypes -Wdeclaration-after-statement
>> -Wno-unused-but-set-variable -Wno-unused-local-typedefs -g3 -Werror -Og
>> -fno-omit-frame-pointer
>> -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__
>> -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs
>> -fno-pie -fno-stack-protector -fno-exceptions
>> -fno-asynchronous-unwind-tables -fno-builtin -g0 -D_64f2 -mavx512fp16
>> -ffixed-xmm0 -Os -DVEC_SIZE=64 -DFLOAT_SIZE=2 -c avx512fp16.c
>> make[6]: Leaving directory
>> '/builddir/build/BUILD/xen-4.18.0/tools/tests/x86_emulator'
>> /tmp/ccrznrqa.s: Assembler messages:
>> /tmp/ccrznrqa.s:98: Error: no such instruction: `vmovw .LC0,%xmm3'
>> /tmp/ccrznrqa.s:99: Error: no such instruction: `vmovw %xmm3,58(%esp)'
>> /tmp/ccrznrqa.s:105: Error: no such instruction: `vcvtsi2shl
>> %eax,%xmm1,%xmm1'
>> /tmp/ccrznrqa.s:106: Error: no such instruction: `vmovw
>> %xmm3,382(%esp,%eax,2)'
>> /tmp/ccrznrqa.s:107: Error: no such instruction: `vmovw
>> %xmm1,-2(%edx,%eax,2)'
>> /tmp/ccrznrqa.s:108: Error: no such instruction: `vcvtsi2shl
>> %ecx,%xmm1,%xmm1'
>> /tmp/ccrznrqa.s:109: Error: no such instruction: `vmovw
>> %xmm1,318(%esp,%eax,2)'
>> /tmp/ccrznrqa.s:113: Error: no such instruction: `vaddph
>> 256(%esp),%zmm7,%zmm5'
>> <snip many>
>> simd-fma.c:208: Error: no such instruction: `vfmaddsub231ph
>> 60(%esp){1to32},%zmm6,%zmm5'
>> simd-fma.c:209: Error: no such instruction: `vfmaddsub231ph
>> 60(%esp){1to32},%zmm6,%zmm1'
>>
>> GCC is 12.2.1, binutils is 2.37
>>
>> AVX512_FP16 was added in bintuils 2.38 so I understand the simd-fma.c
>> complains,
> Right. I assume the gcc is not the system one, or else I'd find it
> odd to have a compiler backed by a less capable assembler.

It is the system one, but it's entirely possible that there has been a
bootstrapping error.  We're trying something a bit new.

There are tasks to update both.  I'll keep an eye on the result.

~Andrew