public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: "Guillermo E. Martinez" <guillermo.e.martinez@oracle.com>
To: Ben Woodard <woodard@redhat.com>
Cc: libabigail@sourceware.org
Subject: Re: Use CTF as a fallback when no DWARFs debug info is present
Date: Thu, 14 Jul 2022 10:06:43 -0500	[thread overview]
Message-ID: <3494804.5fSG56mABF@sali> (raw)
In-Reply-To: <952C9B5A-C267-4BC2-9DF1-4045C73C704C@redhat.com>

On Wednesday, July 13, 2022 8:37:13 PM CDT Ben Woodard wrote:
Hi Ben,

> So let me begin by saying, I think we should push changes to the API like this until after 2.1 is released. Dodji isn’t calling it a beta or RC yet but we have been treating it that way. I’ve been trying to get all my testing done and he has been trying to get those same bugs fixed.
> 
> After that, I want to really revisit the API and make it more generally useful and less closely tied to the tools. What you are proposing below is a baby step along the lines of what I I had in mind.
> 
> > On Jul 13, 2022, at 4:25 PM, Guillermo E. Martinez via Libabigail <libabigail@sourceware.org> wrote:
> > 
> > Hello libabigail team,
> > 
> > Talking with my colleges Jose E.  Marchesi and Nick Alcock  about  of  the  user
> > interface provided by abidw, abidiff and some  other  tools  in  libabigail,  we
> > think such tools should be looks for DWARF debug information, and if it  is  not
> > present fall back for CTF even without --ctf option, this behaviour is  used  by
> > GDB (looking for .ctf and .debug_* section,
> 
> First of all, I think rather than having a:
> 
> —enable-ctf compile time option
> 
> Why don’t we simply make “WITH_CTF” an autoconf option that is disabled if AC_CHECK_HEADER and AC_CHECK_LIB doesn’t find the things that you need.
Ok, in that case we need to decide whether CTF support always will ship in libabigail,
i.e, if the only dependency: libctf is meet, then CTF support will be there by default.
> 
> 
> I don’t think that it should matter where we get the information about the types from.
Yes.
> I would also be tempted to get rid the —use-ctf options except I think that we should have the ability to do something like
> abidw —abidiff —use-ctf somelib.so 
> And it would read the DWARF, then read the CTF and then compare the corpus that we get from them.
Ok, IMHO, it just be needed in the testing harness, because at the end of the day both corpus
with the same input, will be generated no differences, just in some properties e.g: filepath, line,
column, etc.
> Or maybe it would be 
> > but in libabigail  is  done  by  the
> > symtab class), so I did some minor changes in  abidw  and  abidiff  
> 
> don’t forget abicompat
Ok.
> > setting  the
> > corpus origin depending of `opts.use_ctf', trying to build the copus with  DWARF
> > debug information, but if it  is  not  possible  (`STATUS_DEBUG_INFO_NOT_FOUND')
> > looks for CTF info.
> 
> I think that we should do a deeper rework here and work the way that GDB works. In essence doing:
> 
> In essence:
> read ELF
> if( elf-file has DWARF) then use that
> else if (look for .gnu_reflinks DWARF) then use that
> else if( look for DWARF5 split DWARF) then use that
> else if( look for CTF) then use that
> else if (fail-on-no-debug-info) then exit 
> else warn on limited functionality.
> 
> I think that all this logic should be encapsulated in an exported function  read_corpus and then all the programs use that interface rather than the functions which are specific to what format the information is gathered from.
Agree, because now days every reader implements the logic to get ELF info:
   `read_debug_info_into_corpus' and `slurp_elf_info'.
> > corpus::origin origin = opts.use_ctf ? CTF_ORIGIN : DWARF_ORIGIN;
> > ...
> > if (origin == DWARF_ORIGIN)
> >  {
> >     // Build corpus with DWARF
> >  }
> > if ((status & STATUS_DEBUG_INFO_NOT_FOUND) ||
> >    origin == CTF_ORIGIN)
> >  {
> >     // Build corpus with CTF
> >  }
> > 
> > if (status & STATUS_DEBUG_INFO_NOT_FOUND)
> >  {
> >     // lets to know the user that no debug info
> >     // was found in DWARF neither CTF
> >  }
> > 
> > Doing in this way, seem to work, however there are functions  to  notifying  the
> > user about of  missing  debug  information  (e.g  `handle_error')  that  use  an
> > specific DWARF `read_context' reference, so if we want  to  reuse  it,  then  we
> > could split common functionality/interface for both readers  in  a  base  class:
> > 
> 
> Dodji can weigh in here but this whole contex thing as an API is one of the things that I wanted get rid of in the general purpose API, IMHO it is too tightly tied into how the tools work. I kind of know what he was trying to do, he didn’t want 50 parameters for the functions and so he made this class to pass them all at once.
> I think we need to think about making a good general purpose API and I think we can do better than sticking a data structure with parsed command line options in it.
> > class base_read_context
> > {
> >  ...
> >  virtual const string&
> >  alt_debug_info_path() const;
> > 
> >  void
> >  exported_decls_builder(corpus::exported_decls_builder* b);
> >  ...
> > };
> > 
> > class dwarf_reader::read_context : public base_read_context
> > {
> >  ...
> > };
> > 
> > class ctf_reader::read_context : public base_read_context
> > {
> >  ...
> > };
> > 
> > 
> > // Now reusing refers_to_alt_debug_info for both readers
> > bool
> > refers_to_alt_debug_info(const read_context&	ctxt,
> >                         string& alt_di_path)
> > {
> >  ...
> > }
> > 
> > Please let met know your feedback, I'll really appreciate! 
> 
> If you want to collaborate on this, send me a PR and we can iterate on it on my develop branch on github and then I will push it to Dodji right after we release 2.1
Ok, of course I will do so following the advice of people that have more experience working  in libabigail. 
Thanks for your comments!
> https://github.com/woodard/libabigail

Regards,
guillermo




  reply	other threads:[~2022-07-14 15:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-13 23:25 Guillermo E. Martinez
2022-07-14  1:37 ` Ben Woodard
2022-07-14 15:06   ` Guillermo E. Martinez [this message]
2022-07-14 16:01     ` Ben Woodard
2022-07-14 17:35       ` Guillermo E. Martinez
2022-07-14 18:09         ` Ben Woodard
2022-07-14 23:13           ` Guillermo Martinez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3494804.5fSG56mABF@sali \
    --to=guillermo.e.martinez@oracle.com \
    --cc=libabigail@sourceware.org \
    --cc=woodard@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).