| Message ID | 1569939264-12539-1-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/docker: only enable ubsan for test-clang | expand |
On 10/1/19 10:14 AM, Paolo Bonzini wrote: > -fsanitize=undefined is not the same thing as --enable-sanitizers. After > commit 47c823e ("tests/docker: add sanitizers back to clang build", 2019-09-11) > test-clang is almost duplicating the asan (test-debug) test, so > partly revert commit 47c823e5b while leaving ubsan enabled. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> I got confused by this: ``` if compile_prog "$CPU_CFLAGS -Werror -fsanitize=undefined" ""; then have_ubsan=yes fi ``` it looked quite like --enable-sanitizers was indeed the same as -fsanitize=undefined; or at least was a superset of such. I suppose the argument you're making here is that we *only* want ubsan for test-clang? (I guess so, since test-debug also stipulates that clang be used; but enables debug.) In this case, is there a use for test-clang at all, actually? --js > --- > tests/docker/test-clang | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/docker/test-clang b/tests/docker/test-clang > index db9e697..8c51ead 100755 > --- a/tests/docker/test-clang > +++ b/tests/docker/test-clang > @@ -17,7 +17,9 @@ requires clang > > cd "$BUILD_DIR" > > -OPTS="--cxx=clang++ --cc=clang --host-cc=clang --enable-sanitizers" > +OPTS="--cxx=clang++ --cc=clang --host-cc=clang" > +OPTS="$OPTS --extra-cflags=-fsanitize=undefined \ > + --extra-cflags=-fno-sanitize=float-divide-by-zero" > build_qemu $OPTS > check_qemu > install_qemu >
On 01/10/19 19:59, John Snow wrote: > > it looked quite like --enable-sanitizers was indeed the same as > -fsanitize=undefined; or at least was a superset of such. > > I suppose the argument you're making here is that we *only* want ubsan > for test-clang? > > (I guess so, since test-debug also stipulates that clang be used; but > enables debug.) > > In this case, is there a use for test-clang at all, actually? Yes, test-clang checks that clang-specific warnings do not fire. Enabling ubsan isn't particularly useful in this respect, but it's okay for me to keep it. Paolo
On 10/6/19 12:06 PM, Paolo Bonzini wrote: > On 01/10/19 19:59, John Snow wrote: >> >> it looked quite like --enable-sanitizers was indeed the same as >> -fsanitize=undefined; or at least was a superset of such. >> >> I suppose the argument you're making here is that we *only* want ubsan >> for test-clang? >> >> (I guess so, since test-debug also stipulates that clang be used; but >> enables debug.) >> >> In this case, is there a use for test-clang at all, actually? > > Yes, test-clang checks that clang-specific warnings do not fire. > Enabling ubsan isn't particularly useful in this respect, but it's okay > for me to keep it. > > Paolo > If it's a proper subset of test-debug, let's just drop it entirely and leave it to be a "normal" clang invocation. (Oh, it was already pulled. well, whatever.) --js
diff --git a/tests/docker/test-clang b/tests/docker/test-clang index db9e697..8c51ead 100755 --- a/tests/docker/test-clang +++ b/tests/docker/test-clang @@ -17,7 +17,9 @@ requires clang cd "$BUILD_DIR" -OPTS="--cxx=clang++ --cc=clang --host-cc=clang --enable-sanitizers" +OPTS="--cxx=clang++ --cc=clang --host-cc=clang" +OPTS="$OPTS --extra-cflags=-fsanitize=undefined \ + --extra-cflags=-fno-sanitize=float-divide-by-zero" build_qemu $OPTS check_qemu install_qemu
-fsanitize=undefined is not the same thing as --enable-sanitizers. After commit 47c823e ("tests/docker: add sanitizers back to clang build", 2019-09-11) test-clang is almost duplicating the asan (test-debug) test, so partly revert commit 47c823e5b while leaving ubsan enabled. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- tests/docker/test-clang | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)