Re: [PATCH 9/9] [libbacktrace] Add printdwarftest_dwz_cmp.sh test-case

[Tom de Vries <tdevries@suse.de>]

On 19-01-19 01:54, Ian Lance Taylor wrote:
> On Fri, Jan 18, 2019 at 4:45 PM Tom de Vries <tdevries@suse.de> wrote:
>>
>> On 18-01-19 15:23, Ian Lance Taylor wrote:
>>> On Thu, Jan 17, 2019 at 5:59 AM Tom de Vries <tdevries@suse.de> 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