This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Keith Seitz <keiths at redhat dot com>, "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>, Yao Qi <yao at codesourcery dot com>
- Date: Fri, 12 Aug 2016 13:00:19 +0100
- Subject: Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]
- Authentication-results: sourceware.org; auth=none
- References: <20160201030638.GG4008@adacore.com> <56AFB750.3030702@redhat.com> <56B29F17.6020603@redhat.com> <56B344A5.1060406@redhat.com>
Hi Keith,
Are you still working on this patch? This problem bites me again in the 7.12
testing, and that leads me digging out this patch. I don't think we need to
pick it up in 7.12, but it's better to pick it in master after 7.12
release, so that
we have a plenty of time to see how good (or bad) it is.
On Thu, Feb 4, 2016 at 12:31 PM, Pedro Alves <palves@redhat.com> wrote:
> On 02/04/2016 12:45 AM, Keith Seitz wrote:
>
>> At Pedro's urging, I found another, simpler solution. The big problem
>> with just about all solutions I was attempting to implement is
>> [p]symtab_to_fululname/find_and_open_source/openp. These functions are
>> just completely unaware of what the caller is attempting to do.
>
> Thanks!
>
>
>> I'm sure there are corner cases and a whole bunch of other problems with
>> this approach, but at least it is isolated to one place (for better or
>> worse).
>
> Things to watch out for, out of the blue:
>
> - relative SYMTAB_DIRNAMEs -- is there such a thing?
>
> - symlinks. Say, what happens if foo/f.c is a symlink to f.c (perhaps
> each is compiled differently, with -DWHATNOT, for example).
>
>> +static VEC (symtab_ptr) *
>> +filter_default_symtabs (const char *fullname, VEC (symtab_ptr) *symtabs)
>> +{
>> + int ix;
>> + struct symtab *symtab;
>> + VEC (symtab_ptr) *filtered_symtabs = NULL;
>> + struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
>
> I think filtered_symtabs should be guarded with a cleanup as well.
>
>> +
>> + /* Iterate through the symtabs, searching for matches to FULLNAME. */
>> + for (ix = 0; VEC_iterate (symtab_ptr, symtabs, ix, symtab); ++ix)
>> + {
>> + const char *basename = lbasename (fullname);
>> + char *symtab_with_dir;
>> +
>> + if (SYMTAB_DIRNAME (symtab) == NULL)
>> + continue;
>> +
>> + symtab_with_dir = concat (SYMTAB_DIRNAME (symtab), SLASH_STRING,
>> + basename, NULL);
>> + make_cleanup (xfree, symtab_with_dir);
>> + if (streq (fullname, symtab_with_dir))
>> + VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
>> + else
>> + {
>> + /* Now try any path substitution rules. */
>> + symtab_with_dir = rewrite_source_path (symtab_with_dir);
>> + if (symtab_with_dir != NULL)
>> + {
>> + make_cleanup (xfree, symtab_with_dir);
>> + if (streq (fullname, symtab_with_dir))
>> + VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
>> + }
>> + }
>
> This creates two cleanups per iteration here. Only two for the whole
> loop would suffice, if you used free_current_contents instead of xfree.
>
>> + }
>> +
>> + /* If we found no matches, use whatever symtabs were originally
>> + "collected." */
>> + if (filtered_symtabs == NULL)
>
> Pedantically, checking VEC_empty instead would be better, as an initial
>
> VEC_reserve (symtab_ptr, filtered_symtabs, VEC_size (symtab_ptr, symtab));
>
> would make a NULL check wrong. That reserve might make sense if most of
> the time we'll match all symtabs.
>
>> + {
>> + /* Found no exact matches -- use the original list. */
>> + filtered_symtabs = symtabs;
>> + }
>> + else
>> + VEC_free (symtab_ptr, symtabs);
>> +
>> + do_cleanups (back_to);
>> + return filtered_symtabs;
>> +}
>
> Thanks,
> Pedro Alves
>
--
Yao (齐尧)