Skip to content

Handle unsized array element types in bounds_check_index#8843

Open
tautschnig wants to merge 1 commit intodiffblue:developfrom
tautschnig:bounds-check-integer
Open

Handle unsized array element types in bounds_check_index#8843
tautschnig wants to merge 1 commit intodiffblue:developfrom
tautschnig:bounds-check-integer

Conversation

@tautschnig
Copy link
Collaborator

bounds_check_index crashes when the array element type has no computable byte size (e.g., arrays of unbounded arrays used in heap models). object_descriptor_exprt::build calls size_of_expr on the element type, and when that returns nullopt, the resulting ID_unknown offset causes a precondition violation in from_integer downstream.

The fix checks whether size_of_expr on the element type returns a value. When it does not, a simple index >= 0 lower-bound check is emitted instead of the byte-offset-based one, and the object descriptor is not built. A DATA_INVARIANT asserts that the array expression is a symbol or nested index so that unexpected cases are caught rather than silently skipping bounds checks. The object descriptor is wrapped in std::optional so that the upper-bound checks, which are shared by both paths, can guard their use of it.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

bounds_check_index crashes when the array element type has no computable
byte size (e.g., arrays of unbounded arrays used in heap models).
object_descriptor_exprt::build calls size_of_expr on the element type,
and when that returns nullopt, the resulting ID_unknown offset causes a
precondition violation in from_integer downstream.

The fix checks whether size_of_expr on the element type returns a value.
When it does not, a simple index >= 0 lower-bound check is emitted
instead of the byte-offset-based one, and the object descriptor is not
built. A DATA_INVARIANT asserts that the array expression is a symbol or
nested index so that unexpected cases are caught rather than silently
skipping bounds checks. The object descriptor is wrapped in
std::optional so that the upper-bound checks, which are shared by both
paths, can guard their use of it.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a crash in bounds_check_index that occurs when checking bounds for arrays with element types that have no computable byte size, such as arrays of __CPROVER_integer (mathematical integers) or unbounded arrays used in heap models.

The crash occurred because object_descriptor_exprt::build calls size_of_expr on the element type, and when that returns std::nullopt, the resulting ID_unknown offset causes a precondition violation in from_integer downstream.

Changes:

  • Modified bounds_check_index to check if the element type has a computable size before building the object descriptor
  • Introduced a simplified index-based lower-bound check when element size is unavailable
  • Wrapped object_descriptor_exprt in std::optional to conditionally build and use it
  • Added defensive DATA_INVARIANT to catch unexpected array expression types with unsized elements
  • Added regression test for arrays with mathematical integer element types

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/ansi-c/goto-conversion/goto_check_c.cpp Modified bounds_check_index to handle unsized array element types with conditional object descriptor building and simplified bounds checks
regression/cbmc/bounds_check_integer_index1/main.c Added test case with __CPROVER_integer array to verify bounds checking doesn't crash
regression/cbmc/bounds_check_integer_index1/test.desc Test configuration verifying both lower and upper bound checks are generated
regression/validate-trace-xml-schema/check.py Registered new test in validation exclusion list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tautschnig tautschnig assigned tautschnig and unassigned kroening Feb 24, 2026
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.

3 participants