<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 15, 2021 at 3:34 PM Alex White <<a href="mailto:alex.white@oarcorp.com">alex.white@oarcorp.com</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">I honestly can't remember why I changed 1024 to 20,000.<br>
<br>
I've looked back at that code and changed it back to 1024 without any issues. I think I might have missed that this is all happening in a loop, and at one point during a (long) debugging session I convinced myself that it wasn't reading all of the entries.<br>
<br>
At least that's the most rational explanation I can think up for that particular change. 😊<br>
<br>
If I revert ENTRIES from 20000 back to 1024, are we satisfied to leave the "entries" array as-is?<br>
<br></blockquote><div><br></div><div>I think that Chris' main points here are that, as you get covoar working again and cleaning it up, it should be made more C++ (and less C).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Alex<br>
<br>
-----Original Message-----<br>
From: Chris Johns <<a href="mailto:chrisj@rtems.org" target="_blank">chrisj@rtems.org</a>> <br>
Sent: Sunday, March 14, 2021 7:27 PM<br>
To: Alex White <<a href="mailto:alex.white@oarcorp.com" target="_blank">alex.white@oarcorp.com</a>>; <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
Subject: Re: [PATCH 1/2] covoar/CoverageReaderQEMU: Fix infinite loop<br>
<br>
On 12/3/21 5:30 am, Alex White wrote:<br>
> There was a potential that the branch info loop never terminated.<br>
> This has been fixed by adding a more reliable termination condition <br>
> and logging an error if it cannot find the branch target.<br>
> ---<br>
>  tester/covoar/CoverageReaderQEMU.cc | 11 +++++++++--<br>
>  1 file changed, 9 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/tester/covoar/CoverageReaderQEMU.cc <br>
> b/tester/covoar/CoverageReaderQEMU.cc<br>
> index 7c344e4..fb1709d 100644<br>
> --- a/tester/covoar/CoverageReaderQEMU.cc<br>
> +++ b/tester/covoar/CoverageReaderQEMU.cc<br>
> @@ -76,7 +76,7 @@ namespace Coverage {<br>
>      //<br>
>      // Read ENTRIES number of trace entries.<br>
>      //<br>
> -#define ENTRIES 1024<br>
> +#define ENTRIES 20000<br>
<br>
1024 sure, 20,000 ... hmmm ... I am not so sure. If you need more would is the change 200,000? Maybe a better solution exists.<br>
<br>
>      while (true) {<br>
>        CoverageMapBase     *aCoverageMap = NULL;<br>
>        struct trace_entry  entries[ENTRIES];<br>
<br>
Can an array or resized vector be used here?<br>
<br>
> @@ -118,8 +118,15 @@ namespace Coverage {<br>
>          // Determine if additional branch information is available.<br>
>          if ( (entry->op & branchInfo) != 0 ) {<br>
>            uint32_t  a = entry->pc + entry->size - 1;<br>
<br>
An aside ... more pointers being used.<br>
<br>
Chris<br>
<br>
> -            while (!aCoverageMap->isStartOfInstruction(a))<br>
> +            while (a > entry->pc && <br>
> + !aCoverageMap->isStartOfInstruction(a))<br>
>                a--;<br>
> +            if (a == entry->pc && !aCoverageMap->isStartOfInstruction(a)) {<br>
> +              // Something went wrong parsing the objdump.<br>
> +              std::ostringstream what;<br>
> +              what << "Reached beginning of range in " << file<br>
> +                << " at " << entry->pc << " with no start of instruction.";<br>
> +              throw rld::error( what, "CoverageReaderQEMU::processFile" );<br>
> +            }<br>
>              if (entry->op & taken) {<br>
>                aCoverageMap->setWasTaken( a );<br>
>              } else if (entry->op & notTaken) {<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></blockquote></div></div>