On Wed, Jul 04, 2018 at 08:20:47PM +0300, Maxim Ostapenko wrote:
> On 07/04/2018 05:45 AM, Jeff Law wrote:
> > On 05/23/2018 11:15 AM, Maxim Ostapenko wrote:
> >> as described in PR, when using both ASan and UBSan
> >> (-fsanitize=address,undefined ), we have symbols collision for global
> >> functions, like __sanitizer_set_report_path. This leads to fuzzy results
> >> when printing reports into files e.g. for this test case:
> >>
> >> #include <sanitizer/common_interface_defs.h>
> >> int main(int argc, char **argv) {
> >> __sanitizer_set_report_path("/tmp/sanitizer.txt");
> >> int i = 23;
> >> i <<= 32;
> >> int *array = new int[100];
> >> delete [] array;
> >> return array[argc];
> >> }
> >>
> >> only ASan's report gets written to file; UBSan output goes to stderr.
> >>
> >> To resolve this issue we could use two approaches:
> >>
> >> 1) Use the same approach to that is implemented in Clang (UBSan embedded
> >> to ASan). The only caveat here is that we need to link (unused) C++ part
> >> of UBSan even in C programs when linking static ASan runtime. This
> >> happens because GCC, as opposed to Clang, doesn't split C and C++
> >> runtimes for sanitizers.
> >>
> >> 2) Just add SANITIZER_INTERFACE_ATTRIBUTE to report_file global
> >> variable. In this case all __sanitizer_set_report_path calls will set
> >> the same report_file variable. IMHO this is a hacky way to fix the
> >> issue, it's better to use the first option if possible.
> >>
> >>
> >> The attached patch fixes the symbols collision by embedding UBSan into
> >> ASan (variant 1), just like we do for LSan.
> >>
> >>
> >> Regtested/bootstrapped on x86_64-unknown-linux-gnu, looks reasonable
> >> enough for trunk?
> >>
> >>
> >> -Maxim
> >>
> >>
> >> pr84250-2.diff
> >>
> >>
> >> gcc/ChangeLog:
> >>
> >> 2018-05-23 Maxim Ostapenko <m.ostape...@samsung.com>
> >>
> >> * config/gnu-user.h (LIBASAN_EARLY_SPEC): Pass -lstdc++ for static
> >> libasan.
> >> * gcc.c: Do not pass LIBUBSAN_SPEC if ASan is enabled with UBSan.
> >>
> >> libsanitizer/ChangeLog:
> >>
> >> 2018-05-23 Maxim Ostapenko <m.ostape...@samsung.com>
> >>
> >> * Makefile.am: Reorder libs.
> >> * Makefile.in: Regenerate.
> >> * asan/Makefile.am: Define DCAN_SANITIZE_UB=1, add dependancy from
> >> libsanitizer_ubsan.la.
> >> * asan/Makefile.in: Regenerate.
> >> * ubsan/Makefile.am: Define new libsanitizer_ubsan.la library.
> >> * ubsan/Makefile.in: Regenerate.
> > You know this code better than anyone else working on GCC. My only
> > concern would be the kernel builds with asan, but I suspect they're
> > providing their own runtime anyway, so the libstdc++ caveat shouldn't apply.
>
> Yes, you are right, kernel provides its own runtime.
>
> >
> > OK for the trunk.
>
> Ok, thanks, I'll apply the patch today (with fixed ChangeLog entry).
This broke the c-c++-common/asan/pr59063-2.c test:
FAIL: c-c++-common/asan/pr59063-2.c -O1 (test for excess errors)
Excess errors:
/usr/bin/ld: cannot find -lstdc++
While it could be fixed by tweaking asan-dg.exp, thinking about this, the
1) variant is actually not a good idea, it will not work properly anyway
if you link one library with -fsanitize=undefined and another library
with -fsanitize=address, the right solution is to make the two libraries
coexist sanely, so I'd prefer 2) or if not exporting a variable, export
an accessor function to get the address of the variable (or whole block of
shared state in one object between the libraries).
Yes, it means trying to get something accepted upstream, but anything else
is an ugly hack.
Jakub
