Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[monodroid] Prevent a segfault when Java class name is unavailable #5623

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

grendello
Copy link
Contributor

Context: #5619

It appears that Xamarin.Android application uploaded to Google Console
internal Test Track can fail the pre-launch test because the
MonodroidRuntime::get_java_class_name_for_TypeManager method, can
sometimes get a nullptr Java class name from JNI:

Build fingerprint: 'google/blueline/blueline:9/PQ3A.190801.002/5670241:user/release-keys'
Revision: 'MP1.0'
ABI: 'arm64'
pid: 15773, tid: 15773, name: com.Myapp  >>> com.Myapp<<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
Cause: null pointer dereference
    x0  0000000000000000  x1  0000000000000000  x2  0000000000000000  x3  00000000000000e1
    x4  0000000000000000  x5  0000000000000305  x6  0000000000000006  x7  0000000000000010
    x8  0101010101010101  x9  4adfd1b7ed2e23e2  x10 0000000000430000  x11 000000000000000a
    x12 0000000000000002  x13 0000000000000001  x14 00000000000000a8  x15 000000000000000a
    x16 0000007f703f00f8  x17 0000007f7031f290  x18 0000000000000008  x19 0000000000000000
    x20 0000007eecae0460  x21 0000000000000000  x22 0000000000000000  x23 0000000000000099
    x24 0000000000000085  x25 0000007eecb30010  x26 0000000000000099  x27 0000000000000000
    x28 0000007f724255e0  x29 0000007fe01ad010
    sp  0000007fe01acff0  lr  0000007f7036f3b4  pc  0000007f7031f2a0
backtrace:
    #00 pc 000000000001e2a0  /system/lib64/libc.so (strlen+16)
    #01 pc 000000000006e3b0  /system/lib64/libc.so (strdup+20)
    #02 pc 000000000000d4e8  /data/app/com.Myapp-1lufsJe3FIZ0YHoEAB08KA==/split_config.arm64_v8a.apk (offset 0x165d000) (xamarin::android::internal::MonodroidRuntime::get_java_class_name_for_TypeManager(_jclass*)+92)
    #03 pc 000000000005f6d0  <anonymous:0000007ecc4ac000>

This can happen if either one of the below points is true:

  1. The Java Class.getName
    method is absent
  2. Class.getName returns nullptr (a nameless class?)
  3. env->GetStringUTFChars returns nullptr (a memory allocation
    failure?)

Out of these, 3. appears to be most probable and this commit adds a
check for a nullptr pointer there, failing gracefully instead of
segfaulting. This is NOT a fix for the original problem as we don't
know by what it is caused but, nevertheless, the nullptr check should
be there.

@radekdoulik
Copy link
Member

I think another possibility (4.) is that the klass is not pointing to valid jclass object. It might make sense to print klass and name in the error message? Maybe the name is nullptr before the crash?

Context: dotnet#5619

It appears that Xamarin.Android application uploaded to Google Console
internal Test Track can fail the pre-launch test because the
`MonodroidRuntime::get_java_class_name_for_TypeManager` method, can
sometimes get a `nullptr` Java class name from JNI:

    Build fingerprint: 'google/blueline/blueline:9/PQ3A.190801.002/5670241:user/release-keys'
    Revision: 'MP1.0'
    ABI: 'arm64'
    pid: 15773, tid: 15773, name: com.Myapp  >>> com.Myapp<<<
    signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
    Cause: null pointer dereference
        x0  0000000000000000  x1  0000000000000000  x2  0000000000000000  x3  00000000000000e1
        x4  0000000000000000  x5  0000000000000305  x6  0000000000000006  x7  0000000000000010
        x8  0101010101010101  x9  4adfd1b7ed2e23e2  x10 0000000000430000  x11 000000000000000a
        x12 0000000000000002  x13 0000000000000001  x14 00000000000000a8  x15 000000000000000a
        x16 0000007f703f00f8  x17 0000007f7031f290  x18 0000000000000008  x19 0000000000000000
        x20 0000007eecae0460  x21 0000000000000000  x22 0000000000000000  x23 0000000000000099
        x24 0000000000000085  x25 0000007eecb30010  x26 0000000000000099  x27 0000000000000000
        x28 0000007f724255e0  x29 0000007fe01ad010
        sp  0000007fe01acff0  lr  0000007f7036f3b4  pc  0000007f7031f2a0
    backtrace:
        #00 pc 000000000001e2a0  /system/lib64/libc.so (strlen+16)
        #1 pc 000000000006e3b0  /system/lib64/libc.so (strdup+20)
        #2 pc 000000000000d4e8  /data/app/com.Myapp-1lufsJe3FIZ0YHoEAB08KA==/split_config.arm64_v8a.apk (offset 0x165d000) (xamarin::android::internal::MonodroidRuntime::get_java_class_name_for_TypeManager(_jclass*)+92)
        #3 pc 000000000005f6d0  <anonymous:0000007ecc4ac000>

This can happen if either one of the below points is true:

  1. The Java	[`Class.getName`](https://developer.android.com/reference/java/lang/Class?hl=en#getName())
     method is absent
  2. `Class.getName` returns `nullptr` (a nameless class?)
  3. `env->GetStringUTFChars` returns `nullptr` (a memory allocation
     failure?)

Out of these, 3. appears to be most probable and this commit adds a
check for a `nullptr` pointer there, failing gracefully instead of
segfaulting.  This is NOT a fix for the original problem as we don't
know by what it is caused but, nevertheless, the `nullptr` check should
be there.
@grendello
Copy link
Contributor Author

I think another possibility (4.) is that the klass is not pointing to valid jclass object. It might make sense to print klass and name in the error message? Maybe the name is nullptr before the crash?

if klass is not a valid jclass object, we won't be able to get the class name. The whole problem here is that name is nullptr, and we can't print it before we convert the Java's internal representation to utf8 which is what appears to fail. I included (2) for completeness, but I doubt this is really what happens. Java supports anonymous classes, but I suppose that they are given some unique name internally and I don't think our Java generator uses them? Also, the same code works correctly locally so my suspicion is that we're observing (3).

However, I updated the code to at least print an error when jstring name is nullptr, including the klass pointer value (which will probably be useless, but it won't hurt to include it)

@jonpryor jonpryor merged commit d8df924 into dotnet:master Feb 18, 2021
@grendello grendello deleted the issue-5619 branch February 18, 2021 12:56
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants