Avoid using fts_open and fts_read
b39ae485a46b("Link against libfts when compiling against musl") had to
introduce an additional dependency on libfts when building with musl as
libc.
Turns out that base::ListFilesRecursive() can be used instead. This has
two benefits:
* Doesn't require libfts when building with musl.
* Reduces platform dependend duplicated code.
perfetto::profiling::WalkDirectories() used a callback, while
base::ListFilesRecursive() returns a vector, so there might be a
performance difference with very large trees. If this turns out to be a
problem, base::ListFilesRecursive() can be changed to use a callback (or
return an iterator instead).
Bug: 200136875
Change-Id: I420e7b0ad7fe10666da9c39baf1ca046649588fd
diff --git a/src/profiling/symbolizer/filesystem.h b/src/profiling/symbolizer/filesystem.h
index d9cf87c..9d11233 100644
--- a/src/profiling/symbolizer/filesystem.h
+++ b/src/profiling/symbolizer/filesystem.h
@@ -22,9 +22,7 @@
namespace perfetto {
namespace profiling {
-using FileCallback = std::function<void(const char*, size_t)>;
size_t GetFileSize(const std::string& file_path);
-bool WalkDirectories(std::vector<std::string> dirs, FileCallback fn);
} // namespace profiling
} // namespace perfetto
diff --git a/src/profiling/symbolizer/filesystem_posix.cc b/src/profiling/symbolizer/filesystem_posix.cc
index cc983e2..311f444 100644
--- a/src/profiling/symbolizer/filesystem_posix.cc
+++ b/src/profiling/symbolizer/filesystem_posix.cc
@@ -20,7 +20,6 @@
#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
#if PERFETTO_BUILDFLAG(PERFETTO_LOCAL_SYMBOLIZER)
-#include <fts.h>
#include <sys/stat.h>
#endif
@@ -31,26 +30,6 @@
namespace perfetto {
namespace profiling {
#if PERFETTO_BUILDFLAG(PERFETTO_LOCAL_SYMBOLIZER)
-bool WalkDirectories(std::vector<std::string> dirs, FileCallback fn) {
- std::vector<char*> dir_cstrs;
- dir_cstrs.reserve(dirs.size());
- for (std::string& dir : dirs)
- dir_cstrs.emplace_back(&dir[0]);
- dir_cstrs.push_back(nullptr);
- base::ScopedResource<FTS*, fts_close, nullptr> fts(
- fts_open(&dir_cstrs[0], FTS_LOGICAL | FTS_NOCHDIR, nullptr));
- if (!fts) {
- PERFETTO_PLOG("fts_open");
- return false;
- }
- FTSENT* ent;
- while ((ent = fts_read(*fts))) {
- if (ent->fts_info & FTS_F)
- fn(ent->fts_path, static_cast<size_t>(ent->fts_statp->st_size));
- }
- return true;
-}
-
size_t GetFileSize(const std::string& file_path) {
base::ScopedFile fd(base::OpenFile(file_path, O_RDONLY | O_CLOEXEC));
if (!fd) {
@@ -64,9 +43,6 @@
return static_cast<size_t>(buf.st_size);
}
#else
-bool WalkDirectories(std::vector<std::string>, FileCallback) {
- return false;
-}
size_t GetFileSize(const std::string&) {
return 0;
}
diff --git a/src/profiling/symbolizer/filesystem_windows.cc b/src/profiling/symbolizer/filesystem_windows.cc
index bb2652c..f7a381e 100644
--- a/src/profiling/symbolizer/filesystem_windows.cc
+++ b/src/profiling/symbolizer/filesystem_windows.cc
@@ -23,35 +23,6 @@
namespace perfetto {
namespace profiling {
-bool WalkDirectories(std::vector<std::string> dirs, FileCallback fn) {
- std::vector<std::string> sub_dirs;
- for (const std::string& dir : dirs) {
- WIN32_FIND_DATAA file;
- HANDLE fh = FindFirstFileA((dir + "\\*").c_str(), &file);
- if (fh != INVALID_HANDLE_VALUE) {
- do {
- std::string file_path = dir + "\\" + file.cFileName;
- if (file.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
- if (strcmp(file.cFileName, ".") != 0 &&
- strcmp(file.cFileName, "..") != 0) {
- sub_dirs.push_back(file_path);
- }
- } else {
- ULARGE_INTEGER size;
- size.HighPart = file.nFileSizeHigh;
- size.LowPart = file.nFileSizeLow;
- fn(file_path.c_str(), size.QuadPart);
- }
- } while (FindNextFileA(fh, &file));
- }
- CloseHandle(fh);
- }
- if (!sub_dirs.empty()) {
- WalkDirectories(sub_dirs, fn);
- }
- return true;
-}
-
size_t GetFileSize(const std::string& file_path) {
HANDLE file =
CreateFileA(file_path.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr,
diff --git a/src/profiling/symbolizer/local_symbolizer.cc b/src/profiling/symbolizer/local_symbolizer.cc
index 3375837..8970cca 100644
--- a/src/profiling/symbolizer/local_symbolizer.cc
+++ b/src/profiling/symbolizer/local_symbolizer.cc
@@ -217,12 +217,12 @@
uint64_t load_bias;
};
-base::Optional<BuildIdAndLoadBias> GetBuildIdAndLoadBias(const char* fname,
- size_t size) {
+base::Optional<BuildIdAndLoadBias> GetBuildIdAndLoadBias(const std::string& fname) {
+ size_t size = GetFileSize(fname);
static_assert(EI_CLASS > EI_MAG3, "mem[EI_MAG?] accesses are in range.");
if (size <= EI_CLASS)
return base::nullopt;
- ScopedReadMmap map(fname, size);
+ ScopedReadMmap map(fname.c_str(), size);
if (!map.IsValid()) {
PERFETTO_PLOG("mmap");
return base::nullopt;
@@ -252,35 +252,48 @@
return base::nullopt;
}
+bool StartsWithElfMagic(const std::string& fname) {
+ base::ScopedFile fd(base::OpenFile(fname, O_RDONLY));
+ char magic[EI_MAG3 + 1];
+ if (!fd) {
+ PERFETTO_PLOG("Failed to open %s", fname.c_str());
+ return false;
+ }
+ ssize_t rd = base::Read(*fd, &magic, sizeof(magic));
+ if (rd != sizeof(magic)) {
+ PERFETTO_PLOG("Failed to read %s", fname.c_str());
+ return false;
+ }
+ if (!IsElf(magic, static_cast<size_t>(rd))) {
+ PERFETTO_DLOG("%s not an ELF.", fname.c_str());
+ return false;
+ }
+ return true;
+}
+
std::map<std::string, FoundBinary> BuildIdIndex(std::vector<std::string> dirs) {
std::map<std::string, FoundBinary> result;
- WalkDirectories(std::move(dirs), [&result](const char* fname, size_t size) {
- char magic[EI_MAG3 + 1];
- // Scope file access. On windows OpenFile opens an exclusive lock.
- // This lock needs to be released before mapping the file.
- {
- base::ScopedFile fd(base::OpenFile(fname, O_RDONLY));
- if (!fd) {
- PERFETTO_PLOG("Failed to open %s", fname);
- return;
+ for (const std::string& dir : dirs) {
+ std::vector<std::string> files;
+ base::Status status = base::ListFilesRecursive(dir, files);
+ if (!status.ok()) {
+ PERFETTO_PLOG("Failed to list directory %s", dir.c_str());
+ continue;
+ }
+ for (const std::string& basename : files) {
+ std::string fname = dir + "/" + basename;
+ if (!StartsWithElfMagic(fname)) {
+ continue;
}
- ssize_t rd = base::Read(*fd, &magic, sizeof(magic));
- if (rd != sizeof(magic)) {
- PERFETTO_PLOG("Failed to read %s", fname);
- return;
- }
- if (!IsElf(magic, static_cast<size_t>(rd))) {
- PERFETTO_DLOG("%s not an ELF.", fname);
- return;
+ base::Optional<BuildIdAndLoadBias> build_id_and_load_bias =
+ GetBuildIdAndLoadBias(fname);
+ if (build_id_and_load_bias) {
+ result.emplace(build_id_and_load_bias->build_id,
+ FoundBinary{fname, build_id_and_load_bias->load_bias});
}
}
- base::Optional<BuildIdAndLoadBias> build_id_and_load_bias =
- GetBuildIdAndLoadBias(fname, size);
- if (build_id_and_load_bias) {
- result.emplace(build_id_and_load_bias->build_id,
- FoundBinary{fname, build_id_and_load_bias->load_bias});
- }
- });
+ }
+
return result;
}
@@ -351,15 +364,8 @@
if (!base::FileExists(symbol_file)) {
return base::nullopt;
}
- // Openfile opens the file with an exclusive lock on windows.
- size_t size = GetFileSize(symbol_file);
-
- if (size == 0) {
- return base::nullopt;
- }
-
base::Optional<BuildIdAndLoadBias> build_id_and_load_bias =
- GetBuildIdAndLoadBias(symbol_file.c_str(), size);
+ GetBuildIdAndLoadBias(symbol_file);
if (!build_id_and_load_bias)
return base::nullopt;
if (build_id_and_load_bias->build_id != build_id) {
diff --git a/src/profiling/symbolizer/local_symbolizer_unittest.cc b/src/profiling/symbolizer/local_symbolizer_unittest.cc
index d52ed69..921c878 100644
--- a/src/profiling/symbolizer/local_symbolizer_unittest.cc
+++ b/src/profiling/symbolizer/local_symbolizer_unittest.cc
@@ -140,13 +140,7 @@
return std::string(reinterpret_cast<const char*>(&e), sizeof e);
}
-#if defined(MEMORY_SANITIZER)
-// fts_read() causes some error under msan.
-#define NOMSAN_SimpleTree DISABLED_SimpleTree
-#else
-#define NOMSAN_SimpleTree SimpleTree
-#endif
-TEST(LocalBinaryIndexerTest, NOMSAN_SimpleTree) {
+TEST(LocalBinaryIndexerTest, SimpleTree) {
base::TmpDirTree tmp;
tmp.AddDir("dir1");
tmp.AddFile("dir1/elf1", CreateElfWithBuildId("AAAAAAAAAAAAAAAAAAAA"));
