[PATCH 09/22] covoar: Fix DWARF reading

[Chris Johns]

On 2/3/21 7:01 am, Alex White wrote:
> There were a couple of issues with the way the DWARF info was being
> read. The first issue was that it inefficiently included all symbols,
> even symbols that were not desired. The second issue is that it did
> not handle inline functions correctly. These have been fixed.
> ---
>  rtemstoolkit/rld-dwarf.cpp      |   8 ++-
>  rtemstoolkit/rld-dwarf.h        |   5 ++

The RLD dwarf changes are fine. Should this be a separate patch to the covoar
changes?

Chris

>  tester/covoar/ExecutableInfo.cc |  30 ++++++++-
>  tester/covoar/covoar.cc         | 110 ++++++++++++++++----------------
>  4 files changed, 94 insertions(+), 59 deletions(-)
> 
> diff --git a/rtemstoolkit/rld-dwarf.cpp b/rtemstoolkit/rld-dwarf.cpp
> index d9ac6f3..acb4fd4 100644
> --- a/rtemstoolkit/rld-dwarf.cpp
> +++ b/rtemstoolkit/rld-dwarf.cpp
> @@ -884,6 +884,12 @@ namespace rld
>        return addr;
>      }
>  
> +    bool
> +    function::has_entry_pc () const
> +    {
> +      return has_entry_pc_;
> +    }
> +
>      bool
>      function::has_machine_code () const
>      {
> @@ -1702,7 +1708,7 @@ namespace rld
>          if (daddr.is_an_end_sequence ())
>            seq_base = 0;
>          address addr (daddr, loc);
> -        if (loc >= pc_low_ && loc < pc_high_)
> +        if (loc >= pc_low_ && loc <= pc_high_)
>          {
>            pc = loc;
>            addr_lines_.push_back (addr);
> diff --git a/rtemstoolkit/rld-dwarf.h b/rtemstoolkit/rld-dwarf.h
> index 45fbab1..1210813 100644
> --- a/rtemstoolkit/rld-dwarf.h
> +++ b/rtemstoolkit/rld-dwarf.h
> @@ -376,6 +376,11 @@ namespace rld
>         */
>        dwarf_unsigned pc_high () const;
>  
> +      /**
> +       * Does the function have an entry PC?
> +       */
> +      bool has_entry_pc () const;
> +
>        /**
>         * Does the function have machine code in the image?
>         */


> diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc
> index c593e1d..c4257f0 100644
> --- a/tester/covoar/ExecutableInfo.cc
> +++ b/tester/covoar/ExecutableInfo.cc
> @@ -45,10 +45,34 @@ namespace Coverage {
>      try {
>        for (auto& cu : debug.get_cus()) {
>          for (auto& func : cu.get_functions()) {
> -          if (func.has_machine_code() && (!func.is_inlined() || func.is_external())) {
> -            createCoverageMap (cu.name(), func.name(),
> -                               func.pc_low(), func.pc_high() - 1);
> +          if (!func.has_machine_code()) {
> +            continue;
>            }
> +
> +          if (!SymbolsToAnalyze->isDesired(func.name())) {
> +            continue;
> +          }
> +
> +          if (func.is_inlined()) {
> +            if (func.is_external()) {
> +              // Flag it
> +              std::cerr << "Function is both external and inlined: "
> +                        << func.name() << std::endl;
> +            }
> +
> +            if (func.has_entry_pc()) {
> +              continue;
> +            }
> +
> +            // If the low PC address is zero, the symbol does not appear in
> +            // this executable.
> +            if (func.pc_low() == 0) {
> +              continue;
> +            }
> +          }
> +
> +          createCoverageMap (cu.name(), func.name(),
> +                              func.pc_low(), func.pc_high() - 1);
>          }
>        }
>      } catch (...) {
> diff --git a/tester/covoar/covoar.cc b/tester/covoar/covoar.cc
> index cbb0e4f..84d883a 100644
> --- a/tester/covoar/covoar.cc
> +++ b/tester/covoar/covoar.cc
> @@ -222,6 +222,61 @@ int covoar(
>    if ( !projectName )
>      throw option_error( "project name -p" );
>  
> +  //
> +  // Find the top of the BSP's build tree and if we have found the top
> +  // check the executable is under the same path and BSP.
> +  //
> +  std::string buildPath;
> +  std::string buildTarget;
> +  std::string buildBSP;
> +  createBuildPath(executablesToAnalyze,
> +                  buildPath,
> +                  buildTarget,
> +                  buildBSP);
> +
> +  //
> +  // Use a command line target if provided.
> +  //
> +  if (!target.empty()) {
> +    buildTarget = target;
> +  }
> +
> +  if (Verbose) {
> +    if (singleExecutable) {
> +      std::cerr << "Processing a single executable and multiple coverage files"
> +                << std::endl;
> +    } else {
> +      std::cerr << "Processing multiple executable/coverage file pairs" << std::endl;
> +    }
> +    std::cerr << "Coverage Format : " << format << std::endl
> +              << "Target          : " << buildTarget.c_str() << std::endl
> +              << std::endl;
> +
> +    // Process each executable/coverage file pair.
> +    Executables::iterator eitr = executablesToAnalyze.begin();
> +    for (const auto& cname : coverageFileNames) {
> +      std::cerr << "Coverage file " << cname
> +                << " for executable: " << (*eitr)->getFileName() << std::endl;
> +      if (!singleExecutable)
> +        eitr++;
> +    }
> +  }
> +
> +  //
> +  // Create data to support analysis.
> +  //
> +
> +  // Create data based on target.
> +  TargetInfo = Target::TargetFactory( buildTarget );
> +
> +  // Create the set of desired symbols.
> +  SymbolsToAnalyze = new Coverage::DesiredSymbols();
> +
> +  //
> +  // Read symbol configuration file and load needed symbols.
> +  //
> +  SymbolsToAnalyze->load( symbolSet, buildTarget, buildBSP, Verbose );
> +
>    // If a single executable was specified, process the remaining
>    // arguments as coverage file names.
>    if (singleExecutable) {
> @@ -294,61 +349,6 @@ int covoar(
>    if (executablesToAnalyze.size() != coverageFileNames.size())
>      throw rld::error( "executables and coverage name size mismatch", "covoar" );
>  
> -  //
> -  // Find the top of the BSP's build tree and if we have found the top
> -  // check the executable is under the same path and BSP.
> -  //
> -  std::string buildPath;
> -  std::string buildTarget;
> -  std::string buildBSP;
> -  createBuildPath(executablesToAnalyze,
> -                  buildPath,
> -                  buildTarget,
> -                  buildBSP);
> -
> -  //
> -  // Use a command line target if provided.
> -  //
> -  if (!target.empty()) {
> -    buildTarget = target;
> -  }
> -
> -  if (Verbose) {
> -    if (singleExecutable) {
> -      std::cerr << "Processing a single executable and multiple coverage files"
> -                << std::endl;
> -    } else {
> -      std::cerr << "Processing multiple executable/coverage file pairs" << std::endl;
> -    }
> -    std::cerr << "Coverage Format : " << format << std::endl
> -              << "Target          : " << buildTarget.c_str() << std::endl
> -              << std::endl;
> -
> -    // Process each executable/coverage file pair.
> -    Executables::iterator eitr = executablesToAnalyze.begin();
> -    for (const auto& cname : coverageFileNames) {
> -      std::cerr << "Coverage file " << cname
> -                << " for executable: " << (*eitr)->getFileName() << std::endl;
> -      if (!singleExecutable)
> -        eitr++;
> -    }
> -  }
> -
> -  //
> -  // Create data to support analysis.
> -  //
> -
> -  // Create data based on target.
> -  TargetInfo = Target::TargetFactory( buildTarget );
> -
> -  // Create the set of desired symbols.
> -  SymbolsToAnalyze = new Coverage::DesiredSymbols();
> -
> -  //
> -  // Read symbol configuration file and load needed symbols.
> -  //
> -  SymbolsToAnalyze->load( symbolSet, buildTarget, buildBSP, Verbose );
> -
>    if ( Verbose )
>      std::cerr << "Analyzing " << SymbolsToAnalyze->set.size()
>                << " symbols" << std::endl;
>