Revert "Change order of creation of JIT mappings."
This reverts commit 145fce1b7bdee2bfc205b767d558b4ef09bbde22.
Reason for revert: b/200284993
Change-Id: Ifcdfc5eb04b9f3e8cd799b5a1423ab64d9357c21
diff --git a/runtime/jit/jit_memory_region.cc b/runtime/jit/jit_memory_region.cc
index 88d927e..58a4041 100644
--- a/runtime/jit/jit_memory_region.cc
+++ b/runtime/jit/jit_memory_region.cc
@@ -64,13 +64,6 @@
// File descriptor enabling dual-view mapping of code section.
unique_fd mem_fd;
-
- // The memory mappings we are going to create.
- MemMap data_pages;
- MemMap exec_pages;
- MemMap non_exec_pages;
- MemMap writable_data_pages;
-
if (is_zygote) {
// Because we are not going to GC code generated by the zygote, just use all available.
current_capacity_ = max_capacity;
@@ -99,12 +92,17 @@
}
}
- // Map name specific for android_os_Debug.cpp accounting.
std::string data_cache_name = is_zygote ? "zygote-data-code-cache" : "data-code-cache";
std::string exec_cache_name = is_zygote ? "zygote-jit-code-cache" : "jit-code-cache";
std::string error_str;
+ // Map name specific for android_os_Debug.cpp accounting.
+ // Map in low 4gb to simplify accessing root tables for x86_64.
+ // We could do PC-relative addressing to avoid this problem, but that
+ // would require reserving code and data area before submitting, which
+ // means more windows for the code memory to be RWX.
int base_flags;
+ MemMap data_pages;
if (mem_fd.get() >= 0) {
// Dual view of JIT code cache case. Create an initial mapping of data pages large enough
// for data and non-writable view of JIT code pages. We use the memory file descriptor to
@@ -133,63 +131,7 @@
// Additionally, the zyzote will create a dual view of the data portion of
// the cache. This mapping will be read-only, whereas the second mapping
// will be writable.
-
base_flags = MAP_SHARED;
-
- // Create the writable mappings now, so that in case of the zygote, we can
- // prevent any future writable mappings through sealing.
- if (exec_capacity > 0) {
- // For dual view, create the secondary view of code memory used for updating code. This view
- // is never executable.
- std::string name = exec_cache_name + "-rw";
- non_exec_pages = MemMap::MapFile(exec_capacity,
- kIsDebugBuild ? kProtR : kProtRW,
- base_flags,
- mem_fd,
- /* start= */ data_capacity,
- /* low_4GB= */ false,
- name.c_str(),
- &error_str);
- if (!non_exec_pages.IsValid()) {
- // This is unexpected.
- *error_msg = "Failed to map non-executable view of JIT code cache";
- return false;
- }
- // Create a dual view of the data cache.
- name = data_cache_name + "-rw";
- writable_data_pages = MemMap::MapFile(data_capacity,
- kProtRW,
- base_flags,
- mem_fd,
- /* start= */ 0,
- /* low_4GB= */ false,
- name.c_str(),
- &error_str);
- if (!writable_data_pages.IsValid()) {
- std::ostringstream oss;
- oss << "Failed to create dual data view: " << error_str;
- *error_msg = oss.str();
- return false;
- }
- if (writable_data_pages.MadviseDontFork() != 0) {
- *error_msg = "Failed to MadviseDontFork the writable data view";
- return false;
- }
- if (non_exec_pages.MadviseDontFork() != 0) {
- *error_msg = "Failed to MadviseDontFork the writable code view";
- return false;
- }
- // Now that we have created the writable and executable mappings, prevent creating any new
- // ones.
- if (is_zygote && !ProtectZygoteMemory(mem_fd.get(), error_msg)) {
- return false;
- }
- }
-
- // Map in low 4gb to simplify accessing root tables for x86_64.
- // We could do PC-relative addressing to avoid this problem, but that
- // would require reserving code and data area before submitting, which
- // means more windows for the code memory to be RWX.
data_pages = MemMap::MapFile(
data_capacity + exec_capacity,
kProtR,
@@ -230,6 +172,9 @@
return false;
}
+ MemMap exec_pages;
+ MemMap non_exec_pages;
+ MemMap writable_data_pages;
if (exec_capacity > 0) {
uint8_t* const divider = data_pages.Begin() + data_capacity;
// Set initial permission for executable view to catch any SELinux permission problems early
@@ -248,6 +193,59 @@
*error_msg = oss.str();
return false;
}
+
+ if (mem_fd.get() >= 0) {
+ // For dual view, create the secondary view of code memory used for updating code. This view
+ // is never executable.
+ std::string name = exec_cache_name + "-rw";
+ non_exec_pages = MemMap::MapFile(exec_capacity,
+ kIsDebugBuild ? kProtR : kProtRW,
+ base_flags,
+ mem_fd,
+ /* start= */ data_capacity,
+ /* low_4GB= */ false,
+ name.c_str(),
+ &error_str);
+ if (!non_exec_pages.IsValid()) {
+ static const char* kFailedNxView = "Failed to map non-executable view of JIT code cache";
+ if (rwx_memory_allowed) {
+ // Log and continue as single view JIT (requires RWX memory).
+ VLOG(jit) << kFailedNxView;
+ } else {
+ *error_msg = kFailedNxView;
+ return false;
+ }
+ }
+ // Create a dual view of the data cache.
+ name = data_cache_name + "-rw";
+ writable_data_pages = MemMap::MapFile(data_capacity,
+ kProtRW,
+ base_flags,
+ mem_fd,
+ /* start= */ 0,
+ /* low_4GB= */ false,
+ name.c_str(),
+ &error_str);
+ if (!writable_data_pages.IsValid()) {
+ std::ostringstream oss;
+ oss << "Failed to create dual data view: " << error_str;
+ *error_msg = oss.str();
+ return false;
+ }
+ if (writable_data_pages.MadviseDontFork() != 0) {
+ *error_msg = "Failed to madvise dont fork the writable data view";
+ return false;
+ }
+ if (non_exec_pages.MadviseDontFork() != 0) {
+ *error_msg = "Failed to madvise dont fork the writable code view";
+ return false;
+ }
+ // Now that we have created the writable and executable mappings, prevent creating any new
+ // ones.
+ if (is_zygote && !ProtectZygoteMemory(mem_fd.get(), error_msg)) {
+ return false;
+ }
+ }
} else {
// Profiling only. No memory for code required.
}
diff --git a/runtime/jit/jit_memory_region_test.cc b/runtime/jit/jit_memory_region_test.cc
index 18f34fb..2049611 100644
--- a/runtime/jit/jit_memory_region_test.cc
+++ b/runtime/jit/jit_memory_region_test.cc
@@ -492,62 +492,6 @@
munmap(addr, kPageSize);
munmap(shared, kPageSize);
}
-
- // Test that a readable mapping created befire sealing future writes, can be
- // changed into a writable mapping.
- void TestVmMayWriteBefore() {
- // Zygote JIT memory only works on kernels that don't segfault on flush.
- TEST_DISABLED_FOR_KERNELS_WITH_CACHE_SEGFAULT();
- std::string error_msg;
- size_t size = kPageSize;
- int32_t* addr = nullptr;
- {
- android::base::unique_fd fd(JitMemoryRegion::CreateZygoteMemory(size, &error_msg));
- CHECK_NE(fd.get(), -1);
-
- // Create a shared readable mapping.
- addr = reinterpret_cast<int32_t*>(
- mmap(nullptr, kPageSize, PROT_READ, MAP_SHARED, fd.get(), 0));
- CHECK(addr != nullptr);
- CHECK_NE(addr, MAP_FAILED);
-
- // Protect the memory.
- bool res = JitMemoryRegion::ProtectZygoteMemory(fd.get(), &error_msg);
- CHECK(res);
- }
- // At this point, the fd has been dropped, but the memory mappings are still
- // there.
- int res = mprotect(addr, kPageSize, PROT_WRITE);
- CHECK_EQ(res, 0);
- }
-
- // Test that we cannot create a writable mapping after sealing future writes.
- void TestVmMayWriteAfter() {
- // Zygote JIT memory only works on kernels that don't segfault on flush.
- TEST_DISABLED_FOR_KERNELS_WITH_CACHE_SEGFAULT();
- std::string error_msg;
- size_t size = kPageSize;
- int32_t* addr = nullptr;
- {
- android::base::unique_fd fd(JitMemoryRegion::CreateZygoteMemory(size, &error_msg));
- CHECK_NE(fd.get(), -1);
-
- // Protect the memory.
- bool res = JitMemoryRegion::ProtectZygoteMemory(fd.get(), &error_msg);
- CHECK(res);
-
- // Create a shared readable mapping.
- addr = reinterpret_cast<int32_t*>(
- mmap(nullptr, kPageSize, PROT_READ, MAP_SHARED, fd.get(), 0));
- CHECK(addr != nullptr);
- CHECK_NE(addr, MAP_FAILED);
- }
- // At this point, the fd has been dropped, but the memory mappings are still
- // there.
- int res = mprotect(addr, kPageSize, PROT_WRITE);
- CHECK_EQ(res, -1);
- CHECK_EQ(errno, EACCES);
- }
};
TEST_F(TestZygoteMemory, BasicTest) {
@@ -566,14 +510,6 @@
TestFromSharedToPrivate();
}
-TEST_F(TestZygoteMemory, TestVmMayWriteBefore) {
- TestVmMayWriteBefore();
-}
-
-TEST_F(TestZygoteMemory, TestVmMayWriteAfter) {
- TestVmMayWriteAfter();
-}
-
#endif // defined (__BIONIC__)
} // namespace jit