Bug 26261 - Fix logic for canonicalizing DW_TAG_subroutine_type DIEs
In the function compare_dies, the handling of DW_TAG_subroutine_type
was unfinished.
This DIE represents function types pointed-to by function pointer. It
doesn't represent the type of a "real" function type like the
abigail::ir::function_type would. The represent more an interface to
a function, which a pointer can point to or which can be used to issue
a call.
So intended idea is that compare_dies compares two DIEs of the
DW_TAG_subroutine_type and DW_TAG_subprogram kind (the later
represents real function definitions) among other kinds, structurally,
but by trying to optimize for speed, for the purpose of canonicalizing
DIEs even before type canonicalization happens at the libabigail IR
level. This is critical to save space (and time) by doing DIE
de-duplication on huge binaries.
This patch finishes to implement the comparison for
DW_TAG_subroutine_type and comes with a carefully crafted test case
that hits that code path.
* src/abg-dwarf-reader.cc (compare_dies): Get out early if we are
are in the middle of a potential recursive comparison of function
types. Likewise if we detect that the two function types have
different textual representations, linkage names, or have a the
same textual representation, linkage names and are defined in the
same translation unit.
* tests/data/test-read-dwarf/PR26261/PR26261-exe: New test binary
input file.
* tests/data/test-read-dwarf/PR26261/PR26261-exe.abi: New
reference test output file.
* tests/data/test-read-dwarf/PR26261/PR26261-main.c: Source code
of the binary above.
* tests/data/test-read-dwarf/PR26261/PR26261-obj{a,b}.{c,h}:
Likewise.
* tests/data/test-read-dwarf/PR26261/Makefile: Makefile to
build the exe out of the source files.
* tests/data/Makefile.am: Add the new test input files to source
distribution.
* tests/test-read-dwarf.cc (in_out_spec): Add the binary test
input above to the test harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 533c2dc..1184cbf 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -11874,76 +11874,100 @@
!= aggregates_being_compared.end())
|| (aggregates_being_compared.find(rn)
!= aggregates_being_compared.end()))
- result = true;
+ {
+ result = true;
+ break;
+ }
else if (l_tag == DW_TAG_subroutine_type)
{
- // The string reprs of l and r are already equal. Now
- // let's just check if they both come from the same TU.
- bool from_the_same_tu = false;
- if (compare_dies_cu_decl_file(l, r, from_the_same_tu)
- && from_the_same_tu)
- result = true;
- }
- else
- {
- if (!fn_die_equal_by_linkage_name(ctxt, l, r))
+ // So, we are looking at types that are pointed to by a
+ // function pointer. These are not real concrete function
+ // types, rather, they denote interfaces of functions.
+ //
+ // If the textual representations are different, then
+ // obviously they are different DIEs.
+ if (ln != rn)
{
result = false;
break;
}
- if (!ctxt.die_is_in_c(l) && !ctxt.die_is_in_c(r))
+ // So if their textual representation are the same and
+ // they come from the same TU, then they represent the
+ // same DIE.
+ bool from_the_same_tu = false;
+ if (compare_dies_cu_decl_file(l, r, from_the_same_tu)
+ && from_the_same_tu)
{
- // In C, we cannot have two different functions with the
- // same linkage name in a given binary. But here we are
- // looking at DIEs that don't originate from C. So we
- // need to compare return types and parameter types.
- Dwarf_Die l_return_type, r_return_type;
- bool l_return_type_is_void = !die_die_attribute(l, DW_AT_type,
- l_return_type);
- bool r_return_type_is_void = !die_die_attribute(r, DW_AT_type,
- r_return_type);
- if (l_return_type_is_void != r_return_type_is_void
- || (!l_return_type_is_void
- && !compare_dies(ctxt,
- &l_return_type, &r_return_type,
- aggregates_being_compared,
- update_canonical_dies_on_the_fly)))
- result = false;
- else
- {
- Dwarf_Die l_child, r_child;
- bool found_l_child, found_r_child;
- for (found_l_child = dwarf_child(const_cast<Dwarf_Die*>(l),
- &l_child) == 0,
- found_r_child = dwarf_child(const_cast<Dwarf_Die*>(r),
- &r_child) == 0;
- found_l_child && found_r_child;
- found_l_child = dwarf_siblingof(&l_child,
- &l_child) == 0,
- found_r_child = dwarf_siblingof(&r_child,
- &r_child)==0)
- {
- int l_child_tag = dwarf_tag(&l_child);
- int r_child_tag = dwarf_tag(&r_child);
- if (l_child_tag != r_child_tag
- || (l_child_tag == DW_TAG_formal_parameter
- && !compare_dies(ctxt, &l_child, &r_child,
- aggregates_being_compared,
- update_canonical_dies_on_the_fly)))
- {
- result = false;
- break;
- }
- }
- if (found_l_child != found_r_child)
- result = false;
- }
+ result = true;
+ break;
}
-
- aggregates_being_compared.erase(ln);
- aggregates_being_compared.erase(rn);
}
+
+ if (l_tag == DW_TAG_subprogram
+ && !fn_die_equal_by_linkage_name(ctxt, l, r))
+ {
+ result = false;
+ break;
+ }
+ else if (l_tag == DW_TAG_subprogram
+ && ctxt.die_is_in_c(l) && ctxt.die_is_in_c(r)
+ /*&& fn_die_equal_by_linkage_name(ctxt, l, r)*/)
+ {
+ result = true;
+ break;
+ }
+ else if (!ctxt.die_is_in_c(l) && !ctxt.die_is_in_c(r))
+ {
+ // In C, we cannot have two different functions with the
+ // same linkage name in a given binary. But here we are
+ // looking at DIEs that don't originate from C. So we
+ // need to compare return types and parameter types.
+ Dwarf_Die l_return_type, r_return_type;
+ bool l_return_type_is_void = !die_die_attribute(l, DW_AT_type,
+ l_return_type);
+ bool r_return_type_is_void = !die_die_attribute(r, DW_AT_type,
+ r_return_type);
+ if (l_return_type_is_void != r_return_type_is_void
+ || (!l_return_type_is_void
+ && !compare_dies(ctxt,
+ &l_return_type, &r_return_type,
+ aggregates_being_compared,
+ update_canonical_dies_on_the_fly)))
+ result = false;
+ else
+ {
+ Dwarf_Die l_child, r_child;
+ bool found_l_child, found_r_child;
+ for (found_l_child = dwarf_child(const_cast<Dwarf_Die*>(l),
+ &l_child) == 0,
+ found_r_child = dwarf_child(const_cast<Dwarf_Die*>(r),
+ &r_child) == 0;
+ found_l_child && found_r_child;
+ found_l_child = dwarf_siblingof(&l_child,
+ &l_child) == 0,
+ found_r_child = dwarf_siblingof(&r_child,
+ &r_child)==0)
+ {
+ int l_child_tag = dwarf_tag(&l_child);
+ int r_child_tag = dwarf_tag(&r_child);
+ if (l_child_tag != r_child_tag
+ || (l_child_tag == DW_TAG_formal_parameter
+ && !compare_dies(ctxt, &l_child, &r_child,
+ aggregates_being_compared,
+ update_canonical_dies_on_the_fly)))
+ {
+ result = false;
+ break;
+ }
+ }
+ if (found_l_child != found_r_child)
+ result = false;
+ }
+ }
+
+ aggregates_being_compared.erase(ln);
+ aggregates_being_compared.erase(rn);
}
break;
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index ea94f0b..e59331e 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -496,6 +496,14 @@
test-read-dwarf/test25-bogus-binary.elf \
test-read-dwarf/test26-bogus-binary.elf \
test-read-dwarf/test27-bogus-binary.elf \
+test-read-dwarf/PR26261/Makefile \
+test-read-dwarf/PR26261/PR26261-exe.abi \
+test-read-dwarf/PR26261/PR26261-obja.c \
+test-read-dwarf/PR26261/PR26261-objb.c \
+test-read-dwarf/PR26261/PR26261-exe \
+test-read-dwarf/PR26261/PR26261-main.c \
+test-read-dwarf/PR26261/PR26261-obja.h \
+test-read-dwarf/PR26261/PR26261-objb.h \
\
test-annotate/test0.abi \
test-annotate/test1.abi \
diff --git a/tests/data/test-read-dwarf/PR26261/Makefile b/tests/data/test-read-dwarf/PR26261/Makefile
new file mode 100644
index 0000000..f66fa38
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/Makefile
@@ -0,0 +1,21 @@
+SRCS = PR26261-obja.c PR26261-objb.c PR26261-main.c
+EXE = PR26261-exe
+OBJS = $(SRCS:.c=.o)
+CFLAGS = -Wall -g
+
+all: $(EXE)
+
+%.o: %.c
+ $(CC) $(CFLAGS) -c $<
+
+$(EXE): $(OBJS)
+ gcc -fPIC $(OBJS) -o $@
+
+cleanobjs:
+ rm -rf $(OBJS)
+
+cleanexe:
+ rm $(EXE)
+
+clean: cleanobjs
+ rm -rf *~
diff --git a/tests/data/test-read-dwarf/PR26261/PR26261-exe b/tests/data/test-read-dwarf/PR26261/PR26261-exe
new file mode 100755
index 0000000..564a424
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/PR26261-exe
Binary files differ
diff --git a/tests/data/test-read-dwarf/PR26261/PR26261-exe.abi b/tests/data/test-read-dwarf/PR26261/PR26261-exe.abi
new file mode 100644
index 0000000..02c032b
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/PR26261-exe.abi
@@ -0,0 +1,86 @@
+<abi-corpus path='data/test-read-dwarf/PR26261/PR26261-exe'>
+ <elf-needed>
+ <dependency name='libc.so.6'/>
+ </elf-needed>
+ <elf-function-symbols>
+ <elf-symbol name='__libc_csu_fini' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ <elf-symbol name='__libc_csu_init' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ <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='_start' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ <elf-symbol name='fun_obja' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ <elf-symbol name='fun_objb' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ <elf-symbol name='main' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ <elf-symbol name='wrapped_call' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ </elf-function-symbols>
+ <elf-variable-symbols>
+ <elf-symbol name='_IO_stdin_used' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ </elf-variable-symbols>
+ <abi-instr version='1.0' address-size='64' path='PR26261-main.c' comp-dir-path='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261' language='LANG_C89'>
+ <pointer-type-def type-id='type-id-1' size-in-bits='64' id='type-id-2'/>
+ <pointer-type-def type-id='type-id-2' size-in-bits='64' id='type-id-3'/>
+ <function-decl name='wrapped_call' mangled-name='wrapped_call' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-main.c' line='18' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='wrapped_call'>
+ <parameter type-id='type-id-4' name='fun' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-main.c' line='18' column='1'/>
+ <parameter type-id='type-id-5' name='a' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-main.c' line='19' column='1'/>
+ <parameter type-id='type-id-5' name='b' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-main.c' line='20' column='1'/>
+ <return type-id='type-id-6'/>
+ </function-decl>
+ <function-decl name='main' mangled-name='main' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-main.c' line='27' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='main'>
+ <parameter type-id='type-id-5' name='argc' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-main.c' line='27' column='1'/>
+ <parameter type-id='type-id-3' name='argv' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-main.c' line='27' column='1'/>
+ <return type-id='type-id-5'/>
+ </function-decl>
+ </abi-instr>
+ <abi-instr version='1.0' address-size='64' path='PR26261-obja.c' comp-dir-path='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261' language='LANG_C89'>
+ <type-decl name='int' size-in-bits='32' id='type-id-5'/>
+ <type-decl name='void' id='type-id-6'/>
+ <class-decl name='SA' size-in-bits='192' is-struct='yes' visibility='default' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.h' line='5' column='1' id='type-id-7'>
+ <data-member access='public' layout-offset-in-bits='0'>
+ <var-decl name='m1' type-id='type-id-5' visibility='default' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.h' line='7' column='1'/>
+ </data-member>
+ <data-member access='public' layout-offset-in-bits='64'>
+ <var-decl name='m2' type-id='type-id-8' visibility='default' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.h' line='8' column='1'/>
+ </data-member>
+ <data-member access='public' layout-offset-in-bits='128'>
+ <var-decl name='m3' type-id='type-id-9' visibility='default' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.h' line='9' column='1'/>
+ </data-member>
+ </class-decl>
+ <typedef-decl name='fn_ptr_type_a_t' type-id='type-id-4' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.h' line='1' column='1' id='type-id-8'/>
+ <typedef-decl name='fn_ptr_type_a2_t' type-id='type-id-4' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.h' line='3' column='1' id='type-id-9'/>
+ <pointer-type-def type-id='type-id-7' size-in-bits='64' id='type-id-10'/>
+ <pointer-type-def type-id='type-id-11' size-in-bits='64' id='type-id-4'/>
+ <function-decl name='fun_obja' mangled-name='fun_obja' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.c' line='4' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='fun_obja'>
+ <parameter type-id='type-id-10' name='s' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-obja.c' line='4' column='1'/>
+ <return type-id='type-id-6'/>
+ </function-decl>
+ <function-type size-in-bits='64' id='type-id-11'>
+ <parameter type-id='type-id-5'/>
+ <parameter type-id='type-id-5'/>
+ <return type-id='type-id-6'/>
+ </function-type>
+ </abi-instr>
+ <abi-instr version='1.0' address-size='64' path='PR26261-objb.c' comp-dir-path='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261' language='LANG_C89'>
+ <type-decl name='char' size-in-bits='8' id='type-id-1'/>
+ <class-decl name='SB' size-in-bits='128' is-struct='yes' visibility='default' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-objb.h' line='3' column='1' id='type-id-12'>
+ <data-member access='public' layout-offset-in-bits='0'>
+ <var-decl name='m1' type-id='type-id-5' visibility='default' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-objb.h' line='5' column='1'/>
+ </data-member>
+ <data-member access='public' layout-offset-in-bits='64'>
+ <var-decl name='m2' type-id='type-id-13' visibility='default' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-objb.h' line='6' column='1'/>
+ </data-member>
+ </class-decl>
+ <typedef-decl name='fn_ptr_type_b_t' type-id='type-id-14' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-objb.h' line='1' column='1' id='type-id-13'/>
+ <pointer-type-def type-id='type-id-12' size-in-bits='64' id='type-id-15'/>
+ <pointer-type-def type-id='type-id-16' size-in-bits='64' id='type-id-14'/>
+ <function-decl name='fun_objb' mangled-name='fun_objb' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-objb.c' line='4' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='fun_objb'>
+ <parameter type-id='type-id-15' name='s' filepath='/home/dodji/git/libabigail/patches/tests/data/test-read-dwarf/PR26261/PR26261-objb.c' line='4' column='1'/>
+ <return type-id='type-id-6'/>
+ </function-decl>
+ <function-type size-in-bits='64' id='type-id-16'>
+ <parameter type-id='type-id-5'/>
+ <parameter type-id='type-id-5'/>
+ <parameter type-id='type-id-1'/>
+ <return type-id='type-id-6'/>
+ </function-type>
+ </abi-instr>
+</abi-corpus>
diff --git a/tests/data/test-read-dwarf/PR26261/PR26261-main.c b/tests/data/test-read-dwarf/PR26261/PR26261-main.c
new file mode 100644
index 0000000..60ea256
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/PR26261-main.c
@@ -0,0 +1,40 @@
+/**
+ *
+ * In this test case, we want the first parameter of the function
+ * wrapped_call to be represented in the output abixml by a pointer to
+ * the same function type as the one pointed to by the types
+ * fn_ptr_type_a_t and fn_ptr_type_b_t.
+ *
+ * This means that we want the function type pointed by these three
+ * function pointer to be the same, as a result of the
+ * canonicalization of the DIEs that represent these three function
+ * types.
+ *
+ */
+#include "PR26261-obja.h"
+#include "PR26261-objb.h"
+
+void
+wrapped_call(void (*fun)(int, int),
+ int a,
+ int b)
+{
+ if (fun)
+ fun(a, b);
+}
+
+int
+main(int argc, char ** argv)
+{
+ struct SA sa = {0, 0, 0};
+ struct SB sb = {0, 0};
+
+ sa.m1 = 0;
+ sb.m1 = 0;
+
+ fun_obja(&sa);
+ fun_objb(&sb);
+ wrapped_call(sa.m2, sa.m1, sb.m1);
+
+ return 0;
+}
diff --git a/tests/data/test-read-dwarf/PR26261/PR26261-obja.c b/tests/data/test-read-dwarf/PR26261/PR26261-obja.c
new file mode 100644
index 0000000..04d6d00
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/PR26261-obja.c
@@ -0,0 +1,6 @@
+#include "PR26261-obja.h"
+
+void
+fun_obja(struct SA* s __attribute__((unused)))
+{
+}
diff --git a/tests/data/test-read-dwarf/PR26261/PR26261-obja.h b/tests/data/test-read-dwarf/PR26261/PR26261-obja.h
new file mode 100644
index 0000000..b6ca8f4
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/PR26261-obja.h
@@ -0,0 +1,12 @@
+typedef void (*fn_ptr_type_a_t)(int, int);
+
+typedef void (*fn_ptr_type_a2_t)(int, int);
+
+struct SA
+{
+ int m1;
+ fn_ptr_type_a_t m2;
+ fn_ptr_type_a2_t m3;
+};
+
+void fun_obja(struct SA*);
diff --git a/tests/data/test-read-dwarf/PR26261/PR26261-objb.c b/tests/data/test-read-dwarf/PR26261/PR26261-objb.c
new file mode 100644
index 0000000..49e26bf
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/PR26261-objb.c
@@ -0,0 +1,6 @@
+#include "PR26261-objb.h"
+
+void
+fun_objb(struct SB* s __attribute__((unused)))
+{
+}
diff --git a/tests/data/test-read-dwarf/PR26261/PR26261-objb.h b/tests/data/test-read-dwarf/PR26261/PR26261-objb.h
new file mode 100644
index 0000000..92f92ca
--- /dev/null
+++ b/tests/data/test-read-dwarf/PR26261/PR26261-objb.h
@@ -0,0 +1,9 @@
+typedef void (*fn_ptr_type_b_t)(int, int, char);
+
+struct SB
+{
+ int m1;
+ fn_ptr_type_b_t m2;
+};
+
+void fun_objb(struct SB*);
diff --git a/tests/test-read-dwarf.cc b/tests/test-read-dwarf.cc
index 8502f82..3f93d6d 100644
--- a/tests/test-read-dwarf.cc
+++ b/tests/test-read-dwarf.cc
@@ -373,6 +373,13 @@
NULL,
NULL,
},
+ {
+ "data/test-read-dwarf/PR26261/PR26261-exe",
+ "",
+ SEQUENCE_TYPE_ID_STYLE,
+ "data/test-read-dwarf/PR26261/PR26261-exe.abi",
+ "output/test-read-dwarf/PR26261/PR26261-exe.abi",
+ },
// This should be the last entry.
{NULL, NULL, SEQUENCE_TYPE_ID_STYLE, NULL, NULL}
};
