[PATCH v3] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Vijay Kumar Banerjee
Add support in tester to run covoar and generate an html report to display
the summary of the coverage reports generated from covoar.

Co-authored-by : Cillian O'Donnell 
---
 .gitignore|   1 +
 tester/rt/coverage.py | 385 ++
 tester/rt/test.py |  34 ++-
 tester/rtems/testing/bsps/leon3-qemu-cov.ini  |   3 +-
 tester/rtems/testing/coverage/symbol-sets.ini |  36 +++
 tester/rtems/testing/qemu.cfg |   4 +-
 6 files changed, 450 insertions(+), 13 deletions(-)
 create mode 100644 tester/rt/coverage.py
 create mode 100644 tester/rtems/testing/coverage/symbol-sets.ini

diff --git a/.gitignore b/.gitignore
index 6ab3709..93cb5d2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,3 +9,4 @@ waf3-*
 .lock-waf*
 build
 VERSION*
+*symbols.ini
diff --git a/tester/rt/coverage.py b/tester/rt/coverage.py
new file mode 100644
index 000..54933d5
--- /dev/null
+++ b/tester/rt/coverage.py
@@ -0,0 +1,385 @@
+#
+# RTEMS Tools Project (http://www.rtems.org/)
+# Copyright 2014 Krzysztof Miesowicz (krzysztof.miesow...@gmail.com)
+# All rights reserved.
+#
+# This file is part of the RTEMS Tools package in 'rtems-tools'.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# 1. Redistributions of source code must retain the above copyright notice,
+# this list of conditions and the following disclaimer.
+#
+# 2. Redistributions in binary form must reproduce the above copyright notice,
+# this list of conditions and the following disclaimer in the documentation
+# and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 'AS IS'
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+#
+
+from rtemstoolkit import error
+from rtemstoolkit import path
+from rtemstoolkit import log
+from rtemstoolkit import execute
+from rtemstoolkit import macros
+
+from datetime import datetime
+
+from . import options
+
+import shutil
+import os
+
+try:
+import configparser
+except:
+import ConfigParser as configparser
+
+class summary:
+def __init__(self, p_summary_dir):
+self.summary_file_path = path.join(p_summary_dir, 'summary.txt')
+self.index_file_path = path.join(p_summary_dir, 'index.html')
+self.bytes_analyzed = 0
+self.bytes_not_executed = 0
+self.percentage_executed = 0.0
+self.percentage_not_executed = 100.0
+self.ranges_uncovered = 0
+self.branches_uncovered = 0
+self.branches_total = 0
+self.branches_always_taken = 0
+self.branches_never_taken = 0
+self.percentage_branches_covered = 0.0
+self.is_failure = False
+
+def parse(self):
+if(not path.exists(self.summary_file_path)):
+log.notice('summary file %s does not exist!' % 
(self.summary_file_path))
+self.is_failure = True
+return
+
+with open(self.summary_file_path,'r') as summary_file:
+   self.bytes_analyzed = self._get_next_with_colon(summary_file)
+   self.bytes_not_executed = self._get_next_with_colon(summary_file)
+   self.percentage_executed = self._get_next_with_colon(summary_file)
+   self.percentage_not_executed = 
self._get_next_with_colon(summary_file)
+   self.ranges_uncovered = self._get_next_with_colon(summary_file)
+   self.branches_total = self._get_next_with_colon(summary_file)
+   self.branches_uncovered = self._get_next_with_colon(summary_file)
+   self.branches_always_taken = 
self._get_next_without_colon(summary_file)
+   self.branches_never_taken = 
self._get_next_without_colon(summary_file)
+if len(self.branches_uncovered) > 0 and len(self.branches_total) > 0:
+self.percentage_branches_covered = \
+1 - (float(self.branches_uncovered) / float(self.branches_total))
+else:
+self.percentage_branches_covered = 0.0
+return
+
+def _get_next_with_colon(self, summary_file):
+line = summary_file.readline()
+if ':' in line:
+return line.split(':')[1].strip()
+else:
+return ''
+
+

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Cillian O'Donnell
On Mon, 4 Jun 2018, 21:20 Vijay Kumar Banerjee, 
wrote:

> On 5 June 2018 at 01:28, Cillian O'Donnell  wrote:
>
>>
>>
>> On 4 June 2018 at 20:29, Vijay Kumar Banerjee 
>> wrote:
>>
>>> On 5 June 2018 at 00:51, Joel Sherrill  wrote:
>>>


 On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee <
 vijaykumar9...@gmail.com> wrote:

> On 5 June 2018 at 00:31, Joel Sherrill  wrote:
>
>> I will add that covoar was originally designed to generate a report
>> on one
>> set at a time. The iteration over symbol sets was done at the
>> scripting
>> level above that.
>>
>> This had the advantage of being simple at the time. It may still be
>> simple
>> but moving the iteration over sets to covoar will probably be faster.
>>
>> I think DesiredSymbols is instanced a single time. As a starting
>> point, there
>> would have to be multiple instances of this -- one for each symbol
>> set.
>>
>> Beyond that, there is a correlation between the report generated and
>> the desired symbols set.
>>
>> So I am thinking that we need to define a "context" structure for each
>> set. One simple thought is that there is one "master/unified"
>> DesiredSymbol
>> set under the hood and the symbols in each set are used as a filter
>> when generating reports. So process the executables for every symbol
>> but generate the report subdirectories based on one of the sets of
>> DesiredSymbols.
>>
>> I think that should work.
>>
>> Cillian.. you have been through the flow. Am I thinking right that it
>> is
>>
> That sounds like it will work, I'll go back over the covoar code and
>> have a think about it. See if I can find any road blocks.
>>
>>
>>>
>> And I think we need to merge before doing this type of work. If we
>> can process
>> a single set correctly, that's a baseline. Adding iteration will be
>> easier to review
>> as another patch.
>>
>>
> we can process a single set correctly.
> Shall we proceed with merging the above patch with the suggested small
> edits
> and then file a ticket for the iteration and then start working on it ?
>

 Please.

>
> I am confused with running of coverage for multiple bsp. If a single
> report.html has to show the reports of multiple bsps (as hinted by
> Cillian)
>
 Actually I meant separate report.htmls, but if they're all outputed in
>> the the top level directory there'll have to be something internal to know
>> to search for the right output directory with all the coverage data, that's
>> all I meant. Nothing major.
>>
>>
>>> Then a lot of rewroking would be needed. If we're headed in that way
> then I think making separate report.html like leon3-report.html would
> be simpler
> to achieve, and then create a master index.html for listing all the
> report htmls.
>

 A single run of the tester will test a single BSP. If you open two
 shell windows
 and run the tester in both, will those collide?

 In it's current state. Yes, it will collide as all the names of files
>>> (including report.html)
>>> are constant. :(
>>>
>> This is the main fix for now just making adding unique identifiers for
>> files and dirs that will be called in the process. Focus on that, then we
>> can try and merge and the covoar fix for multi sets can come later.
>>
> I have added the update .
> after the update the directory is BSP-coverage ( example :-
> leon3-qemu-coverage)
> they symbol file created is leon3-qemu-symbols.ini
> I have made the necessary changes and leon3-qemu-report.html is showing
> proper results.
>

Perfect! Post patch V3 then and we'll see about this merge.

> covoar does not need support internally to process multiple BSPs
 concurrently.

 It is common practice to build and test multiple BSPs in parallel to
 take advantage
 of machines with many cores.

 But if you aren't careful, you can't even have two build/test trees at
 the same time
 if they collide on file names in shared directories.

 Does that make sense?


>> On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell <
>> cpodonne...@gmail.com> wrote:
>>
>>>
>>>
>>> On 4 June 2018 at 19:03, Vijay Kumar Banerjee <
>>> vijaykumar9...@gmail.com> wrote:
>>>
 On 2 June 2018 at 01:00, Vijay Kumar Banerjee <
 vijaykumar9...@gmail.com> wrote:

> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>
>>
>>
>> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>>>
 On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
  wrote:
 > On 1 June 2018 at 19:24, Joel Sherrill 
 wrote:

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Vijay Kumar Banerjee
On 5 June 2018 at 01:28, Cillian O'Donnell  wrote:

>
>
> On 4 June 2018 at 20:29, Vijay Kumar Banerjee 
> wrote:
>
>> On 5 June 2018 at 00:51, Joel Sherrill  wrote:
>>
>>>
>>>
>>> On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee <
>>> vijaykumar9...@gmail.com> wrote:
>>>
 On 5 June 2018 at 00:31, Joel Sherrill  wrote:

> I will add that covoar was originally designed to generate a report on
> one
> set at a time. The iteration over symbol sets was done at the scripting
> level above that.
>
> This had the advantage of being simple at the time. It may still be
> simple
> but moving the iteration over sets to covoar will probably be faster.
>
> I think DesiredSymbols is instanced a single time. As a starting
> point, there
> would have to be multiple instances of this -- one for each symbol set.
>
> Beyond that, there is a correlation between the report generated and
> the desired symbols set.
>
> So I am thinking that we need to define a "context" structure for each
> set. One simple thought is that there is one "master/unified"
> DesiredSymbol
> set under the hood and the symbols in each set are used as a filter
> when generating reports. So process the executables for every symbol
> but generate the report subdirectories based on one of the sets of
> DesiredSymbols.
>
> I think that should work.
>
> Cillian.. you have been through the flow. Am I thinking right that it
> is
>
 That sounds like it will work, I'll go back over the covoar code and
> have a think about it. See if I can find any road blocks.
>
>
>>
> And I think we need to merge before doing this type of work. If we can
> process
> a single set correctly, that's a baseline. Adding iteration will be
> easier to review
> as another patch.
>
>
 we can process a single set correctly.
 Shall we proceed with merging the above patch with the suggested small
 edits
 and then file a ticket for the iteration and then start working on it ?

>>>
>>> Please.
>>>

 I am confused with running of coverage for multiple bsp. If a single
 report.html has to show the reports of multiple bsps (as hinted by
 Cillian)

>>> Actually I meant separate report.htmls, but if they're all outputed in
> the the top level directory there'll have to be something internal to know
> to search for the right output directory with all the coverage data, that's
> all I meant. Nothing major.
>
>
>> Then a lot of rewroking would be needed. If we're headed in that way
 then I think making separate report.html like leon3-report.html would
 be simpler
 to achieve, and then create a master index.html for listing all the
 report htmls.

>>>
>>> A single run of the tester will test a single BSP. If you open two shell
>>> windows
>>> and run the tester in both, will those collide?
>>>
>>> In it's current state. Yes, it will collide as all the names of files
>> (including report.html)
>> are constant. :(
>>
> This is the main fix for now just making adding unique identifiers for
> files and dirs that will be called in the process. Focus on that, then we
> can try and merge and the covoar fix for multi sets can come later.
>
I have added the update .
after the update the directory is BSP-coverage ( example :-
leon3-qemu-coverage)
they symbol file created is leon3-qemu-symbols.ini
I have made the necessary changes and leon3-qemu-report.html is showing
proper results.

> covoar does not need support internally to process multiple BSPs
>>> concurrently.
>>>
>>> It is common practice to build and test multiple BSPs in parallel to
>>> take advantage
>>> of machines with many cores.
>>>
>>> But if you aren't careful, you can't even have two build/test trees at
>>> the same time
>>> if they collide on file names in shared directories.
>>>
>>> Does that make sense?
>>>
>>>
> On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell <
> cpodonne...@gmail.com> wrote:
>
>>
>>
>> On 4 June 2018 at 19:03, Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 2 June 2018 at 01:00, Vijay Kumar Banerjee <
>>> vijaykumar9...@gmail.com> wrote:
>>>
 On 2 June 2018 at 00:48, Joel Sherrill  wrote:

>
>
> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
> vijaykumar9...@gmail.com> wrote:
>
>> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>>
>>> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>>>  wrote:
>>> > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
>>> >>
>>> >>
>>> >>
>>> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
>>> >>  wrote:
>>> >>>
>>> >>> Here's the list of Ideas for improvements:
>>> >>>
>>> >>> 1. Include the section coverage in the bsp config 

Re: buffer overrun in rtems_rfs_bitmap_create_search()

2018-06-04 Thread Joel Sherrill
On Mon, Jun 4, 2018 at 2:27 PM, Walter Lee  wrote:

> Hi Gedare.  Thanks for the response.  I am using a snapshot of RTEMS
> provided by a third party, based on commit #821acce on master.  The
> bug should still be there on the tip of master and on 4.11 (and
> probably 4.10 also, but that version seems to be missing another
> patch).
>

Any idea which patch or ticket that was? I am curious whether it was
a bug or improvement and there are two patches to apply to 4.11.


>
> I've updated the patch to master, and also added a test.
>

Thank you!  Helps long term to make sure we don't get a regression.

--joel


>
> Thanks,
>
> Walter
> On Mon, Jun 4, 2018 at 9:55 AM Gedare Bloom  wrote:
> >
> > Hello Walter,
> >
> > Thank you for the bug report and patch. The patch is outdated, what
> > version of RTEMS are you using? I think the problem also affects the
> > master branch, but we need a ticket for each affected open branch.
> >
> > The fix looks OK to me, but I'd like Chris Johns to approve it. I
> > assigned the ticket to him.
> >
> > Gedare
> >
> > On Wed, May 30, 2018 at 1:24 PM, Walter Lee  wrote:
> > > Hi.  I am encountering a buffer overrun in
> > > rtems_rfs_bitmap_create_search().  It seems that whenever the bitmap
> > > uses the last bit of its search_map (i.e. (control->size + 31) % 32 ==
> > > 32)), the loop will write to the word one beyond the end of
> > > search_map.
> > >
> > > I filed a bug at https://devel.rtems.org/ticket/3439, with a patch
> > > that fixes the problem.
> > >
> > > Please let me know if I'm missing something, and if not what I need to
> > > do to get this fixed.
> > >
> > > Thanks,
> > >
> > > Walter
> > > ___
> > > 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
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Cillian O'Donnell
On 4 June 2018 at 20:29, Vijay Kumar Banerjee 
wrote:

> On 5 June 2018 at 00:51, Joel Sherrill  wrote:
>
>>
>>
>> On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 5 June 2018 at 00:31, Joel Sherrill  wrote:
>>>
 I will add that covoar was originally designed to generate a report on
 one
 set at a time. The iteration over symbol sets was done at the scripting
 level above that.

 This had the advantage of being simple at the time. It may still be
 simple
 but moving the iteration over sets to covoar will probably be faster.

 I think DesiredSymbols is instanced a single time. As a starting point,
 there
 would have to be multiple instances of this -- one for each symbol set.

 Beyond that, there is a correlation between the report generated and
 the desired symbols set.

 So I am thinking that we need to define a "context" structure for each
 set. One simple thought is that there is one "master/unified"
 DesiredSymbol
 set under the hood and the symbols in each set are used as a filter
 when generating reports. So process the executables for every symbol
 but generate the report subdirectories based on one of the sets of
 DesiredSymbols.

 I think that should work.

 Cillian.. you have been through the flow. Am I thinking right that it
 is

>>> That sounds like it will work, I'll go back over the covoar code and
have a think about it. See if I can find any road blocks.


>
 And I think we need to merge before doing this type of work. If we can
 process
 a single set correctly, that's a baseline. Adding iteration will be
 easier to review
 as another patch.


>>> we can process a single set correctly.
>>> Shall we proceed with merging the above patch with the suggested small
>>> edits
>>> and then file a ticket for the iteration and then start working on it ?
>>>
>>
>> Please.
>>
>>>
>>> I am confused with running of coverage for multiple bsp. If a single
>>> report.html has to show the reports of multiple bsps (as hinted by
>>> Cillian)
>>>
>> Actually I meant separate report.htmls, but if they're all outputed in
the the top level directory there'll have to be something internal to know
to search for the right output directory with all the coverage data, that's
all I meant. Nothing major.


> Then a lot of rewroking would be needed. If we're headed in that way
>>> then I think making separate report.html like leon3-report.html would be
>>> simpler
>>> to achieve, and then create a master index.html for listing all the
>>> report htmls.
>>>
>>
>> A single run of the tester will test a single BSP. If you open two shell
>> windows
>> and run the tester in both, will those collide?
>>
>> In it's current state. Yes, it will collide as all the names of files
> (including report.html)
> are constant. :(
>
This is the main fix for now just making adding unique identifiers for
files and dirs that will be called in the process. Focus on that, then we
can try and merge and the covoar fix for multi sets can come later.

> covoar does not need support internally to process multiple BSPs
>> concurrently.
>>
>> It is common practice to build and test multiple BSPs in parallel to take
>> advantage
>> of machines with many cores.
>>
>> But if you aren't careful, you can't even have two build/test trees at
>> the same time
>> if they collide on file names in shared directories.
>>
>> Does that make sense?
>>
>>
 On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell <
 cpodonne...@gmail.com> wrote:

>
>
> On 4 June 2018 at 19:03, Vijay Kumar Banerjee <
> vijaykumar9...@gmail.com> wrote:
>
>> On 2 June 2018 at 01:00, Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>>>


 On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
 vijaykumar9...@gmail.com> wrote:

> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>
>> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>>  wrote:
>> > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
>> >>
>> >>
>> >>
>> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
>> >>  wrote:
>> >>>
>> >>> Here's the list of Ideas for improvements:
>> >>>
>> >>> 1. Include the section coverage in the bsp config file.
>> >>> If the section is not found then the script will show
>> >>> proper error showing coverage is not supported for the
>> >>> provided bsp config file.
>> >>>
>> >>> 2. Update covoar to add support for separate coverage report
>> >>> for each symbol set.
>> >>>
>> >>> 3. Add a method somewhere in covoar to get the size of an
>> instruction

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Joel Sherrill
On Mon, Jun 4, 2018 at 2:29 PM, Vijay Kumar Banerjee <
vijaykumar9...@gmail.com> wrote:

> On 5 June 2018 at 00:51, Joel Sherrill  wrote:
>
>>
>>
>> On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 5 June 2018 at 00:31, Joel Sherrill  wrote:
>>>
 I will add that covoar was originally designed to generate a report on
 one
 set at a time. The iteration over symbol sets was done at the scripting
 level above that.

 This had the advantage of being simple at the time. It may still be
 simple
 but moving the iteration over sets to covoar will probably be faster.

 I think DesiredSymbols is instanced a single time. As a starting point,
 there
 would have to be multiple instances of this -- one for each symbol set.

 Beyond that, there is a correlation between the report generated and
 the desired symbols set.

 So I am thinking that we need to define a "context" structure for each
 set. One simple thought is that there is one "master/unified"
 DesiredSymbol
 set under the hood and the symbols in each set are used as a filter
 when generating reports. So process the executables for every symbol
 but generate the report subdirectories based on one of the sets of
 DesiredSymbols.

 I think that should work.

 Cillian.. you have been through the flow. Am I thinking right that it
 is

 And I think we need to merge before doing this type of work. If we can
 process
 a single set correctly, that's a baseline. Adding iteration will be
 easier to review
 as another patch.


>>> we can process a single set correctly.
>>> Shall we proceed with merging the above patch with the suggested small
>>> edits
>>> and then file a ticket for the iteration and then start working on it ?
>>>
>>
>> Please.
>>
>>>
>>> I am confused with running of coverage for multiple bsp. If a single
>>> report.html has to show the reports of multiple bsps (as hinted by
>>> Cillian)
>>> Then a lot of rewroking would be needed. If we're headed in that way
>>> then I think making separate report.html like leon3-report.html would be
>>> simpler
>>> to achieve, and then create a master index.html for listing all the
>>> report htmls.
>>>
>>
>> A single run of the tester will test a single BSP. If you open two shell
>> windows
>> and run the tester in both, will those collide?
>>
>> In it's current state. Yes, it will collide as all the names of files
> (including report.html)
> are constant. :(
>

But are these all under a directory which includes the BSP? The contents of
each BSP/build specific directory can be identical if the top directory
name is
different.


> covoar does not need support internally to process multiple BSPs
>> concurrently.
>>
>> It is common practice to build and test multiple BSPs in parallel to take
>> advantage
>> of machines with many cores.
>>
>> But if you aren't careful, you can't even have two build/test trees at
>> the same time
>> if they collide on file names in shared directories.
>>
>> Does that make sense?
>>
>>
 On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell <
 cpodonne...@gmail.com> wrote:

>
>
> On 4 June 2018 at 19:03, Vijay Kumar Banerjee <
> vijaykumar9...@gmail.com> wrote:
>
>> On 2 June 2018 at 01:00, Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>>>


 On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
 vijaykumar9...@gmail.com> wrote:

> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>
>> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>>  wrote:
>> > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
>> >>
>> >>
>> >>
>> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
>> >>  wrote:
>> >>>
>> >>> Here's the list of Ideas for improvements:
>> >>>
>> >>> 1. Include the section coverage in the bsp config file.
>> >>> If the section is not found then the script will show
>> >>> proper error showing coverage is not supported for the
>> >>> provided bsp config file.
>> >>>
>> >>> 2. Update covoar to add support for separate coverage report
>> >>> for each symbol set.
>> >>>
>> >>> 3. Add a method somewhere in covoar to get the size of an
>> instruction
>> >>> and fix the hard coded size 4 in ObjdumpProcessor.cc
>> >>
>> >>
>> >> What about a single XXX_run command? What about that
>> suggestion?
>> >>
>> > The suggestion was to turn test_run and coverage_run into a
>> single command.
>> > I have kept them separate so that there's a possibility to just
>> run the
>> 

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Vijay Kumar Banerjee
On 5 June 2018 at 00:51, Joel Sherrill  wrote:

>
>
> On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee <
> vijaykumar9...@gmail.com> wrote:
>
>> On 5 June 2018 at 00:31, Joel Sherrill  wrote:
>>
>>> I will add that covoar was originally designed to generate a report on
>>> one
>>> set at a time. The iteration over symbol sets was done at the scripting
>>> level above that.
>>>
>>> This had the advantage of being simple at the time. It may still be
>>> simple
>>> but moving the iteration over sets to covoar will probably be faster.
>>>
>>> I think DesiredSymbols is instanced a single time. As a starting point,
>>> there
>>> would have to be multiple instances of this -- one for each symbol set.
>>>
>>> Beyond that, there is a correlation between the report generated and
>>> the desired symbols set.
>>>
>>> So I am thinking that we need to define a "context" structure for each
>>> set. One simple thought is that there is one "master/unified"
>>> DesiredSymbol
>>> set under the hood and the symbols in each set are used as a filter
>>> when generating reports. So process the executables for every symbol
>>> but generate the report subdirectories based on one of the sets of
>>> DesiredSymbols.
>>>
>>> I think that should work.
>>>
>>> Cillian.. you have been through the flow. Am I thinking right that it is
>>>
>>> And I think we need to merge before doing this type of work. If we can
>>> process
>>> a single set correctly, that's a baseline. Adding iteration will be
>>> easier to review
>>> as another patch.
>>>
>>>
>> we can process a single set correctly.
>> Shall we proceed with merging the above patch with the suggested small
>> edits
>> and then file a ticket for the iteration and then start working on it ?
>>
>
> Please.
>
>>
>> I am confused with running of coverage for multiple bsp. If a single
>> report.html has to show the reports of multiple bsps (as hinted by
>> Cillian)
>> Then a lot of rewroking would be needed. If we're headed in that way
>> then I think making separate report.html like leon3-report.html would be
>> simpler
>> to achieve, and then create a master index.html for listing all the
>> report htmls.
>>
>
> A single run of the tester will test a single BSP. If you open two shell
> windows
> and run the tester in both, will those collide?
>
> In it's current state. Yes, it will collide as all the names of files
(including report.html)
are constant. :(

> covoar does not need support internally to process multiple BSPs
> concurrently.
>
> It is common practice to build and test multiple BSPs in parallel to take
> advantage
> of machines with many cores.
>
> But if you aren't careful, you can't even have two build/test trees at the
> same time
> if they collide on file names in shared directories.
>
> Does that make sense?
>
>
>>> On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell >> > wrote:
>>>


 On 4 June 2018 at 19:03, Vijay Kumar Banerjee >>> > wrote:

> On 2 June 2018 at 01:00, Vijay Kumar Banerjee <
> vijaykumar9...@gmail.com> wrote:
>
>> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>>
>>>
>>>
>>> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
>>> vijaykumar9...@gmail.com> wrote:
>>>
 On 1 June 2018 at 20:30, Gedare Bloom  wrote:

> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>  wrote:
> > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
> >>
> >>
> >>
> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
> >>  wrote:
> >>>
> >>> Here's the list of Ideas for improvements:
> >>>
> >>> 1. Include the section coverage in the bsp config file.
> >>> If the section is not found then the script will show
> >>> proper error showing coverage is not supported for the
> >>> provided bsp config file.
> >>>
> >>> 2. Update covoar to add support for separate coverage report
> >>> for each symbol set.
> >>>
> >>> 3. Add a method somewhere in covoar to get the size of an
> instruction
> >>> and fix the hard coded size 4 in ObjdumpProcessor.cc
> >>
> >>
> >> What about a single XXX_run command? What about that suggestion?
> >>
> > The suggestion was to turn test_run and coverage_run into a
> single command.
> > I have kept them separate so that there's a possibility to just
> run the
> > test.
> >
> > If we want to run coverage everytime we run the test. we can do
> it.
> > Then I think the --coverage option can be changed to
> --coverage-sets
> > to mention the sets.
> > If that's what we're looking for then I don't think a separate
> ticket is
> > needed,
> > I can try to do it within tomorrow and submit an updated patch.
> >
> >>
> >> 

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Joel Sherrill
On Mon, Jun 4, 2018 at 2:12 PM, Vijay Kumar Banerjee <
vijaykumar9...@gmail.com> wrote:

> On 5 June 2018 at 00:31, Joel Sherrill  wrote:
>
>> I will add that covoar was originally designed to generate a report on one
>> set at a time. The iteration over symbol sets was done at the scripting
>> level above that.
>>
>> This had the advantage of being simple at the time. It may still be simple
>> but moving the iteration over sets to covoar will probably be faster.
>>
>> I think DesiredSymbols is instanced a single time. As a starting point,
>> there
>> would have to be multiple instances of this -- one for each symbol set.
>>
>> Beyond that, there is a correlation between the report generated and
>> the desired symbols set.
>>
>> So I am thinking that we need to define a "context" structure for each
>> set. One simple thought is that there is one "master/unified"
>> DesiredSymbol
>> set under the hood and the symbols in each set are used as a filter
>> when generating reports. So process the executables for every symbol
>> but generate the report subdirectories based on one of the sets of
>> DesiredSymbols.
>>
>> I think that should work.
>>
>> Cillian.. you have been through the flow. Am I thinking right that it is
>>
>> And I think we need to merge before doing this type of work. If we can
>> process
>> a single set correctly, that's a baseline. Adding iteration will be
>> easier to review
>> as another patch.
>>
>>
> we can process a single set correctly.
> Shall we proceed with merging the above patch with the suggested small
> edits
> and then file a ticket for the iteration and then start working on it ?
>

Please.

>
> I am confused with running of coverage for multiple bsp. If a single
> report.html has to show the reports of multiple bsps (as hinted by Cillian)
> Then a lot of rewroking would be needed. If we're headed in that way
> then I think making separate report.html like leon3-report.html would be
> simpler
> to achieve, and then create a master index.html for listing all the report
> htmls.
>

A single run of the tester will test a single BSP. If you open two shell
windows
and run the tester in both, will those collide?

covoar does not need support internally to process multiple BSPs
concurrently.

It is common practice to build and test multiple BSPs in parallel to take
advantage
of machines with many cores.

But if you aren't careful, you can't even have two build/test trees at the
same time
if they collide on file names in shared directories.

Does that make sense?


>> On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell 
>> wrote:
>>
>>>
>>>
>>> On 4 June 2018 at 19:03, Vijay Kumar Banerjee 
>>> wrote:
>>>
 On 2 June 2018 at 01:00, Vijay Kumar Banerjee >>> > wrote:

> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>
>>
>>
>> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>>>
 On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
  wrote:
 > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
 >>
 >>
 >>
 >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
 >>  wrote:
 >>>
 >>> Here's the list of Ideas for improvements:
 >>>
 >>> 1. Include the section coverage in the bsp config file.
 >>> If the section is not found then the script will show
 >>> proper error showing coverage is not supported for the
 >>> provided bsp config file.
 >>>
 >>> 2. Update covoar to add support for separate coverage report
 >>> for each symbol set.
 >>>
 >>> 3. Add a method somewhere in covoar to get the size of an
 instruction
 >>> and fix the hard coded size 4 in ObjdumpProcessor.cc
 >>
 >>
 >> What about a single XXX_run command? What about that suggestion?
 >>
 > The suggestion was to turn test_run and coverage_run into a
 single command.
 > I have kept them separate so that there's a possibility to just
 run the
 > test.
 >
 > If we want to run coverage everytime we run the test. we can do
 it.
 > Then I think the --coverage option can be changed to
 --coverage-sets
 > to mention the sets.
 > If that's what we're looking for then I don't think a separate
 ticket is
 > needed,
 > I can try to do it within tomorrow and submit an updated patch.
 >
 >>
 >> Will there be an update to this patch?
 >>
 > This patch is working an showing results. I don't have any work
 > going related to this patch currently.
 > If there are any suggestions, I'll try to include all the
 suggested updates
 > as soon as possible and submit. So that we can get it merged.
 >

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Vijay Kumar Banerjee
On 5 June 2018 at 00:31, Joel Sherrill  wrote:

> I will add that covoar was originally designed to generate a report on one
> set at a time. The iteration over symbol sets was done at the scripting
> level above that.
>
> This had the advantage of being simple at the time. It may still be simple
> but moving the iteration over sets to covoar will probably be faster.
>
> I think DesiredSymbols is instanced a single time. As a starting point,
> there
> would have to be multiple instances of this -- one for each symbol set.
>
> Beyond that, there is a correlation between the report generated and
> the desired symbols set.
>
> So I am thinking that we need to define a "context" structure for each
> set. One simple thought is that there is one "master/unified" DesiredSymbol
> set under the hood and the symbols in each set are used as a filter
> when generating reports. So process the executables for every symbol
> but generate the report subdirectories based on one of the sets of
> DesiredSymbols.
>
> I think that should work.
>
> Cillian.. you have been through the flow. Am I thinking right that it is
>
> And I think we need to merge before doing this type of work. If we can
> process
> a single set correctly, that's a baseline. Adding iteration will be easier
> to review
> as another patch.
>
>
we can process a single set correctly.
Shall we proceed with merging the above patch with the suggested small edits
and then file a ticket for the iteration and then start working on it ?

I am confused with running of coverage for multiple bsp. If a single
report.html has to show the reports of multiple bsps (as hinted by Cillian)
Then a lot of rewroking would be needed. If we're headed in that way
then I think making separate report.html like leon3-report.html would be
simpler
to achieve, and then create a master index.html for listing all the report
htmls.

>
> On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell 
> wrote:
>
>>
>>
>> On 4 June 2018 at 19:03, Vijay Kumar Banerjee 
>> wrote:
>>
>>> On 2 June 2018 at 01:00, Vijay Kumar Banerjee 
>>> wrote:
>>>
 On 2 June 2018 at 00:48, Joel Sherrill  wrote:

>
>
> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
> vijaykumar9...@gmail.com> wrote:
>
>> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>>
>>> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>>>  wrote:
>>> > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
>>> >>
>>> >>
>>> >>
>>> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
>>> >>  wrote:
>>> >>>
>>> >>> Here's the list of Ideas for improvements:
>>> >>>
>>> >>> 1. Include the section coverage in the bsp config file.
>>> >>> If the section is not found then the script will show
>>> >>> proper error showing coverage is not supported for the
>>> >>> provided bsp config file.
>>> >>>
>>> >>> 2. Update covoar to add support for separate coverage report
>>> >>> for each symbol set.
>>> >>>
>>> >>> 3. Add a method somewhere in covoar to get the size of an
>>> instruction
>>> >>> and fix the hard coded size 4 in ObjdumpProcessor.cc
>>> >>
>>> >>
>>> >> What about a single XXX_run command? What about that suggestion?
>>> >>
>>> > The suggestion was to turn test_run and coverage_run into a single
>>> command.
>>> > I have kept them separate so that there's a possibility to just
>>> run the
>>> > test.
>>> >
>>> > If we want to run coverage everytime we run the test. we can do it.
>>> > Then I think the --coverage option can be changed to
>>> --coverage-sets
>>> > to mention the sets.
>>> > If that's what we're looking for then I don't think a separate
>>> ticket is
>>> > needed,
>>> > I can try to do it within tomorrow and submit an updated patch.
>>> >
>>> >>
>>> >> Will there be an update to this patch?
>>> >>
>>> > This patch is working an showing results. I don't have any work
>>> > going related to this patch currently.
>>> > If there are any suggestions, I'll try to include all the
>>> suggested updates
>>> > as soon as possible and submit. So that we can get it merged.
>>> >
>>>
>>> I get confused by the similarity between test_run() and
>>> coverage_run()
>>> names, and now I'm also seeing some confusion because there is a
>>> function coverage_run() and a class coverage_run. I suggest you
>>> remove
>>> this function coverage_run(), and make either coverage_run.__init__()
>>> or coverage_run.run() take the executables as a parameter directly.
>>>
>>> Thank you for the suggestion. :)
>> I have removed the function and taken executables as a parameter in
>> coverage_run.__init__()
>>
>> I have a question.
>> The ini file that is fed to covoar is written by the script according
>> to the
>> symbols mentioned by the user. I 

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Joel Sherrill
I will add that covoar was originally designed to generate a report on one
set at a time. The iteration over symbol sets was done at the scripting
level above that.

This had the advantage of being simple at the time. It may still be simple
but moving the iteration over sets to covoar will probably be faster.

I think DesiredSymbols is instanced a single time. As a starting point,
there
would have to be multiple instances of this -- one for each symbol set.

Beyond that, there is a correlation between the report generated and
the desired symbols set.

So I am thinking that we need to define a "context" structure for each
set. One simple thought is that there is one "master/unified" DesiredSymbol
set under the hood and the symbols in each set are used as a filter
when generating reports. So process the executables for every symbol
but generate the report subdirectories based on one of the sets of
DesiredSymbols.

I think that should work.

Cillian.. you have been through the flow. Am I thinking right that it is

And I think we need to merge before doing this type of work. If we can
process
a single set correctly, that's a baseline. Adding iteration will be easier
to review
as another patch.


On Mon, Jun 4, 2018 at 1:43 PM, Cillian O'Donnell 
wrote:

>
>
> On 4 June 2018 at 19:03, Vijay Kumar Banerjee 
> wrote:
>
>> On 2 June 2018 at 01:00, Vijay Kumar Banerjee 
>> wrote:
>>
>>> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>>>


 On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
 vijaykumar9...@gmail.com> wrote:

> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>
>> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>>  wrote:
>> > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
>> >>
>> >>
>> >>
>> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
>> >>  wrote:
>> >>>
>> >>> Here's the list of Ideas for improvements:
>> >>>
>> >>> 1. Include the section coverage in the bsp config file.
>> >>> If the section is not found then the script will show
>> >>> proper error showing coverage is not supported for the
>> >>> provided bsp config file.
>> >>>
>> >>> 2. Update covoar to add support for separate coverage report
>> >>> for each symbol set.
>> >>>
>> >>> 3. Add a method somewhere in covoar to get the size of an
>> instruction
>> >>> and fix the hard coded size 4 in ObjdumpProcessor.cc
>> >>
>> >>
>> >> What about a single XXX_run command? What about that suggestion?
>> >>
>> > The suggestion was to turn test_run and coverage_run into a single
>> command.
>> > I have kept them separate so that there's a possibility to just run
>> the
>> > test.
>> >
>> > If we want to run coverage everytime we run the test. we can do it.
>> > Then I think the --coverage option can be changed to --coverage-sets
>> > to mention the sets.
>> > If that's what we're looking for then I don't think a separate
>> ticket is
>> > needed,
>> > I can try to do it within tomorrow and submit an updated patch.
>> >
>> >>
>> >> Will there be an update to this patch?
>> >>
>> > This patch is working an showing results. I don't have any work
>> > going related to this patch currently.
>> > If there are any suggestions, I'll try to include all the suggested
>> updates
>> > as soon as possible and submit. So that we can get it merged.
>> >
>>
>> I get confused by the similarity between test_run() and coverage_run()
>> names, and now I'm also seeing some confusion because there is a
>> function coverage_run() and a class coverage_run. I suggest you remove
>> this function coverage_run(), and make either coverage_run.__init__()
>> or coverage_run.run() take the executables as a parameter directly.
>>
>> Thank you for the suggestion. :)
> I have removed the function and taken executables as a parameter in
> coverage_run.__init__()
>
> I have a question.
> The ini file that is fed to covoar is written by the script according
> to the
> symbols mentioned by the user. I haven't include the ini file in the
> patch.
> I'm planning to delete the file after the run, unless --no-clean
> option is given,
> which currently deletes the .cov trace files after the run.
>

 That makes sense.


> Can I proceed with this ?
>

 Yes.

>>> Added. Thanks. :)
>>>
 also, shall I include that in the .gitignore ?
>

 Is the name of the file constant? The same across multiple BSPs? If so,
 then this will be a problem doing automated testing of multiple BSPs in
 parallel.

 The ini file I'm talking about is the symbol sets config file not the
>>> bsp
>>> config file. yes the name is constant. Should it be unique to the bsp ?
>>> something like, leon3-symbols.ini ?
>>> How 

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Cillian O'Donnell
On 4 June 2018 at 19:03, Vijay Kumar Banerjee 
wrote:

> On 2 June 2018 at 01:00, Vijay Kumar Banerjee 
> wrote:
>
>> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>>
>>>
>>>
>>> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
>>> vijaykumar9...@gmail.com> wrote:
>>>
 On 1 June 2018 at 20:30, Gedare Bloom  wrote:

> On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
>  wrote:
> > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
> >>
> >>
> >>
> >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
> >>  wrote:
> >>>
> >>> Here's the list of Ideas for improvements:
> >>>
> >>> 1. Include the section coverage in the bsp config file.
> >>> If the section is not found then the script will show
> >>> proper error showing coverage is not supported for the
> >>> provided bsp config file.
> >>>
> >>> 2. Update covoar to add support for separate coverage report
> >>> for each symbol set.
> >>>
> >>> 3. Add a method somewhere in covoar to get the size of an
> instruction
> >>> and fix the hard coded size 4 in ObjdumpProcessor.cc
> >>
> >>
> >> What about a single XXX_run command? What about that suggestion?
> >>
> > The suggestion was to turn test_run and coverage_run into a single
> command.
> > I have kept them separate so that there's a possibility to just run
> the
> > test.
> >
> > If we want to run coverage everytime we run the test. we can do it.
> > Then I think the --coverage option can be changed to --coverage-sets
> > to mention the sets.
> > If that's what we're looking for then I don't think a separate
> ticket is
> > needed,
> > I can try to do it within tomorrow and submit an updated patch.
> >
> >>
> >> Will there be an update to this patch?
> >>
> > This patch is working an showing results. I don't have any work
> > going related to this patch currently.
> > If there are any suggestions, I'll try to include all the suggested
> updates
> > as soon as possible and submit. So that we can get it merged.
> >
>
> I get confused by the similarity between test_run() and coverage_run()
> names, and now I'm also seeing some confusion because there is a
> function coverage_run() and a class coverage_run. I suggest you remove
> this function coverage_run(), and make either coverage_run.__init__()
> or coverage_run.run() take the executables as a parameter directly.
>
> Thank you for the suggestion. :)
 I have removed the function and taken executables as a parameter in
 coverage_run.__init__()

 I have a question.
 The ini file that is fed to covoar is written by the script according
 to the
 symbols mentioned by the user. I haven't include the ini file in the
 patch.
 I'm planning to delete the file after the run, unless --no-clean option
 is given,
 which currently deletes the .cov trace files after the run.

>>>
>>> That makes sense.
>>>
>>>
 Can I proceed with this ?

>>>
>>> Yes.
>>>
>> Added. Thanks. :)
>>
>>> also, shall I include that in the .gitignore ?

>>>
>>> Is the name of the file constant? The same across multiple BSPs? If so,
>>> then this will be a problem doing automated testing of multiple BSPs in
>>> parallel.
>>>
>>> The ini file I'm talking about is the symbol sets config file not the bsp
>> config file. yes the name is constant. Should it be unique to the bsp ?
>> something like, leon3-symbols.ini ?
>> How does the automated testing work?
>>
>>> I think the name needs to be unique enough.to support running testing
>>> with coverage on multiple BSPs in parallel. That means you can't add it to
>>> gitignore. And can add another issue and FIXME to the code.
>>>
 If it is needed then I have a fix in mind. I can take the bsp name and
>> add
>> '-symbols.ini' to it. and add *-symbols.ini to .gitignore .
>>
> Shall I add this or put a fixme in the code and post a patch ?
> Are there any other suggestions for the patch ?
>
> I was looking into covoar for generating separate reports for each
> symbolset.
> Seems like  all the coverage reports are generated together and are written
> in the _outputDirectory_ .
>

The output directory should be aligned with the bsp and the report.html
changed to keep record of this instead of searching for the generic
coverage dir. Look for leon3-coverage and so on. As well as the
symbol-sets.ini change you mentioned above. That would probably be enough
to not cause any conflicts with parallel testing (I may be missing a case
there, I let you know if anything else comes to mind). The main thing to
think about is if multiple bsps are being tested at the same time, they
have to know which config file is there's and which output directory and
whatever else they may be looking for, the names have to be unique. These
things are all fed to covoar 

Re: [PATCH v2] tester: Add script to generate html coverage report from covoar output

2018-06-04 Thread Vijay Kumar Banerjee
On 2 June 2018 at 01:00, Vijay Kumar Banerjee 
wrote:

> On 2 June 2018 at 00:48, Joel Sherrill  wrote:
>
>>
>>
>> On Fri, Jun 1, 2018, 11:21 AM Vijay Kumar Banerjee <
>> vijaykumar9...@gmail.com> wrote:
>>
>>> On 1 June 2018 at 20:30, Gedare Bloom  wrote:
>>>
 On Fri, Jun 1, 2018 at 10:28 AM, Vijay Kumar Banerjee
  wrote:
 > On 1 June 2018 at 19:24, Joel Sherrill  wrote:
 >>
 >>
 >>
 >> On Fri, Jun 1, 2018 at 2:46 AM, Vijay Kumar Banerjee
 >>  wrote:
 >>>
 >>> Here's the list of Ideas for improvements:
 >>>
 >>> 1. Include the section coverage in the bsp config file.
 >>> If the section is not found then the script will show
 >>> proper error showing coverage is not supported for the
 >>> provided bsp config file.
 >>>
 >>> 2. Update covoar to add support for separate coverage report
 >>> for each symbol set.
 >>>
 >>> 3. Add a method somewhere in covoar to get the size of an
 instruction
 >>> and fix the hard coded size 4 in ObjdumpProcessor.cc
 >>
 >>
 >> What about a single XXX_run command? What about that suggestion?
 >>
 > The suggestion was to turn test_run and coverage_run into a single
 command.
 > I have kept them separate so that there's a possibility to just run
 the
 > test.
 >
 > If we want to run coverage everytime we run the test. we can do it.
 > Then I think the --coverage option can be changed to --coverage-sets
 > to mention the sets.
 > If that's what we're looking for then I don't think a separate ticket
 is
 > needed,
 > I can try to do it within tomorrow and submit an updated patch.
 >
 >>
 >> Will there be an update to this patch?
 >>
 > This patch is working an showing results. I don't have any work
 > going related to this patch currently.
 > If there are any suggestions, I'll try to include all the suggested
 updates
 > as soon as possible and submit. So that we can get it merged.
 >

 I get confused by the similarity between test_run() and coverage_run()
 names, and now I'm also seeing some confusion because there is a
 function coverage_run() and a class coverage_run. I suggest you remove
 this function coverage_run(), and make either coverage_run.__init__()
 or coverage_run.run() take the executables as a parameter directly.

 Thank you for the suggestion. :)
>>> I have removed the function and taken executables as a parameter in
>>> coverage_run.__init__()
>>>
>>> I have a question.
>>> The ini file that is fed to covoar is written by the script according to
>>> the
>>> symbols mentioned by the user. I haven't include the ini file in the
>>> patch.
>>> I'm planning to delete the file after the run, unless --no-clean option
>>> is given,
>>> which currently deletes the .cov trace files after the run.
>>>
>>
>> That makes sense.
>>
>>
>>> Can I proceed with this ?
>>>
>>
>> Yes.
>>
> Added. Thanks. :)
>
>> also, shall I include that in the .gitignore ?
>>>
>>
>> Is the name of the file constant? The same across multiple BSPs? If so,
>> then this will be a problem doing automated testing of multiple BSPs in
>> parallel.
>>
>> The ini file I'm talking about is the symbol sets config file not the bsp
> config file. yes the name is constant. Should it be unique to the bsp ?
> something like, leon3-symbols.ini ?
> How does the automated testing work?
>
>> I think the name needs to be unique enough.to support running testing
>> with coverage on multiple BSPs in parallel. That means you can't add it to
>> gitignore. And can add another issue and FIXME to the code.
>>
>>> If it is needed then I have a fix in mind. I can take the bsp name and
> add
> '-symbols.ini' to it. and add *-symbols.ini to .gitignore .
>
Shall I add this or put a fixme in the code and post a patch ?
Are there any other suggestions for the patch ?

I was looking into covoar for generating separate reports for each
symbolset.
Seems like  all the coverage reports are generated together and are written
in the _outputDirectory_ . I couldn't figure out how to cleanly address
this.
If covoar is intended to generate reports from multiple
subsystems/symbolsets,
then I think this would be a needed update. Otherwise we can do it from the
script, by feeding covoar with a single set ini and putting the result in a
separate
directory .

> Can we do this ?
>
>>
>>>
 -Gedare

>>>
>>>
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: question about rtc command

2018-06-04 Thread Sebastian Huber
- Am 4. Jun 2018 um 16:43 schrieb joel j...@rtems.org:

> On Mon, Jun 4, 2018 at 9:36 AM, Sebastian Huber <
> sebastian.hu...@embedded-brains.de> wrote:
> 
>> - Am 4. Jun 2018 um 16:27 schrieb joel j...@rtems.org:
>>
>> > On Mon, Jun 4, 2018 at 9:14 AM, Gedare Bloom  wrote:
>> >
>> >> Indeed, this 'rtc' shell command seems to be a bit of a misnomer. It
>> >> conflates access to two different clocks, the tod and rtc. Probably,
>> >> it would be better to provide a separate 'tod' command that
>> >> reads/writes the tod values, and refactor this 'rtc' to read/write the
>> >> rtc. I'm not quite sure about the correctness of writing to the rtc
>> >> without updating the tod, though.
>> >>
>> >
>> > Speaking from RTEMS history here. We tended to use Time of Day
>> > to refer to what RTEMS was maintaining and Real-Time Clock to
>> > review to battery backed clocks that are ICs. This matches Wikipedia's
>> > definition and example of a DS1307.
>> >
>> > The driver framework in RTEMS is referred to as RTC but some BSPs
>> > call the driver TOD and name files and directories accordingly.
>> >
>> > Renaming TOD drivers to RTC derived names has been on my wish
>> > list for a long time.
>>
>> All the RTC drivers should be now in an "rtc" directory.
>>
> 
> Great! Did you rename the files at the same time? There were some
> todcfg.c files as I recall and other variants.

No, I didn't look at the file names.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: question about rtc command

2018-06-04 Thread Joel Sherrill
On Mon, Jun 4, 2018 at 9:36 AM, Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> - Am 4. Jun 2018 um 16:27 schrieb joel j...@rtems.org:
>
> > On Mon, Jun 4, 2018 at 9:14 AM, Gedare Bloom  wrote:
> >
> >> Indeed, this 'rtc' shell command seems to be a bit of a misnomer. It
> >> conflates access to two different clocks, the tod and rtc. Probably,
> >> it would be better to provide a separate 'tod' command that
> >> reads/writes the tod values, and refactor this 'rtc' to read/write the
> >> rtc. I'm not quite sure about the correctness of writing to the rtc
> >> without updating the tod, though.
> >>
> >
> > Speaking from RTEMS history here. We tended to use Time of Day
> > to refer to what RTEMS was maintaining and Real-Time Clock to
> > review to battery backed clocks that are ICs. This matches Wikipedia's
> > definition and example of a DS1307.
> >
> > The driver framework in RTEMS is referred to as RTC but some BSPs
> > call the driver TOD and name files and directories accordingly.
> >
> > Renaming TOD drivers to RTC derived names has been on my wish
> > list for a long time.
>
> All the RTC drivers should be now in an "rtc" directory.
>

Great! Did you rename the files at the same time? There were some
todcfg.c files as I recall and other variants.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: question about rtc command

2018-06-04 Thread Sebastian Huber
- Am 4. Jun 2018 um 16:27 schrieb joel j...@rtems.org:

> On Mon, Jun 4, 2018 at 9:14 AM, Gedare Bloom  wrote:
> 
>> Indeed, this 'rtc' shell command seems to be a bit of a misnomer. It
>> conflates access to two different clocks, the tod and rtc. Probably,
>> it would be better to provide a separate 'tod' command that
>> reads/writes the tod values, and refactor this 'rtc' to read/write the
>> rtc. I'm not quite sure about the correctness of writing to the rtc
>> without updating the tod, though.
>>
> 
> Speaking from RTEMS history here. We tended to use Time of Day
> to refer to what RTEMS was maintaining and Real-Time Clock to
> review to battery backed clocks that are ICs. This matches Wikipedia's
> definition and example of a DS1307.
> 
> The driver framework in RTEMS is referred to as RTC but some BSPs
> call the driver TOD and name files and directories accordingly.
> 
> Renaming TOD drivers to RTC derived names has been on my wish
> list for a long time.

All the RTC drivers should be now in an "rtc" directory.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: question about rtc command

2018-06-04 Thread Joel Sherrill
On Mon, Jun 4, 2018 at 9:14 AM, Gedare Bloom  wrote:

> Indeed, this 'rtc' shell command seems to be a bit of a misnomer. It
> conflates access to two different clocks, the tod and rtc. Probably,
> it would be better to provide a separate 'tod' command that
> reads/writes the tod values, and refactor this 'rtc' to read/write the
> rtc. I'm not quite sure about the correctness of writing to the rtc
> without updating the tod, though.
>

Speaking from RTEMS history here. We tended to use Time of Day
to refer to what RTEMS was maintaining and Real-Time Clock to
review to battery backed clocks that are ICs. This matches Wikipedia's
definition and example of a DS1307.

The driver framework in RTEMS is referred to as RTC but some BSPs
call the driver TOD and name files and directories accordingly.

Renaming TOD drivers to RTC derived names has been on my wish
list for a long time.

>From the command line, the POSIX date command (or a light version)
is available. The rtc shell command should be dealing with the RTC
hardware device. This way you can see RTEMS view of time and the
hardware view. If you have a NTP client, I would think you might like
to see its view of time also.

So the rtc command makes sense if you have a battery backed RTC.
But it often doesn't make sense to even include one in an embedded
system.

This is a case where a man with two watches never really knows what
time it is. :)

Sorry for the historical confusion. But the distinction originally was
pretty clear.

--joel



>
> On Mon, May 28, 2018 at 3:22 AM, jameszxj  wrote:
> > hi all,
> >rtc command return the current time of day, it seems unreasonable. I
> > think it should return the time of real time clock.
> > Any other reasons to do so?
> > ___
> > 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
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: question about rtc command

2018-06-04 Thread Gedare Bloom
Indeed, this 'rtc' shell command seems to be a bit of a misnomer. It
conflates access to two different clocks, the tod and rtc. Probably,
it would be better to provide a separate 'tod' command that
reads/writes the tod values, and refactor this 'rtc' to read/write the
rtc. I'm not quite sure about the correctness of writing to the rtc
without updating the tod, though.

On Mon, May 28, 2018 at 3:22 AM, jameszxj  wrote:
> hi all,
>rtc command return the current time of day, it seems unreasonable. I
> think it should return the time of real time clock.
> Any other reasons to do so?
> ___
> 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


Re: buffer overrun in rtems_rfs_bitmap_create_search()

2018-06-04 Thread Gedare Bloom
Hello Walter,

Thank you for the bug report and patch. The patch is outdated, what
version of RTEMS are you using? I think the problem also affects the
master branch, but we need a ticket for each affected open branch.

The fix looks OK to me, but I'd like Chris Johns to approve it. I
assigned the ticket to him.

Gedare

On Wed, May 30, 2018 at 1:24 PM, Walter Lee  wrote:
> Hi.  I am encountering a buffer overrun in
> rtems_rfs_bitmap_create_search().  It seems that whenever the bitmap
> uses the last bit of its search_map (i.e. (control->size + 31) % 32 ==
> 32)), the loop will write to the word one beyond the end of
> search_map.
>
> I filed a bug at https://devel.rtems.org/ticket/3439, with a patch
> that fixes the problem.
>
> Please let me know if I'm missing something, and if not what I need to
> do to get this fixed.
>
> Thanks,
>
> Walter
> ___
> 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


Re: [GSoC - x86_64 BSP] Using fPIC to compile RTEMS as a shared library

2018-06-04 Thread Joel Sherrill
On Mon, Jun 4, 2018 at 5:44 AM, Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

>
>
> - Am 4. Jun 2018 um 12:20 schrieb Amaan Cheval amaan.che...@gmail.com:
>
> > That's a good idea. That way based on the multilib variant, Newlib would
> be
> > compiled using fPIC, yes?
>
> Yes.
>

This would be desirable for the i386 as well. Having the RTEMS libraries as
dynamic libraries would be more natural under Deos.

Just a statement. Not a requirement on the GSoC project.

>
> >
> > Then I could simply figure out how to solve the crtbegin and crtend
> dilemma
> > (which I believe should be easier), and use those to have a
> dynamic/shared
> > RTEMS kernel + user application.
>
> These files will be compiled with -fPIC as well.
>



>
> >
> > If that sounds right, I'll look into that first. Not familiar with the
> GCC
> > source yet, but it should be doable.
>
> Sorry, I have no idea how the x86_64 configuration of GCC works for RTEMS.
>
> >
> >
> > On Mon, Jun 4, 2018, 3:43 PM Sebastian Huber <
> > sebastian.hu...@embedded-brains.de> wrote:
> >
> >> Hello Amaan,
> >>
> >> can't you add a new multilib variant which includes -fPIC to the GCC
> >> configuration for RTEMS?
> ___
> 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

Re: [GSoC - x86_64 BSP] Using fPIC to compile RTEMS as a shared library

2018-06-04 Thread Sebastian Huber



- Am 4. Jun 2018 um 12:20 schrieb Amaan Cheval amaan.che...@gmail.com:

> That's a good idea. That way based on the multilib variant, Newlib would be
> compiled using fPIC, yes?

Yes.

> 
> Then I could simply figure out how to solve the crtbegin and crtend dilemma
> (which I believe should be easier), and use those to have a dynamic/shared
> RTEMS kernel + user application.

These files will be compiled with -fPIC as well.

> 
> If that sounds right, I'll look into that first. Not familiar with the GCC
> source yet, but it should be doable.

Sorry, I have no idea how the x86_64 configuration of GCC works for RTEMS.

> 
> 
> On Mon, Jun 4, 2018, 3:43 PM Sebastian Huber <
> sebastian.hu...@embedded-brains.de> wrote:
> 
>> Hello Amaan,
>>
>> can't you add a new multilib variant which includes -fPIC to the GCC
>> configuration for RTEMS?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [GSoC - x86_64 BSP] Using fPIC to compile RTEMS as a shared library

2018-06-04 Thread Amaan Cheval
That's a good idea. That way based on the multilib variant, Newlib would be
compiled using fPIC, yes?

Then I could simply figure out how to solve the crtbegin and crtend dilemma
(which I believe should be easier), and use those to have a dynamic/shared
RTEMS kernel + user application.

If that sounds right, I'll look into that first. Not familiar with the GCC
source yet, but it should be doable.


On Mon, Jun 4, 2018, 3:43 PM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> Hello Amaan,
>
> can't you add a new multilib variant which includes -fPIC to the GCC
> configuration for RTEMS?
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [GSoC - x86_64 BSP] Using fPIC to compile RTEMS as a shared library

2018-06-04 Thread Sebastian Huber
Hello Amaan,

can't you add a new multilib variant which includes -fPIC to the GCC 
configuration for RTEMS?
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


[GSoC - x86_64 BSP] Using fPIC to compile RTEMS as a shared library

2018-06-04 Thread Amaan Cheval
Hi!

I figured I'd quickly confirm the direction I'm taking towards
compiling RTEMS as a dynamic/shared library.

Problems I've run into in setting up amd64.cfg to compile all of RTEMS
as a shared library:

- In the x86_64 tools, gcc's "-shared" flag has a different effect
than the "-Wl,-shared" flag used to pass the flag to the linker - the
former silently seems to compile a static library. The latter leads us
to our next issue

- Newlib seems to be compiled as a static libc.a. This leads to errors
such as the following:

https://gist.github.com/AmaanC/475bc0298697d22b944577ac80ec2736#file-rtems-make-fpic-log-L178

I believe this _should_ be solvable by compiling newlib as a shared
library, and _then_ linking the shared libc with RTEMS together. See
[1][2][3] for more.

Please let me know if that approach doesn't make sense - given that
there is no dynamic loader in the RTEMS kernel as far as I know, what
we really want _is_ a static file, but for it to be a relocatable PE,
we need to convince GCC to spit out a relocatable but fully resolved
shared library.

- Similarly, the gcc-compiled crtbegin and crtend also include static
relocations which are invalid when compiling with fPIC.

I think we can use crtbeginS.o and crtendS.o[4] in place of those -
that way, we might still be able to have GCC handle their inclusion,
without needing our bsp_specs. I'll look into this after figuring
newlib out.

---

I'd just like some confirmation on this being the correct path to
follow. I'm not quite sure, because if newlib is a shared library, I
think we'll need to divide the current build stage up to add stages
like:
- Compile librtemscpu.a and librtemsbsp.a with -fPIC
- Compile test_xyz.c while linking it with libc, librtems*, lib*efi,
etc. to create a fully resolved test_xyz.so
- Convert .so to PE using objcopy

Does that make sense to you all?

---

[1] 
https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/README;h=e793d57ce75e56d1eb044e2c0325631e9eeef1af;hb=HEAD#l498
[2] https://sourceware.org/ml/newlib/2016/msg01106.html
[3] 
https://forum.osdev.org/viewtopic.php?p=276046=c7798911615bef866354e92a64125b1c#p276046
[4] https://dev.gentoo.org/~vapier/crt.txtz
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel