Merge "Fixes for __cxa_finalize"

This commit is contained in:
Dmitriy Ivanov 2014-05-05 21:04:32 +00:00 committed by Gerrit Code Review
commit 580b246917
6 changed files with 218 additions and 34 deletions

View File

@ -26,11 +26,20 @@
* SUCH DAMAGE.
*/
#include <stddef.h>
extern void* __dso_handle;
extern int __cxa_atexit(void (*)(void*), void*, void*);
__attribute__ ((visibility ("hidden")))
int atexit(void (*func)(void)) {
return (__cxa_atexit((void (*)(void*)) func, (void*) 0, &__dso_handle));
void __atexit_handler_wrapper(void* func) {
if (func != NULL) {
(*(void (*)(void))func)();
}
}
__attribute__ ((visibility ("hidden")))
int atexit(void (*func)(void)) {
return (__cxa_atexit(&__atexit_handler_wrapper, func, &__dso_handle));
}

View File

@ -40,12 +40,53 @@
int __atexit_invalid = 1;
struct atexit *__atexit;
/*
* TODO: Read this before upstreaming:
*
* As of Apr 2014 there is a bug regaring function type detection logic in
* Free/Open/NetBSD implementations of __cxa_finalize().
*
* What it is about:
* First of all there are two kind of atexit handlers:
* 1) void handler(void) - this is the regular type
* available for to user via atexit(.) function call.
*
* 2) void internal_handler(void*) - this is the type
* __cxa_atexit() function expects. This handler is used
* by C++ compiler to register static destructor calls.
* Note that calling this function as the handler of type (1)
* results in incorrect this pointer in static d-tors.
*
* What is wrong with BSD implementations:
*
* They use dso argument to identify the handler type. The problem
* with it is dso is also used to identify the handlers associated
* with particular dynamic library and allow __cxa_finalize to call correct
* set of functions on dlclose(). And it cannot identify both.
*
* What is correct way to identify function type?
*
* Consider this:
* 1. __cxa_finalize and __cxa_atexit are part of libc and do not have access to hidden
* &__dso_handle.
* 2. __cxa_atexit has only 3 arguments: function pointer, function argument, dso.
* none of them can be reliably used to pass information about handler type.
* 3. following http://www.codesourcery.com/cxx-abi/abi.html#dso-dtor (3.3.5.3 - B)
* translation of user atexit -> __cxa_atexit(f, NULL, NULL) results in crashes
* on exit() after dlclose() of a library with an atexit() call.
*
* One way to resolve this is to always call second form of handler, which will
* result in storing unused argument in register/stack depending on architecture
* and should not present any problems.
*
* Another way is to make them dso-local in one way or the other.
*/
/*
* Function pointers are stored in a linked list of pages. The list
* is initially empty, and pages are allocated on demand. The first
* function pointer in the first allocated page (the last one in
* the linked list) was reserved for the cleanup function.
* TODO: switch to the regular FreeBSD/NetBSD atexit implementation.
*
* Outside the following functions, all pages are mprotect()'ed
* to prevent unintentional/malicious corruption.
@ -94,7 +135,7 @@ __cxa_atexit(void (*func)(void *), void *arg, void *dso)
__atexit_invalid = 0;
}
fnp = &p->fns[p->ind++];
fnp->fn_ptr.cxa_func = func;
fnp->cxa_func = func;
fnp->fn_arg = arg;
fnp->fn_dso = dso;
if (mprotect(p, pgsize, PROT_READ))
@ -113,57 +154,59 @@ unlock:
void
__cxa_finalize(void *dso)
{
struct atexit *p, *q;
struct atexit *p, *q, *original_atexit;
struct atexit_fn fn;
int n, pgsize = getpagesize();
int n, pgsize = getpagesize(), original_ind;
static int call_depth;
if (__atexit_invalid)
return;
_ATEXIT_LOCK();
call_depth++;
for (p = __atexit; p != NULL; p = p->next) {
for (n = p->ind; --n >= 0;) {
if (p->fns[n].fn_ptr.cxa_func == NULL)
continue; /* already called */
if (dso != NULL && dso != p->fns[n].fn_dso)
continue; /* wrong DSO */
p = original_atexit = __atexit;
n = original_ind = p != NULL ? p->ind : 0;
while (p != NULL) {
if (p->fns[n].cxa_func != NULL /* not called */
&& (dso == NULL || dso == p->fns[n].fn_dso)) { /* correct DSO */
/*
* Mark handler as having been already called to avoid
* dupes and loops, then call the appropriate function.
*/
fn = p->fns[n];
if (mprotect(p, pgsize, PROT_READ | PROT_WRITE) == 0) {
p->fns[n].fn_ptr.cxa_func = NULL;
p->fns[n].cxa_func = NULL;
mprotect(p, pgsize, PROT_READ);
}
_ATEXIT_UNLOCK();
#if ANDROID
/* it looks like we should always call the function
* with an argument, even if dso is not NULL. Otherwise
* static destructors will not be called properly on
* the ARM.
*/
(*fn.fn_ptr.cxa_func)(fn.fn_arg);
#else /* !ANDROID */
if (dso != NULL)
(*fn.fn_ptr.cxa_func)(fn.fn_arg);
else
(*fn.fn_ptr.std_func)();
#endif /* !ANDROID */
(*fn.cxa_func)(fn.fn_arg);
_ATEXIT_LOCK();
// check for new atexit handlers
if ((__atexit->ind != original_ind) || (__atexit != original_atexit)) {
// need to restart now to preserve correct
// call order - LIFO
p = original_atexit = __atexit;
n = original_ind = p->ind;
continue;
}
}
if (n == 0) {
p = p->next;
n = p != NULL ? p->ind : 0;
} else {
--n;
}
}
--call_depth;
/*
* If called via exit(), unmap the pages since we have now run
* all the handlers. We defer this until calldepth == 0 so that
* we don't unmap things prematurely if called recursively.
*/
if (dso == NULL && --call_depth == 0) {
if (dso == NULL && call_depth == 0) {
for (p = __atexit; p != NULL; ) {
q = p;
p = p->next;

View File

@ -37,10 +37,7 @@ struct atexit {
int ind; /* next index in this table */
int max; /* max entries >= ATEXIT_SIZE */
struct atexit_fn {
union {
void (*std_func)(void);
void (*cxa_func)(void *);
} fn_ptr;
void (*cxa_func)(void *);
void *fn_arg; /* argument for CXA callback */
void *fn_dso; /* shared module handle */
} fns[1]; /* the table itself */

View File

@ -209,6 +209,18 @@ build_type := target
build_target := SHARED_LIBRARY
include $(LOCAL_PATH)/Android.build.mk
# -----------------------------------------------------------------------------
# This library used by atexit tests
# -----------------------------------------------------------------------------
libtest_atexit_src_files := \
atexit_testlib.cpp
module := libtest_atexit
build_type := target
build_target := SHARED_LIBRARY
include $(LOCAL_PATH)/Android.build.mk
# -----------------------------------------------------------------------------
# Tests for the device using bionic's .so. Run with:
# adb shell /data/nativetest/bionic-unit-tests/bionic-unit-tests
@ -217,6 +229,7 @@ bionic-unit-tests_whole_static_libraries := \
libBionicTests \
bionic-unit-tests_src_files := \
atexit_test.cpp \
dlext_test.cpp \
dlfcn_test.cpp \
@ -263,11 +276,14 @@ include $(LOCAL_PATH)/Android.build.mk
ifeq ($(HOST_OS)-$(HOST_ARCH),linux-x86)
bionic-unit-tests-glibc_src_files := \
atexit_test.cpp \
bionic-unit-tests-glibc_whole_static_libraries := \
libBionicStandardTests \
bionic-unit-tests-glibc_ldlibs := \
-lrt \
-lrt -ldl \
module := bionic-unit-tests-glibc
module_tag := optional

44
tests/atexit_test.cpp Normal file
View File

@ -0,0 +1,44 @@
/*
* Copyright (C) 2014 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 <gtest/gtest.h>
#include <dlfcn.h>
#include <libgen.h>
#include <limits.h>
#include <stdio.h>
#include <stdint.h>
#include <string>
TEST(atexit, combined_test) {
std::string atexit_call_sequence;
bool valid_this_in_static_dtor = false;
void* handle = dlopen("libtest_atexit.so", RTLD_NOW);
ASSERT_TRUE(handle != NULL);
void* sym = dlsym(handle, "register_atexit");
ASSERT_TRUE(sym != NULL);
reinterpret_cast<void (*)(std::string*, bool*)>(sym)(&atexit_call_sequence, &valid_this_in_static_dtor);
ASSERT_EQ(0, dlclose(handle));
// this test verifies atexit call from atexit handler. as well as the order of calls
ASSERT_EQ("Humpty Dumpty sat on a wall", atexit_call_sequence);
ASSERT_TRUE(valid_this_in_static_dtor);
}
// TODO: test for static dtor calls from from exit(.) -> __cxa_finalize(NULL)

75
tests/atexit_testlib.cpp Normal file
View File

@ -0,0 +1,75 @@
/*
* Copyright (C) 2014 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 <stdio.h>
#include <stdlib.h>
#include <string>
// use external control number from main test
static std::string* atexit_sequence = NULL;
static bool* atexit_valid_this_in_static_dtor = NULL;
class AtExitStaticClass;
static const AtExitStaticClass* valid_this = NULL;
static class AtExitStaticClass {
public:
AtExitStaticClass() { valid_this = this; }
~AtExitStaticClass() {
if (atexit_valid_this_in_static_dtor) {
*atexit_valid_this_in_static_dtor = (valid_this == this);
}
}
} staticObj;
// 4
static void atexit_handler_from_atexit_from_atexit2() {
*atexit_sequence += " on";
}
// 3
static void atexit_handler_from_atexit_from_atexit1() {
*atexit_sequence += " sat";
}
// 2
static void atexit_handler_from_atexit() {
*atexit_sequence += " Dumpty";
// register 2 others
atexit(atexit_handler_from_atexit_from_atexit2);
atexit(atexit_handler_from_atexit_from_atexit1);
}
// 1
static void atexit_handler_with_atexit() {
*atexit_sequence += "Humpty";
atexit(atexit_handler_from_atexit);
}
// last
static void atexit_handler_regular() {
*atexit_sequence += " a wall";
}
extern "C" void register_atexit(std::string* sequence, bool* valid_this_in_static_dtor) {
atexit_sequence = sequence;
atexit_valid_this_in_static_dtor = valid_this_in_static_dtor;
atexit(atexit_handler_regular);
atexit(atexit_handler_with_atexit);
atexit(NULL);
}