Bug 175994 – Define *_GIGACAGE_MASK when Gigacage is not supported

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Don Olmstead]

After https://trac.webkit.org/changeset/221148/webkit the WinCairo bots report the following errors

error C2065: 'JSVALUE_GIGACAGE_MASK': undeclared identifier
error C2065: 'PRIMITIVE_GIGACAGE_MASK': undeclared identifier

[Don Olmstead]

Created attachment 319095 [details]
Patch

Fix suggested by mlam on IRC. Built WinCairo successfully.

[Filip Pizlo]

Comment on attachment 319095 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319095&action=review

> Source/WTF/wtf/Gigacage.h:33
> +#define PRIMITIVE_GIGACAGE_MASK UINTPTR_MAX
> +#define JSVALUE_GIGACAGE_MASK UINTPTR_MAX

Why?

[Mark Lam]

Comment on attachment 319095 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319095&action=review

>> Source/WTF/wtf/Gigacage.h:33
>> +#define JSVALUE_GIGACAGE_MASK UINTPTR_MAX
> 
> Why?

The wincairo build which does not use bmalloc is failing to build.  Is this the wrong fix for when bmalloc is not used?

[Filip Pizlo]

(In reply to Mark Lam from comment #3)
> Comment on attachment 319095 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319095&action=review
> 
> >> Source/WTF/wtf/Gigacage.h:33
> >> +#define JSVALUE_GIGACAGE_MASK UINTPTR_MAX
> > 
> > Why?
> 
> The wincairo build which does not use bmalloc is failing to build.  Is this
> the wrong fix for when bmalloc is not used?

I see.  OK.

[Filip Pizlo]

Comment on attachment 319095 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319095&action=review

>>>> Source/WTF/wtf/Gigacage.h:33
>>>> +#define JSVALUE_GIGACAGE_MASK UINTPTR_MAX
>>> 
>>> Why?
>> 
>> The wincairo build which does not use bmalloc is failing to build.  Is this the wrong fix for when bmalloc is not used?
> 
> I see.  OK.

Please set these to 0, not UINTPTR_MAX.

[Mark Lam]

Comment on attachment 319095 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319095&action=review

>>>>> Source/WTF/wtf/Gigacage.h:33
>>>>> +#define JSVALUE_GIGACAGE_MASK UINTPTR_MAX
>>>> 
>>>> Why?
>>> 
>>> The wincairo build which does not use bmalloc is failing to build.  Is this the wrong fix for when bmalloc is not used?
>> 
>> I see.  OK.
> 
> Please set these to 0, not UINTPTR_MAX.

At first, I thought that the llint code is unconditionally applying the masm.  But I see that the llint code only applies the mask if GIGACAGE_ENABLED.  Hence, setting these to 0 makes sense.

[Don Olmstead]

Created attachment 319096 [details]
Patch

Setting masks to 0

[Mark Lam]

Comment on attachment 319096 [details]
Patch

r=me if EWS is green and if this patch makes the code build on wincairo (which was where this issue was detected) and tests passes there as well.

[WebKit Commit Bot]

Comment on attachment 319096 [details]
Patch

Clearing flags on attachment: 319096

Committed r221211: <http://trac.webkit.org/changeset/221211>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34091627>