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

[Dodji Seketeli <dodji@seketeli.org>]

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