<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 2, 2021 at 5:16 PM Chris Johns <<a href="mailto:chrisj@rtems.org" target="_blank">chrisj@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/3/21 7:01 am, Alex White wrote:<br>
> There were a couple of issues with the way the DWARF info was being<br>
> read. The first issue was that it inefficiently included all symbols,<br>
> even symbols that were not desired. The second issue is that it did<br>
> not handle inline functions correctly. These have been fixed.<br>
> ---<br>
> rtemstoolkit/rld-dwarf.cpp | 8 ++-<br>
> rtemstoolkit/rld-dwarf.h | 5 ++<br>
<br>
The RLD dwarf changes are fine. Should this be a separate patch to the covoar<br>
changes?<br></blockquote><div><br></div><div>That's what Gedare asked for when he suggested splitting the set into areas. </div><div><br></div><div>Alex can you separate out the RLD?</div><div><br></div><div>If Chris was ok with them all, just repost them as a self-contained set and</div><div>he can ack the cover-letter for the RLD set.</div><div><br></div><div>The set needs to be nibbled down somehow.</div><div><br></div><div>--joel</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Chris<br>
<br>
> tester/covoar/ExecutableInfo.cc | 30 ++++++++-<br>
> tester/covoar/covoar.cc | 110 ++++++++++++++++----------------<br>
> 4 files changed, 94 insertions(+), 59 deletions(-)<br>
> <br>
> diff --git a/rtemstoolkit/rld-dwarf.cpp b/rtemstoolkit/rld-dwarf.cpp<br>
> index d9ac6f3..acb4fd4 100644<br>
> --- a/rtemstoolkit/rld-dwarf.cpp<br>
> +++ b/rtemstoolkit/rld-dwarf.cpp<br>
> @@ -884,6 +884,12 @@ namespace rld<br>
> return addr;<br>
> }<br>
> <br>
> + bool<br>
> + function::has_entry_pc () const<br>
> + {<br>
> + return has_entry_pc_;<br>
> + }<br>
> +<br>
> bool<br>
> function::has_machine_code () const<br>
> {<br>
> @@ -1702,7 +1708,7 @@ namespace rld<br>
> if (daddr.is_an_end_sequence ())<br>
> seq_base = 0;<br>
> address addr (daddr, loc);<br>
> - if (loc >= pc_low_ && loc < pc_high_)<br>
> + if (loc >= pc_low_ && loc <= pc_high_)<br>
> {<br>
> pc = loc;<br>
> addr_lines_.push_back (addr);<br>
> diff --git a/rtemstoolkit/rld-dwarf.h b/rtemstoolkit/rld-dwarf.h<br>
> index 45fbab1..1210813 100644<br>
> --- a/rtemstoolkit/rld-dwarf.h<br>
> +++ b/rtemstoolkit/rld-dwarf.h<br>
> @@ -376,6 +376,11 @@ namespace rld<br>
> */<br>
> dwarf_unsigned pc_high () const;<br>
> <br>
> + /**<br>
> + * Does the function have an entry PC?<br>
> + */<br>
> + bool has_entry_pc () const;<br>
> +<br>
> /**<br>
> * Does the function have machine code in the image?<br>
> */<br>
<br>
<br>
> diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc<br>
> index c593e1d..c4257f0 100644<br>
> --- a/tester/covoar/ExecutableInfo.cc<br>
> +++ b/tester/covoar/ExecutableInfo.cc<br>
> @@ -45,10 +45,34 @@ namespace Coverage {<br>
> try {<br>
> for (auto& cu : debug.get_cus()) {<br>
> for (auto& func : cu.get_functions()) {<br>
> - if (func.has_machine_code() && (!func.is_inlined() || func.is_external())) {<br>
> - createCoverageMap (<a href="http://cu.name" rel="noreferrer" target="_blank">cu.name</a>(), <a href="http://func.name" rel="noreferrer" target="_blank">func.name</a>(),<br>
> - func.pc_low(), func.pc_high() - 1);<br>
> + if (!func.has_machine_code()) {<br>
> + continue;<br>
> }<br>
> +<br>
> + if (!SymbolsToAnalyze->isDesired(<a href="http://func.name" rel="noreferrer" target="_blank">func.name</a>())) {<br>
> + continue;<br>
> + }<br>
> +<br>
> + if (func.is_inlined()) {<br>
> + if (func.is_external()) {<br>
> + // Flag it<br>
> + std::cerr << "Function is both external and inlined: "<br>
> + << <a href="http://func.name" rel="noreferrer" target="_blank">func.name</a>() << std::endl;<br>
> + }<br>
> +<br>
> + if (func.has_entry_pc()) {<br>
> + continue;<br>
> + }<br>
> +<br>
> + // If the low PC address is zero, the symbol does not appear in<br>
> + // this executable.<br>
> + if (func.pc_low() == 0) {<br>
> + continue;<br>
> + }<br>
> + }<br>
> +<br>
> + createCoverageMap (<a href="http://cu.name" rel="noreferrer" target="_blank">cu.name</a>(), <a href="http://func.name" rel="noreferrer" target="_blank">func.name</a>(),<br>
> + func.pc_low(), func.pc_high() - 1);<br>
> }<br>
> }<br>
> } catch (...) {<br>
> diff --git a/tester/covoar/covoar.cc b/tester/covoar/covoar.cc<br>
> index cbb0e4f..84d883a 100644<br>
> --- a/tester/covoar/covoar.cc<br>
> +++ b/tester/covoar/covoar.cc<br>
> @@ -222,6 +222,61 @@ int covoar(<br>
> if ( !projectName )<br>
> throw option_error( "project name -p" );<br>
> <br>
> + //<br>
> + // Find the top of the BSP's build tree and if we have found the top<br>
> + // check the executable is under the same path and BSP.<br>
> + //<br>
> + std::string buildPath;<br>
> + std::string buildTarget;<br>
> + std::string buildBSP;<br>
> + createBuildPath(executablesToAnalyze,<br>
> + buildPath,<br>
> + buildTarget,<br>
> + buildBSP);<br>
> +<br>
> + //<br>
> + // Use a command line target if provided.<br>
> + //<br>
> + if (!target.empty()) {<br>
> + buildTarget = target;<br>
> + }<br>
> +<br>
> + if (Verbose) {<br>
> + if (singleExecutable) {<br>
> + std::cerr << "Processing a single executable and multiple coverage files"<br>
> + << std::endl;<br>
> + } else {<br>
> + std::cerr << "Processing multiple executable/coverage file pairs" << std::endl;<br>
> + }<br>
> + std::cerr << "Coverage Format : " << format << std::endl<br>
> + << "Target : " << buildTarget.c_str() << std::endl<br>
> + << std::endl;<br>
> +<br>
> + // Process each executable/coverage file pair.<br>
> + Executables::iterator eitr = executablesToAnalyze.begin();<br>
> + for (const auto& cname : coverageFileNames) {<br>
> + std::cerr << "Coverage file " << cname<br>
> + << " for executable: " << (*eitr)->getFileName() << std::endl;<br>
> + if (!singleExecutable)<br>
> + eitr++;<br>
> + }<br>
> + }<br>
> +<br>
> + //<br>
> + // Create data to support analysis.<br>
> + //<br>
> +<br>
> + // Create data based on target.<br>
> + TargetInfo = Target::TargetFactory( buildTarget );<br>
> +<br>
> + // Create the set of desired symbols.<br>
> + SymbolsToAnalyze = new Coverage::DesiredSymbols();<br>
> +<br>
> + //<br>
> + // Read symbol configuration file and load needed symbols.<br>
> + //<br>
> + SymbolsToAnalyze->load( symbolSet, buildTarget, buildBSP, Verbose );<br>
> +<br>
> // If a single executable was specified, process the remaining<br>
> // arguments as coverage file names.<br>
> if (singleExecutable) {<br>
> @@ -294,61 +349,6 @@ int covoar(<br>
> if (executablesToAnalyze.size() != coverageFileNames.size())<br>
> throw rld::error( "executables and coverage name size mismatch", "covoar" );<br>
> <br>
> - //<br>
> - // Find the top of the BSP's build tree and if we have found the top<br>
> - // check the executable is under the same path and BSP.<br>
> - //<br>
> - std::string buildPath;<br>
> - std::string buildTarget;<br>
> - std::string buildBSP;<br>
> - createBuildPath(executablesToAnalyze,<br>
> - buildPath,<br>
> - buildTarget,<br>
> - buildBSP);<br>
> -<br>
> - //<br>
> - // Use a command line target if provided.<br>
> - //<br>
> - if (!target.empty()) {<br>
> - buildTarget = target;<br>
> - }<br>
> -<br>
> - if (Verbose) {<br>
> - if (singleExecutable) {<br>
> - std::cerr << "Processing a single executable and multiple coverage files"<br>
> - << std::endl;<br>
> - } else {<br>
> - std::cerr << "Processing multiple executable/coverage file pairs" << std::endl;<br>
> - }<br>
> - std::cerr << "Coverage Format : " << format << std::endl<br>
> - << "Target : " << buildTarget.c_str() << std::endl<br>
> - << std::endl;<br>
> -<br>
> - // Process each executable/coverage file pair.<br>
> - Executables::iterator eitr = executablesToAnalyze.begin();<br>
> - for (const auto& cname : coverageFileNames) {<br>
> - std::cerr << "Coverage file " << cname<br>
> - << " for executable: " << (*eitr)->getFileName() << std::endl;<br>
> - if (!singleExecutable)<br>
> - eitr++;<br>
> - }<br>
> - }<br>
> -<br>
> - //<br>
> - // Create data to support analysis.<br>
> - //<br>
> -<br>
> - // Create data based on target.<br>
> - TargetInfo = Target::TargetFactory( buildTarget );<br>
> -<br>
> - // Create the set of desired symbols.<br>
> - SymbolsToAnalyze = new Coverage::DesiredSymbols();<br>
> -<br>
> - //<br>
> - // Read symbol configuration file and load needed symbols.<br>
> - //<br>
> - SymbolsToAnalyze->load( symbolSet, buildTarget, buildBSP, Verbose );<br>
> -<br>
> if ( Verbose )<br>
> std::cerr << "Analyzing " << SymbolsToAnalyze->set.size()<br>
> << " symbols" << std::endl;<br>
> <br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div>