Skip to content

Fix unsigned underflow in StackP4_16::pop_front - #1435

Merged
jafingerhut merged 1 commit into
p4lang:mainfrom
sanket-jadhav-cse:fix-pop-front-underflow
Jul 30, 2026
Merged

Fix unsigned underflow in StackP4_16::pop_front#1435
jafingerhut merged 1 commit into
p4lang:mainfrom
sanket-jadhav-cse:fix-pop-front-underflow

Conversation

@sanket-jadhav-cse

Copy link
Copy Markdown
Contributor

Summary

StackP4_16::pop_front(size_t num) computes the loop bound using size - num, where size is an unsigned value. When num is greater than the stack size, this subtraction underflows, resulting in an invalid loop bound and out-of-bounds access.

This change avoids the underflow by skipping the swap loop when num >= size and directly invalidating the remaining headers. The existing behavior for valid inputs is preserved.

Before

  • pop_front() computed size - num unconditionally.
  • Calling pop_front() with num > stack size caused unsigned integer underflow.
  • Running the new regression test against the original implementation reproduced the issue with a std::vector::operator[] assertion failure.

After

  • The swap loop executes only when num < size.
  • For num >= size, all headers are invalidated directly.
  • No unsigned underflow occurs.
  • pop_front() safely handles requests larger than the stack size.

Testing

  • Added HeaderStackP4_16Test.PopFrontLargeNum regression test.
  • Added HeaderStackP4_16Test.PopFrontExactDepth boundary test.
  • Verified that PopFrontLargeNum reproduces the failure with the original implementation.
  • Verified that both new tests pass with the fix.
  • Ran the complete test_header_stacks suite successfully.
  • Ran the full CTest suite. The only remaining failure was the existing test_switch test due to a missing Python thrift dependency, which is unrelated to this change.

@sanket-jadhav-cse
sanket-jadhav-cse force-pushed the fix-pop-front-underflow branch from 2322c27 to e67b99d Compare July 30, 2026 11:14
Signed-off-by: Sanket Jadhav <sj546400@gmail.com>
@sanket-jadhav-cse
sanket-jadhav-cse force-pushed the fix-pop-front-underflow branch from e67b99d to 3aa913c Compare July 30, 2026 11:58

@jafingerhut jafingerhut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Apparently there are some C++ code style checks failing in CI. If you need help learning how to correct those, please ask. There should be some script you can run locally (I hope) that can modify the code formatting in a way that passes the style check, without having to do it manually.

@sanket-jadhav-cse

sanket-jadhav-cse commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@jafingerhut Thanks! I found the issue. It was a cpplint line-length check in the new test. I have fixed the formatting and force-pushed the update. Thanks for the review!

@jafingerhut
jafingerhut merged commit 883d4ea into p4lang:main Jul 30, 2026
11 checks passed
@sanket-jadhav-cse
sanket-jadhav-cse deleted the fix-pop-front-underflow branch July 30, 2026 12:34
@fruffy

fruffy commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

This fixes a crash: p4lang/p4c#5721 Very nice!

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.

4 participants