Skip to content

fix(gpu): distinguish ELF string scan limits from corruption - #8955

Open
xu91102 wants to merge 1 commit into
morluto:mainfrom
xu91102:codex/elf-string-scan-capacity
Open

xu91102 wants to merge 1 commit into
morluto:mainfrom
xu91102:codex/elf-string-scan-capacity

Conversation

@xu91102

@xu91102 xu91102 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #8952. A valid CUBIN symbol name whose NUL follows the fixed ELF scan window was reported as malformed input. It now reports inspection: capacity and partial payload coverage, without raising the scan allowance or claiming corruption from bytes that were not examined.

Problem and expected behavior

At the same immutable inputs on main 71f680d7: 65,536/70,000-byte terminated names changed from malformed to capacity. A short name remains complete; a 65,535-byte name retains its existing partial section-name coverage; a genuinely unterminated short table remains malformed. Artifact hashes are unchanged.

Change and scope

The shared ELF string reader reuses the existing typed read-limit failure only when declared table bytes remain unexamined. GPU inventory consumes that type; the legacy superclass/kind remains intact for per-symbol native-export handling and error caching. The existing bounded-string diagnostic describes unknown termination accurately. This is an ordinary static-inspection fix within existing scope; no maintainer scope expansion is required.

Contract and boundary impact

  • Existing CLI/MCP schemas, limits, dependencies, identity and cleanup owners are unchanged.
  • Outside-table offsets and wholly examined missing-NUL strings remain malformed. Empty/non-UTF8 bytes and unrelated exports are preserved; JNI mapped-string behavior retains the same typed limit and maximum.
  • No target execution, network access, new parser or runtime override. Generated metadata/package version have no authored change.

Evidence and regression coverage

Original public CLI regression: 1 FAIL/23 PASS before the fix. Final distinct local coverage is eight focused files/158 PASS plus README 9: 167 PASS, zero skip, with overlap replaced rather than accumulated. The last two changed test files reran (51 PASS); the other unchanged test inputs retain their executed results.

The same five immutable format fixtures passed five compiled CLI calls + five calls in one real stdio MCP session, and repeated successfully against a normal installed package. Independent Python byte/range checks confirmed the long inputs have their NUL inside the ELF string table. These are format fixtures, not real CUDA/GPU execution.

Representative normalized output:

{"objects":[{"kind":"cubin","inspection":"capacity"}],"kernels":[],"coverage":{"payloads":"partial"}}

Validation performed

  • Relevant focused tests and native-export/JNI preservation checks: PASS; malformed/normal controls retained.
  • npm run check, npm run docs:check, npm run verify:test-discovery: PASS (3,029 files exactly once).
  • npm run verify:package and npm pack --dry-run: PASS; official installed MCP 234 tools/6 prompts, exact update and awaited cleanup. All 2,781 published files match archive/current/installed bytes.
  • Independent Standards and Spec reviews: PASS on the frozen final source. Original RED, lint/format and local harness failures are retained; no checks were suppressed.

Compatibility, safety, and release

No migration or breaking schema change. Verified on macOS arm64/Node 24.22.1. Real GPU/CUDA execution, other hosts, complete external engines/apps, agent/model and performance were not run. Local validation is separate from upstream CI, Codex and human review.

Author readiness

Human owner: @xu91102

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The reviewed changes appear safe to merge; no actionable defects were found.

Summary

The PR distinguishes an exhausted ELF string scan from proven malformed input without increasing the read limit.

  • GPU inspection reports capacity when a name exceeds the scan window.

Reviews (1) · Last reviewed commit: "fix(gpu): distinguish ELF string scan li..." · Reviewed by Greptile

@xu91102
xu91102 marked this pull request as ready for review November 5, 2026 02:57
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-05T02:57:39.815907Z
We couldn’t complete this review. Comment @codex review to try again.
8f7cc89 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] CUBIN symbol string scan limit is reported as malformed input

1 participant