Surprising behavior of suppress_type drop = yes

Surprising behavior of suppress_type drop = yes

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 4477 bytes --]

Hi,

In my project we have a lot of large private structures that the user
can never access directly, they will always be referenced through
opaque pointers. And we don't guarantee the data layout of those
private data structures. So we suppress them with [suppress_type]
source_location_not_in = list.h, of.h, public.h, headers.h.

Since they are not necessary and they make the xml abi files pretty big
we also remove them from the abi files with drop = yes.

But now we cannot use any opaque handles anymore since any function
parameter that uses a pointer to an opaque typedef/structure will also
be dropped from the abi. This is highly confusing (at least to me). I
had assumed they would just be treated as void pointers instead.

As an example of what I had hoped would work consider the foolib
project as attached. The initial version uses void pointers. But a new
version adds some type safety to the library by replacing the void
pointers by opaque Foo pointers. This should be abi compatible, but
gives the following surprising abi diff:

$ make
gcc -fPIC -g -O2 -Wall -Wextra   -c -o foo.o foo.c
gcc -fPIC -g -O2 -Wall -Wextra -shared foo.o -o libfoo.so
$ make libfoo.abi
abidw --no-show-locs --no-corpus-path --no-comp-dir-path \
      --suppressions libfoo.supp \
      --out-file libfoo.abi libfoo.so
$ cp libfoo.abi{,.orig}
$ patch -p1 < 0001-Add-Foo-handle-to-replace-generic-void.patch
$ make
gcc -fPIC -g -O2 -Wall -Wextra   -c -o foo.o foo.c
gcc -fPIC -g -O2 -Wall -Wextra -shared foo.o -o libfoo.so
$ make libfoo.abi
abidw --no-show-locs --no-corpus-path --no-comp-dir-path \
      --suppressions libfoo.supp \
      --out-file libfoo.abi libfoo.so
$ abidiff --suppressions libfoo.supp libfoo.abi{.orig,}
Functions changes summary: 0 Removed, 4 Changed, 0 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

4 functions with some indirect sub-type change:

  [C]'function void* create_foo()' has some indirect sub-type changes:
    return type changed:
      entity changed from 'void*' to 'void'
      type size changed from 64 to 0 (in bits)

  [C]'function void destroy_foo(void*)' has some indirect sub-type changes:
    parameter 1 of type 'void*' was removed


  [C]'function int get_foo(void*)' has some indirect sub-type changes:
    parameter 1 of type 'void*' was removed


  [C]'function void set_foo(void*, int)' has some indirect sub-type changes:
    parameter 1 of type 'void*' changed:
      entity changed from 'void*' to 'int'
      type size changed from 64 to 32 (in bits)
    parameter 2 of type 'int' was removed

Note how abidiff treats the function return type and parameters changed
from void * to Foo * as disappearing.

Is there another way to achieve what I want/need? Or can we have a
"drop mode" that treats handles to dropped types a simple void pointers
instead?

Thanks,

Mark

P.S. If we leave off the drop = yes, but keep the [suppress_type]
     source_location_not_in = foo.h we still get a report on the
     type change, even though I think it is abi compatible:

$ abidiff --suppressions libfoo.supp libfoo.abi{.orig,}
Functions changes summary: 0 Removed, 4 Changed, 0 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

4 functions with some indirect sub-type change:

  [C]'function void* create_foo()' has some indirect sub-type changes:
    return type changed:
      in pointed to type 'void':
        entity changed from 'void' to 'typedef Foo'
        type size changed from 0 to 32 (in bits)

  [C]'function void destroy_foo(void*)' has some indirect sub-type changes:
    parameter 1 of type 'void*' changed:
      in pointed to type 'void':
        entity changed from 'void' to 'typedef Foo'
        type size changed from 0 to 32 (in bits)

  [C]'function int get_foo(void*)' has some indirect sub-type changes:
    parameter 1 of type 'void*' changed:
      in pointed to type 'void':
        entity changed from 'void' to 'typedef Foo'
        type size changed from 0 to 32 (in bits)

  [C]'function void set_foo(void*, int)' has some indirect sub-type changes:
    parameter 1 of type 'void*' changed:
      in pointed to type 'void':
        entity changed from 'void' to 'typedef Foo'
        type size changed from 0 to 32 (in bits)

How would I tell abidw/diff that an (opaque) pointer change like that
isn't an ABI break?

[-- Attachment #2: foo.c --]
[-- Type: text/x-csrc, Size: 360 bytes --]

#include "fooP.h"
#include <stdlib.h>

void *
create_foo (void)
{
  return malloc (sizeof (struct foo));
}

void
destroy_foo (void *foo)
{
  free (foo);
}

void
set_foo (void *foo, int num)
{
  struct foo *instance = (struct foo *) foo;
  instance->num = num;
}

int
get_foo (void *foo)
{
  struct foo *instance = (struct foo *) foo;
  return instance->num;
}

[-- Attachment #3: foo.h --]
[-- Type: text/x-chdr, Size: 116 bytes --]

void *create_foo (void);
void destroy_foo (void *foo);
void set_foo (void *foo, int num);
int  get_foo (void *foo);

[-- Attachment #4: fooP.h --]
[-- Type: text/x-chdr, Size: 45 bytes --]

#include "foo.h"

struct foo
{
  int num;
};

[-- Attachment #5: libfoo.supp --]
[-- Type: text/plain, Size: 62 bytes --]

[suppress_type]
  source_location_not_in = foo.h
  drop = yes

[-- Attachment #6: Makefile --]
[-- Type: text/x-makefile, Size: 489 bytes --]

SOURCES = foo.c
OBJECTS = $(SOURCES:.c=.o)
TARGET = libfoo.so
TARGETABI = $(TARGET:.so=.abi)
ABISUPPRESSION = $(TARGETABI:.abi=.supp)

CC = gcc
CFLAGS = -fPIC -g -O2 -Wall -Wextra
LDFLAGS = -shared

all: $(TARGET)

$(TARGET): $(OBJECTS)
	$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@

$(TARGETABI): $(TARGET)
	abidw --no-show-locs --no-corpus-path --no-comp-dir-path \
	      --suppressions $(ABISUPPRESSION) \
	      --out-file $@ $<

clean:
	rm -f $(OBJECTS) $(TARGET) $(TARGETABI)

.PHONY: clean

[-- Attachment #7: 0001-Add-Foo-handle-to-replace-generic-void.patch --]
[-- Type: text/x-patch, Size: 1339 bytes --]

From bed1888184cfcb69c1332980591fc2a9af20d8de Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Sun, 5 Apr 2020 15:46:52 +0200
Subject: [PATCH] Add Foo handle to replace generic void *

---
 foo.c | 16 +++++++---------
 foo.h | 10 ++++++----
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/foo.c b/foo.c
index 74b6310..50ba43d 100644
--- a/foo.c
+++ b/foo.c
@@ -1,28 +1,26 @@
 #include "fooP.h"
 #include <stdlib.h>
 
-void *
+Foo *
 create_foo (void)
 {
-  return malloc (sizeof (struct foo));
+  return (Foo *) malloc (sizeof (struct foo));
 }
 
 void
-destroy_foo (void *foo)
+destroy_foo (Foo *foo)
 {
   free (foo);
 }
 
 void
-set_foo (void *foo, int num)
+set_foo (Foo *foo, int num)
 {
-  struct foo *instance = (struct foo *) foo;
-  instance->num = num;
+  foo->num = num;
 }
 
 int
-get_foo (void *foo)
+get_foo (Foo *foo)
 {
-  struct foo *instance = (struct foo *) foo;
-  return instance->num;
+  return foo->num;
 }
diff --git a/foo.h b/foo.h
index 67a197d..0b80354 100644
--- a/foo.h
+++ b/foo.h
@@ -1,4 +1,6 @@
-void *create_foo (void);
-void destroy_foo (void *foo);
-void set_foo (void *foo, int num);
-int  get_foo (void *foo);
+typedef struct foo Foo;
+
+Foo *create_foo (void);
+void destroy_foo (Foo *foo);
+void set_foo (Foo *foo, int num);
+int  get_foo (Foo *foo);
-- 
2.18.2

Re: Surprising behavior of suppress_type drop = yes

[Dodji Seketeli]

Hello Mark,


> Note how abidiff treats the function return type and parameters changed
> from void * to Foo * as disappearing.
>
> Is there another way to achieve what I want/need?

I think there is a non-documented way to do this, yes.  It hasn't been
yet documented because nobody expressed the need for it yet.

Basically you want to do this:

[suppress_type]
  label = "Artificial·private·types·suppression·specification"
  source_location_not_in = list.h, of.h, public.h, headers.h
  drop = yes

The 'label' property there is important and it needs to have that exact
string value.

Libabigail generates this kind of suppression specifications internally
to handle types that it considers to be private types, namely, types
that are defined in files specified by --headers-dir{1,2}.

So I guess you can use that undocumented feature for now.

> Or can we have a "drop mode" that treats handles to dropped types a
> simple void pointers instead?

I think what you'd need, really, is a combination of:

    1/ a new --header-files{1,2} option
    2/ an "opaque = yes" attribute in the suppression specification.

I think either 1/ or 2/ should work, but it'd be nice to have both.

If you think that suggession holds water, then we should probably file a
bug to add this new feature.

Sorry for the inconvenience.

-- 
		Dodji

Re: Surprising behavior of suppress_type drop = yes

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]

Hi Dodji,

On Mon, 2020-04-06 at 19:15 +0200, Dodji Seketeli wrote:
> > Note how abidiff treats the function return type and parameters changed
> > from void * to Foo * as disappearing.
> > 
> > Is there another way to achieve what I want/need?
> 
> I think there is a non-documented way to do this, yes.  It hasn't been
> yet documented because nobody expressed the need for it yet.
> 
> Basically you want to do this:
> 
> [suppress_type]
>   label = "Artificial·private·types·suppression·specification"

Assuming those fancy dots are actually spaces.

>   source_location_not_in = list.h, of.h, public.h, headers.h
>   drop = yes
> 
> The 'label' property there is important and it needs to have that exact
> string value.
> 
> Libabigail generates this kind of suppression specifications internally
> to handle types that it considers to be private types, namely, types
> that are defined in files specified by --headers-dir{1,2}.
> 
> So I guess you can use that undocumented feature for now.

It doesn't seem to change anything for my example.
I assume it does for you? Can you attach the xml abi file it is
supposed to generate? Mine still looks like the attached with missing
return types and arguments where there should be pointers to Foo. Maybe
my suppression file still isn't correct? I attached both.

> > Or can we have a "drop mode" that treats handles to dropped types a
> > simple void pointers instead?
> 
> I think what you'd need, really, is a combination of:
> 
>     1/ a new --header-files{1,2} option
>     2/ an "opaque = yes" attribute in the suppression specification.
> 
> I think either 1/ or 2/ should work, but it'd be nice to have both.
> 
> If you think that suggession holds water, then we should probably
> file a
> bug to add this new feature.

Why not fix the default?

I am trying to figure out why the command line option is different from
the suppress_type default way of handling drop. And when that default
way of working is actually useful?

Thanks,

Mark

[-- Attachment #2: libfoo.abi --]
[-- Type: text/plain, Size: 2305 bytes --]

<abi-corpus architecture='elf-amd-x86_64'>
  <elf-needed>
    <dependency name='libc.so.6'/>
  </elf-needed>
  <elf-function-symbols>
    <elf-symbol name='_fini' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='_init' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='create_foo' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='destroy_foo' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='get_foo' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='set_foo' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
  </elf-function-symbols>
  <abi-instr version='1.0' address-size='64' path='foo.c' language='LANG_C99'>
    <type-decl name='int' size-in-bits='32' id='type-id-1'/>
    <type-decl name='void' id='type-id-2'/>
    <function-decl name='get_foo' mangled-name='get_foo' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='get_foo'>
      <return type-id='type-id-1'/>
    </function-decl>
    <function-decl name='set_foo' mangled-name='set_foo' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='set_foo'>
      <parameter type-id='type-id-1' name='num'/>
      <return type-id='type-id-2'/>
    </function-decl>
    <function-decl name='destroy_foo' mangled-name='destroy_foo' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='destroy_foo'>
      <return type-id='type-id-2'/>
    </function-decl>
    <function-decl name='create_foo' mangled-name='create_foo' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='create_foo'>
      <return type-id='type-id-2'/>
    </function-decl>
    <function-decl name='free' mangled-name='free' visibility='default' binding='global' size-in-bits='64'>
      <return type-id='type-id-2'/>
    </function-decl>
    <function-decl name='malloc' mangled-name='malloc' visibility='default' binding='global' size-in-bits='64'>
      <return type-id='type-id-2'/>
    </function-decl>
  </abi-instr>
</abi-corpus>

[-- Attachment #3: libfoo.supp --]
[-- Type: text/plain, Size: 125 bytes --]

[suppress_type]
  label = "Artificial private types suppression specification"
  source_location_not_in = foo.h
  drop = yes

Re: Surprising behavior of suppress_type drop = yes

[Dodji Seketeli]

[-- Attachment #1: Type: text/plain, Size: 3372 bytes --]

Hello Mark,

Mark Wielaard <mark@klomp.org> a écrit:

> Hi Dodji,
>
> On Mon, 2020-04-06 at 19:15 +0200, Dodji Seketeli wrote:
>> > Note how abidiff treats the function return type and parameters changed
>> > from void * to Foo * as disappearing.
>> > 
>> > Is there another way to achieve what I want/need?
>> 
>> I think there is a non-documented way to do this, yes.  It hasn't been
>> yet documented because nobody expressed the need for it yet.
>> 
>> Basically you want to do this:
>> 
>> [suppress_type]
>>   label = "Artificial·private·types·suppression·specification"
>
> Assuming those fancy dots are actually spaces.

Pfff, sorry, my copy-paste-fu failed miserably there.

I am attaching below the libfoo.supp suppression file I used. Actually,
there are no '"' delimitation character either.

[...]


> It doesn't seem to change anything for my example.  I assume it does
> for you?

Yes, sorry, it's my fault.  I hope with suppression file below it gets
better.

> Can you attach the xml abi file it is supposed to generate? Mine still
> looks like the attached with missing return types and arguments where
> there should be pointers to Foo. Maybe my suppression file still isn't
> correct? I attached both.

I attached the resulting abixml file I am getting below as well.


>> > Or can we have a "drop mode" that treats handles to dropped types a
>> > simple void pointers instead?
>> 
>> I think what you'd need, really, is a combination of:
>> 
>>     1/ a new --header-files{1,2} option
>>     2/ an "opaque = yes" attribute in the suppression specification.
>> 
>> I think either 1/ or 2/ should work, but it'd be nice to have both.
>> 
>> If you think that suggession holds water, then we should probably
>> file a
>> bug to add this new feature.
>
> Why not fix the default?

I am not sure what you mean by default.

"drop = yes" in the [suppress_type] section of a suppression
specification is not meant to be the same feature as
--header-files{1,2}.

drop = yes really drops *all* types that match the suppression
specification.  Those types disappear completement from the internal
representation.  This can be useful to aggressively reduce the size of
the internal representation.

--header-files{1,2} generates a suppression specification that drop all
types that are not defined in header files, but, for class/struct/
types, it does something different.  Rather than just droping those
struct/class types from the internal representation, it replaces them by
a (forward) declaration -- as opposed to the full type definition.  So a
pointer to that declaration becomes a pointer to an opaque type, in
practise.

These two features are useful to people in different use cases.

If your question is "how to do exactly what --headers-files{1,2} does
using suppression specfications", then the official answer today is "you
can't".


> I am trying to figure out why the command line option is different from
> the suppress_type default way of handling drop. And when that default
> way of working is actually useful?

Being able completly drop types from the IR is mainly used for size
optimization purposes.  And in that case, you really want to drop the
type on the floor, not replace it with something else.

I hope this is useful.

Cheers,


[-- Attachment #2: libfoo.supp --]
[-- Type: application/octet-stream, Size: 223 bytes --]

[suppress_function]
  file_name_not_regexp = foo.c
  drop = yes


[suppress_type]
  label = Artificial private types suppression specification
  source_location_not_in = foo.h, list.h, of.h, public.h, headers.h
  drop = yes

[-- Attachment #3: libfoo.so.suppr.abi --]
[-- Type: application/octet-stream, Size: 3932 bytes --]

<abi-corpus path='libfoo.so' architecture='elf-amd-x86_64'>
  <elf-needed>
    <dependency name='libc.so.6'/>
  </elf-needed>
  <elf-function-symbols>
    <elf-symbol name='create_foo' version='FOOLIB_1.0' is-default-version='yes' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='destroy_foo' version='FOOLIB_1.0' is-default-version='yes' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='get_foo' version='FOOLIB_1.0' is-default-version='yes' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
    <elf-symbol name='set_foo' version='FOOLIB_1.0' is-default-version='yes' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
  </elf-function-symbols>
  <abi-instr version='1.0' address-size='64' path='foo.c' comp-dir-path='/home/dodji/tests/foolib/foolib' language='LANG_C89'>
    <type-decl name='int' size-in-bits='32' id='type-id-1'/>
    <type-decl name='unsigned long int' size-in-bits='64' id='type-id-2'/>
    <type-decl name='void' id='type-id-3'/>
    <typedef-decl name='Foo' type-id='type-id-4' filepath='/home/dodji/tests/foolib/foolib/foo.h' line='1' column='1' id='type-id-5'/>
    <typedef-decl name='size_t' type-id='type-id-2' filepath='/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include/stddef.h' line='212' column='1' id='type-id-6'/>
    <pointer-type-def type-id='type-id-5' size-in-bits='64' id='type-id-7'/>
    <pointer-type-def type-id='type-id-3' size-in-bits='64' id='type-id-8'/>
    <function-decl name='create_foo' mangled-name='create_foo' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='5' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='create_foo@@FOOLIB_1.0'>
      <return type-id='type-id-7'/>
    </function-decl>
    <function-decl name='destroy_foo' mangled-name='destroy_foo' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='14' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='destroy_foo@@FOOLIB_1.0'>
      <parameter type-id='type-id-7' name='foo' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='14' column='1'/>
      <return type-id='type-id-3'/>
    </function-decl>
    <function-decl name='set_foo' mangled-name='set_foo' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='20' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='set_foo@@FOOLIB_1.0'>
      <parameter type-id='type-id-7' name='foo' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='20' column='1'/>
      <parameter type-id='type-id-1' name='num' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='20' column='1'/>
      <return type-id='type-id-3'/>
    </function-decl>
    <function-decl name='get_foo' mangled-name='get_foo' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='27' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='get_foo@@FOOLIB_1.0'>
      <parameter type-id='type-id-7' name='foo' filepath='/home/dodji/tests/foolib/foolib/foo.c' line='27' column='1'/>
      <return type-id='type-id-1'/>
    </function-decl>
    <function-decl name='malloc' filepath='/usr/include/stdlib.h' line='465' column='1' visibility='default' binding='global' size-in-bits='64'>
      <parameter type-id='type-id-6'/>
      <return type-id='type-id-8'/>
    </function-decl>
    <function-decl name='free' filepath='/usr/include/stdlib.h' line='482' column='1' visibility='default' binding='global' size-in-bits='64'>
      <parameter type-id='type-id-8'/>
      <return type-id='type-id-3'/>
    </function-decl>
    <class-decl name='foo' is-struct='yes' is-artificial='yes' visibility='default' filepath='/home/dodji/tests/foolib/foolib/fooP.h' line='3' column='1' is-declaration-only='yes' id='type-id-4'/>
  </abi-instr>
</abi-corpus>

[-- Attachment #4: Type: text/plain, Size: 13 bytes --]


-- 
		Dodji

Re: Surprising behavior of suppress_type drop = yes

[Mark Wielaard]

Hi Dodji,

On Thu, 2020-04-09 at 15:52 +0200, Dodji Seketeli wrote:
> I am attaching below the libfoo.supp suppression file I used.
> Actually, there are no '"' delimitation character either.
> 
> [...]
> 
> > It doesn't seem to change anything for my example.  I assume it
> > does for you?
> 
> Yes, sorry, it's my fault.  I hope with suppression file below it
> gets better.

It does! Thanks. Now it looks like expected.

> > > > Or can we have a "drop mode" that treats handles to dropped
> > > > types a
> > > > simple void pointers instead?
> > > 
> > > I think what you'd need, really, is a combination of:
> > > 
> > >     1/ a new --header-files{1,2} option
> > >     2/ an "opaque = yes" attribute in the suppression
> > > specification.
> > > 
> > > I think either 1/ or 2/ should work, but it'd be nice to have
> > > both.
> > > 
> > > If you think that suggession holds water, then we should probably
> > > file a
> > > bug to add this new feature.
> > 
> > Why not fix the default?
> 
> I am not sure what you mean by default.
> 
> "drop = yes" in the [suppress_type] section of a suppression
> specification is not meant to be the same feature as
> --header-files{1,2}.
> 
> drop = yes really drops *all* types that match the suppression
> specification.  Those types disappear completement from the internal
> representation.  This can be useful to aggressively reduce the size
> of
> the internal representation.
> 
> --header-files{1,2} generates a suppression specification that drop
> all
> types that are not defined in header files, but, for class/struct/
> types, it does something different.  Rather than just droping those
> struct/class types from the internal representation, it replaces them
> by
> a (forward) declaration -- as opposed to the full type
> definition.  So a
> pointer to that declaration becomes a pointer to an opaque type, in
> practise.
> 
> These two features are useful to people in different use cases.

I see. But I find the dropping of function parameters odd. It means you
will be unable to see some issues when functions change an argument
with a dropped type. Say you have an function:

alert (Foobar *f, char *msg);

Where Foobar is an opaque (dropped) type. And you realize the alert
doesn't really depend on the given Foobar. So you remove it.

alert (char *msg);

That is an incompatible abi change, you won't be able to spot with the
current behavior of drop = yes.

I had expected that drop = yes would do what --headers-dir --drop-
private-types did and not remove the arguments that (indirectly)
reference the type got dropped too.

> If your question is "how to do exactly what --headers-files{1,2} does
> using suppression specfications", then the official answer today is
> "you can't".

OK. I don't particularly want to do it through a suppression
specification, but currently cannot easily do it through the command
line because I need something like --header-file because my public and
private headers are in the same dir (as the sources). And abidw doesn't
take --drop-private-types.

I'll sent patches to implement that.

Thanks,

Mark

[PATCH 1/2] Add --header-file option to add individual public header files.

[Mark Wielaard]

Sometimes public header files are in the same directory as private
header files. In such cases --headers-dir cannot be used. Add
--header-file to add individual public header files.

	* include/abg-tools-utils.h (gen_suppr_spec_from_headers): Add
	hdr_files string vector argument.
	* src/abg-tools-utils.cc (handle_file_entry): New function that
	adds one specific file to the type_suppression. Implementation
	lifted from...
	(handle_fts_entry): ...here. Call handle_file_entry for each file.
	(gen_suppr_spec_from_headers): Also takes a header_files string
	vector as argument. Call handle_file_entry for each file entry.
	* tools/abidiff.cc (options): Add header_files1 and header_files2
	string vectors.
	(display_usage): Print --header-file1 and --header-file2 usage.
	(parse_command_line): Handle --header-file1, --hf1 and
	--header-file2, --hf2.
	(set_diff_context_from_opts): Call gen_suppr_spec_from_headers
	with header_files1 and header_files2.
	(set_suppressions): Likewise.
	* tools/abidw.cc (options): Add header_files string vector.
	(display_usage): Print --header-file usage.
	(parse_command_line): Handle --header-file1, --hf1.
	(maybe_check_header_files): New function.
	(set_suppressions): Call gen_suppr_spec_from_headers with
	header_files.
	(main): Call maybe_check_header_files.
	* tools/abilint.cc (options): Add header_files string vector.
	(display_usage): Print --header-file usage.
	(parse_command_line): Handle --header-file1, --hf1.
	(set_suppressions): Call gen_suppr_spec_from_headers with
	header_files.
	* tools/abipkgdiff.cc (maybe_create_private_types_suppressions):
	Call gen_suppr_spec_from_headers with an empty string vector.
	* doc/manuals/abidiff.rst: Document --header-file1, --hf1 and
	--header-file2, --hf2. Add new options to documentation of
	--drop-private-types.
	* doc/manuals/abidw.rst: Document --header-file, --hf.
	* doc/manuals/abilint.rst: Likewise.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 doc/manuals/abidiff.rst   | 33 ++++++++++-----
 doc/manuals/abidw.rst     |  6 +++
 doc/manuals/abilint.rst   |  6 +++
 include/abg-tools-utils.h |  3 +-
 src/abg-tools-utils.cc    | 86 ++++++++++++++++++++++++++-------------
 tools/abidiff.cc          | 46 +++++++++++++++++----
 tools/abidw.cc            | 35 +++++++++++++++-
 tools/abilint.cc          | 15 ++++++-
 tools/abipkgdiff.cc       |  4 +-
 9 files changed, 183 insertions(+), 51 deletions(-)

diff --git a/doc/manuals/abidiff.rst b/doc/manuals/abidiff.rst
index 8d914db5..6aac73bb 100644
--- a/doc/manuals/abidiff.rst
+++ b/doc/manuals/abidiff.rst
@@ -104,12 +104,24 @@ Options
     library that the tool has to consider.  The tool will thus filter
     out ABI changes on types that are not defined in public headers.
 
+  * ``--header-file1 | --hf1`` <header-file-path-1>
+
+    Specifies where to find one public header of the first shared
+    library that the tool has to consider.  The tool will thus filter
+    out ABI changes on types that are not defined in public headers.
+
   * ``--headers-dir2 | --hd2`` <headers-directory-path-1>
 
     Specifies where to find the public headers of the second shared
     library that the tool has to consider.  The tool will thus filter
     out ABI changes on types that are not defined in public headers.
 
+  * ``--header-file2 | --hf2`` <header-file-path-2>
+
+    Specifies where to find one public header of the second shared
+    library that the tool has to consider.  The tool will thus filter
+    out ABI changes on types that are not defined in public headers.
+
   * ``--no-linux-kernel-mode``
 
     Without this option, if abidiff detects that the binaries it is
@@ -141,13 +153,13 @@ Options
 
   * ``--drop-private-types``
 
-    This option is to be used with the ``--headers-dir1`` and
-    ``--headers-dir2`` options.  With this option, types that are
-    *NOT* defined in the headers are entirely dropped from the
-    internal representation build by Libabigail to represent the ABI.
-    They thus don't have to be filtered out from the final ABI change
-    report because they are not even present in Libabigail's
-    representation.
+    This option is to be used with the ``--headers-dir1``,
+    ``header-file1``, ``header-file2`` and ``--headers-dir2`` options.
+    With this option, types that are *NOT* defined in the headers are
+    entirely dropped from the internal representation build by
+    Libabigail to represent the ABI.  They thus don't have to be
+    filtered out from the final ABI change report because they are not
+    even present in Libabigail's representation.
 
     Without this option however, those private types are kept in the
     internal representation and later filtered out from the report.
@@ -218,9 +230,10 @@ Options
 
     This option might incur some serious performance degradation as
     the number of types analyzed can be huge.  However, if paired with
-    the ``--headers-dir{1,2}`` options, the additional non-reachable
-    types analyzed are restricted to those defined in public headers
-    files, thus hopefully making the performance hit acceptable.
+    the ``--headers-dir{1,2}`` and/or ``header-file{1,2}`` options,
+    the additional non-reachable types analyzed are restricted to
+    those defined in public headers files, thus hopefully making the
+    performance hit acceptable.
 
     Also, using this option alongside suppression specifications (by
     also using the ``--suppressions`` option) might help keep the number of
diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index 4843d009..8f6c3c39 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -132,6 +132,12 @@ Options
     library that the tool has to consider.  The tool will thus filter
     out types that are not defined in public headers.
 
+  * ``--header-file | --hf`` <header-file-path>
+
+    Specifies where to find one of the public headers of the abi file
+    that the tool has to consider.  The tool will thus filter out
+    types that are not defined in public headers.
+
   * ``--no-linux-kernel-mode``
 
     Without this option, if abipkgiff detects that the binaries it is
diff --git a/doc/manuals/abilint.rst b/doc/manuals/abilint.rst
index 9e07f47f..4cc0b428 100644
--- a/doc/manuals/abilint.rst
+++ b/doc/manuals/abilint.rst
@@ -76,6 +76,12 @@ Options
     library that the tool has to consider.  The tool will thus filter
     out types that are not defined in public headers.
 
+  * ``--header-file | --hf`` <header-file-path>
+
+    Specifies where to find one of the public headers of the abi file
+    that the tool has to consider.  The tool will thus filter out
+    types that are not defined in public headers.
+
   * ``--stdin | --``
 
     Read the input content from standard input.
diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h
index 053519a4..564fe646 100644
--- a/include/abg-tools-utils.h
+++ b/include/abg-tools-utils.h
@@ -84,7 +84,8 @@ string trim_leading_string(const string& from, const string& to_trim);
 void convert_char_stars_to_char_star_stars(const vector<char*>&,
 					   vector<char**>&);
 suppr::type_suppression_sptr
-gen_suppr_spec_from_headers(const string& hdrs_root_dir);
+gen_suppr_spec_from_headers(const string& hdrs_root_dir,
+			    const vector<string>& hdr_files);
 
 suppr::suppressions_type
 gen_suppr_spec_from_kernel_abi_whitelists
diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index 11f6129e..b05ed645 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -1787,6 +1787,40 @@ make_path_absolute_to_be_freed(const char*p)
   return result;
 }
 
+/// This is a sub-routine of gen_suppr_spec_from_headers and
+///  handle_fts_entry.
+///
+/// @param file add to the to the vector returned by
+/// type_suppression::get_source_locations_to_keep()
+///
+/// @param suppr the file name is going to be added to the vector
+/// returned by the method type_suppression::get_source_locations_to_keep
+/// of this instance.
+/// If this smart pointer is nil then a new instance @ref
+/// type_suppression is created and this variable is made to point to
+/// it.
+static void
+handle_file_entry(const string& file,
+		  type_suppression_sptr& suppr)
+{
+  if (!suppr)
+    {
+      suppr.reset(new type_suppression(get_private_types_suppr_spec_label(),
+				       /*type_name_regexp=*/"",
+				       /*type_name=*/""));
+
+      // Types that are defined in system headers are usually
+      // OK to be considered as public types.
+      suppr->set_source_location_to_keep_regex_str("^/usr/include/");
+      suppr->set_is_artificial(true);
+    }
+
+  // And types that are defined in header files that are under
+  // the header directory file we are looking are to be
+  // considered public types too.
+  suppr->get_source_locations_to_keep().insert(file);
+}
+
 /// This is a sub-routine of gen_suppr_spec_from_headers.
 ///
 /// @param entry if this file represents a regular (or symlink) file,
@@ -1815,23 +1849,7 @@ handle_fts_entry(const FTSENT *entry,
       if (string_ends_with(fname, ".h")
 	  || string_ends_with(fname, ".hpp")
 	  || string_ends_with(fname, ".hxx"))
-	{
-	  if (!suppr)
-	    {
-	      suppr.reset(new type_suppression(get_private_types_suppr_spec_label(),
-					       /*type_name_regexp=*/"",
-					       /*type_name=*/""));
-
-	      // Types that are defined in system headers are usually
-	      // OK to be considered as public types.
-	      suppr->set_source_location_to_keep_regex_str("^/usr/include/");
-	      suppr->set_is_artificial(true);
-	    }
-	  // And types that are defined in header files that are under
-	  // the header directory file we are looking are to be
-	  // considered public types too.
-	  suppr->get_source_locations_to_keep().insert(fname);
-	}
+	handle_file_entry (fname, suppr);
     }
 }
 
@@ -1845,25 +1863,35 @@ handle_fts_entry(const FTSENT *entry,
 /// @return the resulting type suppression generated, if any file was
 /// found in the directory tree @p headers_root_dir.
 type_suppression_sptr
-gen_suppr_spec_from_headers(const string& headers_root_dir)
+gen_suppr_spec_from_headers(const string& headers_root_dir,
+			    const vector<string>& header_files)
 {
   type_suppression_sptr result;
 
-  if (headers_root_dir.empty())
-    // We were given no headers root dir so the resulting suppression
-    // specification shall be empty.
+  if (headers_root_dir.empty() && header_files.empty())
+    // We were given no headers root dir and no header files
+    // so the resulting suppression specification shall be empty.
     return result;
 
-  char* paths[] = {const_cast<char*>(headers_root_dir.c_str()), 0};
+  if (!headers_root_dir.empty())
+    {
+      char* paths[] = {const_cast<char*>(headers_root_dir.c_str()), 0};
 
-  FTS *file_hierarchy = fts_open(paths, FTS_LOGICAL|FTS_NOCHDIR, NULL);
-  if (!file_hierarchy)
-    return result;
+      FTS *file_hierarchy = fts_open(paths, FTS_LOGICAL|FTS_NOCHDIR, NULL);
+      if (!file_hierarchy)
+	return result;
+
+      FTSENT *entry;
+      while ((entry = fts_read(file_hierarchy)))
+	handle_fts_entry(entry, result);
+      fts_close(file_hierarchy);
+    }
+
+  for (vector<string>::const_iterator file = header_files.begin();
+       file != header_files.end();
+       ++file)
+    handle_file_entry(*file, result);
 
-  FTSENT *entry;
-  while ((entry = fts_read(file_hierarchy)))
-    handle_fts_entry(entry, result);
-  fts_close(file_hierarchy);
   return result;
 }
 
diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index 44ad1878..162d5ebc 100644
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -79,7 +79,9 @@ struct options
   vector<string>	keep_fn_regex_patterns;
   vector<string>	keep_var_regex_patterns;
   string		headers_dir1;
+  vector<string>        header_files1;
   string		headers_dir2;
+  vector<string>        header_files2;
   bool			drop_private_types;
   bool			linux_kernel_mode;
   bool			no_default_supprs;
@@ -183,7 +185,9 @@ display_usage(const string& prog_name, ostream& out)
     << " --debug-info-dir1|--d1 <path> the root for the debug info of file1\n"
     << " --debug-info-dir2|--d2 <path> the root for the debug info of file2\n"
     << " --headers-dir1|--hd1 <path>  the path to headers of file1\n"
+    << " --header-file1|--hf1 <path>  the path to one header of file1\n"
     << " --headers-dir2|--hd2 <path>  the path to headers of file2\n"
+    << " --header-file2|--hf2 <path>  the path to one header of file2\n"
     << " --drop-private-types  drop private types from "
     "internal representation\n"
     << " --no-linux-kernel-mode  don't consider the input binaries as "
@@ -317,6 +321,19 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir1 = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file1")
+	       || !strcmp(argv[i], "--hf1"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
+	      opts.missing_operand = true;
+	      opts.wrong_option = argv[i];
+	      return true;
+	    }
+	  opts.header_files1.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--headers-dir2")
 	       || !strcmp(argv[i], "--hd2"))
 	{
@@ -330,6 +347,19 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir2 = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file2")
+	       || !strcmp(argv[i], "--hf2"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
+	      opts.missing_operand = true;
+	      opts.wrong_option = argv[i];
+	      return true;
+	    }
+	  opts.header_files2.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--kmi-whitelist")
 	       || !strcmp(argv[i], "-w"))
 	{
@@ -690,22 +720,22 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
       load_default_user_suppressions(supprs);
     }
 
-  if (!opts.headers_dir1.empty())
+  if (!opts.headers_dir1.empty() || !opts.header_files1.empty())
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir1);
+	gen_suppr_spec_from_headers(opts.headers_dir1, opts.header_files1);
       if (suppr)
 	ctxt->add_suppression(suppr);
     }
 
-    if (!opts.headers_dir2.empty())
+  if (!opts.headers_dir2.empty() || !opts.header_files2.empty())
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir2);
+	gen_suppr_spec_from_headers(opts.headers_dir2, opts.header_files2);
       if (suppr)
 	ctxt->add_suppression(suppr);
     }
@@ -740,7 +770,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
     read_suppressions(*i, supprs);
 
   if (read_context_get_path(read_ctxt) == opts.file1
-      && !opts.headers_dir1.empty())
+      && (!opts.headers_dir1.empty() || !opts.header_files1.empty()))
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers for
@@ -750,7 +780,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
       // corpus loading, they are going to be dropped from the
       // internal representation altogether.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir1);
+	gen_suppr_spec_from_headers(opts.headers_dir1, opts.header_files1);
       if (suppr)
 	{
 	  if (opts.drop_private_types)
@@ -760,7 +790,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
     }
 
   if (read_context_get_path(read_ctxt) == opts.file2
-      && !opts.headers_dir2.empty())
+      && (!opts.headers_dir2.empty() || !opts.header_files2.empty()))
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers for
@@ -770,7 +800,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
       // corpus loading, they are going to be dropped from the
       // internal representation altogether.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir2);
+	gen_suppr_spec_from_headers(opts.headers_dir2, opts.header_files2);
       if (suppr)
 	{
 	  if (opts.drop_private_types)
diff --git a/tools/abidw.cc b/tools/abidw.cc
index 3b531999..a8b9ad32 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -88,6 +88,7 @@ struct options
   vector<char*>	di_root_paths;
   vector<char**>	prepared_di_root_paths;
   string		headers_dir;
+  vector<string>	header_files;
   string		vmlinux;
   vector<string>	suppression_paths;
   vector<string>	kabi_whitelist_paths;
@@ -149,6 +150,7 @@ display_usage(const string& prog_name, ostream& out)
     << "  --version|-v  display program version information and exit\n"
     << "  --debug-info-dir|-d <dir-path>  look for debug info under 'dir-path'\n"
     << "  --headers-dir|--hd <path> the path to headers of the elf file\n"
+    << "  --header-file|--hf <path> the path one header of the elf file\n"
     << "  --out-file <file-path>  write the output to 'file-path'\n"
     << "  --noout  do not emit anything after reading the binary\n"
     << "  --suppressions|--suppr <path> specify a suppression file\n"
@@ -217,6 +219,15 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file")
+	       || !strcmp(argv[i], "--hf"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    return false;
+	  opts.header_files.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--out-file"))
 	{
 	  if (argc <= i + 1
@@ -349,6 +360,24 @@ maybe_check_suppression_files(const options& opts)
   return true;
 }
 
+/// Check that the header files supplied are present.
+/// If not, emit an error on stderr.
+///
+/// @param opts the options instance to use.
+///
+/// @return true if all header files are present, false otherwise.
+static bool
+maybe_check_header_files(const options& opts)
+{
+  for (vector<string>::const_iterator file = opts.header_files.begin();
+       file != opts.header_files.end();
+       ++file)
+    if (!check_file(*file, cerr, "abidw"))
+      return false;
+
+  return true;
+}
+
 /// Set suppression specifications to the @p read_context used to load
 /// the ABI corpus from the ELF/DWARF file.
 ///
@@ -371,7 +400,8 @@ set_suppressions(read_context& read_ctxt, options& opts)
     read_suppressions(*i, supprs);
 
   suppression_sptr suppr =
-    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir);
+    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir,
+						      opts.header_files);
   if (suppr)
     supprs.push_back(suppr);
 
@@ -721,6 +751,9 @@ main(int argc, char* argv[])
   if (!maybe_check_suppression_files(opts))
     return 1;
 
+  if (!maybe_check_header_files(opts))
+    return 1;
+
   abigail::tools_utils::file_type type =
     abigail::tools_utils::guess_file_type(opts.in_file_path);
   if (type != abigail::tools_utils::FILE_TYPE_ELF
diff --git a/tools/abilint.cc b/tools/abilint.cc
index b06ffd72..e9d6f93e 100644
--- a/tools/abilint.cc
+++ b/tools/abilint.cc
@@ -86,6 +86,7 @@ struct options
   abg_compat::shared_ptr<char>	di_root_path;
   vector<string>		suppression_paths;
   string			headers_dir;
+  vector<string>		header_files;
 
   options()
     : display_version(false),
@@ -107,6 +108,8 @@ display_usage(const string& prog_name, ostream& out)
     << "  --debug-info-dir <path> the path under which to look for "
     << "  --headers-dir|--hd <patch> the path to headers of the elf file\n"
     "debug info for the elf <abi-file>\n"
+    << "  --header-file|--hf <path> the path to one header of the elf file\n"
+    "debug info for the elf <abi-file>\n"
     << "  --suppressions|--suppr <path> specify a suppression file\n"
     << "  --diff  for xml inputs, perform a text diff between "
     "the input and the memory model saved back to disk\n"
@@ -161,6 +164,15 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file")
+	       || !strcmp(argv[i], "--hf"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    return false;
+	  opts.header_files.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--suppressions")
 	       || !strcmp(argv[i], "--suppr"))
 	{
@@ -240,7 +252,8 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
     read_suppressions(*i, supprs);
 
   suppression_sptr suppr =
-    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir);
+    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir,
+						      opts.header_files);
   if (suppr)
     supprs.push_back(suppr);
 
diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index 611b4d07..ea4efb0e 100644
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -1529,8 +1529,10 @@ maybe_create_private_types_suppressions(package& pkg, const options &opts)
   if (!is_dir(headers_path))
     return false;
 
+  // We don't list individual files, just look under the headers_path.
+  vector<string> no_header_files;
   suppression_sptr suppr =
-    gen_suppr_spec_from_headers(headers_path);
+    gen_suppr_spec_from_headers(headers_path, no_header_files);
 
   if (suppr)
     {
-- 
2.18.2

[PATCH 2/2] Add --drop-private-types to abidw.

[Mark Wielaard]

To create smaller abi XML files it is useful to be able to drop
the private types from the representation.

	* tools/abidw.cc (options): Add drop_private_types bool.
	(display_usage): Add --drop-private-types.
	(parse_command_line): Parse --drop-private-types, set opts.
	(set_suppressions): Call set_drops_artifact_from_ir when
	drop_private_types set.
	* doc/manuals/abidw.rst: Document --drop-private-types.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 doc/manuals/abidw.rst |  8 ++++++++
 tools/abidw.cc        | 13 +++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index 8f6c3c39..05a90003 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -138,6 +138,14 @@ Options
     that the tool has to consider.  The tool will thus filter out
     types that are not defined in public headers.
 
+  * ``--drop-private-types``
+
+    This option is to be used with the ``--headers-dir`` and/or
+    ``header-file`` options.  With this option, types that are *NOT*
+    defined in the headers are entirely dropped from the internal
+    representation build by Libabigail to represent the ABI and will
+    not end up in the abi XML file.
+
   * ``--no-linux-kernel-mode``
 
     Without this option, if abipkgiff detects that the binaries it is
diff --git a/tools/abidw.cc b/tools/abidw.cc
index a8b9ad32..8b163ccd 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -109,6 +109,7 @@ struct options
   bool			abidiff;
   bool			annotate;
   bool			do_log;
+  bool			drop_private_types;
 
   options()
     : display_version(),
@@ -126,7 +127,8 @@ struct options
       show_locs(true),
       abidiff(),
       annotate(),
-      do_log()
+      do_log(),
+      drop_private_types(false)
   {}
 
   ~options()
@@ -158,6 +160,7 @@ display_usage(const string& prog_name, ostream& out)
     << "  --no-corpus-path  do not take the path to the corpora into account\n"
     << "  --no-show-locs  do not show location information\n"
     << "  --short-locs  only print filenames rather than paths\n"
+    << "  --drop-private-types  drop private types from representation\n"
     << "  --no-comp-dir-path  do not show compilation path information\n"
     << "  --check-alternate-debug-info <elf-path>  check alternate debug info "
     "of <elf-path>\n"
@@ -294,6 +297,8 @@ parse_command_line(int argc, char* argv[], options& opts)
 	}
       else if (!strcmp(argv[i], "--load-all-types"))
 	opts.load_all_types = true;
+      else if (!strcmp(argv[i], "--drop-private-types"))
+	opts.drop_private_types = true;
       else if (!strcmp(argv[i], "--no-linux-kernel-mode"))
 	opts.linux_kernel_mode = false;
       else if (!strcmp(argv[i], "--abidiff"))
@@ -403,7 +408,11 @@ set_suppressions(read_context& read_ctxt, options& opts)
     abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir,
 						      opts.header_files);
   if (suppr)
-    supprs.push_back(suppr);
+    {
+      if (opts.drop_private_types)
+	suppr->set_drops_artifact_from_ir(true);
+      supprs.push_back(suppr);
+    }
 
   using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelists;
   const suppressions_type& wl_suppr =
-- 
2.18.2

Re: [PATCH 1/2] Add --header-file option to add individual public header files.

[Dodji Seketeli]

[-- Attachment #1: Type: text/plain, Size: 11274 bytes --]

Hello Mark,

Mark Wielaard <mark@klomp.org> a écrit:

> Sometimes public header files are in the same directory as private
> header files. In such cases --headers-dir cannot be used. Add
> --header-file to add individual public header files.
>
> 	* include/abg-tools-utils.h (gen_suppr_spec_from_headers): Add
> 	hdr_files string vector argument.
> 	* src/abg-tools-utils.cc (handle_file_entry): New function that
> 	adds one specific file to the type_suppression. Implementation
> 	lifted from...
> 	(handle_fts_entry): ...here. Call handle_file_entry for each file.
> 	(gen_suppr_spec_from_headers): Also takes a header_files string
> 	vector as argument. Call handle_file_entry for each file entry.
> 	* tools/abidiff.cc (options): Add header_files1 and header_files2
> 	string vectors.
> 	(display_usage): Print --header-file1 and --header-file2 usage.
> 	(parse_command_line): Handle --header-file1, --hf1 and
> 	--header-file2, --hf2.
> 	(set_diff_context_from_opts): Call gen_suppr_spec_from_headers
> 	with header_files1 and header_files2.
> 	(set_suppressions): Likewise.
> 	* tools/abidw.cc (options): Add header_files string vector.
> 	(display_usage): Print --header-file usage.
> 	(parse_command_line): Handle --header-file1, --hf1.
> 	(maybe_check_header_files): New function.
> 	(set_suppressions): Call gen_suppr_spec_from_headers with
> 	header_files.
> 	(main): Call maybe_check_header_files.
> 	* tools/abilint.cc (options): Add header_files string vector.
> 	(display_usage): Print --header-file usage.
> 	(parse_command_line): Handle --header-file1, --hf1.
> 	(set_suppressions): Call gen_suppr_spec_from_headers with
> 	header_files.
> 	* tools/abipkgdiff.cc (maybe_create_private_types_suppressions):
> 	Call gen_suppr_spec_from_headers with an empty string vector.
> 	* doc/manuals/abidiff.rst: Document --header-file1, --hf1 and
> 	--header-file2, --hf2. Add new options to documentation of
> 	--drop-private-types.
> 	* doc/manuals/abidw.rst: Document --header-file, --hf.
> 	* doc/manuals/abilint.rst: Likewise.

Thanks for this nice patch!  I have applied it to master!

I have just made some cosmetic changes to a few parts that I am going to
point to below.  Also, after my comments of your patch, I have attached
a diff of the changes I made to it.  I have also attached the resulting
amended patch.

[...]

> diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
> index 611b4d07..ea4efb0e 100644
> --- a/tools/abipkgdiff.cc
> +++ b/tools/abipkgdiff.cc
> @@ -1529,8 +1529,10 @@ maybe_create_private_types_suppressions(package& pkg, const options &opts)
>    if (!is_dir(headers_path))
>      return false;
>  
> +  // We don't list individual files, just look under the headers_path.
> +  vector<string> no_header_files;
>    suppression_sptr suppr =
> -    gen_suppr_spec_from_headers(headers_path);
> +    gen_suppr_spec_from_headers(headers_path, no_header_files);

In order to avoid this change, I have added an overload for the
gen_suppr_spec_from_headers function that takes just one parameter, like
it was before your patch.  That way, this hunk becomes unnecessary.  You
can see the details for that in the diff representing my changes at the
end of this message.

[...]

> diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
> index 11f6129e..b05ed645 100644
> --- a/src/abg-tools-utils.cc
> +++ b/src/abg-tools-utils.cc
> @@ -1787,6 +1787,40 @@ make_path_absolute_to_be_freed(const char*p)
>    return result;
>  }
>  
> +/// This is a sub-routine of gen_suppr_spec_from_headers and
> +///  handle_fts_entry.
> +///
> +/// @param file add to the to the vector returned by
> +/// type_suppression::get_source_locations_to_keep()
> +///
> +/// @param suppr the file name is going to be added to the vector
> +/// returned by the method type_suppression::get_source_locations_to_keep
> +/// of this instance.
> +/// If this smart pointer is nil then a new instance @ref
> +/// type_suppression is created and this variable is made to point to
> +/// it.

I have slightly edited the comments above.
> +static void
> +handle_file_entry(const string& file,
> +		  type_suppression_sptr& suppr)
> +{

[...]

> @@ -1845,25 +1863,35 @@ handle_fts_entry(const FTSENT *entry,
>  /// @return the resulting type suppression generated, if any file was
>  /// found in the directory tree @p headers_root_dir.
>  type_suppression_sptr
> -gen_suppr_spec_from_headers(const string& headers_root_dir)
> +gen_suppr_spec_from_headers(const string& headers_root_dir,
> +			    const vector<string>& header_files)

I have added a comment for the new parameter of this function.

>  {

[...]

>    type_suppression_sptr result;
>  
> -  if (headers_root_dir.empty())
> -    // We were given no headers root dir so the resulting suppression
> -    // specification shall be empty.
> +  if (headers_root_dir.empty() && header_files.empty())
> +    // We were given no headers root dir and no header files
> +    // so the resulting suppression specification shall be empty.
>      return result;
>  
> -  char* paths[] = {const_cast<char*>(headers_root_dir.c_str()), 0};
> +  if (!headers_root_dir.empty())
> +    {
> +      char* paths[] = {const_cast<char*>(headers_root_dir.c_str()), 0};
>  
> -  FTS *file_hierarchy = fts_open(paths, FTS_LOGICAL|FTS_NOCHDIR, NULL);
> -  if (!file_hierarchy)
> -    return result;
> +      FTS *file_hierarchy = fts_open(paths, FTS_LOGICAL|FTS_NOCHDIR, NULL);
> +      if (!file_hierarchy)
> +	return result;
> +
> +      FTSENT *entry;
> +      while ((entry = fts_read(file_hierarchy)))
> +	handle_fts_entry(entry, result);
> +      fts_close(file_hierarchy);
> +    }
> +
> +  for (vector<string>::const_iterator file = header_files.begin();
> +       file != header_files.end();
> +       ++file)
> +    handle_file_entry(*file, result);
>  
> -  FTSENT *entry;
> -  while ((entry = fts_read(file_hierarchy)))
> -    handle_fts_entry(entry, result);
> -  fts_close(file_hierarchy);
>    return result;
>  }
>

[...]


Here is the diff representing my changes to your patch.

--------------------------------->8<-------------------------------
diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h
index 564fe646..c47e3f4e 100644
--- a/include/abg-tools-utils.h
+++ b/include/abg-tools-utils.h
@@ -83,6 +83,10 @@ string trim_white_space(const string&);
 string trim_leading_string(const string& from, const string& to_trim);
 void convert_char_stars_to_char_star_stars(const vector<char*>&,
 					   vector<char**>&);
+
+suppr::type_suppression_sptr
+gen_suppr_spec_from_headers(const string& hdrs_root_dir);
+
 suppr::type_suppression_sptr
 gen_suppr_spec_from_headers(const string& hdrs_root_dir,
 			    const vector<string>& hdr_files);
diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index b05ed645..a06e8615 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -1788,19 +1788,22 @@ make_path_absolute_to_be_freed(const char*p)
 }
 
 /// This is a sub-routine of gen_suppr_spec_from_headers and
-///  handle_fts_entry.
+/// handle_fts_entry.
 ///
-/// @param file add to the to the vector returned by
+/// It setups a type suppression which is meant to keep types defined
+/// in a given file and suppress all other types.
+///
+/// @param file_path the path to the file that defines types that are
+/// meant to be kept by the type suppression.  All other types defined
+/// in other files are to be suppressed.  Note that this file path is
+/// added to the vector returned by
 /// type_suppression::get_source_locations_to_keep()
 ///
-/// @param suppr the file name is going to be added to the vector
-/// returned by the method type_suppression::get_source_locations_to_keep
-/// of this instance.
-/// If this smart pointer is nil then a new instance @ref
-/// type_suppression is created and this variable is made to point to
-/// it.
+/// @param suppr the type suppression to setup.  If this smart pointer
+/// is nil then a new instance @ref type_suppression is created and
+/// this variable is made to point to it.
 static void
-handle_file_entry(const string& file,
+handle_file_entry(const string& file_path,
 		  type_suppression_sptr& suppr)
 {
   if (!suppr)
@@ -1818,7 +1821,7 @@ handle_file_entry(const string& file,
   // And types that are defined in header files that are under
   // the header directory file we are looking are to be
   // considered public types too.
-  suppr->get_source_locations_to_keep().insert(file);
+  suppr->get_source_locations_to_keep().insert(file_path);
 }
 
 /// This is a sub-routine of gen_suppr_spec_from_headers.
@@ -1854,12 +1857,16 @@ handle_fts_entry(const FTSENT *entry,
 }
 
 /// Generate a type suppression specification that suppresses ABI
-/// changes for types defines in source files that are *NOT* in a give
-/// header root dir.
+/// changes for types defined in source files that are neither in a
+/// given header root dir, not in a set of header files.
 ///
 /// @param headers_root_dir ABI changes in types defined in files
 /// *NOT* found in this directory tree are going be suppressed.
 ///
+/// @param header_files a set of additional header files that define
+/// types that are to be kept (not supressed) by the returned type
+/// suppression.
+///
 /// @return the resulting type suppression generated, if any file was
 /// found in the directory tree @p headers_root_dir.
 type_suppression_sptr
@@ -1895,6 +1902,23 @@ gen_suppr_spec_from_headers(const string& headers_root_dir,
   return result;
 }
 
+/// Generate a type suppression specification that suppresses ABI
+/// changes for types defined in source files that are not in a given
+/// header root dir.
+///
+/// @param headers_root_dir ABI changes in types defined in files
+/// *NOT* found in this directory tree are going be suppressed.
+///
+/// @return the resulting type suppression generated, if any file was
+/// found in the directory tree @p headers_root_dir.
+type_suppression_sptr
+gen_suppr_spec_from_headers(const string& headers_root_dir)
+{
+  // We don't list individual files, just look under the headers_path.
+  vector<string> header_files;
+  return gen_suppr_spec_from_headers(headers_root_dir, header_files);
+}
+
 /// Generate a suppression specification from kernel abi whitelist
 /// files.
 ///
diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index ea4efb0e..611b4d07 100644
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -1529,10 +1529,8 @@ maybe_create_private_types_suppressions(package& pkg, const options &opts)
   if (!is_dir(headers_path))
     return false;
 
-  // We don't list individual files, just look under the headers_path.
-  vector<string> no_header_files;
   suppression_sptr suppr =
-    gen_suppr_spec_from_headers(headers_path, no_header_files);
+    gen_suppr_spec_from_headers(headers_path);
 
   if (suppr)
     {

--------------------------------->8<-------------------------------

And below is the final resulting patch that I have applied to master.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Final amended patch --]
[-- Type: text/x-patch, Size: 22613 bytes --]

From d203555b0817a8cc5c8c25da7d4e30d230b789b2 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Sun, 12 Apr 2020 03:47:56 +0200
Subject: [PATCH] Add --header-file option to add individual public header
 files.

Sometimes public header files are in the same directory as private
header files. In such cases --headers-dir cannot be used. Add
--header-file to add individual public header files.

	* include/abg-tools-utils.h (gen_suppr_spec_from_headers): Add
	hdr_files string vector argument.
	* src/abg-tools-utils.cc (handle_file_entry): New function that
	adds one specific file to the type_suppression. Implementation
	lifted from...
	(handle_fts_entry): ...here. Call handle_file_entry for each file.
	(gen_suppr_spec_from_headers): Also takes a header_files string
	vector as argument. Call handle_file_entry for each file entry.
	* tools/abidiff.cc (options): Add header_files1 and header_files2
	string vectors.
	(display_usage): Print --header-file1 and --header-file2 usage.
	(parse_command_line): Handle --header-file1, --hf1 and
	--header-file2, --hf2.
	(set_diff_context_from_opts): Call gen_suppr_spec_from_headers
	with header_files1 and header_files2.
	(set_suppressions): Likewise.
	* tools/abidw.cc (options): Add header_files string vector.
	(display_usage): Print --header-file usage.
	(parse_command_line): Handle --header-file1, --hf1.
	(maybe_check_header_files): New function.
	(set_suppressions): Call gen_suppr_spec_from_headers with
	header_files.
	(main): Call maybe_check_header_files.
	* tools/abilint.cc (options): Add header_files string vector.
	(display_usage): Print --header-file usage.
	(parse_command_line): Handle --header-file1, --hf1.
	(set_suppressions): Call gen_suppr_spec_from_headers with
	header_files.
	* doc/manuals/abidiff.rst: Document --header-file1, --hf1 and
	--header-file2, --hf2. Add new options to documentation of
	--drop-private-types.
	* doc/manuals/abidw.rst: Document --header-file, --hf.
	* doc/manuals/abilint.rst: Likewise.

Signed-off-by: Mark Wielaard <mark@klomp.org>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 doc/manuals/abidiff.rst   |  33 +++++++----
 doc/manuals/abidw.rst     |   6 ++
 doc/manuals/abilint.rst   |   6 ++
 include/abg-tools-utils.h |   5 ++
 src/abg-tools-utils.cc    | 114 +++++++++++++++++++++++++++-----------
 tools/abidiff.cc          |  46 ++++++++++++---
 tools/abidw.cc            |  35 +++++++++++-
 tools/abilint.cc          |  15 ++++-
 8 files changed, 209 insertions(+), 51 deletions(-)

diff --git a/doc/manuals/abidiff.rst b/doc/manuals/abidiff.rst
index 8d914db5..6aac73bb 100644
--- a/doc/manuals/abidiff.rst
+++ b/doc/manuals/abidiff.rst
@@ -104,12 +104,24 @@ Options
     library that the tool has to consider.  The tool will thus filter
     out ABI changes on types that are not defined in public headers.
 
+  * ``--header-file1 | --hf1`` <header-file-path-1>
+
+    Specifies where to find one public header of the first shared
+    library that the tool has to consider.  The tool will thus filter
+    out ABI changes on types that are not defined in public headers.
+
   * ``--headers-dir2 | --hd2`` <headers-directory-path-1>
 
     Specifies where to find the public headers of the second shared
     library that the tool has to consider.  The tool will thus filter
     out ABI changes on types that are not defined in public headers.
 
+  * ``--header-file2 | --hf2`` <header-file-path-2>
+
+    Specifies where to find one public header of the second shared
+    library that the tool has to consider.  The tool will thus filter
+    out ABI changes on types that are not defined in public headers.
+
   * ``--no-linux-kernel-mode``
 
     Without this option, if abidiff detects that the binaries it is
@@ -141,13 +153,13 @@ Options
 
   * ``--drop-private-types``
 
-    This option is to be used with the ``--headers-dir1`` and
-    ``--headers-dir2`` options.  With this option, types that are
-    *NOT* defined in the headers are entirely dropped from the
-    internal representation build by Libabigail to represent the ABI.
-    They thus don't have to be filtered out from the final ABI change
-    report because they are not even present in Libabigail's
-    representation.
+    This option is to be used with the ``--headers-dir1``,
+    ``header-file1``, ``header-file2`` and ``--headers-dir2`` options.
+    With this option, types that are *NOT* defined in the headers are
+    entirely dropped from the internal representation build by
+    Libabigail to represent the ABI.  They thus don't have to be
+    filtered out from the final ABI change report because they are not
+    even present in Libabigail's representation.
 
     Without this option however, those private types are kept in the
     internal representation and later filtered out from the report.
@@ -218,9 +230,10 @@ Options
 
     This option might incur some serious performance degradation as
     the number of types analyzed can be huge.  However, if paired with
-    the ``--headers-dir{1,2}`` options, the additional non-reachable
-    types analyzed are restricted to those defined in public headers
-    files, thus hopefully making the performance hit acceptable.
+    the ``--headers-dir{1,2}`` and/or ``header-file{1,2}`` options,
+    the additional non-reachable types analyzed are restricted to
+    those defined in public headers files, thus hopefully making the
+    performance hit acceptable.
 
     Also, using this option alongside suppression specifications (by
     also using the ``--suppressions`` option) might help keep the number of
diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index 4843d009..8f6c3c39 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -132,6 +132,12 @@ Options
     library that the tool has to consider.  The tool will thus filter
     out types that are not defined in public headers.
 
+  * ``--header-file | --hf`` <header-file-path>
+
+    Specifies where to find one of the public headers of the abi file
+    that the tool has to consider.  The tool will thus filter out
+    types that are not defined in public headers.
+
   * ``--no-linux-kernel-mode``
 
     Without this option, if abipkgiff detects that the binaries it is
diff --git a/doc/manuals/abilint.rst b/doc/manuals/abilint.rst
index 9e07f47f..4cc0b428 100644
--- a/doc/manuals/abilint.rst
+++ b/doc/manuals/abilint.rst
@@ -76,6 +76,12 @@ Options
     library that the tool has to consider.  The tool will thus filter
     out types that are not defined in public headers.
 
+  * ``--header-file | --hf`` <header-file-path>
+
+    Specifies where to find one of the public headers of the abi file
+    that the tool has to consider.  The tool will thus filter out
+    types that are not defined in public headers.
+
   * ``--stdin | --``
 
     Read the input content from standard input.
diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h
index 053519a4..c47e3f4e 100644
--- a/include/abg-tools-utils.h
+++ b/include/abg-tools-utils.h
@@ -83,9 +83,14 @@ string trim_white_space(const string&);
 string trim_leading_string(const string& from, const string& to_trim);
 void convert_char_stars_to_char_star_stars(const vector<char*>&,
 					   vector<char**>&);
+
 suppr::type_suppression_sptr
 gen_suppr_spec_from_headers(const string& hdrs_root_dir);
 
+suppr::type_suppression_sptr
+gen_suppr_spec_from_headers(const string& hdrs_root_dir,
+			    const vector<string>& hdr_files);
+
 suppr::suppressions_type
 gen_suppr_spec_from_kernel_abi_whitelists
    (const vector<string>& abi_whitelist_paths);
diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index 11f6129e..a06e8615 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -1787,6 +1787,43 @@ make_path_absolute_to_be_freed(const char*p)
   return result;
 }
 
+/// This is a sub-routine of gen_suppr_spec_from_headers and
+/// handle_fts_entry.
+///
+/// It setups a type suppression which is meant to keep types defined
+/// in a given file and suppress all other types.
+///
+/// @param file_path the path to the file that defines types that are
+/// meant to be kept by the type suppression.  All other types defined
+/// in other files are to be suppressed.  Note that this file path is
+/// added to the vector returned by
+/// type_suppression::get_source_locations_to_keep()
+///
+/// @param suppr the type suppression to setup.  If this smart pointer
+/// is nil then a new instance @ref type_suppression is created and
+/// this variable is made to point to it.
+static void
+handle_file_entry(const string& file_path,
+		  type_suppression_sptr& suppr)
+{
+  if (!suppr)
+    {
+      suppr.reset(new type_suppression(get_private_types_suppr_spec_label(),
+				       /*type_name_regexp=*/"",
+				       /*type_name=*/""));
+
+      // Types that are defined in system headers are usually
+      // OK to be considered as public types.
+      suppr->set_source_location_to_keep_regex_str("^/usr/include/");
+      suppr->set_is_artificial(true);
+    }
+
+  // And types that are defined in header files that are under
+  // the header directory file we are looking are to be
+  // considered public types too.
+  suppr->get_source_locations_to_keep().insert(file_path);
+}
+
 /// This is a sub-routine of gen_suppr_spec_from_headers.
 ///
 /// @param entry if this file represents a regular (or symlink) file,
@@ -1815,58 +1852,73 @@ handle_fts_entry(const FTSENT *entry,
       if (string_ends_with(fname, ".h")
 	  || string_ends_with(fname, ".hpp")
 	  || string_ends_with(fname, ".hxx"))
-	{
-	  if (!suppr)
-	    {
-	      suppr.reset(new type_suppression(get_private_types_suppr_spec_label(),
-					       /*type_name_regexp=*/"",
-					       /*type_name=*/""));
-
-	      // Types that are defined in system headers are usually
-	      // OK to be considered as public types.
-	      suppr->set_source_location_to_keep_regex_str("^/usr/include/");
-	      suppr->set_is_artificial(true);
-	    }
-	  // And types that are defined in header files that are under
-	  // the header directory file we are looking are to be
-	  // considered public types too.
-	  suppr->get_source_locations_to_keep().insert(fname);
-	}
+	handle_file_entry (fname, suppr);
     }
 }
 
 /// Generate a type suppression specification that suppresses ABI
-/// changes for types defines in source files that are *NOT* in a give
-/// header root dir.
+/// changes for types defined in source files that are neither in a
+/// given header root dir, not in a set of header files.
 ///
 /// @param headers_root_dir ABI changes in types defined in files
 /// *NOT* found in this directory tree are going be suppressed.
 ///
+/// @param header_files a set of additional header files that define
+/// types that are to be kept (not supressed) by the returned type
+/// suppression.
+///
 /// @return the resulting type suppression generated, if any file was
 /// found in the directory tree @p headers_root_dir.
 type_suppression_sptr
-gen_suppr_spec_from_headers(const string& headers_root_dir)
+gen_suppr_spec_from_headers(const string& headers_root_dir,
+			    const vector<string>& header_files)
 {
   type_suppression_sptr result;
 
-  if (headers_root_dir.empty())
-    // We were given no headers root dir so the resulting suppression
-    // specification shall be empty.
+  if (headers_root_dir.empty() && header_files.empty())
+    // We were given no headers root dir and no header files
+    // so the resulting suppression specification shall be empty.
     return result;
 
-  char* paths[] = {const_cast<char*>(headers_root_dir.c_str()), 0};
+  if (!headers_root_dir.empty())
+    {
+      char* paths[] = {const_cast<char*>(headers_root_dir.c_str()), 0};
 
-  FTS *file_hierarchy = fts_open(paths, FTS_LOGICAL|FTS_NOCHDIR, NULL);
-  if (!file_hierarchy)
-    return result;
+      FTS *file_hierarchy = fts_open(paths, FTS_LOGICAL|FTS_NOCHDIR, NULL);
+      if (!file_hierarchy)
+	return result;
+
+      FTSENT *entry;
+      while ((entry = fts_read(file_hierarchy)))
+	handle_fts_entry(entry, result);
+      fts_close(file_hierarchy);
+    }
+
+  for (vector<string>::const_iterator file = header_files.begin();
+       file != header_files.end();
+       ++file)
+    handle_file_entry(*file, result);
 
-  FTSENT *entry;
-  while ((entry = fts_read(file_hierarchy)))
-    handle_fts_entry(entry, result);
-  fts_close(file_hierarchy);
   return result;
 }
 
+/// Generate a type suppression specification that suppresses ABI
+/// changes for types defined in source files that are not in a given
+/// header root dir.
+///
+/// @param headers_root_dir ABI changes in types defined in files
+/// *NOT* found in this directory tree are going be suppressed.
+///
+/// @return the resulting type suppression generated, if any file was
+/// found in the directory tree @p headers_root_dir.
+type_suppression_sptr
+gen_suppr_spec_from_headers(const string& headers_root_dir)
+{
+  // We don't list individual files, just look under the headers_path.
+  vector<string> header_files;
+  return gen_suppr_spec_from_headers(headers_root_dir, header_files);
+}
+
 /// Generate a suppression specification from kernel abi whitelist
 /// files.
 ///
diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index 44ad1878..162d5ebc 100644
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -79,7 +79,9 @@ struct options
   vector<string>	keep_fn_regex_patterns;
   vector<string>	keep_var_regex_patterns;
   string		headers_dir1;
+  vector<string>        header_files1;
   string		headers_dir2;
+  vector<string>        header_files2;
   bool			drop_private_types;
   bool			linux_kernel_mode;
   bool			no_default_supprs;
@@ -183,7 +185,9 @@ display_usage(const string& prog_name, ostream& out)
     << " --debug-info-dir1|--d1 <path> the root for the debug info of file1\n"
     << " --debug-info-dir2|--d2 <path> the root for the debug info of file2\n"
     << " --headers-dir1|--hd1 <path>  the path to headers of file1\n"
+    << " --header-file1|--hf1 <path>  the path to one header of file1\n"
     << " --headers-dir2|--hd2 <path>  the path to headers of file2\n"
+    << " --header-file2|--hf2 <path>  the path to one header of file2\n"
     << " --drop-private-types  drop private types from "
     "internal representation\n"
     << " --no-linux-kernel-mode  don't consider the input binaries as "
@@ -317,6 +321,19 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir1 = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file1")
+	       || !strcmp(argv[i], "--hf1"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
+	      opts.missing_operand = true;
+	      opts.wrong_option = argv[i];
+	      return true;
+	    }
+	  opts.header_files1.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--headers-dir2")
 	       || !strcmp(argv[i], "--hd2"))
 	{
@@ -330,6 +347,19 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir2 = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file2")
+	       || !strcmp(argv[i], "--hf2"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
+	      opts.missing_operand = true;
+	      opts.wrong_option = argv[i];
+	      return true;
+	    }
+	  opts.header_files2.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--kmi-whitelist")
 	       || !strcmp(argv[i], "-w"))
 	{
@@ -690,22 +720,22 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
       load_default_user_suppressions(supprs);
     }
 
-  if (!opts.headers_dir1.empty())
+  if (!opts.headers_dir1.empty() || !opts.header_files1.empty())
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir1);
+	gen_suppr_spec_from_headers(opts.headers_dir1, opts.header_files1);
       if (suppr)
 	ctxt->add_suppression(suppr);
     }
 
-    if (!opts.headers_dir2.empty())
+  if (!opts.headers_dir2.empty() || !opts.header_files2.empty())
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir2);
+	gen_suppr_spec_from_headers(opts.headers_dir2, opts.header_files2);
       if (suppr)
 	ctxt->add_suppression(suppr);
     }
@@ -740,7 +770,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
     read_suppressions(*i, supprs);
 
   if (read_context_get_path(read_ctxt) == opts.file1
-      && !opts.headers_dir1.empty())
+      && (!opts.headers_dir1.empty() || !opts.header_files1.empty()))
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers for
@@ -750,7 +780,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
       // corpus loading, they are going to be dropped from the
       // internal representation altogether.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir1);
+	gen_suppr_spec_from_headers(opts.headers_dir1, opts.header_files1);
       if (suppr)
 	{
 	  if (opts.drop_private_types)
@@ -760,7 +790,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
     }
 
   if (read_context_get_path(read_ctxt) == opts.file2
-      && !opts.headers_dir2.empty())
+      && (!opts.headers_dir2.empty() || !opts.header_files2.empty()))
     {
       // Generate suppression specification to avoid showing ABI
       // changes on types that are not defined in public headers for
@@ -770,7 +800,7 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
       // corpus loading, they are going to be dropped from the
       // internal representation altogether.
       suppression_sptr suppr =
-	gen_suppr_spec_from_headers(opts.headers_dir2);
+	gen_suppr_spec_from_headers(opts.headers_dir2, opts.header_files2);
       if (suppr)
 	{
 	  if (opts.drop_private_types)
diff --git a/tools/abidw.cc b/tools/abidw.cc
index 3b531999..a8b9ad32 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -88,6 +88,7 @@ struct options
   vector<char*>	di_root_paths;
   vector<char**>	prepared_di_root_paths;
   string		headers_dir;
+  vector<string>	header_files;
   string		vmlinux;
   vector<string>	suppression_paths;
   vector<string>	kabi_whitelist_paths;
@@ -149,6 +150,7 @@ display_usage(const string& prog_name, ostream& out)
     << "  --version|-v  display program version information and exit\n"
     << "  --debug-info-dir|-d <dir-path>  look for debug info under 'dir-path'\n"
     << "  --headers-dir|--hd <path> the path to headers of the elf file\n"
+    << "  --header-file|--hf <path> the path one header of the elf file\n"
     << "  --out-file <file-path>  write the output to 'file-path'\n"
     << "  --noout  do not emit anything after reading the binary\n"
     << "  --suppressions|--suppr <path> specify a suppression file\n"
@@ -217,6 +219,15 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file")
+	       || !strcmp(argv[i], "--hf"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    return false;
+	  opts.header_files.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--out-file"))
 	{
 	  if (argc <= i + 1
@@ -349,6 +360,24 @@ maybe_check_suppression_files(const options& opts)
   return true;
 }
 
+/// Check that the header files supplied are present.
+/// If not, emit an error on stderr.
+///
+/// @param opts the options instance to use.
+///
+/// @return true if all header files are present, false otherwise.
+static bool
+maybe_check_header_files(const options& opts)
+{
+  for (vector<string>::const_iterator file = opts.header_files.begin();
+       file != opts.header_files.end();
+       ++file)
+    if (!check_file(*file, cerr, "abidw"))
+      return false;
+
+  return true;
+}
+
 /// Set suppression specifications to the @p read_context used to load
 /// the ABI corpus from the ELF/DWARF file.
 ///
@@ -371,7 +400,8 @@ set_suppressions(read_context& read_ctxt, options& opts)
     read_suppressions(*i, supprs);
 
   suppression_sptr suppr =
-    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir);
+    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir,
+						      opts.header_files);
   if (suppr)
     supprs.push_back(suppr);
 
@@ -721,6 +751,9 @@ main(int argc, char* argv[])
   if (!maybe_check_suppression_files(opts))
     return 1;
 
+  if (!maybe_check_header_files(opts))
+    return 1;
+
   abigail::tools_utils::file_type type =
     abigail::tools_utils::guess_file_type(opts.in_file_path);
   if (type != abigail::tools_utils::FILE_TYPE_ELF
diff --git a/tools/abilint.cc b/tools/abilint.cc
index b06ffd72..e9d6f93e 100644
--- a/tools/abilint.cc
+++ b/tools/abilint.cc
@@ -86,6 +86,7 @@ struct options
   abg_compat::shared_ptr<char>	di_root_path;
   vector<string>		suppression_paths;
   string			headers_dir;
+  vector<string>		header_files;
 
   options()
     : display_version(false),
@@ -107,6 +108,8 @@ display_usage(const string& prog_name, ostream& out)
     << "  --debug-info-dir <path> the path under which to look for "
     << "  --headers-dir|--hd <patch> the path to headers of the elf file\n"
     "debug info for the elf <abi-file>\n"
+    << "  --header-file|--hf <path> the path to one header of the elf file\n"
+    "debug info for the elf <abi-file>\n"
     << "  --suppressions|--suppr <path> specify a suppression file\n"
     << "  --diff  for xml inputs, perform a text diff between "
     "the input and the memory model saved back to disk\n"
@@ -161,6 +164,15 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  opts.headers_dir = argv[j];
 	  ++i;
 	}
+      else if (!strcmp(argv[i], "--header-file")
+	       || !strcmp(argv[i], "--hf"))
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    return false;
+	  opts.header_files.push_back(argv[j]);
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--suppressions")
 	       || !strcmp(argv[i], "--suppr"))
 	{
@@ -240,7 +252,8 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
     read_suppressions(*i, supprs);
 
   suppression_sptr suppr =
-    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir);
+    abigail::tools_utils::gen_suppr_spec_from_headers(opts.headers_dir,
+						      opts.header_files);
   if (suppr)
     supprs.push_back(suppr);
 
-- 
2.25.0


[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


-- 
		Dodji

Re: [PATCH 2/2] Add --drop-private-types to abidw.

[Dodji Seketeli]

Mark Wielaard <mark@klomp.org> a écrit:

> To create smaller abi XML files it is useful to be able to drop
> the private types from the representation.
>
> 	* tools/abidw.cc (options): Add drop_private_types bool.
> 	(display_usage): Add --drop-private-types.
> 	(parse_command_line): Parse --drop-private-types, set opts.
> 	(set_suppressions): Call set_drops_artifact_from_ir when
> 	drop_private_types set.
> 	* doc/manuals/abidw.rst: Document --drop-private-types.

Applied to master.  Thanks!

Cheers,

-- 
		Dodji