Skip to content

fix(fill): prevent SIGSEGV from varlen vector self-alias during FILL(next) - #26608

Open
VioletQwQ-0 wants to merge 5 commits into
matrixorigin:mainfrom
VioletQwQ-0:fix/issue-26558-fill-next-varlen-sigsegv
Open

fix(fill): prevent SIGSEGV from varlen vector self-alias during FILL(next)#26608
VioletQwQ-0 wants to merge 5 commits into
matrixorigin:mainfrom
VioletQwQ-0:fix/issue-26558-fill-next-varlen-sigsegv

Conversation

@VioletQwQ-0

@VioletQwQ-0 VioletQwQ-0 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind bug
/area sql

Which issue(s) does this PR fix?

issue #26558

What this PR does / why we need it

FILL(next) can write a pending run back into the same off-heap varlen vector that supplies its next value. If a destination write grows the vector area, the source bytes can become invalid during the copy and terminate the CN.

For a same-batch, non-inline varlen run, consumeNext now snapshots the source once into an mpool-owned off-heap vector before writing any destination row. Cross-batch and inline paths remain unchanged. The snapshot is released on append failure, set failure, and success, so the fix is bounded to one payload copy per resolved run rather than one Go allocation per row.

Regression coverage

  • The focused consumeNext regression uses production-shaped off-heap vectors and covers all 14 varlen OIDs accepted by the fill byte-copy path.
  • The same regression fails on exact base 820ee12144439c5b63c03b261f4330ec0364fdb3 and passes on PR head 1d82b78f6322ab44dec7560888cb39f979e136cf.
  • The BVT asserts row count, total bytes, checksum, and exact a_rows/b_rows, so wrong fill direction or same-length payload corruption cannot pass.

Validation

  • mo-cgo-test -run '^TestConsumeNextStabilizesSelfAliasedVarlenSources$' ./pkg/sql/colexec/fill
  • mo-cgo-test ./pkg/sql/colexec/fill
  • mo-cgo-test -race ./pkg/sql/colexec/fill
  • go build ./pkg/sql/colexec/fill
  • go vet ./pkg/sql/colexec/fill
  • exact-head mo-service build and committed SQL BVT: 1001 / 256256 / 96362590929 / 1 / 1000; follow-up login/query succeeded
  • git diff --check
  • mo-pr-preflight-review: PASS review=PASS validation=PENDING
  • mo-self-review: PASS

QA decision

QA required: yes

This is an S0 user-visible CN termination fix. Keep #26558 open for QA validation after merge; the BVT is added but does not replace deployment-version verification.

…next)

When FILL(next) backfills a varlen column (VARCHAR, TEXT, etc.) across a large
gap, consumeNext may call setValue with the same vector as both source and
destination. The previous implementation passed w.GetBytesAt(j) directly to
SetBytesAt, which can trigger BuildVarlenaNoInline -> MPool.Grow2 area
reallocation before copying the source bytes. When v == w, this reallocation
invalidates the source slice, causing a use-after-free SIGSEGV in memmove.

This commit detects self-aliasing (v == w) in setValue's varlen branch and
copies the source bytes to a temporary buffer before calling SetBytesAt,
ensuring the source remains valid through any area growth.

Regression coverage:
- pkg/container/vector: TestSetBytesAtSelfAliasWithGrowth verifies SetBytesAt
  correctness under self-alias + growth at the vector layer.
- pkg/sql/colexec/fill: TestSetValueSelfAliasVarlen and
  TestSetValueSelfAliasCrossArea verify setValue correctness for self-alias
  scenarios that cross area reallocation boundaries.
- test/distributed/cases/function: BVT case reproduces the public 1001-row
  VARCHAR(256) FILL(next) materialization scenario from the issue report.

Fixes matrixorigin#26558

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@VioletQwQ-0

Copy link
Copy Markdown
Collaborator Author

The exact-head Coverage job failed at 7/11 changed statements (63.64%, required >75%). I added a deterministic mpool-exhaustion regression that executes the snapshot appendValue failure and verifies cleanup accounting. On head 97d7494329421c4a832669ef253362d5816faced, focused tests, focused race (-count=3), the full pkg/sql/colexec/fill package, a local coverage profile, and git diff --check pass. The previously uncovered two-statement append-failure block is now covered, raising the original changed-code count to at least 9/11. Preflight: PASS review=PASS validation=PENDING; fresh CI is pending.

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

Labels

size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants