public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: "Guillermo E. Martinez" <guillermo.e.martinez@oracle.com>
To: Dodji Seketeli <dodji@seketeli.org>,
	"Guillermo E. Martinez via Libabigail"
	<libabigail@sourceware.org>
Subject: Re: [PATCH v4] ELF based front-end readers fallback feature
Date: Mon, 28 Nov 2022 15:59:11 -0600	[thread overview]
Message-ID: <522c1be2-4ea0-aaa4-b512-f16b6de13e01@oracle.com> (raw)
In-Reply-To: <87fse3oywm.fsf@seketeli.org>



On 28/11/22 9:56, Dodji Seketeli wrote:
> Hello Guillermo,
> 

Hello Dodji,

> Many thanks for the update!  The patch looks good to me.
> 

Your welcome!.

> I have amended it somewhat and applied the result to the master branch.
> 
> Please find below my comments.
> 
> "Guillermo E. Martinez via Libabigail" <libabigail@sourceware.org> a
> écrit:
> 
>> By default, `abidw', `abidiff', `abipkgdiff' and `kmidiff' tools use
>> debug information in `DWARF` format, if present, otherwise now
>> automatically the tools try to extract and build the IR using
>> debug information in `CTF` format without use of `--ctf' option, if
>> present, finally, if neither is found, they use only `ELF` symbols to
>> extract, build, compare and report the binary IR. To force the use of
>> CTF front-end the `--ctf' option should be pass to command line.
>>
>> It works for libraries and Linux kernel.  The `--ctf' option is
>> preserved to explicitly indicate to those tools that we want to use
>> CTF support. By other hand, if tools use `--ctf' but binary doesn't
>> have CTF debug information it looks for DWARF automatically.
> 
> I have slightly amended this part of the commit log.  You'll see the
> resulting patch at the very end of this message.
> 

OK.

> [...]
> 
>> diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
>> index 5fde94f3..fdefd3a6 100644
>> --- a/src/abg-ctf-reader.cc
>> +++ b/src/abg-ctf-reader.cc
> 
> [...]
> 
> 
>>     /// Getter of the environment of the current CTF reader.
>>     ///
>>     /// @return the environment of the current CTF reader.
>> @@ -349,27 +355,24 @@ public:
>>       // Read the ELF-specific parts of the corpus.
>>       elf::reader::read_corpus(status);
>>   
>> -    if ((status & STATUS_NO_SYMBOLS_FOUND)
>> -	|| !(status & STATUS_OK))
>> -      // Either we couldn't find ELF symbols or something went badly
>> -      // wrong.  There is nothing we can do with this ELF file.  Bail
>> -      // out.
>> -      return;
>> -
>>       corpus_sptr corp = corpus();
>>       if ((corp->get_origin() & corpus::LINUX_KERNEL_BINARY_ORIGIN)
>>   	&& corpus_group())
>>         {
>> -	status |= fe_iface::STATUS_OK;
>> +	// Is expected not find debug information when we're building
>> +	// a kABI.
> 
> I have slightly amended the comment here to make it read like this:
> 
> +	// Not finding any debug info so far is expected if we are
> +	// building a kABI.
> 
>> +        status &= static_cast<abigail::fe_iface::status>
>> +                    (~STATUS_DEBUG_INFO_NOT_FOUND);
>>   	return;
>>         }
>>   
>> -    /* Get the raw ELF section contents for libctf.  */
>> -    if (!find_ctf_section())
>> -      {
>> -	status |= fe_iface::STATUS_DEBUG_INFO_NOT_FOUND;
>> -	return;
>> -      }
>> +    if ((status & (STATUS_NO_SYMBOLS_FOUND |
>> +                   STATUS_DEBUG_INFO_NOT_FOUND))
>> +	|| !(status & STATUS_OK))
> 
> the STATUS_DEBUG_INFO_NOT_FOUND bit cannot be set here because you have
> unset it above by doing:
> 
>> +        status &= static_cast<abigail::fe_iface::status>
>> +                    (~STATUS_DEBUG_INFO_NOT_FOUND);
> 
> So I have changed that condition to make it read:
> 

OK.

> +    if ((status & STATUS_NO_SYMBOLS_FOUND)
> +	|| !(status & STATUS_OK))
> 
> [...]
> 
> 
>> diff --git a/src/abg-elf-reader.cc b/src/abg-elf-reader.cc
>> index eedeaf8e..3f191bda 100644
>> --- a/src/abg-elf-reader.cc
>> +++ b/src/abg-elf-reader.cc
> 
> [...]
> 
> 
>>   
>> +bool
>> +reader::has_dwarf_debug_info() const
> 
> I have added a doxygen comment to this function.
> 
>> +{return ((priv_->dwarf_handle != nullptr)
>> +	  || (priv_->alt_dwarf_handle != nullptr));}
>> +
>> +bool
>> +reader::has_ctf_debug_info() const
> 
> Likewise.
> 
>> +{return (priv_->ctf_section != nullptr);}
>> +
> 
> [...]
> 
>> diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
>> index 8898ef97..0a523b87 100644
> 
> [...]
> 
>> diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h
>> index 9dc9b8d3..13d6ad75 100644
>> --- a/include/abg-tools-utils.h
>> +++ b/include/abg-tools-utils.h
>> @@ -322,7 +322,10 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>>   elf_based_reader_sptr
>>   create_best_elf_based_reader(const string& elf_file_path,
>>   			     const vector<char**>& debug_info_root_paths,
>> -			     environment& env);
>> +			     environment& env,
>> +			     bool use_ctf,
> 
> Rather than adding a boolean here, I have added a parameter
> 'corpus::origin requested_fe_kind'.  This is basically the kind of
> front-end requested by the user when invoking the tool.  For instance,
> if --ctf is provided on the command line, requested_fe_kind should be
> set to corpus::CTF_ORIGIN.  If nothing is provided on the command line,
> either requested_fe_kind could be set to either
> corpus::ARTIFICIAL_ORIGIN or corpus::DWARF_ORIGIN.  When we have another
> kind of front-end tomorrow, requested_fe_kind will still be able to be
> used because that new front-end is likely to be for a new kind
> corpus::origin.  So, I think that doing this is a more generic solution.
> 

Oh, much better to be ready for adding new frond-ends.

>> +			     bool show_all_types,
>> +			     bool linux_kernel_mode = false);
>>   
>>   }// end namespace tools_utils
>>   
> 
>> --- a/src/abg-tools-utils.cc
>> +++ b/src/abg-tools-utils.cc
>> @@ -405,6 +405,23 @@ is_regular_file(const string& path)
>>     return false;
>>   }
> 
> [...]
> 
>> @@ -2853,13 +2807,16 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>>   /// Create the best elf based reader (or front-end), given an ELF file.
>>   ///
>>   /// This function looks into the ELF file.  If it contains DWARF debug
>> -/// info, then a DWARF Reader front-end is created and returned.
>> -/// Otherwise, if it contains CTF debug info, then a CTF Reader
>> -/// front-end is created and returned.
>> +/// info, then a DWARF Reader front-end is created and returned, unless
>> +/// that @ref use_ctf be true.  Otherwise, if it contains CTF debug info,
>> +/// then a CTF Reader front-end is created and returned.
>> +///
>> +/// By other hand, it selects the DWARF front-end when @ref use_ctf is
>> +/// true but ELF file doesn't have CTF debug information.
>>   ///
>>   /// Otherwise, if the file contains no debug info or if the king of
>> -/// debug info is not yet recognized, a DWARF Reader front-end is
>> -/// created and returned.
>> +/// debug info is not yet recognized, a DWARF or CTF  Reader front-end is
>> +/// created and returned depending of @ref use_ctf parameter.
>>   ///
>>   /// @param elf_file_path a path to the ELF file to consider
>>   ///
>> @@ -2868,32 +2825,62 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>>   ///
>>   /// @param env the environment to use for the front-end.
>>   ///
>> +/// @param use_ctf set to true if it's going to use CTF front-end.
>> +///
>> +/// @param show_all_types option to be passed to elf based readers.
>> +///
>> +/// @param linux_kernel_mode option to bed passed to elf based readers,
>> +///
>>   /// @return the ELF based Reader that is better adapted for the binary
>>   /// designated by @p elf_file_path.
>>   elf_based_reader_sptr
>>   create_best_elf_based_reader(const string& elf_file_path,
>>   			     const vector<char**>& debug_info_root_paths,
>> -			     environment& env)
>> +			     environment& env,
>> +			     bool use_ctf,
>> +			     bool show_all_types,
>> +			     bool linux_kernel_mode)
> 
> Following my comment above about the declaration of this function, I
> have updated its definition here.
> 
> Near the end of this message, I'll paste my changes against the tree
> containing your patch, so that you can see clearly the changes I've made
> to create_best_elf_based_reader and to all the other related parts of
> the patch.  But right now, please find the hunk I have come up with
> about this specific location of the source code:

OK.

> 
>      -/// Create the best elf based reader (or front-end), given an ELF file.
>      +/// Create the best elf based reader (or front-end), given an ELF
>      +/// file.
>      +///
>      +/// This function looks into the ELF file; depending on the kind of
>      +/// debug info it contains and on the request of the user, the "best"
>      +/// front-end is created.
>       ///
>      -/// This function looks into the ELF file.  If it contains DWARF debug
>      -/// info, then a DWARF Reader front-end is created and returned, unless
>      -/// that @ref use_ctf be true.  Otherwise, if it contains CTF debug info,
>      -/// then a CTF Reader front-end is created and returned.
>      +/// If the user requested the use of the CTF front-end, then, if the
>      +/// file contains CTF debug info, the CTF front-end is created,
>      +/// assuming libabigail is built with CTF support.
>       ///
>      -/// By other hand, it selects the DWARF front-end when @ref use_ctf is
>      -/// true but ELF file doesn't have CTF debug information.
>      +/// If the binary ONLY has CTF debug info, then CTF front-end is
>      +/// created, even if the user hasn't explicitly requested the creation
>      +/// of the CTF front-end.
>       ///
>      -/// Otherwise, if the file contains no debug info or if the king of
>      -/// debug info is not yet recognized, a DWARF or CTF  Reader front-end is
>      -/// created and returned depending of @ref use_ctf parameter.
>      +/// Otherwise, by default, the DWARF front-end is created.
>       ///
>       /// @param elf_file_path a path to the ELF file to consider
>       ///
>      @@ -2825,7 +2824,10 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>       ///
>       /// @param env the environment to use for the front-end.
>       ///
>      -/// @param use_ctf set to true if it's going to use CTF front-end.
>      +/// @param requested_fe_kind the kind of front-end specifically
>      +/// requested by the user. At the moment, only the CTF front-end can
>      +/// be requested, using the "--ctf" command line option on some tools
>      +/// using the library.
>       ///
>       /// @param show_all_types option to be passed to elf based readers.
>       ///
>      @@ -2837,7 +2839,7 @@ elf_based_reader_sptr
>       create_best_elf_based_reader(const string& elf_file_path,
>                                   const vector<char**>& debug_info_root_paths,
>                                   environment& env,
>      -			     bool use_ctf,
>      +			     corpus::origin requested_fe_kind,
>                                   bool show_all_types,
>                                   bool linux_kernel_mode)
>       {
>      @@ -2845,41 +2847,36 @@ create_best_elf_based_reader(const string& elf_file_path,
>         if (guess_file_type(elf_file_path) != FILE_TYPE_ELF)
>           return result;
> 
>      +  if (requested_fe_kind & corpus::CTF_ORIGIN)
>      +    {
>       #ifdef WITH_CTF
>      -  if (!use_ctf)
>      +      if (file_has_ctf_debug_info(elf_file_path, debug_info_root_paths))
>      +	result = ctf::create_reader(elf_file_path, debug_info_root_paths, env);
>      +#endif
>      +    }
>      +  else
>           {
>      +      // The user hasn't formally requested the use of the CTF front-end.
>      +#ifdef WITH_CTF
>      +      if (!file_has_dwarf_debug_info(elf_file_path, debug_info_root_paths)
>      +	  && file_has_ctf_debug_info(elf_file_path, debug_info_root_paths))
>      +	// The file has CTF debug info and no DWARF, let's use the CTF
>      +	// front end even if it wasn't formally requested by the user.
>      +	result = ctf::create_reader(elf_file_path, debug_info_root_paths, env);
>       #endif
>      +    }
>      +
>      +  if (!result)
>      +    {
>      +      // This is the default case.  At worst, the DWARF reader knows
>      +      // how to handle just ELF data for the case where there is no
>      +      // DWARF debug info present.
>             result = dwarf::create_reader(elf_file_path,
>                                          debug_info_root_paths,
>                                          env,
>                                          show_all_types,
>                                          linux_kernel_mode);
>      -#ifdef WITH_CTF
>      -      if (!file_has_dwarf_debug_info(elf_file_path,
>      -				     debug_info_root_paths)
>      -	    && file_has_ctf_debug_info(elf_file_path,
>      -				       debug_info_root_paths))
>      -	result = ctf::create_reader(elf_file_path,
>      -				    debug_info_root_paths,
>      -				    env);
>           }
>      -  else
>      -    {
>      -      result = ctf::create_reader(elf_file_path,
>      -				  debug_info_root_paths,
>      -				  env);
>      -
>      -      if (!file_has_ctf_debug_info(elf_file_path,
>      -				   debug_info_root_paths)
>      -	    && file_has_dwarf_debug_info(elf_file_path,
>      -					 debug_info_root_paths))
>      -        result = dwarf::create_reader(elf_file_path,
>      -				      debug_info_root_paths,
>      -				      env,
>      -				      show_all_types,
>      -				      linux_kernel_mode);
>      -    }
>      -#endif
> 
>         return result;
>       }
> 
> [...]
> 
> I have adjusted the rest of the code accordingly.
> 
> Please find below the diff of my changed related to
> create_best_elf_based_reader and to other related parts.
> 
> --------------------------->8<------------------------------------------
> diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h
> index 13d6ad75..3d7f0d23 100644
> --- a/include/abg-tools-utils.h
> +++ b/include/abg-tools-utils.h
> @@ -317,13 +317,13 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>   					  suppr::suppressions_type&	supprs,
>   					  bool				verbose,
>   					  environment&			env,
> -					  corpus::origin	origin = corpus::DWARF_ORIGIN);
> +					  corpus::origin	requested_fe_kind = corpus::DWARF_ORIGIN);
>   
>   elf_based_reader_sptr
>   create_best_elf_based_reader(const string& elf_file_path,
>   			     const vector<char**>& debug_info_root_paths,
>   			     environment& env,
> -			     bool use_ctf,
> +			     corpus::origin requested_debug_info_kind,
>   			     bool show_all_types,
>   			     bool linux_kernel_mode = false);
>   
> diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
> index 0a523b87..b089b69c 100644
> --- a/src/abg-tools-utils.cc
> +++ b/src/abg-tools-utils.cc
> @@ -2755,7 +2755,7 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>   					  suppressions_type&	supprs,
>   					  bool			verbose,
>   					  environment&		env,
> -					  corpus::origin	origin)
> +					  corpus::origin	requested_fe_kind)
>   {
>     string vmlinux = vmlinux_path;
>     corpus_group_sptr group;
> @@ -2787,11 +2787,7 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>           create_best_elf_based_reader(vmlinux,
>                                        di_roots,
>                                        env,
> -#ifdef WITH_CTF
> -                                     origin & corpus::CTF_ORIGIN,
> -#else
> -                                     false,
> -#endif
> +				     requested_fe_kind,
>                                        /*read_all_types=*/false,
>                                        /*linux_kernel_mode=*/true);
>         ABG_ASSERT(reader);
> @@ -2804,19 +2800,22 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>     return group;
>   }
>   
> -/// Create the best elf based reader (or front-end), given an ELF file.
> +/// Create the best elf based reader (or front-end), given an ELF
> +/// file.
> +///
> +/// This function looks into the ELF file; depending on the kind of
> +/// debug info it contains and on the request of the user, the "best"
> +/// front-end is created.
>   ///
> -/// This function looks into the ELF file.  If it contains DWARF debug
> -/// info, then a DWARF Reader front-end is created and returned, unless
> -/// that @ref use_ctf be true.  Otherwise, if it contains CTF debug info,
> -/// then a CTF Reader front-end is created and returned.
> +/// If the user requested the use of the CTF front-end, then, if the
> +/// file contains CTF debug info, the CTF front-end is created,
> +/// assuming libabigail is built with CTF support.
>   ///
> -/// By other hand, it selects the DWARF front-end when @ref use_ctf is
> -/// true but ELF file doesn't have CTF debug information.
> +/// If the binary ONLY has CTF debug info, then CTF front-end is
> +/// created, even if the user hasn't explicitly requested the creation
> +/// of the CTF front-end.
>   ///
> -/// Otherwise, if the file contains no debug info or if the king of
> -/// debug info is not yet recognized, a DWARF or CTF  Reader front-end is
> -/// created and returned depending of @ref use_ctf parameter.
> +/// Otherwise, by default, the DWARF front-end is created.
>   ///
>   /// @param elf_file_path a path to the ELF file to consider
>   ///
> @@ -2825,7 +2824,10 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>   ///
>   /// @param env the environment to use for the front-end.
>   ///
> -/// @param use_ctf set to true if it's going to use CTF front-end.
> +/// @param requested_fe_kind the kind of front-end specifically
> +/// requested by the user. At the moment, only the CTF front-end can
> +/// be requested, using the "--ctf" command line option on some tools
> +/// using the library.
>   ///
>   /// @param show_all_types option to be passed to elf based readers.
>   ///
> @@ -2837,7 +2839,7 @@ elf_based_reader_sptr
>   create_best_elf_based_reader(const string& elf_file_path,
>   			     const vector<char**>& debug_info_root_paths,
>   			     environment& env,
> -			     bool use_ctf,
> +			     corpus::origin requested_fe_kind,
>   			     bool show_all_types,
>   			     bool linux_kernel_mode)
>   {
> @@ -2845,41 +2847,36 @@ create_best_elf_based_reader(const string& elf_file_path,
>     if (guess_file_type(elf_file_path) != FILE_TYPE_ELF)
>       return result;
>   
> +  if (requested_fe_kind & corpus::CTF_ORIGIN)
> +    {
>   #ifdef WITH_CTF
> -  if (!use_ctf)
> +      if (file_has_ctf_debug_info(elf_file_path, debug_info_root_paths))
> +	result = ctf::create_reader(elf_file_path, debug_info_root_paths, env);
> +#endif
> +    }
> +  else
>       {
> +      // The user hasn't formally requested the use of the CTF front-end.
> +#ifdef WITH_CTF
> +      if (!file_has_dwarf_debug_info(elf_file_path, debug_info_root_paths)
> +	  && file_has_ctf_debug_info(elf_file_path, debug_info_root_paths))
> +	// The file has CTF debug info and no DWARF, let's use the CTF
> +	// front end even if it wasn't formally requested by the user.
> +	result = ctf::create_reader(elf_file_path, debug_info_root_paths, env);
>   #endif
> +    }
> +
> +  if (!result)
> +    {
> +      // This is the default case.  At worst, the DWARF reader knows
> +      // how to handle just ELF data for the case where there is no
> +      // DWARF debug info present.
>         result = dwarf::create_reader(elf_file_path,
>   				    debug_info_root_paths,
>   				    env,
>   				    show_all_types,
>   				    linux_kernel_mode);
> -#ifdef WITH_CTF
> -      if (!file_has_dwarf_debug_info(elf_file_path,
> -				     debug_info_root_paths)
> -	    && file_has_ctf_debug_info(elf_file_path,
> -				       debug_info_root_paths))
> -	result = ctf::create_reader(elf_file_path,
> -				    debug_info_root_paths,
> -				    env);
>       }
> -  else
> -    {
> -      result = ctf::create_reader(elf_file_path,
> -				  debug_info_root_paths,
> -				  env);
> -
> -      if (!file_has_ctf_debug_info(elf_file_path,
> -				   debug_info_root_paths)
> -	    && file_has_dwarf_debug_info(elf_file_path,
> -					 debug_info_root_paths))
> -        result = dwarf::create_reader(elf_file_path,
> -				      debug_info_root_paths,
> -				      env,
> -				      show_all_types,
> -				      linux_kernel_mode);
> -    }
> -#endif
>   
>     return result;
>   }
> diff --git a/tools/abidiff.cc b/tools/abidiff.cc
> index 6cd948bf..5ffe47a3 100644
> --- a/tools/abidiff.cc
> +++ b/tools/abidiff.cc
> @@ -1197,15 +1197,15 @@ main(int argc, char* argv[])
>   	case abigail::tools_utils::FILE_TYPE_ELF: // fall through
>   	case abigail::tools_utils::FILE_TYPE_AR:
>   	  {
> +	    corpus::origin requested_fe_kind = corpus::DWARF_ORIGIN;
> +#ifdef WITH_CTF
> +	    if (opts.use_ctf)
> +	      requested_fe_kind = corpus::CTF_ORIGIN;
> +#endif
>   	    abigail::elf_based_reader_sptr rdr =
>   	      create_best_elf_based_reader(opts.file1,
>   					   opts.prepared_di_root_paths1,
> -					   env,
> -#ifdef WITH_CTF
> -					   opts.use_ctf,
> -#else
> -                                           false,
> -#endif
> +					   env, requested_fe_kind,
>   					   opts.show_all_types);
>               ABG_ASSERT(rdr);
>   
> @@ -1271,15 +1271,15 @@ main(int argc, char* argv[])
>   	case abigail::tools_utils::FILE_TYPE_ELF: // Fall through
>   	case abigail::tools_utils::FILE_TYPE_AR:
>   	  {
> +	    corpus::origin requested_fe_kind = corpus::DWARF_ORIGIN;
> +#ifdef WITH_CTF
> +	    if (opts.use_ctf)
> +	      requested_fe_kind = corpus::CTF_ORIGIN;
> +#endif
>               abigail::elf_based_reader_sptr rdr =
>   	      create_best_elf_based_reader(opts.file2,
>   					   opts.prepared_di_root_paths2,
> -					   env,
> -#ifdef WITH_CTF
> -					   opts.use_ctf,
> -#else
> -					   false,
> -#endif
> +					   env, requested_fe_kind,
>   					   opts.show_all_types);
>               ABG_ASSERT(rdr);
>   
> diff --git a/tools/abidw.cc b/tools/abidw.cc
> index d711751f..3b1a1bd5 100644
> --- a/tools/abidw.cc
> +++ b/tools/abidw.cc
> @@ -553,17 +553,18 @@ load_corpus_and_write_abixml(char* argv[],
>   
>     corpus_sptr corp;
>     fe_iface::status s = fe_iface::STATUS_UNKNOWN;
> +  corpus::origin requested_fe_kind = corpus::DWARF_ORIGIN;
> +#ifdef WITH_CTF
> +  if (opts.use_ctf)
> +    requested_fe_kind = corpus::CTF_ORIGIN;
> +#endif
> +
>     // First of all, create a reader to read the ABI from the file
>     // specfied in opts ...
>     abigail::elf_based_reader_sptr reader =
>       create_best_elf_based_reader(opts.in_file_path,
>   				 opts.prepared_di_root_paths,
> -				 env,
> -#ifdef WITH_CTF
> -				 opts.use_ctf,
> -#else
> -				 false,
> -#endif
> +				 env, requested_fe_kind,
>   				 opts.load_all_types,
>   				 opts.linux_kernel_mode);
>     ABG_ASSERT(reader);
> @@ -819,7 +820,7 @@ load_kernel_corpus_group_and_write_abixml(char* argv[],
>   
>     global_timer.start();
>     t.start();
> -  corpus::origin origin =
> +  corpus::origin requested_fe_kind =
>   #ifdef WITH_CTF
>       opts.use_ctf ? corpus::CTF_ORIGIN :
>   #endif
> @@ -830,7 +831,8 @@ load_kernel_corpus_group_and_write_abixml(char* argv[],
>   					      opts.vmlinux,
>   					      opts.suppression_paths,
>   					      opts.kabi_whitelist_paths,
> -					      supprs, opts.do_log, env, origin);
> +					      supprs, opts.do_log, env,
> +					      requested_fe_kind);
>     t.stop();
>   
>     if (opts.do_log)
> diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
> index ecdfb45f..1feb3d9e 100644
> --- a/tools/abipkgdiff.cc
> +++ b/tools/abipkgdiff.cc
> @@ -1323,15 +1323,15 @@ compare(const elf_file&		elf1,
>     abigail::elf_based_reader_sptr reader;
>     corpus_sptr corpus1;
>     {
> +    corpus::origin requested_fe_kind = corpus::DWARF_ORIGIN;
> +#ifdef WITH_CTF
> +    if (opts.use_ctf)
> +      requested_fe_kind = corpus::CTF_ORIGIN;
> +#endif
>       abigail::elf_based_reader_sptr reader =
>         create_best_elf_based_reader(elf1.path,
>   				   di_dirs1,
> -				   env,
> -#ifdef WITH_CTF
> -				   opts.use_ctf,
> -#else
> -				   false,
> -#endif
> +				   env, requested_fe_kind,
>   				   opts.show_all_types);
>       ABG_ASSERT(reader);
>   
> @@ -1423,15 +1423,15 @@ compare(const elf_file&		elf1,
>   
>     corpus_sptr corpus2;
>     {
> +    corpus::origin requested_fe_kind = corpus::DWARF_ORIGIN;
> +#ifdef WITH_CTF
> +    if (opts.use_ctf)
> +      requested_fe_kind = corpus::CTF_ORIGIN;
> +#endif
>       abigail::elf_based_reader_sptr reader =
>         create_best_elf_based_reader(elf2.path,
>   				   di_dirs2,
> -				   env,
> -#ifdef WITH_CTF
> -				   opts.use_ctf,
> -#else
> -				   false,
> -#endif
> +				   env, requested_fe_kind,
>   				   opts.show_all_types);
>       ABG_ASSERT(reader);
>   
> @@ -1589,15 +1589,15 @@ compare_to_self(const elf_file&		elf,
>     corpus_sptr corp;
>     abigail::elf_based_reader_sptr reader;
>     {
> +    corpus::origin requested_fe_kind = corpus::DWARF_ORIGIN;
> +#ifdef WITH_CTF
> +    if (opts.use_ctf)
> +      requested_fe_kind = corpus::CTF_ORIGIN;
> +#endif
>       abigail::elf_based_reader_sptr reader =
>         create_best_elf_based_reader(elf.path,
>   				   di_dirs,
> -				   env,
> -#ifdef WITH_CTF
> -				   opts.use_ctf,
> -#else
> -				   false,
> -#endif
> +				   env, requested_fe_kind,
>   				   opts.show_all_types);
>       ABG_ASSERT(reader);
>   
> diff --git a/tools/kmidiff.cc b/tools/kmidiff.cc
> index 391677ca..728392e3 100644
> --- a/tools/kmidiff.cc
> +++ b/tools/kmidiff.cc
> @@ -421,7 +421,7 @@ main(int argc, char* argv[])
>   
>     corpus_group_sptr group1, group2;
>     string debug_info_root_dir;
> -  corpus::origin origin =
> +  corpus::origin requested_fe_kind =
>   #ifdef WITH_CTF
>      opts.use_ctf ? corpus::CTF_ORIGIN :
>   #endif
> @@ -443,8 +443,8 @@ main(int argc, char* argv[])
>   						      opts.suppression_paths,
>   						      opts.kabi_whitelist_paths,
>   						      opts.read_time_supprs,
> -						      opts.verbose,
> -						      env, origin);
> +						      opts.verbose, env,
> +						      requested_fe_kind);
>   	  print_kernel_dist_binary_paths_under(opts.kernel_dist_root1, opts);
>   	}
>         else if (ftype == FILE_TYPE_XML_CORPUS_GROUP)
> @@ -469,8 +469,8 @@ main(int argc, char* argv[])
>   						      opts.suppression_paths,
>   						      opts.kabi_whitelist_paths,
>   						      opts.read_time_supprs,
> -						      opts.verbose,
> -						      env, origin);
> +						      opts.verbose, env,
> +						      requested_fe_kind);
>   	  print_kernel_dist_binary_paths_under(opts.kernel_dist_root2, opts);
>   	}
>         else if (ftype == FILE_TYPE_XML_CORPUS_GROUP)
> 
> --------------------------->8<------------------------------------------
> 
> 
> So, in the end, please find below the whole final patch that I have applied to the
> master branch.
> 
> Many thanks!
> 

Thanks to you for comments and review!


> Cheers,
> 
> [...]

Best regards,
guillermo

      reply	other threads:[~2022-11-28 21:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-01  0:15 [PATCH] CTF as a fallback when no DWARF debug info is present Guillermo E. Martinez
2022-10-04  9:04 ` Dodji Seketeli
2022-10-04 23:13   ` Guillermo E. Martinez
2022-10-06  7:42     ` Dodji Seketeli
2022-10-06 14:12       ` Dodji Seketeli
2022-10-07 14:13         ` Guillermo E. Martinez
2022-10-06 19:53       ` Guillermo Martinez
2022-10-06 19:50         ` Guillermo E. Martinez
2022-10-07 13:38         ` Dodji Seketeli
2022-10-07 16:04           ` Ben Woodard
2022-11-15 20:13 ` [PATCHv2] ELF based front-end readers fallback feature Guillermo E. Martinez
2022-11-21 18:51   ` [PATCHv3] " Guillermo E. Martinez
2022-11-22 14:19     ` Dodji Seketeli
2022-11-22 16:02       ` Guillermo E. Martinez
2022-11-22 16:00     ` [PATCH v4] " Guillermo E. Martinez
2022-11-28 15:56       ` Dodji Seketeli
2022-11-28 21:59         ` Guillermo E. Martinez [this message]

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=522c1be2-4ea0-aaa4-b512-f16b6de13e01@oracle.com \
    --to=guillermo.e.martinez@oracle.com \
    --cc=dodji@seketeli.org \
    --cc=libabigail@sourceware.org \
    /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).