Bug 131508 - win64 build fix
Summary: win64 build fix
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-10 14:16 PDT by Alex Christensen
Modified: 2014-04-14 07:20 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.35 KB, patch)
2014-04-10 14:19 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2014-04-10 14:16:55 PDT
In 64-bit Windows, "unsigned" means a 32-bit unsigned integer, "size_t" means a 64-bit unsigned integer, and "unsigned long" means a 32-bit unsigned integer.  Visual Studio is having trouble deciding whether to use std::min<unsigned> or std::min<size_t> in X86Assembler.h.  Here's a simple fix.
Comment 1 Alex Christensen 2014-04-10 14:19:47 PDT
Created attachment 229080 [details]
Patch
Comment 2 Geoffrey Garen 2014-04-10 18:46:26 PDT
Comment on attachment 229080 [details]
Patch

r=me
Comment 3 WebKit Commit Bot 2014-04-10 19:18:03 PDT
Comment on attachment 229080 [details]
Patch

Clearing flags on attachment: 229080

Committed r167108: <http://trac.webkit.org/changeset/167108>
Comment 4 WebKit Commit Bot 2014-04-10 19:18:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2014-04-12 11:58:18 PDT
Comment on attachment 229080 [details]
Patch

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

> Source/JavaScriptCore/assembler/X86Assembler.h:2276
> -            unsigned nopSize = std::min(size, 15UL);
> +            unsigned nopSize = std::min<unsigned>(size, 15UL);

Once we add <unsigned>, then there is no point in having the UL there any more. The L is particularly silly, since it gives us an unsigned long constant for no good reason.

Also, this function won’t work for any sizes that are greater than the maximum unsigned. It probably should either take an argument of type unsigned or work properly for large values of type size_t. Taking a size_t, and malfunctioning, seems like a strange choice.
Comment 6 Alex Christensen 2014-04-12 18:37:07 PDT
(In reply to comment #5)
> Also, this function won’t work for any sizes that are greater than the maximum unsigned. It probably should either take an argument of type unsigned or work properly for large values of type size_t. Taking a size_t, and malfunctioning, seems like a strange choice.
I was shooting for a non-invasive fix, and I was assuming that size wouldn't get anywhere near ULONG_MAX.  Do you think it would be worth switching all the unsigned types to size_t?  I'm a big fan of using size_t unless a different type is needed, but I also don't work on JavaScriptCore much and don't want to muck up their code.
Comment 7 Alex Christensen 2014-04-14 07:20:27 PDT
You're right, Darin.  A fix is in https://bugs.webkit.org/show_bug.cgi?id=131615