[PATCH] UBSan on Windows

[Timur Iskhodzhanov]

2014-10-24 10:38 GMT-07:00 Aaron Ballman <aaron at aaronballman.com>:
> On Fri, Oct 24, 2014 at 1:32 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> 2014-10-24 10:26 GMT-07:00 Aaron Ballman <aaron at aaronballman.com>:
>>> On Fri, Oct 24, 2014 at 1:14 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>>>> Hi Aaron,
>>>>
>>>> See also http://reviews.llvm.org/D5368 – Amine has made some progress
>>>> on UBSan lately as well, but didn't quite finish it.
>>>>
>>>> Can you please send the review via phab?
>>>
>>> I prefer to avoid phab; it's never worked as part of my workflow. Is
>>> it required for compiler-rt patches?
>>
>> Not required, we just prefer it this way :)
>
> I may give it another shot then, as time progresses. :-)
>
>>
>>>> Comments:
>>>> * Your implementation of atomic_exchange doesn't seem to be atomic?
>>>
>>> The same is true of all the atomic_exchange functions, no? This one is
>>> simply a copy of the other two, with the registers corrected for a
>>> 32-bit value.
>>
>> Oh that's surprising the other implementations were done this way.
>> [I didn't see it 'cause there wasn't enough context in the patch]
>
> Ah!
>
>>
>>> We should probably be using the InterlockedExchange functions on
>>> Windows instead of hand-rolling our own, but I felt that was a
>>> separate change for another patch.
>>
>> Sounds reasonable, but please add a FIXME then.
>
> Added.
>
>> Dmitry, can you please take a look?
>>
>>>> * lib/ubsan/ubsan_value.h – please update the comment
>>>
>>> Can do.
>>>
>>>> * Does check-ubsan (?) work?
>>>
>>> I cannot get any of the check-*san tests to work within MSVC. There
>>> are significant CMake issues for this workflow, which I'm hoping to
>>> address as I have the time. For instance, I get a considerable number
>>> of link errors because the libraries are placed in:
>>>
>>> E:\llvm\2013\Release\lib\clang\3.6.0\lib\windows\Release\
>>>
>>> But the linker is given a path to find them at:
>>>
>>> E:\llvm\2013\Release\lib\clang\3.6.0\lib\windows\

btw, this is http://llvm.org/bugs/show_bug.cgi?id=20885

>>> When I attempt to run check-ubsan, I get:
>>>
>>> 59>  Building Custom Rule
>>> E:/llvm/llvm/projects/compiler-rt/test/ubsan/CMakeLists.txt
>>> 59>  CMake does not need to re-run because
>>> E:\llvm\2013\projects\compiler-rt\test\ubsan\CMakeFiles\generate.stamp
>>> is up-to-date.
>>> 59>  Running UndefinedBehaviorSanitizer tests
>>> 59>  lit.py: LitConfig.py:79: warning: Unable to find 'bash'.
>>> 59>  lit.py: lit.common.cfg:20: fatal: Can't find compiler on path
>>> 'E:/llvm/2013/$(Configuration)/bin/clang.exe'
>>> 59>C:\Program Files
>>> (x86)\MSBuild\Microsoft.Cpp\v4.0\V120\Microsoft.CppCommon.targets(170,5):
>>> error MSB6006: "cmd.exe" exited with code 2.
>>>
>>> I get this same error when attempting to run check-asan after manually
>>> fixing up the library paths.
>>
>> Can you try to use ninja-based build for the time being?
>
> I can, but have a strong preference for not using that. It certainly
> shouldn't be required (which is why I've spent a bunch of time
> recently trying to fix the CMake instead).

Sure, but it's better to fix one thing at a time rather than tie two
problems together :)

>>>> * Have you tried Win64?
>>>
>>> I have not, just x86.
>>
>> Please try and/or see Amine's patch – he has only enabled UBSan on 32-bit arch.
>
> Corrected in the attached patch, along with some other nits.

I'll give it a try today.

> Thanks!
>
> ~Aaron