Re: [PATCH] ctf: make libabigail::ctf_reader::read_corpus reentrant

[Dodji Seketeli <dodji@seketeli.org>]

Hello Jose,

"Jose E. Marchesi via Libabigail" <libabigail@sourceware.org> a écrit:

> The libctf call ctf_open is not reentrant.  This is because it uses
> bfd_open (and other BFD calls) internally in order to fetch the
> different bits of CTF from the ELF file.
>
> This is unfortunate, as it makes libabigail::ctf_reader::read_corpus
> non-reentrant.  We detected this problem thanks to one of the
> libabigail test driver, that exercises tests in parallel using
> threads.
>
> Fortunately libctf provides an alternate way to decode CTF data, that
> involves the user to provide the raw contents of the relevant ELF
> sections (.ctf, the symtab, the string table) to ctf_arc_bufopen
> call.
>
> This patch changes the CTF reader in libabigail to use this
> mechanism.  libelf is used in order to extract the contents of these
> sections.
>
> 	* src/abg-ctf-reader.cc (class read_context): New attributes
> 	elf_handler, elf_fd, ctf_sect, symtab_sec and strtab_sect.
> 	(read_context): Do not read the CTF archive here.
> 	(slurp_elf_info): Adjust to use attributes instead of locals, and
> 	fetch the raw ELF section contents for libctf.
> 	(close_elf_handler): New function.
> 	(fill_ctf_section): Likewise.
> 	(read_corpus): Call open_elf_handler, close_elf_handler and build
> 	the CTF archive using ctf_arc_bufopen instead of ctf_open.
>
> Signed-by: Jose E. Marchesi <jose.marchesi@oracle.com>

The patch looks great to me.

I just picked some minor nits below and applied the result to master.

[...]

>  src/abg-ctf-reader.cc | 135 ++++++++++++++++++++++++++++++++++++++------------

[...]

>  static int
> -slurp_elf_info(read_context *ctxt, corpus_sptr corp)
> +open_elf_handler (read_context *ctxt)

I removed the space before the parenthesis.
>  {
>    /* libelf requires to negotiate/set the version of ELF.  */
>    if (elf_version(EV_CURRENT) == EV_NONE)
>      return 0;

[...]


> @@ -1012,19 +1077,27 @@ read_corpus(read_context *ctxt)
>    corpus_sptr corp
>      = std::make_shared<corpus>(ctxt->ir_env, ctxt->filename);

[...]


> +  /* Build the cfta from the contents of the relevant ELF sections,

I changed cfta into ctfa.

> +     and process the CTF archive in the read context, if any.
> +     Information about the types, variables, functions, etc contained
> +     in the archive are added to the given corpus.  */
> +  int errp;
> +  ctxt->ctfa = ctf_arc_bufopen(&ctxt->ctf_sect, &ctxt->symtab_sect,
> +                               &ctxt->strtab_sect, &errp);
> +  if (ctxt->ctfa != NULL)
> +    process_ctf_archive(ctxt, corp);
> +
> +  /* Cleanup and return.  */
> +  close_elf_handler(ctxt);
>    return corp;
>  }

[...]

Applied to master.

Thanks!

Cheers,

-- 
		Dodji