Skip to content

zstd: decodeSync's unsafe path has been dead code since 2022 (#644) — remove or fix? #1168

Description

@lizthegrey

While working on the arm64 lowering for #1160/#1167 I noticed (*sequenceDecs).decodeSyncSimple in zstd/seqdec_asm.go has:

// FIXME: Using unsafe memory copies leads to rare, random crashes
// with fuzz testing. It is therefore disabled for now.
const useSafe = true

with the real dynamic logic (computing useSafe from buffer slack, same shape as executeSimple's still-active dynamic useSafe) commented out below it. This dates to #644 (2022-07-17), soon after #637 added the faster unsafe copy (up to ~1.46x on some benchmarks) and shortly after #562/#563 (an earlier decodeSync crash, root-caused and fixed). #644 doesn't link an issue or root cause — so this reads as "another crash turned up, we cut losses" rather than a one-off.

Four years later, sequenceDecs_decodeSync_{amd64,bmi2} are still generated and shipped, but useSafe being a compile-time true means nothing ever calls them — no test or fuzz target exercises them. The arm64 avo lowering (#1160) mechanically translates the same IR, so this dead, untested path now exists on a second architecture too (_arm64/_bmi2), doubling the footprint of code nobody runs.

Given the track record (fixed once, still had to be disabled again within months), I don't think restoring the commented-out logic as-is is right either. Two reasonable paths:

  1. Remove it — drop the unsafe generation from _generate/gen.go, simplify the safe bool out of decodeSyncAsm's dispatch, drop the dead code in seqdec_asm.go. Mechanical, no behavior change from what ships today.
  2. Root-cause and re-enable it with real regression coverage, matching how executeSimple's analogous path stayed dynamically toggled.

Happy to send a PR either way — let me know which you'd prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions