Re: [PATCH] abipkgdiff: Fix comparing CTF kABIs using Oracle Linux RPMs

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

Hello,

Any comment of this patch?

Thanks!,
guillermo

On Thu, Feb 16, 2023 at 06:57:43PM -0600, Guillermo E. Martinez wrote:
> Hello,
> 
> This patch is meant to fix how `abipkgdiff' using CTF origin is looking
> for dependencies to analyze kABIs. IMHO `file_is_kernel_package'
> function is relying on package name, so when `--ctf' is provided, it only
> will work with executables under uncompressed directories and it never
> will try to find `vmlinux.cfa' because it is just needed by KABIs. I
> think that an option to force handle a packages as kernel would be nice
> regardless the package name.
> 
> Please let me know your comments,
> Thanks in advanced!,
> guillermo
> --
> 
> Using abipkgdiff to analyze kABIs from OL packages with CTF debug format
> is not able to identify kernel packages, so the analysts is done as user
> space RPMs, `file_is_kernel_package' function uses package name to
> identify if the package contains kernel information, in OL distribution
> the those packages are named as: `kernel-uek-core-*', then a wrong flow
> is executed, additionally `vmlinux.ctfa' ship in a non debug package, so
> the expected location is not satisfied .
> 
> 	* src/abg-ctf-reader.cc (ctf::reader::find_ctfa_file): Use `find_file_under_dir'
> 	utility function to locate `vmlinux.ctfa' file.
> 	(ctf::reader::process_ctf_archive): Adjust dictionary name
> 	according to module name, removing characters after dot.
> 	* src/abg-tools-utils.cc (file_has_ctf_debug_info): Use `find_file_under_dir'
> 	utility function to locate `vmlinux.ctfa' file.
> 	(file_is_kernel_package): Adjust package name identifying a kernel package.
> 	(build_corpus_group_from_kernel_dist_under): Add `root' package directory
> 	to debug info array.
> 
> Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
> ---
>  src/abg-ctf-reader.cc  | 16 ++++++----------
>  src/abg-tools-utils.cc | 14 ++++++++++++--
>  2 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
> index 7159a578..ac0d3104 100644
> --- a/src/abg-ctf-reader.cc
> +++ b/src/abg-ctf-reader.cc
> @@ -336,12 +336,8 @@ public:
>      // for vmlinux.ctfa should be provided with --debug-info-dir
>      // option.
>      for (const auto& path : debug_info_root_paths())
> -      {
> -	ctfa_dirname = *path;
> -	ctfa_file = ctfa_dirname + "/vmlinux.ctfa";
> -	if (file_exists(ctfa_file))
> -	  return true;
> -      }
> +      if (tools_utils::find_file_under_dir(*path, "vmlinux.ctfa", ctfa_file))
> +        return true;
>  
>      return false;
>    }
> @@ -428,10 +424,10 @@ public:
>  	&& corpus_group())
>        {
>  	tools_utils::base_name(corpus_path(), dict_name);
> -
> -	if (dict_name != "vmlinux")
> -	  // remove .ko suffix
> -	  dict_name.erase(dict_name.length() - 3, 3);
> +	// remove .* suffix
> +	std::size_t pos = dict_name.find(".");
> +	if (pos != string::npos)
> +	  dict_name.erase(pos);
>  
>  	std::replace(dict_name.begin(), dict_name.end(), '-', '_');
>        }
> diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
> index 81f9aa75..d7f8b71f 100644
> --- a/src/abg-tools-utils.cc
> +++ b/src/abg-tools-utils.cc
> @@ -501,7 +501,7 @@ file_has_ctf_debug_info(const string& elf_file_path,
>  
>    // vmlinux.ctfa could be provided with --debug-info-dir
>    for (const auto& path : debug_info_root_paths)
> -    if (dir_contains_ctf_archive(*path, vmlinux))
> +    if (find_file_under_dir(*path, "vmlinux.ctfa", vmlinux))
>        return true;
>  
>    return false;
> @@ -1780,7 +1780,7 @@ file_is_kernel_package(const string& file_name, file_type file_type)
>      {
>        if (!get_rpm_name(file_name, package_name))
>  	return false;
> -      result = (package_name == "kernel");
> +      result = (string_begins_with(package_name, "kernel"));
>      }
>    else if (file_type == FILE_TYPE_DEB)
>      {
> @@ -2812,6 +2812,16 @@ build_corpus_group_from_kernel_dist_under(const string&	root,
>        vector<char**> di_roots;
>        di_roots.push_back(&di_root_ptr);
>  
> +#ifdef WITH_CTF
> +      shared_ptr<char> di_root_ctf;
> +      if (requested_fe_kind & corpus::CTF_ORIGIN)
> +        {
> +          di_root_ctf = make_path_absolute(root.c_str());
> +          char *di_root_ctf_ptr = di_root_ctf.get();
> +          di_roots.push_back(&di_root_ctf_ptr);
> +        }
> +#endif
> +
>        abigail::elf_based_reader_sptr reader =
>          create_best_elf_based_reader(vmlinux,
>                                       di_roots,
> -- 
> 2.39.1
>