Revert "Fix CFI initialization crash on x86."
This reverts commit be46d3871c91902504e5ec4c7f575c86f647aafc.
Reverting due to broken build:
https://android-build.googleplex.com/builds/branch-dashboard/aosp-master?build_id=3687823
Change-Id: I30968398edd9a094fce878839ba0a002ed487a77
diff --git a/libdl/libdl_cfi.cpp b/libdl/libdl_cfi.cpp
index 8458564..362b093 100644
--- a/libdl/libdl_cfi.cpp
+++ b/libdl/libdl_cfi.cpp
@@ -29,12 +29,10 @@
char padding[PAGE_SIZE - sizeof(v)];
} shadow_base_storage alignas(PAGE_SIZE);
-// __cfi_init is called by the loader as soon as the shadow is mapped. This may happen very early
-// during startup, before libdl.so global constructors, and, on i386, even before __libc_sysinfo is
-// initialized. This function should not do any system calls.
extern "C" uintptr_t* __cfi_init(uintptr_t shadow_base) {
shadow_base_storage.v = shadow_base;
static_assert(sizeof(shadow_base_storage) == PAGE_SIZE, "");
+ mprotect(&shadow_base_storage, PAGE_SIZE, PROT_READ);
return &shadow_base_storage.v;
}
diff --git a/linker/linker_cfi.cpp b/linker/linker_cfi.cpp
index 28d2eaf..e9cdab6 100644
--- a/linker/linker_cfi.cpp
+++ b/linker/linker_cfi.cpp
@@ -193,7 +193,6 @@
shadow_start = reinterpret_cast<uintptr_t* (*)(uintptr_t)>(cfi_init)(p);
CHECK(shadow_start != nullptr);
CHECK(*shadow_start == p);
- mprotect(shadow_start, PAGE_SIZE, PROT_READ);
return true;
}
diff --git a/tests/cfi_test.cpp b/tests/cfi_test.cpp
index a73fd34..0f93edb 100644
--- a/tests/cfi_test.cpp
+++ b/tests/cfi_test.cpp
@@ -1,25 +1,7 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <dlfcn.h>
#include <gtest/gtest.h>
+#include <dlfcn.h>
#include "BionicDeathTest.h"
-#include "gtest_globals.h"
-#include "utils.h"
// Private libdl interface.
extern "C" {
@@ -110,24 +92,3 @@
handle = dlopen("libcfi-test-bad.so", RTLD_NOW | RTLD_LOCAL);
ASSERT_FALSE(handle != nullptr) << dlerror();
}
-
-// cfi_test_helper exports __cfi_check, which triggers CFI initialization at startup.
-TEST(cfi_test, early_init) {
-#if defined(__BIONIC__)
- std::string helper = get_testlib_root() + "/cfi_test_helper/cfi_test_helper";
- ExecTestHelper eth;
- eth.SetArgs({ helper.c_str(), nullptr });
- eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0, nullptr);
-#endif
-}
-
-// cfi_test_helper2 depends on a library that exports __cfi_check, which triggers CFI initialization
-// at startup.
-TEST(cfi_test, early_init2) {
-#if defined(__BIONIC__)
- std::string helper = get_testlib_root() + "/cfi_test_helper2/cfi_test_helper2";
- ExecTestHelper eth;
- eth.SetArgs({ helper.c_str(), nullptr });
- eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0, nullptr);
-#endif
-}
diff --git a/tests/libs/Android.bp b/tests/libs/Android.bp
index 9f9c407..b5855af 100644
--- a/tests/libs/Android.bp
+++ b/tests/libs/Android.bp
@@ -494,20 +494,3 @@
cfi: false,
},
}
-
-cc_test {
- name: "cfi_test_helper",
- host_supported: false,
- defaults: ["bionic_testlib_defaults"],
- srcs: ["cfi_test_helper.cpp"],
- ldflags: [" -Wl,-export-dynamic-symbol=__cfi_check"],
-}
-
-cc_test {
- name: "cfi_test_helper2",
- host_supported: false,
- defaults: ["bionic_testlib_defaults"],
- srcs: ["cfi_test_helper2.cpp"],
- shared_libs: ["libcfi-test"],
- ldflags: ["-Wl,--rpath,${ORIGIN}/.."],
-}
diff --git a/tests/libs/cfi_test_helper.cpp b/tests/libs/cfi_test_helper.cpp
deleted file mode 100644
index ff313a2..0000000
--- a/tests/libs/cfi_test_helper.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <assert.h>
-#include <stdint.h>
-#include <stdlib.h>
-
-#include "libs_utils.h"
-
-// This library is built for all targets, including host tests, so __cfi_slowpath may not be
-// present. But it is only used in the bionic loader tests.
-extern "C" __attribute__((weak)) void __cfi_slowpath(uint64_t, void*);
-
-static int g_count;
-
-// Mock a CFI-enabled library without relying on the compiler.
-extern "C" __attribute__((aligned(4096))) void __cfi_check(uint64_t /*CallSiteTypeId*/,
- void* /*TargetAddr*/, void* /*Diag*/) {
- ++g_count;
-}
-
-void preinit_ctor() {
- CHECK(g_count == 0);
- __cfi_slowpath(42, reinterpret_cast<void*>(&preinit_ctor));
- CHECK(g_count == 1);
-}
-
-__attribute__((section(".preinit_array"), used)) void (*preinit_ctor_p)(void) = preinit_ctor;
-
-__attribute__((constructor, used)) void ctor() {
- CHECK(g_count == 1);
- __cfi_slowpath(42, reinterpret_cast<void*>(&ctor));
- CHECK(g_count == 2);
-}
-
-int main(void) {
- CHECK(g_count == 2);
- __cfi_slowpath(42, reinterpret_cast<void*>(&main));
- CHECK(g_count == 3);
- return 0;
-}
diff --git a/tests/libs/cfi_test_helper2.cpp b/tests/libs/cfi_test_helper2.cpp
deleted file mode 100644
index 11a6036..0000000
--- a/tests/libs/cfi_test_helper2.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <assert.h>
-#include <dlfcn.h>
-
-#include "libs_utils.h"
-
-int main(void) {
- void* handle;
- // libcfi-test.so does some basic testing in a global constructor. Check that it is linked.
- handle = dlopen("libcfi-test.so", RTLD_NOW | RTLD_NOLOAD);
- CHECK(handle != nullptr);
- dlclose(handle);
- return 0;
-}
diff --git a/tests/libs/cfi_test_lib.cpp b/tests/libs/cfi_test_lib.cpp
index 959b102..b0e2f42 100644
--- a/tests/libs/cfi_test_lib.cpp
+++ b/tests/libs/cfi_test_lib.cpp
@@ -1,19 +1,3 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
#include <assert.h>
#include <stdint.h>
#include <stdlib.h>
diff --git a/tests/libs/libs_utils.h b/tests/libs/libs_utils.h
deleted file mode 100644
index f11cbe7..0000000
--- a/tests/libs/libs_utils.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef LIBS_UTILS_H
-#define LIBS_UTILS_H
-
-#include <stdio.h>
-#include <stdlib.h>
-
-#define CHECK(x) \
- do { \
- fprintf(stderr, "CHECK(" #x ") failed at %s:%d\n", __FILE__, __LINE__); \
- if (!(x)) abort(); \
- } while (0)
-
-#endif // LIBS_UTILS_H
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index ddf0df5..9d809f0 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -1203,6 +1203,52 @@
}
}
+class ExecTestHelper {
+ public:
+ char** GetArgs() { return const_cast<char**>(args_.data()); }
+ char** GetEnv() { return const_cast<char**>(env_.data()); }
+
+ void SetArgs(const std::vector<const char*> args) { args_ = args; }
+ void SetEnv(const std::vector<const char*> env) { env_ = env; }
+
+ void Run(const std::function<void ()>& child_fn,
+ int expected_exit_status,
+ const char* expected_output) {
+ int fds[2];
+ ASSERT_NE(pipe2(fds, 0), -1);
+
+ pid_t pid = fork();
+ ASSERT_NE(pid, -1);
+
+ if (pid == 0) {
+ // Child.
+ close(fds[0]);
+ dup2(fds[1], STDOUT_FILENO);
+ dup2(fds[1], STDERR_FILENO);
+ if (fds[1] != STDOUT_FILENO && fds[1] != STDERR_FILENO) close(fds[1]);
+ child_fn();
+ FAIL();
+ }
+
+ // Parent.
+ close(fds[1]);
+ std::string output;
+ char buf[BUFSIZ];
+ ssize_t bytes_read;
+ while ((bytes_read = TEMP_FAILURE_RETRY(read(fds[0], buf, sizeof(buf)))) > 0) {
+ output.append(buf, bytes_read);
+ }
+ close(fds[0]);
+
+ AssertChildExited(pid, expected_exit_status);
+ ASSERT_EQ(expected_output, output);
+ }
+
+ private:
+ std::vector<const char*> args_;
+ std::vector<const char*> env_;
+};
+
#if defined(__GLIBC__)
#define BIN_DIR "/bin/"
#else
diff --git a/tests/utils.h b/tests/utils.h
index 0b9b886..79eed10 100644
--- a/tests/utils.h
+++ b/tests/utils.h
@@ -125,13 +125,8 @@
static inline void AssertChildExited(int pid, int expected_exit_status) {
int status;
ASSERT_EQ(pid, waitpid(pid, &status, 0));
- if (expected_exit_status >= 0) {
- ASSERT_TRUE(WIFEXITED(status));
- ASSERT_EQ(expected_exit_status, WEXITSTATUS(status));
- } else {
- ASSERT_TRUE(WIFSIGNALED(status));
- ASSERT_EQ(-expected_exit_status, WTERMSIG(status));
- }
+ ASSERT_TRUE(WIFEXITED(status));
+ ASSERT_EQ(expected_exit_status, WEXITSTATUS(status));
}
// The absolute path to the executable
@@ -147,59 +142,4 @@
char** get_argv();
char** get_envp();
-class ExecTestHelper {
- public:
- char** GetArgs() {
- return const_cast<char**>(args_.data());
- }
- char** GetEnv() {
- return const_cast<char**>(env_.data());
- }
-
- void SetArgs(const std::vector<const char*> args) {
- args_ = args;
- }
- void SetEnv(const std::vector<const char*> env) {
- env_ = env;
- }
-
- void Run(const std::function<void()>& child_fn, int expected_exit_status,
- const char* expected_output) {
- int fds[2];
- ASSERT_NE(pipe2(fds, 0), -1);
-
- pid_t pid = fork();
- ASSERT_NE(pid, -1);
-
- if (pid == 0) {
- // Child.
- close(fds[0]);
- dup2(fds[1], STDOUT_FILENO);
- dup2(fds[1], STDERR_FILENO);
- if (fds[1] != STDOUT_FILENO && fds[1] != STDERR_FILENO) close(fds[1]);
- child_fn();
- FAIL();
- }
-
- // Parent.
- close(fds[1]);
- std::string output;
- char buf[BUFSIZ];
- ssize_t bytes_read;
- while ((bytes_read = TEMP_FAILURE_RETRY(read(fds[0], buf, sizeof(buf)))) > 0) {
- output.append(buf, bytes_read);
- }
- close(fds[0]);
-
- AssertChildExited(pid, expected_exit_status);
- if (expected_output != nullptr) {
- ASSERT_EQ(expected_output, output);
- }
- }
-
- private:
- std::vector<const char*> args_;
- std::vector<const char*> env_;
-};
-
#endif