Refactor slicing test runner and get rid of UB in tests #413
Conversation
Clang will always be installed in `LLVM_TOOLS_DIR` so we don't need to search for it individually.
TOOLSDIR -> DG_TOOLS_DIR SOURCESDIR -> TEST_SOURCES_DIR
lzaoral
changed the title
lzaoral
changed the title
|
Blocks #412 |
They will be removed in the cleanup phase anyway.
lzaoral
force-pushed
the
improve-slicer-tests
branch
3 times, most recently
from
9dbc9fa to
51478dc
Compare
|
@mchalupa This ended up being a rather big PR but it should have a significant impact (among other things) on the reproducibility of the tests. Thanks in advance! |
So that we know that the original code behaves in an expected way.
... if they are available.
as some tests used C11 features and many violate the strict aliasing rules. Tests that rely on GNU C extensions have set the correct compiler flags on their own.
According to C11, unaligned access is UB: A pointer to an object type may be converted to a pointer to a different object type. If the resulting pointer is not correctly aligned for the referenced type, the behavior is undefined.
Namely, -Wimplicit-function-declaration and -Wincompatible-library-redeclaration.
lzaoral
force-pushed
the
improve-slicer-tests
branch
from
51478dc to
1b70277
Compare
|
@mchalupa I've incorporated your suggestions and additionally relaxed the input conditions of |
lzaoral
force-pushed
the
improve-slicer-tests
branch
from
1b70277 to
4aa3201
Compare
tests/slicing/sources/bitcast2.c
Outdated
|
|
||
| int result = 0; | ||
| for (unsigned i = 0; i < sizeof(int); i++) | ||
| result |= (byte[i] = 0x1b) << i * CHAR_BIT; |
There was a problem hiding this comment.
This completely changes the test, at least from the point of view of the slicer. Accessing the bytes via constants (e.g., byte[1]) leads to a precise knowledge of what memory is accessed, while accessing it via a variable (bytes[i]) leads to over-approximating the precise knowledge. Please, keep the spirit of the test as it was (accessing bytes via constants). If you want this test with the loop, just add it as a new test (but check that there is not a similar test already).
There was a problem hiding this comment.
Oh, I see. I wanted to make the test platform as platform independent as possible so didn't even think about this. I'll revert this change and add this test as a new one. There is already a rather similar one but that one is accessing the bytes by *byte; byte++ pattern and not by byte[i] so I guess that's ok.
There was a problem hiding this comment.
Nevertheless, I'll change the the type of the char pointer to be explicitly unsigned to silence a clang-tidy warning about implementation defined behaviour of a narrowing conversion from int to char for values greater than 127 which 0xab is.
to silence the following clang-tidy warning: Narrowing conversion from constant value 171 (0x000000AB) of type 'int' to signed type 'char' is implementation-defined
... that sets bytes in int in a for loop using `bytes[i]` expression.
lzaoral
force-pushed
the
improve-slicer-tests
branch
from
4aa3201 to
6131568
Compare
|
Very well, let's merge this then. Thanks a lot! |
Some small fixes and improvements that I've additionally made during #412 but are independent of it.