Re: [PATCH] ctf-reader: Fix symbols alias report in ABI representation.

[Guillermo Martinez <guillermo.e.martinez@oracle.com>]

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",