Merge "Move the Rust system property bindings into librustutils."
diff --git a/libc/bionic/malloc_heapprofd.cpp b/libc/bionic/malloc_heapprofd.cpp
index 741b45e..9c6ccb4 100644
--- a/libc/bionic/malloc_heapprofd.cpp
+++ b/libc/bionic/malloc_heapprofd.cpp
@@ -237,8 +237,6 @@
// heapprofd client initialization.
MallocHeapprofdState expected2 = kHookInstalled;
if (atomic_compare_exchange_strong(&gHeapprofdState, &expected,
- kInstallingEphemeralHook) ||
- atomic_compare_exchange_strong(&gHeapprofdState, &expected2,
kInstallingEphemeralHook)) {
const MallocDispatch* default_dispatch = GetDefaultDispatchTable();
@@ -248,14 +246,8 @@
// initialized, allocations may need to be serviced. There are three
// possible configurations:
- if (default_dispatch == nullptr) {
- // 1. No malloc hooking has been done (heapprofd, GWP-ASan, etc.). In
- // this case, everything but malloc() should come from the system
- // allocator.
- atomic_store(&gPreviousDefaultDispatchTable, nullptr);
- gEphemeralDispatch = *NativeAllocatorDispatch();
- } else if (DispatchIsGwpAsan(default_dispatch)) {
- // 2. GWP-ASan was installed. We should use GWP-ASan for everything but
+ if (DispatchIsGwpAsan(default_dispatch)) {
+ // 1. GWP-ASan was installed. We should use GWP-ASan for everything but
// malloc() in the interim period before heapprofd is properly
// installed. After heapprofd is finished installing, we will use
// GWP-ASan as heapprofd's backing allocator to allow heapprofd and
@@ -263,8 +255,16 @@
atomic_store(&gPreviousDefaultDispatchTable, default_dispatch);
gEphemeralDispatch = *default_dispatch;
} else {
+ // Either,
+ // 2. No malloc hooking has been done (heapprofd, GWP-ASan, etc.). In
+ // this case, everything but malloc() should come from the system
+ // allocator.
+ //
+ // or,
+ //
// 3. It may be possible at this point in time that heapprofd is
- // *already* the default dispatch, and as such we don't want to use
+ // *already* the default dispatch, and when it was initialized there
+ // was no default dispatch installed. As such we don't want to use
// heapprofd as the backing store for itself (otherwise infinite
// recursion occurs). We will use the system allocator functions. Note:
// We've checked that no other malloc interceptors are being used by
@@ -273,23 +273,41 @@
atomic_store(&gPreviousDefaultDispatchTable, nullptr);
gEphemeralDispatch = *NativeAllocatorDispatch();
}
-
- // Now, replace the malloc function so that the next call to malloc() will
- // initialize heapprofd.
- gEphemeralDispatch.malloc = MallocInitHeapprofdHook;
-
- // And finally, install these new malloc-family interceptors.
- __libc_globals.mutate([](libc_globals* globals) {
- atomic_store(&globals->default_dispatch_table, &gEphemeralDispatch);
- if (!MallocLimitInstalled()) {
- atomic_store(&globals->current_dispatch_table, &gEphemeralDispatch);
- }
- });
- atomic_store(&gHeapprofdState, kEphemeralHookInstalled);
+ } else if (atomic_compare_exchange_strong(&gHeapprofdState, &expected2,
+ kInstallingEphemeralHook)) {
+ // if we still have hook installed, we can reuse the previous
+ // decision. THIS IS REQUIRED FOR CORRECTNESS, because otherwise the
+ // following can happen
+ // 1. Assume DispatchIsGwpAsan(default_dispatch)
+ // 2. This function is ran, sets gPreviousDefaultDispatchTable to
+ // GWP ASan.
+ // 3. The sessions ends, DispatchReset FAILS due to a race. Now
+ // heapprofd hooks are default dispatch.
+ // 4. We re-enter this function later. If we did NOT look at the
+ // previously recorded gPreviousDefaultDispatchTable, we would
+ // incorrectly reach case 3. below.
+ // 5. The session ends, DispatchReset now resets the hooks to the
+ // system allocator. This is incorrect.
+ const MallocDispatch* prev_dispatch =
+ atomic_load(&gPreviousDefaultDispatchTable);
+ gEphemeralDispatch = prev_dispatch ? *prev_dispatch : *NativeAllocatorDispatch();
} else {
error_log("%s: heapprofd: failed to transition kInitialState -> kInstallingEphemeralHook. "
"current state (possible race): %d", getprogname(), expected2);
+ return;
}
+ // Now, replace the malloc function so that the next call to malloc() will
+ // initialize heapprofd.
+ gEphemeralDispatch.malloc = MallocInitHeapprofdHook;
+
+ // And finally, install these new malloc-family interceptors.
+ __libc_globals.mutate([](libc_globals* globals) {
+ atomic_store(&globals->default_dispatch_table, &gEphemeralDispatch);
+ if (!MallocLimitInstalled()) {
+ atomic_store(&globals->current_dispatch_table, &gEphemeralDispatch);
+ }
+ });
+ atomic_store(&gHeapprofdState, kEphemeralHookInstalled);
});
// Otherwise, we're racing against malloc_limit's enable logic (at most once
// per process, and a niche feature). This is highly unlikely, so simply give
@@ -335,16 +353,15 @@
return;
}
+ FinishInstallHooks(globals, nullptr, kHeapprofdPrefix);
+}
+
+void HeapprofdInstallHooksAtInit(libc_globals *globals) {
// Before we set the new default_dispatch_table in FinishInstallHooks, save
// the previous dispatch table. If DispatchReset() gets called later, we want
// to be able to restore the dispatch. We're still under
// MaybeModifyGlobals locks at this point.
atomic_store(&gPreviousDefaultDispatchTable, GetDefaultDispatchTable());
-
- FinishInstallHooks(globals, nullptr, kHeapprofdPrefix);
-}
-
-void HeapprofdInstallHooksAtInit(libc_globals* globals) {
MaybeModifyGlobals(kWithoutLock, [globals] {
MallocHeapprofdState expected = kInitialState;
if (atomic_compare_exchange_strong(&gHeapprofdState, &expected, kInstallingHook)) {
diff --git a/libc/bionic/strerror.cpp b/libc/bionic/strerror.cpp
index 5733567..0deb200 100644
--- a/libc/bionic/strerror.cpp
+++ b/libc/bionic/strerror.cpp
@@ -196,8 +196,7 @@
length = async_safe_format_buffer(buf, buf_len, "Unknown error %d", error_number);
}
if (length >= buf_len) {
- errno_restorer.override(ERANGE);
- return -1;
+ return ERANGE;
}
return 0;
diff --git a/tests/Android.bp b/tests/Android.bp
index 476b8f5..0f9af41 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -406,6 +406,7 @@
"string_nofortify_test.cpp",
"string_test.cpp",
"string_posix_strerror_r_test.cpp",
+ "string_posix_strerror_r_wrapper.cpp",
"strings_nofortify_test.cpp",
"strings_test.cpp",
"struct_layout_test.cpp",
diff --git a/tests/string_posix_strerror_r_test.cpp b/tests/string_posix_strerror_r_test.cpp
index 596684b..c4757ae 100644
--- a/tests/string_posix_strerror_r_test.cpp
+++ b/tests/string_posix_strerror_r_test.cpp
@@ -14,51 +14,40 @@
* limitations under the License.
*/
-#undef _GNU_SOURCE
-#include <features.h> // Get __BIONIC__ or __GLIBC__ so we can tell what we're using.
-
-#if defined(__GLIBC__)
-
-// At the time of writing, libcxx -- which is dragged in by gtest -- assumes
-// declarations from glibc of things that aren't available without _GNU_SOURCE.
-// This means we can't even build this test (which is a problem because that
-// means it doesn't get included in CTS).
-// For glibc 2.15, the symbols in question are:
-// at_quick_exit, quick_exit, vasprintf, strtoll_l, strtoull_l, and strtold_l.
-
-# if __GLIBC_PREREQ(2, 19)
-# error check whether we can build this now...
-# endif
-
-#else
-
-#include <string.h>
-
#include <errno.h>
#include <gtest/gtest.h>
+// Defined in string_posix_strerror_r_wrapper.cpp as a wrapper around the posix
+// strerror_r to work around an incompatibility between libc++ (required by
+// gtest) and !_GNU_SOURCE.
+int posix_strerror_r(int errnum, char* buf, size_t buflen);
+
TEST(string, posix_strerror_r) {
char buf[256];
// Valid.
- ASSERT_EQ(0, strerror_r(0, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(0, buf, sizeof(buf)));
ASSERT_STREQ("Success", buf);
- ASSERT_EQ(0, strerror_r(1, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(1, buf, sizeof(buf)));
ASSERT_STREQ("Operation not permitted", buf);
+#if defined(__BIONIC__)
// Invalid.
- ASSERT_EQ(0, strerror_r(-1, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(-1, buf, sizeof(buf)));
ASSERT_STREQ("Unknown error -1", buf);
- ASSERT_EQ(0, strerror_r(1234, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(1234, buf, sizeof(buf)));
ASSERT_STREQ("Unknown error 1234", buf);
+#else
+ // glibc returns EINVAL for unknown errors
+ ASSERT_EQ(EINVAL, posix_strerror_r(-1, buf, sizeof(buf)));
+ ASSERT_EQ(EINVAL, posix_strerror_r(1234, buf, sizeof(buf)));
+#endif
// Buffer too small.
errno = 0;
memset(buf, 0, sizeof(buf));
- ASSERT_EQ(-1, strerror_r(4567, buf, 2));
- ASSERT_STREQ("U", buf);
- // The POSIX strerror_r sets errno to ERANGE (the GNU one doesn't).
- ASSERT_EQ(ERANGE, errno);
+ ASSERT_EQ(ERANGE, posix_strerror_r(EPERM, buf, 2));
+ ASSERT_STREQ("O", buf);
+ // POSIX strerror_r returns an error without updating errno.
+ ASSERT_EQ(0, errno);
}
-
-#endif
diff --git a/tests/string_posix_strerror_r_wrapper.cpp b/tests/string_posix_strerror_r_wrapper.cpp
new file mode 100644
index 0000000..78d5d90
--- /dev/null
+++ b/tests/string_posix_strerror_r_wrapper.cpp
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+#undef _GNU_SOURCE
+#include <string.h>
+
+// At the time of writing, libcxx -- which is dragged in by gtest -- assumes
+// declarations from glibc of things that aren't available without _GNU_SOURCE.
+// This means we can't even build a test that directly calls the posix
+// strerror_r. Add a wrapper in a separate file that doesn't use any gtest.
+// For glibc 2.15, the symbols in question are:
+// at_quick_exit, quick_exit, vasprintf, strtoll_l, strtoull_l, and strtold_l.
+
+int posix_strerror_r(int errnum, char* buf, size_t buflen) {
+ return strerror_r(errnum, buf, buflen);
+}