From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21901 invoked by alias); 18 Jul 2012 19:14:42 -0000 Received: (qmail 21880 invoked by uid 22791); 18 Jul 2012 19:14:41 -0000 X-SWARE-Spam-Status: No, hits=-6.0 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,TW_BJ,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, 18 Jul 2012 19:14:25 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q6IJEO4Y014304 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 18 Jul 2012 15:14:24 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q6IJELNX012531 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 18 Jul 2012 15:14:23 -0400 Message-ID: <50070B0D.2050002@redhat.com> Date: Wed, 18 Jul 2012 19:14:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 MIME-Version: 1.0 To: Tom Tromey CC: "gp >> \"gdb-patches@sourceware.org ml\"" Subject: Re: [RFA] Cleanup: Use add_sal_to_sals for expressions References: <5005D93D.7020808@redhat.com> <878veh3u4z.fsf@fleche.redhat.com> In-Reply-To: <878veh3u4z.fsf@fleche.redhat.com> Content-Type: multipart/mixed; boundary="------------050001060506040904020001" 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-07/txt/msg00286.txt.bz2 This is a multi-part message in MIME format. --------------050001060506040904020001 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1861 On 07/18/2012 09:34 AM, Tom Tromey wrote: >>>>>> "Keith" == Keith Seitz writes: > > Keith> This patch causes expressions to use add_sal_to_sals, which will then > Keith> set a canonical name for the linespec, thereby negating the need to > Keith> double-check for missing linespecs later (in decode_line_full). > > Keith> @@ -842,6 +842,8 @@ add_sal_to_sals (struct linespec_state *self, > Keith> else > Keith> canonical_name = xstrprintf ("%s:%d", filename, sal->line); > Keith> } > Keith> + else if (symname != NULL) > Keith> + canonical_name = xstrdup (symname); > > [...] > > Keith> + sal = find_pc_line (ls->expr_pc, 0); > Keith> + sal.pc = ls->expr_pc; > Keith> + sal.section = find_pc_overlay (ls->expr_pc); > Keith> + sal.explicit_pc = 1; > Keith> + add_sal_to_sals (state, &sals, &sal, ls->expression); > > It seems to me that find_pc_line can return a sal with symtab set; > search for "val.symtab = " in find_pc_sect_line. In this case it seems > that you'd wind up with the wrong canonical form. Yes, you are correct. [As if you didn't know that already!] As much as I hate to extend the API for add_sal_to_sals (especially for a special case like this), but I really want [to try?] to isolate name canonicalization in one place, and add_sal_to_sals seems like the best candidate for that. So, what do you think of this revised patch? [No regressions on x86-64 linux.] Keith ChangeLog 2012-07-18 Keith Seitz * linespec.c (add_sal_to_sals): Add LITERAL_CANONICAL parameter. If non-zero, use SYMNAME as the canonical name for the SaL. Update all callers. (convert_linespec_to_sals): Use add_sal_to_sals for expressions, too. (decode_line_full): No need to "fill in missing canonical names" anymore. Simply make cleanups for the allocated names. --------------050001060506040904020001 Content-Type: text/x-patch; name="use-add_sal_to_sals-for-expressions-2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="use-add_sal_to_sals-for-expressions-2.patch" Content-length: 4746 diff --git a/gdb/linespec.c b/gdb/linespec.c index 4156694..4c9c110 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -810,13 +810,16 @@ add_sal_to_sals_basic (struct symtabs_and_lines *sals, /* Add SAL to SALS, and also update SELF->CANONICAL_NAMES to reflect the new sal, if needed. If not NULL, SYMNAME is the name of the - symbol to use when constructing the new canonical name. */ + symbol to use when constructing the new canonical name. + + If LITERAL_CANONICAL is non-zero, SYMNAME will be used as the + canonical name for the SAL. */ static void add_sal_to_sals (struct linespec_state *self, struct symtabs_and_lines *sals, struct symtab_and_line *sal, - const char *symname) + const char *symname, int literal_canonical) { add_sal_to_sals_basic (sals, sal); @@ -826,7 +829,7 @@ add_sal_to_sals (struct linespec_state *self, self->canonical_names = xrealloc (self->canonical_names, sals->nelts * sizeof (char *)); - if (sal->symtab && sal->symtab->filename) + if (!literal_canonical && sal->symtab && sal->symtab->filename) { char *filename = sal->symtab->filename; @@ -842,6 +845,8 @@ add_sal_to_sals (struct linespec_state *self, else canonical_name = xstrprintf ("%s:%d", filename, sal->line); } + else if (symname != NULL) + canonical_name = xstrdup (symname); self->canonical_names[sals->nelts - 1] = canonical_name; } @@ -1809,7 +1814,7 @@ create_sals_line_offset (struct linespec_state *self, found. */ intermediate_results.sals[i].line = val.line; add_sal_to_sals (self, &values, &intermediate_results.sals[i], - sym ? SYMBOL_NATURAL_NAME (sym) : NULL); + sym ? SYMBOL_NATURAL_NAME (sym) : NULL, 0); } do_cleanups (cleanup); @@ -1837,13 +1842,14 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) if (ls->expression != NULL) { + struct symtab_and_line sal; + /* We have an expression. No other attribute is allowed. */ - sals.sals = XMALLOC (struct symtab_and_line); - sals.nelts = 1; - sals.sals[0] = find_pc_line (ls->expr_pc, 0); - sals.sals[0].pc = ls->expr_pc; - sals.sals[0].section = find_pc_overlay (ls->expr_pc); - sals.sals[0].explicit_pc = 1; + sal = find_pc_line (ls->expr_pc, 0); + sal.pc = ls->expr_pc; + sal.section = find_pc_overlay (ls->expr_pc); + sal.explicit_pc = 1; + add_sal_to_sals (state, &sals, &sal, ls->expression, 1); } else if (ls->labels.label_symbols != NULL) { @@ -1856,7 +1862,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) { symbol_to_sal (&sal, state->funfirstline, sym); add_sal_to_sals (state, &sals, &sal, - SYMBOL_NATURAL_NAME (sym)); + SYMBOL_NATURAL_NAME (sym), 0); } } else if (ls->function_symbols != NULL || ls->minimal_symbols != NULL) @@ -1882,7 +1888,8 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) set_current_program_space (pspace); symbol_to_sal (&sal, state->funfirstline, sym); if (maybe_add_address (state->addr_set, pspace, sal.pc)) - add_sal_to_sals (state, &sals, &sal, SYMBOL_NATURAL_NAME (sym)); + add_sal_to_sals (state, &sals, &sal, + SYMBOL_NATURAL_NAME (sym), 0); } } @@ -2284,19 +2291,15 @@ decode_line_full (char **argptr, int flags, gdb_assert (canonical->addr_string != NULL); canonical->pre_expanded = 1; - /* Fill in the missing canonical names. */ + /* Arrange for allocated canonical names to be freed. */ if (result.nelts > 0) { int i; - if (state->canonical_names == NULL) - state->canonical_names = xcalloc (result.nelts, sizeof (char *)); make_cleanup (xfree, state->canonical_names); for (i = 0; i < result.nelts; ++i) { - if (state->canonical_names[i] == NULL) - state->canonical_names[i] = savestring (arg_start, - *argptr - arg_start); + gdb_assert (state->canonical_names[i] != NULL); make_cleanup (xfree, state->canonical_names[i]); } } @@ -3110,7 +3113,7 @@ decode_digits_list_mode (struct linespec_state *self, val.pc = 0; val.explicit_line = 1; - add_sal_to_sals (self, values, &val, NULL); + add_sal_to_sals (self, values, &val, NULL, 0); } } @@ -3254,7 +3257,7 @@ minsym_found (struct linespec_state *self, struct objfile *objfile, skip_prologue_sal (&sal); if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc)) - add_sal_to_sals (self, result, &sal, SYMBOL_NATURAL_NAME (msymbol)); + add_sal_to_sals (self, result, &sal, SYMBOL_NATURAL_NAME (msymbol), 0); } /* A helper struct to pass some data through --------------050001060506040904020001--