Skip to content

Verify member expressions [blocks: #4023] #4021

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions src/util/std_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Author: Daniel Kroening, kroening@kroening.com
#include "c_types.h"
#include "expr_util.h"
#include "mathematical_types.h"
#include "namespace.h"
#include "pointer_offset_size.h"
#include "simplify_expr.h"

Expand Down Expand Up @@ -226,3 +227,43 @@ const exprt &object_descriptor_exprt::root_object() const

return *p;
}

/// Check that the member expression has the right number of operands, refers
/// to a component that exists on its underlying compound type, and uses the
/// same type as is declared on that compound type. Throws or raises an
/// invariant if not, according to validation mode.
/// \param expr: expression to validate
/// \param ns: global namespace
/// \param vm: validation mode (see \ref exprt::validate)
void member_exprt::validate(
const exprt &expr,
const namespacet &ns,
const validation_modet vm)
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid abbreviations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every other validate method is doing the same thing, so I'll leave this as-is for now (change them en masse if we want to)

{
check(expr, vm);

const auto &member_expr = to_member_expr(expr);
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ why not take a member_exprt as argument instead, and let the caller call to_member_expr if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Appears to be normal for validate methods, again change en masse or not at all


const typet &compound_type = ns.follow(member_expr.compound().type());
const auto *struct_union_type =
type_try_dynamic_cast<struct_union_typet>(compound_type);
DATA_CHECK(
vm,
struct_union_type != nullptr,
"member must address a struct, union or compatible type");

const auto &component =
struct_union_type->get_component(member_expr.get_component_name());

DATA_CHECK(
vm,
component.is_not_nil(),
"member component '" + id2string(member_expr.get_component_name()) +
"' must exist on addressed type");

DATA_CHECK(
vm,
component.type() == member_expr.type(),
"member expression's type must match the addressed struct or union "
"component");
}
15 changes: 15 additions & 0 deletions src/util/std_expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3795,6 +3795,21 @@ class member_exprt:public unary_exprt
{
return op0();
}

static void check(
Copy link
Contributor

Choose a reason for hiding this comment

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

The two functions you introduce should be documented

Copy link
Collaborator

Choose a reason for hiding this comment

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

@smowton Still to be done?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there really any value in documenting one-liner that says "assert(ops.count == 1)"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not so sure, I was just confused by you giving a thumbs-up but then seemingly no such change was included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I documented validate, which is much more substantial

const exprt &expr,
const validation_modet vm = validation_modet::INVARIANT)
{
DATA_CHECK(
vm,
expr.operands().size() == 1,
"member expression must have one operand");
}

static void validate(
const exprt &expr,
const namespacet &ns,
const validation_modet vm = validation_modet::INVARIANT);
};

template <>
Expand Down
4 changes: 4 additions & 0 deletions src/util/validate_expressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ void call_on_expr(const exprt &expr, Args &&... args)
{
CALL_ON_EXPR(ssa_exprt);
}
else if(expr.id() == ID_member)
{
CALL_ON_EXPR(member_exprt);
}
else
{
#ifdef REPORT_UNIMPLEMENTED_EXPRESSION_CHECKS
Expand Down