Closed Bug 985566 Opened 6 years ago Closed 4 years ago

add some gdb pretty printers to .gdbinit

Categories

(Firefox Build System :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(1 file, 3 obsolete files)

custom commands are annoying and we can do better
Attachment #8393638 - Flags: review?(mh+mozilla)
Comment on attachment 8393638 [details] [diff] [review]
add some pretty printers to .gdbinit

Review of attachment 8393638 [details] [diff] [review]:
-----------------------------------------------------------------

::: .gdbinit
@@ +181,5 @@
>  end
> +
> +python
> +import gdb
> +import gdb.printing

Ideally, we should put this python code in a python module. I don't know how hard it would be to find its location.

@@ +189,5 @@
> +        self.value = value['mRawPtr']
> +
> +    def to_string(self):
> +        if not self.value:
> +            type_name = str(self.value.type)

I don't know what the type is for self.value, but this sure looks weird to the eye, even if that actually works.

@@ +196,5 @@
> +
> +        return '[(%s) %s]' % (type_name, str(self.value))
> +
> +pp = gdb.printing.RegexpCollectionPrettyPrinter('GeckoPrettyPrinters')
> +pp.add_printer('nsAutoPtr', 'nsAutoPtr<\w+>', smartptr_printer)

It /might/ be better to prepend the regexp with ^ and append $. I haven't looked at what the RegExpCollectionPrettyPrinter itself does, but the examples in gdb documentation do use ^$.

@@ +202,5 @@
> +pp.add_printer('nsRefPtr', 'nsRefPtr<\w+>', smartptr_printer)
> +pp.add_printer('nsWeakPtr', 'nsWeakPtr<\w+>', smartptr_printer)
> +
> +class string_printer(object):
> +    def __init__(self, v):

value instead of v?

@@ +206,5 @@
> +    def __init__(self, v):
> +        self.value = v
> +
> +    def to_string(self):
> +        string = str(self.value['mData'])

Does that actually work properly for all types of strings? (I'm especially thinking about the UTF-16 types)

@@ +221,5 @@
> +        if flags & (1 << 4):
> +            attrs += ' fixed'
> +        if flags & (1 << 5):
> +            attrs += ' literal'
> +        return '[%s%s]' % (string, attrs)

You should probably add a display_hint method returning 'string', and just return the string here. A separate helper function would probably be better to know the flag values.

@@ +226,5 @@
> +
> +pp.add_printer('nsString', 'ns.*String$', string_printer)
> +
> +class tarray_printer(object):
> +    def __init__(self, v):

value instead of v?

@@ +237,5 @@
> +        elements = data.cast(self.elem_type.pointer())
> +        children = list()
> +        for i in xrange(0, length):
> +            element = (str(i), (elements + i).dereference())
> +            children.append(element)

children = [(str(i), (elements + i).dereference()) for i in range(0, length)]

@@ +242,5 @@
> +
> +        return children
> +
> +    def to_string(self):
> +        return self.value.type.name

As I was curious what this returns, i tested just this class... and that throws:
Python Exception <type 'exceptions.AttributeError'> 'gdb.Type' object has no attribute 'name'
which, in itself is weird, because that's supposed to exist according to https://sourceware.org/gdb/onlinedocs/gdb/Types-In-Python.html#Types-In-Python (self.value.type.sizeof does return a value, and hasattr(self.value.type, 'name') returns false...)

@@ +250,5 @@
> +
> +pp.add_printer('TArray', '.*TArray<.*>$', tarray_printer)
> +
> +class owning_thread_printer(object):
> +    def __init__(self, v):

value instead of v?

@@ +257,5 @@
> +    def to_string(self):
> +        prthread_type = gdb.lookup_type('PRThread').pointer()
> +        prthread = self.value['mThread'].cast(prthread_type)
> +        name = prthread['name']
> +        return name if name else '(PRThread *) %s' % prthread

What does that print in the else case?

@@ +263,5 @@
> +pp.add_printer('nsAutoOwningThread', '^nsAutoOwningThread$',
> +               owning_thread_printer)
> +
> +class ccrefcnt_printer(object):
> +    def __init__(self, v):

value instead of v?

@@ +272,5 @@
> +        is_in_purple_buf = bool(bits & (1 << 0))
> +        is_purple = bool(bits & (1 << 1))
> +        refs = bits >> 2
> +        return '[refs=%d purple=%s InPurpleBuf=%s]' % (refs, is_purple,
> +                                                       is_in_purple_buf)

I wonder if we wouldn't be better served by bit fields in the code, here.
Attachment #8393638 - Flags: review?(mh+mozilla) → feedback+
> ::: .gdbinit
> @@ +181,5 @@
> >  end
> > +
> > +python
> > +import gdb
> > +import gdb.printing
> 
> Ideally, we should put this python code in a python module. I don't know how
> hard it would be to find its location.

I was thinking this would be tricky, but maybe not, we could just put a .gdb_pythonpath file in $topsrcdir pointing at the python module, and generate ones to go in the objdir.

(file name open to bikesheding)

> @@ +189,5 @@
> > +        self.value = value['mRawPtr']
> > +
> > +    def to_string(self):
> > +        if not self.value:
> > +            type_name = str(self.value.type)
> 
> I don't know what the type is for self.value, but this sure looks weird to
> the eye, even if that actually works.

I'm pretty sure its gdb.value, but I'm not sure why I shouldn't just do self.value.type.name / tag modulo the weird python exception you get.

> @@ +196,5 @@
> > +
> > +        return '[(%s) %s]' % (type_name, str(self.value))
> > +
> > +pp = gdb.printing.RegexpCollectionPrettyPrinter('GeckoPrettyPrinters')
> > +pp.add_printer('nsAutoPtr', 'nsAutoPtr<\w+>', smartptr_printer)
> 
> It /might/ be better to prepend the regexp with ^ and append $. I haven't
> looked at what the RegExpCollectionPrettyPrinter itself does, but the
> examples in gdb documentation do use ^$.

probably can't hurt to be as precise as possible.

> @@ +206,5 @@
> > +    def __init__(self, v):
> > +        self.value = v
> > +
> > +    def to_string(self):
> > +        string = str(self.value['mData'])
> 
> Does that actually work properly for all types of strings? (I'm especially
> thinking about the UTF-16 types)

I think I tested this and it worked with 16 bit types I think because we use char16-t know 

> @@ +221,5 @@
> > +        if flags & (1 << 4):
> > +            attrs += ' fixed'
> > +        if flags & (1 << 5):
> > +            attrs += ' literal'
> > +        return '[%s%s]' % (string, attrs)
> 
> You should probably add a display_hint method returning 'string', and just
> return the string here. A separate helper function would probably be better
> to know the flag values.

I'm not really sure which is better, but just printing the string doesn't seem too crazy.

> @@ +242,5 @@
> > +
> > +        return children
> > +
> > +    def to_string(self):
> > +        return self.value.type.name
> 
> As I was curious what this returns, i tested just this class... and that
> throws:
> Python Exception <type 'exceptions.AttributeError'> 'gdb.Type' object has no
> attribute 'name'
> which, in itself is weird, because that's supposed to exist according to
> https://sourceware.org/gdb/onlinedocs/gdb/Types-In-Python.html#Types-In-
> Python (self.value.type.sizeof does return a value, and
> hasattr(self.value.type, 'name') returns false...)

huh! I thought I tested this and it dtrt (not sure what version of gdb that was with atm though)

> @@ +257,5 @@
> > +    def to_string(self):
> > +        prthread_type = gdb.lookup_type('PRThread').pointer()
> > +        prthread = self.value['mThread'].cast(prthread_type)
> > +        name = prthread['name']
> > +        return name if name else '(PRThread *) %s' % prthread
> 
> What does that print in the else case?

something like PRThread *0xfffff saddly the main thread hits the else case iirc, so trying to something else might be a good idea.

> @@ +272,5 @@
> > +        is_in_purple_buf = bool(bits & (1 << 0))
> > +        is_purple = bool(bits & (1 << 1))
> > +        refs = bits >> 2
> > +        return '[refs=%d purple=%s InPurpleBuf=%s]' % (refs, is_purple,
> > +                                                       is_in_purple_buf)
> 
> I wonder if we wouldn't be better served by bit fields in the code, here.


I guess it makes getting the purple buf pointer a bit gross, but its probablyy an overall win.
> > @@ +272,5 @@
> > > +        is_in_purple_buf = bool(bits & (1 << 0))
> > > +        is_purple = bool(bits & (1 << 1))
> > > +        refs = bits >> 2
> > > +        return '[refs=%d purple=%s InPurpleBuf=%s]' % (refs, is_purple,
> > > +                                                       is_in_purple_buf)
> > 
> > I wonder if we wouldn't be better served by bit fields in the code, here.
> 
> 
> I guess it makes getting the purple buf pointer a bit gross, but its
> probablyy an overall win.

actually smaug claims it is the way it is because bit fields result in worse code generation and that matters here.
Attachment #8393638 - Attachment is obsolete: true
Attachment #8400349 - Flags: review?(mh+mozilla)
Comment on attachment 8400349 [details] [diff] [review]
add some pretty printers to .gdbinit

Review of attachment 8400349 [details] [diff] [review]:
-----------------------------------------------------------------

I /think/ it would make sense to have the relevant module owners/peers take a look at the kind of output for the various types concerned, and give some insight as to what they think would be nice to show for them (or if the proposed output is good for them).

I essentially have nits on the helper intrastructure itself. The rest seems fine on a cursory look.

::: .gdbinit_python
@@ +1,3 @@
> +python
> +import sys
> +sys.path += ['python/mozgdb/']

sys.path.append('...')

@@ +1,4 @@
> +python
> +import sys
> +sys.path += ['python/mozgdb/']
> +import mozgdb

moz-gdb is the name of a specific gdb wrapper (along the corresponding cross-gdb) we have for fennec. I'd rather avoid that particular name.

::: build/Makefile.in
@@ +128,5 @@
>  
> +# tell the .gdbinits in the objdir and dist/bin where their python code is
> +libs::
> +	echo "python import sys; sys.path += ['$(topsrcdir)/python/mozgdb/']; import mozgdb;" > $(DEPTH)/.gdbinit_python
> +	echo "python import sys; sys.path += ['$(topsrcdir)/python/mozgdb/']; import mozgdb;" > $(DEPTH)/dist/bin/.gdbinit_python

I think it would be better to have a .in file and preprocess it.

::: python/mozgdb/mozgdb/__init__.py
@@ +1,4 @@
> +import gdb
> +import gdb.printing
> +
> +import ccrefcnt

import .ccrefcnt

@@ +1,5 @@
> +import gdb
> +import gdb.printing
> +
> +import ccrefcnt
> +import mozgdb.owningthread

import .owningthread
etc.

@@ +11,5 @@
> +ccrefcnt.init(pp)
> +owningthread.init(pp)
> +smartptr.init(pp)
> +string.init(pp)
> +tarray.init(pp)

I think it would be nicer to use a decorator on each of the classes instead.

Something like:

class GeckoPrettyPrinter(object):
    pp = gdb.printing.RegexpCollectionPrettyPrinter('GeckoPrettyPrinters')

    def __init__(self, name, regexp):
        self.name = name
        self.regexp = regexp

    def __call__(self, wrapped):
        GeckoPrettyPrinter.pp.add_printer(self.name, self.regexp, wrapped)
        return wrapped

import .ccrefcnt
(...)

gdb.printing.register_pretty_printer(None, GeckoPrettyPrinter.pp)

and in ccrefcnt.py:

@GeckoPrettyPrinter('nsCycleCollectingAutoRefCnt', '^nsCycleCollectingAutoRefCnt$')
class ccrefcnt_printer(object):

etc.

If several need to be registered, you can just use @GeckoPrettyPrinter several times.

::: python/mozgdb/mozgdb/owningthread.py
@@ +10,5 @@
> +        name = prthread['name']
> +
> +        # if the thread doesn't have a name try to get its thread id (might not
> +        # work on !posix)
> +        name = prthread['tid']

s/!posix/!linux/; s/might/does/. What does this do when the field doesn't exist? does it return None, throws an exception, other?

@@ +12,5 @@
> +        # if the thread doesn't have a name try to get its thread id (might not
> +        # work on !posix)
> +        name = prthread['tid']
> +
> +        return name if name else '(PRThread *) %s' % prthread

/might/ be better to make some verbose printing in the case where name does something like "[Thread tid=%d]" for the tid case or '[Thread "%s"]' for an actual named string.
Attachment #8400349 - Flags: review?(mh+mozilla) → feedback+
ok, coming back to this because I've poked at a TArray manually one too many times.

> I /think/ it would make sense to have the relevant module owners/peers take
> a look at the kind of output for the various types concerned, and give some
> insight as to what they think would be nice to show for them (or if the
> proposed output is good for them).

yeah, makes sense.

> @@ +1,4 @@
> > +python
> > +import sys
> > +sys.path += ['python/mozgdb/']
> > +import mozgdb
> 
> moz-gdb is the name of a specific gdb wrapper (along the corresponding
> cross-gdb) we have for fennec. I'd rather avoid that particular name.

huh, ok.  other ideas?  I guess we have lldbutils, but that isn't really clear.  gdbpp/ mozgdbpp/ ?

> ::: build/Makefile.in
> @@ +128,5 @@
> >  
> > +# tell the .gdbinits in the objdir and dist/bin where their python code is
> > +libs::
> > +	echo "python import sys; sys.path += ['$(topsrcdir)/python/mozgdb/']; import mozgdb;" > $(DEPTH)/.gdbinit_python
> > +	echo "python import sys; sys.path += ['$(topsrcdir)/python/mozgdb/']; import mozgdb;" > $(DEPTH)/dist/bin/.gdbinit_python
> 
> I think it would be better to have a .in file and preprocess it.

sounds like more work, but ok

> ::: python/mozgdb/mozgdb/owningthread.py
> @@ +10,5 @@
> > +        name = prthread['name']
> > +
> > +        # if the thread doesn't have a name try to get its thread id (might not
> > +        # work on !posix)
> > +        name = prthread['tid']
> 
> s/!posix/!linux/; s/might/does/. What does this do 

err, yeah

when the field doesn't
> exist? does it return None, throws an exception, other?

gdb throws if you try to get a non existant field, so I guess that should be in try / catch, or maybe we shouldn't bother with it I'm not sure how useful it is in practice.

> 
> @@ +12,5 @@
> > +        # if the thread doesn't have a name try to get its thread id (might not
> > +        # work on !posix)
> > +        name = prthread['tid']
> > +
> > +        return name if name else '(PRThread *) %s' % prthread
> 
> /might/ be better to make some verbose printing in the case where name does
> something like "[Thread tid=%d]" for the tid case or '[Thread "%s"]' for an
> actual named string.

I'm not sure what the right formating for this one is lets see what froydnj says as module owner.
(In reply to Trevor Saunders (:tbsaunde) from comment #7)

> > > +        name = prthread['tid']

> gdb throws if you try to get a non existant field, so I guess that should be
> in try / catch, or maybe we shouldn't bother with it I'm not sure how useful
> it is in practice.

You can also see if the field exists by querying the type:

   if prthread.type.has_key('tid'):


I read through the patch a bit.  From a gdb perspective it looks pretty reasonable.

I think tarray_printer would be somewhat improved if it returned an iterator
from its 'children' method.  This lets gdb evaluate the elements lazily, which
matters if various array-limited settings are enabled and if the array is large
(also this sort of thing is reasonably typical in gdb GUIs...).
Ping :)
Flags: needinfo?(tbsaunde+mozbugs)
(In reply to Mike Hommey [:glandium] from comment #9)
> Ping :)


ok, I'm working on this again, I should have something with a little more work.
Flags: needinfo?(tbsaunde+mozbugs)
r? glandium for the build stuff
and r? froydnj for the pretty printer stuff since its all xpcomish stuff
Attachment #8756940 - Flags: review?(nfroyd)
Attachment #8756940 - Flags: review?(mh+mozilla)
Comment on attachment 8756940 [details] [diff] [review]
add some pretty printers to .gdbinit

Review of attachment 8756940 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with changes below.  The UniquePtr thing can be a followup, but I'm pretty sure the nsRefPtr bit is busted.

If nsString doesn't display as a string, we should try to fix that somehow...

::: python/gdbpp/gdbpp/smartptr.py
@@ +23,5 @@
> +            return '[(%s) %s]' % (weak_ptr.type, weak_ptr)
> +
> +        return '[(%s) %s]' % (weak_ptr.dynamic_type, weak_ptr)
> +
> +@GeckoPrettyPrinter('nsAutoPtr', 'nsAutoPtr<\w+>')

Can we add a printer for mozilla::UniquePtr?

@@ +25,5 @@
> +        return '[(%s) %s]' % (weak_ptr.dynamic_type, weak_ptr)
> +
> +@GeckoPrettyPrinter('nsAutoPtr', 'nsAutoPtr<\w+>')
> +@GeckoPrettyPrinter('nsCOMPtr', 'nsCOMPtr<\w+>')
> +@GeckoPrettyPrinter('RefPtr', 'nsRefPtr<\w+>')

Presumably this should be:

@GeckoPrettyPrinter('RefPtr', 'RefPtr<\w+>')

?

::: python/gdbpp/gdbpp/string.py
@@ +12,5 @@
> +    def __init__(self, value):
> +        self.value = value
> +
> +    def to_string(self):
> +        return self.value['mData']

Is it possible to respect |show print elements| here, so that somebody printing out something huge doesn't get a terminal full of stuff?  Or is that done by the display_hint method below, internally to GDB?

Does this do intelligent things for 16-bit strings, or does it return them as a bunch of separate characters?

::: python/gdbpp/gdbpp/tarray.py
@@ +17,5 @@
> +    def children(self):
> +        length = self.value['mHdr'].dereference()['mLength']
> +        data = self.value['mHdr'] + 1
> +        elements = data.cast(self.elem_type.pointer())
> +        return map(lambda i: ('%d' % i, (elements + i).dereference()), range(0, int(length)))

Same question about |show print elements| here as well.
Attachment #8756940 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #12)

> > +@GeckoPrettyPrinter('RefPtr', 'nsRefPtr<\w+>')
> 
> Presumably this should be:
> 
> @GeckoPrettyPrinter('RefPtr', 'RefPtr<\w+>')
> 
> ?

It's worth noting that the SpiderMonkey printers have test cases...

> ::: python/gdbpp/gdbpp/string.py
> @@ +12,5 @@
> > +    def __init__(self, value):
> > +        self.value = value
> > +
> > +    def to_string(self):
> > +        return self.value['mData']
> 
> Is it possible to respect |show print elements| here, so that somebody
> printing out something huge doesn't get a terminal full of stuff?  Or is
> that done by the display_hint method below, internally to GDB?

gdb handles this.

> Does this do intelligent things for 16-bit strings, or does it return them
> as a bunch of separate characters?

If the types are correct, gdb will handle this as well.
At least pointer-to-{char,wchar_t,char16_t,char32_t} should be ok.
Sometimes this can be iffy, since gdb sometimes loses information about typedefs;
so if you want to be really safe you can return a "lazy string" and set the encoding
there.

If the strings have lengths and can have embedded zeros (I don't recall), lazy string
can solve this too.  (Or should be able to, there might be a gdb bug, I forget)

> ::: python/gdbpp/gdbpp/tarray.py
> @@ +17,5 @@
> > +    def children(self):
> > +        length = self.value['mHdr'].dereference()['mLength']
> > +        data = self.value['mHdr'] + 1
> > +        elements = data.cast(self.elem_type.pointer())
> > +        return map(lambda i: ('%d' % i, (elements + i).dereference()), range(0, int(length)))
> 
> Same question about |show print elements| here as well.

gdb handles this.  However, it's important to compute the children lazily.  Otherwise
you'll hit a case like an uninitialized object and python will go off into the weeds.

map might be lazy in Python 3, but IIRC not in Python 2.  It's normally important to
support both (different distros and different versions of the same distro build gdb
differently...); it's easy to shim this stuff though.
> ::: python/gdbpp/gdbpp/smartptr.py
> @@ +23,5 @@
> > +            return '[(%s) %s]' % (weak_ptr.type, weak_ptr)
> > +
> > +        return '[(%s) %s]' % (weak_ptr.dynamic_type, weak_ptr)
> > +
> > +@GeckoPrettyPrinter('nsAutoPtr', 'nsAutoPtr<\w+>')
> 
> Can we add a printer for mozilla::UniquePtr?

I expect so I just wanted to get something in then we can add more stuff.

> > +        return '[(%s) %s]' % (weak_ptr.dynamic_type, weak_ptr)
> > +
> > +@GeckoPrettyPrinter('nsAutoPtr', 'nsAutoPtr<\w+>')
> > +@GeckoPrettyPrinter('nsCOMPtr', 'nsCOMPtr<\w+>')
> > +@GeckoPrettyPrinter('RefPtr', 'nsRefPtr<\w+>')
> 
> Presumably this should be:
> 
> @GeckoPrettyPrinter('RefPtr', 'RefPtr<\w+>')
> 
> ?

yeah, I guess I only tried nsCOMPtr, oops

> Does this do intelligent things for 16-bit strings, or does it return them
> as a bunch of separate characters?

experimentally it does


(In reply to Tom Tromey :tromey from comment #13)
> (In reply to Nathan Froyd [:froydnj] from comment #12)
> 
> > > +@GeckoPrettyPrinter('RefPtr', 'nsRefPtr<\w+>')
> > 
> > Presumably this should be:
> > 
> > @GeckoPrettyPrinter('RefPtr', 'RefPtr<\w+>')
> > 
> > ?
> 
> It's worth noting that the SpiderMonkey printers have test cases...

yeah, it might be worth extricating that from spider monkey at some point, but iirc its fairly tangled with the jsapi-tests or something like that.

> > Does this do intelligent things for 16-bit strings, or does it return them
> > as a bunch of separate characters?
> 
> If the types are correct, gdb will handle this as well.
> At least pointer-to-{char,wchar_t,char16_t,char32_t} should be ok.
> Sometimes this can be iffy, since gdb sometimes loses information about
> typedefs;
> so if you want to be really safe you can return a "lazy string" and set the
> encoding
> there.
> 
> If the strings have lengths and can have embedded zeros (I don't recall),
> lazy string
> can solve this too.  (Or should be able to, there might be a gdb bug, I
> forget)

they do so we should probably do that.  I'm not sure how important it is though, I've only seen it actually happen once.

> > ::: python/gdbpp/gdbpp/tarray.py
> > @@ +17,5 @@
> > > +    def children(self):
> > > +        length = self.value['mHdr'].dereference()['mLength']
> > > +        data = self.value['mHdr'] + 1
> > > +        elements = data.cast(self.elem_type.pointer())
> > > +        return map(lambda i: ('%d' % i, (elements + i).dereference()), range(0, int(length)))
> > 
> > Same question about |show print elements| here as well.
> 
> gdb handles this.  However, it's important to compute the children lazily. 
> Otherwise
> you'll hit a case like an uninitialized object and python will go off into
> the weeds.

Well, all the elements from 0 to length - 1 should be printable / have some sort of defined value, so does it actually matter for correctness, or is it just an optimization then?

> map might be lazy in Python 3, but IIRC not in Python 2.  It's normally
> important to
> support both (different distros and different versions of the same distro
> build gdb
> differently...); it's easy to shim this stuff though.

yeah, I was just lazy and wanted to avoid spending too much time on it unless we really need to.  map() is lazy in python3, and in python2 you are supposed to use itertools.imap() but that doesn't exist in python3, and I'm not sure if you can check the python version though I guess you can somehow.
You can do:

try:
    from itertools import imap
except ImportError:
    imap = map

Or, if gdb allows it, you can return an anonymous generator function instead of map:
  return (('%d' % i, (elements + i).dereference()) for i in range(0, int(length)))
(In reply to Trevor Saunders (:tbsaunde) from comment #14)

> Well, all the elements from 0 to length - 1 should be printable / have some
> sort of defined value, so does it actually matter for correctness, or is it
> just an optimization then?

There were gdb bug reports about this that turned out to be trying to inspect
an object that either had the (equivalent of) "length" field trashed, or
were being inspected before the constructor was run, so the field was garbage.

(In reply to Mike Hommey [:glandium] from comment #15)
> Or, if gdb allows it, you can return an anonymous generator function instead
> of map:
>   return (('%d' % i, (elements + i).dereference()) for i in range(0,
> int(length)))

gdb just uses the python iterator API in C.  So if they act like iterators, they work.
It's pretty normal to see 'children' method using 'yield' in a loop.
> Presumably this should be:
> 
> @GeckoPrettyPrinter('RefPtr', 'RefPtr<\w+>')
> 
> ?

actually the \w+ is wrong too, because : and * aren't in \w.  are there other things that could be in a type name? or is [\w:\*]+ correct?
(In reply to Trevor Saunders (:tbsaunde) from comment #17)
> > Presumably this should be:
> > 
> > @GeckoPrettyPrinter('RefPtr', 'RefPtr<\w+>')
> > 
> > ?
> 
> actually the \w+ is wrong too, because : and * aren't in \w.  are there
> other things that could be in a type name? or is [\w:\*]+ correct?

'const ' of course, so probably easiest to just do .*
updated patch, carrying over r=froydnj, but the build stuff still needs review
Attachment #8758840 - Flags: review?(mh+mozilla)
Attachment #8400349 - Attachment is obsolete: true
Attachment #8756940 - Attachment is obsolete: true
Attachment #8756940 - Flags: review?(mh+mozilla)
Comment on attachment 8758840 [details] [diff] [review]
add some pretty printers to .gdbinit

Review of attachment 8758840 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/gdbpp/gdbpp/__init__.py
@@ +6,5 @@
> +
> +import gdb
> +import gdb.printing
> +
> +class GeckoPrettyPrinter(object):

I wouldn't be surprised if the circular dependencies cause problems on some setups. It would be better to have GeckoPrettyPrinter in, say, util.py and import from there.

That said, I'm also not sure, considering the sizes, if it's worth having 4 modules.
Attachment #8758840 - Flags: review?(mh+mozilla) → review+
Pushed by tsaunders@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c993782dbd52
add some pretty printers to .gdbinit r=froydnj r=glandium
https://hg.mozilla.org/mozilla-central/rev/c993782dbd52
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.