Fix unit test issues on address sanitizer and fortify (#437)
This commit does four things: 1. On various images, the linker was not able to correctly load the flto optimizations from the archive generated for unit tests, and was throwing errors. I was able to solve this by updating the plugin for the fortify test, but was unable to reproduce it on the ASAN tests or find a solution. So I decided to go with a single solution for now, which was to just disable the linker optimizations for those tests. This shouldn't weaken the protections provided by ASAN. 2. The change to remove flto for some reason caused some odd inlining behavior in the intset test, that I wasn't really able to understand. The error was basically that we were doing a 4 byte write, starting at byte offset 8, for the first addition to listpack that was of size 10. Practically this has no effect, since I'm not aware of any allocator that would give us a 10 byte block as opposed to 12 (or more likely 16) bytes. The isn't the correct behavior, since an uninitialized listpack defaults to 16bit encoding, which should only be writing 2 bytes. I rabbit holed like 2 hours into this, and gave up and just ignored the warning on the file. 3. Now that address sanitizer was correctly running, it picked up two issues. A memory leak and uninitialized value, so those were easy to fix. 4. There is also a small change to the fortify to build the test up front instead of later, this is just to be consistent with other tests and has no functional change. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
This commit is contained in:
parent
cc703aa3bc
commit
1b3199e070
8
.github/workflows/daily.yml
vendored
8
.github/workflows/daily.yml
vendored
@ -101,7 +101,7 @@ jobs:
|
||||
run: |
|
||||
apt-get update && apt-get install -y make gcc-13
|
||||
update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-13 100
|
||||
make CC=gcc SERVER_CFLAGS='-Werror -DSERVER_TEST -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3'
|
||||
make all-with-unit-tests CC=gcc OPT=-O3 SERVER_CFLAGS='-Werror -DSERVER_TEST -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3'
|
||||
- name: testprep
|
||||
run: apt-get install -y tcl8.6 tclx procps
|
||||
- name: test
|
||||
@ -121,7 +121,7 @@ jobs:
|
||||
run: ./src/valkey-server test all --accurate
|
||||
- name: new unit tests
|
||||
if: true && !contains(github.event.inputs.skiptests, 'unittest')
|
||||
run: make valkey-unit-tests && ./src/valkey-unit-tests --accurate
|
||||
run: ./src/valkey-unit-tests --accurate
|
||||
|
||||
test-ubuntu-libc-malloc:
|
||||
runs-on: ubuntu-latest
|
||||
@ -598,7 +598,7 @@ jobs:
|
||||
repository: ${{ env.GITHUB_REPOSITORY }}
|
||||
ref: ${{ env.GITHUB_HEAD_REF }}
|
||||
- name: make
|
||||
run: make all-with-unit-tests SANITIZER=address SERVER_CFLAGS='-DSERVER_TEST -Werror -DDEBUG_ASSERTIONS'
|
||||
run: make all-with-unit-tests OPT=-O3 SANITIZER=address SERVER_CFLAGS='-DSERVER_TEST -Werror -DDEBUG_ASSERTIONS'
|
||||
- name: testprep
|
||||
# Work around ASAN issue, see https://github.com/google/sanitizers/issues/1716
|
||||
run: |
|
||||
@ -650,7 +650,7 @@ jobs:
|
||||
repository: ${{ env.GITHUB_REPOSITORY }}
|
||||
ref: ${{ env.GITHUB_HEAD_REF }}
|
||||
- name: make
|
||||
run: make all-with-unit-tests SANITIZER=undefined SERVER_CFLAGS='-DSERVER_TEST -Werror' LUA_DEBUG=yes # we (ab)use this flow to also check Lua C API violations
|
||||
run: make all-with-unit-tests OPT=-O3 SANITIZER=undefined SERVER_CFLAGS='-DSERVER_TEST -Werror' LUA_DEBUG=yes # we (ab)use this flow to also check Lua C API violations
|
||||
- name: testprep
|
||||
run: |
|
||||
sudo apt-get update
|
||||
|
@ -26,7 +26,7 @@ long long _ustime(void) {
|
||||
}
|
||||
|
||||
static int bench_crc64(unsigned char *data, uint64_t size, long long passes, uint64_t check, char *name, int csv) {
|
||||
uint64_t min = size, hash;
|
||||
uint64_t min = size, hash = 0;
|
||||
long long original_start = _ustime(), original_end;
|
||||
for (long long i=passes; i > 0; i--) {
|
||||
hash = crc64(0, data, size);
|
||||
|
@ -3,6 +3,14 @@
|
||||
|
||||
#include "../intset.c"
|
||||
#include "test_help.h"
|
||||
#if defined(__GNUC__) && __GNUC__ >= 7
|
||||
/* Several functions in this file get inlined in such a way that fortify warns there might
|
||||
* be an out of bounds memory access depending on the intset encoding, but they aren't actually
|
||||
* reachable because we check the encoding. There are other strategies to fix this, but they
|
||||
* all require other hacks to prevent the inlining. So for now, just omit the check. */
|
||||
#pragma GCC diagnostic push
|
||||
#pragma GCC diagnostic ignored "-Warray-bounds"
|
||||
#endif
|
||||
|
||||
static long long usec(void) {
|
||||
struct timeval tv;
|
||||
@ -135,7 +143,6 @@ int test_intsetUpgradeFromint16Toint64(int argc, char **argv, int flags) {
|
||||
UNUSED(flags);
|
||||
|
||||
intset *is = intsetNew();
|
||||
is = intsetNew();
|
||||
is = intsetAdd(is,32,NULL);
|
||||
TEST_ASSERT(intrev32ifbe(is->encoding) == INTSET_ENC_INT16);
|
||||
is = intsetAdd(is,4294967295,NULL);
|
||||
@ -227,3 +234,7 @@ int test_intsetStressAddDelete(int argc, char **argv, int flags) {
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
#if defined(__GNUC__) && __GNUC__ >= 12
|
||||
#pragma GCC diagnostic pop
|
||||
#endif
|
||||
|
Loading…
x
Reference in New Issue
Block a user