From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id 338843853815 for ; Mon, 17 May 2021 19:26:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 338843853815 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x42e.google.com with SMTP id i17so7584401wrq.11 for ; Mon, 17 May 2021 12:26:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=fdV7LyyXXTcy9/RPkfyIJcAgv3nS4LdKyo30vJcQftY=; b=c7lHORr95vrbz736xM0Ei8FboP2uLogaPFsxKtPQngWeim5yZRO2RaxAO/xHSDB89P XkTGMo0/RpUXpXb7WoUXns+rOfFFRCO7h/2gl7bGjhwnWiGV3D2sezijx9WuA2w1Jt/a CzupOPOMrfuHY8ZQEdnVDVE42M3EIAy5igy+EYvX4pg2tmF0pfiKBTpMD5fXgRji9xdQ zgfbwECNvzUpPyxysrna2YZQWD08NMtLWzsV27d1PmKmhhNYlZJ5a90P9SeDWkgrnSu0 mH+K3kWq0PUpkhyUdm0yVRJLO3U+76JSDANCxgKwY9Jc/ENc1ArP8tQA60J9V8+cwJWq Fhdg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=fdV7LyyXXTcy9/RPkfyIJcAgv3nS4LdKyo30vJcQftY=; b=YIZle2lTVuW4U8BJ+EfTUkNsvmaSTCXxgOcvHTXXtn26hwrfuF982nRIKHsQ6m1YA2 gqCbVubAvH8M8kBGiGQMxjtRtt2QSd/xu2fgI7ly+OKR3o2w9GbQ7bQDkskXUNKpXoZ8 2ymCMLGGI0GVi4ktFZ5Hn36L7y9ip8U9sOY6FkuXUAzixb+W1gimXOoVoMLjU66Ra9eA juvf52IyiO7hMopTY0i5zqDtaX+c2gu2mson8yUDLaG7ibZlUXdQCDCJEa+/aPSIvz5U ThPjVhXUok1CHxgnfGedWI2ahcrFcTe6Is4kjDlsC7dxDMKcZCx66mRRpJBUAmF3nzjP FApQ== X-Gm-Message-State: AOAM532grIQrwhebtz9mLfmyMB14vdn0xnS0HAXRwU6WdblVAGNxahdb NDBQfRlxSKTkCFc6XZLU/k0HdST/zdGoAg== X-Google-Smtp-Source: ABdhPJzZFD3Peb72s/tnXlHFZCN1K7YVcjK4ZYnv9c7NOoOF3IQvoYWy9Ha3kZOaa2967O6YulwQTw== X-Received: by 2002:a5d:6daa:: with SMTP id u10mr1471598wrs.119.1621279580295; Mon, 17 May 2021 12:26:20 -0700 (PDT) Received: from localhost (host109-151-46-70.range109-151.btcentralplus.com. [109.151.46.70]) by smtp.gmail.com with ESMTPSA id t7sm11975678wrs.87.2021.05.17.12.26.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 May 2021 12:26:19 -0700 (PDT) Date: Mon, 17 May 2021 20:26:18 +0100 From: Andrew Burgess To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: add build mode to reverse order of _initialize function calls Message-ID: <20210517192618.GO3067949@embecosm.com> References: <20210517180003.2147187-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210517180003.2147187-1-simon.marchi@polymtl.ca> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 20:18:48 up 14 days, 8:13, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 May 2021 19:26:22 -0000 * Simon Marchi via Gdb-patches [2021-05-17 14:00:03 -0400]: > We should not assume any order in which the _initialize functions get > called. However, since their order comes from the order of source files > baked into the Makefile, it's possible for such dependencies to creep in > and for us not to notice. > > I think that a good way to test for such dependencies is to reverse the > order of the calls. Add an option in the Makefile to do this. It can > be used as: > > $ make REVERSE_INIT_FILES=1 > > before init.c gets generated (if you want to re-generate init.c, either > do a "make clean" or remove stamp-init). Instead of making this a build option, did you consider modifying initialize_all_files to something like this totally untested code: void initialize_all_files (bool reverse_p) { std::vector func = { _initialize_arc_linux_tdep, _initialize_arc_tdep, _initialize_arm_fbsd_tdep, _initialize_arm_linux_tdep, _initialize_arm_netbsd_tdep, _initialize_armobsd_tdep, ... etc ... }; if (reverse_p) std::reverse(func.begin(), func.end()); for (auto f : func) f (); } then all you'd need is to add a new command line flag which controls reverse_p. The benefit as I see it is that (a) we can add a test that is run in every run of the testsuite that starts GDB with this new command line flag, and does a few basic tests, and (b) we can easily pass the new command line flag to the testsuite from the command line, and so run _all_ tests with the reverse init order. Just a thought. Andrew > > Building with this mode finds one dependency. In symtab.c, we try to > add the alias "maintenance flush-symbol-cache" for the command > "maintenance flush symbol-cache". The add_alias_cmd that takes a target > command name is used, which requires a comman lookup to be done. At > this point, the "maintenance flush" prefix command hasn't been added > (it happens in maint.c), so the lookup fails. The add_alias_cmd doesn't > add an alias and returns NULL. That NULL is then passed to > deprecate_cmd, which causes a segfault. > > Fix that by using the add_alias_cmd overload that accepts a > cmd_list_element as the target. This doesn't care whether the prefix > has been created or not, since it doesn't need to do a command lookup. > > gdb/ChangeLog: > > * Makefile.in: Add option to reverse INIT_FILES. > * symtab.c (_initialize_symtab): Pass cmd_list_element to > add_alias_cmd. > > Change-Id: Id8ae7860c822da48455087ea50741c137f237922 > --- > gdb/Makefile.in | 17 +++++++++++++++++ > gdb/symtab.c | 6 +++--- > 2 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > index ba0cabb0216a..80de9334cd1f 100644 > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -1850,6 +1850,23 @@ INIT_FILES = \ > $(filter-out init.o version.o %_S.o %_U.o,\ > $(COMMON_OBS)))) > > +# This mode can be used to reverse the order in which _initialize functions > +# are called, to verify that there is no hidden dependencies between them. > +# > +# Use with: > +# > +# $ make REVERSE_INIT_FILES=1 > +# > +# _before_ init.c is generated. > + > +ifeq ($(REVERSE_INIT_FILES),1) > +INIT_FILES := \ > + $(shell \ > + for i in $(INIT_FILES); do \ > + echo $$i; \ > + done | tac) > +endif > + > init.c: stamp-init; @true > stamp-init: $(INIT_FILES) config.status > @$(ECHO_INIT_C) echo "Making init.c" > diff --git a/gdb/symtab.c b/gdb/symtab.c > index 9555f94707de..8ad319266772 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -6897,12 +6897,12 @@ If zero then the symbol cache is disabled."), > _("Print symbol cache statistics for each program space."), > &maintenanceprintlist); > > - add_cmd ("symbol-cache", class_maintenance, > + c = add_cmd ("symbol-cache", class_maintenance, > maintenance_flush_symbol_cache, > _("Flush the symbol cache for each program space."), > &maintenanceflushlist); > - c = add_alias_cmd ("flush-symbol-cache", "flush symbol-cache", > - class_maintenance, 0, &maintenancelist); > + c = add_alias_cmd ("flush-symbol-cache", c, class_maintenance, 0, > + &maintenancelist); > deprecate_cmd (c, "maintenancelist flush symbol-cache"); > > gdb::observers::executable_changed.attach (symtab_observer_executable_changed, > -- > 2.31.1 >