1385364 - Invalid strings throw exceptions with gdb prettyprinters

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Steve Fink [:sfink] [:s:]]

Actually, I'm filing this for a grab bag of string output enhancements. But the most annoying thing is when I have a string pointing into 0x2f2f2f2f memory; it ends up filling up my emacs buffer with crap.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: gdb JSString prettyprinter string length limit and invalid char handling

I know, I should write tests. Feel free to throw it back to me for that. I'm just cleaning out my patch queue right now.

Comment 2

[Jim Blandy :jimb]

Comment on attachment 8891432 [details] [diff] [review]
gdb JSString prettyprinter string length limit and invalid char handling

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

I had a suggestion for a simplification, but this is definitely an improvement, so I'm also approving it as is.

::: js/src/gdb/mozilla/JSString.py
@@ +43,5 @@
> +            0x2b2b2b2b: 'JS_SWEPT_NURSERY_PATTERN',
> +            0xe5e5e5e5: 'jemalloc freed memory',
> +        }.get(flags & 0xffffffff)
> +        if invalid:
> +            for ch in "<INVALID:%s>" % invalid:

I love this.

@@ +53,2 @@
>                  yield c
> +                maxlen -= 1

All this maxlen management is really weird. Since we're generating characters, couldn't we just impose the limit in the to_string method below, and just let the generators' laziness take care of things?

Comment 3

[Steve Fink [:sfink] [:s:]]

(In reply to Jim Blandy :jimb from comment #2)
> @@ +53,2 @@
> >                  yield c
> > +                maxlen -= 1
> 
> All this maxlen management is really weird. Since we're generating
> characters, couldn't we just impose the limit in the to_string method below,
> and just let the generators' laziness take care of things?

Yes, and that is a clear and obvious way to do it, and now I feel awfully stupid for not seeing it.

Hm... would you be ok with me changing s/"<INVALID:%s>"/"<CORRUPT:%s"/? The problem is that I was debugging a test case that intentionally inserts an invalid codepoint (or whatever they're called) at the very beginning, so this code produced "<INVALID>". Which I misinterpreted to mean the same thing as one of these "<INVALID:blahblah>" things, namely that the string was bogus. But it wasn't; it just contained problematic data.

Actually, I'd like to go farther than that. I'll ask my question via a followup patch. Thanks for the review.

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: gdb JSString prettyprinter string length limit and invalid char handling,

Comment 5

[Jim Blandy :jimb]

Comment on attachment 8891558 [details] [diff] [review]
gdb JSString prettyprinter string length limit and invalid char handling,

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

This looks great - thanks very much!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/847333a06c62
gdb JSString prettyprinter string length limit and invalid char handling, r=jimb

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/847333a06c62