Merge pull request #2959 from square/harden-hprof-stripping

Strip heap dumps that Android wrote, arrays too big for an Int, and gzipped ones

Author: pyricau

docs/changelog.md

@@ -106,6 +106,13 @@ Releases before 2.8.1 predate these markers.
 * 🔨 The LeakCanary task in Android Studio's Profiler keeps working on LeakCanary 3. Studio drives LeakCanary through a bridge library it injects into the app, [`com.android.tools.studio.leakcanary:leakcanary`](https://android.googlesource.com/platform/tools/base/+/refs/heads/mirror-goog-studio-main/studio-leakcanary/), which has no compile time dependency on LeakCanary and instead resolves every class, method and field it needs reflectively, by name, giving up on the whole integration at the first failed lookup — at which point Studio reports LeakCanary as missing from the app. All of those names survived LeakCanary 3 except the default `GcTrigger`, which used to be an object nested in `GcTrigger` and compiled to `leakcanary.GcTrigger$Default`, and is now `GcTrigger.inProcess()`. A class under the old name is published again for that lookup alone, deprecated so that no source reaches for it and documented as un-deletable. `AndroidStudioProfilerContractTest` replays the bridge's lookups against the `leakcanary-android` artifact, so the next rename fails there rather than silently in the IDE.
 * ⚠️ The hash that identifies a leak is now called its **leak fingerprint**: `LeakTrace.signature` and `Leak.signature` are `LeakTrace.leakFingerprint` and `Leak.leakFingerprint`, and `EventListener.Event.HeapAnalysisDone.HeapAnalysisSucceeded.unreadLeakSignatures` is `unreadLeakFingerprints`. The hash itself is unchanged — same input, same SHA-1 — so a leak keeps the string it already had, and a bug report or a dashboard grouped by it still lines up across the upgrade. The rename is about the word: *signature* is what a Kotlin or Java developer meeting it for the first time reads as a method signature, or as the signature an APK is signed with, and both of those also appear in LeakCanary's own code and change log. It also never said what the hash is *for*, which is that two leaks with the same one are caused by the same bug.
     * 🔀 A heap analysis printed to Logcat or shared as text now reads `Leak fingerprint: <hash>` where it read `Signature: <hash>`, and *"Displaying only 1 leak trace out of N with the same leak fingerprint"* where it read *"…with the same signature"*. Anything parsing that text needs updating.
+* 🐛 `HprofPrimitiveArrayStripper` left most primitive wrapper instances wrapping their real value when stripping a heap dump taken from Android, so `HeapAnalysisConfig(stripHeapDump = true)` and `shark-cli strip-hprof` both fell back to zeroing primitive arrays alone. Zeroing a wrapped value needs the field layout of the wrapper class, which the stripper learned from that class's `CLASS_DUMP` record, so it could only strip the instances dumped after it — and ART walks the heap in memory order, which puts the class dump of `java.lang.Integer` after most of the `Integer` instances. On the Android heap dumps in our test resources, 1175 of the 1291 wrapper instances of `leak_asynctask_o.hprof` and 3289 of the 3365 of `leak_asynctask_m.hprof` kept their value. HotSpot writes every class dump ahead of any instance, so heap dumps taken from a JVM — which is all the test covering this used — were stripped correctly. The class id of each wrapper now comes from its `LOAD_CLASS` record, which both ART and HotSpot write before the heap dump starts, and the value is zeroed at the start of the instance field values, which is where the single field a wrapper declares sits: a class declares its own fields ahead of the ones it inherits. The `CLASS_DUMP` record is still read, to check that layout and fail loudly rather than write a heap dump that looks stripped and isn't.
+* 💥 [#2777](https://github.com/square/leakcanary/issues/2777) `HprofPrimitiveArrayStripper` failed with a `NegativeArraySizeException` on a heap dump holding a primitive array whose content takes more than 2 GB, which is a `long` or `double` array of more than 268435455 elements. Fixing the parser for that issue covered the object array side of the stripper and left the primitive array side alone: the bytes written over an array's content were allocated as one array sized `arrayLength * elementByteSize` in `Int` arithmetic, which wraps negative that far out, so a `long[268435456]` asked for an array of -2147483648 bytes. Those bytes are now written by repeating an 8192 byte pattern over the content, so nothing is sized from the array at all. Verified on a 2.15 GB heap dump taken from a JVM holding a `long[268435456]`, which strips in 3 seconds.
+    * 🔨 Not allocating a copy of every array also takes the memory stripping needs from a function of the largest array in the heap dump down to a constant, which matters on Android, where `HeapAnalysisConfig(stripHeapDump = true)` strips in the app's own process. Stripping a 110 MB heap dump holding a `byte[104857600]` used to need a 216 MB heap and completes in 8 MB now. On a 294 MB heap dump pulled from a real app, the bytes allocated go from 374 MB to 34 MB, and stripping is about 5% faster.
+* 💥 Reading or stripping a heap dump with 8 byte identifiers that holds a heap dump info record failed with *"Unknown tag 0x00"*. That record holds an `Int` heap id and then a string id, and both the reader skipping it and the stripper copying it over treated the heap id as an id too, so with 8 byte identifiers they moved 4 bytes too far and read the middle of whatever came next as a record tag. Heap dump info records only appear in heap dumps written by Android, which always uses 4 byte identifiers — where the two sizes are the same and the bug can't show — so this is about a heap dump written by `HprofWriter`.
+* ✨ `shark-cli strip-hprof` and `HprofPrimitiveArrayStripper.stripPrimitiveArrays(File)` now handle gzip on both ends: a heap dump whose content is gzipped is read gzipped whatever it's named, and the output is written gzipped when its name ends with ".gz", which is what the default output name of a ".hprof.gz" input already gave you — so "app.hprof.gz" strips to a gzipped "app-stripped.hprof.gz". The Android heap dumps in our test resources compress 3.9x to 4.4x, and 5.6x to 5.8x once stripped, so a heap dump that's been shared is usually gzipped by the time you get it, and stripping it meant gunzipping it first and gzipping the result again.
+    * ✨ `StreamingSourceProvider.gunzipIfGzipped()` and `StreamingSinkProvider.gzip()` are what do that. They compose onto any source or sink, so a caller of the `stripPrimitiveArrays` overload that takes a source and a sink can opt into the same behavior, or into only one half of it.
+* ✨ `HprofPrimitiveArrayStripper` and `shark-cli strip-hprof` now say what stripping leaves behind. Everything that isn't a primitive array or a wrapped primitive is copied over unchanged, and that includes the string records holding the class, field and method names the rest of the heap dump refers to. Those hold no runtime data in a heap dump from Android, but a heap dump from a JVM also holds every string constant of every loaded class in them, so stripping a JVM heap dump leaves the constants written in the code behind.
 
 ## Version 3.0 Alpha 9 (2026-06-25)
 

shark/shark-android/src/test/java/shark/LegacyHprofTest.kt

@@ -13,6 +13,14 @@ import shark.LegacyHprofTest.WRAPS_ACTIVITY.DESTROYED
 import shark.LegacyHprofTest.WRAPS_ACTIVITY.NOT_ACTIVITY
 import shark.LegacyHprofTest.WRAPS_ACTIVITY.NOT_DESTROYED
 import shark.SharkLog.Logger
+import shark.ValueHolder.BooleanHolder
+import shark.ValueHolder.ByteHolder
+import shark.ValueHolder.CharHolder
+import shark.ValueHolder.DoubleHolder
+import shark.ValueHolder.FloatHolder
+import shark.ValueHolder.IntHolder
+import shark.ValueHolder.LongHolder
+import shark.ValueHolder.ShortHolder
 
 class LegacyHprofTest {
 
@@ -62,6 +70,31 @@ class LegacyHprofTest {
     }
   }
 
+  @Test fun androidOStrippedWrapsZeroes() {
+    val stripper = HprofPrimitiveArrayStripper()
+    val sourceHprof = "leak_asynctask_o.hprof".classpathFile()
+    val strippedHprof = stripper.stripPrimitiveArrays(sourceHprof)
+
+    assertThat(readWrappedPrimitives(sourceHprof)).anyMatch { (className, value) ->
+      value != ZERO_VALUE_BY_WRAPPER_CLASS_NAME.getValue(className)
+    }
+    assertThat(readWrappedPrimitives(strippedHprof)).isNotEmpty.allMatch { (className, value) ->
+      value == ZERO_VALUE_BY_WRAPPER_CLASS_NAME.getValue(className)
+    }
+  }
+
+  private fun readWrappedPrimitives(hprofFile: File): List<Pair<String, ValueHolder>> {
+    return hprofFile.openHeapGraph().use { graph ->
+      ZERO_VALUE_BY_WRAPPER_CLASS_NAME.keys.flatMap { className ->
+        val wrapperClass = graph.findClassByName(className) ?: return@flatMap emptyList()
+        wrapperClass.instances.map { instance ->
+          className to instance[className, "value"]!!.value.holder
+        }
+          .toList()
+      }
+    }
+  }
+
   private fun readThreadNames(hprofFile: File): List<String> {
     return hprofFile.openHeapGraph().use { graph ->
       graph.findClassByName("java.lang.Thread")!!.instances.map { instance ->
@@ -273,4 +306,18 @@ class LegacyHprofTest {
     println(analysis)
     return analysis as HeapAnalysisSuccess
   }
+
+  companion object {
+    private val ZERO_VALUE_BY_WRAPPER_CLASS_NAME: Map<String, ValueHolder> =
+      mapOf(
+        "java.lang.Boolean" to BooleanHolder(false),
+        "java.lang.Byte" to ByteHolder(0),
+        "java.lang.Character" to CharHolder(0.toChar()),
+        "java.lang.Short" to ShortHolder(0),
+        "java.lang.Integer" to IntHolder(0),
+        "java.lang.Long" to LongHolder(0),
+        "java.lang.Float" to FloatHolder(0f),
+        "java.lang.Double" to DoubleHolder(0.0)
+      )
+  }
 }

shark/shark-cli/src/main/java/shark/StripHprofCommand.kt

@@ -6,7 +6,18 @@ import shark.SharkCliCommand.Companion.sharkCliParams
 
 class StripHprofCommand : CliktCommand(
   name = "strip-hprof",
-  help = "Replace all primitive arrays from the provided heap dump with arrays of zeroes and generate a new \"-stripped.hprof\" file."
+  help = """
+    |Replace all primitive arrays from the provided heap dump with arrays of zeroes and generate a new "-stripped.hprof" file.
+    |
+    |Primitive wrapper instances are updated to wrap 0 as well. Everything else is copied over
+    |unchanged, which includes the string records holding the class, field and method names the rest
+    |of the heap dump refers to. Those hold no runtime data in a heap dump from Android, but a heap
+    |dump from a JVM also holds every string constant of every loaded class in them, so stripping a
+    |JVM heap dump leaves the constants written in the code behind.
+    |
+    |A gzipped heap dump is read gzipped and written back gzipped, so "app.hprof.gz" gives you
+    |"app-stripped.hprof.gz".
+    """.trimMargin()
 ) {
 
   override fun run() {

shark/shark-graph/src/test/java/shark/HprofPrimitiveArrayStripperTest.kt

@@ -52,6 +52,38 @@ class HprofPrimitiveArrayStripperTest {
     assertThat(strippedBuffer.readByteArray()).isEqualTo(expectedByteArray)
   }
 
+  @Test
+  fun `wrapped primitive stripped when the wrapper class is dumped after its instances`() {
+    val sourceByteArray =
+      Buffer()
+        .apply {
+          writeRawTestHprof(
+            secretLongArray = longArrayOf(0xCAFE, 0xDAD),
+            secretCharArray = charArrayOf('P', 'Y'),
+            secretWrappedLong = 42,
+            wrapperClassDumpedAfterInstance = true,
+          )
+        }
+        .readByteArray()
+
+    val strippedBuffer = Buffer()
+    val stripper = HprofPrimitiveArrayStripper()
+    stripper.stripPrimitiveArrays(ByteArraySourceProvider(sourceByteArray), { strippedBuffer })
+
+    val expectedByteArray =
+      Buffer()
+        .apply {
+          writeRawTestHprof(
+            secretLongArray = longArrayOf(0, 0),
+            secretCharArray = charArrayOf('?', '?'),
+            secretWrappedLong = 0,
+            wrapperClassDumpedAfterInstance = true,
+          )
+        }
+        .readByteArray()
+    assertThat(strippedBuffer.readByteArray()).isEqualTo(expectedByteArray)
+  }
+
   @Test
   fun `ByteArray based String content is replaced with question marks`() {
     val hprofFolder = testFolder.newFolder()
@@ -114,6 +146,11 @@ class HprofPrimitiveArrayStripperTest {
     secretLongArray: LongArray,
     secretCharArray: CharArray,
     secretWrappedLong: Byte,
+    /**
+     * Android walks the heap in memory order when it dumps it, so a class is often dumped after
+     * instances of that class.
+     */
+    wrapperClassDumpedAfterInstance: Boolean = false,
   ) {
     HprofWriter.openWriterFor(
         this,
@@ -152,7 +189,7 @@ class HprofPrimitiveArrayStripperTest {
             fields = emptyList(),
           )
         )
-        writer.write(
+        val wrapperClassDump =
           ClassDumpRecord(
             id = 2,
             stackTraceSerialNumber = 0,
@@ -164,7 +201,9 @@ class HprofPrimitiveArrayStripperTest {
             staticFields = emptyList(),
             fields = listOf(FieldRecord(3, PrimitiveType.LONG.hprofType)),
           )
-        )
+        if (!wrapperClassDumpedAfterInstance) {
+          writer.write(wrapperClassDump)
+        }
         writer.write(LongArrayDump(id = 4, stackTraceSerialNumber = 0, array = secretLongArray))
         writer.write(
           InstanceDumpRecord(
@@ -174,6 +213,9 @@ class HprofPrimitiveArrayStripperTest {
             fieldValues = byteArrayOf(0, 0, 0, 0, 0, 0, 0, secretWrappedLong),
           )
         )
+        if (wrapperClassDumpedAfterInstance) {
+          writer.write(wrapperClassDump)
+        }
         writer.write(CharArrayDump(id = 6, stackTraceSerialNumber = 0, array = secretCharArray))
       }
   }

shark/shark-hprof/api/shark-hprof.api

@@ -42,6 +42,7 @@ public final class shark/CopyingSource {
 	public final fun getBytesRead ()J
 	public final fun indexOf (B)J
 	public final fun overwrite ([B)V
+	public final fun overwriteRepeating (J[B)V
 	public final fun transfer (I)V
 	public final fun transfer (J)V
 	public final fun transferByte ()B
@@ -628,10 +629,18 @@ public abstract interface class shark/StreamingSinkProvider {
 	public abstract fun openStreamingSink ()Lokio/BufferedSink;
 }
 
+public final class shark/StreamingSinkProviderKt {
+	public static final fun gzip (Lshark/StreamingSinkProvider;)Lshark/StreamingSinkProvider;
+}
+
 public abstract interface class shark/StreamingSourceProvider {
 	public abstract fun openStreamingSource ()Lokio/BufferedSource;
 }
 
+public final class shark/StreamingSourceProviderKt {
+	public static final fun gunzipIfGzipped (Lshark/StreamingSourceProvider;)Lshark/StreamingSourceProvider;
+}
+
 public abstract class shark/ValueHolder {
 	public static final field Companion Lshark/ValueHolder$Companion;
 	public static final field NULL_REFERENCE J

shark/shark-hprof/src/main/java/shark/CopyingSource.kt

@@ -90,5 +90,24 @@ class CopyingSource(
     sink.write(byteArray)
   }
 
+  /**
+   * Skips [byteCount] bytes of the source and writes [pattern] over and over in their place, until
+   * [byteCount] bytes have been written. This makes it possible to replace bytes without first
+   * allocating a byte array to hold all of them, however many there are.
+   *
+   * The last repetition is cut short when [byteCount] isn't a multiple of the size of [pattern], so
+   * a [pattern] longer than one byte should be one whose repetition divides [byteCount].
+   */
+  fun overwriteRepeating(byteCount: Long, pattern: ByteArray) {
+    bytesRead += byteCount
+    source.skip(byteCount)
+    var remainingByteCount = byteCount
+    while (remainingByteCount > 0) {
+      val writeByteCount = minOf(remainingByteCount, pattern.size.toLong()).toInt()
+      sink.write(pattern, 0, writeByteCount)
+      remainingByteCount -= writeByteCount
+    }
+  }
+
   fun exhausted() = source.exhausted()
 }