public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ctf-reader: Fix symbols alias report in ABI representation.
@ 2022-09-08  4:48 Guillermo E. Martinez
  2022-10-31 18:10 ` Guillermo Martinez
  2022-11-17 14:33 ` Dodji Seketeli
  0 siblings, 2 replies; 6+ messages in thread
From: Guillermo E. Martinez @ 2022-09-08  4:48 UTC (permalink / raw)
  To: libabigail; +Cc: Guillermo E. Martinez

Hello,

This patch fix the alias report in the ABI XML file generated by ctf
reader, some symbols are mentioned as alias, but there isn't relationship
between them.

Comments will be grateful and appreciated!.

Thanks in advanced,
guillermo
--

Symbol alias are not correctly enumerated in ABI representation using
the ctf reader when it operates on Elf ET_REL files, due to the
mechanism applied to find the symbol alias, this rely indirectly in
the value of `sh_addr' in Elf section header structures, reading
the same value for different Elf sections, depending of which section the
symbol is stored in Elf file, even though the first symbols in those
section have the same `st_value', it doesn't mean that they are
aliases. So, is necessary update the section's virtual addr field, just
if it is a SHF_ALLOC section, it is done by adding the previous
section size and assigning this value to `sh_addr' field, otherwise
alias symbols (if the symbol has one) are mix up:

	* src/abg-ctf-reader.cc (open_elf_handler): Update addr
	field if it is section is SHF_ALLOC for ET_REL files.
	* tests/data/Makefile.am: New test case input.
	* tests/data/test-read-ctf/test-alias.c: Update testcase.
	* tests/data/test-read-ctf/test-alias.o.abi: Likewise.
	* tests/test-read-ctf.cc (in_out_specs): Likewise.

Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
---
 src/abg-ctf-reader.cc                     |  35 ++++++++++++++++++++++
 tests/data/Makefile.am                    |   1 +
 tests/data/test-read-ctf/test-alias.c     |  18 +++++------
 tests/data/test-read-ctf/test-alias.o     | Bin 0 -> 1688 bytes
 tests/data/test-read-ctf/test-alias.o.abi |  22 +++++++-------
 tests/test-read-ctf.cc                    |   8 +++++
 6 files changed, 64 insertions(+), 20 deletions(-)
 create mode 100644 tests/data/test-read-ctf/test-alias.o

diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
index f5f58c7a..907290d2 100644
--- a/src/abg-ctf-reader.cc
+++ b/src/abg-ctf-reader.cc
@@ -1396,6 +1396,41 @@ open_elf_handler(read_context *ctxt)
       return 0;
     }
 
+  /* It Elf type is ET_REL it updates SHF_ALLOC sections in
+     its virtual addr field, it is done by adding the previous
+     section size and assigning this value to sh_addr field,
+     otherwise alias symbols (if the symbol has one) is mix up */
+  Elf *elf = ctxt->elf_handler;
+  GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr(elf, &ehdr_mem);
+  if (ehdr == NULL)
+    return 0;
+
+  GElf_Addr end = 0;
+  Elf_Scn *scn = NULL;
+
+  if (ehdr->e_type == ET_REL)
+    while ((scn = elf_nextscn(elf, scn)) != NULL)
+      {
+        GElf_Shdr shdr_mem;
+        GElf_Shdr *shdr = gelf_getshdr(scn, &shdr_mem);
+
+        if (shdr == NULL)
+          return 0;
+
+        if (shdr->sh_flags & SHF_ALLOC)
+          {
+            if (shdr->sh_addr == 0)
+              {
+                shdr->sh_addr = end;
+                end = shdr->sh_addr + shdr->sh_size;
+                if ((shdr->sh_addr != 0) && !gelf_update_shdr(scn, shdr))
+                  return 0;
+              }
+            else
+              end = shdr->sh_addr + shdr->sh_size;
+          }
+      }
+
   return 1;
 }
 
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 782dd7f3..793a0e89 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -632,6 +632,7 @@ test-read-ctf/test9.c		\
 test-read-ctf/test9.o		\
 test-read-ctf/test9.o.abi	\
 test-read-ctf/test-alias.c	\
+test-read-ctf/test-alias.o	\
 test-read-ctf/test-alias.o.abi	\
 test-read-ctf/test-ambiguous-struct-A.c	\
 test-read-ctf/test-ambiguous-struct-A.o	\
diff --git a/tests/data/test-read-ctf/test-alias.c b/tests/data/test-read-ctf/test-alias.c
index 1b497a18..afc0aced 100644
--- a/tests/data/test-read-ctf/test-alias.c
+++ b/tests/data/test-read-ctf/test-alias.c
@@ -1,15 +1,13 @@
+// gcc -gctf tests/data/test-read-ctf/test-alias .c \
+//  -c -o tests/data/test-read-ctf/test-alias.o
 
-// function aliases
+void func(void);
+void alias_func(void) __attribute__((alias("func")));
+int foo;
 
-void main_func(void);
-void alias_func(void) __attribute__((weak, alias("main_func")));
-
-void main_func(void)
+void func(void)
 {
-
 }
 
-// variables aliases
-
-int main_var;
-extern int alias_var __attribute__((weak, alias("main_var")));
+int var;
+extern int alias_var __attribute__((alias("var")));
diff --git a/tests/data/test-read-ctf/test-alias.o b/tests/data/test-read-ctf/test-alias.o
new file mode 100644
index 0000000000000000000000000000000000000000..ff5e5c536d824fc639ba4338182a870c57ddf548
GIT binary patch
literal 1688
zcmbtUUuzRV5T8q9YBedThzeD>;)5-ky);sgK1eiZiin_4@THW!%ceQJ%f-7*Nkq^m
z!MA=4AN>dgzkuJsC!w$U1$5?aH`y-n$$`0<`OW-h|I8*Io;-hAavad&z(bfv8U=X0
zH22%N+lCrcVSn%Q&y$01@cT!(q$@VGgSLuVN4<&aqCP<Fp}s|>b+l6Z3#cooHB_o6
zOL@Y{Qcl;STPfqmMd&(OluBX1Auh_bd_&<-LO)61n5V#_kY|VF9VcNA`r}~$-XMvE
z=TH5qm!v$1gqNj(7ll6e!y_I>jqxaxso=3EMJ6+^$0g6*27<jtAp1p>_htb=x6|2m
zH@h$Px7_XftjU^gtJ!KbcUn8{rkIRG8pdKMd9;;pQk@|`BXXbw&M9G992L{KNv;+K
zoOiFlnbe*2>hi@COwyUvxnY@V9<A0tl{&S`V=PddMTYv8;n$x<xuyKJ&u3OKT!Z;z
z(0jds_!Xl*-$RZq#&4@Wny>Axf8ap3-_Vy#11*M`&u~6hdJnnK>}L;>5SC42$$hjk
zE$l#hQcNT;8Z0nB%Yflf@sq?fC}6`x3Wl?3WReGORD3iXvr(GhIOP;rDk9Fr;IN<a
zSmf*f?`Itw&}{Uh*8SSb(f6xysx@T;1H$H$DzaU($nb6c9n8_YvH5$7W4|dqKi%>k
zI(ARWY5t0pv_jZc{3ekMd8rET8W_2nukEgZnWgV_fe9|CeTEp_@wqpI@khnC_hsLQ
zuX@`2uZmCqgQnR015DDLw#_2+zpIAydn37oN?gs?I4xtZjS!rv^-n24xr|CPw)ym5
fYtJ4b_@bK9`!Gedok45pE#jXkel)8r9Gm|afdYk1

literal 0
HcmV?d00001

diff --git a/tests/data/test-read-ctf/test-alias.o.abi b/tests/data/test-read-ctf/test-alias.o.abi
index 1fe61b8f..f0f53ad1 100644
--- a/tests/data/test-read-ctf/test-alias.o.abi
+++ b/tests/data/test-read-ctf/test-alias.o.abi
@@ -1,19 +1,21 @@
 <abi-corpus version='2.1' path='data/test-read-ctf/test-alias.o'>
   <elf-function-symbols>
-    <elf-symbol name='alias_func' type='func-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
-    <elf-symbol name='main_func' type='func-type' binding='global-binding' visibility='default-visibility' alias='alias_func,main_var,alias_var' is-defined='yes'/>
+    <elf-symbol name='alias_func' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='func' type='func-type' binding='global-binding' visibility='default-visibility' alias='alias_func' is-defined='yes'/>
   </elf-function-symbols>
   <elf-variable-symbols>
-    <elf-symbol name='alias_var' size='4' type='object-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
-    <elf-symbol name='main_var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='alias_var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='foo' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' alias='alias_var' is-defined='yes'/>
   </elf-variable-symbols>
   <abi-instr address-size='64' language='LANG_C'>
-    <type-decl name='int' size-in-bits='32' alignment-in-bits='32' id='95e97e5e'/>
-    <function-decl name='main_func' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='8'>
-      <return type-id='48b5725f'/>
+    <type-decl name='int' size-in-bits='32' alignment-in-bits='32' id='type-id-1'/>
+    <function-decl name='func' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='8' elf-symbol-id='func'>
+      <return type-id='type-id-2'/>
     </function-decl>
-    <var-decl name='alias_var' type-id='95e97e5e' mangled-name='alias_var' visibility='default'/>
-    <var-decl name='main_var' type-id='95e97e5e' mangled-name='main_var' visibility='default'/>
-    <type-decl name='void' id='48b5725f'/>
+    <var-decl name='alias_var' type-id='type-id-1' mangled-name='alias_var' visibility='default' elf-symbol-id='alias_var'/>
+    <var-decl name='foo' type-id='type-id-1' mangled-name='foo' visibility='default' elf-symbol-id='foo'/>
+    <var-decl name='var' type-id='type-id-1' mangled-name='var' visibility='default' elf-symbol-id='var'/>
+    <type-decl name='void' id='type-id-2'/>
   </abi-instr>
 </abi-corpus>
diff --git a/tests/test-read-ctf.cc b/tests/test-read-ctf.cc
index b0ef7de6..0b4e4b9d 100644
--- a/tests/test-read-ctf.cc
+++ b/tests/test-read-ctf.cc
@@ -293,6 +293,14 @@ static InOutSpec in_out_specs[] =
     "data/test-read-ctf/test-callback2.abi",
     "output/test-read-ctf/test-callback2.abi",
   },
+  {
+    "data/test-read-ctf/test-alias.o",
+    "",
+    "",
+    SEQUENCE_TYPE_ID_STYLE,
+    "data/test-read-ctf/test-alias.o.abi",
+    "output/test-read-ctf/test-alias.o.abi",
+  },
   // out-of-tree kernel module.
   {
     "data/test-read-ctf/test-linux-module.ko",
-- 
2.35.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ctf-reader: Fix symbols alias report in ABI representation.
  2022-09-08  4:48 [PATCH] ctf-reader: Fix symbols alias report in ABI representation Guillermo E. Martinez
@ 2022-10-31 18:10 ` Guillermo Martinez
  2022-11-17 14:33 ` Dodji Seketeli
  1 sibling, 0 replies; 6+ messages in thread
From: Guillermo Martinez @ 2022-10-31 18:10 UTC (permalink / raw)
  To: libabigail

Hello,

Is there any comment with this patch?

thanks,
guillermo

On 07/09/2022 11:48 p. m., Guillermo E. Martinez wrote:
> Hello,
> 
> This patch fix the alias report in the ABI XML file generated by ctf
> reader, some symbols are mentioned as alias, but there isn't relationship
> between them.
> 
> Comments will be grateful and appreciated!.
> 
> Thanks in advanced,
> guillermo
> --
> 
> Symbol alias are not correctly enumerated in ABI representation using
> the ctf reader when it operates on Elf ET_REL files, due to the
> mechanism applied to find the symbol alias, this rely indirectly in
> the value of `sh_addr' in Elf section header structures, reading
> the same value for different Elf sections, depending of which section the
> symbol is stored in Elf file, even though the first symbols in those
> section have the same `st_value', it doesn't mean that they are
> aliases. So, is necessary update the section's virtual addr field, just
> if it is a SHF_ALLOC section, it is done by adding the previous
> section size and assigning this value to `sh_addr' field, otherwise
> alias symbols (if the symbol has one) are mix up:
> 
> 	* src/abg-ctf-reader.cc (open_elf_handler): Update addr
> 	field if it is section is SHF_ALLOC for ET_REL files.
> 	* tests/data/Makefile.am: New test case input.
> 	* tests/data/test-read-ctf/test-alias.c: Update testcase.
> 	* tests/data/test-read-ctf/test-alias.o.abi: Likewise.
> 	* tests/test-read-ctf.cc (in_out_specs): Likewise.
> 
> Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
> ---
>  src/abg-ctf-reader.cc                     |  35 ++++++++++++++++++++++
>  tests/data/Makefile.am                    |   1 +
>  tests/data/test-read-ctf/test-alias.c     |  18 +++++------
>  tests/data/test-read-ctf/test-alias.o     | Bin 0 -> 1688 bytes
>  tests/data/test-read-ctf/test-alias.o.abi |  22 +++++++-------
>  tests/test-read-ctf.cc                    |   8 +++++
>  6 files changed, 64 insertions(+), 20 deletions(-)
>  create mode 100644 tests/data/test-read-ctf/test-alias.o
> 
> diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
> index f5f58c7a..907290d2 100644
> --- a/src/abg-ctf-reader.cc
> +++ b/src/abg-ctf-reader.cc
> @@ -1396,6 +1396,41 @@ open_elf_handler(read_context *ctxt)
>        return 0;
>      }
>  
> +  /* It Elf type is ET_REL it updates SHF_ALLOC sections in
> +     its virtual addr field, it is done by adding the previous
> +     section size and assigning this value to sh_addr field,
> +     otherwise alias symbols (if the symbol has one) is mix up */
> +  Elf *elf = ctxt->elf_handler;
> +  GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr(elf, &ehdr_mem);
> +  if (ehdr == NULL)
> +    return 0;
> +
> +  GElf_Addr end = 0;
> +  Elf_Scn *scn = NULL;
> +
> +  if (ehdr->e_type == ET_REL)
> +    while ((scn = elf_nextscn(elf, scn)) != NULL)
> +      {
> +        GElf_Shdr shdr_mem;
> +        GElf_Shdr *shdr = gelf_getshdr(scn, &shdr_mem);
> +
> +        if (shdr == NULL)
> +          return 0;
> +
> +        if (shdr->sh_flags & SHF_ALLOC)
> +          {
> +            if (shdr->sh_addr == 0)
> +              {
> +                shdr->sh_addr = end;
> +                end = shdr->sh_addr + shdr->sh_size;
> +                if ((shdr->sh_addr != 0) && !gelf_update_shdr(scn, shdr))
> +                  return 0;
> +              }
> +            else
> +              end = shdr->sh_addr + shdr->sh_size;
> +          }
> +      }
> +
>    return 1;
>  }
>  
> diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> index 782dd7f3..793a0e89 100644
> --- a/tests/data/Makefile.am
> +++ b/tests/data/Makefile.am
> @@ -632,6 +632,7 @@ test-read-ctf/test9.c		\
>  test-read-ctf/test9.o		\
>  test-read-ctf/test9.o.abi	\
>  test-read-ctf/test-alias.c	\
> +test-read-ctf/test-alias.o	\
>  test-read-ctf/test-alias.o.abi	\
>  test-read-ctf/test-ambiguous-struct-A.c	\
>  test-read-ctf/test-ambiguous-struct-A.o	\
> diff --git a/tests/data/test-read-ctf/test-alias.c b/tests/data/test-read-ctf/test-alias.c
> index 1b497a18..afc0aced 100644
> --- a/tests/data/test-read-ctf/test-alias.c
> +++ b/tests/data/test-read-ctf/test-alias.c
> @@ -1,15 +1,13 @@
> +// gcc -gctf tests/data/test-read-ctf/test-alias .c \
> +//  -c -o tests/data/test-read-ctf/test-alias.o
>  
> -// function aliases
> +void func(void);
> +void alias_func(void) __attribute__((alias("func")));
> +int foo;
>  
> -void main_func(void);
> -void alias_func(void) __attribute__((weak, alias("main_func")));
> -
> -void main_func(void)
> +void func(void)
>  {
> -
>  }
>  
> -// variables aliases
> -
> -int main_var;
> -extern int alias_var __attribute__((weak, alias("main_var")));
> +int var;
> +extern int alias_var __attribute__((alias("var")));
> diff --git a/tests/data/test-read-ctf/test-alias.o b/tests/data/test-read-ctf/test-alias.o
> new file mode 100644
> index 0000000000000000000000000000000000000000..ff5e5c536d824fc639ba4338182a870c57ddf548
> GIT binary patch
> literal 1688
> zcmbtUUuzRV5T8q9YBedThzeD>;)5-ky);sgK1eiZiin_4@THW!%ceQJ%f-7*Nkq^m
> z!MA=4AN>dgzkuJsC!w$U1$5?aH`y-n$$`0<`OW-h|I8*Io;-hAavad&z(bfv8U=X0
> zH22%N+lCrcVSn%Q&y$01@cT!(q$@VGgSLuVN4<&aqCP<Fp}s|>b+l6Z3#cooHB_o6
> zOL@Y{Qcl;STPfqmMd&(OluBX1Auh_bd_&<-LO)61n5V#_kY|VF9VcNA`r}~$-XMvE
> z=TH5qm!v$1gqNj(7ll6e!y_I>jqxaxso=3EMJ6+^$0g6*27<jtAp1p>_htb=x6|2m
> zH@h$Px7_XftjU^gtJ!KbcUn8{rkIRG8pdKMd9;;pQk@|`BXXbw&M9G992L{KNv;+K
> zoOiFlnbe*2>hi@COwyUvxnY@V9<A0tl{&S`V=PddMTYv8;n$x<xuyKJ&u3OKT!Z;z
> z(0jds_!Xl*-$RZq#&4@Wny>Axf8ap3-_Vy#11*M`&u~6hdJnnK>}L;>5SC42$$hjk
> zE$l#hQcNT;8Z0nB%Yflf@sq?fC}6`x3Wl?3WReGORD3iXvr(GhIOP;rDk9Fr;IN<a
> zSmf*f?`Itw&}{Uh*8SSb(f6xysx@T;1H$H$DzaU($nb6c9n8_YvH5$7W4|dqKi%>k
> zI(ARWY5t0pv_jZc{3ekMd8rET8W_2nukEgZnWgV_fe9|CeTEp_@wqpI@khnC_hsLQ
> zuX@`2uZmCqgQnR015DDLw#_2+zpIAydn37oN?gs?I4xtZjS!rv^-n24xr|CPw)ym5
> fYtJ4b_@bK9`!Gedok45pE#jXkel)8r9Gm|afdYk1
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/data/test-read-ctf/test-alias.o.abi b/tests/data/test-read-ctf/test-alias.o.abi
> index 1fe61b8f..f0f53ad1 100644
> --- a/tests/data/test-read-ctf/test-alias.o.abi
> +++ b/tests/data/test-read-ctf/test-alias.o.abi
> @@ -1,19 +1,21 @@
>  <abi-corpus version='2.1' path='data/test-read-ctf/test-alias.o'>
>    <elf-function-symbols>
> -    <elf-symbol name='alias_func' type='func-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
> -    <elf-symbol name='main_func' type='func-type' binding='global-binding' visibility='default-visibility' alias='alias_func,main_var,alias_var' is-defined='yes'/>
> +    <elf-symbol name='alias_func' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> +    <elf-symbol name='func' type='func-type' binding='global-binding' visibility='default-visibility' alias='alias_func' is-defined='yes'/>
>    </elf-function-symbols>
>    <elf-variable-symbols>
> -    <elf-symbol name='alias_var' size='4' type='object-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
> -    <elf-symbol name='main_var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> +    <elf-symbol name='alias_var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> +    <elf-symbol name='foo' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> +    <elf-symbol name='var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' alias='alias_var' is-defined='yes'/>
>    </elf-variable-symbols>
>    <abi-instr address-size='64' language='LANG_C'>
> -    <type-decl name='int' size-in-bits='32' alignment-in-bits='32' id='95e97e5e'/>
> -    <function-decl name='main_func' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='8'>
> -      <return type-id='48b5725f'/>
> +    <type-decl name='int' size-in-bits='32' alignment-in-bits='32' id='type-id-1'/>
> +    <function-decl name='func' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='8' elf-symbol-id='func'>
> +      <return type-id='type-id-2'/>
>      </function-decl>
> -    <var-decl name='alias_var' type-id='95e97e5e' mangled-name='alias_var' visibility='default'/>
> -    <var-decl name='main_var' type-id='95e97e5e' mangled-name='main_var' visibility='default'/>
> -    <type-decl name='void' id='48b5725f'/>
> +    <var-decl name='alias_var' type-id='type-id-1' mangled-name='alias_var' visibility='default' elf-symbol-id='alias_var'/>
> +    <var-decl name='foo' type-id='type-id-1' mangled-name='foo' visibility='default' elf-symbol-id='foo'/>
> +    <var-decl name='var' type-id='type-id-1' mangled-name='var' visibility='default' elf-symbol-id='var'/>
> +    <type-decl name='void' id='type-id-2'/>
>    </abi-instr>
>  </abi-corpus>
> diff --git a/tests/test-read-ctf.cc b/tests/test-read-ctf.cc
> index b0ef7de6..0b4e4b9d 100644
> --- a/tests/test-read-ctf.cc
> +++ b/tests/test-read-ctf.cc
> @@ -293,6 +293,14 @@ static InOutSpec in_out_specs[] =
>      "data/test-read-ctf/test-callback2.abi",
>      "output/test-read-ctf/test-callback2.abi",
>    },
> +  {
> +    "data/test-read-ctf/test-alias.o",
> +    "",
> +    "",
> +    SEQUENCE_TYPE_ID_STYLE,
> +    "data/test-read-ctf/test-alias.o.abi",
> +    "output/test-read-ctf/test-alias.o.abi",
> +  },
>    // out-of-tree kernel module.
>    {
>      "data/test-read-ctf/test-linux-module.ko",

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ctf-reader: Fix symbols alias report in ABI representation.
  2022-09-08  4:48 [PATCH] ctf-reader: Fix symbols alias report in ABI representation Guillermo E. Martinez
  2022-10-31 18:10 ` Guillermo Martinez
@ 2022-11-17 14:33 ` Dodji Seketeli
  2022-11-18 21:48   ` Guillermo E. Martinez
  2022-11-18 21:55   ` [PATCH] ctf-front-end: Add test for alias symbols Guillermo E. Martinez
  1 sibling, 2 replies; 6+ messages in thread
From: Dodji Seketeli @ 2022-11-17 14:33 UTC (permalink / raw)
  To: Guillermo E. Martinez via Libabigail; +Cc: Guillermo E. Martinez

Hello Guillermo,

"Guillermo E. Martinez via Libabigail" <libabigail@sourceware.org> a
écrit:

[...]


> @@ -1396,6 +1396,41 @@ open_elf_handler(read_context *ctxt)
>        return 0;
>      }
>  
> +  /* It Elf type is ET_REL it updates SHF_ALLOC sections in
> +     its virtual addr field, it is done by adding the previous
> +     section size and assigning this value to sh_addr field,
> +     otherwise alias symbols (if the symbol has one) is mix up */
> +  Elf *elf = ctxt->elf_handler;
> +  GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr(elf, &ehdr_mem);
> +  if (ehdr == NULL)
> +    return 0;
> +
> +  GElf_Addr end = 0;
> +  Elf_Scn *scn = NULL;
> +
> +  if (ehdr->e_type == ET_REL)
> +    while ((scn = elf_nextscn(elf, scn)) != NULL)
> +      {
> +        GElf_Shdr shdr_mem;
> +        GElf_Shdr *shdr = gelf_getshdr(scn, &shdr_mem);
> +
> +        if (shdr == NULL)
> +          return 0;
> +
> +        if (shdr->sh_flags & SHF_ALLOC)
> +          {
> +            if (shdr->sh_addr == 0)
> +              {
> +                shdr->sh_addr = end;
> +                end = shdr->sh_addr + shdr->sh_size;
> +                if ((shdr->sh_addr != 0) && !gelf_update_shdr(scn, shdr))
> +                  return 0;
> +              }
> +            else
> +              end = shdr->sh_addr + shdr->sh_size;
> +          }
> +      }
> +

Hmmh, doing this amounts to doing what the DWARF Front End Library, aka
dwfl, from elfutils does for us.  The DWARF front-end of libabigail uses
dwfl exactly for this kinds of tricks.

I think that in the new front-end branch at
https://sourceware.org/git/?p=libabigail.git;a=shortlog;h=refs/heads/users/dodji/front-end,
the CTF front end is implicitly uses dwfl as well, just like the DWARF
front end.  This is because they both derive from the
abigail::elf::reader which used the dwfl.  You can see that at
https://sourceware.org/git/?p=libabigail.git;a=blob;f=src/abg-elf-reader.cc;h=eedeaf8ece3e7cfb99a0ef392962d777630ae66b;hb=9b4a6e9e304bf8a4f21e44a497a4379826b4b1ae#l304.


>    return 1;
>  }

So I think you should maybe just check that this functionality work on
that "front-end" branch.

If it does, maybe you can just add the tests below to that branch, to
make sure we don't regress on this functionality in the future.


What do you think?

>
> diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> index 782dd7f3..793a0e89 100644
> --- a/tests/data/Makefile.am
> +++ b/tests/data/Makefile.am
> @@ -632,6 +632,7 @@ test-read-ctf/test9.c		\
>  test-read-ctf/test9.o		\
>  test-read-ctf/test9.o.abi	\
>  test-read-ctf/test-alias.c	\
> +test-read-ctf/test-alias.o	\

[...]

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ctf-reader: Fix symbols alias report in ABI representation.
  2022-11-17 14:33 ` Dodji Seketeli
@ 2022-11-18 21:48   ` Guillermo E. Martinez
  2022-11-18 21:55   ` [PATCH] ctf-front-end: Add test for alias symbols Guillermo E. Martinez
  1 sibling, 0 replies; 6+ messages in thread
From: Guillermo E. Martinez @ 2022-11-18 21:48 UTC (permalink / raw)
  To: Dodji Seketeli, Guillermo E. Martinez via Libabigail



On 11/17/22 08:33, Dodji Seketeli wrote:
> Hello Guillermo,
> 
> "Guillermo E. Martinez via Libabigail" <libabigail@sourceware.org> a
> écrit:
> 
> [...]
> 
> 
>> @@ -1396,6 +1396,41 @@ open_elf_handler(read_context *ctxt)
>>         return 0;
>>       }
>>   
>> +  /* It Elf type is ET_REL it updates SHF_ALLOC sections in
>> +     its virtual addr field, it is done by adding the previous
>> +     section size and assigning this value to sh_addr field,
>> +     otherwise alias symbols (if the symbol has one) is mix up */
>> +  Elf *elf = ctxt->elf_handler;
>> +  GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr(elf, &ehdr_mem);
>> +  if (ehdr == NULL)
>> +    return 0;
>> +
>> +  GElf_Addr end = 0;
>> +  Elf_Scn *scn = NULL;
>> +
>> +  if (ehdr->e_type == ET_REL)
>> +    while ((scn = elf_nextscn(elf, scn)) != NULL)
>> +      {
>> +        GElf_Shdr shdr_mem;
>> +        GElf_Shdr *shdr = gelf_getshdr(scn, &shdr_mem);
>> +
>> +        if (shdr == NULL)
>> +          return 0;
>> +
>> +        if (shdr->sh_flags & SHF_ALLOC)
>> +          {
>> +            if (shdr->sh_addr == 0)
>> +              {
>> +                shdr->sh_addr = end;
>> +                end = shdr->sh_addr + shdr->sh_size;
>> +                if ((shdr->sh_addr != 0) && !gelf_update_shdr(scn, shdr))
>> +                  return 0;
>> +              }
>> +            else
>> +              end = shdr->sh_addr + shdr->sh_size;
>> +          }
>> +      }
>> +
> 
> Hmmh, doing this amounts to doing what the DWARF Front End Library, aka
> dwfl, from elfutils does for us.  The DWARF front-end of libabigail uses
> dwfl exactly for this kinds of tricks.
> 
> I think that in the new front-end branch at
> https://sourceware.org/git/?p=libabigail.git;a=shortlog;h=refs/heads/users/dodji/front-end,
> the CTF front end is implicitly uses dwfl as well, just like the DWARF
> front end.  This is because they both derive from the
> abigail::elf::reader which used the dwfl.  You can see that at
> https://sourceware.org/git/?p=libabigail.git;a=blob;f=src/abg-elf-reader.cc;h=eedeaf8ece3e7cfb99a0ef392962d777630ae66b;hb=9b4a6e9e304bf8a4f21e44a497a4379826b4b1ae#l304.
> 
> 
>>     return 1;
>>   }
> 
> So I think you should maybe just check that this functionality work on
> that "front-end" branch.
> 

It's working there :-). So I just add a new test case.

> If it does, maybe you can just add the tests below to that branch, to
> make sure we don't regress on this functionality in the future.
> 

Consider it done.
> 
> What do you think?
> 

Sure. Thanks!

>>
>> diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>> index 782dd7f3..793a0e89 100644
>> --- a/tests/data/Makefile.am
>> +++ b/tests/data/Makefile.am
>> @@ -632,6 +632,7 @@ test-read-ctf/test9.c		\
>>   test-read-ctf/test9.o		\
>>   test-read-ctf/test9.o.abi	\
>>   test-read-ctf/test-alias.c	\
>> +test-read-ctf/test-alias.o	\
> 
> [...]
> 
> Cheers,
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ctf-front-end: Add test for alias symbols
  2022-11-17 14:33 ` Dodji Seketeli
  2022-11-18 21:48   ` Guillermo E. Martinez
@ 2022-11-18 21:55   ` Guillermo E. Martinez
  2022-11-21  3:46     ` [PATCHv2] " Guillermo E. Martinez
  1 sibling, 1 reply; 6+ messages in thread
From: Guillermo E. Martinez @ 2022-11-18 21:55 UTC (permalink / raw)
  To: libabigail; +Cc: Guillermo E. Martinez

This patch add a new test case in the ctf-front-end test suite to
to test for alias symbols.

Comments will be appreciated!.

Thanks,
guillermo

---
 tests/data/Makefile.am                |   1 +
 tests/data/test-read-ctf/test-alias.o | Bin 0 -> 1664 bytes
 tests/test-read-ctf.cc                |   8 ++++++++
 3 files changed, 9 insertions(+)
 create mode 100644 tests/data/test-read-ctf/test-alias.o

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 5ec33924..3d9eb9d1 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -640,6 +640,7 @@ test-read-ctf/test9.c		\
 test-read-ctf/test9.o		\
 test-read-ctf/test9.o.abi	\
 test-read-ctf/test-alias.c	\
+test-read-ctf/test-alias.o	\
 test-read-ctf/test-alias.o.abi	\
 test-read-ctf/test-ambiguous-struct-A.c	\
 test-read-ctf/test-ambiguous-struct-A.o	\
diff --git a/tests/data/test-read-ctf/test-alias.o b/tests/data/test-read-ctf/test-alias.o
new file mode 100644
index 0000000000000000000000000000000000000000..18549b793a81754d6bc319eaa0f46179a4686b49
GIT binary patch
literal 1664
zcmbtUv2GJV5S??36TpcQgb-3da-fJK@SbfX1SycgfFpz`5GkW*@4SvrobRl=O|WU8
zLZZtjQ1S_sNc;dL9Tgt{H4@CMcfekZC>UvOX5Y-4_3hj9i+d067d#KxJh%nFSBwI*
z7N&BOwqO~`u)Fj6%kka^`1ZM2;EqjBFfO5&(a)lvN3WtkLD%TC4>^rKfxd!%(MG}v
z;~q>)MSN5cqB&@#NdS*HXy?oih!PD0872EiG6NY$GB?>#8g(al?=T6$@23Oh2jgJu
zr<n|6<>y)G$59}I=s-qsEm6;W;cJ!a-0w;)O<K#8?AAivo2<-q5dw7D?d?jn^JsUg
z(zqt-qF!m%o6Sa}ezQ_lqoK;8fl9QDw*c%rn0G@06u^5=7@KF;^xkB2Ze-r`$KZ`t
zz4h|q(lHh<AVyBMNRQhqtFH>}<<cE&p!Hvj<`&`Wk9p+<bK0{N(&Wq(ZuUZqG`KVe
zzl`{rQ4|w2ez&WbZ_L4gj=07rX@oNZO_Vd~95u~7F!M?CUl)%Mk&g#j1{ifViG9wr
z8fg%uCWs)<L7<ZONC%`2h$PiYpeD6k%kZgPA0&rjn5C$p9)rkKEQRXt_cA$9X8-?v
ztm6REgfBDCYuSX)n{mDuyxzcsu&K|}lseqtM3=vbb-FJuf5-AzKOW<*Nmq9!^)mlE
z6K!;R8;BEDclz$y#w!lae2&|e@3cY64Nk#@A7Gu%c;*h_dT1Nn-{pQ=0rpa>%YS3}
zH?THsE`JY;bf&H8Cd}X2f%d-+P9a8I<}*HTZyNQ<?*En+Xs6LhW0!x@;uvP^rQQGf
U4Em?mYgl*h4pBZ@e*6#qPtYBU=>Px#

literal 0
HcmV?d00001

diff --git a/tests/test-read-ctf.cc b/tests/test-read-ctf.cc
index 6dc2d53f..8da196c8 100644
--- a/tests/test-read-ctf.cc
+++ b/tests/test-read-ctf.cc
@@ -302,6 +302,14 @@ static InOutSpec in_out_specs[] =
     "data/test-read-ctf/test-linux-module.abi",
     "output/test-read-ctf/test-linux-module.abi",
   },
+  {
+    "data/test-read-ctf/test-alias.o",
+    "",
+    "",
+    SEQUENCE_TYPE_ID_STYLE,
+    "data/test-read-ctf/test-alias.o.abi",
+    "output/test-read-ctf/test-alias.o.abi",
+  },
   // This should be the last entry.
   {NULL, NULL, NULL, SEQUENCE_TYPE_ID_STYLE, NULL, NULL}
 };
-- 
2.35.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2] ctf-front-end: Add test for alias symbols
  2022-11-18 21:55   ` [PATCH] ctf-front-end: Add test for alias symbols Guillermo E. Martinez
@ 2022-11-21  3:46     ` Guillermo E. Martinez
  0 siblings, 0 replies; 6+ messages in thread
From: Guillermo E. Martinez @ 2022-11-21  3:46 UTC (permalink / raw)
  To: libabigail; +Cc: Guillermo E. Martinez

This patch v2 to add a new test case in the ctf-front-end test suite to
to test for alias symbols.

Changes from v1:
  + I forget to add the correct tests/data/test-read-ctf/test-alias.o.abi file.

Comments will be appreciated!.

Thanks,
guillermo

---
 tests/data/Makefile.am                    |   1 +
 tests/data/test-read-ctf/test-alias.o     | Bin 0 -> 1664 bytes
 tests/data/test-read-ctf/test-alias.o.abi |  16 ++++++++--------
 tests/test-read-ctf.cc                    |   8 ++++++++
 4 files changed, 17 insertions(+), 8 deletions(-)
 create mode 100644 tests/data/test-read-ctf/test-alias.o

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 5ec33924..3d9eb9d1 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -640,6 +640,7 @@ test-read-ctf/test9.c		\
 test-read-ctf/test9.o		\
 test-read-ctf/test9.o.abi	\
 test-read-ctf/test-alias.c	\
+test-read-ctf/test-alias.o	\
 test-read-ctf/test-alias.o.abi	\
 test-read-ctf/test-ambiguous-struct-A.c	\
 test-read-ctf/test-ambiguous-struct-A.o	\
diff --git a/tests/data/test-read-ctf/test-alias.o b/tests/data/test-read-ctf/test-alias.o
new file mode 100644
index 0000000000000000000000000000000000000000..18549b793a81754d6bc319eaa0f46179a4686b49
GIT binary patch
literal 1664
zcmbtUv2GJV5S??36TpcQgb-3da-fJK@SbfX1SycgfFpz`5GkW*@4SvrobRl=O|WU8
zLZZtjQ1S_sNc;dL9Tgt{H4@CMcfekZC>UvOX5Y-4_3hj9i+d067d#KxJh%nFSBwI*
z7N&BOwqO~`u)Fj6%kka^`1ZM2;EqjBFfO5&(a)lvN3WtkLD%TC4>^rKfxd!%(MG}v
z;~q>)MSN5cqB&@#NdS*HXy?oih!PD0872EiG6NY$GB?>#8g(al?=T6$@23Oh2jgJu
zr<n|6<>y)G$59}I=s-qsEm6;W;cJ!a-0w;)O<K#8?AAivo2<-q5dw7D?d?jn^JsUg
z(zqt-qF!m%o6Sa}ezQ_lqoK;8fl9QDw*c%rn0G@06u^5=7@KF;^xkB2Ze-r`$KZ`t
zz4h|q(lHh<AVyBMNRQhqtFH>}<<cE&p!Hvj<`&`Wk9p+<bK0{N(&Wq(ZuUZqG`KVe
zzl`{rQ4|w2ez&WbZ_L4gj=07rX@oNZO_Vd~95u~7F!M?CUl)%Mk&g#j1{ifViG9wr
z8fg%uCWs)<L7<ZONC%`2h$PiYpeD6k%kZgPA0&rjn5C$p9)rkKEQRXt_cA$9X8-?v
ztm6REgfBDCYuSX)n{mDuyxzcsu&K|}lseqtM3=vbb-FJuf5-AzKOW<*Nmq9!^)mlE
z6K!;R8;BEDclz$y#w!lae2&|e@3cY64Nk#@A7Gu%c;*h_dT1Nn-{pQ=0rpa>%YS3}
zH?THsE`JY;bf&H8Cd}X2f%d-+P9a8I<}*HTZyNQ<?*En+Xs6LhW0!x@;uvP^rQQGf
U4Em?mYgl*h4pBZ@e*6#qPtYBU=>Px#

literal 0
HcmV?d00001

diff --git a/tests/data/test-read-ctf/test-alias.o.abi b/tests/data/test-read-ctf/test-alias.o.abi
index 1fe61b8f..b5fd92ad 100644
--- a/tests/data/test-read-ctf/test-alias.o.abi
+++ b/tests/data/test-read-ctf/test-alias.o.abi
@@ -1,19 +1,19 @@
 <abi-corpus version='2.1' path='data/test-read-ctf/test-alias.o'>
   <elf-function-symbols>
     <elf-symbol name='alias_func' type='func-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
-    <elf-symbol name='main_func' type='func-type' binding='global-binding' visibility='default-visibility' alias='alias_func,main_var,alias_var' is-defined='yes'/>
+    <elf-symbol name='main_func' type='func-type' binding='global-binding' visibility='default-visibility' alias='alias_func' is-defined='yes'/>
   </elf-function-symbols>
   <elf-variable-symbols>
     <elf-symbol name='alias_var' size='4' type='object-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
-    <elf-symbol name='main_var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='main_var' size='4' type='object-type' binding='global-binding' visibility='default-visibility' alias='alias_var' is-defined='yes'/>
   </elf-variable-symbols>
   <abi-instr address-size='64' language='LANG_C'>
-    <type-decl name='int' size-in-bits='32' alignment-in-bits='32' id='95e97e5e'/>
-    <function-decl name='main_func' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='8'>
-      <return type-id='48b5725f'/>
+    <type-decl name='int' size-in-bits='32' alignment-in-bits='32' id='type-id-1'/>
+    <function-decl name='main_func' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='8' elf-symbol-id='main_func'>
+      <return type-id='type-id-2'/>
     </function-decl>
-    <var-decl name='alias_var' type-id='95e97e5e' mangled-name='alias_var' visibility='default'/>
-    <var-decl name='main_var' type-id='95e97e5e' mangled-name='main_var' visibility='default'/>
-    <type-decl name='void' id='48b5725f'/>
+    <var-decl name='alias_var' type-id='type-id-1' mangled-name='alias_var' visibility='default' elf-symbol-id='alias_var'/>
+    <var-decl name='main_var' type-id='type-id-1' mangled-name='main_var' visibility='default' elf-symbol-id='main_var'/>
+    <type-decl name='void' id='type-id-2'/>
   </abi-instr>
 </abi-corpus>
diff --git a/tests/test-read-ctf.cc b/tests/test-read-ctf.cc
index 6dc2d53f..8da196c8 100644
--- a/tests/test-read-ctf.cc
+++ b/tests/test-read-ctf.cc
@@ -302,6 +302,14 @@ static InOutSpec in_out_specs[] =
     "data/test-read-ctf/test-linux-module.abi",
     "output/test-read-ctf/test-linux-module.abi",
   },
+  {
+    "data/test-read-ctf/test-alias.o",
+    "",
+    "",
+    SEQUENCE_TYPE_ID_STYLE,
+    "data/test-read-ctf/test-alias.o.abi",
+    "output/test-read-ctf/test-alias.o.abi",
+  },
   // This should be the last entry.
   {NULL, NULL, NULL, SEQUENCE_TYPE_ID_STYLE, NULL, NULL}
 };
-- 
2.35.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-21  3:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-08  4:48 [PATCH] ctf-reader: Fix symbols alias report in ABI representation Guillermo E. Martinez
2022-10-31 18:10 ` Guillermo Martinez
2022-11-17 14:33 ` Dodji Seketeli
2022-11-18 21:48   ` Guillermo E. Martinez
2022-11-18 21:55   ` [PATCH] ctf-front-end: Add test for alias symbols Guillermo E. Martinez
2022-11-21  3:46     ` [PATCHv2] " Guillermo E. Martinez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).