RFR [10] 8189800: Add support for AddressSanitizer

[Magnus Ihse Bursie]

On 2017-10-30 10:31, Artem Smotrakov wrote:
> Hi Magnus,
>
> The current approach uses AddressSanitizer as a shared library 
> (libasan.so) which is part of GCC/Clang toolkit. In case you use 
> system toolkit, then libasan.so is available for linker and at 
> runtime. But if you set a custom toolkit by --with-devkit option, then 
> libasan.so form this toolkit may not be available for linker and at 
> runtime by default. As a result, you can get errors while linking and 
> running. To fix that, you normally need to make it available using 
> ldconfig, or update LD_LIBRARY_PATH. That's why it updates 
> LD_LIBRARY_PATH with DEVKIT_LIB_DIR if a custom toolkit was used. That 
> may be helpful when you build JDK in environment like jib/jprt.
>
> I tried to remove exporting ASAN_ENABLED and DEVKIT_LIB_DIR, and as a 
> result, ASAN_OPTIONS and DEVKIT_LIB_DIR didn't go to jtreg command 
> which caused tests to fail when you run "make test". If we don't 
> export ASAN_OPTIONS and DEVKIT_LIB_DIR, then the updates in 
> TestCommon.gmk don't make much sense to me because those variables 
> have to be explicitly set for "make" anyway.
>
> I can remove exporting those variables and revert TestCommon.gmk. 
> Although, it looks nicer to me if we can run the tests just with "make 
> test" without specifying ASAN_OPTIONS and DEVKIT_LIB_DIR explicitly.
>
> What do you think?
Ah, I see. TestCommon.gmk is not properly integrated into the rest of 
the build system. I'm still a bit surprised at this behavior, but I 
accept your explanation.

Keep it as it is. TestCommon is due to be removed by the new 
RunTests.gmk (which is properly integrated), and when that happens, we 
can remove the exports then.

/Magnus

>
> Artem
>
>
> On 10/30/2017 10:50 AM, Magnus Ihse Bursie wrote:
>> On 2017-10-30 08:39, Artem Smotrakov wrote:
>>> cc'ing hotspot-dev at openjdk.java.net as David suggested.
>>>
>>> Artem
>>>
>>>
>>> On 10/27/2017 11:02 PM, Artem Smotrakov wrote:
>>>> Hello,
>>>>
>>>> Please review the following patch which adds support for 
>>>> AddressSanitizer.
>>>>
>>>> AddressSanitizer is a runtime memory error detector which looks for 
>>>> various memory corruption issues and leaks.
>>>>
>>>> Please refer to [1] for details. AddressSanitizer is available in 
>>>> gcc 4.8+ and clang 3.1+
>>>>
>>>> The patch below introduces --enable-asan parameter for the 
>>>> configure script which enables AddressSanitizer.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189800
>>>> Webrev: http://cr.openjdk.java.net/~asmotrak/8189800/webrev.00/
>> spec.gmk.in should only have export for variables that needs to be 
>> exported in the environment for executing binaries, that is 
>> ASAN_OPTIONS and LD_LIBRARY_PATH, not ASAN_ENABLED or DEVKIT_LIB_DIR.
>>
>> I'm also a bit curious about the addition of of DEVKIT_LIB_DIR. Would 
>> you care to elaborate your thinking?
>>
>> Otherwise it looks good.
>>
>> /Magnus
>>
>>>>
>>>> [1] https://github.com/google/sanitizers/wiki/AddressSanitizer
>>>>
>>>> Artem
>>>
>>
>