Re: [PATCH v2] Add regression tests for ctf reading

[Guillermo Martinez <guillermo.e.martinez@oracle.com>]

On Thursday, November 25, 2021 4:40:29 AM CST Dodji Seketeli wrote:
> Guillermo Martinez via Libabigail <libabigail@sourceware.org> a écrit:
> 
> [...]
> 
> > On Tuesday, November 23, 2021 9:48:35 AM CST Jose E. Marchesi wrote:
> >
> > Hello Jose/everybody
> >
> > Thanks for your comments!, answers below:
> >
> > The following items summarise steps currently implemented in
> > the testsuite for DWARF and CTF readers:
> >
> > 1) Create corpus using the ELF input file, e.g:
> >       *src*/libabigail/tests/data/test-read-ctf/test0
> >      - In this step the SUTs are: *the front-end and readers*.
> >
> > 2) Serialize the corpus object to XML ABI description in output
> >     directory:
> >      *build*/libabigail_x86_64/tests/output/test-read-ctf/test0.abi
> >      - SUT: *writter* using write_corpus function
> 
> Right.
> 
> > (it doesn't use libxml (IMHO something to change if we want to use in
> > the future properties, name space, etc).
> 
> I am not sure about this, but hey, the future will tell :-)
> 
> >
> > 3) Spawn *abidw* tool with ELF input file using --abidiff argument, e.g:
> >      abidw --abidiff  --ctf *src*/libabigail/tests/data/test-read-ctf/test0
> >
> >     Internally abiw works as follow:
> >      
> >    3.1) Create corpus from ELF input file, e.g:
> >             *src*/libabigail/tests/data/test-read-ctf/test0
> >     - In this step SUTs: *DWARF/CTF frond-end readers*
> >
> >    3.2) Serialize the (*first*) corpus object to XML ABI description in 
> >           *temp* directory:
> >              /tmp/libbigail-xyz
> >     - SUTs: ABI writter,
> >
> >    3.3) Build a (*second*) corpus from this temporary file (*same file!*):
> >         - SUTs: *XML reader*.
> >
> >    3.4) Compute the corpus differences
> >               compute_diff(corp, corp2 ..)
> >      - SUTs: *comparison algorithm* (diff_context/corpus diff)
> >
> >        if there are differences return 1 otherwise return 0.
> >
> 
> Right.
> 
> > 4) The return value is read by test-read-ctf and if it's 1 test
> >     is marked as *FAILED*
> >
> > 5) Use a external *diff command* to compare the XML abi input file with
> >     the file outputted in the step 2. The return code of the diff command
> >     is used to mark the test as SUCCESS or FAILED
> >
> > So, the abidw doesn't use the expected XML file used in the testsuite, so
> > if there are changes on the input ELF or in the libabigail subsystems (readers,
> > writer, corpus, etc) we could get false negatives, because it is working with
> > itself result as an incoming file, instead of use an expected file.
> 
> Actually, this won't be a false negative.  It's a true negative. because
> what we want to see with abidw --abidiff is that the abixml file that is
> emitted has the same *ABI* as the incoming ELF file.  In other words, we
> want to see if the DWARF/CTF, the in-memory IR, and the abixml are all
> coherent, so to speak.  The exact output details of the XML itself
> (things like spaces or order of type definitions etc) don't matter to us
> at that point.  We want those insignificant details of the abixml format
> to be able to fluctuate without having the fundamentals of ABI
> compatibility in general to be impacted.
hmm, in order to probe my assumption of the use of abidw --abidiff in test-read-*
to protect against false negatives I've commented in src/abg-dwarf-reader.cc
the call to ctxt.load_elf_properties, simulating a broken feature(no elf-needed node
in abixml) in dwarf_reader::read_corpus_from_elf so the /tmp/abixml hasn't 
elf-needed node:

<abi-corpus version='2.1' path='/home/byby/oracle/src/libabigail-upstream/tests/data/test-read-dwarf/test0'>
  <elf-function-symbols>
    <elf-symbol name='_ZN3ns03barEiz' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
 ....

after that, as we already know this /tmp/abixml is used to built the second corpus used
by compute_diff function, and as you can guess: no differences will be found. So regressions could
not be detected, because we don't used the expected abixml file: test0.abi, is there where the elf-needed
node was written for example: in a previous version of libabigail.

  <elf-needed>
    <dependency name='libstdc++.so.6'/>
    <dependency name='libm.so.6'/>
    <dependency name='libgcc_s.so.1'/>
    <dependency name='libc.so.6'/>
  </elf-needed>

> However, regardless of abidw --abidiff (which cares about stability of
> the ABI representation across DWARF/CTF, in-memory IR and abixml), the
> test-read-dwarf.cc harness also wants to detect minutes changes to the
> abixml that is emitted by abidw. 
hmm, /tmp/abixml is not used by test-read-dwarf.cc.
> This is so that developers are forced
> to inspect abixml changes that are due to source code modification of
> libabigail before submitting their changes.
 Ok, agree.
> This is why we use "diff" to compare the emitted abixml against the
> expected/reference one If the emitted one is different because of a new
> source code change, but the ABI is stable, we'll just accept that new
> change and promote the new abidw output as being the new
> reference/expected output.
Yes, but those differences found by "diff" could be there and it doesn't mean
changes in the libabigail source code nor differences between the abixml
file generated from the ELF input object and the expected abixml file. 
I found weird behaviour running CTF testsuite (test-PR26568-1-ctf.o.abi):

https://sourceware.org/pipermail/libabigail/2021q4/003893.html

> > I think that we should use the ABI XML reader from the expected file (e.g:
> > tests/data/test-read-ctf/test0.abi) to build the corpus to be compared
> > with the corpus built with ELF input file (e.g: tests/data/test-read-ctf/test0)
> > and in this way replace the external abidw and diff command calls,
> 
> Heh.  This is exactly what abidw --abidiff does.  And we want to use
> abidw too to, so that the *tool* is tested too.
> this also
> 
> But as I have explained above, we also want to use 'diff' for a
> different level of testing.
> 
> [...]
> 
> > helps to avoid false positives when XML ABIs files has the same nodes but not
> > in the same order.
> 
> 
> [...]
> 
> 
> On Tuesday, November 23, 2021 9:48:35 AM CST Jose E. Marchesi wrote
> 
> >> 2) I am surprised by the fact the CTF reader seems to be working as good
> >>    as the DWARF reader re. the testsuite.
> 
> Well, we don't know that ;-)
> 
> To know that, it's needed to *inspect* the result of abidw --ctf on the
> binaries and compare it to what we see in the source code of the binary.
This is exactly what I did :-)  and also I execute abidw  using same source
code but compiled with -g option and comparing common information
in the nodes: elf-symbol, *-decl, function-type, data-member, etc. and
common nodes properties: name, path, size*, id, etc. in ~19 test sent in
patch v2 they looks good! (those files now are the expected result for
CTF testsuite). 

The pending work (in which I'm doing progress) is translate the valid CTF
testcases from tests/data/test-read-dwarf/*cc files to c files and add to
testusite, also I'm planning to extract other test from the CTF specs:
   http://www.esperi.org.uk/~oranix/ctf/ctf-spec.pdf

> We can also introduce new ctf binaries for abidiff-based tests and
> compare their output to their DWARF counter part.
Ok,
> There are going to be differences, I am quite sure of that.  But at
> least we'll see what they are.  I am not afraid, though.  I think the
> CTF support is super cleanly done.  If there are issues, we'll tackle
> them.
> 
> This testing work is a critical part of that work.  Many thanks to
> Guillermo for handling it!
No, thanks to you Dodji for your comments, they are very useful to follow the
right direction in the CTF test harness! :-).  
> [...]
> 
> Cheers,

Guillermo