From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7644 invoked by alias); 14 Mar 2017 09:05:01 -0000 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 Received: (qmail 7458 invoked by uid 89); 14 Mar 2017 09:05:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 spammy= X-HELO: gnu.wildebeest.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (212.238.236.112) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 14 Mar 2017 09:04:58 +0000 Received: from tarox.wildebeest.org (herd.wildebeest.org [80.127.118.209]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 0F9E430012D1; Tue, 14 Mar 2017 10:04:57 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id EAC6D4000705; Tue, 14 Mar 2017 10:04:56 +0100 (CET) Message-ID: <1489482296.21350.77.camel@klomp.org> Subject: Re: [PATCH] libiberty: Initialize d_printing in all cplus_demangle_* functions. From: Mark Wielaard To: Pedro Alves Cc: Markus Trippelsdorf , gdb-patches@sourceware.org, Nathan Sidwell , Ian Lance Taylor , Nick Clifton Date: Tue, 14 Mar 2017 09:05:00 -0000 In-Reply-To: References: <1489356354-27648-1-git-send-email-mark@klomp.org> <20170313181150.GA287@x4> <20170313182959.GC2167@stream> <1489437334.21350.72.camel@klomp.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 X-SW-Source: 2017-03/txt/msg00229.txt.bz2 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. >=20 > Using that seems to work, testing the patch below on x86_64 Fedora 23 > shows no regressions. >=20 > From e55453c67bbe772ce001ea15b152f8dc44b8945e Mon Sep 17 00:00:00 2001 > From: Pedro Alves > Date: Mon, 13 Mar 2017 22:54:09 +0000 > Subject: [PATCH] gdb/cp-name-parser.y: Eliminate make_empty, use > cplus_demangle_fill_component >=20 > 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 =3D 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