Neat, a toy to play with over the weekend...

Oh we should probably change the fetchFromGitHub owner to AFLplusplus now they've got an organization, and they also seem to have an actual homepage @ https://aflplus.plus/

I've got qemu building in 229f58b

Now I'm getting a failure building unsigaction32...

Oh but it seems unsigaction32's build is broken anyway AFLplusplus/AFLplusplus@079fdbf#commitcomment-38428357 so we'll just have to skip it.

(see d4e6258)

Ok and I've got the tests working again in 5fee991

I also added cmocka to get the "unit tests" working, and they do work - not sure of the value though...?

I also haven't used it yet.

Some discussion points:

1. Not sure how to commit this while acknowledging your changes. Is mentioning you in the commit ok or how is this typically done? If I squash I'm the only author, afaik.
2. Should the clang_9 and llvm_9 variables be defined in the top-level/all-packages.nix file or is it ok as-is? The advantage of doing it in the top-level could be that someone can override them once clang-10 and llvm-10 are out of RC phase, I think. At least, that's what I think the top-level file does.

1. Don't worry about acknowledging my commits, squash them, actually probably worth a mention to spread the blame when it all blows up ;)
2. If there is a canonical answer to this, I don't know it. I guess it depends to what extent we find it works with other llvm versions (I haven't tried others, I have to compile llvm enough as it is...). e.g. if we find it blows up with anything other than llvm_9, there's little point in giving users that knob to twiddle.

Hmm, I can't get qemu_mode/libcompcov qemu_mode/unsigaction to build on i686. Which is annoying because the wine mode only works in 32bit qemu mode.

Seems to be possible to use an older llvm/clang version, but I don't think that's too useful for most people. Let's just leave them out of the configurable options.

Maybe you should create an issue upstream, if you think the issue lies there.

I've managed to get unsigaction building acceptably again by applying the fix that's already in master:

    patches = [
      (fetchpatch {
        url = "https://github.com/AFLplusplus/AFLplusplus/commit/5b9928f1a9d4b017ea04365ca8b522fde71236eb.patch";
        sha256 = "1m4w9w4jaxb2mjkwvr6r4qa2j5cdzzpchjphpwd95861h0zvb6hh";
      })
    ];

This also fixes build on i686 for me.

I've applied the patch you sent in. It seems to be working ok, but I'm on a 64-bit system. Should the unsigaction32.so be built on a 64-bit system? It doesn't build for me. Only unsigaction64.so seems to build for me.

I find that unsigaction32 only builds on 32bit, unsigaction64 only on 64bit, and you can test i686 easily because it's all available under pkgsi686Linux.aflplusplus.

However, I can't get even a straightforward afl-clang-fast build based on this package to work for me:

[-] PROGRAM ABORT : Oops, failed to execute 'clang' - check your PATH
             Location : main(), afl-clang-fast.c:846

This is when I afl-build yajl by adding

  AFL_HARDEN="1";
  AFL_LLVM_LAF_SPLIT_SWITCHES="1";
  AFL_LLVM_LAF_TRANSFORM_COMPARES="1";
  AFL_LLVM_LAF_SPLIT_COMPARES="1";
  preConfigure = ''
    export CC=${aflplusplus}/bin/afl-clang-fast
  '';

to its attrs.

I'm pretty sure were still going to need to bind the afl tools to their underlying compilers, if you run strings on the resulting afl-clang-fast binary you can see it has references to the llvm-9.0.0 package, but that package doesn't actually have clang only llvm utilities.

The way I do this is by making a shell like so:

with import /home/rick/nixpkgs-aflplusplus {}; {
  qpidEnv = stdenvNoCC.mkDerivation {
    name = "fuzz-env";
    buildInputs = [
      pkgs.clang_9
      pkgs.llvm_9
      pkgs.cmake
      pkgs.aflplusplus
      pkgs.pkg-config
      pkgs.gcc9
    ];
  };
}

Note the pkgs.clang_9. I've looked at the documentation about this and there are ways to pass these dependencies through, but I wasn't sure if we should do so. It looks like depsTargetTarget might be what we need, but there is a disclaimer:

A list of dependencies whose host platform matches the new derivation's target platform. This means a 1 offset from the new derivation's platforms. These are packages that run on the target platform, e.g. the standard library or run-time deps of standard library that a compiler insists on knowing about. It's poor form in almost all cases for a package to depend on another from a future stage [future stage corresponding to positive offset]. Do not use this attribute unless you are packaging a compiler and are sure it is needed.

s.a.: https://nixos.org/nixpkgs/manual/#ssec-stdenv-dependencies

Maybe just putting clang_9 in the buildInputs would be fine too, not sure about that.

Edit: yeah, I think it should be in buildInputs. Will try shortly.

Even if you are able to pass these extras through, these will be runtime-resolved dependencies (if you coincidentally end up with the another clang in your runtime env it will break), and a well built package shouldn't have to rely on its runtime environment to work properly.

We also shouldn't unnecessarily spew our dependencies into the package user's runtime env, because it makes using more than one package which has taken this attitude at the same time almost impossible.

Ah yes, thanks for the insights!

You're completely right, I didn't think through that the implicit dependency (without having the correct env variables set) during runtime makes the package impure.

However, I do think that there's a runtime dependency on clang (and AFL assumes the version that it's built with, too, as far as I can see. Some optimizations are enabled/disabled based on the version of the compiler used). So I'm not sure if we shouldn't pass it through, because otherwise you'll always have to specify the correct version of clang, I think. That doesn't make the package more friendly to use and I'm not sure why it'd be beneficial if you could use afl without having clang in your environment.

You shouldn't have to - unless for some reason you need to also call the unwrapped clang during a build process. Otherwise the correctly wrapped afl-clang-fast will do everything you need clang to do, and its' wrapper's reference to our special clang will get picked up by nix as a runtime dependency - it just won't get injected into the target's PATH etc. (which TBH I think would just be confusing and a bit of a footgun)

It's certainly worked for me in quite heavy use for most of a year now.

It's helped by the fact that clang is near 100% interchangeable with gcc and its tools on linux (doesn't need its own linker etc., GNU tools are quite happy with its output)

Applying the following patch adds the full path to the clang binary inside the built binary. However, AFL_PATH should still be set as well (to prevent any issues with loading the shared libs). Maybe the hack you used is the best method after all, not sure.

+
+    postPatch = ''
+      # Replace the CLANG_BIN variables with the correct path
+      substituteInPlace llvm_mode/afl-clang-fast.c \
+        --replace "CLANGPP_BIN" '"${clang_9}/bin/clang++"' \
+        --replace "CLANG_BIN" '"${clang_9}/bin/clang"'
+    '';
+

I've also found out that AFL (depending on what you build) links afl-clang either to afl-clang-fast or to afl-gcc. I'm not sure what's best, but we could consider just removing it. It doesn't add anything if it's symlinked to afl-clang-fast anyway.

Ah I hadn't considered patching the source - there's definitely a neatness to it. It should be possible to patch AFL_PATH in as a "default" too a la

--replace 'getenv("AFL_PATH")' "(getenv(\"AFL_PATH\") ? getenv(\"AFL_PATH\") : \"$out/lib/afl\")"

the only disadvantage of this I see is it being more sensitive to source changes, which we're kind of invulnerable to using wrappers. But then again, using wrappers adds a few runtime weirdness possibilities...

🤷

As for afl-clang I think they discontinued it because afl-clang-fast was better in every way and now they just symlink to afl-clang-fast it for compatibility.

I've pushed new changes with the source changes. I've removed the symlinks from afl-clang to afl-clang-fast. I've applied the getenv("AFL_PATH") patch as well.

Neat - do we need to do similar for afl-gcc / afl-gcc-fast?

Yes, we'd need to do the same.

One advantage of doing it this way is that you can still pass the AFL_CC and AFL_CXX to the compilers.

Will look into doing the same for gcc now.

Do we want to support gcj as well? It's a java compiler that's part of the gcc collection, but it has been deprecated according to wikipedia, so I don't think we should support it. I'll ask upstream if they'd like to keep it or what they think.

It was part of the GNU Compiler Collection for over ten years but as of 2017 it is no longer maintained and will not be part of future releases.

Edit: started discussion about this upstream as well AFLplusplus/AFLplusplus#184 (comment)

Edit2: A patch has been applied for afl-gcc. afl-gcc-fast already picks up the correct gcc from the PATH during build time (it's passed as a define), and inspection reveals the path is indeed hardcoded.

I think this makes the whole thing ready for review. I can't seem to test the 32-bit binaries, as I get failures that show that clang can't be built for 32-bit systems. Could you check this @risicle? We might need to rebase master against this if it keeps failing. I've tried building it with nix-shell -I nixpkgs=~/nixpkgs-aflplusplus/ -p pkgsi686Linux.aflplusplus.

This seems to be working... think this is ready?

One annoying bug 2.64 has is AFLplusplus/AFLplusplus#342 unfortunately the patch won't apply cleanly. Will have to wait for 2.65.

Let's do that then. It's quite a fast-moving target so it's hard to keep track of all changes and ensure everything works as intended.

:high-five: