[ASan] Make for-Windows RTL compileable using Clang++

Make ASan/RTL compileable using Clang++ on Windows

[timurrrr_at_google_com, 2012-05-21 13:56:12]

Hi Kostya,

Can you please review this small patch?

Please note it might not be compileable with the upstream trunk version but
works fine for me when compiled with a slightly patches Clang [the patches
pending review & cleanup].

--
Timur

[kcc1, 2012-05-21 14:01:32]

Looks good. 

On 2012/05/21 13:56:12, timurrrr_at_google_com wrote:
> Hi Kostya,
> 
> Can you please review this small patch?
> 
> Please note it might not be compileable with the upstream trunk version but
> works fine for me when compiled with a slightly patches Clang [the patches
> pending review & cleanup].

I assume it may not build on Windows, but still builds on Linux/Mac. 

> 
> --
> Timur

[anton_korobeynikov.info, 2012-05-21 14:03:56]

Hi Timur,

> -#ifdef _WIN32
> +#if defined(_WIN32) && !defined(__clang__)
>  #include <intrin.h>
>  #endif
I think it should check for _MSC_VER here instead. And in many other
places as well.

>  #if defined(_WIN32)
> +# if defined(__clang__)
> +typedef int              intptr_t;
> +typedef unsigned int     uintptr_t;
> +# endif
These should go from stddef.h. What's the problem with it?

> +#if !defined(_WIN32) || defined(__clang__)
So, on Win64 you will provide all this stuff regardless of compiler?
This looks plain wrong.

Same problems in other places.

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

[timurrrr_at_google_com, 2012-05-21 14:07:21]

Anton,

On Mon, May 21, 2012 at 6:03 PM, Anton Korobeynikov
<anton@korobeynikov.info> wrote:
>> -#ifdef _WIN32
>> +#if defined(_WIN32) && !defined(__clang__)
>>  #include <intrin.h>
>>  #endif
> I think it should check for _MSC_VER here instead. And in many other
> places as well.
Clang defines _MSC_VER on Windows:
$ clang++ -dM -E -xc empty | grep MSC
#define _MSC_EXTENSIONS 1
#define _MSC_VER 1300

>> +#if !defined(_WIN32) || defined(__clang__)
> So, on Win64 you will provide all this stuff regardless of compiler?
> This looks plain wrong.
on Win64 both _WIN32 and _WIN64 are defined.

[kcc1, 2012-05-21 14:07:45]

On Mon, May 21, 2012 at 6:03 PM, Anton Korobeynikov <anton@korobeynikov.info
> wrote:

> Hi Timur,
>
> > -#ifdef _WIN32
> > +#if defined(_WIN32) && !defined(__clang__)
> >  #include <intrin.h>
> >  #endif
> I think it should check for _MSC_VER here instead. And in many other
> places as well.
>
> >  #if defined(_WIN32)
> > +# if defined(__clang__)
> > +typedef int              intptr_t;
> > +typedef unsigned int     uintptr_t;
> > +# endif
> These should go from stddef.h. What's the problem with it?
>

It doesn't define what's needed. On windows these folks live somewhere
else.
Adding other system headers brings in too much stuff which makes further
porting efforts more complicated.
We've spend quite a bit of time geting rid of things like stdlib.h


>
> > +#if !defined(_WIN32) || defined(__clang__)
> So, on Win64 you will provide all this stuff regardless of compiler?
> This looks plain wrong.
>
> Same problems in other places.
>
> --
> With best regards, Anton Korobeynikov
> Faculty of Mathematics and Mechanics, Saint Petersburg State University
>

[timurrrr_at_google_com, 2012-05-21 14:27:25]

Committed as r157188.
If we eventually find out the approach is wrong, I'll fix it as a separate
patch.

[anton_korobeynikov.info, 2012-05-21 14:27:35]

Timur,

> On Mon, May 21, 2012 at 6:03 PM, Anton Korobeynikov
> <anton@korobeynikov.info> wrote:
>>> -#ifdef _WIN32
>>> +#if defined(_WIN32) && !defined(__clang__)
>>>  #include <intrin.h>
>>>  #endif
>> I think it should check for _MSC_VER here instead. And in many other
>> places as well.
> Clang defines _MSC_VER on Windows:
> $ clang++ -dM -E -xc empty | grep MSC
> #define _MSC_EXTENSIONS 1
> #define _MSC_VER 1300

Ok, it seems that the following scenarios are possible:

1. clang on non-windows (or gcc, this should not matter, in theory)
2. clang on windows with ms extensions enabled
3. clang on windows w/o ms extensions enabled
4. MS VC on windows

For me it seems you're trying to "unify" 4 and 2 in MS-way - with
confusing defines (_WIN32 defined in _WIN64 mode as well), etc. The 3
as it seems to me is not supported as well.
It's surely up to you, but maybe there is better way to untangle all
the mess? For example, LLVM itself has portability layer which seems
to contain bunch of stuff you're using in asan.
Maybe you can consider looking there and "steal" some design ideas
from there? Check e.g. Support/DataTypes.h, Support/MathExtras.h,
etc...

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

[timurrrr_at_google_com, 2012-05-21 14:29:39]

On Mon, May 21, 2012 at 6:27 PM, Anton Korobeynikov
<anton@korobeynikov.info> wrote:
> Timur,
>
>> On Mon, May 21, 2012 at 6:03 PM, Anton Korobeynikov
>> <anton@korobeynikov.info> wrote:
>>>> -#ifdef _WIN32
>>>> +#if defined(_WIN32) && !defined(__clang__)
>>>>  #include <intrin.h>
>>>>  #endif
>>> I think it should check for _MSC_VER here instead. And in many other
>>> places as well.
>> Clang defines _MSC_VER on Windows:
>> $ clang++ -dM -E -xc empty | grep MSC
>> #define _MSC_EXTENSIONS 1
>> #define _MSC_VER 1300
>
> Ok, it seems that the following scenarios are possible:
>
> 1. clang on non-windows (or gcc, this should not matter, in theory)
> 2. clang on windows with ms extensions enabled
> 3. clang on windows w/o ms extensions enabled
> 4. MS VC on windows
>
> For me it seems you're trying to "unify" 4 and 2 in MS-way - with
> confusing defines (_WIN32 defined in _WIN64 mode as well), etc. The 3
> as it seems to me is not supported as well.
Right.
We only want to support 2 and 4 for now.

> It's surely up to you, but maybe there is better way to untangle all
> the mess? For example, LLVM itself has portability layer which seems
> to contain bunch of stuff you're using in asan.
> Maybe you can consider looking there and "steal" some design ideas
> from there? Check e.g. Support/DataTypes.h, Support/MathExtras.h,
> etc...
>
> --
> With best regards, Anton Korobeynikov
> Faculty of Mathematics and Mechanics, Saint Petersburg State University