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

Check for String.offset < API 23 #350

Merged
merged 1 commit into from
Jan 2, 2016
Merged

Check for String.offset < API 23 #350

merged 1 commit into from
Jan 2, 2016

Conversation

jrodbx
Copy link
Collaborator

@jrodbx jrodbx commented Dec 30, 2015

As of M, the int offset and char[] value fields were removed from String.java and could not be parsed in heap dumps.

However, in Preview 2, the char buffer was still serialized in the heap dump, 16 bytes after the String address. This commit restored parsing of M Preview 2 heap dumps.

After Android M Preview 2, a virtual char[] value field was inserted in all String objects when dumping the heap. This resulted in execution of the pre-M code path which expected the "offset" field defined in the original String.java. This commit accounts for the third case.

@jrodbx jrodbx changed the title Check for String.offset > M Preview 2 Check for String.offset < API 23 Dec 30, 2015
import static org.junit.Assert.fail;

public class HahaHelperTest {
public static final int STRING_CLASS_ID = 100;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Private static fields

@pyricau
Copy link
Member

pyricau commented Dec 31, 2015

LGTM. Once updated with feedback, feel free to merge.

@jrodbx
Copy link
Collaborator Author

jrodbx commented Dec 31, 2015

updated per feedback

@jrodbx jrodbx self-assigned this Dec 31, 2015

@Override
public int readInt() {
if(intsToRead == null || intIndex < 0 || intIndex >= intsToRead.length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code not properly formatted (missing space after if)

* Added check for offset field in String class for versions < API 23
* Added heap dump for M post-Preview 2
* Added unit test to cover String instance cases
@jrodbx
Copy link
Collaborator Author

jrodbx commented Jan 2, 2016

updated per feedback

jrodbx added a commit that referenced this pull request Jan 2, 2016
Check for String.offset < API 23
@jrodbx jrodbx merged commit b8e444b into square:master Jan 2, 2016
@jrodbx jrodbx deleted the check-string-offset branch January 2, 2016 18:52
@pyricau pyricau added this to the 1.4 milestone Jan 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants