[PATCH 09/22] covoar: Fix DWARF reading
Joel Sherrill
joel at rtems.org
Tue Mar 2 23:32:58 UTC 2021
On Tue, Mar 2, 2021 at 5:16 PM Chris Johns <chrisj at 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 at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210302/2b552353/attachment-0001.html>
More information about the devel
mailing list