Skip to content

Commit

Permalink
fuzz: yet another attempt to deal with mkdtemp() failures. (envoyprox…
Browse files Browse the repository at this point in the history
…y#4228)

Still more failures overnight.

1. Avoid doing mkdtemp() more than once per fuzz run.

2. Make sure we pickup failures in RELEASE_ASSERT line, we were losing messages in truncated logs on
   ClusterFuzz.

3. Use C++ level std::filesystem for directory creation for each fuzz test case.

Hopefully this stabilizes or makes it clearer what is failing on ClusterFuzz.

Risk level: Low
Testing: bazel and oss-fuzz Docker server_fuzz_test.

Signed-off-by: Harvey Tuch <htuch@google.com>
  • Loading branch information
htuch authored Aug 22, 2018
1 parent 4acd243 commit ee3992d
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 26 deletions.
17 changes: 5 additions & 12 deletions test/fuzz/fuzz_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,12 @@ namespace Fuzz {

spdlog::level::level_enum Runner::log_level_;

uint32_t PerTestEnvironment::test_num_;

PerTestEnvironment::PerTestEnvironment()
: test_tmpdir_([] {
static uint32_t test_num;
const std::string fuzz_path =
TestEnvironment::temporaryPath(fmt::format("fuzz_{}.XXXXXX", test_num++));
char test_tmpdir[fuzz_path.size() + 1];
StringUtil::strlcpy(test_tmpdir, fuzz_path.data(), fuzz_path.size() + 1);
if (::mkdtemp(test_tmpdir) == nullptr) {
ENVOY_LOG_MISC(critical, "Failed to create tmpdir {} {}", fuzz_path, strerror(errno));
RELEASE_ASSERT(false, "");
}
return std::string(test_tmpdir);
}()) {}
: test_tmpdir_(TestEnvironment::temporaryPath(fmt::format("fuzz_{}", test_num_++))) {
TestEnvironment::createPath(test_tmpdir_);
}

void Runner::setupEnvironment(int argc, char** argv, spdlog::level::level_enum default_log_level) {
Event::Libevent::Global::initialize();
Expand Down
1 change: 1 addition & 0 deletions test/fuzz/fuzz_runner.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class PerTestEnvironment {
std::string temporaryPath(const std::string& path) const { return test_tmpdir_ + "/" + path; }

private:
static uint32_t test_num_;
const std::string test_tmpdir_;
};

Expand Down
40 changes: 26 additions & 14 deletions test/test_common/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,6 @@
namespace Envoy {
namespace {

// Create the parent directory of a given filesystem path.
void createParentPath(const std::string& path) {
#ifdef __APPLE__
// No support in Clang OS X libc++ today for std::filesystem.
RELEASE_ASSERT(::system(("mkdir -p $(dirname " + path + ")").c_str()) == 0, "");
#else
// We don't want to rely on mkdir etc. if we can avoid it, since it might not
// exist in some environments such as ClusterFuzz.
std::experimental::filesystem::create_directories(
std::experimental::filesystem::path(path).parent_path());
#endif
}

std::string getOrCreateUnixDomainSocketDirectory() {
const char* path = ::getenv("TEST_UDSDIR");
if (path != nullptr) {
Expand All @@ -62,7 +49,10 @@ std::string getTemporaryDirectory() {
if (::getenv("TMPDIR")) {
return TestEnvironment::getCheckedEnvVar("TMPDIR");
}
return "/tmp";
char test_tmpdir[] = "/tmp/envoy_test_tmp.XXXXXX";
RELEASE_ASSERT(::mkdtemp(test_tmpdir) != nullptr,
fmt::format("Failed to create tmpdir {} {}", test_tmpdir, strerror(errno)));
return std::string(test_tmpdir);
}

// Allow initializeOptions() to remember CLI args for getOptions().
Expand All @@ -71,6 +61,28 @@ char** argv_;

} // namespace

void TestEnvironment::createPath(const std::string& path) {
#ifdef __APPLE__
// No support in Clang OS X libc++ today for std::filesystem.
RELEASE_ASSERT(::system(("mkdir -p " + path).c_str()) == 0, "");
#else
// We don't want to rely on mkdir etc. if we can avoid it, since it might not
// exist in some environments such as ClusterFuzz.
std::experimental::filesystem::create_directories(std::experimental::filesystem::path(path));
#endif
}

void TestEnvironment::createParentPath(const std::string& path) {
#ifdef __APPLE__
// No support in Clang OS X libc++ today for std::filesystem.
RELEASE_ASSERT(::system(("mkdir -p $(dirname " + path + ")").c_str()) == 0, "");
#else
// We don't want to rely on mkdir etc. if we can avoid it, since it might not
// exist in some environments such as ClusterFuzz.
std::experimental::filesystem::create_directories(
std::experimental::filesystem::path(path).parent_path());
#endif
}
absl::optional<std::string> TestEnvironment::getOptionalEnvVar(const std::string& var) {
const char* path = ::getenv(var.c_str());
if (path == nullptr) {
Expand Down
12 changes: 12 additions & 0 deletions test/test_common/environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,5 +175,17 @@ class TestEnvironment {
* @return string the contents of the file.
*/
static std::string readFileToStringForTest(const std::string& filename);

/**
* Create a path on the filesystem (mkdir -p ... equivalent).
* @param path.
*/
static void createPath(const std::string& path);

/**
* Create a parent path on the filesystem (mkdir -p $(dirname ...) equivalent).
* @param path.
*/
static void createParentPath(const std::string& path);
};
} // namespace Envoy

0 comments on commit ee3992d

Please sign in to comment.