I don’t haver a specific opinion on size_t vs unsigned, but I have a related point:
If Vector uses unsigned, then it must RELEASE_ASSERT that it does not overflow unsigned when growing. Even if most normal content allocates < 4GB, exploit content surely will try to allocate > 4GB. Chris, maybe you can make this change in trunk, since trunk uses unsigned? Geoff > On Nov 19, 2014, at 1:50 PM, Alexey Proskuryakov <a...@webkit.org> wrote: > > > That makes good sense for internal implementation, do you think that class > API should also use a custom type, or should it use size_t? > > With Vector though, I don't know how we would differentiate code paths that > need large allocations from ones that don't. Nearly anything that is exposed > as a JS API or deals with external world can hit sizes over 4Gb. That's not > out of reach in most scenarios, not even for resources loaded from network. > > - Alexey > > > 19 нояб. 2014 г., в 13:19, Filip Pizlo <fpi...@apple.com > <mailto:fpi...@apple.com>> написал(а): > >> ArrayBuffers are very special because they are part of ECMAScript. >> >> We use unsigned for the length, because once upon a time, that would have >> been the right type; these days the right type would be a 53-bit integer. >> So, size_t would be wrong. I believe that ArrayBuffers should be changed to >> use a very special size type; probably it wouldn't even be a primitive type >> but a class that carefully checked that you never overflowed 53 bits. >> >> -Filip >> >> >>> On Nov 19, 2014, at 12:54 PM, Alexey Proskuryakov <a...@webkit.org >>> <mailto:a...@webkit.org>> wrote: >>> >>> >>> This is not exactly about Vector, but if one uses >>> FileReader.prototype.readAsArrayBuffer() on a large file, I think that it >>> overflows ArrayBuffer. WebKit actually crashes when uploading >>> multi-gigabyte files to YouTube, Google Drive and other similar services, >>> although I haven't checked whether it's because of ArrayBuffers, or because >>> of a use of int/unsigned in another code path. >>> >>> I think that we should use size_t everywhere except for perhaps a few key >>> places where memory impact is critical (and of course we need explicit >>> checks when casting down to an unsigned). Or perhaps the rule can be even >>> simpler, and unsigned may never be used for indices and sizes, period. >>> >>> - Alexey >>> >>> >>> 19 нояб. 2014 г., в 12:32, Filip Pizlo <fpi...@apple.com >>> <mailto:fpi...@apple.com>> написал(а): >>> >>>> Whatever we do, the clients of Vector should be consistent about what type >>>> they use. I'm actually fine with Vector internally using unsigned even if >>>> the API uses size_t, but right now we have lots of code that uses a mix of >>>> size_t and unsigned when indexing into Vectors. That's confusing. >>>> >>>> If I picked one type to use for all Vector indices, it would be unsigned >>>> rather than size_t. Vector being limited to unsigned doesn't imply 4GB >>>> unless you do Vector<char>. Usually we have Vectors of pointer-width >>>> things, which means 32GB on 64-bit systems (UINT_MAX * sizeof(void*)). >>>> Even in a world where we had more than 32GB of memory to use within a >>>> single web process, I would hope that we wouldn't use it all on a single >>>> Vector and that if we did, we would treat that one specially for a bunch >>>> of other sensible reasons (among them being that allocating a contiguous >>>> slab of virtual memory of that size is rather taxing). So, size_t would >>>> buy us almost nothing since if we had a vector grow large enough to need >>>> it, we would be having a bad time already. >>>> >>>> I wonder: are there cases that anyone remembers where we have tried to use >>>> Vector to store more than UINT_MAX things? Another interesting question >>>> is: What's the largest number of things that we store into any Vector? We >>>> could use such a metric to project how big Vectors might get in the future. >>>> >>>> -Filip >>>> >>>> >>>>> On Nov 19, 2014, at 12:20 PM, Chris Dumez <cdu...@apple.com >>>>> <mailto:cdu...@apple.com>> wrote: >>>>> >>>>> Hi all, >>>>> >>>>> I recently started updating the WTF::Vector API to use unsigned types >>>>> instead of size_t [1][2], because: >>>>> - WTF::Vector is already using unsigned type for size/capacity internally >>>>> to save memory on 64-bit, causing a mismatch between the API and the >>>>> internal representation [3] >>>>> - Some reviewers have asked me to use unsigned for loop counters >>>>> iterating over vectors (which looks unsafe as the Vector API, e.g. >>>>> size(), returns a size_t). >>>>> - I heard from Joseph that this type mismatch is forcing us (and other >>>>> projects using WTF) to disable some build time warnings >>>>> - The few people I talked to before making that change said we should do >>>>> it >>>>> >>>>> However, Alexey recently raised concerns about this change. it doesn't >>>>> "strike him as the right direction. 4Gb is not much, and we should have >>>>> more of WebKit work with the right data types, not less.”. >>>>> I did not initially realize that this change was controversial, but now >>>>> that it seems it is, I thought I would raise the question on webkit-dev >>>>> to see what people think about this. >>>>> >>>>> Kr, >>>>> -- >>>>> Chris Dumez - Apple Inc. >>>>> Cupertino, CA >>>>> >>>>> >>>>> [1] http://trac.webkit.org/changeset/176275 >>>>> <http://trac.webkit.org/changeset/176275> >>>>> [2] http://trac.webkit.org/changeset/176293 >>>>> <http://trac.webkit.org/changeset/176293> >>>>> [3] http://trac.webkit.org/changeset/148891 >>>>> <http://trac.webkit.org/changeset/148891> >>>>> _______________________________________________ >>>>> webkit-dev mailing list >>>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> >>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>>> >>>> _______________________________________________ >>>> webkit-dev mailing list >>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> >>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>> >>> >> > > > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev