google / syzkaller

syzkaller is an unsupervised coverage-guided kernel fuzzer
Apache License 2.0
5.37k stars 1.23k forks source link

all: Yocto build compatibility #3118

Open sephalon opened 2 years ago

sephalon commented 2 years ago

Is your feature request related to a problem? Please describe. I am facing build compatibility issues while preparing Yocto/OpenEmbedded recipes [1] for syzkaller because the build process currently does not allow certain variables to be set from the environment.

Describe the solution you'd like There are two issues to be addressed; please follow the links for implementation proposals:

  1. Allow LDFLAGS to be set from the environment. See efd9a70 for a related patch regarding GOHOSTFLAGS and GOTARGETFLAGS.
  2. Allow setting CC with --sysroot from environment. I propose that if FORCECC is set [2], targets.go will populate CCompiler as well as EarlyCFlags (for --sysroot) from the environment.

Additional context [1] I intend to have separate recipes for host and target tools to be upstreamed to meta-openembedded; my recipes are not to be confused with meta-syzkaller-wrl, which targets a very specific use case. [2] In practice, during Yocto build FORCECC is set to CC.

dvyukov commented 2 years ago

Hi @sephalon,

  1. Allow LDFLAGS to be set from the environment. See efd9a70 for a related patch regarding GOHOSTFLAGS and GOTARGETFLAGS.

This part looks reasonable. But what flags do you want to add? Maybe we could add them always unconditionally?

  1. Allow setting CC with --sysroot from environment. I propose that if FORCECC is set [2], targets.go will populate CCompiler as well as EarlyCFlags (for --sysroot) from the environment.

Can't EarlyCFlags be merged into existing CFlags? In both places you added EarlyCFlags, CFlags are already used. Also can't LDFLAGS in this change https://github.com/nokia/syzkaller/commit/b97ea81afe4076c24aa41a47c4eef3cf79e20580 be part of FORCECC flags?

sephalon commented 2 years ago

This part looks reasonable. But what flags do you want to add? Maybe we could add them always unconditionally?

In my specific case e.g. --hash-style=gnu. In general it is considered to be good practice to follow the build environment variables set by Yocto to allow for e.g. a certain optimizations globally.

Can't EarlyCFlags be merged into existing CFlags? In both places you added EarlyCFlags, CFlags are already used.

--sysroot must be set before any other options; it is essentially part of CC.

Also can't LDFLAGS in this change nokia@b97ea81 be part of FORCECC flags?

I guess it could, but it does not look very clean to me: CFLAGS are also provided by the build environment in the Makefile, while ADDCFLAGS provides target-dependent flags from syz-make.

In general I tried to find a minimally invasive solution here that allows injection of flags from the environment, but does not interfere with syz-make to avoid introducing compatibility issues in the future.

dvyukov commented 2 years ago

--sysroot must be set before any other options; it is essentially part of CC.

Interesting. I don't see anything in man gcc re this. Do you have any references?

Trying to compile this:

#include <stdio.h>
int main() {}

It gives errors both ways:

$ gcc --sysroot=/foobar /tmp/test.c
/tmp/test.c:1:10: fatal error: stdio.h: No such file or directory
    1 | #include <stdio.h>
      |          ^~~~~~~~~
compilation terminated.

$ gcc /tmp/test.c --sysroot=/foobar
/tmp/test.c:1:10: fatal error: stdio.h: No such file or directory
    1 | #include <stdio.h>
sephalon commented 2 years ago

Interesting. I don't see anything in man gcc re this. Do you have any references?

I have to correct myself, the order of options seems to irrelevant as you say - whatever issue I had before with this I am unable to reproduce anymore.

I have adapted my proposal accordingly.