Message ID | 1403189133-7667-1-git-send-email-simon.marchi@ericsson.com |
---|---|
State | Committed |
Headers |
Received: (qmail 25020 invoked by alias); 19 Jun 2014 14:45:46 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25010 invoked by uid 89); 19 Jun 2014 14:45:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg20.ericsson.net Received: from usevmg20.ericsson.net (HELO usevmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 19 Jun 2014 14:45:44 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id 1A.DD.27529.C56A2A35; Thu, 19 Jun 2014 10:59:09 +0200 (CEST) Received: from simark-hp.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.96) with Microsoft SMTP Server (TLS) id 14.3.174.1; Thu, 19 Jun 2014 10:45:40 -0400 From: Simon Marchi <simon.marchi@ericsson.com> To: <gdb-patches@sourceware.org> CC: Simon Marchi <simon.marchi@ericsson.com> Subject: [PATCH] Handle OP_STRING in dump_subexp_body_standard Date: Thu, 19 Jun 2014 10:45:33 -0400 Message-ID: <1403189133-7667-1-git-send-email-simon.marchi@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes |
Commit Message
Simon Marchi
June 19, 2014, 2:45 p.m. UTC
For some reason, OP_STRING is not handled in dump_subexp_body_standard. This makes the output of "set debug expression 1" very bad when a string is involved. Example: (gdb) set debug expression 1 (gdb) print "hello" ... (random garbage, possibly segfault) This commit handles OP_STRING and skips the appropriate number of exp elements. The line corresponding to the string now looks like: 0 OP_STRING Language-specific string type: 0 gdb/ChangeLog: 2014-06-19 Simon Marchi <simon.marchi@ericsson.com> * expprint.c (dump_subexp_body_standard): Handle OP_STRING. --- gdb/expprint.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
Comments
On 14-06-19 10:45 AM, Simon Marchi wrote: > For some reason, OP_STRING is not handled in dump_subexp_body_standard. > This makes the output of "set debug expression 1" very bad when a string > is involved. Example: > > (gdb) set debug expression 1 > (gdb) print "hello" > ... (random garbage, possibly segfault) > > This commit handles OP_STRING and skips the appropriate number of exp > elements. The line corresponding to the string now looks like: > > 0 OP_STRING Language-specific string type: 0 > > gdb/ChangeLog: > > 2014-06-19 Simon Marchi <simon.marchi@ericsson.com> > > * expprint.c (dump_subexp_body_standard): Handle OP_STRING. > --- > gdb/expprint.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/gdb/expprint.c b/gdb/expprint.c > index 97188ed..60971a5 100644 > --- a/gdb/expprint.c > +++ b/gdb/expprint.c > @@ -1011,12 +1011,29 @@ dump_subexp_body_standard (struct expression *exp, > elt = dump_subexp (exp, stream, elt); > } > break; > + case OP_STRING: > + { > + LONGEST len = exp->elts[elt].longconst; > + LONGEST type = exp->elts[elt].longconst; > + > + fprintf_filtered (stream, "Language-specific string type: %s", > + plongest (type)); > + > + /* Skip length. */ > + elt += 1; > + > + /* Skip string content. */ > + elt += BYTES_TO_EXP_ELEM(len); > + > + /* Skip length and ending OP_STRING. */ > + elt += 2; > + } > + break; > default: > case OP_NULL: > case MULTI_SUBSCRIPT: > case OP_F77_UNDETERMINED_ARGLIST: > case OP_COMPLEX: > - case OP_STRING: > case OP_BOOL: > case OP_M2_STRING: > case OP_THIS: Ping!
Hello Simon, > > For some reason, OP_STRING is not handled in dump_subexp_body_standard. > > This makes the output of "set debug expression 1" very bad when a string > > is involved. Example: > > > > (gdb) set debug expression 1 > > (gdb) print "hello" > > ... (random garbage, possibly segfault) > > > > This commit handles OP_STRING and skips the appropriate number of exp > > elements. The line corresponding to the string now looks like: > > > > 0 OP_STRING Language-specific string type: 0 > > > > gdb/ChangeLog: > > > > 2014-06-19 Simon Marchi <simon.marchi@ericsson.com> > > > > * expprint.c (dump_subexp_body_standard): Handle OP_STRING. Sorry about the delay. > > --- > > gdb/expprint.c | 19 ++++++++++++++++++- > > 1 file changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/gdb/expprint.c b/gdb/expprint.c > > index 97188ed..60971a5 100644 > > --- a/gdb/expprint.c > > +++ b/gdb/expprint.c > > @@ -1011,12 +1011,29 @@ dump_subexp_body_standard (struct expression *exp, > > elt = dump_subexp (exp, stream, elt); > > } > > break; > > + case OP_STRING: > > + { > > + LONGEST len = exp->elts[elt].longconst; > > + LONGEST type = exp->elts[elt].longconst; These two are the same :-). Looking at parse.c::write_exp_string_vector, which seems to be the function responsible for creating OP_STRING nodes, it writes the opcode, then the length, then the type, and finally the vector: write_exp_elt_opcode (ps, OP_STRING); write_exp_elt_longcst (ps, len); write_exp_elt_longcst (ps, type); [loop writing the vector] So, I would say that it should be "elt + 1" for variable "type". > > + fprintf_filtered (stream, "Language-specific string type: %s", > > + plongest (type)); > > + > > + /* Skip length. */ > > + elt += 1; > > + > > + /* Skip string content. */ > > + elt += BYTES_TO_EXP_ELEM(len); Missing space before '('.
Hi Joel, On 14-07-15 09:20 AM, Joel Brobecker wrote: > Hello Simon, > >>> For some reason, OP_STRING is not handled in dump_subexp_body_standard. >>> This makes the output of "set debug expression 1" very bad when a string >>> is involved. Example: >>> >>> (gdb) set debug expression 1 >>> (gdb) print "hello" >>> ... (random garbage, possibly segfault) >>> >>> This commit handles OP_STRING and skips the appropriate number of exp >>> elements. The line corresponding to the string now looks like: >>> >>> 0 OP_STRING Language-specific string type: 0 >>> >>> gdb/ChangeLog: >>> >>> 2014-06-19 Simon Marchi <simon.marchi@ericsson.com> >>> >>> * expprint.c (dump_subexp_body_standard): Handle OP_STRING. > > Sorry about the delay. > >>> --- >>> gdb/expprint.c | 19 ++++++++++++++++++- >>> 1 file changed, 18 insertions(+), 1 deletion(-) >>> >>> diff --git a/gdb/expprint.c b/gdb/expprint.c >>> index 97188ed..60971a5 100644 >>> --- a/gdb/expprint.c >>> +++ b/gdb/expprint.c >>> @@ -1011,12 +1011,29 @@ dump_subexp_body_standard (struct expression *exp, >>> elt = dump_subexp (exp, stream, elt); >>> } >>> break; >>> + case OP_STRING: >>> + { >>> + LONGEST len = exp->elts[elt].longconst; >>> + LONGEST type = exp->elts[elt].longconst; > > These two are the same :-). > > Looking at parse.c::write_exp_string_vector, which seems to be > the function responsible for creating OP_STRING nodes, it writes > the opcode, then the length, then the type, and finally the vector: > > write_exp_elt_opcode (ps, OP_STRING); > write_exp_elt_longcst (ps, len); > write_exp_elt_longcst (ps, type); > [loop writing the vector] > > So, I would say that it should be "elt + 1" for variable "type". You are totally right. >>> + fprintf_filtered (stream, "Language-specific string type: %s", >>> + plongest (type)); >>> + >>> + /* Skip length. */ >>> + elt += 1; >>> + >>> + /* Skip string content. */ >>> + elt += BYTES_TO_EXP_ELEM(len); > > Missing space before '('. Ack. Thanks for the review. Do these small fixes warrant a v2? Simon
> >>> gdb/ChangeLog: > >>> > >>> 2014-06-19 Simon Marchi <simon.marchi@ericsson.com> > >>> > >>> * expprint.c (dump_subexp_body_standard): Handle OP_STRING. [...] > > So, I would say that it should be "elt + 1" for variable "type". > > You are totally right. > > >>> + /* Skip string content. */ > >>> + elt += BYTES_TO_EXP_ELEM(len); > > > > Missing space before '('. > > Ack. > > Thanks for the review. Do these small fixes warrant a v2? OK, pre-approved with the changes above, but our procedures do require you to re-post a patch whenever what's committed is different from what was originally posted. I'd say, make the modifications, test them, commit & push, and then reply to this thread with the updated patch. Thank you,
diff --git a/gdb/expprint.c b/gdb/expprint.c index 97188ed..60971a5 100644 --- a/gdb/expprint.c +++ b/gdb/expprint.c @@ -1011,12 +1011,29 @@ dump_subexp_body_standard (struct expression *exp, elt = dump_subexp (exp, stream, elt); } break; + case OP_STRING: + { + LONGEST len = exp->elts[elt].longconst; + LONGEST type = exp->elts[elt].longconst; + + fprintf_filtered (stream, "Language-specific string type: %s", + plongest (type)); + + /* Skip length. */ + elt += 1; + + /* Skip string content. */ + elt += BYTES_TO_EXP_ELEM(len); + + /* Skip length and ending OP_STRING. */ + elt += 2; + } + break; default: case OP_NULL: case MULTI_SUBSCRIPT: case OP_F77_UNDETERMINED_ARGLIST: case OP_COMPLEX: - case OP_STRING: case OP_BOOL: case OP_M2_STRING: case OP_THIS: