also looking at this
perf(android): ship libcraft.so without its DWARF, and keep the DWARF for symbols
#217
6 files
+235
-14
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -55,7 +55,10 @@ into the APK and once without: | ||
| 55 | 55 | |
| 56 | 56 | The Zig Android library links bionic, so building it needs the NDK — |
| 57 | 57 | `sdkmanager --install "ndk;26.1.10909125"`. Without it `build-android` refuses |
| 58 | rather than producing a library that installs and then fails to load. | |
| 58 | rather than producing a library that installs and then fails to load. The NDK's | |
| 59 | `llvm-objcopy` also strips each release library's DWARF into | |
| 60 | `zig-out/android-symbols/`, and the suite refuses to start on a `libcraft.so` | |
| 61 | that still carries any. | |
| 59 | 62 | |
| 60 | 63 | ```bash |
| 61 | 64 | cd packages/zig && zig build build-android-all -Doptimize=ReleaseSafe -Dandroid-ndk="$ANDROID_NDK_HOME" && cd - |
| @@ -611,6 +611,20 @@ For automated uploads, integrate with fastlane: | ||
| 611 | 611 | fastlane supply --aab ./android/app/build/outputs/bundle/release/app-release.aab |
| 612 | 612 | ``` |
| 613 | 613 | |
| 614 | ### Native crash symbols | |
| 615 | ||
| 616 | `zig build build-android-all` strips the DWARF out of each release | |
| 617 | `libcraft.so` before the generator copies it into `jniLibs`. With it, the | |
| 618 | library was about 5.5 MB per ABI in every APK. The symbol table stays, so a | |
| 619 | native crash tombstone still names functions. The DWARF goes to | |
| 620 | `zig-out/android-symbols/<abi>/libcraft.so.debug`, linked from the library by | |
| 621 | `.gnu_debuglink`. | |
| 622 | ||
| 623 | To symbolicate a crash, point `ndk-stack -sym` at that directory. To have Play | |
| 624 | Console symbolicate for you, upload the `.debug` files as the release's native | |
| 625 | debug symbols. A `-Doptimize=Debug` build keeps the debug info in the library | |
| 626 | itself. | |
| 627 | ||
| 614 | 628 | ## License |
| 615 | 629 | |
| 616 | 630 | MIT |
| @@ -2471,11 +2471,9 @@ pub fn build(b: *std.Build) void { | ||
| 2471 | 2471 | }); |
| 2472 | 2472 | linkAndroidLibc(b, android_arm64_jni, ndk, "aarch64-linux-android"); |
| 2473 | 2473 | |
| 2474 | const android_arm64_jni_install = b.addInstallArtifact(android_arm64_jni, .{ | |
| 2475 | .dest_dir = .{ .override = .{ .custom = "android/arm64-v8a" } }, | |
| 2476 | }); | |
| 2477 | build_android.dependOn(&android_arm64_jni_install.step); | |
| 2478 | build_android_all.dependOn(&android_arm64_jni_install.step); | |
| 2474 | const android_arm64_jni_install = installAndroidJni(b, android_arm64_jni, ndk, "arm64-v8a", debug_build); | |
| 2475 | build_android.dependOn(android_arm64_jni_install); | |
| 2476 | build_android_all.dependOn(android_arm64_jni_install); | |
| 2479 | 2477 | } else { |
| 2480 | 2478 | // Not built at all rather than built without bionic. The library would |
| 2481 | 2479 | // compile, install, ship in the APK and then fail at `dlopen` with two |
| @@ -2521,11 +2519,9 @@ pub fn build(b: *std.Build) void { | ||
| 2521 | 2519 | |
| 2522 | 2520 | // `x86_64`, matching the jniLibs directory the loader looks in — not |
| 2523 | 2521 | // `x86`, which is the 32-bit ABI and would be silently ignored. |
| 2524 | const android_x86_jni_install = b.addInstallArtifact(android_x86_jni, .{ | |
| 2525 | .dest_dir = .{ .override = .{ .custom = "android/x86_64" } }, | |
| 2526 | }); | |
| 2527 | build_android_x86.dependOn(&android_x86_jni_install.step); | |
| 2528 | build_android_all.dependOn(&android_x86_jni_install.step); | |
| 2522 | const android_x86_jni_install = installAndroidJni(b, android_x86_jni, ndk, "x86_64", debug_build); | |
| 2523 | build_android_x86.dependOn(android_x86_jni_install); | |
| 2524 | build_android_all.dependOn(android_x86_jni_install); | |
| 2529 | 2525 | } |
| 2530 | 2526 | |
| 2531 | 2527 | const android_x86_lib = b.addLibrary(.{ |
| @@ -2618,6 +2614,82 @@ fn androidSysroot(b: *std.Build, ndk: []const u8) ?[]const u8 { | ||
| 2618 | 2614 | return null; |
| 2619 | 2615 | } |
| 2620 | 2616 | |
| 2617 | /// A tool from the NDK's LLVM toolchain, or null when this NDK has none for | |
| 2618 | /// this host. Found the way `androidSysroot` finds the sysroot, and for the | |
| 2619 | /// same reason. | |
| 2620 | fn androidTool(b: *std.Build, ndk: []const u8, name: []const u8) ?[]const u8 { | |
| 2621 | const io = b.graph.io; | |
| 2622 | for ([_][]const u8{ "darwin-x86_64", "darwin-arm64", "linux-x86_64", "windows-x86_64" }) |host| { | |
| 2623 | const exe = if (std.mem.startsWith(u8, host, "windows")) b.fmt("{s}.exe", .{name}) else name; | |
| 2624 | const candidate = b.pathJoin(&.{ ndk, "toolchains/llvm/prebuilt", host, "bin", exe }); | |
| 2625 | std.Io.Dir.cwd().access(io, candidate, .{}) catch continue; | |
| 2626 | return candidate; | |
| 2627 | } | |
| 2628 | return null; | |
| 2629 | } | |
| 2630 | ||
| 2631 | /// Install a JNI library where the Android generator looks for it, without its | |
| 2632 | /// DWARF, and install the DWARF beside it for symbolication (#204). | |
| 2633 | /// | |
| 2634 | /// Every generated app's APK carries this library, and at `ReleaseSafe` each | |
| 2635 | /// one was about 5.5 MB, most of it debug info nothing read: AGP strips native | |
| 2636 | /// libraries by running the NDK's strip, and a generated project configures no | |
| 2637 | /// NDK, so it never did. So it happens here, with the same tool AGP would use. | |
| 2638 | /// | |
| 2639 | /// `--strip-debug` and not `--strip-all`. The symbol table stays, which is what | |
| 2640 | /// turns a native crash tombstone's addresses into function names, and it is a | |
| 2641 | /// small fraction of what the DWARF was. The DWARF goes to | |
| 2642 | /// `android-symbols/<abi>/libcraft.so.debug`, linked from the stripped library | |
| 2643 | /// by `.gnu_debuglink`, which is the shape `ndk-stack` and Play Console's | |
| 2644 | /// native debug symbols both take. | |
| 2645 | /// | |
| 2646 | /// `llvm-objcopy` rather than Zig's own ObjCopy step, because ELF to ELF | |
| 2647 | /// copying is `fatal("unimplemented")` in `zig objcopy` at 0.17.0-dev.1963; | |
| 2648 | /// every strip mode fails. The NDK is already required to build these at all. | |
| 2649 | /// | |
| 2650 | /// A Debug build installs the library as built: whoever asked for Debug wants | |
| 2651 | /// the debug info in place. | |
| 2652 | fn installAndroidJni( | |
| 2653 | b: *std.Build, | |
| 2654 | lib: *std.Build.Step.Compile, | |
| 2655 | ndk: []const u8, | |
| 2656 | abi: []const u8, | |
| 2657 | keep_debug_info: bool, | |
| 2658 | ) *std.Build.Step { | |
| 2659 | const dest = b.fmt("android/{s}", .{abi}); | |
| 2660 | if (keep_debug_info) { | |
| 2661 | const install = b.addInstallArtifact(lib, .{ .dest_dir = .{ .override = .{ .custom = dest } } }); | |
| 2662 | return &install.step; | |
| 2663 | } | |
| 2664 | ||
| 2665 | const objcopy = androidTool(b, ndk, "llvm-objcopy") orelse { | |
| 2666 | return &b.addFail(b.fmt( | |
| 2667 | "-Dandroid-ndk={s} has no toolchains/llvm/prebuilt/<host>/bin/llvm-objcopy, " ++ | |
| 2668 | "which strips libcraft.so's debug info for release builds. " ++ | |
| 2669 | "Point it at an NDK root, or build with -Doptimize=Debug to keep it.", | |
| 2670 | .{ndk}, | |
| 2671 | )).step; | |
| 2672 | }; | |
| 2673 | ||
| 2674 | const keep = b.addSystemCommand(&.{ objcopy, "--only-keep-debug" }); | |
| 2675 | keep.addFileArg(lib.getEmittedBin()); | |
| 2676 | const debug_info = keep.addOutputFileArg("libcraft.so.debug"); | |
| 2677 | ||
| 2678 | const strip = b.addSystemCommand(&.{ objcopy, "--strip-debug" }); | |
| 2679 | strip.addPrefixedFileArg("--add-gnu-debuglink=", debug_info); | |
| 2680 | strip.addFileArg(lib.getEmittedBin()); | |
| 2681 | const stripped = strip.addOutputFileArg("libcraft.so"); | |
| 2682 | ||
| 2683 | const install = b.addInstallFileWithDir(stripped, .{ .custom = dest }, "libcraft.so"); | |
| 2684 | const symbols = b.addInstallFileWithDir( | |
| 2685 | debug_info, | |
| 2686 | .{ .custom = b.fmt("android-symbols/{s}", .{abi}) }, | |
| 2687 | "libcraft.so.debug", | |
| 2688 | ); | |
| 2689 | install.step.dependOn(&symbols.step); | |
| 2690 | return &install.step; | |
| 2691 | } | |
| 2692 | ||
| 2621 | 2693 | /// Link one Android artifact against the NDK's bionic. |
| 2622 | 2694 | /// |
| 2623 | 2695 | /// A `--libc` file, not include and library paths. Paths alone are what |
| @@ -1,8 +1,8 @@ | ||
| 1 | 1 | import type { LegOutcome, RunnerOptions } from './types' |
| 2 | import { copyFileSync, existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' | |
| 2 | import { copyFileSync, existsSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs' | |
| 3 | 3 | import { join } from 'node:path' |
| 4 | 4 | import { init } from '../../packages/android/src/index' |
| 5 | import { androidDeclines, awaitedNeeds, DISMISS_SHARE_MENU, evaluateRun, hasTerminated, runtimePermissionGranted, shareMenuInFront } from './protocol' | |
| 5 | import { androidDeclines, awaitedNeeds, DISMISS_SHARE_MENU, elfSectionNames, evaluateRun, hasTerminated, runtimePermissionGranted, shareMenuInFront, strippedLibraryProblems } from './protocol' | |
| 6 | 6 | import { command, driverPage, waitForFile } from './support' |
| 7 | 7 | |
| 8 | 8 | /** |
| @@ -321,6 +321,20 @@ export async function runAndroid(options: RunnerOptions): Promise<LegOutcome[]> | ||
| 321 | 321 | ) |
| 322 | 322 | } |
| 323 | 323 | |
| 324 | // What every generated app will ship, checked before any leg uses it. The | |
| 325 | // runtime leg asserts the library loads and answers; this asserts it is the | |
| 326 | // release shape #204 settled on, stripped with its DWARF kept beside it, | |
| 327 | // because a regression there costs every APK megabytes and fails no case. | |
| 328 | const symbolsPath = join(runtimeDir, '..', 'android-symbols', 'x86_64', 'libcraft.so.debug') | |
| 329 | const libraryProblems = strippedLibraryProblems( | |
| 330 | elfSectionNames(new Uint8Array(readFileSync(abi))), | |
| 331 | existsSync(symbolsPath) ? elfSectionNames(new Uint8Array(readFileSync(symbolsPath))) : null, | |
| 332 | ) | |
| 333 | if (libraryProblems.length) | |
| 334 | throw new Error(`${abi}: ${libraryProblems.join('; ')}`) | |
| 335 | const megabytes = (path: string) => `${(statSync(path).size / 1024 / 1024).toFixed(2)} MB` | |
| 336 | console.log(`x86_64/libcraft.so ships at ${megabytes(abi)}; its symbols are ${megabytes(symbolsPath)} beside it`) | |
| 337 | ||
| 324 | 338 | const outcomes: LegOutcome[] = [] |
| 325 | 339 | for (const leg of legs(runtimeDir)) { |
| 326 | 340 | try { |
| @@ -1,7 +1,7 @@ | ||
| 1 | 1 | import { describe, expect, it } from 'bun:test' |
| 2 | 2 | import { readFileSync } from 'node:fs' |
| 3 | 3 | import { join } from 'node:path' |
| 4 | import { ANDROID_DECLINE_PHRASES, androidDeclines, awaitedNeeds, deepLinkProblems, deepLinkResults, DISMISS_SHARE_MENU, evaluateRun, hasTerminated, parseDriverOutput, REQUIRED_CASES, requiredCaseProblems, runtimePermissionGranted, shareMenuInFront, ZIG_REFUSED_ACTIONS, ZIG_SERVED_ACTIONS, ZIG_TESTED_ACTIONS, zigDispatchedActions, zigHandBacks, zigRefusals } from './protocol' | |
| 4 | import { ANDROID_DECLINE_PHRASES, androidDeclines, awaitedNeeds, deepLinkProblems, deepLinkResults, DISMISS_SHARE_MENU, elfSectionNames, evaluateRun, hasTerminated, parseDriverOutput, REQUIRED_CASES, requiredCaseProblems, runtimePermissionGranted, shareMenuInFront, strippedLibraryProblems, ZIG_REFUSED_ACTIONS, ZIG_SERVED_ACTIONS, ZIG_TESTED_ACTIONS, zigDispatchedActions, zigHandBacks, zigRefusals } from './protocol' | |
| 5 | 5 | |
| 6 | 6 | const ESC = String.fromCharCode(27) |
| 7 | 7 | |
| @@ -372,6 +372,65 @@ describe('cold-start deep links', () => { | ||
| 372 | 372 | }) |
| 373 | 373 | }) |
| 374 | 374 | |
| 375 | /** A minimal ELF64 little-endian file whose section header table names `sections`. */ | |
| 376 | function elfWith(sections: string[]): Uint8Array { | |
| 377 | const names = ['', ...sections, '.shstrtab'] | |
| 378 | const strtab = new TextEncoder().encode(`${names.join('\0')}\0`) | |
| 379 | const headerSize = 64 | |
| 380 | const entrySize = 64 | |
| 381 | const tableOffset = headerSize + strtab.length | |
| 382 | const bytes = new Uint8Array(tableOffset + entrySize * names.length) | |
| 383 | const view = new DataView(bytes.buffer) | |
| 384 | bytes.set([0x7F, 0x45, 0x4C, 0x46, 2, 1, 1], 0) | |
| 385 | view.setBigUint64(0x28, BigInt(tableOffset), true) | |
| 386 | view.setUint16(0x3A, entrySize, true) | |
| 387 | view.setUint16(0x3C, names.length, true) | |
| 388 | view.setUint16(0x3E, names.length - 1, true) | |
| 389 | bytes.set(strtab, headerSize) | |
| 390 | let nameOffset = 0 | |
| 391 | names.forEach((name, index) => { | |
| 392 | const at = tableOffset + entrySize * index | |
| 393 | view.setUint32(at, nameOffset, true) | |
| 394 | nameOffset += new TextEncoder().encode(name).length + 1 | |
| 395 | if (name === '.shstrtab') { | |
| 396 | view.setBigUint64(at + 0x18, BigInt(headerSize), true) | |
| 397 | view.setBigUint64(at + 0x20, BigInt(strtab.length), true) | |
| 398 | } | |
| 399 | }) | |
| 400 | return bytes | |
| 401 | } | |
| 402 | ||
| 403 | describe('the shipped Android library', () => { | |
| 404 | const stripped = ['.dynsym', '.text', '.symtab', '.gnu_debuglink'] | |
| 405 | const debug = ['.debug_info', '.debug_line', '.symtab'] | |
| 406 | ||
| 407 | it('reads section names out of an ELF64 file, and refuses anything else', () => { | |
| 408 | expect(elfSectionNames(elfWith(['.text', '.debug_info']))).toEqual(['', '.text', '.debug_info', '.shstrtab']) | |
| 409 | expect(elfSectionNames(new Uint8Array([0x7F, 0x45, 0x4C, 0x46]))).toBeNull() | |
| 410 | expect(elfSectionNames(new TextEncoder().encode('not an elf file at all, but long enough to have a header, surely'))).toBeNull() | |
| 411 | }) | |
| 412 | ||
| 413 | it('passes a stripped library with its DWARF moved beside it', () => { | |
| 414 | expect(strippedLibraryProblems(elfSectionNames(elfWith(stripped)), elfSectionNames(elfWith(debug)))).toEqual([]) | |
| 415 | }) | |
| 416 | ||
| 417 | // #204 as found: the release library still carrying its debug info. | |
| 418 | it('fails a library that still ships DWARF', () => { | |
| 419 | const unstripped = elfSectionNames(elfWith(['.dynsym', '.text', '.debug_info', '.debug_str', '.symtab'])) | |
| 420 | expect(strippedLibraryProblems(unstripped, elfSectionNames(elfWith(debug)))).toEqual([ | |
| 421 | 'libcraft.so still carries .debug_info, .debug_str; release builds ship without DWARF (#204)', | |
| 422 | 'libcraft.so has no .gnu_debuglink, so nothing ties it to its symbols file', | |
| 423 | ]) | |
| 424 | }) | |
| 425 | ||
| 426 | it('fails a strip that threw the DWARF away instead of keeping it', () => { | |
| 427 | expect(strippedLibraryProblems(elfSectionNames(elfWith(stripped)), null)) | |
| 428 | .toEqual(['android-symbols has no libcraft.so.debug beside the stripped library']) | |
| 429 | expect(strippedLibraryProblems(elfSectionNames(elfWith(stripped)), elfSectionNames(elfWith(['.symtab'])))) | |
| 430 | .toEqual(['libcraft.so.debug holds no .debug_info; the strip discarded the DWARF instead of moving it']) | |
| 431 | }) | |
| 432 | }) | |
| 433 | ||
| 375 | 434 | describe('runtime permission state', () => { |
| 376 | 435 | // As `dumpsys package` prints a coarse-only grant on Android 14. |
| 377 | 436 | const dumpsys = [ |
| @@ -441,6 +441,65 @@ export function runtimePermissionGranted(dumpsysPackage: string, permission: str | ||
| 441 | 441 | return match ? match[1] === 'true' : undefined |
| 442 | 442 | } |
| 443 | 443 | |
| 444 | /** | |
| 445 | * The section names in an ELF file, or null when `bytes` is not a 64-bit | |
| 446 | * little-endian ELF. | |
| 447 | * | |
| 448 | * Read directly rather than through `readelf`, which a macOS host does not | |
| 449 | * have and a Linux runner has only by accident. Every Android ABI the harness | |
| 450 | * ships (arm64-v8a, x86_64) is ELF64 little-endian, so that is all this reads. | |
| 451 | */ | |
| 452 | export function elfSectionNames(bytes: Uint8Array): string[] | null { | |
| 453 | const ELF_MAGIC = [0x7F, 0x45, 0x4C, 0x46] | |
| 454 | if (bytes.length < 64 || ELF_MAGIC.some((byte, index) => bytes[index] !== byte)) return null | |
| 455 | if (bytes[4] !== 2 || bytes[5] !== 1) return null | |
| 456 | ||
| 457 | const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength) | |
| 458 | const sectionTable = Number(view.getBigUint64(0x28, true)) | |
| 459 | const entrySize = view.getUint16(0x3A, true) | |
| 460 | const count = view.getUint16(0x3C, true) | |
| 461 | const namesIndex = view.getUint16(0x3E, true) | |
| 462 | if (entrySize < 0x28 || sectionTable + entrySize * count > bytes.length || namesIndex >= count) return null | |
| 463 | ||
| 464 | const header = (index: number) => sectionTable + entrySize * index | |
| 465 | const namesOffset = Number(view.getBigUint64(header(namesIndex) + 0x18, true)) | |
| 466 | const namesSize = Number(view.getBigUint64(header(namesIndex) + 0x20, true)) | |
| 467 | if (namesOffset + namesSize > bytes.length) return null | |
| 468 | ||
| 469 | const names: string[] = [] | |
| 470 | for (let index = 0; index < count; index++) { | |
| 471 | const start = namesOffset + view.getUint32(header(index), true) | |
| 472 | let end = start | |
| 473 | while (end < namesOffset + namesSize && bytes[end] !== 0) end++ | |
| 474 | names.push(new TextDecoder().decode(bytes.subarray(start, end))) | |
| 475 | } | |
| 476 | return names | |
| 477 | } | |
| 478 | ||
| 479 | /** | |
| 480 | * Why a shipped `libcraft.so` and its symbols file are not what #204 settled | |
| 481 | * on, one line per reason. | |
| 482 | * | |
| 483 | * The library must carry no DWARF: every generated app's APK includes it, and | |
| 484 | * with DWARF it was about 5.5 MB per ABI. It must carry `.gnu_debuglink`, so a | |
| 485 | * crash can be matched to the symbols file. And the symbols file must actually | |
| 486 | * hold the DWARF, or the strip threw it away rather than moving it. | |
| 487 | */ | |
| 488 | export function strippedLibraryProblems(library: string[] | null, symbols: string[] | null): string[] { | |
| 489 | const problems: string[] = [] | |
| 490 | if (!library) return ['libcraft.so is not an ELF64 little-endian file'] | |
| 491 | const dwarf = library.filter(name => name.startsWith('.debug_')) | |
| 492 | if (dwarf.length) | |
| 493 | problems.push(`libcraft.so still carries ${dwarf.join(', ')}; release builds ship without DWARF (#204)`) | |
| 494 | if (!library.includes('.gnu_debuglink')) | |
| 495 | problems.push('libcraft.so has no .gnu_debuglink, so nothing ties it to its symbols file') | |
| 496 | if (!symbols) | |
| 497 | problems.push('android-symbols has no libcraft.so.debug beside the stripped library') | |
| 498 | else if (!symbols.includes('.debug_info')) | |
| 499 | problems.push('libcraft.so.debug holds no .debug_info; the strip discarded the DWARF instead of moving it') | |
| 500 | return problems | |
| 501 | } | |
| 502 | ||
| 444 | 503 | /** |
| 445 | 504 | * The actions the Zig dispatcher saw, by name. |
| 446 | 505 | * |