Skip to content

Add checks for symbol well-formedness. #3193

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
merged 1 commit into from
Dec 18, 2018
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
23 changes: 23 additions & 0 deletions src/util/symbol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Author: Daniel Kroening, [email protected]

#include "source_location.h"
#include "std_expr.h"
#include "suffix.h"

/// Dump the state of a symbol object to a given output stream.
/// \param out: The stream to output object state to.
Expand Down Expand Up @@ -121,3 +122,25 @@ symbol_exprt symbolt::symbol_expr() const
{
return symbol_exprt(name, type);
}

/// Check that the instance object is well formed.
/// \return: true if well-formed; false otherwise.
bool symbolt::is_well_formed() const
{
// Well-formedness criterion number 1 is for a symbol
// to have a non-empty mode (see #1880)
if(mode.empty())
{
return false;
}

// Well-formedness criterion number 2 is for a symbol
// to have it's base name as a suffix to it's more
// general name.
if(!has_suffix(id2string(name), id2string(base_name)))
{
return false;
}

return true;
}
3 changes: 3 additions & 0 deletions src/util/symbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ class symbolt
PRECONDITION(type.id() == ID_code);
value = exprt(ID_compiled);
}

/// Check that a symbol is well formed.
bool is_well_formed() const;
};

std::ostream &operator<<(std::ostream &out, const symbolt &symbol);
Expand Down
4 changes: 4 additions & 0 deletions src/util/symbol_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ void symbol_tablet::validate(const validation_modet vm) const
const auto symbol_key = elem.first;
const auto &symbol = elem.second;

// First of all, ensure symbol well-formedness
DATA_CHECK_WITH_DIAGNOSTICS(
vm, symbol.is_well_formed(), "Symbol is malformed: ", symbol_key);

// Check that symbols[id].name == id
DATA_CHECK_WITH_DIAGNOSTICS(
vm,
Expand Down
1 change: 1 addition & 0 deletions unit/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ SRC += analyses/ai/ai.cpp \
util/string_utils/split_string.cpp \
util/string_utils/strip_string.cpp \
util/symbol_table.cpp \
util/symbol.cpp \
util/unicode.cpp \
# Empty last line

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ SCENARIO(
code_typet fun_type2({}, type2);

symbolt function_symbol;
function_symbol.mode = "C";
irep_idt function_symbol_name = "foo";
function_symbol.name = function_symbol_name;

Expand Down
54 changes: 54 additions & 0 deletions unit/util/symbol.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/// Author: Diffblue Ltd.

/// \file Tests for symbol_tablet

#include <testing-utils/catch.hpp>
#include <util/exception_utils.h>
#include <util/symbol.h>

SCENARIO(
"Constructed symbol validity checks",
"[core][utils][symbol__validity_checks]")
{
GIVEN("A valid symbol")
{
symbolt symbol;
irep_idt symbol_name = "Test_TestBase";
symbol.name = symbol_name;
symbol.base_name = "TestBase";
symbol.module = "TestModule";
symbol.mode = "C";

THEN("Symbol should be well formed")
{
REQUIRE(symbol.is_well_formed());
}
}

GIVEN("An improperly initialised symbol")
{
symbolt symbol;

WHEN("The symbol doesn't have a valid mode")
{
symbol.mode = "";

THEN("Symbol well-formedness check should fail")
{
REQUIRE_FALSE(symbol.is_well_formed());
}
}

WHEN("The symbol doesn't have base name as a suffix to name")
{
symbol.name = "TEST";
Copy link
Contributor

Choose a reason for hiding this comment

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

That will fail due to a non-empty mode, which isn't what you intend to test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you were right. I fixed this now.

symbol.base_name = "TestBase";
symbol.mode = "C";

THEN("Symbol well-formedness check should fail")
{
REQUIRE_FALSE(symbol.is_well_formed());
}
}
}
}
3 changes: 2 additions & 1 deletion unit/util/symbol_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ SCENARIO(
symbol_tablet symbol_table;

symbolt symbol;
irep_idt symbol_name = "Test";
irep_idt symbol_name = "Test_TestBase";
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this test doesn't check symbol.is_well_formed(), it's a little surprising to see this check happening at the same time.

Copy link
Contributor Author

@NlightNFotis NlightNFotis Dec 14, 2018

Choose a reason for hiding this comment

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

This is because this was a test for something related added by @chrisr-diffblue but went in first, so my change had now broken this test, so I had to edit this test to conform to what the validity checks expected, and make it pass again.

symbol.name = symbol_name;
symbol.base_name = "TestBase";
symbol.module = "TestModule";
symbol.mode = "C";

symbol_table.insert(symbol);

Expand Down