From 81a68382d806e0cba066ee5b2c2b27cd00d7cb3f Mon Sep 17 00:00:00 2001
From: Wiktor Garbacz <wiktorg@google.com>
Date: Fri, 11 Dec 2020 00:47:58 -0800
Subject: [PATCH] Use file helpers in `minielf_test`

PiperOrigin-RevId: 346949861
Change-Id: Ib323a9ecd8fd8f268f09b028d13b220d3d8b60d1
---
 sandboxed_api/sandbox2/util/BUILD.bazel     |  2 +
 sandboxed_api/sandbox2/util/CMakeLists.txt  |  2 +
 sandboxed_api/sandbox2/util/minielf.h       |  4 +-
 sandboxed_api/sandbox2/util/minielf_test.cc | 47 +++++++++------------
 4 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/sandboxed_api/sandbox2/util/BUILD.bazel b/sandboxed_api/sandbox2/util/BUILD.bazel
index a608b82..72cbad0 100644
--- a/sandboxed_api/sandbox2/util/BUILD.bazel
+++ b/sandboxed_api/sandbox2/util/BUILD.bazel
@@ -157,10 +157,12 @@ cc_test(
     ],
     features = ["-dynamic_link_test_srcs"],  # see go/dynamic_link_test_srcs
     deps = [
+        ":file_helpers",
         ":maps_parser",
         ":minielf",
         "//sandboxed_api/sandbox2:testing",
         "//sandboxed_api/util:status_matchers",
+        "@com_google_absl//absl/algorithm:container",
         "@com_google_absl//absl/strings",
         "@com_google_googletest//:gtest_main",
     ],
diff --git a/sandboxed_api/sandbox2/util/CMakeLists.txt b/sandboxed_api/sandbox2/util/CMakeLists.txt
index 46af750..be22043 100644
--- a/sandboxed_api/sandbox2/util/CMakeLists.txt
+++ b/sandboxed_api/sandbox2/util/CMakeLists.txt
@@ -186,7 +186,9 @@ if(SAPI_ENABLE_TESTS)
   configure_file(testdata/chrome_grte_header
                  testdata/chrome_grte_header COPYONLY)
   target_link_libraries(minielf_test PRIVATE
+    absl::algorithm_container
     absl::strings
+    sandbox2::file_helpers
     sandbox2::maps_parser
     sandbox2::minielf
     sandbox2::testing
diff --git a/sandboxed_api/sandbox2/util/minielf.h b/sandboxed_api/sandbox2/util/minielf.h
index 027e21c..637adc3 100644
--- a/sandboxed_api/sandbox2/util/minielf.h
+++ b/sandboxed_api/sandbox2/util/minielf.h
@@ -38,8 +38,8 @@ class ElfFile {
 
   int64_t file_size() const { return file_size_; }
   const std::string& interpreter() const { return interpreter_; }
-  const std::vector<Symbol> symbols() const { return symbols_; }
-  const std::vector<std::string> imported_libraries() const {
+  const std::vector<Symbol>& symbols() const { return symbols_; }
+  const std::vector<std::string>& imported_libraries() const {
     return imported_libraries_;
   }
   bool position_independent() const { return position_independent_; }
diff --git a/sandboxed_api/sandbox2/util/minielf_test.cc b/sandboxed_api/sandbox2/util/minielf_test.cc
index db6b188..8409e42 100644
--- a/sandboxed_api/sandbox2/util/minielf_test.cc
+++ b/sandboxed_api/sandbox2/util/minielf_test.cc
@@ -15,11 +15,14 @@
 #include "sandboxed_api/sandbox2/util/minielf.h"
 
 #include <cstdint>
+#include <string>
 #include <vector>
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "absl/algorithm/container.h"
 #include "sandboxed_api/sandbox2/testing.h"
+#include "sandboxed_api/sandbox2/util/file_helpers.h"
 #include "sandboxed_api/sandbox2/util/maps_parser.h"
 #include "sandboxed_api/util/status_matchers.h"
 
@@ -30,9 +33,10 @@ extern "C" void ExportedFunctionName() {
 namespace sandbox2 {
 namespace {
 
+using ::sapi::IsOk;
 using ::testing::Eq;
 using ::testing::IsTrue;
-using ::testing::Not;
+using ::testing::Ne;
 using ::testing::StrEq;
 
 TEST(MinielfTest, Chrome70) {
@@ -51,36 +55,27 @@ TEST(MinielfTest, SymbolResolutionWorks) {
   ASSERT_THAT(elf.position_independent(), IsTrue());
 
   // Load /proc/self/maps to take ASLR into account.
-  char maps_buffer[1024 * 1024]{};
-  FILE *f = fopen("/proc/self/maps", "r");
-  ASSERT_THAT(f, Not(Eq(nullptr)));
-  fread(maps_buffer, 1, sizeof(maps_buffer), f);
-  fclose(f);
-
+  std::string maps_buffer;
+  ASSERT_THAT(sandbox2::file::GetContents("/proc/self/maps", &maps_buffer,
+                                          sandbox2::file::Defaults()),
+              IsOk());
   SAPI_ASSERT_OK_AND_ASSIGN(std::vector<MapsEntry> maps, ParseProcMaps(maps_buffer));
 
   // Find maps entry that covers this entry.
   uint64_t function_address = reinterpret_cast<uint64_t>(ExportedFunctionName);
-  bool entry_found = false;
-  for (const auto &entry : maps) {
-    if (entry.start <= function_address && entry.end > function_address) {
-      entry_found = true;
-      function_address -= entry.start;
-      break;
-    }
-  }
-  ASSERT_THAT(entry_found, IsTrue());
+  auto function_entry =
+      absl::c_find_if(maps, [function_address](const MapsEntry& entry) {
+        return entry.start <= function_address && entry.end > function_address;
+      });
+  ASSERT_THAT(function_entry, Ne(maps.end()));
+  function_address -= function_entry->start;
 
-  uint64_t exported_function_name__symbol_value = 0;
-
-  for (const auto &s : elf.symbols()) {
-    if (s.name == "ExportedFunctionName") {
-      exported_function_name__symbol_value = s.address;
-      break;
-    }
-  }
-
-  EXPECT_THAT(exported_function_name__symbol_value, function_address);
+  auto function_symbol =
+      absl::c_find_if(elf.symbols(), [](const ElfFile::Symbol& symbol) {
+        return symbol.name == "ExportedFunctionName";
+      });
+  ASSERT_THAT(function_symbol, Ne(elf.symbols().end()));
+  EXPECT_THAT(function_symbol->address, Eq(function_address));
 }
 
 TEST(MinielfTest, ImportedLibraries) {