Last Comment Bug 442012 - Allocating more than 4GB using nsMemory::Alloc(size_t) succeeds but less memory is allocated
: Allocating more than 4GB using nsMemory::Alloc(size_t) succeeds but less memo...
Status: RESOLVED FIXED
[sg:low]
: fixed1.8.1.21, fixed1.9.0.6, fixed1.9.1
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Benjamin Smedberg [:bsmedberg]
:
: Nathan Froyd [:froydnj]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-26 05:31 PDT by Michal Novotny (:michal)
Modified: 2009-09-02 22:39 PDT (History)
6 users (show)
samuel.sidler+old: wanted1.9.0.x+
dveditz: blocking1.8.1.next+
asac: wanted1.8.1.x+
asac: blocking1.8.0.next+
asac: wanted1.8.0.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Prevent allocations >2GB, rev. 1 (612 bytes, patch)
2008-10-30 12:28 PDT, Benjamin Smedberg [:bsmedberg]
wtc: review+
Details | Diff | Splinter Review
Prevent allocations >2GB, rev. 1.1 (879 bytes, patch)
2008-11-06 12:42 PST, Benjamin Smedberg [:bsmedberg]
benjamin: review+
wtc: review+
dveditz: superreview+
mbeltzner: approval1.9.1+
dveditz: approval1.9.0.6+
dveditz: approval1.8.1.next+
asac: approval1.8.0.next+
Details | Diff | Splinter Review
1.8.0 version (777 bytes, patch)
2009-01-27 04:44 PST, Martin Stránský
no flags Details | Diff | Splinter Review

Description Michal Novotny (:michal) 2008-06-26 05:31:39 PDT
Type size_t is unsigned long int on 64-bit OS. So it is possible to ask for more than 4GB of memory using nsMemory::Alloc(size_t). But nsMemory::Alloc(size_t) calls NS_Alloc(PRSize) which calls PR_Malloc(PRUint32) where the most significant half of the parameter is thrown away. E.g. allocating 5GB of memory will succeed but only 1GB will be allocated.
Comment 1 Daniel Veditz [:dveditz] 2008-10-10 18:52:52 PDT
Benjamin: assigning to you to figure out what approach we take. Change the nsMemory::Alloc() signature, or change the backend implementation to work correctly on 64-bit systems?

Surely NSPR has run into this problem, what do they do on 64-bit systems?
Comment 2 Wan-Teh Chang 2008-10-10 19:21:39 PDT
The NSPR bug for this issue is bug 445295.

You can change NS_Alloc(PRSize) to call malloc(size_t) instead
of PR_Malloc(PRUint32).  Remember to call calloc and free instead
of PR_Calloc and PR_Free in NS_Realloc and NS_Free.
Comment 3 Wan-Teh Chang 2008-10-10 19:23:44 PDT
That should have been "Remember to call realloc and free instead
of PR_Realloc and PR_Free in NS_Realloc and NS_Free."
Comment 4 Benjamin Smedberg [:bsmedberg] 2008-10-29 13:39:19 PDT
What size is PRSize on 64-bit systems?
Comment 5 Michal Novotny (:michal) 2008-10-29 15:17:12 PDT
(In reply to comment #4)
> What size is PRSize on 64-bit systems?

(gdb) whatis PRSize
type = size_t
(gdb) whatis size_t
type = long unsigned int
(gdb) p sizeof(PRSize)
$1 = 8
Comment 6 Benjamin Smedberg [:bsmedberg] 2008-10-30 12:28:10 PDT
Created attachment 345539 [details] [diff] [review]
Prevent allocations >2GB, rev. 1

Let's just bail on allocations >2GB... I can't think of any reason they should be allowed.
Comment 7 Wan-Teh Chang 2008-10-30 13:26:23 PDT
Comment on attachment 345539 [details] [diff] [review]
Prevent allocations >2GB, rev. 1

Don't forget to patch NS_Realloc, too.
Comment 8 Benjamin Smedberg [:bsmedberg] 2008-11-06 12:42:27 PST
Created attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1
Comment 9 Wan-Teh Chang 2008-11-06 13:00:50 PST
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1

r=wtc.
Comment 10 Daniel Veditz [:dveditz] 2008-11-12 12:51:18 PST
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1

sr=dveditz
Comment 11 Benjamin Smedberg [:bsmedberg] 2008-12-02 12:29:57 PST
http://hg.mozilla.org/mozilla-central/rev/9f3807b5e936
Comment 12 Benjamin Smedberg [:bsmedberg] 2008-12-02 12:30:41 PST
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1

Assuming the perf metrics aren't affected, this should be very safe.
Comment 13 Mike Beltzner [:beltzner, not reading bugmail] 2008-12-09 08:41:58 PST
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1

a191=beltzner, didn't see any perf impact after landing.
Comment 14 Benjamin Smedberg [:bsmedberg] 2008-12-11 09:10:42 PST
Fixed on 1.9.1, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5b26e181c6d5
Comment 15 Daniel Veditz [:dveditz] 2008-12-22 12:02:54 PST
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1

Approved for 1.9.0.6, a=dveditz for release-drivers.
Comment 16 Benjamin Smedberg [:bsmedberg] 2009-01-05 13:08:53 PST
Landed on CVS HEAD for 1.9.0.6
Comment 17 Al Billings [:abillings] 2009-01-06 10:59:16 PST
Is there anything that really needs to be done to verified that this is fixed in 1.9.0.6?
Comment 18 Benjamin Smedberg [:bsmedberg] 2009-01-06 11:06:52 PST
I don't think you could verify it with a 32-bit OS. Since there's no testcase here, I think it can safely be left alone.
Comment 19 Alexander Sack 2009-01-12 02:48:41 PST
seems we want it on 1.8 too.
Comment 20 Daniel Veditz [:dveditz] 2009-01-16 11:33:34 PST
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1

Approved for 1.8.1.21, a=dveditz for release-drivers.
Comment 21 Alexander Sack 2009-01-22 05:26:42 PST
fixed for 1.8.1.21

Checking in xpcom/base/nsMemoryImpl.cpp;
/cvsroot/mozilla/xpcom/base/nsMemoryImpl.cpp,v  <--  nsMemoryImpl.cpp
new revision: 1.29.4.2; previous revision: 1.29.4.1
Comment 22 Martin Stránský 2009-01-27 04:44:54 PST
Created attachment 359027 [details] [diff] [review]
1.8.0 version
Comment 23 Alexander Sack 2009-02-09 07:13:45 PST
Comment on attachment 346730 [details] [diff] [review]
Prevent allocations >2GB, rev. 1.1

a=asac for 1.8.0

Note You need to log in before you can comment on or make changes to this bug.