This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] libiberty: Initialize d_printing in all cplus_demangle_* functions.
- From: Mark Wielaard <mark at klomp dot org>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Markus Trippelsdorf <markus at trippelsdorf dot de>, gdb-patches at sourceware dot org, Nathan Sidwell <nathan at acm dot org>, Ian Lance Taylor <iant at google dot com>, Nick Clifton <nickc at redhat dot com>
- Date: Tue, 14 Mar 2017 10:04:56 +0100
- Subject: Re: [PATCH] libiberty: Initialize d_printing in all cplus_demangle_* functions.
- Authentication-results: sourceware.org; auth=none
- References: <1489356354-27648-1-git-send-email-mark@klomp.org> <20170313181150.GA287@x4> <20170313182959.GC2167@stream> <a3993cdc-dbb0-e377-3e46-a84d8cf971a0@redhat.com> <1489437334.21350.72.camel@klomp.org> <f341a66c-8a47-b38c-6355-818400e9f7c7@redhat.com>
On Tue, 2017-03-14 at 01:26 +0000, Pedro Alves wrote:
> And there's a cplus_demangle_fill_component function to fill
> these in. It's already used by cp-name-parser.y in other
> cases, via the fill_comp wrapper function.
>
> Using that seems to work, testing the patch below on x86_64 Fedora 23
> shows no regressions.
>
> From e55453c67bbe772ce001ea15b152f8dc44b8945e Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 13 Mar 2017 22:54:09 +0000
> Subject: [PATCH] gdb/cp-name-parser.y: Eliminate make_empty, use
> cplus_demangle_fill_component
>
> The demangler exports the cplus_demangle_fill_component function that
> clients should use to initialize demangle_component components that
> use the "s_binary" union member. cp-name-parser.y uses it in some
> places, via the fill_comp wrapper, but not all. Several places
> instead use a GDB-specific "make_empty" function. Because this
> function does not call any of the demangler "fill" functions, we had
> to patch it recently to clear the allocated demangle_component's
> "d_printing" field, which is supposedly a "private" demangler field.
> To avoid such problems in the future, this commit switches those
> places to use "fill_comp" instead, and eliminates the "make_empty"
> function.
That looks good. But note that there is one behavioral change.
cplus_demangle_fill_component is defined as:
/* Fill in most component types with a left subtree and a right
subtree. Returns non-zero on success, zero on failure, such as an
unrecognized or inappropriate component type. */
And gdb/cp-name-parser.y fill_comp does:
i = cplus_demangle_fill_component (ret, d_type, lhs, rhs);
gdb_assert (i);
So you have an extra sanity check. Where before you might have silently
created a bogus demangle_component. Which I assume is what you want. But
it might depend on what gdb_assert precisely does and if the parser
input is normally "sane" or not.
Cheers,
Mark