This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] don't use /dev/null for DiscardOuput in Fuchsia.
ClosedPublic

Authored by charco on Oct 29 2019, 3:42 PM.

Details

Summary

This commit adds a specialized version of DiscardOutput for fuchsia,
removing an usage of /dev/null.

In fuchsia, accessing /dev/null is not supported, and there's nothing
similar to a file that discards everything that is written to it. The
way of doing something similar in fuchsia is by using fdio_null_create
and binding that to a file descriptor with fdio_bind_to_fd.

This change should fix one of the issues with the -close_fd_mask flag
in libfuzzer, in which closing stdout was not working due to
fopen("/dev/null", "w") returning NULL.

Diff Detail

Event Timeline

charco created this revision.Oct 29 2019, 3:42 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 29 2019, 3:42 PM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript

Hi Reviewers!

I have two versions of this change:

This version adds an ifdef guard to specialize the code for fuchsia, and I have another set of changes that fork FuzzerIOPosix into FuzzerIOFuchsia and adds the specialized version there:

Please review whichever you think it the best approach. So far, the only thing that has been causing issues in FuzzerIOPosix for Fuchsia is /dev/null usage, I am not sure whether there will be other changes that would justify the fork of this file. There's some usage of the getDevNull function that we also need to address, but that could be handled in fuchsia specific code (checking if some path == getDevNull, or refactoring the code to not assume that there is a devnull-like file).

kcc added a comment.Nov 4 2019, 6:21 PM

please avoid #ifdefs -- they are pure evil.
Can you move this function to FuzzerUtil*.cpp?
In this case it will have it's own implementation in FuzzerUtilFuchsia.cpp

It sucks in a different way, but less than using #ifdefs

charco updated this revision to Diff 227935.Nov 5 2019, 11:26 AM

Move DiscardOutput to FuzzerUtil*.

As suggested by the reviewers, I'm moving the DiscardOutput function to
FuzzerUtil(Darwin|Linux|Windows|Fuchsia).cpp, so now each platform can have
its own specialized version.

hctim accepted this revision.Nov 12 2019, 1:41 PM

Ping :)

Sorry, missed this in email churn.

compiler-rt/lib/fuzzer/FuzzerUtilLinux.cpp
39

Nit: remove extra newline

compiler-rt/lib/fuzzer/FuzzerUtilPosix.cpp
163 ↗(On Diff #227935)

Nit: no need to remove, pollutes diff only

This revision is now accepted and ready to land.Nov 12 2019, 1:41 PM
charco updated this revision to Diff 228983.Nov 12 2019, 4:29 PM
charco marked 2 inline comments as done.

Fix reviewer comments.

charco updated this revision to Diff 230567.Nov 21 2019, 4:54 PM

Add missing header for _dup2 in windows.

This revision was automatically updated to reflect the committed changes.