Opened on 05/12/2017 at 11:52:01 AM
Closed on 05/12/2017 at 03:44:29 PM
#5239 closed defect (fixed)
JsValue access crash
Reported by: | asmirnov | Assignee: | |
---|---|---|---|
Priority: | P2 | Milestone: | |
Module: | Libadblockplus-Android | Keywords: | |
Cc: | sergz | Blocked By: | |
Blocking: | Platform: | Android | |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
Description (last modified by sergz)
Environment
In Java classes we keep a pointer to a native (C++) class in a Java long (64 bits) member, however, in order to construct that Java class we call JNIEnv::NewObject(jclass, jmethodId, ...) method which is essentially calling Java constructor accepting Java long. This method JNIEnv::NewObject takes a variable number of arguments and converts them to corresponding Java values, so it tries to read the value as 64 bit value but currently we are passing a 32 bit value (our current builds for ARM are 32bit, armeabi-v7a, therefore pointer size is 32 bits), so it reads some irrelevant value.
What to change
Before passing a pointer value to JNIEnv::NewObject it should be converted into corresponding 64 bit value jlong (aka long long).
In order to do it for 32 bit platform, we firstly need to say that it is unsigned integer (e.g. uintptr_t) and then convert it to 64 bit jlong (aka long long) because a 32 bit unsigned value can be represented in a 64 bit signed value (see https://isocpp.org/std/the-standard, e.g. from 4.7 Integral conversions of November 2014 working draft:
If the destination type is signed, the value is unchanged if it can be represented in the destination type; otherwise, the value is implementation-defined.
).
For 64 bit platform, according to the standard the value is implementation-defined and it seems the implementation with current compilers is fine, namely it leaves the bits unchanged, so we likely don't have to do anything with it, to have tests would be useful though (is someone willing to share the assembly code (for release of course), could be interesting to check it).
Conversion from signed to unsigned (jlong -> pointer) is defined
If the destination type is unsigned, the resulting value is the least unsigned integer congruent to the source integer (modulo 2n where n is the number of bits used to represent the unsigned type). [ Note: In a two’s complement representation, this conversion is conceptual and there is no change in the bit pattern (if there is no truncation). — end note ]
so, nothing to do with it.
How to reproduce
- Run AdblockWebView app on hardware Android device (not emulator)
- Wait for the crash
...
Observed behaviour
The app crashes
Expected behaviour
The app does not crash
Attachments (0)
Change History (4)
comment:1 Changed on 05/12/2017 at 11:58:13 AM by asmirnov
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 05/12/2017 at 03:43:47 PM by abpbot
A commit referencing this issue has landed:
Issue 5239 - JsValue access crash
comment:4 Changed on 05/12/2017 at 03:44:29 PM by asmirnov
- Resolution set to fixed
- Status changed from reviewing to closed
Added more details about it.