Skip to content

hostarch: omit a head truncated to zero bytes in AddrRangeSeq.DropFirst - #17402

Merged
copybara-service[bot] merged 1 commit into
google:masterfrom
0z5a:fix/hostarch-dropfirst-truncated-head
Nov 11, 2026
Merged

copybara-service[bot] merged 1 commit into
google:masterfrom
0z5a:fix/hostarch-dropfirst-truncated-head

Conversation

@0z5a

@0z5a 0z5a commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #17399

AddrRangeSeq.DropFirst documents that it always omits a zero-length head, even if n is 0, so that the Head/DropFirst loop in its doc comment ends.

It only checked whether all bytes of the head were dropped. If TakeFirst truncated the head to 0 bytes (limit == 0) and two or more AddrRanges remain, DropFirst(0) returned the sequence unchanged. usermem.CopyInVec and CopyOutVec run that loop. For example, IOSequence.CopyIn with an 8-byte dst, over {[0,8) [16,24)}.TakeFirst(4), copied 4 bytes and then did not return.

This change also treats the head as empty if the limit is 0. Each range that TakeFirst truncated to 0 bytes is still visited, as a zero-length head, and is then dropped. A head that was partly consumed leaves one extra zero-length step, at [4,4) in the example above.

Compatibility tag: none. This is a library fix in pkg/hostarch. I found no syscall path that reaches it today: in-tree callers size their buffers to NumBytes() or iterate in other ways.

Tests:

  • New TestAddrRangeSeqTakeFirstDropFirstIteration in pkg/hostarch/addr_range_seq_test.go. It walks TakeFirst(n) for n = 0..12 over three ranges with Head/DropFirst.
    • Before: it fails at TakeFirst(0) because DropFirst(0) returns the same sequence.
    • After: it passes.
  • go test ./pkg/hostarch/ ./pkg/usermem/ ./pkg/safemem/ pass. They were run on the go branch with master's test files, on Linux amd64 with go1.27.3.
  • gofmt and go vet are clean. Not run: make test TARGETS=//pkg/hostarch:hostarch_test, make lint and nogo (no bazel or docker on the host).

Performance: DropFirst64 is on the usermem copy path. The change adds one comparison in the branch for sequences of two or more ranges.

AI disclosure

Claude Code, model claude-opus-5-5. AI was used to:

  • find the bug;
  • write the repro, the fix, the test and this description.

AI-generated code:

  • pkg/hostarch/addr_range_seq_unsafe.go: the 3-line change in DropFirst64;
  • pkg/hostarch/addr_range_seq_test.go: TestAddrRangeSeqTakeFirstDropFirstIteration (26 lines).

The commit carries Assisted-by: Claude Code (claude-opus-5-5), the same trailer form used in merged gVisor commits.

AddrRangeSeq.DropFirst documents that it always omits a zero-length
head, even if n is 0, so that a Head/DropFirst loop ends. It only
checked whether all bytes of the head were dropped. If TakeFirst
truncated the head to 0 bytes and two or more AddrRanges remain,
DropFirst(0) returned the sequence unchanged.

usermem.CopyInVec and CopyOutVec use this loop. For example,
IOSequence.CopyIn with an 8-byte dst, over two 8-byte AddrRanges
limited by TakeFirst(4), copied 4 bytes and then did not return.

Treat the head as empty if the limit is 0.

Assisted-by: Claude Code (claude-opus-5-5)
Signed-off-by: 0z5a <[email protected]>

@aaltinaydev aaltinaydev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🤖] Review agent has no findings.


Automated review agent. Please address valid findings before human review, or close them with a reason.

copybara-service Bot pushed a commit that referenced this pull request Nov 11, 2026
Fixes #17399

`AddrRangeSeq.DropFirst` documents that it always omits a zero-length head, even if `n` is 0, so that the Head/DropFirst loop in its doc comment ends.

It only checked whether all bytes of the head were dropped. If `TakeFirst` truncated the head to 0 bytes (`limit == 0`) and two or more AddrRanges remain, `DropFirst(0)` returned the sequence unchanged. `usermem.CopyInVec` and `CopyOutVec` run that loop. For example, `IOSequence.CopyIn` with an 8-byte `dst`, over `{[0,8) [16,24)}.TakeFirst(4)`, copied 4 bytes and then did not return.

This change also treats the head as empty if the limit is 0. Each range that `TakeFirst` truncated to 0 bytes is still visited, as a zero-length head, and is then dropped. A head that was partly consumed leaves one extra zero-length step, at `[4,4)` in the example above.

**Compatibility tag:** none. This is a library fix in `pkg/hostarch`. I found no syscall path that reaches it today: in-tree callers size their buffers to `NumBytes()` or iterate in other ways.

**Tests:**
- New `TestAddrRangeSeqTakeFirstDropFirstIteration` in `pkg/hostarch/addr_range_seq_test.go`. It walks `TakeFirst(n)` for n = 0..12 over three ranges with Head/DropFirst.
  - Before: it fails at `TakeFirst(0)` because `DropFirst(0)` returns the same sequence.
  - After: it passes.
- `go test ./pkg/hostarch/ ./pkg/usermem/ ./pkg/safemem/` pass. They were run on the `go` branch with master's test files, on Linux amd64 with go1.27.3.
- `gofmt` and `go vet` are clean. Not run: `make test TARGETS=//pkg/hostarch:hostarch_test`, `make lint` and nogo (no bazel or docker on the host).

**Performance:** `DropFirst64` is on the usermem copy path. The change adds one comparison in the branch for sequences of two or more ranges.

### AI disclosure
Claude Code, model claude-opus-5-5. AI was used to:
- find the bug;
- write the repro, the fix, the test and this description.

AI-generated code:
- `pkg/hostarch/addr_range_seq_unsafe.go`: the 3-line change in `DropFirst64`;
- `pkg/hostarch/addr_range_seq_test.go`: `TestAddrRangeSeqTakeFirstDropFirstIteration` (26 lines).

The commit carries `Assisted-by: Claude Code (claude-opus-5-5)`, the same trailer form used in merged gVisor commits.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#17402 from 0z5a:fix/hostarch-dropfirst-truncated-head 3fc515d
PiperOrigin-RevId: 1018828650
@copybara-service
copybara-service Bot merged commit f7f08db into google:master Nov 11, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hostarch: AddrRangeSeq.DropFirst(0) does not omit a zero-length head left by TakeFirst; usermem.CopyInVec spins

3 participants