How to drop all non-global/defined function declarations?

How to drop all non-global/defined function declarations?

[Mark Wielaard]

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

Hi,

This is somewhat similar to my previous struggle with dropping
unused/opaque types from the abi xml file. My ultimate goal is to
generate an abi xml file that contains just the variables, functions
and types defined/exported by my library. I like to check these into
our git repo and so like to keep them as small as possible and not
have them change for variable, type or function changes that are
internal only and won't be used when checking for abi changes.

Attached is an extension of the previous example, now extended with an
internal function libfoo_frob which isn't exported, but only used
internally. The actual exported functions have been added to a symbol
version file.

When generating the abidw xml file both the internal function and
functions used by the library (free and malloc in this case) are
included. The four exported functions (create_foo, destroy_foo,
get_foo and set_foo) do have corresponding elf-symbols in the abi xml
file.

Ideally I'll simply drop any function declaration that doesn't have a
corresponding global elf-symbol. But the suppression format doesn't
seem to support that.

I am able to drop the internal libfoo_frob internal function using:

[suppress_function]
  file_name_not_regexp = foo.h
  drop = yes

But for some reason that doesn't drop the malloc and free function
declarations.

Also this shouldn't actually work according to the suppress_function
drop documentation:

  Please note that for this property to be effective, the enclosing
  suppression specification must have at least one of the following
  properties specified: name_regexp, name, name_regexp,
  source_location_not_in or source_location_not_regexp.

It doesn't list file_name_not_regexp (and source_location_* isn't
documentated as a suppress_function property). I assume that is a
typo, but now I am confused which properties do/don't work with drop.

Is it a bug that malloc and free aren't dropped in this example?
And/Or can we get a way to drop any non-exported function declaration
(any that doesn't have a corresponding global elf-symbol)?

Thanks,

Mark

[-- Attachment #2: foolib.tar.gz --]
[-- Type: application/gzip, Size: 15923 bytes --]

Re: How to drop all non-global/defined function declarations?

[Dodji Seketeli]

Hello Mark,

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

> Hi,
>
> This is somewhat similar to my previous struggle with dropping
> unused/opaque types from the abi xml file. My ultimate goal is to
> generate an abi xml file that contains just the variables, functions
> and types defined/exported by my library. I like to check these into
> our git repo and so like to keep them as small as possible and not
> have them change for variable, type or function changes that are
> internal only and won't be used when checking for abi changes.
>
> Attached is an extension of the previous example, now extended with an
> internal function libfoo_frob which isn't exported, but only used
> internally. The actual exported functions have been added to a symbol
> version file.
>
> When generating the abidw xml file both the internal function and
> functions used by the library (free and malloc in this case) are
> included. The four exported functions (create_foo, destroy_foo,
> get_foo and set_foo) do have corresponding elf-symbols in the abi xml
> file.

From what I see, just invoking abidw on libfoo.so (without any
suppression specification) captures only the exported functions (which
have global function symbol in the binary) of the binary, that is:
create_foo, destroy_foo, get_foo and set_foo as well as free and malloc.

The libfoo_frob function which is not exported (doesn't have a global
ELF symbol) is not captured.

> Ideally I'll simply drop any function declaration that doesn't have a
> corresponding global elf-symbol. But the suppression format doesn't
> seem to support that.

I think that is supported already by default.  What is not supported
however is to filter out functions that are exported by the binary,
based on the source location of their definition point, for instance.

> I am able to drop the internal libfoo_frob internal function using:
>
> [suppress_function]
>   file_name_not_regexp = foo.h
>   drop = yes

I think you didn't need to do this.  libfoo_frob is dropped by default,
because it's not exported.

Besides, the file_name_not_regexp property is for the name of the
*binary* (e.g libfoo.so) that contains the function you want to keep.
It's not about the declaration (or definition) point (in the source
code) of the function you are interested in.

> But for some reason that doesn't drop the malloc and free function
> declarations.

Right.  At the moment, there is no way to filter out a function that is
actually exported by the binary like this.  I guess the original idea
was to prevent users from accidentally missing a potential ABI
incomptable change due to the toolchain or something like that.  Also,
because those toolchain-generated functions usually don't come with
debug info, including them in the abixml file doesn't take much space.

But as usual, if you argue that this is would be a useful feature, then
we can add an option to handle this usecase.  We just need to discuss
the details of that option, I guess.



>
> Also this shouldn't actually work according to the suppress_function
> drop documentation:
>
>   Please note that for this property to be effective, the enclosing
>   suppression specification must have at least one of the following
>   properties specified: name_regexp, name, name_regexp,
>   source_location_not_in or source_location_not_regexp.
>
> It doesn't list file_name_not_regexp (and source_location_* isn't
> documentated as a suppress_function property). I assume that is a
> typo, but now I am confused which properties do/don't work with drop.

Right, just having file_name_not_regexp in there doesn't do anything
currently.  The effect you are seeing is the effect of the default
behaviour that doesn't take the suppression specification file into
account.

I hope this helps to clarify the situation somewhat.

Cheers,


-- 
		Dodji

Re: How to drop all non-global/defined function declarations?

[Mark Wielaard]

Hi Dodji,

On Tue, 2020-04-07 at 11:38 +0200, Dodji Seketeli wrote:
> From what I see, just invoking abidw on libfoo.so (without any
> suppression specification) captures only the exported functions (which
> have global function symbol in the binary) of the binary, that is:
> create_foo, destroy_foo, get_foo and set_foo as well as free and malloc.

Why are free and malloc included? They are explicitly marked as
undefined in the symbol table and the xml abi definition doesn't
include an elf-function-symbol for them?

> The libfoo_frob function which is not exported (doesn't have a global
> ELF symbol) is not captured.

You are right, for my example, but I see internal, not exported,
symbols in my actual library libelf, an example being __libelf_seterrno
which is definitely LOCAL. Will try to come up with a smaller
reproducer. But you can simply try by running libdw on libelf.so.
__libelf_seterrno will be included if you have debuginfo installed (but
not without debuginfo).

> > But for some reason that doesn't drop the malloc and free function
> > declarations.
> 
> Right.  At the moment, there is no way to filter out a function that is
> actually exported by the binary like this.  I guess the original idea
> was to prevent users from accidentally missing a potential ABI
> incomptable change due to the toolchain or something like that.  Also,
> because those toolchain-generated functions usually don't come with
> debug info, including them in the abixml file doesn't take much space.
> 
> But as usual, if you argue that this is would be a useful feature, then
> we can add an option to handle this usecase.  We just need to discuss
> the details of that option, I guess.

I think them being undefined in the symbol table make them unexported.
They are just there to show the library consumes that symbol. That is
also an abi issue, but not one I am interested in. Users of my library
should assume that my library makes sure such symbols will be resolved
(by having the correct DT_NEEDED entries), but they cannot use that
information themselves to rely on those symbols being there.

The main issue is that they are really noise. They don't come with an
argument list or a real return type. And they make the xml abi file
harder to compare (visually).

My goal here is to have an xml abi file checked into git. Then when we
update the abi, it should be accompanied by a change in the xml abi
file. The smaller and more concise we can make the xml abi file the
clearer it is what we are exactly changing that impacts the abi.

Thanks,

Mark

Re: How to drop all non-global/defined function declarations?

[Dodji Seketeli]

Hello Mark,

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

> Hi Dodji,
>
> On Tue, 2020-04-07 at 11:38 +0200, Dodji Seketeli wrote:
>> From what I see, just invoking abidw on libfoo.so (without any
>> suppression specification) captures only the exported functions (which
>> have global function symbol in the binary) of the binary, that is:
>> create_foo, destroy_foo, get_foo and set_foo as well as free and malloc.
>
> Why are free and malloc included? They are explicitly marked as
> undefined in the symbol table and the xml abi definition doesn't
> include an elf-function-symbol for them?

They are included because we capture all global function and variable
symbols, including those that are undefined.  That can be useful down
the road for tools like abicompat:
https://sourceware.org/libabigail/manual/abicompat.html.  It's true that
they are not defined by the librarie so if you know that you need those,
so yeah, maybe we should add an option to get rid of them.  Right now,
you can't.

>
>> The libfoo_frob function which is not exported (doesn't have a global
>> ELF symbol) is not captured.
>
> You are right, for my example, but I see internal, not exported,
> symbols in my actual library libelf, an example being __libelf_seterrno
> which is definitely LOCAL. Will try to come up with a smaller
> reproducer. But you can simply try by running libdw on libelf.so.
> __libelf_seterrno will be included if you have debuginfo installed (but
> not without debuginfo).

From the top of my head, I'd say that is a bug then.  But I'd need to
look into it deeper to be sure.

>
>> > But for some reason that doesn't drop the malloc and free function
>> > declarations.
>> 
>> Right.  At the moment, there is no way to filter out a function that is
>> actually exported by the binary like this.  I guess the original idea
>> was to prevent users from accidentally missing a potential ABI
>> incomptable change due to the toolchain or something like that.  Also,
>> because those toolchain-generated functions usually don't come with
>> debug info, including them in the abixml file doesn't take much space.
>> 
>> But as usual, if you argue that this is would be a useful feature, then
>> we can add an option to handle this usecase.  We just need to discuss
>> the details of that option, I guess.
>
> I think them being undefined in the symbol table make them unexported.
> They are just there to show the library consumes that symbol.

You are right.

> That is also an abi issue, but not one I am interested in.

Well, that's the gotcha I think :-)

In your particular use case, you are not interested.  But others might
use that information, to for instance, build the transitive closure of
dependencies (in terms of either symbols or libraries) of the binary
which ABI is being modelled by the current abixml file.  That's
precisely why we are adding this information in.  As I said above, the
abicompat tool need that information, for instance.  Users of the
library might need it too.


[...]

>
> The main issue is that they are really noise. They don't come with an
> argument list or a real return type.

If you think having that information in is a real problem for you, we
can find a way to let you remove it.  But I am not yet convinced that
it's better to remove it by default because it might be useful for other
use cases, and as it stands right now, the size of undefined entry
points in the abixml file is quite small.

> And they make the xml abi file harder to compare (visually).

Hmmh.  I think I see your point here.  But Historically, the abixml file
wasn't meant to be compared textually.  The tool to compare abixml files
is abidiff and it knows how to deal with that.

> My goal here is to have an xml abi file checked into git. Then when we
> update the abi, it should be accompanied by a change in the xml abi
> file. The smaller and more concise we can make the xml abi file the
> clearer it is what we are exactly changing that impacts the abi.

I get that.  We try to make this possible as much as we can.  We also
have other constraints to satisfy.  We try to make all these ends meet.

Cheers,

-- 
		Dodji

Re: How to drop all non-global/defined function declarations?

[Mark Wielaard]

Hi Dodji,

On Fri, Apr 10, 2020 at 10:48:14AM +0200, Dodji Seketeli wrote:
> Mark Wielaard <mark@klomp.org> a écrit:
> > Why are free and malloc included? They are explicitly marked as
> > undefined in the symbol table and the xml abi definition doesn't
> > include an elf-function-symbol for them?
> 
> They are included because we capture all global function and variable
> symbols, including those that are undefined.  That can be useful down
> the road for tools like abicompat:
> https://sourceware.org/libabigail/manual/abicompat.html.  It's true that
> they are not defined by the librarie so if you know that you need those,
> so yeah, maybe we should add an option to get rid of them.  Right now,
> you can't.

I have a patch to get rid of them for the dwarf reader and abidw.
Which can be extended to the xml reader and other tools if userful.

> > I think them being undefined in the symbol table make them unexported.
> > They are just there to show the library consumes that symbol.
> 
> You are right.
> 
> > That is also an abi issue, but not one I am interested in.
> 
> Well, that's the gotcha I think :-)
> 
> In your particular use case, you are not interested.  But others might
> use that information, to for instance, build the transitive closure of
> dependencies (in terms of either symbols or libraries) of the binary
> which ABI is being modelled by the current abixml file.  That's
> precisely why we are adding this information in.  As I said above, the
> abicompat tool need that information, for instance.  Users of the
> library might need it too.

You are right too. I hadn't considered abicompat. It is certainly
useful (and necessary) in the case.

> > The main issue is that they are really noise. They don't come with an
> > argument list or a real return type.
> 
> If you think having that information in is a real problem for you, we
> can find a way to let you remove it.  But I am not yet convinced that
> it's better to remove it by default because it might be useful for other
> use cases, and as it stands right now, the size of undefined entry
> points in the abixml file is quite small.
> 
> > And they make the xml abi file harder to compare (visually).
> 
> Hmmh.  I think I see your point here.  But Historically, the abixml file
> wasn't meant to be compared textually.  The tool to compare abixml files
> is abidiff and it knows how to deal with that.
> 
> > My goal here is to have an xml abi file checked into git. Then when we
> > update the abi, it should be accompanied by a change in the xml abi
> > file. The smaller and more concise we can make the xml abi file the
> > clearer it is what we are exactly changing that impacts the abi.
> 
> I get that.  We try to make this possible as much as we can.  We also
> have other constraints to satisfy.  We try to make all these ends meet.

I'll sent a patch for abidw --drop-undefined-syms which should help my case.

Cheers,

Mark

[PATCH] Add --drop-undefined-syms to abidw.

[Mark Wielaard]

Add a drop_undefined_syms properties to the read_context that
indicates the reader wants to drop any undefined symbols (which don't
have associated addresses in the corpus). Implement this for the
dwarf_reader and abidw (when using the --drop-undefined-syms option).

	* include/abg-dwarf-reader.h (set_drop_undefined_syms):
	New declaration.
	* src/abg-dwarf-reader.cc (class read_context): Add private
	bool drop_undefined_syms_.
	(drop_undefined_syms): New getter and setter method.
	(set_drop_undefined_syms): New function.
	(function_is_suppressed): Check drop_undefined_syms on
	read_context.
	(variable_is_suppressed): Likewise.
	* src/abg-reader.cc (read_context): Add private bool
	m_drop_undefined_syms.
	(drop_undefined_syms): New getter and setter method.
	* tools/abidw.cc (struct options): Add drop_undefined_syms.
	(display_usage): Print --drop-undefined-syms.
	(parse_command_line): Parse --drop-undefined-syms.
	(main): Call set_drop_undefined_syms.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 doc/manuals/abidw.rst      |  7 +++++++
 include/abg-dwarf-reader.h |  4 ++++
 src/abg-dwarf-reader.cc    | 40 ++++++++++++++++++++++++++++++++++++--
 src/abg-reader.cc          | 21 +++++++++++++++++++-
 tools/abidw.cc             |  8 +++++++-
 5 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index 05a90003..6cc4693c 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -146,6 +146,13 @@ Options
     representation build by Libabigail to represent the ABI and will
     not end up in the abi XML file.
 
+  * ``--drop-undefined-syms``
+
+    With this option functions or variables for which the (exported)
+    ELF symbol is undefined are 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/include/abg-dwarf-reader.h b/include/abg-dwarf-reader.h
index 3b783638..d0329aed 100644
--- a/include/abg-dwarf-reader.h
+++ b/include/abg-dwarf-reader.h
@@ -188,6 +188,10 @@ void
 set_show_stats(read_context& ctxt,
 	       bool f);
 
+void
+set_drop_undefined_syms(read_context& ctxt,
+			bool f);
+
 void
 set_do_log(read_context& ctxt, bool f);
 
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 249e196d..f74fb144 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -3259,6 +3259,7 @@ public:
   string			elf_architecture_;
   corpus::exported_decls_builder* exported_decls_builder_;
   options_type			options_;
+  bool				drop_undefined_syms_;
   read_context();
 
 public:
@@ -3427,6 +3428,7 @@ public:
     options_.env = environment;
     options_.load_in_linux_kernel_mode = linux_kernel_mode;
     options_.load_all_types = load_all_types;
+    drop_undefined_syms_ = false;
     load_in_linux_kernel_mode(linux_kernel_mode);
   }
 
@@ -3518,6 +3520,23 @@ public:
   env(ir::environment* env)
   {options_.env = env;}
 
+  /// Getter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @return true iff we are dropping functions and variables that have
+  /// undefined symbols.
+  bool
+  drop_undefined_syms() const
+  {return drop_undefined_syms_;}
+
+  /// Setter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @param f the new value of the flag.
+  void
+  drop_undefined_syms(bool f)
+  {drop_undefined_syms_ = f;}
+
   /// Getter of the suppression specifications to be used during
   /// ELF/DWARF parsing.
   ///
@@ -9494,6 +9513,18 @@ void
 set_show_stats(read_context& ctxt, bool f)
 {ctxt.show_stats(f);}
 
+/// Setter of the "drop_undefined_syms" flag.
+///
+/// This flag tells if we should drop functions or variables
+/// with undefined symbols.
+///
+/// @param ctxt the read context to consider for this flag.
+///
+/// @param f the value of the flag.
+void
+set_drop_undefined_syms(read_context& ctxt, bool f)
+{ctxt.drop_undefined_syms(f);}
+
 /// Setter of the "do_log" flag.
 ///
 /// This flag tells if we should emit verbose logs for various
@@ -16683,7 +16714,9 @@ function_is_suppressed(const read_context& ctxt,
   string qualified_name = build_qualified_name(scope, fname);
 
   // A non-member function which symbol is not exported is suppressed.
-  if (!is_class_type(scope) && !die_is_declaration_only(function_die))
+  // Or if the reader context drops undefined symbols.
+  if ((!is_class_type(scope) && !die_is_declaration_only(function_die))
+      || ctxt.drop_undefined_syms())
     {
       Dwarf_Addr fn_addr;
       if (!ctxt.get_function_address(function_die, fn_addr))
@@ -16798,7 +16831,10 @@ variable_is_suppressed(const read_context& ctxt,
   // another concrete variable, then this function looks at
   // suppression specification specifications to know if its
   // suppressed.
-  if (!is_class_type(scope) && !is_required_decl_spec)
+  //
+  // Or if the reader context drops undefined symbols.
+  if ((!is_class_type(scope) && !is_required_decl_spec)
+      || ctxt.drop_undefined_syms())
     {
       Dwarf_Addr var_addr = 0;
       if (!ctxt.get_variable_address(variable_die, var_addr))
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 340223d0..dcaa27e1 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -131,6 +131,7 @@ private:
   corpus::exported_decls_builder*			m_exported_decls_builder;
   suppr::suppressions_type				m_supprs;
   bool							m_tracking_non_reachable_types;
+  bool							m_drop_undefined_syms;
 
   read_context();
 
@@ -141,7 +142,8 @@ public:
       m_reader(reader),
       m_corp_node(),
       m_exported_decls_builder(),
-      m_tracking_non_reachable_types()
+      m_tracking_non_reachable_types(),
+      m_drop_undefined_syms()
   {}
 
   /// Getter for the flag that tells us if we are tracking types that
@@ -162,6 +164,23 @@ public:
   tracking_non_reachable_types(bool f)
   {m_tracking_non_reachable_types = f;}
 
+  /// Getter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @return true iff we are dropping functions and variables that have
+  /// undefined symbols.
+  bool
+  drop_undefined_syms() const
+  {return m_drop_undefined_syms;}
+
+  /// Setter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @param f the new value of the flag.
+  void
+  drop_undefined_syms(bool f)
+  {m_drop_undefined_syms = f;}
+
   /// Getter of the path to the ABI file.
   ///
   /// @return the path to the native xml abi file.
diff --git a/tools/abidw.cc b/tools/abidw.cc
index 8b163ccd..72a8b0f1 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -110,6 +110,7 @@ struct options
   bool			annotate;
   bool			do_log;
   bool			drop_private_types;
+  bool			drop_undefined_syms;
 
   options()
     : display_version(),
@@ -128,7 +129,8 @@ struct options
       abidiff(),
       annotate(),
       do_log(),
-      drop_private_types(false)
+      drop_private_types(false),
+      drop_undefined_syms(false)
   {}
 
   ~options()
@@ -161,6 +163,7 @@ display_usage(const string& prog_name, ostream& out)
     << "  --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"
+    << "  --drop-undefined-syms  drop undefined symbols 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"
@@ -299,6 +302,8 @@ parse_command_line(int argc, char* argv[], options& opts)
 	opts.load_all_types = true;
       else if (!strcmp(argv[i], "--drop-private-types"))
 	opts.drop_private_types = true;
+      else if (!strcmp(argv[i], "--drop-undefined-syms"))
+	opts.drop_undefined_syms = true;
       else if (!strcmp(argv[i], "--no-linux-kernel-mode"))
 	opts.linux_kernel_mode = false;
       else if (!strcmp(argv[i], "--abidiff"))
@@ -785,6 +790,7 @@ main(int argc, char* argv[])
 						opts.load_all_types,
 						opts.linux_kernel_mode);
       read_context& ctxt = *c;
+      set_drop_undefined_syms(ctxt, opts.drop_undefined_syms);
       set_show_stats(ctxt, opts.show_stats);
       set_suppressions(ctxt, opts);
       abigail::dwarf_reader::set_do_log(ctxt, opts.do_log);
-- 
2.20.1

Re: [PATCH] Add --drop-undefined-syms to abidw.

[Dodji Seketeli]

Hello Mark,

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

> Add a drop_undefined_syms properties to the read_context that
> indicates the reader wants to drop any undefined symbols (which don't
> have associated addresses in the corpus). Implement this for the
> dwarf_reader and abidw (when using the --drop-undefined-syms option).
>
> 	* include/abg-dwarf-reader.h (set_drop_undefined_syms):
> 	New declaration.
> 	* src/abg-dwarf-reader.cc (class read_context): Add private
> 	bool drop_undefined_syms_.
> 	(drop_undefined_syms): New getter and setter method.
> 	(set_drop_undefined_syms): New function.
> 	(function_is_suppressed): Check drop_undefined_syms on
> 	read_context.
> 	(variable_is_suppressed): Likewise.
> 	* src/abg-reader.cc (read_context): Add private bool
> 	m_drop_undefined_syms.
> 	(drop_undefined_syms): New getter and setter method.
> 	* tools/abidw.cc (struct options): Add drop_undefined_syms.
> 	(display_usage): Print --drop-undefined-syms.
> 	(parse_command_line): Parse --drop-undefined-syms.
> 	(main): Call set_drop_undefined_syms.

I have applied this to master.  I have just fixed a few things that I
discuss below.

[...]


> diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
> index 249e196d..f74fb144 100644
> --- a/src/abg-dwarf-reader.cc
> +++ b/src/abg-dwarf-reader.cc

[...]


>  /// This flag tells if we should emit verbose logs for various
> @@ -16683,7 +16714,9 @@ function_is_suppressed(const read_context& ctxt,
>    string qualified_name = build_qualified_name(scope, fname);
>  
>    // A non-member function which symbol is not exported is suppressed.
> -  if (!is_class_type(scope) && !die_is_declaration_only(function_die))
> +  // Or if the reader context drops undefined symbols.
> +  if ((!is_class_type(scope) && !die_is_declaration_only(function_die))
> +      || ctxt.drop_undefined_syms())
>      {

For languages like C++, we don't want to remove member functions (those
that are at class scope) even if they don't have defined symbols (e.g,
inline member functions).  So here is how I am writing this:


  // A non-member non-static function which symbol is not exported is
  // suppressed.
  //
  // Note that if the non-member non-static function has an undefined
  // symbol, by default, it's not suppressed.  Unless we are asked to
  // drop undefined symbols too.
  if (!is_class_type(scope)
      && (!die_is_declaration_only(function_die)
	  || ctxt.drop_undefined_syms()))
    {

[...]


> @@ -16798,7 +16831,10 @@ variable_is_suppressed(const read_context& ctxt,
>    // another concrete variable, then this function looks at
>    // suppression specification specifications to know if its
>    // suppressed.
> -  if (!is_class_type(scope) && !is_required_decl_spec)
> +  //
> +  // Or if the reader context drops undefined symbols.
> +  if ((!is_class_type(scope) && !is_required_decl_spec)
> +      || ctxt.drop_undefined_syms())
>      {

So, right now, undefined variable (unlike for functions) are dropped
from the IR, unless they are data members or declaration of static data
members.  So we don't need the ctxt.drop_undefined_syms() here.  So I
re-wrote this as:


  // If a non member variable that is a declaration (has no defined
  // and exported symbol) and is not the specification of another
  // concrete variable, then it's suppressed.  This is a size
  // optimization; it removes useless declaration-only variables from
  // the IR.
  if (!is_class_type(scope) && !is_required_decl_spec)
    {

[...]


Thanks!

Cheers,

-- 
		Dodji