Revert "Deprecate SbMemoryMap APIs. (#2064)" (#2136)

This reverts commit 139da0d. (#2064) Broke builds on Raspi-2 b/302332972
youtube · Dec 28, 2023 · a684fb0 · a684fb0
1 parent 538dc75
commit a684fb0
Show file tree

Hide file tree

Showing 27 changed files with 133 additions and 608 deletions.
diff --git a/starboard/CHANGELOG.md b/starboard/CHANGELOG.md
@@ -21,11 +21,6 @@ The StringFormat management APIs `SbStringFormat`, `SbStringFormatF`,
 standard APIs `vsnprintf`, `vfnprintf`, `vswprintf`, `snprintf`
 from `<stdlib.h>` should be used instead.
 
-### Deprecated SbMemoryMap APIs and migrated to POSIX mmap
-The memory mapping APIs `SbMemoryMap`, `SbMemoryUnmap`, `SbMemoryProtect` and
-`SbMemoryFlush` are deprecated and the standard APIs `mmap`, `munmap`,
-`mprotect`, `msync` from `<sys/mman.h>` should be used instead.
-
 ### Deprecated SbMemory allocation APIs and migrated to POSIX memory APIs
 The memory management APIs `SbMemoryAllocate`, `SbMemoryReallocate`,
 `SbMemoryCalloc`, `SbMemoryAllocateAligned`, `SbMemoryDeallocate`,

diff --git a/starboard/elf_loader/exported_symbols.cc b/starboard/elf_loader/exported_symbols.cc
@@ -15,7 +15,6 @@
 #include "starboard/elf_loader/exported_symbols.h"
 
 #include <stdlib.h>
-#include <sys/mman.h>
 #include <time.h>
 
 #include "starboard/accessibility.h"
@@ -202,32 +201,31 @@ ExportedSymbols::ExportedSymbols() {
   REGISTER_SYMBOL(SbMemoryDeallocate);
   REGISTER_SYMBOL(SbMemoryDeallocateAligned);
   REGISTER_SYMBOL(SbMemoryDeallocateNoReport);
+#endif  // SB_API_VERSION < 16
 #if SB_CAN(MAP_EXECUTABLE_MEMORY)
   REGISTER_SYMBOL(SbMemoryFlush);
 #endif  // SB_CAN(MAP_EXECUTABLE_MEMORY)
+
+#if SB_API_VERSION < 16
   REGISTER_SYMBOL(SbMemoryFree);
   REGISTER_SYMBOL(SbMemoryFreeAligned);
 #endif  // SB_API_VERSION < 16
 
 #if SB_API_VERSION < 15
   REGISTER_SYMBOL(SbMemoryGetStackBounds);
 #endif
-
-#if SB_API_VERSION < 16
   REGISTER_SYMBOL(SbMemoryMap);
   REGISTER_SYMBOL(SbMemoryProtect);
+
+#if SB_API_VERSION < 16
   REGISTER_SYMBOL(SbMemoryReallocate);
   REGISTER_SYMBOL(SbMemoryReallocateUnchecked);
 #endif  // SB_API_VERSION < 16
 
 #if SB_API_VERSION < 15
   REGISTER_SYMBOL(SbMemorySetReporter);
 #endif
-
-#if SB_API_VERSION < 16
   REGISTER_SYMBOL(SbMemoryUnmap);
-#endif  // SB_API_VERSION < 16
-
   REGISTER_SYMBOL(SbMicrophoneClose);
   REGISTER_SYMBOL(SbMicrophoneCreate);
   REGISTER_SYMBOL(SbMicrophoneDestroy);
@@ -416,10 +414,6 @@ ExportedSymbols::ExportedSymbols() {
   REGISTER_SYMBOL(free);
   REGISTER_SYMBOL(vsscanf);
   REGISTER_SYMBOL(time);
-  REGISTER_SYMBOL(mmap);
-  REGISTER_SYMBOL(mprotect);
-  REGISTER_SYMBOL(munmap);
-  REGISTER_SYMBOL(msync);
 
   // Custom mapped POSIX APIs to compatibility wrappers.
   // These will rely on Starboard-side implementations that properly translate

diff --git a/starboard/elf_loader/program_table.cc b/starboard/elf_loader/program_table.cc
@@ -14,8 +14,6 @@
 
 #include "starboard/elf_loader/program_table.h"
 
-#include <sys/mman.h>
-
 #include "starboard/common/log.h"
 #include "starboard/common/string.h"
 #include "starboard/elf_loader/evergreen_info.h"
@@ -26,12 +24,14 @@
 #define MAYBE_MAP_FLAG(x, from, to) (((x) & (from)) ? (to) : 0)
 
 #if SB_CAN(MAP_EXECUTABLE_MEMORY)
-#define PFLAGS_TO_PROT(x)                 \
-  (MAYBE_MAP_FLAG((x), PF_X, PROT_EXEC) | \
-   MAYBE_MAP_FLAG((x), PF_R, PROT_READ) | \
-   MAYBE_MAP_FLAG((x), PF_W, PROT_WRITE))
+#define PFLAGS_TO_PROT(x)                               \
+  (MAYBE_MAP_FLAG((x), PF_X, kSbMemoryMapProtectExec) | \
+   MAYBE_MAP_FLAG((x), PF_R, kSbMemoryMapProtectRead) | \
+   MAYBE_MAP_FLAG((x), PF_W, kSbMemoryMapProtectWrite))
 #endif
 
+#define MAP_FAILED ((void*)-1)
+
 namespace starboard {
 namespace elf_loader {
 
@@ -92,7 +92,7 @@ bool ProgramTable::LoadProgramHeader(const Ehdr* elf_header, File* elf_file) {
     SB_DLOG(INFO) << "Allocated address=" << phdr_mmap_;
   } else {
     phdr_mmap_ =
-        mmap(nullptr, phdr_size_, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
+        SbMemoryMap(phdr_size_, kSbMemoryMapProtectWrite, "program_header");
     if (!phdr_mmap_) {
       SB_LOG(ERROR) << "Failed to allocate memory";
       return false;
@@ -105,7 +105,8 @@ bool ProgramTable::LoadProgramHeader(const Ehdr* elf_header, File* elf_file) {
                     << page_min;
       return false;
     }
-    bool mp_result = mprotect(phdr_mmap_, phdr_size_, PROT_READ) == 0;
+    bool mp_result =
+        SbMemoryProtect(phdr_mmap_, phdr_size_, kSbMemoryMapProtectRead);
     SB_DLOG(INFO) << "mp_result=" << mp_result;
     if (!mp_result) {
       SB_LOG(ERROR) << "Failed to protect program header";
@@ -204,7 +205,8 @@ bool ProgramTable::LoadSegments(File* elf_file) {
         }
       } else {
         SB_DLOG(INFO) << "Not using Memory Mapped Files";
-        mp_ret = mprotect(seg_addr, file_length, PROT_WRITE) == 0;
+        mp_ret =
+            SbMemoryProtect(seg_addr, file_length, kSbMemoryMapProtectWrite);
         SB_DLOG(INFO) << "segment vaddress=" << seg_addr;
 
         if (!mp_ret) {
@@ -219,7 +221,7 @@ bool ProgramTable::LoadSegments(File* elf_file) {
           return false;
         }
       }
-      mp_ret = mprotect(seg_addr, file_length, prot_flags) == 0;
+      mp_ret = SbMemoryProtect(seg_addr, file_length, prot_flags);
       SB_DLOG(INFO) << "mp_ret=" << mp_ret;
       if (!mp_ret) {
         SB_LOG(ERROR) << "Failed to protect segment";
@@ -249,9 +251,9 @@ bool ProgramTable::LoadSegments(File* elf_file) {
     // map for all extra pages.
     if (seg_page_end > seg_file_end) {
 #if SB_CAN(MAP_EXECUTABLE_MEMORY)
-      bool mprotect_fix =
-          mprotect(reinterpret_cast<void*>(seg_file_end),
-                   seg_page_end - seg_file_end, PROT_WRITE) == 0;
+      bool mprotect_fix = SbMemoryProtect(reinterpret_cast<void*>(seg_file_end),
+                                          seg_page_end - seg_file_end,
+                                          kSbMemoryMapProtectWrite);
       SB_DLOG(INFO) << "mprotect_fix=" << mprotect_fix;
       if (!mprotect_fix) {
         SB_LOG(ERROR) << "Failed to unprotect end of segment";
@@ -264,8 +266,9 @@ bool ProgramTable::LoadSegments(File* elf_file) {
       memset(reinterpret_cast<void*>(seg_file_end), 0,
              seg_page_end - seg_file_end);
 #if SB_CAN(MAP_EXECUTABLE_MEMORY)
-      mprotect(reinterpret_cast<void*>(seg_file_end),
-               seg_page_end - seg_file_end, PFLAGS_TO_PROT(phdr->p_flags));
+      SbMemoryProtect(reinterpret_cast<void*>(seg_file_end),
+                      seg_page_end - seg_file_end,
+                      PFLAGS_TO_PROT(phdr->p_flags));
       SB_DLOG(INFO) << "mprotect_fix=" << mprotect_fix;
       if (!mprotect_fix) {
         SB_LOG(ERROR) << "Failed to protect end of segment";
@@ -357,9 +360,9 @@ int ProgramTable::AdjustMemoryProtectionOfReadOnlySegments(
     Addr seg_page_end =
         PAGE_END(phdr->p_vaddr + phdr->p_memsz) + base_memory_address_;
 #if SB_CAN(MAP_EXECUTABLE_MEMORY)
-    int ret = mprotect(reinterpret_cast<void*>(seg_page_start),
-                       seg_page_end - seg_page_start,
-                       PFLAGS_TO_PROT(phdr->p_flags) | extra_prot_flags);
+    int ret = SbMemoryProtect(reinterpret_cast<void*>(seg_page_start),
+                              seg_page_end - seg_page_start,
+                              PFLAGS_TO_PROT(phdr->p_flags) | extra_prot_flags);
     if (ret < 0) {
       return -1;
     }
@@ -380,7 +383,7 @@ bool ProgramTable::ReserveLoadMemory() {
   SB_DLOG(INFO) << "Load size=" << load_size_;
 
   load_start_ =
-      mmap(nullptr, load_size_, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0);
+      SbMemoryMap(load_size_, kSbMemoryMapProtectReserved, "reserved_mem");
   if (load_start_ == MAP_FAILED) {
     SB_LOG(ERROR) << "Could not reserve " << load_size_
                   << " bytes of address space";
@@ -418,10 +421,10 @@ Addr ProgramTable::GetBaseMemoryAddress() {
 ProgramTable::~ProgramTable() {
   SetEvergreenInfo(NULL);
   if (load_start_) {
-    munmap(load_start_, load_size_);
+    SbMemoryUnmap(load_start_, load_size_);
   }
   if (phdr_mmap_) {
-    munmap(phdr_mmap_, phdr_size_);
+    SbMemoryUnmap(phdr_mmap_, phdr_size_);
   }
 }
 

diff --git a/starboard/memory.h b/starboard/memory.h
@@ -50,8 +50,8 @@
 extern "C" {
 #endif
 
-// TODO: Remove the definition once the memory_mapped_file.h extension
-// is deprecated.
+#define SB_MEMORY_MAP_FAILED ((void*)-1)  // NOLINT(readability/casting)
+
 // The bitwise OR of these flags should be passed to SbMemoryMap to indicate
 // how the mapped memory can be used.
 typedef enum SbMemoryMapFlags {
@@ -68,9 +68,6 @@ typedef enum SbMemoryMapFlags {
 } SbMemoryMapFlags;
 
 #if SB_API_VERSION < 16
-
-#define SB_MEMORY_MAP_FAILED ((void*)-1)  // NOLINT(readability/casting)
-
 static SB_C_FORCE_INLINE void SbAbortIfAllocationFailed(size_t requested_bytes,
                                                         void* address) {
   if (SB_UNLIKELY(requested_bytes > 0 && address == NULL)) {
@@ -181,6 +178,8 @@ SB_DEPRECATED_EXTERNAL(SB_EXPORT void SbMemoryFree(void* memory));
 // DO NOT CALL. Call SbMemoryDeallocateAligned(...) instead.
 SB_DEPRECATED_EXTERNAL(SB_EXPORT void SbMemoryFreeAligned(void* memory));
 
+#endif  // SB_API_VERSION < 16
+
 // Allocates |size_bytes| worth of physical memory pages and maps them into
 // an available virtual region. This function returns |SB_MEMORY_MAP_FAILED|
 // on failure. |NULL| is a valid return value.
@@ -220,7 +219,6 @@ SB_EXPORT bool SbMemoryProtect(void* virtual_address,
 // memory that has been written to and might be executed in the future.
 SB_EXPORT void SbMemoryFlush(void* virtual_address, int64_t size_bytes);
 #endif
-#endif  // SB_API_VERSION < 16
 
 #if SB_API_VERSION < 15
 

diff --git a/starboard/nplb/BUILD.gn b/starboard/nplb/BUILD.gn
@@ -134,7 +134,6 @@ target(gtest_target_type, "nplb") {
     "player_test_util.cc",
     "player_test_util.h",
     "player_write_sample_test.cc",
-    "posix_compliance/posix_memory_map_test.cc",
     "random_helpers.cc",
     "recursive_mutex_test.cc",
     "rwlock_test.cc",

diff --git a/starboard/nplb/memory_map_test.cc b/starboard/nplb/memory_map_test.cc
@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#if SB_API_VERSION < 16
-
 #include <algorithm>
 
 #include "starboard/common/memory.h"
@@ -296,5 +294,3 @@ TEST(SbMemoryMapTest, CanChangeMemoryProtection) {
 }  // namespace
 }  // namespace nplb
 }  // namespace starboard
-
-#endif  // SB_API_VERSION < 16
diff --git a/starboard/nplb/nplb_evergreen_compat_tests/executable_memory_test.cc b/starboard/nplb/nplb_evergreen_compat_tests/executable_memory_test.cc
@@ -12,8 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include <sys/mman.h>
-
+#include "starboard/memory.h"
 #include "starboard/nplb/nplb_evergreen_compat_tests/checks.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -36,11 +35,12 @@ class ExecutableMemoryTest : public ::testing::Test {
 
 TEST_F(ExecutableMemoryTest, VerifyMemoryProtection) {
   void* memory =
-      mmap(nullptr, kSize, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
-  ASSERT_NE(MAP_FAILED, memory);
+      SbMemoryMap(kSize, kSbMemoryMapProtectWrite, "evergreen_buffer");
+  ASSERT_NE(SB_MEMORY_MAP_FAILED, memory);
   memset(memory, 0, kSize);
-  ASSERT_EQ(mprotect(memory, kSmallerSize, PROT_READ | PROT_WRITE), 0);
-  munmap(memory, kSize);
+  ASSERT_TRUE(SbMemoryProtect(
+      memory, kSmallerSize, kSbMemoryMapProtectRead | kSbMemoryMapProtectExec));
+  SbMemoryUnmap(memory, kSize);
   EXPECT_TRUE(true);
 }