Skip to content

Cover failed assertions #5636

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
CORE
test.c
--cover location --pointer-check --malloc-may-fail --malloc-fail-null
\[main.coverage.4\] file test.c line \d+ function main block 4 \(lines test.c:main:12,13\): SATISFIED
\[main.coverage.3\] file test.c line \d+ function main block 3 \(lines test.c:main:10\): FAILED
\[main.coverage.2\] file test.c line \d+ function main block 2 \(lines test.c:main:4,5,7,9\): SATISFIED
\[main.coverage.1\] file test.c line \d+ function main block 1 \(lines test.c:main:4\): SATISFIED
^EXIT=0$
^SIGNAL=0$
--
^warning: ignoring
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ It would be nice to have a comment in each of .desc files explaining what the test is intended to test. Even something pretty short could make this a while lot easier to maintain.

14 changes: 14 additions & 0 deletions regression/cbmc/cover-failed-assertions/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#include <stdlib.h>

int main()
{
int *ptr = malloc(sizeof(*ptr));
int a;

a = *ptr;

if(ptr == NULL)
a = 1;

return 0;
}
11 changes: 11 additions & 0 deletions regression/cbmc/cover-failed-assertions/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
CORE
test.c
--cover location --cover-failed-assertions --pointer-check --malloc-may-fail --malloc-fail-null
\[main.coverage.4\] file test.c line \d+ function main block 4 \(lines test.c:main:12,13\): SATISFIED
\[main.coverage.3\] file test.c line \d+ function main block 3 \(lines test.c:main:10\): SATISFIED
\[main.coverage.2\] file test.c line \d+ function main block 2 \(lines test.c:main:4,5,7,9\): SATISFIED
\[main.coverage.1\] file test.c line \d+ function main block 1 \(lines test.c:main:4\): SATISFIED
^EXIT=0$
^SIGNAL=0$
--
^warning: ignoring
27 changes: 19 additions & 8 deletions src/goto-instrument/cover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ void parse_cover_options(const cmdlinet &cmdline, optionst &options)
options.set_option(
"cover-traces-must-terminate",
cmdline.isset("cover-traces-must-terminate"));
options.set_option(
"cover-failed-assertions", cmdline.isset("cover-failed-assertions"));
}

/// Build data structures controlling coverage from command-line options.
Expand Down Expand Up @@ -223,6 +225,9 @@ cover_configt get_cover_config(
cover_config.traces_must_terminate =
options.get_bool_option("cover-traces-must-terminate");

cover_config.cover_failed_assertions =
options.get_bool_option("cover-failed-assertions");

return cover_config;
}

Expand Down Expand Up @@ -285,16 +290,22 @@ static void instrument_cover_goals(
// Simplify the common case where we have ASSERT(x); ASSUME(x):
if(i_it->is_assert())
{
auto successor = std::next(i_it);
if(
successor != function.body.instructions.end() &&
successor->is_assume() &&
successor->get_condition() == i_it->get_condition())
if(!cover_config.cover_failed_assertions)
{
successor->turn_into_skip();
auto successor = std::next(i_it);
if(
successor != function.body.instructions.end() &&
successor->is_assume() &&
successor->get_condition() == i_it->get_condition())
{
successor->turn_into_skip();
}
i_it->type = goto_program_instruction_typet::ASSUME;
}
else
{
i_it->turn_into_skip();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw we have to skip them rather than just leaving them be because otherwise cbmc would consider these assertions coverage goals.

Copy link
Member

Choose a reason for hiding this comment

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

Why not make --no-assertions (aka turn-assertions-into-skips) useable with --cover then (as @martin-cs suggests)? That would make the behaviour more transparent to the user.

}

i_it->type = goto_program_instruction_typet::ASSUME;
}
}
}
Expand Down
15 changes: 12 additions & 3 deletions src/goto-instrument/cover.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,17 @@ class message_handlert;
class cmdlinet;
class optionst;

#define OPT_COVER "(cover):"
#define HELP_COVER \
" --cover CC create test-suite with coverage criterion CC\n"
#define OPT_COVER \
"(cover):" \
"(cover-failed-assertions)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to have this as an argument to --cover?

Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ If we keep this as a separate option from --cover then we should check that --cover-failed-assertions is not passed without --cover and exit with a suitable error message in the case of that particular invalid usage.


#define HELP_COVER \
" --cover CC create test-suite with coverage criterion " \
"CC\n" \
" --cover-failed-assertions do not stop coverage checking at failed " \
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ Because of the preceding refactor, this option now applies to jdiff and goto-diff as well as cbmc. This means that we should probably test that it works with those entry points.

"assertions\n" \
" (this is the default for --cover " \
"assertions)\n"

enum class coverage_criteriont
{
Expand All @@ -41,6 +49,7 @@ enum class coverage_criteriont
struct cover_configt
{
bool keep_assertions;
bool cover_failed_assertions;
bool traces_must_terminate;
irep_idt mode;
function_filterst function_filters;
Expand Down