]> sourceware.org Git - libabigail.git/commitdiff
Bug 21058 - abipkgdiff wrongly drops non-public types
authorDodji Seketeli <dodji@redhat.com>
Wed, 18 Jan 2017 09:10:10 +0000 (10:10 +0100)
committerDodji Seketeli <dodji@redhat.com>
Wed, 18 Jan 2017 09:14:02 +0000 (10:14 +0100)
When using abipkgdiff types that are defined in files not present in
the devel packages provided by the --devel1 and --devel2 option are
dropped from the internal representation by default.

This has been designed as such, not only to avoid showing changes on
types that are not part of the public headers of a shared library, but
also to help lower the memory consumption of libabigail.

In this particular bug report, we see a library that uses types (in
its public interface) that are defined in headers of a *different*
package.  For instance, suppose a particular package foo that uses
types defined in headers of the glib package.  And some of those Glib
types can be present in its public interface.

So in this case, libabigail is dropping a type that is actually part
of the public interface of the library that is being analyzed, even if
the type was not defined in the devel package of the current package.

This patch addresses the issue by doing a number of things:

    1/ If a type is defined in a file which path starts with
    "/usr/include/", then consider it as a public type.  This is so
    that type coming from the public interface of other packages, and
    that are defined in system headers are considered as part of the
    public types of the package being analyzed.

    2/ by default, don't drop types not defined in the associated
    devel package.  This will hinder our ability to decrease the
    memory usage, but there have been a number of recent optimization
    that help in that regard independently.  So I am hoping this
    shouldn't have a big impact now.

    Incidentally, the option --dont-drop-private-types (from abidiff)
    is changed into --drop-private-types, so that interested users can
    still drop non-private types from the model, if they wish.  That
    --drop-private-types option is added to abipkgdiff too.

As the offended types are not dropped from the model anymore, the
usual filtering mechanisms of libabigail can take place.

* doc/manuals/abidiff.rst (--dont-drop-private-types): Remove documentation.
(--drop-private-types): Document this new option.
* src/abg-tools-utils.cc: Update copyright notice
(handle_fts_entry): On the generated suppression specification, do
not set the flag to drop matched types.  Also, don't match types
defined in files which patch start with "/usr/include/".
* tools/abidiff.cc (options::options): Initialize the
drop_private_types data member to false.
(display_usage): Remove usage string for
--dont-drop-private-types.  Add a new one for
--drop-private-types.
(parse_command_line): Don't part --dont-drop-private-types,
rather, parse --drop-private-types.
(set_suppressions): When the suppression for private types is
generated, if --drop-private-types was provided, then instruct the
suppression to drop matched types.
* tools/abipkgdiff.cc (options::drop_private_types): New option.
(options::options): Initialize the new drop_private_types data
member to false.
(display_usage): Add a usage string for --drop-private-types.
(parse_command_line): Parse the new --drop-private-types option.
(maybe_create_private_types_suppressions): Don't take just a
package, but a package_descriptor because the latter carries the
options.  So when the user used the --drop-private-types option,
make the generated private types suppression to drop matched
types.
* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt:
Adjust.
* tests/test-diff-suppr.cc (in_out_specs): Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
doc/manuals/abidiff.rst
doc/manuals/abipkgdiff.rst
src/abg-tools-utils.cc
tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-1.txt
tests/test-diff-suppr.cc
tools/abidiff.cc
tools/abipkgdiff.cc

index 2a2aabfaf6f320cd0f4e9ad6d7500c5e57f848b9..708c6cd7c264bd2ade2e868554ffa6960307c473 100644 (file)
@@ -107,22 +107,23 @@ Options
     ELF binary.  It thus considers functions and variables which are
     defined and exported in the ELF sense.
 
-  * ``--dont-drop-private-types``
+  * ``--drop-private-types``
 
     This option is to be used with the ``--headers-dir1`` and
-    ``--headers-dir2`` options.  Without this option, types that are
+    ``--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.
 
-    With this option however, those private types are kept in the
+    Without this option however, those private types are kept in the
     internal representation and later filtered out from the report.
 
-    This options thus potentially makes Libabigail to potentially
-    consume more memory.  It's meant to be mainly used for debugging
-    purposes.
+    This options thus potentially makes Libabigail consume less
+    memory.  It's meant to be mainly used to optimize the memory
+    consumption of the tool on binaries with a lot of publicly defined
+    and exported types.
 
   * ``--stat``
 
index 351865a2cb800271a6c60b2b347b0835a598fa69..5623967e1cec36a4761b0cde2cd271d0154da66f 100644 (file)
@@ -100,6 +100,24 @@ Options
     filters out reports about ABI changes to types that are *NOT*
     defined in these header files.
 
+  * ``--drop-private-types``
+
+    This option is to be used with the ``--devel-pkg1`` and
+    ``--devel-pkg2`` 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.
+
+    This options thus potentially makes Libabigail consume less
+    memory.  It's meant to be mainly used to optimize the memory
+    consumption of the tool on binaries with a lot of publicly defined
+    and exported types.
+
   * ``--dso-only``
 
     Compare ELF files that are shared libraries, only.  Do not compare
index 0b2457a64fec4c3786552ea6bf719dccab32ab97..a3e9a45990d0dcb53545d91e5193392877611e7d 100644 (file)
@@ -1,6 +1,6 @@
 // -*- Mode: C++ -*-
 //
-// Copyright (C) 2013-2016 Red Hat, Inc.
+// Copyright (C) 2013-2017 Red Hat, Inc.
 //
 // This file is part of the GNU Application Binary Interface Generic
 // Analysis and Instrumentation Library (libabigail).  This library is
@@ -853,11 +853,19 @@ handle_fts_entry(const FTSENT *entry,
          || string_ends_with(fname, ".hxx"))
        {
          if (!suppr)
-           suppr.reset(new type_suppression(PRIVATE_TYPES_SUPPR_SPEC_NAME,
-                                            /*type_name_regexp=*/"",
-                                            /*type_name=*/""));
-         suppr->set_is_artificial(true);
-         suppr->set_drops_artifact_from_ir(true);
+           {
+             suppr.reset(new type_suppression(PRIVATE_TYPES_SUPPR_SPEC_NAME,
+                                              /*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);
        }
     }
index abdb69d6c527e874c3b49fee3aabded1fc3a8518..d92d21ee58d9e8831cde8a4d5a5a18efdbb82998 100644 (file)
@@ -1,5 +1,5 @@
 ================ changes of 'libtbb.so.2'===============
-  Functions changes summary: 0 Removed, 7 Changed (9 filtered out), 17 Added functions
+  Functions changes summary: 0 Removed, 7 Changed (17 filtered out), 17 Added functions
   Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
   Function symbols changes summary: 0 Removed, 0 Added function symbol not referenced by debug info
   Variable symbols changes summary: 3 Removed, 0 Added variable symbols not referenced by debug info
@@ -49,7 +49,7 @@
           type size hasn't changed
           1 data member insertion:
             'tbb::internal::cpu_ctl_env_space tbb::task_group_context::my_cpu_ctl_env', at offset 896 (in bits) at task.h:380:1
-          1 data member changes (1 filtered):
+          1 data member changes (2 filtered):
            type of 'char tbb::task_group_context::_leading_padding[80]' changed:
              type name changed from 'char[80]' to 'char[72]'
              array type size changed from 640 to 576 bits:
index 09876635b07de7e9a4bae4f2a5b0d1d6fd2fbcc6..80eabf5c623dfe51f5343167bc1d874d1c859d74 100644 (file)
@@ -1604,7 +1604,7 @@ InOutSpec in_out_specs[] =
     "",
     "",
     "",
-    "--no-default-suppression --dont-drop-private-types",
+    "--no-default-suppression",
     "data/test-diff-suppr/test30-report-0.txt",
     "output/test-diff-suppr/test30-report-0.txt"
   },
@@ -1614,7 +1614,7 @@ InOutSpec in_out_specs[] =
     "data/test-diff-suppr/test30-include-dir-v0",
     "data/test-diff-suppr/test30-include-dir-v1",
     "",
-    "--no-default-suppression --dont-drop-private-types",
+    "--no-default-suppression",
     "data/test-diff-suppr/test30-report-1.txt",
     "output/test-diff-suppr/test30-report-1.txt"
   },
@@ -1674,7 +1674,7 @@ InOutSpec in_out_specs[] =
     "data/test-diff-suppr/test34-pub-include-dir-v0",
     "data/test-diff-suppr/test34-pub-include-dir-v1",
     "",
-    "--no-default-suppression --dont-drop-private-types",
+    "--no-default-suppression",
     "data/test-diff-suppr/test34-report-0.txt",
     "output/test-diff-suppr/test34-report-0.txt"
   },
index 2ebe11300a6cf1df6296270ec60d2b8fca066609..7210226b4c8dddd012aacb440a4f1b63f1f05efe 100644 (file)
@@ -109,7 +109,7 @@ struct options
     : display_usage(),
       display_version(),
       missing_operand(),
-      drop_private_types(true),
+      drop_private_types(false),
       linux_kernel_mode(true),
       no_default_supprs(),
       no_arch(),
@@ -149,7 +149,7 @@ display_usage(const string& prog_name, ostream& out)
     << " --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"
     << " --headers-dir2|--hd2 <path>  the path to headers of file2\n"
-    << "  --dont-drop-private-types\n  keep private types in "
+    << "  --drop-private-types\n  drop private types from "
     "internal representation\n"
     << " --no-linux-kernel-mode  don't consider the input binaries as "
        "linux kernel binaries\n"
@@ -307,8 +307,8 @@ parse_command_line(int argc, char* argv[], options& opts)
          opts.display_usage = true;
          return true;
        }
-      else if (!strcmp(argv[i], "--dont-drop-private-types"))
-       opts.drop_private_types = false;
+      else if (!strcmp(argv[i], "--drop-private-types"))
+       opts.drop_private_types = true;
       else if (!strcmp(argv[i], "--no-default-suppression"))
        opts.no_default_supprs = true;
       else if (!strcmp(argv[i], "--no-architecture"))
@@ -664,34 +664,39 @@ set_suppressions(ReadContextType& read_ctxt, const options& opts)
        ++i)
     read_suppressions(*i, supprs);
 
-  if (opts.drop_private_types)
+  if (!opts.headers_dir1.empty())
     {
-      if (!opts.headers_dir1.empty())
+      // Generate suppression specification to avoid showing ABI
+      // changes on types that are not defined in public headers.
+      //
+      // As these suppression specifications are applied during the
+      // corpus loading, they are going to be dropped from the
+      // internal representation altogether.
+      suppression_sptr suppr =
+       gen_suppr_spec_from_headers(opts.headers_dir1);
+      if (suppr)
        {
-         // Generate suppression specification to avoid showing ABI
-         // changes on types that are not defined in public headers.
-         //
-         // As these suppression specifications are applied during the
-         // corpus loading, they are going to be dropped from the
-         // internal representation altogether.
-         suppression_sptr suppr =
-           gen_suppr_spec_from_headers(opts.headers_dir1);
-         if (suppr)
-           supprs.push_back(suppr);
+         if (opts.drop_private_types)
+           suppr->set_drops_artifact_from_ir(true);
+         supprs.push_back(suppr);
        }
+    }
 
-      if (!opts.headers_dir2.empty())
+  if (!opts.headers_dir2.empty())
+    {
+      // Generate suppression specification to avoid showing ABI
+      // changes on types that are not defined in public headers.
+      //
+      // As these suppression specifications are applied during the
+      // corpus loading, they are going to be dropped from the
+      // internal representation altogether.
+      suppression_sptr suppr =
+       gen_suppr_spec_from_headers(opts.headers_dir2);
+      if (suppr)
        {
-         // Generate suppression specification to avoid showing ABI
-         // changes on types that are not defined in public headers.
-         //
-         // As these suppression specifications are applied during the
-         // corpus loading, they are going to be dropped from the
-         // internal representation altogether.
-         suppression_sptr suppr =
-           gen_suppr_spec_from_headers(opts.headers_dir2);
-         if (suppr)
-           supprs.push_back(suppr);
+         if (opts.drop_private_types)
+           suppr->set_drops_artifact_from_ir(true);
+         supprs.push_back(suppr);
        }
     }
 
index 438ef8f10a8ceb16076128bc8a228cd2e0f1c70c..8d905ee0a8f57fc3bbdbf403d2bea6c8dc92d0ac 100644 (file)
@@ -171,6 +171,7 @@ public:
   string       debug_package2;
   string       devel_package1;
   string       devel_package2;
+  bool         drop_private_types;
   bool         show_relative_offset_changes;
   bool         no_default_suppression;
   bool         keep_tmp_files;
@@ -192,6 +193,7 @@ public:
       nonexistent_file(),
       abignore(true),
       parallel(true),
+      drop_private_types(false),
       show_relative_offset_changes(true),
       no_default_suppression(),
       keep_tmp_files(),
@@ -612,6 +614,8 @@ display_usage(const string& prog_name, ostream& out)
     << " --debug-info-pkg2|--d2 <path>  path of debug-info package of package2\n"
     << " --devel-pkg1|--devel1 <path>   path of devel package of pakage1\n"
     << " --devel-pkg2|--devel2 <path>   path of devel package of pakage1\n"
+    << " --drop-private-types\n  drop private types from "
+    "internal representation\n"
     << " --no-default-suppression       don't load any default "
        "suppression specifications\n"
     << " --suppressions|--suppr <path>  specify supression specification path\n"
@@ -1375,8 +1379,10 @@ create_maps_of_package_content(package& package,
 /// @return true iff suppression specifications were generated for
 /// types private to the package.
 static bool
-maybe_create_private_types_suppressions(package& pkg)
+maybe_create_private_types_suppressions(package_descriptor& desc)
 {
+  package& pkg = desc.pkg;
+
   if (!pkg.private_types_suppressions().empty())
     return false;
 
@@ -1400,7 +1406,11 @@ maybe_create_private_types_suppressions(package& pkg)
     gen_suppr_spec_from_headers(headers_path);
 
   if (suppr)
-    pkg.private_types_suppressions().push_back(suppr);
+    {
+      if (desc.opts.drop_private_types)
+       suppr->set_drops_artifact_from_ir(true);
+      pkg.private_types_suppressions().push_back(suppr);
+    }
 
   return suppr;
 }
@@ -1504,7 +1514,7 @@ pthread_routine_extract_pkg_and_map_its_content(package_descriptor *a)
   if (has_devel_pkg && opts.parallel)
     result &= pthread_join(thr_devel);
 
-  maybe_create_private_types_suppressions(package);
+  maybe_create_private_types_suppressions(*a);
 
   // Let's wait for debug package extractions to finish before
   // we exit.
@@ -1925,6 +1935,8 @@ parse_command_line(int argc, char* argv[], options& opts)
            abigail::tools_utils::make_path_absolute(argv[j]).get();
           ++i;
         }
+      else if (!strcmp(argv[i], "--drop-private-types"))
+       opts.drop_private_types = true;
       else if (!strcmp(argv[i], "--no-default-suppression"))
        opts.no_default_suppression = true;
       else if (!strcmp(argv[i], "--keep-tmp-files"))
This page took 0.049228 seconds and 5 git commands to generate.