This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] Generalize libFuzzer build and tests to multiple architectures
AbandonedPublic

Authored by george.karpenkov on May 7 2018, 5:41 PM.

Diff Detail

Event Timeline

jfb added a subscriber: jfb.May 16 2018, 10:47 PM
jfb added inline comments.
lib/fuzzer/tests/CMakeLists.txt
29–61

Is the intent to skip ARM64 here? It seems like you'd be better off keeping the loop and doing an early continue if ARM64 instead.

test/fuzzer/lit.cfg
54–55

Is this change OK?

lib/fuzzer/tests/CMakeLists.txt
29–61

Not really, the intent is to only run it on host machine, which is assumed to be x86_64.

test/fuzzer/lit.cfg
54–55

yes

jfb added inline comments.May 17 2018, 1:10 PM
lib/fuzzer/tests/CMakeLists.txt
29–61

It would be useful to at least have a comment to that effect.

george.karpenkov marked an inline comment as done.
jfb added a comment.May 17 2018, 2:18 PM

lgtm, though of course someone like @kcc should sign off.

delcypher added inline comments.May 17 2018, 4:10 PM
test/fuzzer/lit.cfg
55

The naming isn't great here. You're using cflags but you're passing them to a C++ compiler. Normally the name cxxflags is used for C++ compiler flags and cflags for C compiler flags.

57

I'm not a fan of the old style python string interpolation. I'd rather you used the .format() method on python strings.

79

I realise that -O2 was there before but why are we passing that. Shouldn't the test case decide the optimization level?

Also this looks similar to the ASan/TSan compiler invocation. Could we perhaps do a refactor some common flags shared by all tests types are set in one place?

test/fuzzer/merge-posix.test
1

Any reason for XFAIL instead of UNSUPPORTED?

Also for each test that you marked as XFAIL it would nice to have a short comment explaining why its XFAIL for ios.

test/fuzzer/lit.cfg
55

yeah, i guess _flags might make more sense

57

sorry, let's only post comments specific to this change. old-style python string interpolation was there before, change to .format can be done in a separate patch

79

again, let's only discuss changes in this patch. both change to the compiler level, and extracting out the common code is orthogonal.

test/fuzzer/merge-posix.test
1

I think we don't do ulimit properly for device testing yet. My goal is to bring up something workable fast, and then further changes might be incrementally made to de-XFAILify more tests

george.karpenkov abandoned this revision.May 23 2018, 2:39 PM

OK let's try to split it further.