Skip to content

Commit

Permalink
crazy linker: Disable crbug/448968 fix for all KitKat and earlier.
Browse files Browse the repository at this point in the history
Disable the fix for crbug/448968 for all KitKat and earlier.

LD_PRELOADS and lookup-in-main-executable behaviour now:
  SDK earlier than L:
    nothing
  SDK L:
    load preloads, on lookup search preloads then search main
  SDK L-MR1 and later:
    on lookup search main

BUG=479220

Review URL: https://codereview.chromium.org/1100343002

Cr-Commit-Position: refs/heads/master@{#326576}
  • Loading branch information
simonb authored and Commit bot committed Apr 23, 2015
1 parent d24ad3c commit 6cc3f4a
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 24 deletions.
2 changes: 2 additions & 0 deletions third_party/android_crazy_linker/README.chromium
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,5 @@ Local Modifications:

- Add basic LD_PRELOAD handling, for crbug/448968.

- Speculative fix for crbug/479220.

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

namespace crazy {

// From android.os.Build.VERSION_CODES.LOLLIPOP.
static const int SDK_VERSION_CODE_LOLLIPOP = 21;

class Globals {
public:
Globals();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ namespace crazy {

namespace {

// From android.os.Build.VERSION_CODES.LOLLIPOP.
static const int SDK_VERSION_CODE_LOLLIPOP = 21;

// Page size for alignment in a zip file.
const size_t kZipAlignmentPageSize = 4096;
COMPILE_ASSERT(kZipAlignmentPageSize % PAGE_SIZE == 0,
Expand Down Expand Up @@ -64,8 +61,8 @@ struct SymbolLookupState {
LibraryList::LibraryList() : head_(0), has_error_(false) {
const int sdk_build_version = *Globals::GetSDKBuildVersion();

// If SDK version is Lollipop or earlier, we need to load anything
// listed in LD_PRELOAD explicitly, because dlsym() on the main executable
// If SDK version is Lollipop, we need to load anything listed in
// LD_PRELOAD explicitly, because dlsym() on the main executable
// fails to lookup in preloads on those releases. Also, when doing our
// symbol resolution we need to explicity search preloads *before* we
// search the main executable, to ensure that preloads override symbols
Expand All @@ -79,9 +76,15 @@ LibraryList::LibraryList() : head_(0), has_error_(false) {
// of them for us, and so by not loading preloads here our preloads list
// remains empty, so that searching it for name lookups is a no-op.
//
// If SDK version is earlier than Lollipop then we also do nothing.
// Pre-Lollipop platforms arguably have the same dlsym() issue, but for
// now we disable the dlsym() workround in order to try and address
// crbug/479220.
//
// For more, see:
// https://code.google.com/p/android/issues/detail?id=74255
if (sdk_build_version <= SDK_VERSION_CODE_LOLLIPOP)
// https://code.google.com/p/chromium/issues/detail?id=479220
if (sdk_build_version == SDK_VERSION_CODE_LOLLIPOP)
LoadPreloads();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class SharedLibraryResolver : public ElfRelocations::SymbolResolver {
Vector<LibraryView*>* preloads,
Vector<LibraryView*>* dependencies)
: main_program_handle_(::dlopen(NULL, RTLD_NOW)),
sdk_build_version_(*Globals::GetSDKBuildVersion()),
lib_(lib), preloads_(preloads), dependencies_(dependencies) {}

virtual void* Lookup(const char* symbol_name) {
Expand All @@ -116,30 +117,50 @@ class SharedLibraryResolver : public ElfRelocations::SymbolResolver {
if (address)
return address;

// Then look inside the preloads.
// Make sure that we do nothing here for non-Lollipop platforms.
// In practice, preloads_ will be empty (because we ignore LD_PRELOAD
// when not Lollipop) and searching in the main executable should be
// benign. But crbug/479220 is weird enough that we want to take all
// possible precautions.
//
// Note that searching preloads *before* the main executable is opposite
// to the search ordering used by the system linker, but it is required
// to work round a dlsym() bug in some Android releases (on releases
// without this dlsym() bug preloads_ will be empty, making this preloads
// search a no-op).
// For more, see:
// https://code.google.com/p/chromium/issues/detail?id=479220
if (sdk_build_version_ == SDK_VERSION_CODE_LOLLIPOP) {
// Then look inside the preloads.
//
// Note that searching preloads *before* the main executable is opposite
// to the search ordering used by the system linker, but it is required
// to work round a dlsym() bug in some Android releases (on releases
// without this dlsym() bug preloads_ will be empty, making this preloads
// search a no-op).
//
// For more, see commentary in LibraryList(), and
// https://code.google.com/p/android/issues/detail?id=74255
for (size_t n = 0; n < preloads_->GetCount(); ++n) {
LibraryView* wrap = (*preloads_)[n];
// LOG("%s: Looking into preload %p (%s)\n", __FUNCTION__, wrap,
// wrap->GetName());
address = LookupInWrap(symbol_name, wrap);
if (address)
return address;
}
}

// Do not lookup inside the main executable for pre-Lollipop, for
// crbug/479220. We do however want to do it for Lollipop and
// later, because in Lollipop-mr1 the dlsym() bug is fixed, so that
// we don't explicitly handle LD_PRELOADS but instead rely on dlsym
// correctly searching preloads via the main executable.
//
// For more, see commentary in LibraryList(), and
// https://code.google.com/p/android/issues/detail?id=74255
for (size_t n = 0; n < preloads_->GetCount(); ++n) {
LibraryView* wrap = (*preloads_)[n];
// LOG("%s: Looking into preload %p (%s)\n", __FUNCTION__, wrap,
// wrap->GetName());
address = LookupInWrap(symbol_name, wrap);
// For more, see:
// https://code.google.com/p/chromium/issues/detail?id=479220
if (sdk_build_version_ >= SDK_VERSION_CODE_LOLLIPOP) {
// Then look inside the main executable.
address = ::dlsym(main_program_handle_, symbol_name);
if (address)
return address;
}

// Then lookup inside the main executable.
address = ::dlsym(main_program_handle_, symbol_name);
if (address)
return address;

// Then look inside the dependencies.
for (size_t n = 0; n < dependencies_->GetCount(); ++n) {
LibraryView* wrap = (*dependencies_)[n];
Expand Down Expand Up @@ -188,6 +209,7 @@ class SharedLibraryResolver : public ElfRelocations::SymbolResolver {
return NULL;
}

const int sdk_build_version_;
void* main_program_handle_;
SharedLibrary* lib_;
Vector<LibraryView*>* preloads_;
Expand Down

0 comments on commit 6cc3f4a

Please sign in to comment.