Skip to content

Automated code review of src/subgraph/ by Gemini for potential bugs#9726

Open
copybara-service[bot] wants to merge 1 commit intomasterfrom
test_885703148
Open

Automated code review of src/subgraph/ by Gemini for potential bugs#9726
copybara-service[bot] wants to merge 1 commit intomasterfrom
test_885703148

Conversation

@copybara-service
Copy link
Contributor

@copybara-service copybara-service bot commented Mar 18, 2026

Automated code review of src/subgraph/ by Gemini for potential bugs

The fixes are from a few basic categories:

  • Lack of validation of input tensor rank before using the shape. This is tricky because the tensor shape and rank are not expected to be defined, but we sometimes use these shapes anyways before a reshaping happens.
  • We mix up input input1 and input2 in a lot of places, in ways that expose invalid memory accesses.
  • We mix up filter_id and bias_id often. Sometimes only for logging/error reporting, but sometimes not...
  • Sometimes we check something is valid, but not before using it first.
  • Ignoring the return value of validation functions.

It makes one questionable change: when a loop has an unsigned extent, it wants to use an unsigned index. This is a questionable choice, there are arguments both ways on this one.

@copybara-service copybara-service bot force-pushed the test_885703148 branch 2 times, most recently from 846ef85 to 4ee4281 Compare March 19, 2026 01:18
The fixes are from a few basic categories:
- Lack of validation of input tensor rank before using the shape. This is tricky because the tensor shape and rank are not expected to be defined, but we sometimes use these shapes anyways before a reshaping happens.
- We mix up input `input1` and `input2` in a lot of places, in ways that expose invalid memory accesses.
- We mix up `filter_id` and `bias_id` often. Sometimes only for logging/error reporting, but sometimes not...
- Sometimes we check something is valid, but not before using it first.
- Ignoring the return value of validation functions.

It makes one questionable change: when a loop has an unsigned extent, it wants to use an unsigned index. This is a questionable choice, there are arguments both ways on this one.

PiperOrigin-RevId: 885703148
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.

1 participant