This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][PR sanitizer/84250] Avoid global symbols collision when using both ASan and UBSan


Hi Jeff,

On 07/04/2018 05:45 AM, Jeff Law wrote:
> On 05/23/2018 11:15 AM, Maxim Ostapenko wrote:
>> Hi,
>>
>>
>> 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.ostapenko@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.ostapenko@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).

-Maxim

> jeff
>
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]