Fixed
Status Update
Comments
ja...@gmail.com <ja...@gmail.com> #2
Err, I have no idea how gerrit changesets are reflected in the bug report, but there is a preliminary patch https://android-review.googlesource.com/#/c/159020/
vs...@google.com <vs...@google.com>
ja...@gmail.com <ja...@gmail.com> #3
Scope of changes to bionic are somewhat more complex than initially thought.
The troublesome spots are mmap() and friends, malloc() and friends (due to symbol clash between struct, fieldname, and function names) as well as most of the routines in string.h
string.h exposes some issues, because the routines in it are both defined as inline functions that call out to assembly counterpart of the same name using an inconsistent __builtin_XXXX() convention. Sometimes the routines are available as _builtin___XXXX_chk and sometimes as __builtin_xxx.
I initially thought about limiting the macro magic purely for bionic/arm64, but there does not seem to be a nice way to do this.
Does this mean I need to touch all the .S files that are associated with string.h?
Even for the ones that are NOT for arm64?
Please take a look at:
https://code.google.com/p/android/issues/detail?id=180578
The troublesome spots are mmap() and friends, malloc() and friends (due to symbol clash between struct, fieldname, and function names) as well as most of the routines in string.h
string.h exposes some issues, because the routines in it are both defined as inline functions that call out to assembly counterpart of the same name using an inconsistent __builtin_XXXX() convention. Sometimes the routines are available as _builtin___XXXX_chk and sometimes as __builtin_xxx.
I initially thought about limiting the macro magic purely for bionic/arm64, but there does not seem to be a nice way to do this.
Does this mean I need to touch all the .S files that are associated with string.h?
Even for the ones that are NOT for arm64?
Please take a look at:
so...@gmail.com <so...@gmail.com> #4
Could you please reiterate why is it not possible to fix this in TSan? If the problem is some call happening before __tsan_init, we just need to call __tsan_init right there. Most (if not all) interceptors have this logic.
Does it still happen if __tsan_init is called from .preinit_array?
Does it still happen if __tsan_init is called from .preinit_array?
ja...@gmail.com <ja...@gmail.com> #5
Its troublesome before __tsan_init because we lack __thread keyword support in bionic. I had to hack in a workaround in TSAN that does not use intercepted calls. (more on this below)
The majority of the problem actually occurs after __tsan_init has been called.
For example, pthread_create() calls a bunch of routines for the *new thread*, before it has had a chance to initialize its ThreadState. All of these routines running on the new thread are intercepted by tsan, and crashes because ThreadState has not been initialized yet.
A purely tsan-only fix is to "globally ignore" interceptors during troublesome times. This works fine for two threads, but doesn't scale to multiple threads - now you can miss events because they are being ignored.
I also tried putting in "global partial ignores" (i.e. ignore interceptors only on the new thread), but that got really hairy as well.
The fundamental problem is that TSAN is intercepting calls that it has no business intercepting.
For example, the same thing happens during thread join time, when one thread is being deconstructed.
Similar things happen during process exit time where ALL threads are being deconstructed.
Playing around with purely TSAN solution, all I managed to do was move around the actual place of the crash/hang/missed events.
The majority of the problem actually occurs after __tsan_init has been called.
For example, pthread_create() calls a bunch of routines for the *new thread*, before it has had a chance to initialize its ThreadState. All of these routines running on the new thread are intercepted by tsan, and crashes because ThreadState has not been initialized yet.
A purely tsan-only fix is to "globally ignore" interceptors during troublesome times. This works fine for two threads, but doesn't scale to multiple threads - now you can miss events because they are being ignored.
I also tried putting in "global partial ignores" (i.e. ignore interceptors only on the new thread), but that got really hairy as well.
The fundamental problem is that TSAN is intercepting calls that it has no business intercepting.
For example, the same thing happens during thread join time, when one thread is being deconstructed.
Similar things happen during process exit time where ALL threads are being deconstructed.
Playing around with purely TSAN solution, all I managed to do was move around the actual place of the crash/hang/missed events.
tn...@google.com <tn...@google.com> #6
On x86_64/linux, precisely zero calls are intercepted during pthread_create() on the new thread until it has finished initializing its ThreadState.
That does not happen on aarch64/bionic
That does not happen on aarch64/bionic
de...@google.com <de...@google.com> #7
Oh, the reason why I had to hack in a non-interceptible TLS workaround was that intercepted calls happen during pthread_key maintenance that occurs both during pthread_join time as well as at exit time.
What was happening when I was using pthread_get/setspecific() was that after a thread was destroyed, an intercepted call was GENERATING a NEW key for the dead thread. Hilarity ensued afterwards.
What was happening when I was using pthread_get/setspecific() was that after a thread was destroyed, an intercepted call was GENERATING a NEW key for the dead thread. Hilarity ensued afterwards.
Description
I propose adding a new annotation to support-annotations like @DeprecatedSdkVersion whose value property is an SDK_INT but also provides an optional details property for what method to use instead.
public @Interface DeprecatedSdkVersion {
int value();
String details = "";
}
Which can be used like this:
public final class ViewCompat {
@DeprecatedSdkVersion(Build.VERSION_CODES.LOLLIPOP)
public static int getElevation(View view) { ... }
@DeprecatedSdkVersion(
value = Build.VERSION_CODES.LOLLIPOP,
details = "Use view.getElevation() directly."
)
public static int getElevation(View view) { ... }
}
(Other potential names: @MaxSdkVersion, @UntilSdkVersion)
This would come with a corresponding lint check that would issue a warning if you were using one of these methods with a minSdkVersion higher than the annotation's value.