Handle unsized array element types in bounds_check_index#8843
Open
tautschnig wants to merge 1 commit intodiffblue:developfrom
Open
Handle unsized array element types in bounds_check_index#8843tautschnig wants to merge 1 commit intodiffblue:developfrom
tautschnig wants to merge 1 commit intodiffblue:developfrom
Conversation
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.
There was a problem hiding this comment.
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_indexto 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_exprtinstd::optionalto conditionally build and use it - Added defensive
DATA_INVARIANTto 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.