do not access typet::subtype() without cast #3087
Conversation
kroening
requested review from
chrisr-diffblue,
martin-cs,
peterschrammel,
pkesseli,
smowton and
thk123
as code owners
There was a problem hiding this comment.
Looks good, except I'm confused by parts of the commit message. It says "Casting to
the right specialisation prevents this." which I find misleading. It's the fact that these "casts" are non-trivial conversions doing checks. An actual cast like (type_with_subtypet)t would gain us nothing...
|
"Checked casts"? |
|
"checked casts" sounds like a reasonable solution. |
…tion Rationale: not every type has a subtype, and if a type doesn't have one, then accessing subtype() results in a memory safety violation. A checked cast to the right specialisation prevents this. Eventually, typet::subtype() will be removed to enforce this.
kroening
force-pushed
the
type-cleanup3
branch
from
8e3c091 to
bdfe7af
Compare
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: bdfe7af).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86712175
| DATA_INVARIANT( | ||
| array_type.id()==ID_array, | ||
| "index into array expected, found "+array_type.id_string()); | ||
| index_expr.array().type().id() == ID_array, |
There was a problem hiding this comment.
? Are you sure about removing the ns.follow, couldn't the array have symbol type here?
There was a problem hiding this comment.
No, we don't use symbol types for arrays (anymore - I think many many years ago we might).
Rationale: not every type has a subtype, and if a type doesn't have one,
then accessing subtype() results in a memory safety violation. Casting to
the right specialisation prevents this.
Eventually, typet::subtype() will be removed to enforce this.