[PATCH] UBSan on Windows

Aaron Ballman aaron at aaronballman.com
Fri Oct 24 10:38:59 PDT 2014


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\
>>
>> 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).

>
>>> * 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.

Thanks!

~Aaron
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ubsan_winv2.patch
Type: application/octet-stream
Size: 2365 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141024/23939dbd/attachment.obj>


More information about the llvm-commits mailing list