Bug 46713 - JSC compile fails on 32bit platform when Regexp Tracing is enabled
Summary: JSC compile fails on 32bit platform when Regexp Tracing is enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-28 03:56 PDT by Peter Varga
Modified: 2010-09-29 05:46 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch (1.53 KB, patch)
2010-09-28 03:58 PDT, Peter Varga
ossy: review-
ossy: commit-queue-
Details | Formatted Diff | Diff
proposed patch v2 (1.56 KB, patch)
2010-09-29 05:23 PDT, Peter Varga
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Varga 2010-09-28 03:56:10 PDT
In case of REGEXP_TRACING define is enabled I got the following error during the compile on 32bit platform since -Werror flag is added:

cc1plus: warnings being treated as errors
../../../JavaScriptCore/runtime/RegExp.cpp: In member function 'void JSC::RegExp::printTraceData()':
../../../JavaScriptCore/runtime/RegExp.cpp:257: error: format '%014lx' expects type 'long unsigned int', but argument 3 has type 'uintptr_t'
Comment 1 Peter Varga 2010-09-28 03:58:36 PDT
Created attachment 69036 [details]
proposed patch
Comment 2 Michael Saboff 2010-09-28 10:55:39 PDT
Thanks for looking into this.

I have some concerns with %p is that it doesn't include leading zeros and pretty much ignores precision.  Did you consider changing the cast to (unsigned long)?
Comment 3 Csaba Osztrogonác 2010-09-29 05:07:10 PDT
Comment on attachment 69036 [details]
proposed patch

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

%lx expects "unsigned long int", but uintptr_t is
"unsigned long int" on 64 bit and "unsigned int" on 32 bit

sizeof(uintptr_t) == sizeof(unsigned long int) always,
but unfortunately unsigned int != unsigned long int. :(

I propose to cast unsigned long int with reinterpret_cast and use %014lx.
I prefer reinterpret_cast instead of obsolete C style cast



On 64 bit machine:
-------------------
sizeof(int) = 4
sizeof(long int) = 8
sizeof(long long int) = 8
sizeof(uintptr_t) = 8

sizeof(uintptr_t) == sizeof(unsigned long int) == 8
typedef unsigned long int uintptr_t;

On 32 bit machine:
-------------------
sizeof(int) = 4
sizeof(long int) = 4
sizeof(long long int) = 8
sizeof(uintptr_t) = 4

sizeof(uintptr_t) == sizeof(unsigned long int) == 4
typedef unsigned int uintptr_t;

> JavaScriptCore/runtime/RegExp.cpp:257
> -            sprintf(jitAddr, "0x%014lx", (uintptr_t)codeBlock.getAddr());
> +            sprintf(jitAddr, "%16p", codeBlock.getAddr());

sprintf(jitAddr, "0x%014lx", reinterpret_cast<unsigned long int>(codeBlock.getAddr()));
Comment 4 Peter Varga 2010-09-29 05:23:06 PDT
Created attachment 69180 [details]
proposed patch v2
Comment 5 WebKit Commit Bot 2010-09-29 05:46:03 PDT
Comment on attachment 69180 [details]
proposed patch v2

Clearing flags on attachment: 69180

Committed r68639: <http://trac.webkit.org/changeset/68639>
Comment 6 WebKit Commit Bot 2010-09-29 05:46:09 PDT
All reviewed patches have been landed.  Closing bug.