Skip to content

Add --object-bits option to goto-cc #5440

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
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
5 changes: 5 additions & 0 deletions doc/cprover-manual/goto-cc.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ most important architectural parameters are:
`sizeof(long int)` on various machines.
- The width of pointers; for example, compare the value of `sizeof(int *)` on
various machines.
- The number of bits in a pointer which are used to differentiate between
different objects. The remaining bits of a pointer are used for offsets
within objects.
- The [endianness](http://en.wikipedia.org/wiki/Endianness) of
the architecture.

Expand All @@ -129,6 +132,8 @@ following command-line arguments can be passed to the CPROVER tools:
- The word-width can be set with `--16`, `--32`, `--64`.
- The endianness can be defined with `--little-endian` and
`--big-endian`.
- The number of bits in a pointer used to differentiate between different
objects can be set using `--object-bits x`. Where `x` is the number of bits.

When using a goto binary, CBMC and the other tools read the
configuration from the binary. The setting when running goto-cc is
Expand Down
1 change: 1 addition & 0 deletions regression/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ else()
add_subdirectory(goto-cl)
endif()
add_subdirectory(goto-cc-cbmc)
add_subdirectory(goto-cc-cbmc-shared-options)
add_subdirectory(cbmc-cpp)
add_subdirectory(goto-cc-goto-analyzer)
add_subdirectory(statement-list)
Expand Down
9 changes: 9 additions & 0 deletions regression/goto-cc-cbmc-shared-options/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
if(WIN32)
set(is_windows true)
else()
set(is_windows false)
endif()

add_test_pl_tests(
"${CMAKE_CURRENT_SOURCE_DIR}/chain.sh $<TARGET_FILE:goto-cc> $<TARGET_FILE:cbmc> ${is_windows}"
)
35 changes: 35 additions & 0 deletions regression/goto-cc-cbmc-shared-options/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
default: tests.log

include ../../src/config.inc
include ../../src/common

ifeq ($(BUILD_ENV_),MSVC)
exe=../../../src/goto-cc/goto-cl
is_windows=true
else
exe=../../../src/goto-cc/goto-cc
is_windows=false
endif

test:
@../test.pl -e -p -c '../chain.sh $(exe) ../../../src/cbmc/cbmc $(is_windows)'

tests.log:
@../test.pl -e -p -c '../chain.sh $(exe) ../../../src/cbmc/cbmc $(is_windows)'

show:
@for dir in *; do \
if [ -d "$$dir" ]; then \
vim -o "$$dir/*.c" "$$dir/*.out"; \
fi; \
done;

clean:
@for dir in *; do \
$(RM) tests.log; \
if [ -d "$$dir" ]; then \
cd "$$dir"; \
$(RM) *.out *.gb; \
cd ..; \
fi \
done
19 changes: 19 additions & 0 deletions regression/goto-cc-cbmc-shared-options/chain.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/usr/bin/env bash

goto_cc=$1
cbmc=$2
is_windows=$3

options=${*:4:$#-4}
name=${*:$#}
base_name=${name%.c}
base_name=${base_name%.cpp}

if [[ "${is_windows}" == "true" ]]; then
"${goto_cc}" "${name}" ${options}
mv "${base_name}.exe" "${base_name}.gb"
else
"${goto_cc}" "${name}" -o "${base_name}.gb" ${options}
fi

"${cbmc}" "${base_name}.gb" ${options}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
CORE
test.c
--function main
^EXIT=10$
^SIGNAL=0$
^VERIFICATION FAILED$
assertion object_bits != 6: SUCCESS
assertion object_bits != 8: FAILURE

Choose a reason for hiding this comment

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

can you add the expected results for the other assertions in here as well? Clearly it's supposed to be always 2 successes and one failure but that might be easier to see if its spelt out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

assertion object_bits != 10: SUCCESS
--
^warning: ignoring
--
Test that the default value for object-bits is 8.
14 changes: 14 additions & 0 deletions regression/goto-cc-cbmc-shared-options/object-bits/fewer_bits.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
CORE
test.c
--function main --object-bits 6
^EXIT=10$
^SIGNAL=0$
^VERIFICATION FAILED$
assertion object_bits != 6: FAILURE
assertion object_bits != 8: SUCCESS
assertion object_bits != 10: SUCCESS
--
^warning: ignoring
--
Test test running with fewer bits than usual results in correct setup of
intrinsic constants.
14 changes: 14 additions & 0 deletions regression/goto-cc-cbmc-shared-options/object-bits/more_bits.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
CORE
test.c
--function main --object-bits 10
^EXIT=10$
^SIGNAL=0$
^VERIFICATION FAILED$
assertion object_bits != 6: SUCCESS
assertion object_bits != 8: SUCCESS
assertion object_bits != 10: FAILURE
--
^warning: ignoring
--
Test test running with more bits than usual results in correct setup of
intrinsic constants.
26 changes: 26 additions & 0 deletions regression/goto-cc-cbmc-shared-options/object-bits/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#include <assert.h>
#include <stdlib.h>

size_t
find_first_set(const size_t max_malloc_size, const size_t bits_accumulator)
{
if(max_malloc_size & 1)
return bits_accumulator;
return find_first_set(max_malloc_size >> 1, bits_accumulator + 1);
}

size_t calculate_object_bits()
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it would be unnecessary to reverse engineer the object bits here or even use the value of __CPROVER_max_malloc_size. It's probably sufficient to just run cbmc with a non-default object bits setting after goto-cc and check that it suceeds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I chose to do it this way is that the previous incorrect behaviour resulted in a warning, but might not actually cause a failure without giving an input program which depends on the implications of the object-bits configuration. I could have used a negative regex to check that the warning message does not appear in the output. However this would not be a robust test because if the text of the message were to be updated then the corresponding test could easily be missed. This may be a little over-engineered but I am happy that this is a robust test and it is clear what the positive regexes are actually checking for.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that makes sense.

{
const size_t ptr_size = sizeof(void *) * 8;
return ptr_size - find_first_set(__CPROVER_max_malloc_size, 1);
}

int main()
{
void *temp = malloc(2);
size_t object_bits = calculate_object_bits();
assert(object_bits != 6);
assert(object_bits != 8);
assert(object_bits != 10);
__CPROVER_assume("end of main.");
}
4 changes: 4 additions & 0 deletions regression/goto-cc-cbmc-shared-options/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
This directory is for tests where we -
1) Run `goto-cc` on the specified input file, with the specified options.
2) Run `cbmc` on the goto binary produced in step 1. Using the same options
from the `.desc` file as were specified in step 1.
1 change: 1 addition & 0 deletions src/goto-cc/gcc_cmdline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const char *goto_cc_options_with_separated_argument[]=
"--native-linker",
"--print-rejected-preprocessed-source",
"--mangle-suffix",
"--object-bits",

Choose a reason for hiding this comment

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

Shouldn't this be added to the other cmdlinets as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original ticket referred to goto-cc only, fixing goto-cc only will get a fix ready for use sooner. Yes, the same problem and fix probably applies to the other entry points as well. The time consuming part of fixing the other entry points would be writing tests for each of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no standard list of options in the base class. So it would need to be manually added and tested for each cmdlinet in the heirarchy.

nullptr
};

Expand Down
1 change: 1 addition & 0 deletions src/goto-cc/goto_cc_mode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ void goto_cc_modet::help()
" --native-assembler cmd command to invoke as assembler (goto-as only)\n"
" --print-rejected-preprocessed-source file\n"
" copy failing (preprocessed) source to file\n"
" --object-bits number of bits used for object addresses\n"
"\n";
// clang-format on
}
Expand Down