-
Notifications
You must be signed in to change notification settings - Fork 277
Disable unnecessary pointer checks, enable necessary pointer checks i… #6459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,9 @@ main.c | |
^\[foo.\d+\] line \d+ Check that y is assignable: FAILURE$ | ||
^\[foo.\d+\] line \d+ Check that z is assignable: FAILURE$ | ||
^\[foo.\d+\] line \d+ Check that POINTER_OBJECT\(\(void \*\)z\) is assignable: FAILURE$ | ||
^.*tmp_cc\$\d+.*: FAILURE$ | ||
^.*__car_valid.*: FAILURE$ | ||
^.*__car_lb.*: FAILURE$ | ||
^.*__car_ub.*: FAILURE$ | ||
-- | ||
Checks that invalidated CARs are correctly tracked on `free` and `DEAD`. | ||
|
||
|
@@ -40,4 +42,4 @@ We also check (using positive regex matches above) that | |
`**p` should be assignable in one case and should not be assignable in the other. | ||
|
||
Finally, we check that there should be no "internal" assertion failures | ||
on `tmp_cc` variables used to track CARs. | ||
on `__car_valid`, `__car_ub`, `__car_lb` variables used to track CARs. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes in this file should be folded into the commit introducing the new names. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,14 +52,16 @@ static const slicet normalize_to_slice(const exprt &expr, const namespacet &ns) | |
} | ||
|
||
const symbolt assigns_clauset::conditional_address_ranget::generate_new_symbol( | ||
const std::string &prefix, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Random place for comment, and primarily for future reference: I would like to see commits like this be a pull request of their own. Changing the names of CAR instrumentation variables really has nothing to do with disabling selected checks. |
||
const typet &type, | ||
const source_locationt &location) const | ||
{ | ||
return new_tmp_symbol( | ||
type, | ||
location, | ||
parent.symbol_table.lookup_ref(parent.function_name).mode, | ||
parent.symbol_table); | ||
parent.symbol_table, | ||
prefix); | ||
} | ||
|
||
assigns_clauset::conditional_address_ranget::conditional_address_ranget( | ||
|
@@ -70,11 +72,13 @@ assigns_clauset::conditional_address_ranget::conditional_address_ranget( | |
parent(parent), | ||
slice(normalize_to_slice(expr, parent.ns)), | ||
validity_condition_var( | ||
generate_new_symbol(bool_typet(), location).symbol_expr()), | ||
generate_new_symbol("__car_valid", bool_typet(), location).symbol_expr()), | ||
lower_bound_address_var( | ||
generate_new_symbol(slice.first.type(), location).symbol_expr()), | ||
generate_new_symbol("__car_lb", slice.first.type(), location) | ||
.symbol_expr()), | ||
upper_bound_address_var( | ||
generate_new_symbol(slice.first.type(), location).symbol_expr()) | ||
generate_new_symbol("__car_ub", slice.first.type(), location) | ||
.symbol_expr()) | ||
{ | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,8 +60,10 @@ class assigns_clauset | |
const exprt | ||
generate_unsafe_inclusion_check(const conditional_address_ranget &) const; | ||
|
||
const symbolt | ||
generate_new_symbol(const typet &, const source_locationt &) const; | ||
const symbolt generate_new_symbol( | ||
const std::string &prefix, | ||
const typet &, | ||
const source_locationt &) const; | ||
Comment on lines
+64
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency, add the name of the remaining parameters or remove |
||
|
||
friend class assigns_clauset; | ||
}; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -142,13 +142,18 @@ const symbolt &new_tmp_symbol( | |||||||||
const typet &type, | ||||||||||
const source_locationt &location, | ||||||||||
const irep_idt &mode, | ||||||||||
symbol_table_baset &symtab) | ||||||||||
symbol_table_baset &symtab, | ||||||||||
std::string suffix, | ||||||||||
Comment on lines
+145
to
+146
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
bool is_auxiliary) | ||||||||||
{ | ||||||||||
return get_fresh_aux_symbol( | ||||||||||
symbolt &new_symbol = get_fresh_aux_symbol( | ||||||||||
type, | ||||||||||
id2string(location.get_function()) + "::tmp_cc", | ||||||||||
"tmp_cc", | ||||||||||
id2string(location.get_function()) + "::", | ||||||||||
suffix, | ||||||||||
location, | ||||||||||
mode, | ||||||||||
symtab); | ||||||||||
new_symbol.is_auxiliary = is_auxiliary; | ||||||||||
return new_symbol; | ||||||||||
} | ||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -105,17 +105,21 @@ void insert_before_swap_and_advance( | |||||||||
goto_programt::targett &target, | ||||||||||
goto_programt &payload); | ||||||||||
|
||||||||||
/// \brief Adds a new temporary symbol to the symbol table. | ||||||||||
/// \brief Adds a fresh and uniquely named symbol to the symbol table. | ||||||||||
/// | ||||||||||
/// \param type: The type of the new symbol. | ||||||||||
/// \param location: The source location for the new symbol. | ||||||||||
/// \param mode: The mode for the new symbol, e.g. ID_C, ID_java. | ||||||||||
/// \param symtab: The symbol table to which the new symbol is to be added. | ||||||||||
/// \param suffix: Suffix to use to generate the unique name | ||||||||||
/// \param is_auxiliary: Do not print symbol in traces if true (default = false) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
/// \return The new symbolt object. | ||||||||||
const symbolt &new_tmp_symbol( | ||||||||||
const typet &type, | ||||||||||
const source_locationt &location, | ||||||||||
const irep_idt &mode, | ||||||||||
symbol_table_baset &symtab); | ||||||||||
symbol_table_baset &symtab, | ||||||||||
std::string suffix = "tmp_cc", | ||||||||||
bool is_auxiliary = false); | ||||||||||
|
||||||||||
#endif // CPROVER_GOTO_INSTRUMENT_CONTRACTS_UTILS_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: I'd prefer even more readable names for users:
__car_valid
,__car_lower_bound
, and__car_upper_bound
. However, the fact that these variables exist should be transparent to users. Why would this variables appear as failures? I guess you want to make sure here there are no memory safety errors in any operation using these variables, is that correct?