From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 97392 invoked by alias); 22 Jan 2019 21:43:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 97376 invoked by uid 89); 22 Jan 2019 21:43:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=realized, H*i:sk:CAKOQZ8, UD:la, 180119 X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 Jan 2019 21:43:19 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 10FF6AE22; Tue, 22 Jan 2019 21:43:17 +0000 (UTC) Subject: Re: [PATCH 9/9] [libbacktrace] Add printdwarftest_dwz_cmp.sh test-case To: Ian Lance Taylor Cc: gcc-patches , Ian Lance Taylor References: <20181211101411.7067-1-tdevries@suse.de> <20181211101411.7067-10-tdevries@suse.de> <994e3488-ac34-4aea-16f8-2195f0c7b2d0@suse.de> <08b4eea6-96d6-ee29-fe8b-aab008565995@suse.de> From: Tom de Vries Message-ID: Date: Tue, 22 Jan 2019 22:03:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg01327.txt.bz2 On 19-01-19 01:54, Ian Lance Taylor wrote: > On Fri, Jan 18, 2019 at 4:45 PM Tom de Vries wrote: >> >> On 18-01-19 15:23, Ian Lance Taylor wrote: >>> On Thu, Jan 17, 2019 at 5:59 AM Tom de Vries wrote: >>>> >>>> now that the rest of the patch series has been committed, here's an >>>> updated version of this patch that applies to trunk. >>> >>> I would much rather put dwarf_data into internal.h than to #include >>> "dwarf.c" from a different file. Using #include with a .c file is >>> just a bad path to walk down. >> >> This version avoids the include of dwarf.c. >> >> Does that look better? > >> +printdwarftest_SOURCES = >> +printdwarftest_LDADD = libbacktrace.la printdwarftest.lo testlib.lo > > Seems like you could write > > printdwarftest_SOURCES = printdwarftest.c testlib.c > printdwarftest_LDADD = libbacktrace.la > That's what I had initially, but I realized that that makes it harder to keep dependencies correct. That is, now I've added the dependencies: ... +printdwarftest.lo: config.h backtrace.h internal.h testlib.h +testlib.lo: $(INCDIR)/filenames.h backtrace.h testlib.h ... and that works for "printdwarftest_LDADD = ... printdwarftest.lo testlib.lo". When doing this instead: ... printdwarftest_SOURCES = printdwarftest.c testlib.c printdwarftest_LDADD = libbacktrace.la ... no printdwarftest.lo or testlib.lo is generated. So, I could rewrite the dependencies to: ... +printdwarftest.o: config.h backtrace.h internal.h testlib.h +printdwarftest.obj: config.h backtrace.h internal.h testlib.h +testlib.o: $(INCDIR)/filenames.h backtrace.h testlib.h +testlib.obj: $(INCDIR)/filenames.h backtrace.h testlib.h ... but that looks somewhat fragile, because when adding: ... +printdwarftest_CFLAGS = -fno-tree-tail-merge ... we need to rewrite the dependencies to: ... +printdwarftest-printdwarftest.o: config.h backtrace.h internal.h testlib.h +printdwarftest-printdwarftest.obj: config.h backtrace.h internal.h testlib.h +printdwarftest-testlib.o: $(INCDIR)/filenames.h backtrace.h testlib.h +printdwarftest-testlib.obj: $(INCDIR)/filenames.h backtrace.h testlib.h ... >> -static int >> +int >> dwarf_lookup_pc (struct backtrace_state *state, struct dwarf_data *ddata, > > Ah, I didn't consider this. We can't do this. It will break code > like libsanitizer/libbacktrace/backtrace-rename.h. > > Is there a way that we could run a similar test looking at the output > of readelf --debug? No, not really. We're not interested in the contents of the debug information as such. We're interested in the representation of that information that libbacktrace builds from it, and the test compares that representation with and without dwz, to make sure no information got lost (which could indicate a dwz bug, or a lacking feature in libbacktrace). With a normal backtrace test, you just ask for information about a couple of locations. With this test, you ask for all the information, giving libbacktrace a probing you just can't get with a normal test, so I think it's a good idea to have it. What is an acceptable way to proceed here? I could add a libbacktrace_nodwarf.la, and have the test-case add a -DFOR_TESTING or some such when compiling dwarf.c, and add the necessary handling in dwarf.c conditional on FOR_TESTING. WDYT? Thanks, - Tom