Note: There are a few cases of duplicates in user autocompletion which are being worked on.

MapsMemoryReporter asserts with device #s > 100

RESOLVED FIXED in mozilla15

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla15
All
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 628025 [details] [diff] [review]
patch to handle 3-digit device numbers

My xpcshell test runs are failing locally with the error:

###!!! ASSERTION: Didn't create a vsize node?: 'categoriesSeen.mSeenVsize', file /home/froydnj/src/m-c.git/xpcom/base/MapsMemoryReporter.cpp, line 237

Poking around with GDB reveals that /proc/self/smaps has an unusual header:

00400000-0044e000 r-xp 00000000 103:00 27190725 <other stuff that FILE's members didn't have>

Note the 3-digit major device number, which MapsMemoryReporter is unprepared to handle.

Apparently it's not terribly uncommon:

@nightcrawler:/opt/build/m-c-git-build$ egrep -c '[0-9]{3}:' /proc/*/maps 2>/dev/null | cut -f 2 -d ':' | addup
111

out of some 4k+ map entries.  This an Ubuntu 11.04 box.

The attached patch appears to solve the problem (only gets xpcshell running properly, haven't looked at about:memory or anything).  The *scanf %[ construct scans a range of characters until the field width is hit or a character not in the range is found.  I chose to keep a field width for safety's sake; perhaps there are four-digit device numbers lurking somewhere...
Attachment #628025 - Flags: review?(n.nethercote)
Comment on attachment 628025 [details] [diff] [review]
patch to handle 3-digit device numbers

r=me.

Do we have any reason to believe 3 digits is the right length?  2 digits was a complete guess...
Attachment #628025 - Flags: review?(n.nethercote) → review+
Wait!  The numbers are hex, not decimal.  From the example in "man 5 proc":

    08048000-08056000 r-xp 00000000 03:0c 64593   /usr/sbin/gpm

Please fix that!  And the example uses lower-case letters but I'd use [0-9A-Fa-f] as the pattern just to be safe.

I looked at Valgrind's code for parsing these files.  It just looks for an arbitrary length hex number.  How about we allow 4 or even 5 digits, just in case?
(In reply to Nicholas Nethercote [:njn] from comment #2)
> Wait!  The numbers are hex, not decimal.  From the example in "man 5 proc":

Aha!  That squares with what I see; the major device number of my raid array is 259.

> Please fix that!  And the example uses lower-case letters but I'd use
> [0-9A-Fa-f] as the pattern just to be safe.

Will do.

> I looked at Valgrind's code for parsing these files.  It just looks for an
> arbitrary length hex number.  How about we allow 4 or even 5 digits, just in
> case?

_Linux Device Drivers_ indicates that the 2.6 kernel allocates 12 bits to the major device number and 20 bits to the minor device number, so I'll use that as a guideline for field widths.
> Wait!

I guess I shouldn't review patches during MemShrink meetings, huh?
Attachment #628025 - Flags: review+ → review-
Created attachment 628420 [details] [diff] [review]
revised patch

New patch!
Attachment #628025 - Attachment is obsolete: true
Attachment #628420 - Flags: review?(justin.lebar+bug)
Comment on attachment 628420 [details] [diff] [review]
revised patch

I'd be OK if you wanted to make these |char dev[32]| just so we don't have to worry about relying on an internal kernel data structure -- we don't even use the dev major/minor values for anything in about:memory.  But I'm happy to do this too.
Attachment #628420 - Flags: review?(justin.lebar+bug) → review+
(In reply to Justin Lebar [:jlebar] from comment #6)
> I'd be OK if you wanted to make these |char dev[32]| just so we don't have
> to worry about relying on an internal kernel data structure -- we don't even
> use the dev major/minor values for anything in about:memory.  But I'm happy
> to do this too.

More conservative values would be good.  I'll just use |char dev[17]|; 64 bits should be enough for anybody.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2ba0a2f7c14
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/e2ba0a2f7c14
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.