On Tue, Mar 2, 2021 at 5:16 PM Chris Johns <chr...@rtems.org> wrote: > 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? >
That's what Gedare asked for when he suggested splitting the set into areas. Alex can you separate out the RLD? If Chris was ok with them all, just repost them as a self-contained set and he can ack the cover-letter for the RLD set. The set needs to be nibbled down somehow. --joel > 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; > > > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel