Skip to content

Commit

Permalink
Deprecate SbMemoryMap APIs. (#2064)
Browse files Browse the repository at this point in the history
- Deprecated SbMemoryMap, SbMemoryUnmap, SbMemoryProtect and
SbMemoryFlush.

b/302332972

Change-Id: I198963e11db32c14ecc35096691da00d6d690ddd
  • Loading branch information
y4vor authored Dec 19, 2023
1 parent ebd0e67 commit 139da0d
Show file tree
Hide file tree
Showing 27 changed files with 609 additions and 133 deletions.
5 changes: 5 additions & 0 deletions starboard/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ since the version previous to it.

## Version 16

### 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`,
Expand Down
17 changes: 12 additions & 5 deletions starboard/elf_loader/exported_symbols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "starboard/elf_loader/exported_symbols.h"

#include <stdlib.h>
#include <sys/mman.h>
#include <time.h>

#include "starboard/accessibility.h"
Expand Down Expand Up @@ -201,31 +202,32 @@ 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
REGISTER_SYMBOL(SbMemoryMap);
REGISTER_SYMBOL(SbMemoryProtect);

#if SB_API_VERSION < 16
REGISTER_SYMBOL(SbMemoryMap);
REGISTER_SYMBOL(SbMemoryProtect);
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);
Expand Down Expand Up @@ -412,6 +414,10 @@ 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
Expand All @@ -420,6 +426,7 @@ ExportedSymbols::ExportedSymbols() {
// TODO: b/316603042 - Detect via NPLB and only add the wrapper if needed.
map_["clock_gettime"] = reinterpret_cast<const void*>(&__wrap_clock_gettime);
map_["gettimeofday"] = reinterpret_cast<const void*>(&__wrap_gettimeofday);

#endif // SB_API_VERSION >= 16

} // NOLINT
Expand Down
45 changes: 21 additions & 24 deletions starboard/elf_loader/program_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#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"
Expand All @@ -24,14 +26,12 @@
#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, kSbMemoryMapProtectExec) | \
MAYBE_MAP_FLAG((x), PF_R, kSbMemoryMapProtectRead) | \
MAYBE_MAP_FLAG((x), PF_W, kSbMemoryMapProtectWrite))
#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

#define MAP_FAILED ((void*)-1)

namespace starboard {
namespace elf_loader {

Expand Down Expand Up @@ -92,7 +92,7 @@ bool ProgramTable::LoadProgramHeader(const Ehdr* elf_header, File* elf_file) {
SB_DLOG(INFO) << "Allocated address=" << phdr_mmap_;
} else {
phdr_mmap_ =
SbMemoryMap(phdr_size_, kSbMemoryMapProtectWrite, "program_header");
mmap(nullptr, phdr_size_, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
if (!phdr_mmap_) {
SB_LOG(ERROR) << "Failed to allocate memory";
return false;
Expand All @@ -105,8 +105,7 @@ bool ProgramTable::LoadProgramHeader(const Ehdr* elf_header, File* elf_file) {
<< page_min;
return false;
}
bool mp_result =
SbMemoryProtect(phdr_mmap_, phdr_size_, kSbMemoryMapProtectRead);
bool mp_result = mprotect(phdr_mmap_, phdr_size_, PROT_READ) == 0;
SB_DLOG(INFO) << "mp_result=" << mp_result;
if (!mp_result) {
SB_LOG(ERROR) << "Failed to protect program header";
Expand Down Expand Up @@ -205,8 +204,7 @@ bool ProgramTable::LoadSegments(File* elf_file) {
}
} else {
SB_DLOG(INFO) << "Not using Memory Mapped Files";
mp_ret =
SbMemoryProtect(seg_addr, file_length, kSbMemoryMapProtectWrite);
mp_ret = mprotect(seg_addr, file_length, PROT_WRITE) == 0;
SB_DLOG(INFO) << "segment vaddress=" << seg_addr;

if (!mp_ret) {
Expand All @@ -221,7 +219,7 @@ bool ProgramTable::LoadSegments(File* elf_file) {
return false;
}
}
mp_ret = SbMemoryProtect(seg_addr, file_length, prot_flags);
mp_ret = mprotect(seg_addr, file_length, prot_flags) == 0;
SB_DLOG(INFO) << "mp_ret=" << mp_ret;
if (!mp_ret) {
SB_LOG(ERROR) << "Failed to protect segment";
Expand Down Expand Up @@ -251,9 +249,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 = SbMemoryProtect(reinterpret_cast<void*>(seg_file_end),
seg_page_end - seg_file_end,
kSbMemoryMapProtectWrite);
bool mprotect_fix =
mprotect(reinterpret_cast<void*>(seg_file_end),
seg_page_end - seg_file_end, PROT_WRITE) == 0;
SB_DLOG(INFO) << "mprotect_fix=" << mprotect_fix;
if (!mprotect_fix) {
SB_LOG(ERROR) << "Failed to unprotect end of segment";
Expand All @@ -266,9 +264,8 @@ 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)
SbMemoryProtect(reinterpret_cast<void*>(seg_file_end),
seg_page_end - seg_file_end,
PFLAGS_TO_PROT(phdr->p_flags));
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";
Expand Down Expand Up @@ -360,9 +357,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 = SbMemoryProtect(reinterpret_cast<void*>(seg_page_start),
seg_page_end - seg_page_start,
PFLAGS_TO_PROT(phdr->p_flags) | extra_prot_flags);
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;
}
Expand All @@ -383,7 +380,7 @@ bool ProgramTable::ReserveLoadMemory() {
SB_DLOG(INFO) << "Load size=" << load_size_;

load_start_ =
SbMemoryMap(load_size_, kSbMemoryMapProtectReserved, "reserved_mem");
mmap(nullptr, load_size_, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0);
if (load_start_ == MAP_FAILED) {
SB_LOG(ERROR) << "Could not reserve " << load_size_
<< " bytes of address space";
Expand Down Expand Up @@ -421,10 +418,10 @@ Addr ProgramTable::GetBaseMemoryAddress() {
ProgramTable::~ProgramTable() {
SetEvergreenInfo(NULL);
if (load_start_) {
SbMemoryUnmap(load_start_, load_size_);
munmap(load_start_, load_size_);
}
if (phdr_mmap_) {
SbMemoryUnmap(phdr_mmap_, phdr_size_);
munmap(phdr_mmap_, phdr_size_);
}
}

Expand Down
10 changes: 6 additions & 4 deletions starboard/memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
extern "C" {
#endif

#define SB_MEMORY_MAP_FAILED ((void*)-1) // NOLINT(readability/casting)

// TODO: Remove the definition once the memory_mapped_file.h extension
// is deprecated.
// The bitwise OR of these flags should be passed to SbMemoryMap to indicate
// how the mapped memory can be used.
typedef enum SbMemoryMapFlags {
Expand All @@ -68,6 +68,9 @@ 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)) {
Expand Down Expand Up @@ -178,8 +181,6 @@ 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.
Expand Down Expand Up @@ -219,6 +220,7 @@ 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

Expand Down
1 change: 1 addition & 0 deletions starboard/nplb/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ 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",
Expand Down
4 changes: 4 additions & 0 deletions starboard/nplb/memory_map_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// 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"
Expand Down Expand Up @@ -294,3 +296,5 @@ TEST(SbMemoryMapTest, CanChangeMemoryProtection) {
} // namespace
} // namespace nplb
} // namespace starboard

#endif // SB_API_VERSION < 16
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "starboard/memory.h"
#include <sys/mman.h>

#include "starboard/nplb/nplb_evergreen_compat_tests/checks.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand All @@ -35,12 +36,11 @@ class ExecutableMemoryTest : public ::testing::Test {

TEST_F(ExecutableMemoryTest, VerifyMemoryProtection) {
void* memory =
SbMemoryMap(kSize, kSbMemoryMapProtectWrite, "evergreen_buffer");
ASSERT_NE(SB_MEMORY_MAP_FAILED, memory);
mmap(nullptr, kSize, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
ASSERT_NE(MAP_FAILED, memory);
memset(memory, 0, kSize);
ASSERT_TRUE(SbMemoryProtect(
memory, kSmallerSize, kSbMemoryMapProtectRead | kSbMemoryMapProtectExec));
SbMemoryUnmap(memory, kSize);
ASSERT_EQ(mprotect(memory, kSmallerSize, PROT_READ | PROT_WRITE), 0);
munmap(memory, kSize);
EXPECT_TRUE(true);
}

Expand Down
Loading

0 comments on commit 139da0d

Please sign in to comment.