[libFuzzer] Error and exit if user supplied fuzzer writeable directories don't exist

[dgg5503]

Summary

Currently, libFuzzer will exit with an error message if a non-existent
corpus directory is provided. However, if a user provides a non-existent
directory for the artifact_prefix, exact_artifact_path, or
features_dir, libFuzzer will continue execution but silently fail to
write artifacts/features.

To improve the user experience, this PR adds validation for the existence of
all user supplied directories before executing the main fuzzing loop. If they
don't exist, libFuzzer will exit with an error message.

[dgg5503]

Applied clang-format to some areas of the code but left others alone to be consistent with the rest of the libFuzzer code base. Please let me know if it's appropriate to apply all suggested clang-tidy and clang-format changes.

[dgg5503]

Ping.

[kcc]

I'd rather fail instead of silently creating new dirs, to be consistent with the other behavior

[dgg5503]

In D84808#2194438, @kcc wrote:

I'd rather fail instead of silently creating new dirs, to be consistent with the other behavior

Are you referring to the non-existent corpus case? If so, that's also changed in this PR to have its directory created if it doesn't exist. Please let me know if I've overlooked additional cases where directories are expected to exist.

[kcc]

From the description:

this PR adds automatic directory creation for locations in which libFuzzer expects to write data.

I'd prefer libFuzzer to not create directories, but instead err-and-exit if those don't exist.

[dgg5503]

In D84808#2194844, @kcc wrote:

From the description:

this PR adds automatic directory creation for locations in which libFuzzer expects to write data.

I'd prefer libFuzzer to not create directories, but instead err-and-exit if those don't exist.

I can make this change, but is there a reason why this shouldn't be done? It seems more convenient for the end user but perhaps I'm overlooking a larger issue.

[kcc]

In D84808#2195169, @dgg5503 wrote:

In D84808#2194844, @kcc wrote:

From the description:

this PR adds automatic directory creation for locations in which libFuzzer expects to write data.

I'd prefer libFuzzer to not create directories, but instead err-and-exit if those don't exist.

I can make this change, but is there a reason why this shouldn't be done? It seems more convenient for the end user but perhaps I'm overlooking a larger issue.

First reason is consistency. We currently don't create DIRs if they don't exist.
The second reason is avoiding surprise.
Suppose a user has a typo in a directory name.
Crashing because a wrongly named dir doesn't exist is not a surprise.
Working and using a wrongly named dir *is* a surprise.

[dgg5503]

In D84808#2200407, @kcc wrote:

In D84808#2195169, @dgg5503 wrote:

In D84808#2194844, @kcc wrote:

From the description:

this PR adds automatic directory creation for locations in which libFuzzer expects to write data.

I'd prefer libFuzzer to not create directories, but instead err-and-exit if those don't exist.

I can make this change, but is there a reason why this shouldn't be done? It seems more convenient for the end user but perhaps I'm overlooking a larger issue.

First reason is consistency. We currently don't create DIRs if they don't exist.
The second reason is avoiding surprise.
Suppose a user has a typo in a directory name.
Crashing because a wrongly named dir doesn't exist is not a surprise.
Working and using a wrongly named dir *is* a surprise.

Thank you for the elaboration @kcc . Your reasoning makes sense to me given the current design of my patch. Before I make the appropriate changes, would it be acceptable to have an environment variable or launch parameter that could allow the silent creation of these directories? The reason I'm so keen on this functionality is that I'm working with a remotely accessible embedded environment where it's a bit of a pain to create these directories by hand, especially in CI situations.

[kcc]

would it be acceptable to have an environment variable or launch parameter that could allow the silent creation of these directories?

That would be a very reasonable thing to do (command line flag, not env var)!
Like, -create-nonexistent-dirs=1 (0 by default) or some such.
But please make it a separate patch to simplify the review.

[dgg5503]

Fixed the diff

[kcc]

+Matt

[dgg5503]

Ping.

[morehouse]

compiler-rt/lib/fuzzer/FuzzerDriver.cpp

692

If it is a file, should we print an error message?

708

Nit: remove this space.

[dgg5503]

compiler-rt/lib/fuzzer/FuzzerDriver.cpp

692

The first input can also be a test file rather than a directory. An example of this can be seen in fuzzer-runs.test.

I also want to point out that Options.OutputCorpus is no longer being set if a file is provided as (*Inputs)[0] unlike the old behavior. All fuzzer tests & unit tests pass on my local Linux machine so I don't think this is having any adverse effect.

[dgg5503]

Address @morehouse comment.

compiler-rt/lib/fuzzer/FuzzerDriver.cpp

if (ExitCode != 0)
       *HasErrors = true;
     std::lock_guard<std::mutex> Lock(Mu);
     Printf("================== Job %u exited with exit code %d ============\n",
            C, ExitCode);
     fuzzer::CopyFileToErr(Log);
   }
 }
 
+static void ValidateDirectoryExists(const std::string &Path) {
+  if (!Path.empty() && !IsDirectory(Path)) {
+    Printf("ERROR: The required directory \"%s\" does not exist\n", Path.c_str());
+    exit(1);
+  }
+}
+
 std::string CloneArgsWithoutX(const Vector<std::string> &Args,
                               const char *X1, const char *X2) {
   std::string Cmd;
   for (auto &S : Args) {
     if (FlagValue(S.c_str(), X1) || FlagValue(S.c_str(), X2))
       continue;
     Cmd += S + " ";
   }
int FuzzerDriver(int *argc, char ***argv, UserCallback Callback) {
   Options.PurgeAllocatorIntervalSec = Flags.purge_allocator_interval;
   Options.TraceMalloc = Flags.trace_malloc;
   Options.RssLimitMb = Flags.rss_limit_mb;
   Options.MallocLimitMb = Flags.malloc_limit_mb;
   if (!Options.MallocLimitMb)
     Options.MallocLimitMb = Options.RssLimitMb;
   if (Flags.runs >= 0)
     Options.MaxNumberOfRuns = Flags.runs;
-  if (!Inputs->empty() && !Flags.minimize_crash_internal_step)
-    Options.OutputCorpus = (*Inputs)[0];
+  if (!Inputs->empty() && !Flags.minimize_crash_internal_step) {
+    // Ensure output corpus assumed to be the first arbitrary argument input
+    // is not a path to an existing file.
+    std::string OutputCorpusDir = (*Inputs)[0];
+    if (!IsFile(OutputCorpusDir)) {
+      Options.OutputCorpus = OutputCorpusDir;
+      ValidateDirectoryExists(Options.OutputCorpus);
+    }
+  }
   Options.ReportSlowUnits = Flags.report_slow_units;
-  if (Flags.artifact_prefix)
+  if (Flags.artifact_prefix) {
     Options.ArtifactPrefix = Flags.artifact_prefix;
-  if (Flags.exact_artifact_path)
+
+    // Since the prefix could be a full path to a file name prefix, assume
+    // that if the path ends with the platform's separator that a directory
+    // is desired
+    std::string ArtifactPathDir = Options.ArtifactPrefix;
+    if (!IsSeparator(ArtifactPathDir[ArtifactPathDir.length() - 1])) {
+      ArtifactPathDir = DirName(ArtifactPathDir);
+    }
+    ValidateDirectoryExists(ArtifactPathDir);
+  }
+  if (Flags.exact_artifact_path) {
     Options.ExactArtifactPath = Flags.exact_artifact_path;
+    ValidateDirectoryExists(DirName(Options.ExactArtifactPath));
+  }
   Vector<Unit> Dictionary;
   if (Flags.dict)
     if (!ParseDictionaryFile(FileToString(Flags.dict), &Dictionary))
       return 1;
   if (Flags.verbosity > 0 && !Dictionary.empty())
     Printf("Dictionary: %zd entries\n", Dictionary.size());
   bool RunIndividualFiles = AllInputsAreFiles();
   Options.SaveArtifacts =
       !RunIndividualFiles || Flags.minimize_crash_internal_step;
   Options.PrintNewCovPcs = Flags.print_pcs;
   Options.PrintNewCovFuncs = Flags.print_funcs;
   Options.PrintFinalStats = Flags.print_final_stats;
   Options.PrintCorpusStats = Flags.print_corpus_stats;
   Options.PrintCoverage = Flags.print_coverage;
   if (Flags.exit_on_src_pos)
     Options.ExitOnSrcPos = Flags.exit_on_src_pos;
   if (Flags.exit_on_item)
     Options.ExitOnItem = Flags.exit_on_item;
   if (Flags.focus_function)
     Options.FocusFunction = Flags.focus_function;
   if (Flags.data_flow_trace)
     Options.DataFlowTrace = Flags.data_flow_trace;
-  if (Flags.features_dir)
+  if (Flags.features_dir) {
     Options.FeaturesDir = Flags.features_dir;
+    ValidateDirectoryExists(Options.FeaturesDir);
+  }
   if (Flags.collect_data_flow)
     Options.CollectDataFlow = Flags.collect_data_flow;
   if (Flags.stop_file)
     Options.StopFile = Flags.stop_file;
   Options.Entropic = Flags.entropic;
   Options.EntropicFeatureFrequencyThreshold =
       (size_t)Flags.entropic_feature_frequency_threshold;
   Options.EntropicNumberOfRarestFeatures =

compiler-rt/lib/fuzzer/FuzzerIO.h

 void Printf(const char *Fmt, ...);
 void VPrintf(bool Verbose, const char *Fmt, ...);
 
 // Print using raw syscalls, useful when printing at early init stages.
 void RawPrint(const char *Str);
 
 // Platform specific functions:
 bool IsFile(const std::string &Path);
+bool IsDirectory(const std::string &Path);
 size_t FileSize(const std::string &Path);
 
 void ListFilesInDirRecursive(const std::string &Dir, long *Epoch,
                              Vector<std::string> *V, bool TopDir);
 
 void RmDirRecursive(const std::string &Dir);
 
 // Iterate files and dirs inside Dir, recursively.
 // Call DirPreCallback/DirPostCallback on dirs before/after
 // calling FileCallback on files.
 void IterateDirRecursive(const std::string &Dir,
                          void (*DirPreCallback)(const std::string &Dir),
                          void (*DirPostCallback)(const std::string &Dir),
                          void (*FileCallback)(const std::string &Dir));
 
 struct SizedFile {
   std::string File;
   size_t Size;
   bool operator<(const SizedFile &B) const { return Size < B.Size; }
 };
 
 void GetSizedFilesFromDir(const std::string &Dir, Vector<SizedFile> *V);
 
 char GetSeparator();
+bool IsSeparator(char C);
 // Similar to the basename utility: returns the file name w/o the dir prefix.
 std::string Basename(const std::string &Path);
 
 FILE* OpenFile(int Fd, const char *Mode);
 
 int CloseFile(int Fd);
 
 int DuplicateFile(int Fd);

compiler-rt/lib/fuzzer/FuzzerIOPosix.cpp

 
 bool IsFile(const std::string &Path) {
   struct stat St;
   if (stat(Path.c_str(), &St))
     return false;
   return S_ISREG(St.st_mode);
 }
 
-static bool IsDirectory(const std::string &Path) {
+bool IsDirectory(const std::string &Path) {
   struct stat St;
   if (stat(Path.c_str(), &St))
     return false;
   return S_ISDIR(St.st_mode);
 }
 
 size_t FileSize(const std::string &Path) {
   struct stat St;
void IterateDirRecursive(const std::string &Dir,
   closedir(D);
   DirPostCallback(Dir);
 }
 
 char GetSeparator() {
   return '/';
 }
 
+bool IsSeparator(char C) {
+  return C == '/';
+}
+
 FILE* OpenFile(int Fd, const char* Mode) {
   return fdopen(Fd, Mode);
 }
 
 int CloseFile(int fd) {
   return close(fd);
 }
 

compiler-rt/lib/fuzzer/FuzzerIOWindows.cpp

bool IsFile(const std::string &Path) {
   return IsFile(Path, Att);
 }
 
 static bool IsDir(DWORD FileAttrs) {
   if (FileAttrs == INVALID_FILE_ATTRIBUTES) return false;
   return FileAttrs & FILE_ATTRIBUTE_DIRECTORY;
 }
 
+bool IsDirectory(const std::string &Path) {
+  DWORD Att = GetFileAttributesA(Path.c_str());
+
+  if (Att == INVALID_FILE_ATTRIBUTES) {
+    Printf("GetFileAttributesA() failed for \"%s\" (Error code: %lu).\n",
+           Path.c_str(), GetLastError());
+    return false;
+  }
+
+  return IsDir(Att);
+}
+
 std::string Basename(const std::string &Path) {
   size_t Pos = Path.find_last_of("/\\");
   if (Pos == std::string::npos) return Path;
   assert(Pos < Path.size());
   return Path.substr(Pos + 1);
 }
 
 size_t FileSize(const std::string &Path) {

 void RenameFile(const std::string &OldPath, const std::string &NewPath) {
   rename(OldPath.c_str(), NewPath.c_str());
 }
 
 intptr_t GetHandleFromFd(int fd) {
   return _get_osfhandle(fd);
 }
 
-static bool IsSeparator(char C) {
+bool IsSeparator(char C) {
   return C == '\\' || C == '/';
 }
 
 // Parse disk designators, like "C:\". If Relative == true, also accepts: "C:".
 // Returns number of characters considered if successful.
 static size_t ParseDrive(const std::string &FileName, const size_t Offset,
                          bool Relative = true) {
   if (Offset + 1 >= FileName.size() || FileName[Offset + 1] != ':')

compiler-rt/test/fuzzer/fuzzer-dirs.test

 RUN: cat %t/SUB1/f64 %t/SUB1/f64 %t/SUB1/f64 %t/SUB1/f64 > %t/SUB1/f256
 RUN: cat %t/SUB1/f256 %t/SUB1/f256 %t/SUB1/f256 %t/SUB1/f256 > %t/SUB1/f1024
 RUN: cat %t/SUB1/f1024 %t/SUB1/f1024 %t/SUB1/f1024 %t/SUB1/f1024 > %t/SUB1/f4096
 RUN: cat %t/SUB1/f4096 %t/SUB1/f4096 > %t/SUB1/f8192
 RUN: %run %t-SimpleTest %t/SUB1 -runs=0 2>&1 | FileCheck %s --check-prefix=LONG
 LONG: INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 8192 bytes
 RUN: rm -rf %t/SUB1
 
-RUN: not %run %t-SimpleTest NONEXISTENT_DIR 2>&1 | FileCheck %s --check-prefix=NONEXISTENT_DIR
-NONEXISTENT_DIR: No such file or directory: NONEXISTENT_DIR; exiting
-
+RUN: rm -rf %t.dir && mkdir -p %t.dir
+RUN: not %run %t-SimpleTest -artifact_prefix=%t.dir/NONEXISTENT_DIR/ 2>&1 | FileCheck %s --check-prefix=NONEXISTENT_DIR_RGX
+RUN: not %run %t-SimpleTest -artifact_prefix=%t.dir/NONEXISTENT_DIR/myprefix 2>&1 | FileCheck %s --check-prefix=NONEXISTENT_DIR_RGX
+RUN: not %run %t-SimpleTest -features_dir=%t.dir/NONEXISTENT_DIR/ 2>&1 | FileCheck %s --check-prefix=NONEXISTENT_DIR_RGX
+RUN: not %run %t-SimpleTest %t.dir/NONEXISTENT_DIR 2>&1 | FileCheck %s --check-prefix=NONEXISTENT_DIR_RGX
+RUN: not %run %t-SimpleTest -exact_artifact_path=%t.dir/NONEXISTENT_DIR/myprefix 2>&1 | FileCheck %s --check-prefix=NONEXISTENT_DIR_RGX
+NONEXISTENT_DIR_RGX: ERROR: The required directory "{{.*/NONEXISTENT_DIR/?}}" does not exist