From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x932.google.com (mail-ua1-x932.google.com [IPv6:2607:f8b0:4864:20::932]) by sourceware.org (Postfix) with ESMTPS id 743D23890434 for ; Fri, 16 Apr 2021 12:04:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 743D23890434 Received: by mail-ua1-x932.google.com with SMTP id v23so8463990uaq.13 for ; Fri, 16 Apr 2021 05:04:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=1KTcx41PTaKRKLHDxZ7xYPhQ6hNFcbZK8rGVlCtrksQ=; b=MZ/v38eN7LxLXJEC9+LX12pbz/zduaVhNb9JQchTPbYMrJoAGDGSukRmXKLzaThr/J S0mSC237PmSWaSkdm8yrPrBCTnwADJMJHLhcOIWhi4rlDftDLEleU+XA82a0ez1VbINU 3kjDEaNhZ26E3kjHB25w7ab1qzB1Eg+DBhK6HqpdqSGg8SXN5r5mZkvA+H9UXQ2WF3J2 WGacVElmA52oC/2ZOyQY1YubN0W8rHCn50QpOG5Yq1bbieJ00HlTSyxsNlYPxkd4SU5p az4daQLRLvQuZqMSrrPqdDpMb8QOReHfjX5adQAaQAiwCXb9UZNPMMj2jTdnGD652HPT HGXg== X-Gm-Message-State: AOAM533omLukVw3xP4ZrH73cgZC99IAUiyhrBb13b4NrJNi8TexLteOt lpGIGyfL3lSqqdu9GEpMGhvp3CM49Zd01KKiDIDHuw== X-Google-Smtp-Source: ABdhPJwv7k17fvmd+MHqow3V32bB0qvKN4M2TbYdbyhHuXjgvmeya/KG6lXMtglmICShURvtA81htvaOVP9UqvmE8HU= X-Received: by 2002:ab0:3570:: with SMTP id e16mr5968542uaa.13.1618574670664; Fri, 16 Apr 2021 05:04:30 -0700 (PDT) MIME-Version: 1.0 References: <20210331092352.619148-1-gprocida@google.com> <20210331170454.951900-1-gprocida@google.com> <8735w3vdkv.fsf@seketeli.org> <87o8eftz5v.fsf_-_@seketeli.org> In-Reply-To: <87o8eftz5v.fsf_-_@seketeli.org> From: Giuliano Procida Date: Fri, 16 Apr 2021 13:03:53 +0100 Message-ID: Subject: Re: [PATCH v2, v4] XML reader: improve XML node traversal To: Dodji Seketeli Cc: Giuliano Procida via Libabigail , kernel-team@android.com, =?UTF-8?Q?Matthias_M=C3=A4nnich?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-28.0 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL 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: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Apr 2021 12:04:42 -0000 Hi Dodji. On Thu, 15 Apr 2021 at 15:12, Dodji Seketeli wrote: > > Giuliano Procida a =C3=A9crit: > > > Hi. > > > > Thanks for reviewing. > > > > On Tue, 6 Apr 2021 at 12:48, Dodji Seketeli wrote: > > > >> Hello Giuliano, > >> > >> Giuliano Procida a =C3=A9crit: > >> > >> [...] > >> > >> Thanks for looking into this. > >> > >> Sadly, as several other patches got in before I got to review this, th= is > >> patch doesn't apply to the current state of master anymore. So overal= l, > >> I think it would need a rebasing. Sorry about that :-( > >> > >> > > No problem. So long as the public master is the same as your working > > master, I can easily rebase and resend. My v2 posting reflects the > > current public master. > > Thanks. I have seen the v4 posting too. This message is also a reply > to that post. > > [...] > > >> > The XML reader uses libxml's Reader API for parsing XML and traverse= s > >> > high-level XML nodes using reader cursor advancement. Low-level node= s > >> > are read into complete node trees and are traversed following > >> > children, next and (in one instance) parent pointers. Some > >> > intermediate-level nodes can be parsed both ways. > >> > > >> > A lot of code cares about node types, typically skipping over > >> > non-element nodes. Unfortunately, in a few places, the tracked "corp= us > >> > node" needs to land on text (whitespace) within elements for the XML > >> > reader to work; stripping text nodes out causes some elements to be > >> > skipped. > >> > >> Can you please give an example of this? It doesn't seem obvious to me= . > >> > > > > Of course. The contributed test case serves as an example (it just has = all > > unnecessary space stripped out). It wasn't obvious to me either! > > Sorry, I wasn't clear enough. I guess my main point is that in this > introductory text, I find it important to be precise about what the > actual issue is. Yes I can dig into the information around and > understand it, but having it clearly stated in the commit log does have > real value for people trying to understand what is going on. > >From my perspective, the issues I was trying to highlight in the commit text above were: 1. the parsing uses two different libxml APIs and sometimes both conditionally in the same function whereas using the Tree API consistently would have simplified the code; there is no direct commentary on this in the code, but I guessed it was probably a performance (memory) optimisation for very large ABIs 2. the repeated checks for XML_ELEMENT_NODE clutter the code (though any performance impact is likely negligible) and there were several variations of the looping schemes for iterating over node children 1. and 2. made it harder for me to a) find the cause of the "significant whitespace" issue and b) be sure no other similar issues might be lurking. In fact, simplifying the traversals helped me to reason about the remaining code. > > No problem. I dug into it and I think I have understood what is going > on. I'll update the commit log accordingly. > > > I'm sure there is a much smaller fix that could be made to address each > > case of sensitivity to whitespace. However, the main contribution here = are > > the new primitives which make, I hope, the parsing code easier to read = and > > understand and harder to hide bugs in. > The above comment was to reflect that I saw two benefits from my changes: a) fixing the "significant whitespace" issue and b) reducing the chance that future XML reader changes would introduce similarly subtle issues. In fact, by rigorously changing all the loops I could to the same form, it became apparent that the XML reader was discarding the contents of certain elements and I added code comments in my commit to highlight this non-obvious behaviour. > > After digging into all this, I think the issue can indeed be addressed > by a smaller fix overall. I don't think an almost re-write like this is > necessary. By default I tend to be reluctant of re-writes in general, > unless the original code really doesn't cut if for what we want to do. > I've accepted re-writes, so I it's really not a religious position. In > any case, I'd rather try to understand and address the root cause of the > particular issue at hand. > I only changed the looping primitives and logic. I didn't attempt to do anything about other things that made it hard for me to understand the code (which were the storing of a node in the context object and the use of two different APIs). While most of the changes were mechanical, the remaining ones pointed to interesting subtleties. The test suite and the extra testing I did gave me confidence that my changes were correct. There is one thing I might have been able to do to improve the code review process; I could have tried to split the fix and the looping changes into separate commits. I did consider this, but I wasn't sure what order I would have done them in; the looping changes might have forced the fix; the fix before the looping changes might be a completely different thing. > > I do appreciate all the time and effort you did put into this, and I > thank you for that. I honestly am not comfortable doing this, but I > really think it's better to keep what's in there and fix the root cause > of issues instead. What is there has been tested and is consistent. > Thank you in turn for going through things carefully. Besides the code itself, I think it's important we understand each other's priorities when it comes to coding and this dialogue helps us align. Generally, my priorities are correctness, simplicity, features, performance (and usually in that order). I've worked in environments where mistakes can result (and have resulted) in large financial losses and reputational damage and that might be a contributing factor. The longer I've programmed, the more I've come to appreciate simplicity and this usually translates into spending time finding abstractions that allow software to be expressed in the clearest fashion, without sacrificing correctness, features or much performance. I've also worked in a role where performance was everything and I achieved significant gains by simplifying APIs. Some of the more elaborate code in the XML reader may be needed for performance reasons. However, by building on primitives which guarantee certain invariants, we can reason on top of them, instead of finding ourselves reasoning everything from scratch. To be concrete, this is the distinction between traversing a tree of arbitrary nodes and traversing just the elements. I believe this choice of abstraction and simplification has real value. And full disclosure, I've since found (a day or so ago) that the libxml Tree API already defines xmlFirstElementChild and xmlNextElementSibling which are essentially identical to the primitives I defined. This also gives me a boost in confidence that they are the right primitives for traversing the XML tree for libabigail. > > When we find issues, if the foundations appear to not be adequate to > serve what we want to do, then I am fine with coming up with something > different and new, as long as it integrates well (in its spirit and > form) with the rest. Otherwise, I'd rather make what we have evolve. > We are all busy people, and if something is working, I will be spending my time on something that is not. However, my attention was directed to XML parser by the issue in question and I set out with the intention of resolving the issue and only changed the loop primitives as a means to an end. > > So I am posting a candidate patch that builds on your work and that > should address the issues your raised in > https://sourceware.org/bugzilla/show_bug.cgi?id=3D27616. > I was going to reply separately to review, but the commit below is quite short. I'm sure it addresses the issue of significant whitespace. If you haven't done so already, please test by removing all whitespace from all the test XML files and then running abidiff vs the originals. There is a short recipe for doing this in the bug report. Best regards, Giuliano. > > [...] > > > Please see the test case. We'd like to have confidence in the semantics= of > > libabigail XML, independent of the current reader and writer > > implementations. Transformations that do not affect the XML element > > structure should not change XML meaning, but do at present. > > Well, I don't disagree with the intent. There are issues in the > current interpretation of the abixml and they ought to be fixed. And > this is hopefully happening right now. > > [...] > > I am proposing this evolutive fix that should address the issue you > raised. > > From 94d0194819a34a4972a1575da93a77fe5e19a010 Mon Sep 17 00:00:00 2001 > From: Dodji Seketeli > Date: Wed, 14 Apr 2021 15:04:35 +0200 > Subject: [PATCH] reader: Handle 'abi-corpus' element being possibly empty > > The abixml reader wrongly assumes that the 'abi-corpus' element is > always non-empty. Note that until now, the only emitter of abixml > consumed in practice was abg-writer.cc and it only emits non-empty > 'abi-corpus' elements. So the issue wasn't exposed. > > So, the reader assumes that an 'abi-corpus' element has at least a > text node. > > For instance, consider this minimal input file named test-v0.abi: > > $cat test-v0.abi > > > > > > > $ > > Now, compare it to this file where the abi-corpus element is an empty > element (doesn't even contain any text): > > $cat test-v0.abi > > > > > > $ > > comparing the two files with abidiff (wrongly) reports: > > $ abidiff test-v0.abi test-v1.abi > ELF architecture changed > Functions changes summary: 0 Removed, 0 Changed, 0 Added function > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable > > architecture changed from 'elf-arm-aarch64' to '' > $ > > What's happening is that read_corpus_from_input is getting out early > when it sees that the node is empty. This is at: > > xmlNodePtr node =3D ctxt.get_corpus_node(); > @@ -1907,10 +1925,14 @@ read_corpus_from_input(read_context& ctxt) > corp.set_soname(reinterpret_cast(soname_str.get())); > } > > if (!node->children) // <---- we get out early here and we > return nil; // forget about the properties of > // the current empty corpus element node > > So, at its core, fixing the issue at hand involves avoiding the early > return there. > > But then, it turns out that's not enough. > > In the current setting, the different abixml processing entry points > are designed to be used in a semi "streaming" mode. > > So for instance, read_translation_unit_from_input can be invoked > repeatedly to "stream in" the next translation unit at each > invocation. > > Alternatively, the lower level xmlTextReaderNext can be used to > iterate over XML node until we reach the translation unit XML element > we are interested in. At that point xmlTextReaderExpand can be used > to expand the XML node, then we let the context know that this is > the current node of the corpus that needs to be processed, using > read_context::get_corpus_node. Once we've done that, > read_translation_unit_from_input can be called to process that > particular corpus node. Note that the corpus node at hand, that needs > to be processed will be retrieved by read_context::get_corpus_node. > > These two modes of operation are also available for > read_corpus_from_input, read_symbol_db_from_input, > read_elf_needed_from_input etc. > > Today, these functions all assume that the current node returned by > read_context::get_corpus_node is the node /before/ the node of the > corpus to be processed. So they all start looking at the /next sibling/ > of the node returned by read_context::get_corpus_node. So the code > was implicitly assuming that read_context::get_corpus_node was > pointing to a text node that was before the node of the corpus that we > want to process. > > This is wrong. read_context::get_corpus_node should just return the > current node of the corpus that needs to be processed and voila. > > And so read_context::set_corpus_node should be used to set the current > node of the corpus to the current element node that needs to be processed= . > > That's the spirit of the change done by this patch. > > As its name suggests, the existing > xml::advance_to_next_sibling_element is used to skip non element xml > nodes (including text nodes) and move to the next element node to > process, which is set to the context using > read_context::set_corpus_node. > > Then the actual processing functions like read_corpus_from_input get > the node to process, using read_context::get_corpus_node and process > it rather than processing the sibling node that comes after it. > > The other changes are either to prevent related crashes that I noticed > while doing various tests, update the abilint tool used to read and > debug abixml input files and add better documentation. > > * src/abg-reader.cc (read_context::get_corpus_node): Add comment > to this member function. > (read_translation_unit_from_input, read_symbol_db_from_input) > (read_elf_needed_from_input): Start processing the current node o= f > the corpus that needs to be processed rather than its next > sibling. Once the processing is done, set the new "current node > of the corpus to be processed" properly by skipping to the next > element node to be processed. > (read_corpus_from_input): Don't get out early when the > 'abi-corpus' element is empty. If, however, it has children node= , > skip to the first child element and flag it -- using > read_context::set_corpus_node -- as being the element node to be > processed by the processing facilities of the reader. If we are > in a mode where we called xmlTextReaderExpand ourselves to get th= e > node to process, then it means we need to free that node > indirectly by calling xmlTextReaderNext. In that case, that node > should not be flagged by read_context::set_corpus_node. Add more > comments. > * src/abg-corpus.cc (corpus::is_empty): Do not crash when no > symtab is around. > * src/abg-libxml-utils.cc (go_to_next_sibling_element_or_stay): > Fix typo in comment. > (advance_to_next_sibling_element): Don't crash when given a nil > node. > * tests/data/test-abidiff/test-PR27616-squished-v0.abi: Add new > test input. > * tests/data/test-abidiff/test-PR27616-squished-v1.abi: Likewise. > * tests/data/test-abidiff/test-PR27616-v0.xml: Likewise. > * tests/data/test-abidiff/test-PR27616-v1.xml: Likewise. > * tests/data/Makefile.am: Add the new test inputs above to source > distribution. > * tests/test-abidiff.cc (specs): Add the new tests inputs above t= o > this harness. > * tools/abilint.cc (main): Support writing corpus groups. > > Signed-off-by: Dodji Seketeli > --- > src/abg-corpus.cc | 2 +- > src/abg-libxml-utils.cc | 6 +- > src/abg-reader.cc | 65 +++++++++++++------ > tests/data/Makefile.am | 4 ++ > .../test-abidiff/test-PR27616-squished-v0.abi | 43 ++++++++++++ > .../test-abidiff/test-PR27616-squished-v1.abi | 1 + > tests/data/test-abidiff/test-PR27616-v0.xml | 4 ++ > tests/data/test-abidiff/test-PR27616-v1.xml | 3 + > tests/test-abidiff.cc | 12 ++++ > tools/abilint.cc | 9 ++- > 10 files changed, 126 insertions(+), 23 deletions(-) > create mode 100644 tests/data/test-abidiff/test-PR27616-squished-v0.abi > create mode 100644 tests/data/test-abidiff/test-PR27616-squished-v1.abi > create mode 100644 tests/data/test-abidiff/test-PR27616-v0.xml > create mode 100644 tests/data/test-abidiff/test-PR27616-v1.xml > > diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc > index c9f5c56b..0c684a93 100644 > --- a/src/abg-corpus.cc > +++ b/src/abg-corpus.cc > @@ -1009,7 +1009,7 @@ corpus::is_empty() const > } > } > return (members_empty > - && !get_symtab()->has_symbols() > + && (!get_symtab() || !get_symtab()->has_symbols()) > && priv_->soname.empty() > && priv_->needed.empty()); > } > diff --git a/src/abg-libxml-utils.cc b/src/abg-libxml-utils.cc > index a1acff1f..6b55d3ed 100644 > --- a/src/abg-libxml-utils.cc > +++ b/src/abg-libxml-utils.cc > @@ -413,7 +413,8 @@ unescape_xml_comment(const std::string& str) > return result; > } > > -/// Maybe get the next sibling element node of an XML node, or stay to t= he sam > +/// Maybe get the next sibling element node of an XML node, or stay to > +/// the same. > /// > /// If there is no next sibling xml element node, the function returns > /// the initial node. > @@ -443,6 +444,9 @@ go_to_next_sibling_element_or_stay(xmlNodePtr node) > xmlNodePtr > advance_to_next_sibling_element(xmlNodePtr node) > { > + if (!node) > + return 0; > + > xmlNodePtr n =3D go_to_next_sibling_element_or_stay(node->next); > if (n =3D=3D 0 || n->type !=3D XML_ELEMENT_NODE) > return 0; > diff --git a/src/abg-reader.cc b/src/abg-reader.cc > index f40e3d22..d2c76a38 100644 > --- a/src/abg-reader.cc > +++ b/src/abg-reader.cc > @@ -196,10 +196,20 @@ public: > get_reader() const > {return m_reader;} > > + /// Getter of the current XML node in the corpus element sub-tree > + /// that needs to be processed. > + /// > + /// @return the current XML node in the corpus element sub-tree that > + /// needs to be processed. > xmlNodePtr > get_corpus_node() const > {return m_corp_node;} > > + /// Setter of the current XML node in the corpus element sub-tree > + /// that needs to be processed. > + /// > + /// @param node set the current XML node in the corpus element > + /// sub-tree that needs to be processed. > void > set_corpus_node(xmlNodePtr node) > {m_corp_node =3D node;} > @@ -1485,7 +1495,7 @@ read_translation_unit_from_input(read_context& c= txt) > else > { > node =3D 0; > - for (xmlNodePtr n =3D ctxt.get_corpus_node()->next; n; n =3D n->ne= xt) > + for (xmlNodePtr n =3D ctxt.get_corpus_node(); n; n =3D n->next) > { > if (!n > || n->type !=3D XML_ELEMENT_NODE) > @@ -1501,15 +1511,17 @@ read_translation_unit_from_input(read_context& c= txt) > return nil; > > tu =3D get_or_read_and_add_translation_unit(ctxt, node); > - // So read_translation_unit() can trigger (under the hood) reading > - // from several translation units just because > - // read_context::get_scope_for_node() has been called. In that > - // case, after that unexpected call to read_translation_unit(), the > - // current corpus node of the context is going to point to that > - // translation unit that has been read under the hood. Let's set > - // the corpus node to the one we initially called > - // read_translation_unit() on here. > - ctxt.set_corpus_node(node); > + > + if (ctxt.get_corpus_node()) > + { > + // We are not in the mode where the current corpus node came > + // from a local invocation of xmlTextReaderExpand. So let's set > + // ctxt.get_corpus_node to the next child element node of the > + // corpus that needs to be processed. > + node =3D xml::advance_to_next_sibling_element(node); > + ctxt.set_corpus_node(node); > + } > + > return tu; > } > > @@ -1583,7 +1595,7 @@ read_symbol_db_from_input(read_context& = ctxt, > xmlTextReaderNext(reader.get()); > } > else > - for (xmlNodePtr n =3D ctxt.get_corpus_node()->next; n; n =3D n->next= ) > + for (xmlNodePtr n =3D ctxt.get_corpus_node(); n; n =3D n->next) > { > if (!n || n->type !=3D XML_ELEMENT_NODE) > continue; > @@ -1594,8 +1606,11 @@ read_symbol_db_from_input(read_context& = ctxt, > else if (xmlStrEqual(n->name, BAD_CAST("elf-variable-symbols"))) > has_var_syms =3D true; > else > - break; > - ctxt.set_corpus_node(n); > + { > + ctxt.set_corpus_node(n); > + break; > + } > + > if (has_fn_syms) > { > fn_symdb =3D build_elf_symbol_db(ctxt, n, true); > @@ -1688,7 +1703,7 @@ read_elf_needed_from_input(read_context& ctxt, > } > else > { > - for (xmlNodePtr n =3D ctxt.get_corpus_node()->next; n; n =3D n->ne= xt) > + for (xmlNodePtr n =3D ctxt.get_corpus_node(); n; n =3D n->next) > { > if (!n || n->type !=3D XML_ELEMENT_NODE) > continue; > @@ -1703,6 +1718,7 @@ read_elf_needed_from_input(read_context& ctxt, > if (node) > { > result =3D build_needed(node, needed); > + node =3D xml::advance_to_next_sibling_element(node); > ctxt.set_corpus_node(node); > } > > @@ -1806,6 +1822,8 @@ read_corpus_from_input(read_context& ctxt) > if (!reader) > return nil; > > + // This is to remember to call xmlTextReaderNext if we ever call > + // xmlTextReaderExpand. > bool call_reader_next =3D false; > > xmlNodePtr node =3D ctxt.get_corpus_node(); > @@ -1907,10 +1925,14 @@ read_corpus_from_input(read_context& ctxt) > corp.set_soname(reinterpret_cast(soname_str.get())); > } > > - if (!node->children) > - return nil; > - > - ctxt.set_corpus_node(node->children); > + // If the corpus element node has children nodes, make > + // ctxt.get_corpus_node() returns the first child element node of > + // the corpus element that *needs* to be processed. > + if (node->children) > + { > + xmlNodePtr n =3D xml::advance_to_next_sibling_element(node->childr= en); > + ctxt.set_corpus_node(n); > + } > > corpus& corp =3D *ctxt.get_corpus(); > > @@ -1966,6 +1988,10 @@ read_corpus_from_input(read_context& ctxt) > // This is the necessary counter-part of the xmlTextReaderExpand() > // call at the beginning of the function. > xmlTextReaderNext(reader.get()); > + // The call above invalidates the xml node returned by > + // xmlTextReaderExpand, which is can still be accessed via > + // ctxt.set_corpus_node. > + ctxt.set_corpus_node(0); > } > else > { > @@ -1974,7 +2000,8 @@ read_corpus_from_input(read_context& ctxt) > if (!node) > { > node =3D ctxt.get_corpus_node(); > - node =3D xml::advance_to_next_sibling_element(node->parent); > + if (node) > + node =3D xml::advance_to_next_sibling_element(node->parent); > } > ctxt.set_corpus_node(node); > } > diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am > index 576309e8..0edfe26c 100644 > --- a/tests/data/Makefile.am > +++ b/tests/data/Makefile.am > @@ -206,6 +206,10 @@ test-abidiff-exit/test-crc-v1.abi \ > test-abidiff-exit/test-missing-alias-report.txt \ > test-abidiff-exit/test-missing-alias.abi \ > test-abidiff-exit/test-missing-alias.suppr \ > +test-abidiff/test-PR27616-squished-v0.abi \ > +test-abidiff/test-PR27616-squished-v1.abi \ > +test-abidiff/test-PR27616-v0.xml \ > +test-abidiff/test-PR27616-v1.xml \ > \ > test-diff-dwarf/test0-v0.cc \ > test-diff-dwarf/test0-v0.o \ > diff --git a/tests/data/test-abidiff/test-PR27616-squished-v0.abi b/tests= /data/test-abidiff/test-PR27616-squished-v0.abi > new file mode 100644 > index 00000000..6b3d0460 > --- /dev/null > +++ b/tests/data/test-abidiff/test-PR27616-squished-v0.abi > @@ -0,0 +1,43 @@ > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > diff --git a/tests/data/test-abidiff/test-PR27616-squished-v1.abi b/tests= /data/test-abidiff/test-PR27616-squished-v1.abi > new file mode 100644 > index 00000000..20495f00 > --- /dev/null > +++ b/tests/data/test-abidiff/test-PR27616-squished-v1.abi > @@ -0,0 +1 @@ > += <= abi-instr address-size=3D'64' path=3D'test6.cc' comp-dir-path=3D'/home/skum= ari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf' language= =3D'LANG_C_plus_plus'>= > diff --git a/tests/data/test-abidiff/test-PR27616-v0.xml b/tests/data/tes= t-abidiff/test-PR27616-v0.xml > new file mode 100644 > index 00000000..5e025ff6 > --- /dev/null > +++ b/tests/data/test-abidiff/test-PR27616-v0.xml > @@ -0,0 +1,4 @@ > + > + > + > + > diff --git a/tests/data/test-abidiff/test-PR27616-v1.xml b/tests/data/tes= t-abidiff/test-PR27616-v1.xml > new file mode 100644 > index 00000000..aa8059bf > --- /dev/null > +++ b/tests/data/test-abidiff/test-PR27616-v1.xml > @@ -0,0 +1,3 @@ > + > + > + > diff --git a/tests/test-abidiff.cc b/tests/test-abidiff.cc > index 3452b7ee..6ef18654 100644 > --- a/tests/test-abidiff.cc > +++ b/tests/test-abidiff.cc > @@ -128,6 +128,18 @@ static InOutSpec specs[] =3D > "data/test-abidiff/test-crc-report.txt", > "output/test-abidiff/test-crc-report-1-2.txt" > }, > + { > + "data/test-abidiff/test-PR27616-v0.xml", > + "data/test-abidiff/test-PR27616-v1.xml", > + "data/test-abidiff/empty-report.txt", > + "output/test-abidiff/empty-report.txt" > + }, > + { > + "data/test-abidiff/test-PR27616-squished-v0.abi", > + "data/test-abidiff/test-PR27616-squished-v1.abi", > + "data/test-abidiff/empty-report.txt", > + "output/test-abidiff/empty-report.txt" > + }, > // This should be the last entry. > {0, 0, 0, 0} > }; > diff --git a/tools/abilint.cc b/tools/abilint.cc > index c8598465..856f935d 100644 > --- a/tools/abilint.cc > +++ b/tools/abilint.cc > @@ -440,11 +440,16 @@ main(int argc, char* argv[]) > else > { > if (type =3D=3D abigail::tools_utils::FILE_TYPE_XML_CORPUS > - ||type =3D=3D abigail::tools_utils::FILE_TYPE_XML_CORPUS_GR= OUP > + || type =3D=3D abigail::tools_utils::FILE_TYPE_XML_CORPUS_G= ROUP > || type =3D=3D abigail::tools_utils::FILE_TYPE_ELF) > { > if (!opts.noout) > - is_ok =3D write_corpus(*ctxt, corp, 0); > + { > + if (corp) > + is_ok =3D write_corpus(*ctxt, corp, 0); > + else if (group) > + is_ok =3D write_corpus_group(*ctxt, group, 0); > + } > } > } > > -- > 2.30.0 > > > > -- > Dodji