Page MenuHomePhabricator

[libFuzzer] Diff 23 - Properly use compiler options supported on Windows.
ClosedPublic

Authored by mpividori on Dec 16 2016, 4:07 PM.

Event Timeline

mpividori updated this revision to Diff 81809.Dec 16 2016, 4:07 PM
mpividori retitled this revision from to [libFuzzer] Diff 23 - Properly use compiler options supported on Windows..
mpividori updated this object.
mpividori set the repository for this revision to rL LLVM.
kcc added inline comments.Dec 16 2016, 4:13 PM
lib/Fuzzer/test/CMakeLists.txt
30 ↗(On Diff #82607)

Nope. =edge should not be used any more with libFuzzer.
I am going to delete the libFuzzer code that supports it pretty soon.

What's wrong with trace-pc-guard,trace-cmp on windows?

37 ↗(On Diff #82607)

hmmm. What is the 'else' compiler?

zturner added inline comments.Dec 16 2016, 4:17 PM
lib/Fuzzer/test/CMakeLists.txt
28 ↗(On Diff #82607)

Technically we don't support MSVC, only clang-cl. So I would change this to if (CLANG_CL).

37 ↗(On Diff #82607)

clang-cl I imagine, but /Z7 is more correct than /Zi. To make the else clear, you could say elseif(CLANG_CL)

kcc added inline comments.Dec 16 2016, 4:18 PM
lib/Fuzzer/test/CMakeLists.txt
37 ↗(On Diff #82607)

Does clang-cl not support -g?

mpividori added inline comments.Dec 16 2016, 4:24 PM
lib/Fuzzer/test/CMakeLists.txt
28 ↗(On Diff #82607)

@zturner, Yes, the only compiler in windows for this code is clang. I can use a more explicit flag.

30 ↗(On Diff #82607)

@kcc Ok. I couldn't get that information from the documentation.
Trace-* options doesn't work on windows. I can try to fix them, I would need some time.

37 ↗(On Diff #82607)

Yes, clang-cl doesn't support -g.

kcc added inline comments.Dec 16 2016, 4:29 PM
lib/Fuzzer/test/CMakeLists.txt
30 ↗(On Diff #82607)

Ok. I couldn't get that information from the documentation.

Which documentation?
http://llvm.org/docs/LibFuzzer.html mentions only trace-pc-guard and trace-pc

Trace-* options doesn't work on windows. I can try to fix them, I would need some time.

What exactly is not working? (probably let's take it into another thread)

37 ↗(On Diff #82607)

Sad, but ok.

mpividori added inline comments.Dec 16 2016, 4:35 PM
lib/Fuzzer/test/CMakeLists.txt
30 ↗(On Diff #82607)

@kcc I refer to the Sanitizer Coverage documentation: http://clang.llvm.org/docs/SanitizerCoverage.html
It doesn't mention which options are supported on Windows.

kcc added inline comments.Dec 16 2016, 4:39 PM
lib/Fuzzer/test/CMakeLists.txt
30 ↗(On Diff #82607)

Yep, because non of it is supported on windows (in terms of 'my team supporting them n windows').
But the trace- options don't have sanitizer-run-time portion when used with libFuzzer, so it's a pure compiler feature, nothing (??) OS-specific.
Except maybe the way how weak functions work... Hmmm. You will need to check this.

Anyway, libFuzzer will soon stop supporting anything but the options being used currently.

mpividori updated this revision to Diff 82240.Dec 21 2016, 9:04 AM

Update diff to include suggested modifications.

kcc edited edge metadata.Dec 27 2016, 11:02 AM

please
a) update the change description.
b) find an analog for -g that works for both versions.
btw, we really only need -gmlt (= -gline-tables-only), so check if those work.

mpividori updated this revision to Diff 82607.Dec 28 2016, 9:40 AM
mpividori updated this object.
mpividori edited edge metadata.
kcc accepted this revision.Dec 28 2016, 2:41 PM
kcc edited edge metadata.

LGTM
Aha, that's much better (assuming tests pass on linux)

This revision is now accepted and ready to land.Dec 28 2016, 2:41 PM
This revision was automatically updated to reflect the committed changes.