From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by sourceware.org (Postfix) with ESMTPS id ADB88385DC1B for ; Wed, 15 Apr 2020 12:48:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org ADB88385DC1B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dodji@seketeli.org Received: from localhost (91-166-131-130.subs.proxad.net [91.166.131.130]) (Authenticated sender: dodji@seketeli.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 0B219100002; Wed, 15 Apr 2020 12:48:56 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id EE5A5581890; Wed, 15 Apr 2020 14:48:55 +0200 (CEST) From: Dodji Seketeli To: Mark Wielaard Cc: libabigail@sourceware.org Subject: Re: [PATCH] Add --drop-undefined-syms to abidw. Organization: Me, myself and I References: <20200413013901.GA3584@wildebeest.org> <20200413014058.57109-1-mark@klomp.org> X-Operating-System: Fedora 33 X-URL: http://www.seketeli.net/~dodji Date: Wed, 15 Apr 2020 14:48:55 +0200 In-Reply-To: <20200413014058.57109-1-mark@klomp.org> (Mark Wielaard's message of "Mon, 13 Apr 2020 03:40:58 +0200") Message-ID: <877dygoqx4.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-17.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Apr 2020 12:49:00 -0000 Hello Mark, Mark Wielaard a =C3=A9crit: > Add a drop_undefined_syms properties to the read_context that > indicates the reader wants to drop any undefined symbols (which don't > have associated addresses in the corpus). Implement this for the > dwarf_reader and abidw (when using the --drop-undefined-syms option). > > * include/abg-dwarf-reader.h (set_drop_undefined_syms): > New declaration. > * src/abg-dwarf-reader.cc (class read_context): Add private > bool drop_undefined_syms_. > (drop_undefined_syms): New getter and setter method. > (set_drop_undefined_syms): New function. > (function_is_suppressed): Check drop_undefined_syms on > read_context. > (variable_is_suppressed): Likewise. > * src/abg-reader.cc (read_context): Add private bool > m_drop_undefined_syms. > (drop_undefined_syms): New getter and setter method. > * tools/abidw.cc (struct options): Add drop_undefined_syms. > (display_usage): Print --drop-undefined-syms. > (parse_command_line): Parse --drop-undefined-syms. > (main): Call set_drop_undefined_syms. I have applied this to master. I have just fixed a few things that I discuss below. [...] > diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc > index 249e196d..f74fb144 100644 > --- a/src/abg-dwarf-reader.cc > +++ b/src/abg-dwarf-reader.cc [...] > /// This flag tells if we should emit verbose logs for various > @@ -16683,7 +16714,9 @@ function_is_suppressed(const read_context& ctxt, > string qualified_name =3D build_qualified_name(scope, fname); >=20=20 > // A non-member function which symbol is not exported is suppressed. > - if (!is_class_type(scope) && !die_is_declaration_only(function_die)) > + // Or if the reader context drops undefined symbols. > + if ((!is_class_type(scope) && !die_is_declaration_only(function_die)) > + || ctxt.drop_undefined_syms()) > { For languages like C++, we don't want to remove member functions (those that are at class scope) even if they don't have defined symbols (e.g, inline member functions). So here is how I am writing this: // A non-member non-static function which symbol is not exported is // suppressed. // // Note that if the non-member non-static function has an undefined // symbol, by default, it's not suppressed. Unless we are asked to // drop undefined symbols too. if (!is_class_type(scope) && (!die_is_declaration_only(function_die) || ctxt.drop_undefined_syms())) { [...] > @@ -16798,7 +16831,10 @@ variable_is_suppressed(const read_context& ctxt, > // another concrete variable, then this function looks at > // suppression specification specifications to know if its > // suppressed. > - if (!is_class_type(scope) && !is_required_decl_spec) > + // > + // Or if the reader context drops undefined symbols. > + if ((!is_class_type(scope) && !is_required_decl_spec) > + || ctxt.drop_undefined_syms()) > { So, right now, undefined variable (unlike for functions) are dropped from the IR, unless they are data members or declaration of static data members. So we don't need the ctxt.drop_undefined_syms() here. So I re-wrote this as: // If a non member variable that is a declaration (has no defined // and exported symbol) and is not the specification of another // concrete variable, then it's suppressed. This is a size // optimization; it removes useless declaration-only variables from // the IR. if (!is_class_type(scope) && !is_required_decl_spec) { [...] Thanks! Cheers, --=20 Dodji