From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22370 invoked by alias); 2 May 2012 18:56:52 -0000 Received: (qmail 22361 invoked by uid 22791); 2 May 2012 18:56:50 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 02 May 2012 18:56:31 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q42IuUwE002580 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 2 May 2012 14:56:30 -0400 Received: from psique (ovpn-112-58.phx2.redhat.com [10.3.112.58]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q42IuOc6031931; Wed, 2 May 2012 14:56:26 -0400 From: Sergio Durigan Junior To: dje@google.com (Doug Evans) Cc: tromey@redhat.com, jan.kratochvil@redhat.com, gdb-patches@sourceware.org Subject: Re: [RFC] Fix build failure in stap-probe.c. References: <20120502081647.A1B041E1374@ruffy2.mtv.corp.google.com> X-URL: http://www.redhat.com Date: Wed, 02 May 2012 18:56:00 -0000 In-Reply-To: <20120502081647.A1B041E1374@ruffy2.mtv.corp.google.com> (Doug Evans's message of "Wed, 2 May 2012 01:16:47 -0700 (PDT)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-05/txt/msg00028.txt.bz2 On Wednesday, May 02 2012, Doug Evans wrote: > Hi. > > I'm getting build failures, gcc is complaining that "opcode" and > "lookahead_opcode" "may be used uninitialized". Hi Doug, I'm not seeing this error when I compile (even with -O2), but I believe you :-). > cc1: warnings being treated as errors > ../../src/gdb/stap-probe.c: In function 'stap_parse_argument_1': > ../../src/gdb/stap-probe.c:1558: error: 'lookahead_opcode' may be used uninitialized in this function > ../../src/gdb/stap-probe.c:813: note: 'lookahead_opcode' was declared here > ../../src/gdb/stap-probe.c:1558: error: 'opcode' may be used uninitialized in this function > ../../src/gdb/stap-probe.c:778: note: 'opcode' was declared here > make: *** [stap-probe.o] Error 1 > > This patch is just RFC. > IIUC the code already watches for valid operators before > calling stap_get_opcode, so stap_get_opcode should "never" return zero. > So I'm wondering if maybe step_get_opcode should be changed > to always succeed and always set the opcode. You're right. Does this patch work for you? 2012-05-02 Sergio Durigan Junior * stap-probe.c (stap_is_operator): Change declaration. (stap_get_opcode): Change return value. (stap_parse_argument_1): Update calls to `stap_get_opcode' and `stap_parse_argument_1'. --- gdb/stap-probe.c | 101 ++++++++++++++++++++++++++++++++--------------------- 1 files changed, 61 insertions(+), 40 deletions(-) diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index d1a38da..273ae07 100644 --- a/gdb/stap-probe.c +++ b/gdb/stap-probe.c @@ -154,7 +154,7 @@ static void stap_parse_argument_conditionally (struct stap_parse_info *p); /* Returns 1 if *S is an operator, zero otherwise. */ -static int stap_is_operator (char op); +static int stap_is_operator (const char *op); static void show_stapexpressiondebug (struct ui_file *file, int from_tty, @@ -209,111 +209,107 @@ stap_get_operator_prec (enum exp_opcode op) /* Given S, read the operator in it and fills the OP pointer with its code. Return 1 on success, zero if the operator was not recognized. */ -static int -stap_get_opcode (const char **s, enum exp_opcode *op) +static enum exp_opcode +stap_get_opcode (const char **s) { const char c = **s; - int ret = 1; + enum exp_opcode op; *s += 1; switch (c) { case '*': - *op = BINOP_MUL; + op = BINOP_MUL; break; case '/': - *op = BINOP_DIV; + op = BINOP_DIV; break; case '%': - *op = BINOP_REM; + op = BINOP_REM; break; case '<': - *op = BINOP_LESS; + op = BINOP_LESS; if (**s == '<') { *s += 1; - *op = BINOP_LSH; + op = BINOP_LSH; } else if (**s == '=') { *s += 1; - *op = BINOP_LEQ; + op = BINOP_LEQ; } else if (**s == '>') { *s += 1; - *op = BINOP_NOTEQUAL; + op = BINOP_NOTEQUAL; } break; case '>': - *op = BINOP_GTR; + op = BINOP_GTR; if (**s == '>') { *s += 1; - *op = BINOP_RSH; + op = BINOP_RSH; } else if (**s == '=') { *s += 1; - *op = BINOP_GEQ; + op = BINOP_GEQ; } break; case '|': - *op = BINOP_BITWISE_IOR; + op = BINOP_BITWISE_IOR; if (**s == '|') { *s += 1; - *op = BINOP_LOGICAL_OR; + op = BINOP_LOGICAL_OR; } break; case '&': - *op = BINOP_BITWISE_AND; + op = BINOP_BITWISE_AND; if (**s == '&') { *s += 1; - *op = BINOP_LOGICAL_AND; + op = BINOP_LOGICAL_AND; } break; case '^': - *op = BINOP_BITWISE_XOR; + op = BINOP_BITWISE_XOR; break; case '!': - *op = UNOP_LOGICAL_NOT; + op = UNOP_LOGICAL_NOT; break; case '+': - *op = BINOP_ADD; + op = BINOP_ADD; break; case '-': - *op = BINOP_SUB; + op = BINOP_SUB; break; case '=': - if (**s != '=') - { - ret = 0; - break; - } - *op = BINOP_EQUAL; + gdb_assert (**s == '='); + op = BINOP_EQUAL; break; default: - /* We didn't find any operator. */ - *s -= 1; - return 0; + internal_error (__FILE__, __LINE__, + _("Invalid opcode in expression `%s' for SystemTap" + "probe"), *s); } - return ret; + return op; } /* Given the bitness of the argument, represented by B, return the @@ -773,7 +769,7 @@ stap_parse_argument_1 (struct stap_parse_info *p, int has_lhs, enum exp_opcode opcode; enum stap_operand_prec cur_prec; - if (!stap_is_operator (*p->arg)) + if (!stap_is_operator (p->arg)) error (_("Invalid operator `%c' on expression `%s'."), *p->arg, p->saved_arg); @@ -782,7 +778,7 @@ stap_parse_argument_1 (struct stap_parse_info *p, int has_lhs, operator. If this operator's precedence is lower than PREC, we should return and not advance the expression buffer pointer. */ tmp_exp_buf = p->arg; - stap_get_opcode (&tmp_exp_buf, &opcode); + opcode = stap_get_opcode (&tmp_exp_buf); cur_prec = stap_get_operator_prec (opcode); if (cur_prec < prec) @@ -803,7 +799,7 @@ stap_parse_argument_1 (struct stap_parse_info *p, int has_lhs, /* While we still have operators, try to parse another right-side, but using the current right-side as a left-side. */ - while (*p->arg && stap_is_operator (*p->arg)) + while (*p->arg && stap_is_operator (p->arg)) { enum exp_opcode lookahead_opcode; enum stap_operand_prec lookahead_prec; @@ -811,7 +807,7 @@ stap_parse_argument_1 (struct stap_parse_info *p, int has_lhs, /* Saving the current expression buffer position. The explanation is the same as above. */ tmp_exp_buf = p->arg; - stap_get_opcode (&tmp_exp_buf, &lookahead_opcode); + lookahead_opcode = stap_get_opcode (&tmp_exp_buf); lookahead_prec = stap_get_operator_prec (lookahead_opcode); if (lookahead_prec <= prec) @@ -1013,11 +1009,36 @@ stap_get_probe_argument_count (struct probe *probe_generic, otherwise. */ static int -stap_is_operator (char op) +stap_is_operator (const char *op) { - return (op == '+' || op == '-' || op == '*' || op == '/' - || op == '>' || op == '<' || op == '!' || op == '^' - || op == '|' || op == '&' || op == '%' || op == '='); + int ret = 1; + + switch (*op) + { + case '*': + case '/': + case '%': + case '^': + case '!': + case '+': + case '-': + case '<': + case '>': + case '|': + case '&': + break; + + case '=': + if (op[1] != '=') + ret = 0; + break; + + default: + /* We didn't find any operator. */ + ret = 0; + } + + return ret; } static struct stap_probe_arg * -- 1.7.7.6 -- Sergio