+cavalcantii@, noel@, cblume@:

This seems to be related to the recent patch on crc32 with Armv8 optimization: https://chromium-review.googlesource.com/c/chromium/src/+/801108

Could you take a look?

This might be device-specific. Most reports are coming from Redmi Note 4 and Redmi Note 4X.

It looks like the Clank team has a Redmi Note 4 which I can test on.

I have a theory given the weird call stack & BUS_ADRERR.

__dl_strcmp
__dl__ZL14find_librariesP6soinfoPKPKcmPS0_PNSt3__16vectorIS0_NS6_9allocatorIS0_EEEEmiPK17android_dlextinfo
__dl__Z9do_dlopenPKciPK17android_dlextinfo
__dl_dlopen
get_elf_hwcap_from_getauxval (inside the NDK)
pthread_once
android_getCpuFeatures (our function)

The NDK function android_getCpuFeatures calls get_elf_hwcap_from_getauxval, which in turn calls dlopen.

OpenCV has some interesting code that I previously didn't fully comprehend. But I now thing it is related to this. They mention that calling dlopen() inside a call to dlopen() fails. And their code works around it: https://github.com/opencv/opencv/blob/master/3rdparty/cpufeatures/cpu-features.c#L519


This call stack has a base of __start_thread.

My theory is some app is calling dlopen("libcronet.so") and part of Cronet's init is calling Cr_z_crc32 (which in turn is calling another dlopen because of the NDK).



Later, I'll take a look at creating a repro case and then confirming this theory.

Correction:

android_getCpuFeatures (the API exposed by the NDK which our function calls)

@cblume: interesting theory (and it makes sense).

/me just back from vacations and still catching up with the never ending pile of email.

I don't have access to a Redmi Note 4 device, but please let me know if there is anything we could do to help.

This is off PostTaskAndReply, though, in a worker thread. Cronet initialization is an explicit function call, too, isn't it?

I don't see a specific Cronet initialization here. But you are right that this thread doesn't have a prior dlopen() call.

I don't think it is one-per-thread though. I think it is one-per-process. I am guessing at this but I believe if another thread is in the middle of a dlopen() call and this thread happens to also call dlopen() we'll encounter this issue.


Is there a way we can check of the Cronet users could possibly have two threads both calling dlopen()?

There may be some not-so-lovely workarounds available. Given the call stack, it is clear that android_getCpuFeatures() is just calling getauxval(). We could use the getauxval() code path on Android  like we do on Linux. Or we could do what Chrome does and have an explicit initialization step which will call into android_getCpuFeatures() so we know the initialization steps happen in serial and there are no parallel calls to dlopen().

It's very possible that we have a parallel call to dlopen().
+pauljensen@: looks like you are on sheriff duty this week, could you help with adding android_getCpuFeatures() to Cronet's initialization step?

I may have said something in an unclear way.

I am not sure if an initialization call to android_getCpuFeatures() will cause future calls to no longer call dlopen(). I would assume so but I don't know that.

What I was suggesting is an initialization call into zlib so it can make its call into android_getCpuFeatures() and never need to call it again.

All reports come from API level 23, Marshmallow.  99.69% of reports come from Redmi Note 4 and 4X.

+dimitry@google.com who has worked a lot on bionic's dynamic linker.

Dimitry: are you aware of any Marshmallow-only dlopen() crashes with similar call-stacks?

tl;dr; if this is a bug I am thinking about this happens on unsuccessful dlopen with RTLD_NODELETE flag or if library has DF_1_NODELETE flag set and this was fixed for NYC.

https://b.corp.google.com/issues/27911891 is the bug in question. I do not remember is this was regression or bug exists in mnc as well, sounds like it might have. If this is the case removing NODELETE flag should result in normal dlerror().

It's Android NDK code:
  http://androidxref.com/6.0.0_r1/xref/ndk/sources/android/cpufeatures/cpu-features.c#506
that looks like this:
  dlopen("libc.so", RTLD_NOW)
so I don't imagine it would be failing to load libc and it doesn't appear to use RTLD_NODELETE.

Ah.. yes. It fails on any subsequent dlopens.

The problem is that previous unsuccessful dlopen failed to clear internal linker structures and left them in the state where next access to get_soinfo() results in attempt to access unmmaped memory.

Are there any NODELETE libraries in the project?

It looks like the only NODELETE dlopen calls in Chromium are in media/ and content/ which aren't included in Cronet:
https://cs.chromium.org/search/?q=RTLD_NODELETE+file:%5C.cc&sq=package:chromium&type=cs

However, the app using Cronet (see b/110262915) could be using NODELETE, though a quick search didn't turn up any results.

Is there any way to prevent the crash?  like issuing a failed dlopen() without NODELETE?

yes, removing NODELETE flag from dlopen will fix this. Come to think of it the failing dlopen with NODELETE should not crash. It should result in normal dlerror(). But all subsequent dlopens are doomed...

That media call makes me wonder if something in the HAL or something for the device may be using that flag... is there any way to check?

I looked quickly through Android Marshmallow source but I can't see into the device-specific HAL stuff.  Unfortunately I don't know where the NODELETE dlopen call is...
It's interesting that dlopen for libcronet succeeds, but then later the dlopen call from Cronet fails.  I have a fix that perform the zlib initialization earlier, but I've no way of knowing if we'd hit this crash later from some other dlopen call in Cronet.

Here's my proposed fix:
https://chromium-review.googlesource.com/c/chromium/src/+/1110377
I'm trying to see if I can get a hold of a Redmi Note 4 to test.

I'm sure you have all considered this but I haven't seen it said yet so I felt the need to say it.

Since this is only happening on one set of devices for one version of Android, there is a very high chance that the device includes modifications to the AOSP which introduced a bug.

Is that perhaps what you are saying with the HAL stuff? Maybe the bug is in the HAL layer?

@paul: my understanding is that cronet does CPU features detection to enable optimizations?

Assuming that the chain of calls allowed (i.e Android app calls dlopen() to load cronet and nothing else calls into zlib), why not pass on to zlib the CPU features information?

We could have a state variable in zlib (i.e. arm_detect_cpu_features) that if set to 'false' we simply skip the call into the NDK function. Then it would be up to cronet to set the flag that enables the optimized code (https://cs.chromium.org/chromium/src/third_party/zlib/arm_features.c?l=22).

One concern in zlib is that it runs in both a privileged level in Chromium (e.g. network operations) as also in a sandboxed level (i.e. in RenderProcess when we perform PNG decoding).

One advantage of implementing this flag in zlib is that it would allow to perform experiments (i.e. disable/enable optimizations). Another advantage is that it would allow to avoid doing the same work twice in zlib (i.e. detect CPU features) in the context of cronet.

Assuming this design is acceptable, it would require one change in cronet and one change in zlib. I would be more than happy to help on this one.

> Is that perhaps what you are saying with the HAL stuff? Maybe the bug is in the HAL layer?

It's possible there is a bug in the HAL, or perhaps the HAL is loading a library with RTLD_NODELETE.  However I suspect updating Cronet may be the most expeditious way to avoid this crash.

> @paul: my understanding is that cronet does CPU features detection to enable optimizations?

Various pieces of Cronet do CPU feature detection (e.g. BoringSSL, zlib etc).

We could have a way to pass CPU feature detection flags into zlib.  I think I'd prefer to try my simpler less-risky fix first to see if it fixes the issue before investing more time going down this route.  A simple safer fix also is more amenable to merging into release branches.

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

commit fccbefeaeb30340b3f4240cec51088fb824581eb
Author: Paul Jensen <pauljensen@chromium.org>
Date: Mon Jun 25 16:23:44 2018

[Cronet] Initialize zlib ARM CPU feature detection early

Later attempts to perform this initialization seem to fail
potentially because of a dlopen() bug, so perform the
initialization early, right after dlopen() on libcronet
succeeded.

Bug:  853725 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I9132dff672f91d7962909531f01c6bada4fee964
Reviewed-on: https://chromium-review.googlesource.com/1110377
Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Reviewed-by: Adenilson Cavalcanti <cavalcantii@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570057}
[modify] https://crrev.com/fccbefeaeb30340b3f4240cec51088fb824581eb/components/cronet/DEPS
[modify] https://crrev.com/fccbefeaeb30340b3f4240cec51088fb824581eb/components/cronet/android/BUILD.gn
[modify] https://crrev.com/fccbefeaeb30340b3f4240cec51088fb824581eb/components/cronet/android/cronet_library_loader.cc

#6 "We could use the getauxval() code path on Android like we do on Linux."  Interesting idea: wonder if that'd work?  Indeed, any reasons I'm missing why it would not work on Android?  

This bug has an owner, thus, it's been triaged. Changing status to "assigned".

Marking fixed per comment 22.