New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 801548 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Cleanup Crazy linker debugging support

Project Member Reported by digit@chromium.org, Jan 12 2018

Issue description

Debugging support in third_party/android_crazy_linker needs to be cleaned up to avoid a few issues:

- The API provides a way to defer certain modifications of
  process-global state (i.e. the list of libraries visible from GDB)
  to a different thread, in order to minimize race conditions when
  the system linker tries to modify the same value. There is
  no reliable and safe way to do this, unfortunately

  However, the implementation of ScopedDelayedCallbackPoster is racy
  (it just modifies a global boolean flag on scope entry/exit, which
  is not thread-safe since it can be used by several Chromium threads
  at the same time).

- The API is also a lot more complicated than necessary (it exposes
  callbacks to the crazy-linker clients for no good reason).

- Finally, the implementation is buggy, since it sends potentially
  dangling pointers to the client. This can result in runtime crashes
  in certain cases (e.g. if a library if loaded and unloaded before
  the callback actually runs).

This bug is to track the issue.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/751fcf391b147439d4637789996f58b37e6e3d37

commit 751fcf391b147439d4637789996f58b37e6e3d37
Author: David 'Digit' Turner <digit@google.com>
Date: Tue Jan 30 15:51:22 2018

android crazy linker: better Globals implementation.

A small cleanup and/or improvement to the way global
data is managed in the crazy linker library:

- Replace ScopedGlobalLock with ScopedLockedGlobals
  which allows reducing the number of Globals::Get()
  call performed at runtime.

- Provide property accessors to the globals that
  don't call Globals::Get() on each call, e.g.

    the static Globals::GetLibraries() is replaced
    by the non-static Globals::libraries() call.

- In crazy_linker_api.cpp, Fix a minor bug where
  the ScopedDelayedCallbackPoster is constructed
  before the global lock is acquired (i.e. a
  thread-race).

  Note that there are still other thread-related
  issues with deferred tasks / callback handling
  but these will be addressed in a future CL.

- Don't use heap allocation for the Globals
  instance storage. This is mostly because there
  were suspicion of invalid pointer addresses
  being passed through callbacks, causing runtime
  crashes (see http://crbug.com/796938 for more
  details). A fixed address in the .bss section
  would be easier to spot in minidump CPU register
  dumps.

BUG=801548
R=agrieve@chromium.org,pasko@chromium.org,rmcilroy@chromium.org

Change-Id: I68cba5360c5b5a3aaa7847172a6e73a87a64d44b
Reviewed-on: https://chromium-review.googlesource.com/893259
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: David Turner <digit@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532889}
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_api.cpp
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_globals.cpp
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_globals.h
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_globals_unittest.cpp
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_library_list.cpp
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_library_view.cpp
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_rdebug.cpp
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_shared_library.cpp
[modify] https://crrev.com/751fcf391b147439d4637789996f58b37e6e3d37/third_party/android_crazy_linker/src/src/crazy_linker_wrappers.cpp

Status: Assigned (was: Available)

Sign in to add a comment