diff mbox series

Add --no-write-default-sizes option.

Message ID 20200426232852.23415-1-mark@klomp.org
State Committed
Headers
Series Add --no-write-default-sizes option. |

Commit Message

Mark Wielaard April 26, 2020, 11:28 p.m. UTC 
  From: Mark Wielaard <mark@klomp.org>

abidw will write out the exact same size-in-bits address for every
pointer type, reference type, function declaration and function type
even though it is always the same as the translation unit address
size. When giving the --no-write-default-sizes option these aren't
written out anymore. The reader is updated to set the default size
when none is given in the XML description.

Even though size and alignment are handled together in the reader,
default alignment is still set to zero, following commit a05384675

Note that this isn't backward compatible with older libabigail
readers, which will set the size to zero when none is given. So this
option isn't the default.

       * doc/manuals/abidw.rst: Document --no-write-default-sizes.
       * include/abg-writer.h (set_write_default_sizes): New function
       declaration.
       (set_common_options): Call set_write_default_sizes.
       * src/abg-reader.cc (build_function_decl): Get default size.
       (build_pointer_type_def): Likewise.
       (build_reference_type_def): Likewise.
       (build_function_type): Likewise.
       * src/abg-writer.cc (write_context): Add m_write_default_sizes
       bool.
       (get_write_default_sizes): New method.
       (set_write_default_sizes): Likewise.
       (write_size_and_alignment): Add default size and alignment
       parameters.
       (set_write_default_sizes): New function.
       (write_type_decl): Set default size and alignment.
       (write_pointer_type_def): Likewise.
       (write_reference_type_def): Likewise.
       (write_function_decl): Likewise.
       (write_function_type): Likewise.
       (write_class_decl_opening_tag): Likewise.
       (write_union_decl_opening_tag): Likewise.
       * tests/test-types-stability.cc (perform): Also test --abidiff
       with --no-write-default-sizes.
       * tools/abidw.cc (option): Add default_sizes bool.
       (parse_command_line): Parse --no-write-default-sizes.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 doc/manuals/abidw.rst         |  9 ++++
 include/abg-writer.h          |  4 ++
 src/abg-reader.cc             | 20 ++++-----
 src/abg-writer.cc             | 82 ++++++++++++++++++++++++++++++-----
 tests/test-types-stability.cc |  8 ++++
 tools/abidw.cc                |  4 ++
 6 files changed, 103 insertions(+), 24 deletions(-)

Comments

Dodji Seketeli April 29, 2020, 10:39 a.m. UTC | #1 
Hello Mark,

"Mark J. Wielaard" <mark@klomp.org> a ?crit:

> From: Mark Wielaard <mark@klomp.org>
>
> abidw will write out the exact same size-in-bits address for every
> pointer type, reference type, function declaration and function type
> even though it is always the same as the translation unit address
> size. When giving the --no-write-default-sizes option these aren't
> written out anymore. The reader is updated to set the default size
> when none is given in the XML description.
>
> Even though size and alignment are handled together in the reader,
> default alignment is still set to zero, following commit a05384675
>
> Note that this isn't backward compatible with older libabigail
> readers, which will set the size to zero when none is given. So this
> option isn't the default.

I have applied this patch to master with a few changes that I comment
below.

Thanks!

[...]

> diff --git a/src/abg-writer.cc b/src/abg-writer.cc

[...]

> +static void write_size_and_alignment(const type_base_sptr, ostream&,
> +				     size_t default_size,
> +				     size_t default_alignment);

I am adding default argument values to this declaration to make it be:

> +static void write_size_and_alignment(const type_base_sptr, ostream&,
> +				     size_t default_size = 0,
> +				     size_t default_alignment = 0);

That way ...

> -  write_size_and_alignment(d, o);
> +  write_size_and_alignment(d, o, 0, 0);

... this change (which has several occurrences) becomes unnecessary.

[...]

> diff --git a/tools/abidw.cc b/tools/abidw.cc

[...]

> @@ -293,6 +295,8 @@ parse_command_line(int argc, char* argv[], options& opts)
>  	opts.write_comp_dir = false;
>        else if (!strcmp(argv[i], "--no-elf-needed"))
>  	opts.write_elf_needed = false;
> +      else if (!strcmp(argv[i], "--no-write-default-sizes"))
> +	opts.default_sizes = false;
>        else if (!strcmp(argv[i], "--no-parameter-names"))
>  	opts.write_parameter_names = false;
>        else if (!strcmp(argv[i], "--check-alternate-debug-info")

I have added a description string from this new --no-write-default-sizes
option in the display_usage function.

Please find below the patch that I applied.

Thanks a lot!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-no-write-default-sizes-option.patch
Type: text/x-patch
Size: 13853 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libabigail/attachments/20200429/fe6a31b9/attachment.bin>
-------------- next part --------------
diff mbox series

Patch

diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index 7ae44737..e1fce997 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -188,6 +188,15 @@  Options
     In the emitted ABI representation, do not show names of function
     parameters, just the types.
 
+  * ``--no-write-default-sizes``
+
+    In the XML ABI representation, do not write the size-in-bits for
+    pointer type definitions, reference type definitions, function
+    declarations and function types when they are equal to the default
+    address size of the translation unit.  Note that libabigail before
+    1.8 will not set the default size and will interpret types without
+    a size-in-bits attribute as zero sized.
+
   * ``--named-type-ids``
 
     Without this option ids used to reference types in the XML file
diff --git a/include/abg-writer.h b/include/abg-writer.h
index d66accd2..8086d828 100644
--- a/include/abg-writer.h
+++ b/include/abg-writer.h
@@ -65,6 +65,9 @@  set_write_comp_dir(write_context& ctxt, bool flag);
 void
 set_write_elf_needed(write_context& ctxt, bool flag);
 
+void
+set_write_default_sizes(write_context& ctxt, bool flag);
+
 void
 set_short_locs(write_context& ctxt, bool flag);
 
@@ -92,6 +95,7 @@  set_common_options(write_context& ctxt, const OPTS& opts)
   set_write_elf_needed(ctxt, opts.write_elf_needed);
   set_write_parameter_names(ctxt, opts.write_parameter_names);
   set_short_locs(ctxt, opts.short_locs);
+  set_write_default_sizes(ctxt, opts.default_sizes);
 }
 
 void
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 3727e044..255a200f 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -3154,7 +3154,7 @@  build_function_decl(read_context&	ctxt,
   decl_base::binding bind = decl_base::BINDING_NONE;
   read_binding(node, bind);
 
-  size_t size = 0, align = 0;
+  size_t size = ctxt.get_translation_unit()->get_address_size(), align = 0;
   read_size_and_alignment(node, size, align);
 
   location loc;
@@ -3677,11 +3677,9 @@  build_pointer_type_def(read_context&	ctxt,
       return result;
     }
 
-  size_t size_in_bits = 0, alignment_in_bits = 0;
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "size-in-bits"))
-    size_in_bits = atoi(CHAR_STR(s));
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "alignment-in-bits"))
-    alignment_in_bits = atoi(CHAR_STR(s));
+  size_t size_in_bits = ctxt.get_translation_unit()->get_address_size();
+  size_t alignment_in_bits = 0;
+  read_size_and_alignment(node, size_in_bits, alignment_in_bits);
 
   string id;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id"))
@@ -3765,11 +3763,9 @@  build_reference_type_def(read_context&		ctxt,
       return result;
     }
 
-  size_t size_in_bits = 0, alignment_in_bits = 0;
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "size-in-bits"))
-    size_in_bits = atoi(CHAR_STR(s));
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "alignment-in-bits"))
-    alignment_in_bits = atoi(CHAR_STR(s));
+  size_t size_in_bits = ctxt.get_translation_unit()->get_address_size();
+  size_t alignment_in_bits = 0;
+  read_size_and_alignment(node, size_in_bits, alignment_in_bits);
 
   string id;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id"))
@@ -3834,7 +3830,7 @@  build_function_type(read_context&	ctxt,
 
   bool is_method_t = !method_class_id.empty();
 
-  size_t size = 0, align = 0;
+  size_t size = ctxt.get_translation_unit()->get_address_size(), align = 0;
   read_size_and_alignment(node, size, align);
 
   environment* env = ctxt.get_environment();
diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 8c6cc91a..36cdc3ae 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -176,6 +176,7 @@  class write_context
   bool					m_write_elf_needed;
   bool					m_write_parameter_names;
   bool					m_short_locs;
+  bool					m_write_default_sizes;
   mutable type_ptr_map			m_type_id_map;
   mutable type_ptr_set_type		m_emitted_type_set;
   type_ptr_set_type			m_emitted_decl_only_set;
@@ -212,7 +213,8 @@  public:
       m_write_comp_dir(true),
       m_write_elf_needed(true),
       m_write_parameter_names(true),
-      m_short_locs(false)
+      m_short_locs(false),
+      m_write_default_sizes(true)
   {}
 
   /// Getter of the environment we are operating from.
@@ -282,6 +284,20 @@  public:
   set_write_elf_needed(bool f)
   {m_write_elf_needed = f;}
 
+  /// Getter of the default-sizes option.
+  ///
+  /// @return true iff default size-in-bits needs to be emitted
+  bool
+  get_write_default_sizes()
+  {return m_write_default_sizes;}
+
+  /// Setter of the default-sizes option.
+  ///
+  /// @param f the new value of the flag.
+  void
+  set_write_default_sizes(bool f)
+  {m_write_default_sizes = f;}
+
   /// Getter of the write-corpus-path option.
   ///
   /// @return true iff corpus-path information shall be emitted
@@ -809,7 +825,9 @@  static bool write_is_non_reachable(const type_base_sptr&, ostream&);
 static bool write_tracking_non_reachable_types(const corpus_sptr&, ostream&);
 static void write_array_size_and_alignment(const array_type_def_sptr,
 					   ostream&);
-static void write_size_and_alignment(const type_base_sptr, ostream&);
+static void write_size_and_alignment(const type_base_sptr, ostream&,
+				     size_t default_size,
+				     size_t default_alignment);
 static void write_access(access_specifier, ostream&);
 static void write_layout_offset(var_decl_sptr, ostream&);
 static void write_layout_offset(class_decl::base_spec_sptr, ostream&);
@@ -1396,15 +1414,24 @@  write_tracking_non_reachable_types(const corpus_sptr& corpus,
 /// @param decl the type to consider.
 ///
 /// @param o the output stream to serialize to.
+///
+/// @param default_size size in bits that is the default for the type.
+///                     No size-in-bits attribute is written if it
+///                     would be the default value.
+///
+/// @param default_alignment alignment in bits that is the default for
+///                     the type.  No alignment-in-bits attribute is
+///                     written if it would be the default value.
 static void
-write_size_and_alignment(const shared_ptr<type_base> decl, ostream& o)
+write_size_and_alignment(const shared_ptr<type_base> decl, ostream& o,
+			 size_t default_size, size_t default_alignment)
 {
   size_t size_in_bits = decl->get_size_in_bits();
-  if (size_in_bits)
+  if (size_in_bits != default_size)
     o << " size-in-bits='" << size_in_bits << "'";
 
   size_t alignment_in_bits = decl->get_alignment_in_bits();
-  if (alignment_in_bits)
+  if (alignment_in_bits != default_alignment)
     o << " alignment-in-bits='" << alignment_in_bits << "'";
 }
 
@@ -2089,6 +2116,21 @@  void
 set_write_elf_needed(write_context& ctxt, bool flag)
 {ctxt.set_write_elf_needed(flag);}
 
+/// Set the 'default-sizes' flag.
+///
+/// When this flag is set then the XML writer will emit default
+/// size-in-bits attributes for pointer type definitions, reference
+/// type definitions, function declarations and function types even
+/// when they are equal to the default address size of the translation
+/// unit.
+///
+/// @param ctxt the context to set this flag on to.
+///
+/// @param flag the new value of the 'default-sizes' flag.
+void
+set_write_default_sizes(write_context& ctxt, bool flag)
+{ctxt.set_write_default_sizes(flag);}
+
 /// Serialize the canonical types of a given scope.
 ///
 /// @param scope the scope to consider.
@@ -2366,7 +2408,7 @@  write_type_decl(const type_decl_sptr& d, write_context& ctxt, unsigned indent)
 
   write_is_anonymous(d, o);
 
-  write_size_and_alignment(d, o);
+  write_size_and_alignment(d, o, 0, 0);
 
   write_location(d, ctxt);
 
@@ -2545,7 +2587,11 @@  write_pointer_type_def(const pointer_type_def_sptr&	decl,
 
   ctxt.record_type_as_referenced(pointed_to_type);
 
-  write_size_and_alignment(decl, o);
+  write_size_and_alignment(decl, o,
+			   (ctxt.get_write_default_sizes()
+			    ? 0
+			    : decl->get_translation_unit()->get_address_size()),
+			   0);
 
   string i = id;
   if (i.empty())
@@ -2623,7 +2669,11 @@  write_reference_type_def(const reference_type_def_sptr&	decl,
   if (function_type_sptr f = is_function_type(decl->get_pointed_to_type()))
     ctxt.record_type_as_referenced(f);
 
-  write_size_and_alignment(decl, o);
+  write_size_and_alignment(decl, o,
+			   (ctxt.get_write_default_sizes()
+			    ? 0
+			    : decl->get_translation_unit()->get_address_size()),
+			   0);
 
   string i = id;
   if (i.empty())
@@ -3171,7 +3221,11 @@  write_function_decl(const function_decl_sptr& decl, write_context& ctxt,
 
   write_binding(decl, o);
 
-  write_size_and_alignment(decl->get_type(), o);
+  write_size_and_alignment(decl->get_type(), o,
+			   (ctxt.get_write_default_sizes()
+			    ? 0
+			    : decl->get_translation_unit()->get_address_size()),
+			   0);
   write_elf_symbol_reference(decl->get_symbol(), o);
 
   o << ">\n";
@@ -3251,7 +3305,11 @@  write_function_type(const function_type_sptr& fn_type,
 
   o << "<function-type";
 
-  write_size_and_alignment(fn_type, o);
+  write_size_and_alignment(fn_type, o,
+			   (ctxt.get_write_default_sizes()
+			    ? 0
+			    : fn_type->get_translation_unit()->get_address_size()),
+			   0);
 
   if (method_type_sptr method_type = is_method_type(fn_type))
     {
@@ -3352,7 +3410,7 @@  write_class_decl_opening_tag(const class_decl_sptr&	decl,
 
   o << "<class-decl name='" << xml::escape_xml_string(decl->get_name()) << "'";
 
-  write_size_and_alignment(decl, o);
+  write_size_and_alignment(decl, o, 0, 0);
 
   write_is_struct(decl, o);
 
@@ -3430,7 +3488,7 @@  write_union_decl_opening_tag(const union_decl_sptr&	decl,
   o << "<union-decl name='" << xml::escape_xml_string(decl->get_name()) << "'";
 
   if (!decl->get_is_declaration_only())
-    write_size_and_alignment(decl, o);
+    write_size_and_alignment(decl, o, 0, 0);
 
   write_is_anonymous(decl, o);
 
diff --git a/tests/test-types-stability.cc b/tests/test-types-stability.cc
index a5d60f84..bc3d4d6e 100644
--- a/tests/test-types-stability.cc
+++ b/tests/test-types-stability.cc
@@ -102,6 +102,14 @@  struct test_task : public abigail::workers::task
 	error_message = "IR stability issue detected for binary " + elf_path;
 	is_ok = false;
       }
+
+    cmd = abidw + " --abidiff --no-write-default-sizes " + elf_path;
+    if (system(cmd.c_str()))
+      {
+	error_message = "IR stability issue detected for binary " + elf_path
+	  + " with --no-write-default-sizes";
+	is_ok = false;
+      }
   }
 }; // end struct test_task
 
diff --git a/tools/abidw.cc b/tools/abidw.cc
index a824f613..7c269aa2 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -102,6 +102,7 @@  struct options
   bool			write_elf_needed;
   bool			write_parameter_names;
   bool			short_locs;
+  bool			default_sizes;
   bool			load_all_types;
   bool			linux_kernel_mode;
   bool			corpus_group_for_linux;
@@ -124,6 +125,7 @@  struct options
       write_elf_needed(true),
       write_parameter_names(true),
       short_locs(false),
+      default_sizes(true),
       load_all_types(),
       linux_kernel_mode(true),
       corpus_group_for_linux(false),
@@ -293,6 +295,8 @@  parse_command_line(int argc, char* argv[], options& opts)
 	opts.write_comp_dir = false;
       else if (!strcmp(argv[i], "--no-elf-needed"))
 	opts.write_elf_needed = false;
+      else if (!strcmp(argv[i], "--no-write-default-sizes"))
+	opts.default_sizes = false;
       else if (!strcmp(argv[i], "--no-parameter-names"))
 	opts.write_parameter_names = false;
       else if (!strcmp(argv[i], "--check-alternate-debug-info")