Refactor MAP_EXECUTABLE_MEMORY to runtime setting (#2956)

Change build-time config SB_CAN_MAP_EXECUTABLE_MEMORY to run-time configuration constant. b/142124156
youtube · Apr 18, 2024 · 69937fe · 69937fe
1 parent 941a842
commit 69937fe
Show file tree

Hide file tree

Showing 36 changed files with 120 additions and 117 deletions.
diff --git a/cobalt/site/docs/reference/starboard/configuration-public.md b/cobalt/site/docs/reference/starboard/configuration-public.md
@@ -3,13 +3,6 @@ Book: /youtube/cobalt/_book.yaml
 
 # Starboard Configuration Reference Guide
 
-## Memory Configuration
-
-| Properties |
-| :--- |
-| **`SB_CAN_MAP_EXECUTABLE_MEMORY`**<br><br>Whether this platform can map executable memory. Implies SB_HAS_MMAP. This is required for platforms that want to JIT.<br><br>The default value in the Stub implementation is `1` |
-
-
 ## Network Configuration
 
 | Properties |

diff --git a/starboard/CHANGELOG.md b/starboard/CHANGELOG.md
@@ -9,6 +9,10 @@ since the version previous to it.
 
 ## Version 16
 
+### MAP_EXECUTABLE_MEMORY changed from build-time to runtime config
+SB_CAN_MAP_EXECUTABLE_MEMORY has been refactored into a run-time configuration
+constant `kSbCanMapExecutableMemory`.
+
 ### Deprecated `SbThreadYield`
 Replaced the `SbThreadYield` with the POSIX sched_yield() defined in the
 `<sched.h>` header.

diff --git a/starboard/android/shared/configuration_constants.cc b/starboard/android/shared/configuration_constants.cc
@@ -137,3 +137,9 @@ const uint32_t kSbUserMaxSignedIn = 1;
 // The maximum size the cache directory is allowed to use in bytes.
 const uint32_t kSbMaxSystemPathCacheDirectorySize = 24 << 20;  // 24MiB
 #endif
+
+#if SB_API_VERSION >= 16
+// Whether this platform can map executable memory. This is required for
+// platforms that want to JIT.
+SB_EXPORT extern const bool kSbCanMapExecutableMemory = true;
+#endif
diff --git a/starboard/android/shared/configuration_public.h b/starboard/android/shared/configuration_public.h
@@ -67,14 +67,14 @@
 // the current linking unit.
 #define SB_IMPORT_PLATFORM
 
-#endif  // SB_API_VERSION < 16
-
 // --- Memory Configuration --------------------------------------------------
 
 // Whether this platform can map executable memory. Implies the platform can map
 // memory. This is required for platforms that want to JIT.
 #define SB_CAN_MAP_EXECUTABLE_MEMORY 1
 
+#endif  // SB_API_VERSION < 16
+
 // --- Network Configuration -------------------------------------------------
 
 // Specifies whether this platform supports IPV6.

diff --git a/starboard/configuration_constants.h b/starboard/configuration_constants.h
@@ -153,4 +153,10 @@ SB_EXPORT extern const uint32_t kSbUserMaxSignedIn;
 SB_EXPORT extern const uint32_t kSbMaxSystemPathCacheDirectorySize;
 #endif
 
+#if SB_API_VERSION >= 16
+// Whether this platform can map executable memory. This is required for
+// platforms that want to JIT.
+SB_EXPORT extern const bool kSbCanMapExecutableMemory;
+#endif
+
 #endif  // STARBOARD_CONFIGURATION_CONSTANTS_H_
diff --git a/starboard/doc/evergreen/cobalt_evergreen_overview.md b/starboard/doc/evergreen/cobalt_evergreen_overview.md
@@ -144,9 +144,9 @@ Evergreen:
         for more details.
 *   `kSbMemoryMapProtectExec`
     *   Ensures mapped memory can be executed
-*   `#define SB_CAN_MAP_EXECUTABLE_MEMORY 1`
+*   Set `kSbCanMapExecutableMemory` to `true`
     *   Specifies that the platform can map executable memory
-    *   Defined in `configuration_public.h`
+    *   Defined in `configuration_constants.h`
 
 Only if necessary, create a customized SABI configuration for your architecture.
 Note, we do not anticipate that you will need to make a new configuration for

diff --git a/starboard/elf_loader/dynamic_section_test.cc b/starboard/elf_loader/dynamic_section_test.cc
@@ -18,8 +18,6 @@
 #include "starboard/string.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if SB_CAN(MAP_EXECUTABLE_MEMORY)
-
 namespace starboard {
 namespace elf_loader {
 
@@ -201,4 +199,3 @@ TEST_F(DynamicSectionTest, LookupNameById) {
 }  // namespace
 }  // namespace elf_loader
 }  // namespace starboard
-#endif  // SB_CAN(MAP_EXECUTABLE_MEMORY)
diff --git a/starboard/elf_loader/elf_header_test.cc b/starboard/elf_loader/elf_header_test.cc
@@ -21,7 +21,6 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if SB_CAN(MAP_EXECUTABLE_MEMORY)
 namespace starboard {
 namespace elf_loader {
 
@@ -132,4 +131,3 @@ TEST_F(ElfHeaderTest, NegativeBadMachine) {
 }  // namespace
 }  // namespace elf_loader
 }  // namespace starboard
-#endif  // SB_CAN(MAP_EXECUTABLE_MEMORY)
diff --git a/starboard/elf_loader/elf_loader_impl.cc b/starboard/elf_loader/elf_loader_impl.cc
@@ -44,10 +44,15 @@ bool EndsWith(const std::string& s, const std::string& suffix) {
 }  // namespace
 
 ElfLoaderImpl::ElfLoaderImpl() {
+#if SB_API_VERSION >= 16
+  SB_CHECK(kSbCanMapExecutableMemory)
+      << "Elf_loader requires executable memory support!";
+#else
 #if !SB_CAN(MAP_EXECUTABLE_MEMORY)
   SB_CHECK(false) << "The elf_loader requires "
                      "executable memory map support!";
 #endif
+#endif
 }
 
 bool ElfLoaderImpl::Load(const char* name,

diff --git a/starboard/elf_loader/elf_loader_test.cc b/starboard/elf_loader/elf_loader_test.cc
@@ -17,7 +17,6 @@
 #include "starboard/common/scoped_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if SB_CAN(MAP_EXECUTABLE_MEMORY)
 namespace starboard {
 namespace elf_loader {
 
@@ -37,4 +36,3 @@ TEST_F(ElfLoaderTest, Initialize) {
 }  // namespace
 }  // namespace elf_loader
 }  // namespace starboard
-#endif  // SB_CAN(MAP_EXECUTABLE_MEMORY)
diff --git a/starboard/elf_loader/exported_symbols.cc b/starboard/elf_loader/exported_symbols.cc
@@ -114,6 +114,9 @@ ExportedSymbols::ExportedSymbols() {
 #if SB_API_VERSION < 16
   REGISTER_SYMBOL(kSbUserMaxSignedIn);
 #endif  // SB_API_VERSION < 16
+#if SB_API_VERSION >= 16
+  REGISTER_SYMBOL(kSbCanMapExecutableMemory);
+#endif
   REGISTER_SYMBOL(SbAccessibilityGetCaptionSettings);
   REGISTER_SYMBOL(SbAccessibilityGetDisplaySettings);
   REGISTER_SYMBOL(SbAccessibilityGetTextToSpeechSettings);

diff --git a/starboard/elf_loader/program_table.cc b/starboard/elf_loader/program_table.cc
@@ -25,12 +25,10 @@
 
 #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))
-#endif
 
 namespace starboard {
 namespace elf_loader {
@@ -44,7 +42,17 @@ ProgramTable::ProgramTable(
       load_start_(NULL),
       load_size_(0),
       base_memory_address_(0),
-      memory_mapped_file_extension_(memory_mapped_file_extension) {}
+      memory_mapped_file_extension_(memory_mapped_file_extension) {
+#if SB_API_VERSION >= 16
+  SB_CHECK(kSbCanMapExecutableMemory)
+      << "This module requires executable memory support!";
+#else
+#if !SB_CAN(MAP_EXECUTABLE_MEMORY)
+  SB_CHECK(false) << "This module requires "
+                     "executable memory map support!";
+#endif
+#endif
+}
 
 bool ProgramTable::LoadProgramHeader(const Ehdr* elf_header, File* elf_file) {
   if (!elf_header) {
@@ -181,12 +189,10 @@ bool ProgramTable::LoadSegments(File* elf_file) {
     Addr file_page_start = PAGE_START(file_start);
     Addr file_length = file_end - file_page_start;
 
-    SB_DLOG(INFO) << "Mapping segment: "
-                  << " file_page_start=" << file_page_start
-                  << " file_length=" << file_length << " seg_page_start=0x"
-                  << std::hex << seg_page_start;
+    SB_DLOG(INFO) << "Mapping segment: " << " file_page_start="
+                  << file_page_start << " file_length=" << file_length
+                  << " seg_page_start=0x" << std::hex << seg_page_start;
 
-#if SB_CAN(MAP_EXECUTABLE_MEMORY)
     if (file_length != 0) {
       const int prot_flags = PFLAGS_TO_PROT(phdr->p_flags);
       SB_DLOG(INFO) << "segment prot_flags=" << std::hex << prot_flags;
@@ -230,9 +236,6 @@ bool ProgramTable::LoadSegments(File* elf_file) {
         return false;
       }
     }
-#else
-    SB_CHECK(false);
-#endif
 
     // if the segment is writable, and does not end on a page boundary,
     // zero-fill it until the page limit.
@@ -248,7 +251,6 @@ bool ProgramTable::LoadSegments(File* elf_file) {
     // between them. This is done by using a private anonymous
     // 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;
@@ -257,23 +259,16 @@ bool ProgramTable::LoadSegments(File* elf_file) {
         SB_LOG(ERROR) << "Failed to unprotect end of segment";
         return false;
       }
-#else
-      SB_CHECK(false);
-#endif
 
       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));
       SB_DLOG(INFO) << "mprotect_fix=" << mprotect_fix;
       if (!mprotect_fix) {
         SB_LOG(ERROR) << "Failed to protect end of segment";
         return false;
       }
-#else
-      SB_CHECK(false);
-#endif
     }
   }
   return true;
@@ -356,16 +351,12 @@ int ProgramTable::AdjustMemoryProtectionOfReadOnlySegments(
     Addr seg_page_start = PAGE_START(phdr->p_vaddr) + base_memory_address_;
     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);
     if (ret < 0) {
       return -1;
     }
-#else
-    SB_CHECK(false);
-#endif
   }
   return 0;
 }

diff --git a/starboard/elf_loader/program_table_test.cc b/starboard/elf_loader/program_table_test.cc
@@ -22,7 +22,6 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if SB_CAN(MAP_EXECUTABLE_MEMORY)
 namespace starboard {
 namespace elf_loader {
 
@@ -189,4 +188,3 @@ TEST_F(ProgramTableTest, LoadSegments) {
 }  // namespace
 }  // namespace elf_loader
 }  // namespace starboard
-#endif  // SB_CAN(MAP_EXECUTABLE_MEMORY)
diff --git a/starboard/elf_loader/relocations_test.cc b/starboard/elf_loader/relocations_test.cc
@@ -20,7 +20,6 @@
 #include "starboard/string.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if SB_CAN(MAP_EXECUTABLE_MEMORY)
 namespace starboard {
 namespace elf_loader {
 
@@ -382,4 +381,3 @@ TEST_F(RelocationsTest, R_X86_64_PC32) {
 }  // namespace
 }  // namespace elf_loader
 }  // namespace starboard
-#endif  // SB_CAN(MAP_EXECUTABLE_MEMORY)
diff --git a/starboard/evergreen/arm/hardfp/configuration_public.h b/starboard/evergreen/arm/hardfp/configuration_public.h
@@ -65,14 +65,14 @@
 // the current linking unit.
 #define SB_IMPORT_PLATFORM
 
-#endif  // SB_API_VERSION < 16
-
 // --- Memory Configuration --------------------------------------------------
 
 // Whether this platform can map executable memory. Implies SB_HAS_MMAP. This is
 // required for platforms that want to JIT.
 #define SB_CAN_MAP_EXECUTABLE_MEMORY 1
 
+#endif  // SB_API_VERSION < 16
+
 // --- Network Configuration -------------------------------------------------
 
 // Specifies whether this platform supports IPV6.

diff --git a/starboard/evergreen/arm/softfp/configuration_public.h b/starboard/evergreen/arm/softfp/configuration_public.h
@@ -65,14 +65,14 @@
 // the current linking unit.
 #define SB_IMPORT_PLATFORM
 
-#endif  // SB_API_VERSION < 16
-
 // --- Memory Configuration --------------------------------------------------
 
 // Whether this platform can map executable memory. Implies SB_HAS_MMAP. This is
 // required for platforms that want to JIT.
 #define SB_CAN_MAP_EXECUTABLE_MEMORY 1
 
+#endif  // SB_API_VERSION < 16
+
 // --- Network Configuration -------------------------------------------------
 
 // Specifies whether this platform supports IPV6.

diff --git a/starboard/evergreen/arm64/configuration_public.h b/starboard/evergreen/arm64/configuration_public.h
@@ -65,14 +65,14 @@
 // the current linking unit.
 #define SB_IMPORT_PLATFORM
 
-#endif  // SB_API_VERSION < 16
-
 // --- Memory Configuration --------------------------------------------------
 
 // Whether this platform can map executable memory. Implies SB_HAS_MMAP. This is
 // required for platforms that want to JIT.
 #define SB_CAN_MAP_EXECUTABLE_MEMORY 1
 
+#endif  // SB_API_VERSION < 16
+
 // --- Network Configuration -------------------------------------------------
 
 // Specifies whether this platform supports IPV6.

diff --git a/starboard/evergreen/x64/configuration_public.h b/starboard/evergreen/x64/configuration_public.h
@@ -65,14 +65,14 @@
 // the current linking unit.
 #define SB_IMPORT_PLATFORM
 
-#endif  // SB_API_VERSION < 16
-
 // --- Memory Configuration --------------------------------------------------
 
 // Whether this platform can map executable memory. Implies SB_HAS_MMAP. This is
 // required for platforms that want to JIT.
 #define SB_CAN_MAP_EXECUTABLE_MEMORY 1
 
+#endif  // SB_API_VERSION < 16
+
 // --- Network Configuration -------------------------------------------------
 
 // Specifies whether this platform supports IPV6.

diff --git a/starboard/linux/shared/configuration_constants.cc b/starboard/linux/shared/configuration_constants.cc
@@ -136,3 +136,7 @@ const uint32_t kSbUserMaxSignedIn = 1;
 // The maximum size the cache directory is allowed to use in bytes.
 const uint32_t kSbMaxSystemPathCacheDirectorySize = 24 << 20;  // 24MiB
 #endif
+
+#if SB_API_VERSION >= 16
+SB_EXPORT extern const bool kSbCanMapExecutableMemory = true;
+#endif
diff --git a/starboard/linux/shared/configuration_public.h b/starboard/linux/shared/configuration_public.h
@@ -67,14 +67,14 @@
 // the current linking unit.
 #define SB_IMPORT_PLATFORM
 
-#endif  // SB_API_VERSION < 16
-
-// --- I/O Configuration -----------------------------------------------------
+// --- Memory Configuration --------------------------------------------------
 
 // Whether this platform can map executable memory. Implies SB_HAS_MMAP. This is
 // required for platforms that want to JIT.
 #define SB_CAN_MAP_EXECUTABLE_MEMORY 1
 
+#endif  // SB_API_VERSION < 16
+
 // --- Network Configuration -------------------------------------------------
 
 // Specifies whether this platform supports IPV6.

diff --git a/starboard/memory.h b/starboard/memory.h
@@ -60,9 +60,7 @@ typedef enum SbMemoryMapFlags {
   kSbMemoryMapProtectReserved = 0,
   kSbMemoryMapProtectRead = 1 << 0,   // Mapped memory can be read.
   kSbMemoryMapProtectWrite = 1 << 1,  // Mapped memory can be written to.
-#if SB_CAN(MAP_EXECUTABLE_MEMORY)
-  kSbMemoryMapProtectExec = 1 << 2,  // Mapped memory can be executed.
-#endif
+  kSbMemoryMapProtectExec = 1 << 2,   // Mapped memory can be executed.
   kSbMemoryMapProtectReadWrite =
       kSbMemoryMapProtectRead | kSbMemoryMapProtectWrite,
 } SbMemoryMapFlags;

diff --git a/starboard/nplb/nplb_evergreen_compat_tests/checks.h b/starboard/nplb/nplb_evergreen_compat_tests/checks.h
@@ -19,9 +19,11 @@
 
 #if SB_IS(EVERGREEN_COMPATIBLE)
 
+#if SB_API_VERSION < 16
 #if !SB_CAN(MAP_EXECUTABLE_MEMORY)
 #error "Evergreen requires executable memory support!"
 #endif
+#endif
 
 #endif  // SB_IS(EVERGREEN_COMPATIBLE)
 #endif  // STARBOARD_NPLB_NPLB_EVERGREEN_COMPAT_TESTS_CHECKS_H_