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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,22 @@ static String asString(Object stringObject) {
ArrayInstance charArray;
if (isCharArray(value)) {
charArray = (ArrayInstance) value;
offset = fieldValue(values, "offset");
offset = 0;
// < API 23
// As of Marshmallow, substrings no longer share their parent strings' char arrays
// eliminating the need for String.offset
// https://android-review.googlesource.com/#/c/83611/
if (hasField(values, "offset")) {
offset = fieldValue(values, "offset");
}
} else {
// In M preview 2+, the underlying char buffer resides in the heap with ID equalling the
// In M preview 2, the underlying char buffer resides in the heap with ID equaling the
// String's ID + 16.
// https://android-review.googlesource.com/#/c/160380/2/android/src/com/android/tools/idea/
// editors/hprof/descriptors/InstanceFieldDescriptorImpl.java
// This workaround is only needed for M preview 2, as it has been fixed on the hprof
// generation end by reintroducing a virtual "value" variable.
// https://android.googlesource.com/platform/art/+/master/runtime/hprof/hprof.cc#1242
Heap heap = instance.getHeap();
Instance inlineInstance = heap.getInstance(instance.getId() + 16);
if (isCharArray(inlineInstance)) {
Expand Down Expand Up @@ -143,6 +153,16 @@ static <T> T fieldValue(List<ClassInstance.FieldValue> values, String fieldName)
throw new IllegalArgumentException("Field " + fieldName + " does not exists");
}

static boolean hasField(List<ClassInstance.FieldValue> values, String fieldName) {
for (ClassInstance.FieldValue fieldValue : values) {
if (fieldValue.getField().getName().equals(fieldName)) {
//noinspection unchecked
return true;
}
}
return false;
}

private HahaHelper() {
throw new AssertionError();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public class AsyncTaskLeakTest {
return Arrays.asList(new Object[][] {
{ fileFromName("leak_asynctask.hprof"), "dc983a12-d029-4003-8890-7dd644c664c5" },
{ fileFromName("leak_asynctask_mpreview2.hprof"), "1114018e-e154-435f-9a3d-da63ae9b47fa" },
{ fileFromName("leak_asynctask_m_postpreview2.hprof"), "25ae1778-7c1d-4ec7-ac50-5cce55424069" }
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package com.squareup.leakcanary;

import com.squareup.haha.perflib.io.HprofBuffer;

import java.io.UnsupportedEncodingException;
import java.util.List;

public final class FakeHprofBuffer implements HprofBuffer {
private List<Byte> byteList;
private List<byte[]> byteArrayList;

private int[] intsToRead;
private int intIndex = -1;
private String[] stringsToRead;
private int stringIndex = -1;

public void setIntsToRead(int... ints) {
intsToRead = ints;
intIndex = 0;
}

public void setStringsToRead(String... strings) {
stringsToRead = strings;
stringIndex = 0;
}

@Override
public byte readByte() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public void read(byte[] bytes) {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public void readSubSequence(byte[] bytes, int start, int length) {
if (stringsToRead == null || stringIndex < 0 || stringIndex >= stringsToRead.length) {
throw new UnsupportedOperationException("no bytes to read");
}

String s = stringsToRead[stringIndex++];
try {
System.arraycopy(s.getBytes("UTF-16BE"), start, bytes, 0, length);
} catch (UnsupportedEncodingException e) {
throw new UnsupportedOperationException(e);
}
}

@Override
public char readChar() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public short readShort() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public int readInt() {
if (intsToRead == null || intIndex < 0 || intIndex >= intsToRead.length) {
throw new UnsupportedOperationException("no bytes to read");
}
return intsToRead[intIndex++];
}

@Override
public long readLong() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public float readFloat() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public double readDouble() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public void setPosition(long l) {}

@Override
public long position() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public boolean hasRemaining() {
throw new UnsupportedOperationException("no bytes to read");
}

@Override
public long remaining() {
throw new UnsupportedOperationException("no bytes to read");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
package com.squareup.leakcanary;

import com.squareup.haha.perflib.ArrayInstance;
import com.squareup.haha.perflib.ClassInstance;
import com.squareup.haha.perflib.ClassObj;
import com.squareup.haha.perflib.Field;
import com.squareup.haha.perflib.Snapshot;
import com.squareup.haha.perflib.Type;

import org.junit.Before;
import org.junit.Test;

import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

public class HahaHelperTest {
private static final int STRING_CLASS_ID = 100;
private static final int CHAR_ARRAY_CLASS_ID = 101;
private static final int STRING_INSTANCE_ID = 102;
private static final int VALUE_ARRAY_INSTANCE_ID = 103;

private static final int VALUE_ARRAY_LENGTH = 6;
private static final int COUNT_VALUE = 5;
private static final int OFFSET_VALUE = 1;

private FakeHprofBuffer buffer;
private Snapshot snapshot;

@Before
public void setUp() {
buffer = new FakeHprofBuffer();

snapshot = new Snapshot(buffer);
// set HPROF identifier size; required for Object instance field lookups
// cf. https://java.net/downloads/heap-snapshot/hprof-binary-format.html
snapshot.setIdSize(4);
Copy link
Member

Choose a reason for hiding this comment

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

What does this do? Is that in other tests too? Seems weird, should be shared in test helper code and commented.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Snapshot.setIdSize(int) sets the size of Type.OBJECT (originally init'd to 0) and initializes mTypeSizes.

This method is called by perflib's HprofParser here early in the HPROF parsing process according to the format spec.

In test code, not calling Snapshot.setIdSize(int) leaves mTypeSizes unallocated resulting in an NPE when ClassInstance.getValues() attempts to read the instance for the "value" field in String.java.

Although Snapshot does not currently enforce it as a constraint, all instances of Snapshot should set the identifier size.

}

@Test
public void readStringOffsetFromHeapDumpInstance() {
buffer.setIntsToRead(COUNT_VALUE, OFFSET_VALUE, VALUE_ARRAY_INSTANCE_ID);
buffer.setStringsToRead("abcdef");

addStringClassToSnapshotWithFields(snapshot, new Field[]{
new Field(Type.INT, "count"),
new Field(Type.INT, "offset"),
new Field(Type.OBJECT, "value")
});

ClassInstance stringInstance = createStringInstance();
createCharArrayValueInstance();

String actual = HahaHelper.asString(stringInstance);
assertTrue(actual.equals("bcdef"));
}

@Test
public void defaultToZeroStringOffsetWhenHeapDumpInstanceIsMissingOffsetValue() {
buffer.setIntsToRead(COUNT_VALUE, VALUE_ARRAY_INSTANCE_ID);
buffer.setStringsToRead("abcdef");

addStringClassToSnapshotWithFields(snapshot, new Field[]{
new Field(Type.INT, "count"),
new Field(Type.OBJECT, "value")
});

ClassInstance stringInstance = createStringInstance();
createCharArrayValueInstance();

String actual = HahaHelper.asString(stringInstance);
assertTrue(actual.equals("abcde"));
}

@Test
public void defaultToZeroStringOffsetWhenReadingMPreview2HeapDump() {
buffer.setIntsToRead(COUNT_VALUE, OFFSET_VALUE, VALUE_ARRAY_INSTANCE_ID);
buffer.setStringsToRead("abcdef");

addStringClassToSnapshotWithFields(snapshot, new Field[]{
new Field(Type.INT, "count"),
new Field(Type.INT, "offset"),
new Field(Type.OBJECT, "value")
});

ClassInstance stringInstance = createStringInstance();
createCharArrayValueInstance_M_Preview2();

String actual = HahaHelper.asString(stringInstance);
assertTrue(actual.equals("abcde"));
}

@Test
public void throwExceptionWhenMissingCharArrayValueForStringInMPreview2HeapDump() {
buffer.setIntsToRead(COUNT_VALUE, OFFSET_VALUE, VALUE_ARRAY_INSTANCE_ID);
buffer.setStringsToRead("abcdef");

addStringClassToSnapshotWithFields(snapshot, new Field[]{
new Field(Type.INT, "count"),
new Field(Type.INT, "offset"),
new Field(Type.OBJECT, "value")
});

ClassInstance stringInstance = createStringInstance();
createObjectValueInstance_M_Preview2();

try {
HahaHelper.asString(stringInstance);
fail("this test should have thrown UnsupportedOperationException");
}
catch (UnsupportedOperationException uoe) {
String message = uoe.getMessage();
assertTrue(message.equals("Could not find char array in " + stringInstance));
}
}

private void addStringClassToSnapshotWithFields(Snapshot snapshot, Field[] fields) {
ClassObj charArrayClass = new ClassObj(0, null, "char[]", 0);
snapshot.addClass(CHAR_ARRAY_CLASS_ID, charArrayClass);

ClassObj stringClass = new ClassObj(0, null, "string", 0);
stringClass.setFields(fields);
snapshot.addClass(STRING_CLASS_ID, stringClass);
}

private void createCharArrayValueInstance() {
ArrayInstance valueArrayInstance = new ArrayInstance(0, null, Type.CHAR, VALUE_ARRAY_LENGTH, 0);
snapshot.addInstance(VALUE_ARRAY_INSTANCE_ID, valueArrayInstance);
}

private void createCharArrayValueInstance_M_Preview2() {
ArrayInstance valueInstance = new ArrayInstance(0, null, Type.CHAR, VALUE_ARRAY_LENGTH, 0);
snapshot.addInstance(STRING_INSTANCE_ID + 16, valueInstance);
}

private void createObjectValueInstance_M_Preview2() {
ClassInstance valueInstance = new ClassInstance(0, null, 0);
snapshot.addInstance(STRING_INSTANCE_ID + 16, valueInstance);
}

private ClassInstance createStringInstance() {
ClassInstance stringInstance = new ClassInstance(STRING_INSTANCE_ID, null, 100);
stringInstance.setClassId(STRING_CLASS_ID);
snapshot.addInstance(0, stringInstance);
return stringInstance;
}
}
Binary file not shown.