| Message ID | 573CA781.50505@samsung.com |
|---|---|
| State | New |
| Headers | show |
On Wed, May 18, 2016 at 08:33:53PM +0300, Maxim Ostapenko wrote: > when compiling with -fsanitize=address we define __SANITIZE_ADDRESS__ > macros, but we don't do this for -fsanitize=thread and -fsanitize=undefined. > Perhaps we should be more symmetric here and define corresponding > __SANITIZE_THREAD__ and __SANITIZE_UNDEFINED__ macros respectively? > > I added two simple test cases to c-c++-common/{ub, t}san/ directories that > just verify if __SANITIZE_THREAD__ (__SANITIZE_UNDEFINED__) is defined. Is > that a proper way how we check that the macros defined correctly? Does this > patch looks reasonable? I can understand __SANITIZE_THREAD__, but I fail to see what __SANITIZE_UNDEFINED__ would be good for, especially when it is not just a single sanitizer, but dozens of them. Jakub
On Wed, May 18, 2016 at 8:36 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, May 18, 2016 at 08:33:53PM +0300, Maxim Ostapenko wrote: >> when compiling with -fsanitize=address we define __SANITIZE_ADDRESS__ >> macros, but we don't do this for -fsanitize=thread and -fsanitize=undefined. >> Perhaps we should be more symmetric here and define corresponding >> __SANITIZE_THREAD__ and __SANITIZE_UNDEFINED__ macros respectively? >> >> I added two simple test cases to c-c++-common/{ub, t}san/ directories that >> just verify if __SANITIZE_THREAD__ (__SANITIZE_UNDEFINED__) is defined. Is >> that a proper way how we check that the macros defined correctly? Does this >> patch looks reasonable? > > I can understand __SANITIZE_THREAD__, but I fail to see what > __SANITIZE_UNDEFINED__ would be good for, especially when it is not just > a single sanitizer, but dozens of them. Some low-level codes do nasty things like unaligned or NULL memory accesses. This would allow them to selectively disable UBSan. Good point about different UB checks though. -Y
On Wed, May 18, 2016 at 8:33 PM, Maxim Ostapenko <m.ostapenko@samsung.com> wrote: > Hi, > > when compiling with -fsanitize=address we define __SANITIZE_ADDRESS__ > macros, but we don't do this for -fsanitize=thread and -fsanitize=undefined. > Perhaps we should be more symmetric here and define corresponding > __SANITIZE_THREAD__ and __SANITIZE_UNDEFINED__ macros respectively? Are these available in Clang as well? > I added two simple test cases to c-c++-common/{ub, t}san/ directories that > just verify if __SANITIZE_THREAD__ (__SANITIZE_UNDEFINED__) is defined. Is > that a proper way how we check that the macros defined correctly? Does this > patch looks reasonable? > > -Maxim
On 18/05/16 20:39, Yuri Gribov wrote: > On Wed, May 18, 2016 at 8:33 PM, Maxim Ostapenko > <m.ostapenko@samsung.com> wrote: >> Hi, >> >> when compiling with -fsanitize=address we define __SANITIZE_ADDRESS__ >> macros, but we don't do this for -fsanitize=thread and -fsanitize=undefined. >> Perhaps we should be more symmetric here and define corresponding >> __SANITIZE_THREAD__ and __SANITIZE_UNDEFINED__ macros respectively? > Are these available in Clang as well? AFAIK Clang has __has_feature(address_sanitizer) and __has_feature(thread_sanitizer). UBSan doesn't have such things. > >> I added two simple test cases to c-c++-common/{ub, t}san/ directories that >> just verify if __SANITIZE_THREAD__ (__SANITIZE_UNDEFINED__) is defined. Is >> that a proper way how we check that the macros defined correctly? Does this >> patch looks reasonable? >> >> -Maxim >
gcc/ChangeLog: 2016-05-19 Maxim Ostapenko <m.ostapenko@samsung.com> PR sanitizer/64354 * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add new builtin __SANITIZE_THREAD__ and __SANITIZE_UNDEFINED__ macros for -fsanitize=thread and -fsanitize=undefined switches respectively. * doc/cpp.texi: Document new macros. gcc/testsuite/ChangeLog: 2016-05-19 Maxim Ostapenko <m.ostapenko@samsung.com> PR sanitizer/64354 * c-c++-common/tsan/sanitize-thread-macro.c: New test. * c-c++-common/ubsan/sanitize-undefined-macro.c: Likewise. diff --git a/gcc/cppbuiltin.c b/gcc/cppbuiltin.c index 6d494ad..8fd0723 100644 --- a/gcc/cppbuiltin.c +++ b/gcc/cppbuiltin.c @@ -92,6 +92,12 @@ define_builtin_macros_for_compilation_flags (cpp_reader *pfile) if (flag_sanitize & SANITIZE_ADDRESS) cpp_define (pfile, "__SANITIZE_ADDRESS__"); + if (flag_sanitize & SANITIZE_THREAD) + cpp_define (pfile, "__SANITIZE_THREAD__"); + + if (flag_sanitize & SANITIZE_UNDEFINED) + cpp_define (pfile, "__SANITIZE_UNDEFINED__"); + if (optimize_size) cpp_define (pfile, "__OPTIMIZE_SIZE__"); if (optimize) diff --git a/gcc/doc/cpp.texi b/gcc/doc/cpp.texi index 9f914b2..965795b 100644 --- a/gcc/doc/cpp.texi +++ b/gcc/doc/cpp.texi @@ -2362,6 +2362,12 @@ in use. This macro is defined, with value 1, when @option{-fsanitize=address} or @option{-fsanitize=kernel-address} are in use. +@item __SANITIZE_THREAD__ +This macro is defined, with value 1, when @option{-fsanitize=thread} is in use. + +@item __SANITIZE_UNDEFINED__ +This macro is defined, with value 1, when @option{-fsanitize=undefined} is in use. + @item __TIMESTAMP__ This macro expands to a string constant that describes the date and time of the last modification of the current source file. The string constant diff --git a/gcc/testsuite/c-c++-common/tsan/sanitize-thread-macro.c b/gcc/testsuite/c-c++-common/tsan/sanitize-thread-macro.c new file mode 100644 index 0000000..2b8a840 --- /dev/null +++ b/gcc/testsuite/c-c++-common/tsan/sanitize-thread-macro.c @@ -0,0 +1,12 @@ +/* Check that -fsanitize=thread options defines __SANITIZE_THREAD__ macros. */ +/* { dg-do compile } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ + +int +main () +{ +#ifndef __SANITIZE_THREAD__ + bad construction +#endif + return 0; +} diff --git a/gcc/testsuite/c-c++-common/ubsan/sanitize-undefined-macro.c b/gcc/testsuite/c-c++-common/ubsan/sanitize-undefined-macro.c new file mode 100644 index 0000000..7461324 --- /dev/null +++ b/gcc/testsuite/c-c++-common/ubsan/sanitize-undefined-macro.c @@ -0,0 +1,13 @@ +/* Check that -fsanitize=undefined options defines __SANITIZE_UNDEFINED__ macros. */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined" } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ + +int +main () +{ +#ifndef __SANITIZE_UNDEFINED__ + bad construction +#endif + return 0; +}
Hi, when compiling with -fsanitize=address we define __SANITIZE_ADDRESS__ macros, but we don't do this for -fsanitize=thread and -fsanitize=undefined. Perhaps we should be more symmetric here and define corresponding __SANITIZE_THREAD__ and __SANITIZE_UNDEFINED__ macros respectively? I added two simple test cases to c-c++-common/{ub, t}san/ directories that just verify if __SANITIZE_THREAD__ (__SANITIZE_UNDEFINED__) is defined. Is that a proper way how we check that the macros defined correctly? Does this patch looks reasonable? -Maxim