Re: Making Covoar More C++

[Chris Johns]

On 25/3/21 11:54 am, Joel Sherrill wrote:
> 
> 
> On Wed, Mar 24, 2021, 7:28 PM Chris Johns  > wrote:
> 
> 
> 
> On 25/3/21 6:54 am, Joel Sherrill wrote:
> >
> >
> > On Wed, Mar 24, 2021 at 2:42 PM Gedare Bloom  
> > >> wrote:
> >
> >     On Wed, Mar 24, 2021 at 1:35 PM Joel Sherrill  
> >     >> wrote:
> >     >
> >     > Hi
> >     >
> >     > There has been a lot of talk about making covoar use more C++
> >     > features. It seems to be an issue on every patch. I almost
> >     > replied to Gedare's comment at the bottom of a patch
> >     > but decided it needed another thread:
> >     >
> >     > "I still struggle reviewing this codebase, in part because it is 
> C+C++
> >     > (TM) and in part because I'm not so proficient in C++ to make 
> concrete
> >     > recommendations how to write this better. I think, if the goal is
> >     > eventually to make this more C++ like code, then new code coming 
> in
> >     > should aim to move the needle in that direction rather than 
> continue
> >     > to propagate the old ways of doing."
> >     >
> >     Thanks for taking this on.
> 
> 
> Alex's willing to make some of the changes. It's just a matter of having a
> baseline so when we make changes they can be tested thoroughly. We now have
> about a half dozen architectures producing reports so it is a good baseline.
> 
> I was actually thinking that once all of these are merged, it might be a good
> place to tag because any git bisect might want to come back to here.
> 
> >
> >     > I personally do NOT want to see changes to C++ in one leaf class 
> and
> >     > the other architectures not get the same changes. I would prefer 
> to see
> >     > all these corrections and base changes in the same style with 
> limited
> >     > changes to C-isms. I'm not opposed to the changes but let's take 
> the
> >     > Target class one. There are multiple target classes. Changing one
> >     > independent of the others isn't a good idea.
> >     >
> >     This is reasonable to me.
> >
> >     > I'd like to see us get a working baseline in and then do 
> something like
> >     > sweep std::string through Target* as a single patch. This is 
> easier to
> >     > test and review since it would only be C string to std::string. 
> Perhaps
> >     > switch to C++ output a file at a time. Redo the report output. 
> Etc.
> >     > Discrete chunks instead of piecemeal.
> >     >
> >     > Covoar has spent years broken and some is from changing working
> >     > things to do things "a better way" with no baseline to check 
> against.
> >     > We need to get a baseline.
> >     >
> >     > Please. Let's get a working baseline and then approach this more
> >     > methodically. No one is going to suffer from seeing a C string a 
> little
> >     > while longer. :)
> >     >
> >     I'm fine, as long as there is a plan in place and some clear
> >     directions. It would help to have tickets to organize the path
> >     forward.
> 
> 
> +1
> 
> One issue is the order of the changes. I at first thought about making the
> string changes and then realized that we would end up having C printf and have
> to temporarily add c_str a lot. It likely makes more sense to sweep all the
> output first and then switch to C++ strings.
> 
> I have also asked Alex to put together some diagrams so we can discuss this
> better. I think there is some data flow and delivered inheritance in this
> program that is not being understood by everyone.
> 
> Plus Alex and I talked earlier this evening and think we have a reasonable 
> path
> to greatly speed it up without massive overhauls.
> 
> >
> >     I'm willing to oblige continued use of C'ism for now, but I want to
> >     know the plan and maybe a deadline of sorts by which I can start to 
> be
> >     picky again :) I don't like to be in limbo.
> >
> >
> > I'd love to have a deadline but I can't guarantee how long Alex can
> > work on it. But unless he gets pulled, he can pick on this for a while.
> 
> Joel, I pushed a number of changes to move covoar towards C++ back when I 
> pushed
> in the ELF and DWARF support. The C++ nature of the interfaces I brought 
> in from
> the tool kit required some refactoring. I have not seen much action since 
> then
> so Gedare's question seems reasonable.
> 
> 
> Yes you did and because we did not review the reports, there was a backlog of
> issues that had to be fixed. Some from this change some from other changes. No
> blame. I just want a better baseline this time.

Yes there issues, I made the change but did limited testing given it 

Re: Making Covoar More C++

[Joel Sherrill]

On Wed, Mar 24, 2021, 7:28 PM Chris Johns  wrote:

>
>
> On 25/3/21 6:54 am, Joel Sherrill wrote:
> >
> >
> > On Wed, Mar 24, 2021 at 2:42 PM Gedare Bloom  > > wrote:
> >
> > On Wed, Mar 24, 2021 at 1:35 PM Joel Sherrill  > > wrote:
> > >
> > > Hi
> > >
> > > There has been a lot of talk about making covoar use more C++
> > > features. It seems to be an issue on every patch. I almost
> > > replied to Gedare's comment at the bottom of a patch
> > > but decided it needed another thread:
> > >
> > > "I still struggle reviewing this codebase, in part because it is
> C+C++
> > > (TM) and in part because I'm not so proficient in C++ to make
> concrete
> > > recommendations how to write this better. I think, if the goal is
> > > eventually to make this more C++ like code, then new code coming in
> > > should aim to move the needle in that direction rather than
> continue
> > > to propagate the old ways of doing."
> > >
> > Thanks for taking this on.
>

Alex's willing to make some of the changes. It's just a matter of having a
baseline so when we make changes they can be tested thoroughly. We now have
about a half dozen architectures producing reports so it is a good baseline.

I was actually thinking that once all of these are merged, it might be a
good place to tag because any git bisect might want to come back to here.

> >
> > > I personally do NOT want to see changes to C++ in one leaf class
> and
> > > the other architectures not get the same changes. I would prefer
> to see
> > > all these corrections and base changes in the same style with
> limited
> > > changes to C-isms. I'm not opposed to the changes but let's take
> the
> > > Target class one. There are multiple target classes. Changing one
> > > independent of the others isn't a good idea.
> > >
> > This is reasonable to me.
> >
> > > I'd like to see us get a working baseline in and then do something
> like
> > > sweep std::string through Target* as a single patch. This is
> easier to
> > > test and review since it would only be C string to std::string.
> Perhaps
> > > switch to C++ output a file at a time. Redo the report output. Etc.
> > > Discrete chunks instead of piecemeal.
> > >
> > > Covoar has spent years broken and some is from changing working
> > > things to do things "a better way" with no baseline to check
> against.
> > > We need to get a baseline.
> > >
> > > Please. Let's get a working baseline and then approach this more
> > > methodically. No one is going to suffer from seeing a C string a
> little
> > > while longer. :)
> > >
> > I'm fine, as long as there is a plan in place and some clear
> > directions. It would help to have tickets to organize the path
> > forward.
>

+1

One issue is the order of the changes. I at first thought about making the
string changes and then realized that we would end up having C printf and
have to temporarily add c_str a lot. It likely makes more sense to sweep
all the output first and then switch to C++ strings.

I have also asked Alex to put together some diagrams so we can discuss this
better. I think there is some data flow and delivered inheritance in this
program that is not being understood by everyone.

Plus Alex and I talked earlier this evening and think we have a reasonable
path to greatly speed it up without massive overhauls.

>
> > I'm willing to oblige continued use of C'ism for now, but I want to
> > know the plan and maybe a deadline of sorts by which I can start to
> be
> > picky again :) I don't like to be in limbo.
> >
> >
> > I'd love to have a deadline but I can't guarantee how long Alex can
> > work on it. But unless he gets pulled, he can pick on this for a while.
>
> Joel, I pushed a number of changes to move covoar towards C++ back when I
> pushed
> in the ELF and DWARF support. The C++ nature of the interfaces I brought
> in from
> the tool kit required some refactoring. I have not seen much action since
> then
> so Gedare's question seems reasonable.
>

Yes you did and because we did not review the reports, there was a backlog
of issues that had to be fixed. Some from this change some from other
changes. No blame. I just want a better baseline this time.


> The need for this code to be C++ and not a mix comes down to what you want
> to
> see happen. There is no real need to move the code closer to C++ other than
> improving the technical quality and that is about the life of the code in
> the
> project. If you are willing to look after the code as is then I am fine
> with
> that. If you would like to see the code move to C++ and away from C then
> can
> also happen. It would be an interesting way to learn more about C++.
>
> I find the code hard to work on because I do not know if I am looking at C
> and
> needing to use C solutions or C++ and 

Re: Making Covoar More C++

[Chris Johns]

On 25/3/21 6:54 am, Joel Sherrill wrote:
> 
> 
> On Wed, Mar 24, 2021 at 2:42 PM Gedare Bloom  > wrote:
> 
> On Wed, Mar 24, 2021 at 1:35 PM Joel Sherrill  > wrote:
> >
> > Hi
> >
> > There has been a lot of talk about making covoar use more C++
> > features. It seems to be an issue on every patch. I almost
> > replied to Gedare's comment at the bottom of a patch
> > but decided it needed another thread:
> >
> > "I still struggle reviewing this codebase, in part because it is C+C++
> > (TM) and in part because I'm not so proficient in C++ to make concrete
> > recommendations how to write this better. I think, if the goal is
> > eventually to make this more C++ like code, then new code coming in
> > should aim to move the needle in that direction rather than continue
> > to propagate the old ways of doing."
> >
> Thanks for taking this on.
> 
> > I personally do NOT want to see changes to C++ in one leaf class and
> > the other architectures not get the same changes. I would prefer to see
> > all these corrections and base changes in the same style with limited
> > changes to C-isms. I'm not opposed to the changes but let's take the
> > Target class one. There are multiple target classes. Changing one
> > independent of the others isn't a good idea.
> >
> This is reasonable to me.
> 
> > I'd like to see us get a working baseline in and then do something like
> > sweep std::string through Target* as a single patch. This is easier to
> > test and review since it would only be C string to std::string. Perhaps
> > switch to C++ output a file at a time. Redo the report output. Etc.
> > Discrete chunks instead of piecemeal.
> >
> > Covoar has spent years broken and some is from changing working
> > things to do things "a better way" with no baseline to check against.
> > We need to get a baseline.
> >
> > Please. Let's get a working baseline and then approach this more
> > methodically. No one is going to suffer from seeing a C string a little
> > while longer. :)
> >
> I'm fine, as long as there is a plan in place and some clear
> directions. It would help to have tickets to organize the path
> forward.
> 
> I'm willing to oblige continued use of C'ism for now, but I want to
> know the plan and maybe a deadline of sorts by which I can start to be
> picky again :) I don't like to be in limbo.
> 
> 
> I'd love to have a deadline but I can't guarantee how long Alex can
> work on it. But unless he gets pulled, he can pick on this for a while.

Joel, I pushed a number of changes to move covoar towards C++ back when I pushed
in the ELF and DWARF support. The C++ nature of the interfaces I brought in from
the tool kit required some refactoring. I have not seen much action since then
so Gedare's question seems reasonable.

The need for this code to be C++ and not a mix comes down to what you want to
see happen. There is no real need to move the code closer to C++ other than
improving the technical quality and that is about the life of the code in the
project. If you are willing to look after the code as is then I am fine with
that. If you would like to see the code move to C++ and away from C then can
also happen. It would be an interesting way to learn more about C++.

I find the code hard to work on because I do not know if I am looking at C and
needing to use C solutions or C++ and I should be pulling on C++ threads and
where they go. I also see this is an issue for those working on the code. In
some recent patches I pointed out new code being call from C++ code with C++
objects that was written in C. I pointed this out and the next patch was fine.

> My guess is that C string to std::string is probably a good pass by
> itself since method signatures may change. 

I think anything you feel can be changed and is in reach is welcome.

> There isn't much file input but that could be a pass by itself
> along the way,
> 
> Then sweep output one file at a time leaving reporting for last as
> a batch.
> 
> Do you see an order?

Maybe we organise a group session online where we all look over the code
together and discuss various aspects of the architecture and what it means from
a strictly C vs C++ point of view. Any change like this cannot happen in a
vacuum and I believe C++ is more of a taught language than C. C styles are
easier to pick up  by reading code. My introduction to C++ was in the mid-90's
with a Rational training course and documentation. It takes time to relearn C
solutions you know and have at hand in C++.

I have no idea what we would need to hold such a session and who would be
interested but I am happy to be there and be part of it.

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

Re: Making Covoar More C++

[Joel Sherrill]

On Wed, Mar 24, 2021 at 2:42 PM Gedare Bloom  wrote:

> On Wed, Mar 24, 2021 at 1:35 PM Joel Sherrill  wrote:
> >
> > Hi
> >
> > There has been a lot of talk about making covoar use more C++
> > features. It seems to be an issue on every patch. I almost
> > replied to Gedare's comment at the bottom of a patch
> > but decided it needed another thread:
> >
> > "I still struggle reviewing this codebase, in part because it is C+C++
> > (TM) and in part because I'm not so proficient in C++ to make concrete
> > recommendations how to write this better. I think, if the goal is
> > eventually to make this more C++ like code, then new code coming in
> > should aim to move the needle in that direction rather than continue
> > to propagate the old ways of doing."
> >
> Thanks for taking this on.
>
> > I personally do NOT want to see changes to C++ in one leaf class and
> > the other architectures not get the same changes. I would prefer to see
> > all these corrections and base changes in the same style with limited
> > changes to C-isms. I'm not opposed to the changes but let's take the
> > Target class one. There are multiple target classes. Changing one
> > independent of the others isn't a good idea.
> >
> This is reasonable to me.
>
> > I'd like to see us get a working baseline in and then do something like
> > sweep std::string through Target* as a single patch. This is easier to
> > test and review since it would only be C string to std::string. Perhaps
> > switch to C++ output a file at a time. Redo the report output. Etc.
> > Discrete chunks instead of piecemeal.
> >
> > Covoar has spent years broken and some is from changing working
> > things to do things "a better way" with no baseline to check against.
> > We need to get a baseline.
> >
> > Please. Let's get a working baseline and then approach this more
> > methodically. No one is going to suffer from seeing a C string a little
> > while longer. :)
> >
> I'm fine, as long as there is a plan in place and some clear
> directions. It would help to have tickets to organize the path
> forward.
>
> I'm willing to oblige continued use of C'ism for now, but I want to
> know the plan and maybe a deadline of sorts by which I can start to be
> picky again :) I don't like to be in limbo.
>

I'd love to have a deadline but I can't guarantee how long Alex can
work on it. But unless he gets pulled, he can pick on this for a while.

My guess is that C string to std::string is probably a good pass by
itself since method signatures may change.

There isn't much file input but that could be a pass by itself
along the way,

Then sweep output one file at a time leaving reporting for last as
a batch.

Do you see an order?

--joel

>
> > Thanks.
> >
> > --joel
> >
> >
> > ___
> > 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: Making Covoar More C++

[Gedare Bloom]

On Wed, Mar 24, 2021 at 1:35 PM Joel Sherrill  wrote:
>
> Hi
>
> There has been a lot of talk about making covoar use more C++
> features. It seems to be an issue on every patch. I almost
> replied to Gedare's comment at the bottom of a patch
> but decided it needed another thread:
>
> "I still struggle reviewing this codebase, in part because it is C+C++
> (TM) and in part because I'm not so proficient in C++ to make concrete
> recommendations how to write this better. I think, if the goal is
> eventually to make this more C++ like code, then new code coming in
> should aim to move the needle in that direction rather than continue
> to propagate the old ways of doing."
>
Thanks for taking this on.

> I personally do NOT want to see changes to C++ in one leaf class and
> the other architectures not get the same changes. I would prefer to see
> all these corrections and base changes in the same style with limited
> changes to C-isms. I'm not opposed to the changes but let's take the
> Target class one. There are multiple target classes. Changing one
> independent of the others isn't a good idea.
>
This is reasonable to me.

> I'd like to see us get a working baseline in and then do something like
> sweep std::string through Target* as a single patch. This is easier to
> test and review since it would only be C string to std::string. Perhaps
> switch to C++ output a file at a time. Redo the report output. Etc.
> Discrete chunks instead of piecemeal.
>
> Covoar has spent years broken and some is from changing working
> things to do things "a better way" with no baseline to check against.
> We need to get a baseline.
>
> Please. Let's get a working baseline and then approach this more
> methodically. No one is going to suffer from seeing a C string a little
> while longer. :)
>
I'm fine, as long as there is a plan in place and some clear
directions. It would help to have tickets to organize the path
forward.

I'm willing to oblige continued use of C'ism for now, but I want to
know the plan and maybe a deadline of sorts by which I can start to be
picky again :) I don't like to be in limbo.

> Thanks.
>
> --joel
>
>
> ___
> 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

Making Covoar More C++

[Joel Sherrill]

Hi

There has been a lot of talk about making covoar use more C++
features. It seems to be an issue on every patch. I almost
replied to Gedare's comment at the bottom of a patch
but decided it needed another thread:

"I still struggle reviewing this codebase, in part because it is C+C++
(TM) and in part because I'm not so proficient in C++ to make concrete
recommendations how to write this better. I think, if the goal is
eventually to make this more C++ like code, then new code coming in
should aim to move the needle in that direction rather than continue
to propagate the old ways of doing."

I personally do NOT want to see changes to C++ in one leaf class and
the other architectures not get the same changes. I would prefer to see
all these corrections and base changes in the same style with limited
changes to C-isms. I'm not opposed to the changes but let's take the
Target class one. There are multiple target classes. Changing one
independent of the others isn't a good idea.

I'd like to see us get a working baseline in and then do something like
sweep std::string through Target* as a single patch. This is easier to
test and review since it would only be C string to std::string. Perhaps
switch to C++ output a file at a time. Redo the report output. Etc.
Discrete chunks instead of piecemeal.

Covoar has spent years broken and some is from changing working
things to do things "a better way" with no baseline to check against.
We need to get a baseline.

Please. Let's get a working baseline and then approach this more
methodically. No one is going to suffer from seeing a C string a little
while longer. :)

Thanks.

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

Re: [PATCH] covoar: Add aarch64 target

[Gedare Bloom]

On Wed, Mar 24, 2021 at 12:27 PM Alex White  wrote:
>
> On Wed, Mar 24, 2021 at 12:25 PM Gedare Bloom  wrote:
> >
> > On Wed, Mar 24, 2021 at 10:46 AM Alex White  wrote:
> > > diff --git a/tester/covoar/Target_aarch64.cc 
> > > b/tester/covoar/Target_aarch64.cc
> > > new file mode 100644
> > > index 000..64472d6
> > > --- /dev/null
> > > +++ b/tester/covoar/Target_aarch64.cc
> > > @@ -0,0 +1,100 @@
> > > +/*! @file Target_aarch64.cc
> > > + *  @brief Target_aarch64 Implementation
> > > + *
> > > + *  This file contains the implementation of the base class for
> > > + *  functions supporting target unique functionallity.
> > only one l in functionality
>
> See final comment below.
>
> >
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +
> > > +#include "Target_aarch64.h"
> > > +
> > > +namespace Target {
> > > +
> > > +  Target_aarch64::Target_aarch64( std::string targetName ):
> > > +TargetBase( targetName )
> > > +  {
> > > +conditionalBranchInstructions.push_back("cbnz");
> > > +conditionalBranchInstructions.push_back("cbz");
> > > +conditionalBranchInstructions.push_back("tbnz");
> > > +conditionalBranchInstructions.push_back("tbz");
> > > +conditionalBranchInstructions.push_back("b.eq");
> > > +conditionalBranchInstructions.push_back("b.ne");
> > > +conditionalBranchInstructions.push_back("b.cs");
> > > +conditionalBranchInstructions.push_back("b.hs");
> > > +conditionalBranchInstructions.push_back("b.cc");
> > > +conditionalBranchInstructions.push_back("b.lo");
> > > +conditionalBranchInstructions.push_back("b.mi");
> > > +conditionalBranchInstructions.push_back("b.pl");
> > > +conditionalBranchInstructions.push_back("b.vs");
> > > +conditionalBranchInstructions.push_back("b.vc");
> > > +conditionalBranchInstructions.push_back("b.hi");
> > > +conditionalBranchInstructions.push_back("b.ls");
> > > +conditionalBranchInstructions.push_back("b.ge");
> > > +conditionalBranchInstructions.push_back("b.lt");
> > > +conditionalBranchInstructions.push_back("b.gt");
> > > +conditionalBranchInstructions.push_back("b.le");
> > > +
> > > +conditionalBranchInstructions.sort();
> >
> > this is kind of lazy :) you could sort them as you push them.
>
> See final comment below.
>
> >
> > > +  }
> > > +
> > > +  Target_aarch64::~Target_aarch64()
> > > +  {
> > > +  }
> > > +
> > > +  bool Target_aarch64::isNopLine(
> > > +const char* const line,
> > > +int&  size
> > > +  )
> > > +  {
> > > +if (!strcmp( [strlen(line)-3], "nop")) {
> > any reason this is strcmp but the others are strncmp?
>
> No reason, no. I should change it to strncmp.
>
> >
> > > +  size = 4;
> > > +  return true;
> > > +}
> > I wonder if you guys want to take a stab at using C++ strings or not?
> > just a thought.
> >
> > This stuff how it is being done is very fickle with these constant
> > numbers, but I guess the interface shouldn't change too much since it
> > derives from the ISA. It just isn't clear at all how this stuff works
> > by casual observation.
>
> See final comment below.
>
> >
> > > +
> > > +if (!strncmp( [strlen(line)-6], "udf", 3)) {
> > > +  size = 4;
> > > +  return true;
> > > +}
> > > +
> > > +// On ARM, there are literal tables at the end of methods.
> > > +// We need to avoid them.
> > > +if (!strncmp( [strlen(line)-10], ".byte", 5)) {
> > > +  size = 1;
> > > +  return true;
> > > +}
> > > +if (!strncmp( [strlen(line)-13], ".short", 6)) {
> > > +  size = 2;
> > > +  return true;
> > > +}
> > > +if (!strncmp( [strlen(line)-16], ".word", 5)) {
> > > +  size = 4;
> > > +  return true;
> > > +}
> > > +
> > > +return false;
> > > +  }
> > > +
> > > +  bool Target_aarch64::isBranch(
> > > +  const char* instruction
> > > +  )
> > > +  {
> > > +throw rld::error(
> > > +  "DETERMINE BRANCH INSTRUCTIONS FOR THIS ARCHITECTURE! -- fix me",
> > Seems like this could be a better message. Is this the "standard"
> > error message being used in covoar for this condition?
>
> See final comment below.
>
> >
> > > +  "Target_aarch64::isBranch"
> > > +);
> > > +  }
> > > +
> > > +  TargetBase *Target_aarch64_Constructor(
> > > +std::string  targetName
> > > +  )
> > > +  {
> > > +return new Target_aarch64( targetName );
> > > +  }
> > > +
> > > +}
> > > diff --git a/tester/covoar/Target_aarch64.h 
> > > b/tester/covoar/Target_aarch64.h
> > > new file mode 100644
> > > index 000..8c15daa
> > > --- /dev/null
> > > +++ b/tester/covoar/Target_aarch64.h
> > > @@ -0,0 +1,77 @@
> > > +/*! @file Target_aarch64.h
> > > + *  @brief Target_aarch64 Specification
> > > + *
> > > + *  This file contains the specification of the Target_aarch64 class.
> > > + */
> > > +
> > > +#ifndef __TARGET_AARCH64_H__
> > > +#define __TARGET_AARCH64_H__
> > > +
> > 

Re: GSoC Project : Package Micro-python

[Gedare Bloom]

On Wed, Mar 24, 2021 at 11:38 AM Eshan Dhawan  wrote:
>
>
>
> On Wed, Mar 24, 2021 at 12:34 AM Gedare Bloom  wrote:
>>
>> On Tue, Mar 23, 2021 at 12:16 PM Eshan Dhawan  
>> wrote:
>> >
>> >
>> > Apologies for the late reply.
>> >
>> > On Mon, Mar 22, 2021 at 10:27 PM Joel Sherrill  wrote:
>> >>
>> >>
>> >>
>> >> On Mon, Mar 22, 2021 at 11:55 AM Gedare Bloom  wrote:
>> >>>
>> >>> On Mon, Mar 22, 2021 at 10:50 AM Joel Sherrill  wrote:
>> >>> >
>> >>> >
>> >>> >
>> >>> > On Mon, Mar 22, 2021 at 11:30 AM Gedare Bloom  wrote:
>> >>> >>
>> >>> >> On Sat, Mar 20, 2021 at 12:33 PM Eshan Dhawan 
>> >>> >>  wrote:
>> >>> >> >
>> >>> >> > Hello Everyone,
>> >>> >> > I wanted to take Packaging Micro Python up as GSOC project this 
>> >>> >> > summer and the project will also include packaging LUA and picoC
>> >>> >> > The ticket for Micro Python  : https://devel.rtems.org/ticket/4349
>> >>> >> > What would be the complete Scope of the project?
>> >>> >> > And what would be a good starting point?
>> >>> >> >
>> >>> >>
>> >>> >> Well, I guess Joel must have described the task, so I'll leave it to
>> >>> >> him to fill in some more details.
>> >>> >>
>> >>> >> Adding RSB packages may be not sufficient coding work for GSoC. It is
>> >>> >> important in the proposal to identify what would be the coding
>> >>> >> activities involved in this project. For example, we know from
>> >>> >> experience that Lua can just be built from some minor tailoring of its
>> >>> >> Makefile, so the package is very straightforward. However, the
>> >>> >> projects you mention are scripting environments, so maybe creating a
>> >>> >> framework in RTEMS for a "shell/intepreter" that can be built as an
>> >>> >> add-on by RSB would be a proper way to scope this effort
>> >
>> > Packaging might not be a lot of coding part but adding its documentation 
>> > and its example would be a very iterative and time consuming process.
>>
>> Remember that code is what counts, while we expect the other stuff to
>> come along too, you don't want to be doing 90% doco and 10% code. Just
>> keep it in mind.
>
> What would be a good inclusion to this project ?
> I was thinking long double support since I worked on porting POSIX functions 
> I might find it easier.
> But it might interfere with matt's project if I understand that project 
> correctly.

Right, please don't include that. You'll want to think/talk through
(with Joel, maybe) what could be good code contributions. If the RSB
packaging is fairly minimal, then creating a suite of examples might
be one way to increase the SLOC contributions. I also think there is
merit to the idea of creating a "plug-in" way to add shells to RTEMS.
Maybe even refactoring our current shell out to a add-on package then.
Just a thought.

>>
>>
>> >>>
>> >>> >
>> >>> >
>> >>> > I agree that Lua and Micropython should build easy but I had more
>> >>> > in mind.
>> >>> >
>> >>> > The full project was language stacks for RTEMS with a better user
>> >>> > experience for Micropython, Lua, Tcl, etc although I am not sure what
>> >>> > etc would entail. I am not sure all three can be completed in the new
>> >>> > GSoC timeframe. All would follow the same pattern:
>> >
>> > Etc can be managed while framing the proposal according to how time is 
>> > being managed.
>> >>>
>> >>> >
>> >>> > + RSB package offering a reasonable default and access to configuration
>> >>> > + Examples including at least bare embedded, use of custom commands,
>> >>> > and integrating with RTEMS shell commands Perhaps  interactive use with
>> >>> > command line history and editing integrated if we have that as a 
>> >>> > library now.
>> >>> > + Documentation specific to RTEMS and the examples
>> >>> >
>> >>> > I imagined completely parallel kits for each embedded language we 
>> >>> > wanted
>> >>> > to support.
>> >>> >
>> >>> > Does that help? Should he plan on Micropython and Lua?
>> >>> >
>> >>>
>> >>> Sure. Lua should be easy way to get started and develop the
>> >>> framework/infrastructure side in Phase 1. Phase 2 could be extension
>> >>> to micropython / other scripting languages.
>> >
>> > Since all the languages will have a similar pattern complex work can be 
>> > put in phase 2.
>> > From my past experience, it is the part when most work is done :)
>>
>> True, but for repeat students, we do expect a bit more acceleration in
>> the first phase. Usually, we want to see code merged in phase 1 by
>> repeat students. Just a reminder that the bar is higher :)
>
> :)
>>
>>
>> >>
>> >>
>> >> OK.
>> >>>
>> >>>
>> >>> I'm not sure about the RSB design of things, and whether they should
>> >>> be parallel or capable of integration. Would anyone want to use
>> >>> multiple interpreters in the same application? If so, they should
>> >>> build together to avoid conflicts. If not, parallel is fine.
>> >
>> > building them can be set to build flags,
>> > but there still needs to be a way if we want to build the package other 
>> > than the default way.
>> > 

RE: [PATCH] covoar: Add aarch64 target

[Alex White]

On Wed, Mar 24, 2021 at 12:25 PM Gedare Bloom  wrote:
>
> On Wed, Mar 24, 2021 at 10:46 AM Alex White  wrote:
> > diff --git a/tester/covoar/Target_aarch64.cc 
> > b/tester/covoar/Target_aarch64.cc
> > new file mode 100644
> > index 000..64472d6
> > --- /dev/null
> > +++ b/tester/covoar/Target_aarch64.cc
> > @@ -0,0 +1,100 @@
> > +/*! @file Target_aarch64.cc
> > + *  @brief Target_aarch64 Implementation
> > + *
> > + *  This file contains the implementation of the base class for
> > + *  functions supporting target unique functionallity.
> only one l in functionality

See final comment below.

>
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +#include "Target_aarch64.h"
> > +
> > +namespace Target {
> > +
> > +  Target_aarch64::Target_aarch64( std::string targetName ):
> > +    TargetBase( targetName )
> > +  {
> > +    conditionalBranchInstructions.push_back("cbnz");
> > +    conditionalBranchInstructions.push_back("cbz");
> > +    conditionalBranchInstructions.push_back("tbnz");
> > +    conditionalBranchInstructions.push_back("tbz");
> > +    conditionalBranchInstructions.push_back("b.eq");
> > +    conditionalBranchInstructions.push_back("b.ne");
> > +    conditionalBranchInstructions.push_back("b.cs");
> > +    conditionalBranchInstructions.push_back("b.hs");
> > +    conditionalBranchInstructions.push_back("b.cc");
> > +    conditionalBranchInstructions.push_back("b.lo");
> > +    conditionalBranchInstructions.push_back("b.mi");
> > +    conditionalBranchInstructions.push_back("b.pl");
> > +    conditionalBranchInstructions.push_back("b.vs");
> > +    conditionalBranchInstructions.push_back("b.vc");
> > +    conditionalBranchInstructions.push_back("b.hi");
> > +    conditionalBranchInstructions.push_back("b.ls");
> > +    conditionalBranchInstructions.push_back("b.ge");
> > +    conditionalBranchInstructions.push_back("b.lt");
> > +    conditionalBranchInstructions.push_back("b.gt");
> > +    conditionalBranchInstructions.push_back("b.le");
> > +
> > +    conditionalBranchInstructions.sort();
>
> this is kind of lazy :) you could sort them as you push them.

See final comment below.

>
> > +  }
> > +
> > +  Target_aarch64::~Target_aarch64()
> > +  {
> > +  }
> > +
> > +  bool Target_aarch64::isNopLine(
> > +    const char* const line,
> > +    int&              size
> > +  )
> > +  {
> > +    if (!strcmp( [strlen(line)-3], "nop")) {
> any reason this is strcmp but the others are strncmp?

No reason, no. I should change it to strncmp.

>
> > +      size = 4;
> > +      return true;
> > +    }
> I wonder if you guys want to take a stab at using C++ strings or not?
> just a thought.
>
> This stuff how it is being done is very fickle with these constant
> numbers, but I guess the interface shouldn't change too much since it
> derives from the ISA. It just isn't clear at all how this stuff works
> by casual observation.

See final comment below.

>
> > +
> > +    if (!strncmp( [strlen(line)-6], "udf", 3)) {
> > +      size = 4;
> > +      return true;
> > +    }
> > +
> > +    // On ARM, there are literal tables at the end of methods.
> > +    // We need to avoid them.
> > +    if (!strncmp( [strlen(line)-10], ".byte", 5)) {
> > +      size = 1;
> > +      return true;
> > +    }
> > +    if (!strncmp( [strlen(line)-13], ".short", 6)) {
> > +      size = 2;
> > +      return true;
> > +    }
> > +    if (!strncmp( [strlen(line)-16], ".word", 5)) {
> > +      size = 4;
> > +      return true;
> > +    }
> > +
> > +    return false;
> > +  }
> > +
> > +  bool Target_aarch64::isBranch(
> > +      const char* instruction
> > +  )
> > +  {
> > +    throw rld::error(
> > +      "DETERMINE BRANCH INSTRUCTIONS FOR THIS ARCHITECTURE! -- fix me",
> Seems like this could be a better message. Is this the "standard"
> error message being used in covoar for this condition?

See final comment below.

>
> > +      "Target_aarch64::isBranch"
> > +    );
> > +  }
> > +
> > +  TargetBase *Target_aarch64_Constructor(
> > +    std::string          targetName
> > +  )
> > +  {
> > +    return new Target_aarch64( targetName );
> > +  }
> > +
> > +}
> > diff --git a/tester/covoar/Target_aarch64.h b/tester/covoar/Target_aarch64.h
> > new file mode 100644
> > index 000..8c15daa
> > --- /dev/null
> > +++ b/tester/covoar/Target_aarch64.h
> > @@ -0,0 +1,77 @@
> > +/*! @file Target_aarch64.h
> > + *  @brief Target_aarch64 Specification
> > + *
> > + *  This file contains the specification of the Target_aarch64 class.
> > + */
> > +
> > +#ifndef __TARGET_AARCH64_H__
> > +#define __TARGET_AARCH64_H__
> > +
> > +#include 
> > +#include 
> > +#include "TargetBase.h"
> > +
> > +namespace Target {
> > +
> > +  /*! @class Target_aarch64
> > +   *
> > +   *  This class is the Target class for the aarch64 processor.
> > +   *
> > +   */
> > +  class Target_aarch64: public TargetBase {
> > +
> > +  public:
> > +
> > +    /*!
> > +     *  This method constructs an Target_aarch64 

Re: GSoC Project : Package Micro-python

[Eshan Dhawan]

On Wed, Mar 24, 2021 at 12:34 AM Gedare Bloom  wrote:

> On Tue, Mar 23, 2021 at 12:16 PM Eshan Dhawan 
> wrote:
> >
> >
> > Apologies for the late reply.
> >
> > On Mon, Mar 22, 2021 at 10:27 PM Joel Sherrill  wrote:
> >>
> >>
> >>
> >> On Mon, Mar 22, 2021 at 11:55 AM Gedare Bloom  wrote:
> >>>
> >>> On Mon, Mar 22, 2021 at 10:50 AM Joel Sherrill  wrote:
> >>> >
> >>> >
> >>> >
> >>> > On Mon, Mar 22, 2021 at 11:30 AM Gedare Bloom 
> wrote:
> >>> >>
> >>> >> On Sat, Mar 20, 2021 at 12:33 PM Eshan Dhawan <
> eshandhawa...@gmail.com> wrote:
> >>> >> >
> >>> >> > Hello Everyone,
> >>> >> > I wanted to take Packaging Micro Python up as GSOC project this
> summer and the project will also include packaging LUA and picoC
> >>> >> > The ticket for Micro Python  :
> https://devel.rtems.org/ticket/4349
> >>> >> > What would be the complete Scope of the project?
> >>> >> > And what would be a good starting point?
> >>> >> >
> >>> >>
> >>> >> Well, I guess Joel must have described the task, so I'll leave it to
> >>> >> him to fill in some more details.
> >>> >>
> >>> >> Adding RSB packages may be not sufficient coding work for GSoC. It
> is
> >>> >> important in the proposal to identify what would be the coding
> >>> >> activities involved in this project. For example, we know from
> >>> >> experience that Lua can just be built from some minor tailoring of
> its
> >>> >> Makefile, so the package is very straightforward. However, the
> >>> >> projects you mention are scripting environments, so maybe creating a
> >>> >> framework in RTEMS for a "shell/intepreter" that can be built as an
> >>> >> add-on by RSB would be a proper way to scope this effort
> >
> > Packaging might not be a lot of coding part but adding its documentation
> and its example would be a very iterative and time consuming process.
>
> Remember that code is what counts, while we expect the other stuff to
> come along too, you don't want to be doing 90% doco and 10% code. Just
> keep it in mind.
>
What would be a good inclusion to this project ?
I was thinking long double support since I worked on porting POSIX
functions I might find it easier.
But it might interfere with matt's project if I understand that project
correctly.

>
> >>>
> >>> >
> >>> >
> >>> > I agree that Lua and Micropython should build easy but I had more
> >>> > in mind.
> >>> >
> >>> > The full project was language stacks for RTEMS with a better user
> >>> > experience for Micropython, Lua, Tcl, etc although I am not sure what
> >>> > etc would entail. I am not sure all three can be completed in the new
> >>> > GSoC timeframe. All would follow the same pattern:
> >
> > Etc can be managed while framing the proposal according to how time is
> being managed.
> >>>
> >>> >
> >>> > + RSB package offering a reasonable default and access to
> configuration
> >>> > + Examples including at least bare embedded, use of custom commands,
> >>> > and integrating with RTEMS shell commands Perhaps  interactive use
> with
> >>> > command line history and editing integrated if we have that as a
> library now.
> >>> > + Documentation specific to RTEMS and the examples
> >>> >
> >>> > I imagined completely parallel kits for each embedded language we
> wanted
> >>> > to support.
> >>> >
> >>> > Does that help? Should he plan on Micropython and Lua?
> >>> >
> >>>
> >>> Sure. Lua should be easy way to get started and develop the
> >>> framework/infrastructure side in Phase 1. Phase 2 could be extension
> >>> to micropython / other scripting languages.
> >
> > Since all the languages will have a similar pattern complex work can be
> put in phase 2.
> > From my past experience, it is the part when most work is done :)
>
> True, but for repeat students, we do expect a bit more acceleration in
> the first phase. Usually, we want to see code merged in phase 1 by
> repeat students. Just a reminder that the bar is higher :)
>
:)

>
> >>
> >>
> >> OK.
> >>>
> >>>
> >>> I'm not sure about the RSB design of things, and whether they should
> >>> be parallel or capable of integration. Would anyone want to use
> >>> multiple interpreters in the same application? If so, they should
> >>> build together to avoid conflicts. If not, parallel is fine.
> >
> > building them can be set to build flags,
> > but there still needs to be a way if we want to build the package other
> than the default way.
> > (any ideas on how to do that )
> >>
> >>
> >> I don't see any reason on our side why that shouldn't work but we
> >> can't guarantee they don't have symbol conflicts. And I'm not sure
> >> it would make much sense to integrate both at the same time.
> >>
> >> I'd think you could install both but we'd focus on only using one
> >> at a time.
> >>
> >> --joel
> >>>
> >>>
> >>> > --joel
> >>> >
> >>> >>
> >>> >> > Thanks
> >>> >> > - Eshan
> >>> >> > ___
> >>> >> > devel mailing list
> >>> >> > devel@rtems.org
> >>> >> > http://lists.rtems.org/mailman/listinfo/devel
> >>> >> 

Re: [PATCH] covoar: Add aarch64 target

[Gedare Bloom]

On Wed, Mar 24, 2021 at 10:46 AM Alex White  wrote:
>
> ---
>  tester/covoar/TargetFactory.cc  |   2 +
>  tester/covoar/Target_aarch64.cc | 100 
>  tester/covoar/Target_aarch64.h  |  77 
>  tester/covoar/wscript   |   1 +
>  4 files changed, 180 insertions(+)
>  create mode 100644 tester/covoar/Target_aarch64.cc
>  create mode 100644 tester/covoar/Target_aarch64.h
>
> diff --git a/tester/covoar/TargetFactory.cc b/tester/covoar/TargetFactory.cc
> index 12de94d..57ba686 100644
> --- a/tester/covoar/TargetFactory.cc
> +++ b/tester/covoar/TargetFactory.cc
> @@ -17,6 +17,7 @@
>
>  #include "TargetFactory.h"
>
> +#include "Target_aarch64.h"
>  #include "Target_arm.h"
>  #include "Target_i386.h"
>  #include "Target_m68k.h"
> @@ -51,6 +52,7 @@ namespace Target {
>//! All must be derived from TargetBase.
>//!
>static FactoryEntry_t FactoryTable[] = {
> +{ "aarch64", Target_aarch64_Constructor },
>  { "arm", Target_arm_Constructor },
>  { "i386",Target_i386_Constructor },
>  { "lm32",Target_lm32_Constructor },
> diff --git a/tester/covoar/Target_aarch64.cc b/tester/covoar/Target_aarch64.cc
> new file mode 100644
> index 000..64472d6
> --- /dev/null
> +++ b/tester/covoar/Target_aarch64.cc
> @@ -0,0 +1,100 @@
> +/*! @file Target_aarch64.cc
> + *  @brief Target_aarch64 Implementation
> + *
> + *  This file contains the implementation of the base class for
> + *  functions supporting target unique functionallity.
only one l in functionality

> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "Target_aarch64.h"
> +
> +namespace Target {
> +
> +  Target_aarch64::Target_aarch64( std::string targetName ):
> +TargetBase( targetName )
> +  {
> +conditionalBranchInstructions.push_back("cbnz");
> +conditionalBranchInstructions.push_back("cbz");
> +conditionalBranchInstructions.push_back("tbnz");
> +conditionalBranchInstructions.push_back("tbz");
> +conditionalBranchInstructions.push_back("b.eq");
> +conditionalBranchInstructions.push_back("b.ne");
> +conditionalBranchInstructions.push_back("b.cs");
> +conditionalBranchInstructions.push_back("b.hs");
> +conditionalBranchInstructions.push_back("b.cc");
> +conditionalBranchInstructions.push_back("b.lo");
> +conditionalBranchInstructions.push_back("b.mi");
> +conditionalBranchInstructions.push_back("b.pl");
> +conditionalBranchInstructions.push_back("b.vs");
> +conditionalBranchInstructions.push_back("b.vc");
> +conditionalBranchInstructions.push_back("b.hi");
> +conditionalBranchInstructions.push_back("b.ls");
> +conditionalBranchInstructions.push_back("b.ge");
> +conditionalBranchInstructions.push_back("b.lt");
> +conditionalBranchInstructions.push_back("b.gt");
> +conditionalBranchInstructions.push_back("b.le");
> +
> +conditionalBranchInstructions.sort();

this is kind of lazy :) you could sort them as you push them.

> +  }
> +
> +  Target_aarch64::~Target_aarch64()
> +  {
> +  }
> +
> +  bool Target_aarch64::isNopLine(
> +const char* const line,
> +int&  size
> +  )
> +  {
> +if (!strcmp( [strlen(line)-3], "nop")) {
any reason this is strcmp but the others are strncmp?

> +  size = 4;
> +  return true;
> +}
I wonder if you guys want to take a stab at using C++ strings or not?
just a thought.

This stuff how it is being done is very fickle with these constant
numbers, but I guess the interface shouldn't change too much since it
derives from the ISA. It just isn't clear at all how this stuff works
by casual observation.

> +
> +if (!strncmp( [strlen(line)-6], "udf", 3)) {
> +  size = 4;
> +  return true;
> +}
> +
> +// On ARM, there are literal tables at the end of methods.
> +// We need to avoid them.
> +if (!strncmp( [strlen(line)-10], ".byte", 5)) {
> +  size = 1;
> +  return true;
> +}
> +if (!strncmp( [strlen(line)-13], ".short", 6)) {
> +  size = 2;
> +  return true;
> +}
> +if (!strncmp( [strlen(line)-16], ".word", 5)) {
> +  size = 4;
> +  return true;
> +}
> +
> +return false;
> +  }
> +
> +  bool Target_aarch64::isBranch(
> +  const char* instruction
> +  )
> +  {
> +throw rld::error(
> +  "DETERMINE BRANCH INSTRUCTIONS FOR THIS ARCHITECTURE! -- fix me",
Seems like this could be a better message. Is this the "standard"
error message being used in covoar for this condition?

> +  "Target_aarch64::isBranch"
> +);
> +  }
> +
> +  TargetBase *Target_aarch64_Constructor(
> +std::string  targetName
> +  )
> +  {
> +return new Target_aarch64( targetName );
> +  }
> +
> +}
> diff --git a/tester/covoar/Target_aarch64.h b/tester/covoar/Target_aarch64.h
> new file mode 100644
> index 000..8c15daa
> --- /dev/null
> +++ b/tester/covoar/Target_aarch64.h
> @@ -0,0 +1,77 @@
> +/*! @file 

[PATCH] covoar: Add aarch64 target

[Alex White]

---
 tester/covoar/TargetFactory.cc  |   2 +
 tester/covoar/Target_aarch64.cc | 100 
 tester/covoar/Target_aarch64.h  |  77 
 tester/covoar/wscript   |   1 +
 4 files changed, 180 insertions(+)
 create mode 100644 tester/covoar/Target_aarch64.cc
 create mode 100644 tester/covoar/Target_aarch64.h

diff --git a/tester/covoar/TargetFactory.cc b/tester/covoar/TargetFactory.cc
index 12de94d..57ba686 100644
--- a/tester/covoar/TargetFactory.cc
+++ b/tester/covoar/TargetFactory.cc
@@ -17,6 +17,7 @@
 
 #include "TargetFactory.h"
 
+#include "Target_aarch64.h"
 #include "Target_arm.h"
 #include "Target_i386.h"
 #include "Target_m68k.h"
@@ -51,6 +52,7 @@ namespace Target {
   //! All must be derived from TargetBase.
   //!
   static FactoryEntry_t FactoryTable[] = {
+{ "aarch64", Target_aarch64_Constructor },
 { "arm", Target_arm_Constructor },
 { "i386",Target_i386_Constructor },
 { "lm32",Target_lm32_Constructor },
diff --git a/tester/covoar/Target_aarch64.cc b/tester/covoar/Target_aarch64.cc
new file mode 100644
index 000..64472d6
--- /dev/null
+++ b/tester/covoar/Target_aarch64.cc
@@ -0,0 +1,100 @@
+/*! @file Target_aarch64.cc
+ *  @brief Target_aarch64 Implementation
+ *
+ *  This file contains the implementation of the base class for
+ *  functions supporting target unique functionallity.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "Target_aarch64.h"
+
+namespace Target {
+
+  Target_aarch64::Target_aarch64( std::string targetName ):
+TargetBase( targetName )
+  {
+conditionalBranchInstructions.push_back("cbnz");
+conditionalBranchInstructions.push_back("cbz");
+conditionalBranchInstructions.push_back("tbnz");
+conditionalBranchInstructions.push_back("tbz");
+conditionalBranchInstructions.push_back("b.eq");
+conditionalBranchInstructions.push_back("b.ne");
+conditionalBranchInstructions.push_back("b.cs");
+conditionalBranchInstructions.push_back("b.hs");
+conditionalBranchInstructions.push_back("b.cc");
+conditionalBranchInstructions.push_back("b.lo");
+conditionalBranchInstructions.push_back("b.mi");
+conditionalBranchInstructions.push_back("b.pl");
+conditionalBranchInstructions.push_back("b.vs");
+conditionalBranchInstructions.push_back("b.vc");
+conditionalBranchInstructions.push_back("b.hi");
+conditionalBranchInstructions.push_back("b.ls");
+conditionalBranchInstructions.push_back("b.ge");
+conditionalBranchInstructions.push_back("b.lt");
+conditionalBranchInstructions.push_back("b.gt");
+conditionalBranchInstructions.push_back("b.le");
+
+conditionalBranchInstructions.sort();
+  }
+
+  Target_aarch64::~Target_aarch64()
+  {
+  }
+
+  bool Target_aarch64::isNopLine(
+const char* const line,
+int&  size
+  )
+  {
+if (!strcmp( [strlen(line)-3], "nop")) {
+  size = 4;
+  return true;
+}
+
+if (!strncmp( [strlen(line)-6], "udf", 3)) {
+  size = 4;
+  return true;
+}
+
+// On ARM, there are literal tables at the end of methods.
+// We need to avoid them.
+if (!strncmp( [strlen(line)-10], ".byte", 5)) {
+  size = 1;
+  return true;
+}
+if (!strncmp( [strlen(line)-13], ".short", 6)) {
+  size = 2;
+  return true;
+}
+if (!strncmp( [strlen(line)-16], ".word", 5)) {
+  size = 4;
+  return true;
+}
+
+return false;
+  }
+
+  bool Target_aarch64::isBranch(
+  const char* instruction
+  )
+  {
+throw rld::error(
+  "DETERMINE BRANCH INSTRUCTIONS FOR THIS ARCHITECTURE! -- fix me",
+  "Target_aarch64::isBranch"
+);
+  }
+
+  TargetBase *Target_aarch64_Constructor(
+std::string  targetName
+  )
+  {
+return new Target_aarch64( targetName );
+  }
+
+}
diff --git a/tester/covoar/Target_aarch64.h b/tester/covoar/Target_aarch64.h
new file mode 100644
index 000..8c15daa
--- /dev/null
+++ b/tester/covoar/Target_aarch64.h
@@ -0,0 +1,77 @@
+/*! @file Target_aarch64.h
+ *  @brief Target_aarch64 Specification
+ *
+ *  This file contains the specification of the Target_aarch64 class.
+ */
+
+#ifndef __TARGET_AARCH64_H__
+#define __TARGET_AARCH64_H__
+
+#include 
+#include 
+#include "TargetBase.h"
+
+namespace Target {
+
+  /*! @class Target_aarch64
+   *
+   *  This class is the Target class for the aarch64 processor.
+   *
+   */
+  class Target_aarch64: public TargetBase {
+
+  public:
+
+/*!
+ *  This method constructs an Target_aarch64 instance.
+ */
+Target_aarch64( std::string targetName );
+
+/*!
+ *  This method destructs an Target_aarch64 instance.
+ */
+virtual ~Target_aarch64();
+
+/*!
+ *  This method determines whether the specified line from a 
+ *  objdump file is a nop instruction.
+ *
+ *  @param[in] line contains the object dump line to check
+ *  @param[out] size is set to the size in bytes 

RE: Recent rtems-tools patches status

[Alex White]

On Wed, Mar 24, 2021 at 11:04 AM Joel Sherrill  wrote:
>
>
>
> On Wed, Mar 24, 2021 at 10:52 AM Gedare Bloom  wrote:
>>
>> On Wed, Mar 24, 2021 at 7:56 AM Alex White  wrote:
>> > My plan is to resend the patches that have not been reviewed and meter 
>> > them out so I don’t overwhelm the list. Hopefully that will allow us to 
>> > better work through this list. Is that ok?
>> >
>>
>> Yes. You and Joel may also like to track these with a spreadsheet. I'd
>> be willing to open one when asked :)
>
>
> Alex.. that's a not so subtle hint to create a Google sheet to track
> these, make it world readable, and perhaps give Gedare and I 
> permission to edit it. :)

Done. The Google sheet is at 
https://docs.google.com/spreadsheets/d/1z_ZgObFxwqf92lx3tVqtUis-zXudtwNSu-UmKogKXpI/edit?usp=sharing

>
> --joel
>>
>>
>> >
>> >
>> > Thanks,
>> >
>> >
>> >
>> > Alex
>> >
>> > ___
>> > 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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v6] cpukit/aarch64: Add ESR register decoding

[Gedare Bloom]

On Tue, Mar 23, 2021 at 2:25 PM Alex White  wrote:
>
> ---
>  .../aarch64/aarch64-exception-frame-print.c   | 135 --
>  1 file changed, 125 insertions(+), 10 deletions(-)
>
> diff --git a/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c 
> b/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
> index 59b5d06032..e207a5a81d 100644
> --- a/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
> +++ b/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
> @@ -45,10 +45,113 @@
>  #include 
>
>  #include 
> +#include 
>  #include 
>
> +typedef struct {
> +   char *s;
> +   size_t n;
> +} String_Context;
> +
> +static void _CPU_Put_char( int c, void *arg )
> +{
> +  String_Context *sctx = arg;
> +  size_t n = sctx->n;
> +
> +  if (n > 0) {
> +char *s = sctx->s;
> +*s = (char) c;
> +sctx->s = s + 1;
> +sctx->n = n - 1;
> +  }
> +}
> +
> +static void _CPU_Binary_sprintf(
> +  char *s,
> +  size_t maxlen,
> +  uint32_t num_bits,
> +  uint32_t value
> +)
> +{
> +  String_Context sctx;
> +  uint32_t mask;
> +
> +  sctx.s = s;
> +  sctx.n = maxlen;
> +
> +  mask = 1 << (num_bits - 1);
> +
> +  while ( mask != 0 ) {
> +_IO_Printf( _CPU_Put_char, , "%d", (value & mask ? 1 : 0) );
> +mask >>= 1;
> +  }
> +
> +  s[num_bits] = '\0';
> +}
> +
> +static const char* _CPU_Exception_class_to_string( uint16_t exception_class )
> +{
> +  /* The 80 character limit is intentionally ignored for these strings. */
> +  switch ( exception_class ) {
> +case 0x01:
> +  return "Trapped WFI or WFE instruction";
> +case 0x03:
> +  return "Trapped MCR or MRC access with (coproc==0b)";
> +case 0x04:
> +  return "Trapped MCRR or MRRC access with (coproc==0b)";
> +case 0x05:
> +  return "Trapped MCR or MRC access with (coproc==0b1110)";
> +case 0x06:
> +  return "Trapped LDC or STC access";
> +case 0x0c:
> +  return "Trapped MRRC access with (coproc==0b1110)";
> +case 0x0e:
> +  return "Illegal Execution state";
> +case 0x18:
> +  return "Trapped MSR, MRS, or System instruction";
> +case 0x20:
> +  return "Instruction Abort from a lower Exception level";
> +case 0x21:
> +  return "Instruction Abort taken without a change in Exception level";
> +case 0x22:
> +  return "PC alignment fault";
> +case 0x24:
> +  return "Data Abort from a lower Exception level";
> +case 0x25:
> +  return "Data Abort taken without a change in Exception level";
> +case 0x26:
> +  return "SP alignment fault";
> +case 0x30:
> +  return "Breakpoint exception from a lower Exception level";
> +case 0x31:
> +  return "Breakpoint exception taken without a change in Exception 
> level";
> +case 0x32:
> +  return "Software Step exception from a lower Exception level";
> +case 0x33:
> +  return "Software Step exception taken without a change in Exception 
> level";
> +case 0x34:
> +  return "Watchpoint exception from a lower Exception level";
> +case 0x35:
> +  return "Watchpoint exception taken without a change in Exception 
> level";
> +case 0x38:
> +  return "BKPT instruction execution in AArch32 state";
> +case 0x3c:
> +  return "BRK instruction execution in AArch64 state";
> +default:
> +  return "Unknown";
> +  }
> +}
> +
>  void _CPU_Exception_frame_print( const CPU_Exception_frame *frame )
>  {
> +  uint32_t ec;
> +  uint32_t il;
> +  uint32_t iss;
> +  char ec_str[7];
> +  char iss_str[26];
> +  int  i;
> +  const uint128_t *qx;
> +
>printk(
>  "\n"
>  "X0   = 0x%016" PRIx64  " X17  = 0x%016" PRIx64 "\n"
> @@ -68,8 +171,7 @@ void _CPU_Exception_frame_print( const CPU_Exception_frame 
> *frame )
>  "X14  = 0x%016" PRIx64  " SP   = 0x%016" PRIxPTR "\n"
>  "X15  = 0x%016" PRIx64  " PC   = 0x%016" PRIxPTR "\n"
>  "X16  = 0x%016" PRIx64  " DAIF = 0x%016" PRIx64 "\n"
> -"VEC  = 0x%016" PRIxPTR " CPSR = 0x%016" PRIx64 "\n"
> -"ESR  = 0x%016" PRIx64  " FAR  = 0x%016" PRIx64 "\n",
> +"VEC  = 0x%016" PRIxPTR " CPSR = 0x%016" PRIx64 "\n",
>  frame->register_x0, frame->register_x17,
>  frame->register_x1, frame->register_x18,
>  frame->register_x2, frame->register_x19,
> @@ -83,23 +185,36 @@ void _CPU_Exception_frame_print( const 
> CPU_Exception_frame *frame )
>  frame->register_x10, frame->register_x27,
>  frame->register_x11, frame->register_x28,
>  frame->register_x12, frame->register_fp,
> -frame->register_x13, (intptr_t)frame->register_lr,
> -frame->register_x14, (intptr_t)frame->register_sp,
> -frame->register_x15, (intptr_t)frame->register_pc,
> +frame->register_x13, (intptr_t) frame->register_lr,
> +frame->register_x14, (intptr_t) frame->register_sp,
> +frame->register_x15, (intptr_t) frame->register_pc,
>  frame->register_x16, frame->register_daif,
> -(intptr_t) frame->vector, frame->register_cpsr,
> -

Re: Recent rtems-tools patches status

[Joel Sherrill]

On Wed, Mar 24, 2021 at 10:52 AM Gedare Bloom  wrote:

> On Wed, Mar 24, 2021 at 7:56 AM Alex White  wrote:
> >
> > Hi,
> >
> >
> >
> > Here is the status of the rtems-tools patches I have sent out over the
> past few weeks (an X means the latest patch revision has been reviewed):
> >
> >
> >
> > [ ] tester: Limit branch coverage percentage precision
> >
> > [ ] coverage: Fix option processing on FreeBSD
> >
> > [ ] coverage/symbol-sets.ini : Add libtrace
> >
> > [ ] covoar/Reports: Fix empty branch report
> >
> > [ ] covoar: Fix overflow of high PC address
> >
> > [ ] covoar: Catch exceptional case
> >
> > [ ] covoar: Add option to create named objdumps
> >
> > [X] covoar: Fix null pointer dereference
> >
> > [X] coverage: Give coverage bars red background
> >
> > [X] coverage/reports: Share common JS and CSS in reports
> >
> > [X] coverage/reports: Improve formatting and clarity
> >
> > [X] covoar/reports: Add new statistics to summary
> >
> > [ ] covoar: Handle periods in symbols from objdump
> >
> > [ ] covoar: Account for build path change
> >
> > [ ] covoar: Fix DWARF reading
> >
> > [ ] covoar/TargetBase: Fix QEMU branch info
> >
> > [ ] covoar/CoverageReaderQEMU: Fix infinite loop
> >
> > [ ] covoar/Target_arm: Add THUMB branch instructions
> >
> > [ ] covoar/Target_i386: Add NOP patterns
> >
> > [ ] covoar: Fix NOP execution marking
> >
> > [X] tester: Add coverage variants for a few BSPs
> >
> > [X] tester: Remove target from BSP coverage configs
> >
> > [X] tester: Update to support new build system
> >
> > [ ] covoar: Add aarch64 target
> >
> > [X] covoar/TargetBase: Rename branchInstructions to
> conditionalBranchInstructions
> >
> > [X] rld-process: Add named tempfile constructor
> >
> > [X] rld-dwarf: Fix file::get_source
> >
> > [X] rld-dwarf: Add function::has_entry_pc
> >
> >
> >
> > I sent these patches as a firehose, so I am not surprised that some
> didn’t get a response. :)
> >
> >
> >
> > My plan is to resend the patches that have not been reviewed and meter
> them out so I don’t overwhelm the list. Hopefully that will allow us to
> better work through this list. Is that ok?
> >
>
> Yes. You and Joel may also like to track these with a spreadsheet. I'd
> be willing to open one when asked :)
>

Alex.. that's a not so subtle hint to create a Google sheet to track
these, make it world readable, and perhaps give Gedare and I
permission to edit it. :)

--joel

>
> >
> >
> > Thanks,
> >
> >
> >
> > Alex
> >
> > ___
> > 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: #4328: New APIs added to POSIX Standard (2021)

[Joel Sherrill]

Wow! Good work. There is a lot to digest here. Comments interspersed.

I assume the spreadsheet is updated.

On Wed, Mar 24, 2021 at 7:38 AM Matthew Joyce  wrote:

> Hi Dr. Joel,
>
> I've gone over the list a few times now and see a few categories shaping
> up:
>
> 1) Already done (In Newlib source, defined in libc.a):
> a) reallocarray
> b) qsort_r
> c) memmem
> d) strlcat / strlcpy
> d) wcslcat / wcslcpy
> *Out of this group, strlcat and strlcpy also show up in
> src/rtems/cpukit. Why is that?
>

The good news is that we support these. :)

It looks to me that strlcat and strlcpy are used in cpukit but not
implemented
there. Where do you think they are implemented.

This is a good example where a source code browser is helpful. grep can
often answer the question but a source code browser can be easier.
Personally,
I use cscope but that is exceedingly old school. Any modern IDE should be
helpful.


> 2) Not done yet (Do not show up in Newlib source or RTEMS):
> a) getlocalename_l
> b) posix_getdents
> c) sem_clockwait
> d) sig2str / str2sig
>
> 3) Not in Newlib; Referenced in RTEMS but hidden behind #ifdef:
> a) pthread_cond_clockwait
> (rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/condition_variable)
> b) pthread_mutex_clocklock
> (rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/mutex)
> c) pthread_rwlock_clockrdlock
> (rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/shared_mutex)
> c) pthread_rwlock_clockwrlock
> (rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/shared_mutex)
> *It looks like some groundwork was done, but the methods are not yet
> supported.
>

The paths you point to are C++ files that would implement C++ features
using the available POSIX services. So they are users, not providers.

All of the pthread services related to these are implemented in
cpukit/posix/src. I think you can configure a clock for all these now
to be used by detailed on wait and timedwait calls. My understanding
is that these would let you use a specific clock on a per blocking call
basis.

First question is which clocks are intended to be supported.

Second is the pattern of picking which timeout queue to go on when
now it is coded to let you pick one which is used for the life of the
object.


> 4) Misc (In Newlib source, not defined in libc.a, appear in RTEMS in
> various ways)
> a) getentropy (an alternate version is defined in RTEMS librtemsbsd.a,
> in src/rtems/bsps/shared/dev/getentropy/getentropy-cpucounter.c. The
> comments note that it is not cryptographically secure, so it may not
> fit the bill for the getentropy() mentioned in the Open Group
> document)
>

I am far from a cryptography expert but this looks like a case where
this method would be considered supported with the disclaimer that
the quality of the entropy value depends on the BSP. If the user has
specific requirements, they will need to verify the implementation
used by the BSP by default is appropriate.


> b) ppoll (appears in rtems/6/share/gdb/syscalls)
>

You need to be more careful with the grep. These again are in the
installed tools and in this case, they appear in an XML file. Referenced
but not implemented.

ppoll() will need to come from rtems-libbsd. The required system call
is included but disabled currently. AFAIK this means it is possible to
provide this but that would require a more detailed discussion in case
some underlying capability is missing. Chris Johns and Sebastian
Huber would be the ones to guide here.

Ruling: Likely possible.


> c) dladdr (appears in rtems/cpukit but not defined)
>

I think this can be implemented in libdl but I am not sure if the
code from NetBSD from this would directly work or just be a guide.


>
> 5) Others?
> It looks like there was work done on methods like sockatmark and
> pselect, but I don't see them supported as yet. Should those be added
> to the list or are they still being worked on?
>

These would come from rtems-libbsd.

I think sockatmark.c is implemented in freebsd/lib/libc/net/sockatmark.c
but I don't know if the ioctl() is implemented. I expect it is but this
would
at least require a test. It may just work.

pselect() looks to be missing and would have to be ported from FreeBSD.


> As you suggested, I'll look into NetBSD for dladdr and do some digging
> on the implementation of the other outstanding methods. You mentioned
> that the "clock" ones have to be strictly added to rtems/cpukit, but
> the references I found above are all in lib/gcc/sparc-rtems6/10.2.1.
> Why is that the case and what is 10.2.1? Also, I'm not sure what to
> make of getentropy and ppoll based on what I found above...at your
> convenience could you please advise?
>

Hopefully the above helped.

You don't have to restrict your possible set to these new additions.
There are others. I think Eshan has done the research for where to
get implementations of the missing long double methods for newlib.
And there are tickets for other missing methods or specific capabilities
in methods that are supported. Those 

Re: [PATCH v2 0/2] Fix QEMU branch analysis

[Gedare Bloom]

these two patches look ok

On Wed, Mar 24, 2021 at 7:45 AM Alex White  wrote:
>
> v2:
> - Revert change to ENTRIES macro in CoverageReaderQEMU.cc
>
> This patch set contains a couple of fixes for issues that were found
> while using covoar to analyze branch coverage in QEMU.
>
> Alex White (2):
>   covoar/CoverageReaderQEMU: Fix infinite loop
>   covoar/TargetBase: Fix QEMU branch info
>
>  tester/covoar/CoverageReaderQEMU.cc | 9 -
>  tester/covoar/TargetBase.cc | 4 ++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> --
> 2.27.0
>
> ___
> 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 2/2] rtems: Simplify rtems_semaphore_set_priority()

[Gedare Bloom]

these two patches look ok to me.

On Wed, Mar 24, 2021 at 1:40 AM Sebastian Huber
 wrote:
>
> Do not write to the object referenced by old_priority in error paths.
> This is in line with other directives.
> ---
>  cpukit/rtems/src/semsetpriority.c | 47 +++
>  1 file changed, 22 insertions(+), 25 deletions(-)
>
> diff --git a/cpukit/rtems/src/semsetpriority.c 
> b/cpukit/rtems/src/semsetpriority.c
> index 119dd85d77..97e53c6584 100644
> --- a/cpukit/rtems/src/semsetpriority.c
> +++ b/cpukit/rtems/src/semsetpriority.c
> @@ -29,20 +29,6 @@
>  #include 
>  #include 
>
> -static rtems_status_code _Semaphore_Is_scheduler_valid(
> -  const CORE_ceiling_mutex_Control *the_mutex,
> -  const Scheduler_Control  *scheduler
> -)
> -{
> -#if defined(RTEMS_SMP)
> -  if ( scheduler != _CORE_ceiling_mutex_Get_scheduler( the_mutex ) ) {
> -return RTEMS_NOT_DEFINED;
> -  }
> -#endif
> -
> -  return RTEMS_SUCCESSFUL;
> -}
> -
>  static rtems_status_code _Semaphore_Set_priority(
>Semaphore_Control   *the_semaphore,
>const Scheduler_Control *scheduler,
> @@ -51,7 +37,6 @@ static rtems_status_code _Semaphore_Set_priority(
>Thread_queue_Context*queue_context
>  )
>  {
> -  rtems_status_code  sc;
>bool   valid;
>Priority_Control   core_priority;
>Priority_Control   old_priority;
> @@ -73,16 +58,26 @@ static rtems_status_code _Semaphore_Set_priority(
>
>switch ( variant ) {
>  case SEMAPHORE_VARIANT_MUTEX_PRIORITY_CEILING:
> -  sc = _Semaphore_Is_scheduler_valid(
> -_semaphore->Core_control.Mutex,
> -scheduler
> -  );
> +#if defined(RTEMS_SMP)
> +  if (
> +scheduler != _CORE_ceiling_mutex_Get_scheduler(
> +  _semaphore->Core_control.Mutex
> +)
> +  ) {
> +_Thread_queue_Release(
> +  _semaphore->Core_control.Wait_queue,
> +  queue_context
> +);
> +
> +return RTEMS_NOT_DEFINED;
> +  }
> +#endif
>
>old_priority = _CORE_ceiling_mutex_Get_priority(
>  _semaphore->Core_control.Mutex
>);
>
> -  if ( sc == RTEMS_SUCCESSFUL && new_priority != RTEMS_CURRENT_PRIORITY 
> ) {
> +  if ( new_priority != RTEMS_CURRENT_PRIORITY ) {
>  _CORE_ceiling_mutex_Set_priority(
>_semaphore->Core_control.Mutex,
>core_priority
> @@ -105,7 +100,6 @@ static rtems_status_code _Semaphore_Set_priority(
>  );
>}
>
> -  sc = RTEMS_SUCCESSFUL;
>break;
>  #endif
>  default:
> @@ -115,9 +109,12 @@ static rtems_status_code _Semaphore_Set_priority(
>|| variant == SEMAPHORE_VARIANT_SIMPLE_BINARY
>|| variant == SEMAPHORE_VARIANT_COUNTING
>);
> -  old_priority = 0;
> -  sc = RTEMS_NOT_DEFINED;
> -  break;
> +  _Thread_queue_Release(
> +_semaphore->Core_control.Wait_queue,
> +queue_context
> +  );
> +
> +  return RTEMS_NOT_DEFINED;
>}
>
>cpu_self = _Thread_queue_Dispatch_disable( queue_context );
> @@ -129,7 +126,7 @@ static rtems_status_code _Semaphore_Set_priority(
>_Thread_Dispatch_enable( cpu_self );
>
>*old_priority_p = _RTEMS_Priority_From_core( scheduler, old_priority );
> -  return sc;
> +  return RTEMS_SUCCESSFUL;
>  }
>
>  rtems_status_code rtems_semaphore_set_priority(
> --
> 2.26.2
>
> ___
> 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: Recent rtems-tools patches status

[Gedare Bloom]

On Wed, Mar 24, 2021 at 7:56 AM Alex White  wrote:
>
> Hi,
>
>
>
> Here is the status of the rtems-tools patches I have sent out over the past 
> few weeks (an X means the latest patch revision has been reviewed):
>
>
>
> [ ] tester: Limit branch coverage percentage precision
>
> [ ] coverage: Fix option processing on FreeBSD
>
> [ ] coverage/symbol-sets.ini : Add libtrace
>
> [ ] covoar/Reports: Fix empty branch report
>
> [ ] covoar: Fix overflow of high PC address
>
> [ ] covoar: Catch exceptional case
>
> [ ] covoar: Add option to create named objdumps
>
> [X] covoar: Fix null pointer dereference
>
> [X] coverage: Give coverage bars red background
>
> [X] coverage/reports: Share common JS and CSS in reports
>
> [X] coverage/reports: Improve formatting and clarity
>
> [X] covoar/reports: Add new statistics to summary
>
> [ ] covoar: Handle periods in symbols from objdump
>
> [ ] covoar: Account for build path change
>
> [ ] covoar: Fix DWARF reading
>
> [ ] covoar/TargetBase: Fix QEMU branch info
>
> [ ] covoar/CoverageReaderQEMU: Fix infinite loop
>
> [ ] covoar/Target_arm: Add THUMB branch instructions
>
> [ ] covoar/Target_i386: Add NOP patterns
>
> [ ] covoar: Fix NOP execution marking
>
> [X] tester: Add coverage variants for a few BSPs
>
> [X] tester: Remove target from BSP coverage configs
>
> [X] tester: Update to support new build system
>
> [ ] covoar: Add aarch64 target
>
> [X] covoar/TargetBase: Rename branchInstructions to 
> conditionalBranchInstructions
>
> [X] rld-process: Add named tempfile constructor
>
> [X] rld-dwarf: Fix file::get_source
>
> [X] rld-dwarf: Add function::has_entry_pc
>
>
>
> I sent these patches as a firehose, so I am not surprised that some didn’t 
> get a response. :)
>
>
>
> My plan is to resend the patches that have not been reviewed and meter them 
> out so I don’t overwhelm the list. Hopefully that will allow us to better 
> work through this list. Is that ok?
>

Yes. You and Joel may also like to track these with a spreadsheet. I'd
be willing to open one when asked :)

>
>
> Thanks,
>
>
>
> Alex
>
> ___
> 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: [rtems commit] gen_uuid.c: Fix two Unchecked return value from library errors

[Gedare Bloom]

On Wed, Mar 24, 2021 at 7:03 AM Joel Sherrill  wrote:
>
> I'm reverting this.
>
> I think this is in a batch that did not get tested before I worked
> with Ryan to correct his procedure.to ensure more testing with
> debug enabled.
>
> On most he has filed issues with the upstream, but as you note,
> this isn't the right solution so that doesn't matter here. Probably
> best to ifndef rtems around the unsupported features.
>
> I'm about to push a revert and reopen the ticket.
>
Thanks.

> --joel
>
> On Wed, Mar 24, 2021 at 2:31 AM Sebastian Huber 
>  wrote:
>>
>> On 08/03/2021 21:56, Joel Sherrill wrote:
>>
>> > Module:rtems
>> > Branch:master
>> > Commit:597e4f476568a225d14dfaff02074cf269ad62ac
>> > Changeset:http://git.rtems.org/rtems/commit/?id=597e4f476568a225d14dfaff02074cf269ad62ac
>> >
>> > Author:Ryan Long
>> > Date:  Tue Mar  2 11:08:28 2021 -0500
>> >
>> > gen_uuid.c: Fix two Unchecked return value from library errors
>> >
>> > CID 1049146: Unchecked return value from library in get_clock().
>> > CID 1049147: Unchecked return value from library in get_random_fd().
>> >
>> > Closes #4280
>> >
>> > ---
>> >
>> >   cpukit/libmisc/uuid/gen_uuid.c | 11 ---
>> >   1 file changed, 8 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/cpukit/libmisc/uuid/gen_uuid.c 
>> > b/cpukit/libmisc/uuid/gen_uuid.c
>> > index 3ca75a0..5bb34c0 100644
>> > --- a/cpukit/libmisc/uuid/gen_uuid.c
>> > +++ b/cpukit/libmisc/uuid/gen_uuid.c
>> > @@ -155,6 +155,7 @@ static int get_random_fd(void)
>> >   struct timeval  tv;
>> >   static int  fd = -2;
>> >   int i;
>> > + int sc;
>> >
>> >   if (fd == -2) {
>> >   gettimeofday(, 0);
>> > @@ -164,8 +165,10 @@ static int get_random_fd(void)
>> >   fd = open("/dev/random", O_RDONLY | O_NONBLOCK);
>> >   if (fd >= 0) {
>> >   i = fcntl(fd, F_GETFD);
>> > - if (i >= 0)
>> > - fcntl(fd, F_SETFD, i | FD_CLOEXEC);
>> > + if (i >= 0) {
>> > + sc = fcntl(fd, F_SETFD, i | FD_CLOEXEC);
>> > + _Assert_Unused_variable_unequal(sc, -1);
>> > + }
>>
>> FD_CLOEXEC is not supported by RTEMS. Do we even have these device files
>> in RTEMS? This is 3rd-party code, what about upstream changes?
>>
>> >   }
>> >   #endif
>> >   srand((getpid() << ((sizeof(pid_t)*CHAR_BIT)>>1)) ^ getuid() 
>> > ^ tv.tv_sec ^ tv.tv_usec);
>> > @@ -334,6 +337,7 @@ static int get_clock(uint32_t *clock_high, uint32_t 
>> > *clock_low,
>> >   uint64_tclock_reg;
>> >   mode_t  save_umask;
>> >   int len;
>> > + int sc;
>> >
>> >   if (state_fd == -2) {
>> >   save_umask = umask(0);
>> > @@ -426,7 +430,8 @@ try_again:
>> >   }
>> >   rewind(state_f);
>> >   fl.l_type = F_UNLCK;
>> > - fcntl(state_fd, F_SETLK, );
>> > + sc = fcntl(state_fd, F_SETLK, );
>> > + _Assert_Unused_variable_unequal(sc, -1);
>> F_SETLK is not supported by RTEMS.
>> >   }
>> >
>> >   *clock_high = clock_reg >> 32;
>>
>> The patch produces warnings like this:
>>
>> ../../../cpukit/libmisc/uuid/gen_uuid.c: In function 'get_clock':
>> ../../../cpukit/libmisc/uuid/gen_uuid.c:434:3: warning: implicit
>> declaration of function '_Assert_Unused_variable_unequal'
>> [-Wimplicit-function-declaration]
>>434 |   _Assert_Unused_variable_unequal(sc, -1);
>>|   ^~~
>> ../../../cpukit/libmisc/uuid/gen_uuid.c:434:3: warning: nested extern
>> declaration of '_Assert_Unused_variable_unequal' [-Wnested-externs]
>>
>> In total, how was this patch tested?
>>
>> If you add _Assert() stuff, please build with RTEMS_DEBUG enabled and
>> run the tests.
>>
>> --
>> embedded brains GmbH
>> Herr Sebastian HUBER
>> Dornierstr. 4
>> 82178 Puchheim
>> Germany
>> email: sebastian.hu...@embedded-brains.de
>> phone: +49-89-18 94 741 - 16
>> fax:   +49-89-18 94 741 - 08
>>
>> Registergericht: Amtsgericht München
>> Registernummer: HRB 157899
>> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
>> Unsere Datenschutzerklärung finden Sie hier:
>> https://embedded-brains.de/datenschutzerklaerung/
>>
>> ___
>> 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

Recent rtems-tools patches status

[Alex White]

Hi,

Here is the status of the rtems-tools patches I have sent out over the past few 
weeks (an X means the latest patch revision has been reviewed):

[ ] tester: Limit branch coverage percentage precision
[ ] coverage: Fix option processing on FreeBSD
[ ] coverage/symbol-sets.ini : Add libtrace
[ ] covoar/Reports: Fix empty branch report
[ ] covoar: Fix overflow of high PC address
[ ] covoar: Catch exceptional case
[ ] covoar: Add option to create named objdumps
[X] covoar: Fix null pointer dereference
[X] coverage: Give coverage bars red background
[X] coverage/reports: Share common JS and CSS in reports
[X] coverage/reports: Improve formatting and clarity
[X] covoar/reports: Add new statistics to summary
[ ] covoar: Handle periods in symbols from objdump
[ ] covoar: Account for build path change
[ ] covoar: Fix DWARF reading
[ ] covoar/TargetBase: Fix QEMU branch info
[ ] covoar/CoverageReaderQEMU: Fix infinite loop
[ ] covoar/Target_arm: Add THUMB branch instructions
[ ] covoar/Target_i386: Add NOP patterns
[ ] covoar: Fix NOP execution marking
[X] tester: Add coverage variants for a few BSPs
[X] tester: Remove target from BSP coverage configs
[X] tester: Update to support new build system
[ ] covoar: Add aarch64 target
[X] covoar/TargetBase: Rename branchInstructions to 
conditionalBranchInstructions
[X] rld-process: Add named tempfile constructor
[X] rld-dwarf: Fix file::get_source
[X] rld-dwarf: Add function::has_entry_pc

I sent these patches as a firehose, so I am not surprised that some didn't get 
a response. :)

My plan is to resend the patches that have not been reviewed and meter them out 
so I don't overwhelm the list. Hopefully that will allow us to better work 
through this list. Is that ok?

Thanks,

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

[PATCH v2 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

[Alex White]

There was a potential that the branch info loop never terminated.
This has been fixed by adding a more reliable termination condition
and logging an error if it cannot find the branch target.
---
 tester/covoar/CoverageReaderQEMU.cc | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tester/covoar/CoverageReaderQEMU.cc 
b/tester/covoar/CoverageReaderQEMU.cc
index 7c344e4..d3c6abe 100644
--- a/tester/covoar/CoverageReaderQEMU.cc
+++ b/tester/covoar/CoverageReaderQEMU.cc
@@ -118,8 +118,15 @@ namespace Coverage {
 // Determine if additional branch information is available.
 if ( (entry->op & branchInfo) != 0 ) {
   uint32_t  a = entry->pc + entry->size - 1;
-while (!aCoverageMap->isStartOfInstruction(a))
+while (a > entry->pc && !aCoverageMap->isStartOfInstruction(a))
   a--;
+if (a == entry->pc && !aCoverageMap->isStartOfInstruction(a)) {
+  // Something went wrong parsing the objdump.
+  std::ostringstream what;
+  what << "Reached beginning of range in " << file
+<< " at " << entry->pc << " with no start of instruction.";
+  throw rld::error( what, "CoverageReaderQEMU::processFile" );
+}
 if (entry->op & taken) {
   aCoverageMap->setWasTaken( a );
 } else if (entry->op & notTaken) {
-- 
2.27.0

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

[PATCH v2 2/2] covoar/TargetBase: Fix QEMU branch info

[Alex White]

The taken/not taken bit was being interpreted incorrectly. This led
to branches being marked "always taken" when they were never taken.
This has been fixed.
---
 tester/covoar/TargetBase.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tester/covoar/TargetBase.cc b/tester/covoar/TargetBase.cc
index 4474fad..c11129b 100644
--- a/tester/covoar/TargetBase.cc
+++ b/tester/covoar/TargetBase.cc
@@ -130,12 +130,12 @@ namespace Target {
 
   uint8_t TargetBase::qemuTakenBit(void)
   {
-return TRACE_OP_BR0;
+return TRACE_OP_BR1;
   }
 
   uint8_t TargetBase::qemuNotTakenBit(void)
   {
-return TRACE_OP_BR1;
+return TRACE_OP_BR0;
   }
 
 }
-- 
2.27.0

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

[PATCH v2 0/2] Fix QEMU branch analysis

[Alex White]

v2:
- Revert change to ENTRIES macro in CoverageReaderQEMU.cc

This patch set contains a couple of fixes for issues that were found
while using covoar to analyze branch coverage in QEMU.

Alex White (2):
  covoar/CoverageReaderQEMU: Fix infinite loop
  covoar/TargetBase: Fix QEMU branch info

 tester/covoar/CoverageReaderQEMU.cc | 9 -
 tester/covoar/TargetBase.cc | 4 ++--
 2 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.27.0

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

Re: [rtems commit] gen_uuid.c: Fix two Unchecked return value from library errors

[Joel Sherrill]

I'm reverting this.

I think this is in a batch that did not get tested before I worked
with Ryan to correct his procedure.to ensure more testing with
debug enabled.

On most he has filed issues with the upstream, but as you note,
this isn't the right solution so that doesn't matter here. Probably
best to ifndef rtems around the unsupported features.

I'm about to push a revert and reopen the ticket.

--joel

On Wed, Mar 24, 2021 at 2:31 AM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> On 08/03/2021 21:56, Joel Sherrill wrote:
>
> > Module:rtems
> > Branch:master
> > Commit:597e4f476568a225d14dfaff02074cf269ad62ac
> > Changeset:
> http://git.rtems.org/rtems/commit/?id=597e4f476568a225d14dfaff02074cf269ad62ac
> >
> > Author:Ryan Long
> > Date:  Tue Mar  2 11:08:28 2021 -0500
> >
> > gen_uuid.c: Fix two Unchecked return value from library errors
> >
> > CID 1049146: Unchecked return value from library in get_clock().
> > CID 1049147: Unchecked return value from library in get_random_fd().
> >
> > Closes #4280
> >
> > ---
> >
> >   cpukit/libmisc/uuid/gen_uuid.c | 11 ---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/cpukit/libmisc/uuid/gen_uuid.c
> b/cpukit/libmisc/uuid/gen_uuid.c
> > index 3ca75a0..5bb34c0 100644
> > --- a/cpukit/libmisc/uuid/gen_uuid.c
> > +++ b/cpukit/libmisc/uuid/gen_uuid.c
> > @@ -155,6 +155,7 @@ static int get_random_fd(void)
> >   struct timeval  tv;
> >   static int  fd = -2;
> >   int i;
> > + int sc;
> >
> >   if (fd == -2) {
> >   gettimeofday(, 0);
> > @@ -164,8 +165,10 @@ static int get_random_fd(void)
> >   fd = open("/dev/random", O_RDONLY | O_NONBLOCK);
> >   if (fd >= 0) {
> >   i = fcntl(fd, F_GETFD);
> > - if (i >= 0)
> > - fcntl(fd, F_SETFD, i | FD_CLOEXEC);
> > + if (i >= 0) {
> > + sc = fcntl(fd, F_SETFD, i | FD_CLOEXEC);
> > + _Assert_Unused_variable_unequal(sc, -1);
> > + }
>
> FD_CLOEXEC is not supported by RTEMS. Do we even have these device files
> in RTEMS? This is 3rd-party code, what about upstream changes?
>
> >   }
> >   #endif
> >   srand((getpid() << ((sizeof(pid_t)*CHAR_BIT)>>1)) ^
> getuid() ^ tv.tv_sec ^ tv.tv_usec);
> > @@ -334,6 +337,7 @@ static int get_clock(uint32_t *clock_high, uint32_t
> *clock_low,
> >   uint64_tclock_reg;
> >   mode_t  save_umask;
> >   int len;
> > + int sc;
> >
> >   if (state_fd == -2) {
> >   save_umask = umask(0);
> > @@ -426,7 +430,8 @@ try_again:
> >   }
> >   rewind(state_f);
> >   fl.l_type = F_UNLCK;
> > - fcntl(state_fd, F_SETLK, );
> > + sc = fcntl(state_fd, F_SETLK, );
> > + _Assert_Unused_variable_unequal(sc, -1);
> F_SETLK is not supported by RTEMS.
> >   }
> >
> >   *clock_high = clock_reg >> 32;
>
> The patch produces warnings like this:
>
> ../../../cpukit/libmisc/uuid/gen_uuid.c: In function 'get_clock':
> ../../../cpukit/libmisc/uuid/gen_uuid.c:434:3: warning: implicit
> declaration of function '_Assert_Unused_variable_unequal'
> [-Wimplicit-function-declaration]
>434 |   _Assert_Unused_variable_unequal(sc, -1);
>|   ^~~
> ../../../cpukit/libmisc/uuid/gen_uuid.c:434:3: warning: nested extern
> declaration of '_Assert_Unused_variable_unequal' [-Wnested-externs]
>
> In total, how was this patch tested?
>
> If you add _Assert() stuff, please build with RTEMS_DEBUG enabled and
> run the tests.
>
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.hu...@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
>
> ___
> 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: #4328: New APIs added to POSIX Standard (2021)

[Matthew Joyce]

Hi Dr. Joel,

I've gone over the list a few times now and see a few categories shaping up:

1) Already done (In Newlib source, defined in libc.a):
a) reallocarray
b) qsort_r
c) memmem
d) strlcat / strlcpy
d) wcslcat / wcslcpy
*Out of this group, strlcat and strlcpy also show up in
src/rtems/cpukit. Why is that?

2) Not done yet (Do not show up in Newlib source or RTEMS):
a) getlocalename_l
b) posix_getdents
c) sem_clockwait
d) sig2str / str2sig

3) Not in Newlib; Referenced in RTEMS but hidden behind #ifdef:
a) pthread_cond_clockwait
(rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/condition_variable)
b) pthread_mutex_clocklock
(rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/mutex)
c) pthread_rwlock_clockrdlock
(rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/shared_mutex)
c) pthread_rwlock_clockwrlock
(rtems/6/lib/gcc/sparc-rtems6/10.2.1/include/c++/shared_mutex)
*It looks like some groundwork was done, but the methods are not yet supported.

4) Misc (In Newlib source, not defined in libc.a, appear in RTEMS in
various ways)
a) getentropy (an alternate version is defined in RTEMS librtemsbsd.a,
in src/rtems/bsps/shared/dev/getentropy/getentropy-cpucounter.c. The
comments note that it is not cryptographically secure, so it may not
fit the bill for the getentropy() mentioned in the Open Group
document)
b) ppoll (appears in rtems/6/share/gdb/syscalls)
c) dladdr (appears in rtems/cpukit but not defined)

5) Others?
It looks like there was work done on methods like sockatmark and
pselect, but I don't see them supported as yet. Should those be added
to the list or are they still being worked on?

As you suggested, I'll look into NetBSD for dladdr and do some digging
on the implementation of the other outstanding methods. You mentioned
that the "clock" ones have to be strictly added to rtems/cpukit, but
the references I found above are all in lib/gcc/sparc-rtems6/10.2.1.
Why is that the case and what is 10.2.1? Also, I'm not sure what to
make of getentropy and ppoll based on what I found above...at your
convenience could you please advise?

Thank you very much!

Matt




On Sun, Mar 21, 2021 at 6:38 PM Joel Sherrill  wrote:
>
>
>
> On Sun, Mar 21, 2021 at 2:28 AM Matthew Joyce  wrote:
>>
>> Gentlemen,
>>
>> Awesome, thanks!  I see how that works now...I'll give it a thorough
>> look tomorrow and will update the spreadsheet accordingly. I'll pipe
>> back up when I have a more accurate look of what's currently there.
>
>
> Knowing what doesn't have to be done is the first step. (rtems, newlib, and 
> libbsd)
>
> I'd be prone to look for things that are easy to add first.
>
> Some may not be implementable on RTEMS due to only supporting a
> single process and no virtual memory. If you have doubts on whether it
> is possible to support a specific method, speak up and let's try to decide.
>
> Then find upstream places for an implementation where possible. I suspect
> all the new "clock" methods will require discussion on an implementation
> pattern but those must strictly be added to rtems/cpukit with tests and
> documentation. At least I can throw you that much. :)
>
>>
>> Thanks again and have a great Sunday!
>>
>> Matt
>>
>> On Fri, Mar 19, 2021 at 8:27 PM Joel Sherrill  wrote:
>> >
>> >
>> >
>> > On Fri, Mar 19, 2021 at 1:08 PM Gedare Bloom  wrote:
>> >>
>> >> On Fri, Mar 19, 2021 at 11:16 AM Matthew Joyce  
>> >> wrote:
>> >> >
>> >> > Dr. Joel,
>> >> >
>> >> > Thanks very much...I'll keep working to get a sense of what goes
>> >> > where! In the meantime, where can I look to get the ground truth of
>> >> > which methods are "in RTEMS" as opposed to those in newlib?
>> >> >
>> >> There is only one ground truth:
>> >> git://git.rtems.org/rtems.git
>> >>
>> >> And for newlib
>> >>
>> >> git://sourceware.org/git/newlib-cygwin.git
>> >>
>> >> That said, searching for the function name symbols in compiled
>> >> libraries is a good first step to rule out newlib. Then, you can
>> >> 'grep' the RTEMS source code for the function names to see if they
>> >> exist there.
>> >
>> >
>> > rtems/cpukit to be specitic. It won't be implemented anywhere else.
>> >
>> > And clearly we both have forgotten that networking APIs are in the
>> > rtems-libbsd repository.
>> >
>> > https://git.rtems.org/rtems-libbsd/
>> >
>> > I suspect ppoll() might already be in there. Or at least supported by
>> > FreeBSD.
>> >
>> > You should clone everything and grep the sources. newlib already has
>> > qsort_r. This is the nm I used:
>> >
>> > $ ~/rtems-work/tools/6/bin/sparc-rtems6-nm 
>> > ~/rtems-work/tools/6/sparc-rtems6/lib/libc.a | grep qsort_r
>> > lib_a-bsd_qsort_r.o:
>> >  T __bsd_qsort_r
>> > lib_a-qsort_r.o:
>> >  T qsort_r
>> >
>> > Notice the last line has "T qsort_r" which says it is defined.
>> >
>> > grep -r in the newlib source shows it is in ./libc/search/qsort_r.c
>> >
>> > dladdr() looks to be prototyped in RTEMS but hidden behind an ifdef like it
>> > wasn't ported from NetBSD so that looks possible. It is in 

[PATCH] rld-cc: Add -target to recognised cflags

[Hesham Almatary]

-target *-*-* flag is necessary for LLVM/Clang while cross-compiling
---
 rtemstoolkit/rld-cc.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rtemstoolkit/rld-cc.cpp b/rtemstoolkit/rld-cc.cpp
index bb03ff6..b424214 100644
--- a/rtemstoolkit/rld-cc.cpp
+++ b/rtemstoolkit/rld-cc.cpp
@@ -110,6 +110,7 @@ namespace rld
   { fg_include, "-I",   2, true,  0 },
   { fg_include, "-isystem", 2, true,  0 },
   { fg_include, "-sysroot", 2, true,  0 },
+  { fg_machine, "-target" , 2, false, 0 },
   { fg_machine, "-O",   1, false, 0 },
   { fg_machine, "-m",   1, false, 0 },
   { fg_machine, "-f",   1, false, 0 },
-- 
2.25.1

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

[PATCH 2/2] rtems: Simplify rtems_semaphore_set_priority()

[Sebastian Huber]

Do not write to the object referenced by old_priority in error paths.
This is in line with other directives.
---
 cpukit/rtems/src/semsetpriority.c | 47 +++
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/cpukit/rtems/src/semsetpriority.c 
b/cpukit/rtems/src/semsetpriority.c
index 119dd85d77..97e53c6584 100644
--- a/cpukit/rtems/src/semsetpriority.c
+++ b/cpukit/rtems/src/semsetpriority.c
@@ -29,20 +29,6 @@
 #include 
 #include 
 
-static rtems_status_code _Semaphore_Is_scheduler_valid(
-  const CORE_ceiling_mutex_Control *the_mutex,
-  const Scheduler_Control  *scheduler
-)
-{
-#if defined(RTEMS_SMP)
-  if ( scheduler != _CORE_ceiling_mutex_Get_scheduler( the_mutex ) ) {
-return RTEMS_NOT_DEFINED;
-  }
-#endif
-
-  return RTEMS_SUCCESSFUL;
-}
-
 static rtems_status_code _Semaphore_Set_priority(
   Semaphore_Control   *the_semaphore,
   const Scheduler_Control *scheduler,
@@ -51,7 +37,6 @@ static rtems_status_code _Semaphore_Set_priority(
   Thread_queue_Context*queue_context
 )
 {
-  rtems_status_code  sc;
   bool   valid;
   Priority_Control   core_priority;
   Priority_Control   old_priority;
@@ -73,16 +58,26 @@ static rtems_status_code _Semaphore_Set_priority(
 
   switch ( variant ) {
 case SEMAPHORE_VARIANT_MUTEX_PRIORITY_CEILING:
-  sc = _Semaphore_Is_scheduler_valid(
-_semaphore->Core_control.Mutex,
-scheduler
-  );
+#if defined(RTEMS_SMP)
+  if (
+scheduler != _CORE_ceiling_mutex_Get_scheduler(
+  _semaphore->Core_control.Mutex
+)
+  ) {
+_Thread_queue_Release(
+  _semaphore->Core_control.Wait_queue,
+  queue_context
+);
+
+return RTEMS_NOT_DEFINED;
+  }
+#endif
 
   old_priority = _CORE_ceiling_mutex_Get_priority(
 _semaphore->Core_control.Mutex
   );
 
-  if ( sc == RTEMS_SUCCESSFUL && new_priority != RTEMS_CURRENT_PRIORITY ) {
+  if ( new_priority != RTEMS_CURRENT_PRIORITY ) {
 _CORE_ceiling_mutex_Set_priority(
   _semaphore->Core_control.Mutex,
   core_priority
@@ -105,7 +100,6 @@ static rtems_status_code _Semaphore_Set_priority(
 );
   }
 
-  sc = RTEMS_SUCCESSFUL;
   break;
 #endif
 default:
@@ -115,9 +109,12 @@ static rtems_status_code _Semaphore_Set_priority(
   || variant == SEMAPHORE_VARIANT_SIMPLE_BINARY
   || variant == SEMAPHORE_VARIANT_COUNTING
   );
-  old_priority = 0;
-  sc = RTEMS_NOT_DEFINED;
-  break;
+  _Thread_queue_Release(
+_semaphore->Core_control.Wait_queue,
+queue_context
+  );
+
+  return RTEMS_NOT_DEFINED;
   }
 
   cpu_self = _Thread_queue_Dispatch_disable( queue_context );
@@ -129,7 +126,7 @@ static rtems_status_code _Semaphore_Set_priority(
   _Thread_Dispatch_enable( cpu_self );
 
   *old_priority_p = _RTEMS_Priority_From_core( scheduler, old_priority );
-  return sc;
+  return RTEMS_SUCCESSFUL;
 }
 
 rtems_status_code rtems_semaphore_set_priority(
-- 
2.26.2

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

[PATCH 1/2] score: Fix _CORE_ceiling_mutex_Set_priority()

[Sebastian Huber]

We have to use a second thread queue context to acquire and release the
thread wait lock.

Close #4356.
---
 cpukit/include/rtems/score/coremuteximpl.h | 13 +++--
 cpukit/rtems/src/semsetpriority.c  |  3 +--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/cpukit/include/rtems/score/coremuteximpl.h 
b/cpukit/include/rtems/score/coremuteximpl.h
index cbc1e720fb..e8d72e5d25 100644
--- a/cpukit/include/rtems/score/coremuteximpl.h
+++ b/cpukit/include/rtems/score/coremuteximpl.h
@@ -357,12 +357,10 @@ _CORE_ceiling_mutex_Get_scheduler(
  *
  * @param[out] the_mutex The ceiling mutex to set the priority of.
  * @param priority_ceiling The new priority ceiling of the mutex.
- * @param queue_context The thread queue context.
  */
 RTEMS_INLINE_ROUTINE void _CORE_ceiling_mutex_Set_priority(
   CORE_ceiling_mutex_Control *the_mutex,
-  Priority_Controlpriority_ceiling,
-  Thread_queue_Context   *queue_context
+  Priority_Controlpriority_ceiling
 )
 {
   Thread_Control *owner;
@@ -370,15 +368,18 @@ RTEMS_INLINE_ROUTINE void 
_CORE_ceiling_mutex_Set_priority(
   owner = _CORE_mutex_Get_owner( _mutex->Recursive.Mutex );
 
   if ( owner != NULL ) {
-_Thread_Wait_acquire( owner, queue_context );
+Thread_queue_Context queue_context;
+
+_Thread_queue_Context_initialize( _context );
+_Thread_Wait_acquire_critical( owner, _context );
 _Thread_Priority_change(
   owner,
   _mutex->Priority_ceiling,
   priority_ceiling,
   false,
-  queue_context
+  _context
 );
-_Thread_Wait_release( owner, queue_context );
+_Thread_Wait_release_critical( owner, _context );
   } else {
 the_mutex->Priority_ceiling.priority = priority_ceiling;
   }
diff --git a/cpukit/rtems/src/semsetpriority.c 
b/cpukit/rtems/src/semsetpriority.c
index adb0320210..119dd85d77 100644
--- a/cpukit/rtems/src/semsetpriority.c
+++ b/cpukit/rtems/src/semsetpriority.c
@@ -85,8 +85,7 @@ static rtems_status_code _Semaphore_Set_priority(
   if ( sc == RTEMS_SUCCESSFUL && new_priority != RTEMS_CURRENT_PRIORITY ) {
 _CORE_ceiling_mutex_Set_priority(
   _semaphore->Core_control.Mutex,
-  core_priority,
-  queue_context
+  core_priority
 );
   }
 
-- 
2.26.2

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

Re: About locks and concurrency rules for SMP systems

[Sebastian Huber]

On 24/03/2021 08:07, Richi Dubey wrote:

How does RTEMS handle cases where two different cores access 
scheduler-related functions at the same time? 

Each scheduler has its own lock. There are a couple of more locks involved.
For ex., Can they access (or modify) the Scheduler_strong_APA_Context 
 
at the same time?

No, unless we have a bug.

--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

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

Re: [rtems commit] gen_uuid.c: Fix two Unchecked return value from library errors

[Sebastian Huber]

On 08/03/2021 21:56, Joel Sherrill wrote:


Module:rtems
Branch:master
Commit:597e4f476568a225d14dfaff02074cf269ad62ac
Changeset:http://git.rtems.org/rtems/commit/?id=597e4f476568a225d14dfaff02074cf269ad62ac

Author:Ryan Long
Date:  Tue Mar  2 11:08:28 2021 -0500

gen_uuid.c: Fix two Unchecked return value from library errors

CID 1049146: Unchecked return value from library in get_clock().
CID 1049147: Unchecked return value from library in get_random_fd().

Closes #4280

---

  cpukit/libmisc/uuid/gen_uuid.c | 11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/cpukit/libmisc/uuid/gen_uuid.c b/cpukit/libmisc/uuid/gen_uuid.c
index 3ca75a0..5bb34c0 100644
--- a/cpukit/libmisc/uuid/gen_uuid.c
+++ b/cpukit/libmisc/uuid/gen_uuid.c
@@ -155,6 +155,7 @@ static int get_random_fd(void)
struct timeval  tv;
static int  fd = -2;
int i;
+   int sc;
  
  	if (fd == -2) {

gettimeofday(, 0);
@@ -164,8 +165,10 @@ static int get_random_fd(void)
fd = open("/dev/random", O_RDONLY | O_NONBLOCK);
if (fd >= 0) {
i = fcntl(fd, F_GETFD);
-   if (i >= 0)
-   fcntl(fd, F_SETFD, i | FD_CLOEXEC);
+   if (i >= 0) {
+   sc = fcntl(fd, F_SETFD, i | FD_CLOEXEC);
+   _Assert_Unused_variable_unequal(sc, -1);
+   }


FD_CLOEXEC is not supported by RTEMS. Do we even have these device files 
in RTEMS? This is 3rd-party code, what about upstream changes?



}
  #endif
srand((getpid() << ((sizeof(pid_t)*CHAR_BIT)>>1)) ^ getuid() ^ 
tv.tv_sec ^ tv.tv_usec);
@@ -334,6 +337,7 @@ static int get_clock(uint32_t *clock_high, uint32_t 
*clock_low,
uint64_tclock_reg;
mode_t  save_umask;
int len;
+   int sc;
  
  	if (state_fd == -2) {

save_umask = umask(0);
@@ -426,7 +430,8 @@ try_again:
}
rewind(state_f);
fl.l_type = F_UNLCK;
-   fcntl(state_fd, F_SETLK, );
+   sc = fcntl(state_fd, F_SETLK, );
+   _Assert_Unused_variable_unequal(sc, -1);

F_SETLK is not supported by RTEMS.

}
  
  	*clock_high = clock_reg >> 32;


The patch produces warnings like this:

../../../cpukit/libmisc/uuid/gen_uuid.c: In function 'get_clock':
../../../cpukit/libmisc/uuid/gen_uuid.c:434:3: warning: implicit 
declaration of function '_Assert_Unused_variable_unequal' 
[-Wimplicit-function-declaration]

  434 |   _Assert_Unused_variable_unequal(sc, -1);
  |   ^~~
../../../cpukit/libmisc/uuid/gen_uuid.c:434:3: warning: nested extern 
declaration of '_Assert_Unused_variable_unequal' [-Wnested-externs]


In total, how was this patch tested?

If you add _Assert() stuff, please build with RTEMS_DEBUG enabled and 
run the tests.


--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

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

Re: [PATCH] Add configuration option for single processor applications

[Richi Dubey]

>
> That depends. I don't think the CPP will handle string concatenation
> so well, so a long string like that can cause an exception to the
> rule. Otherwise you would end up with something like
>   #ifndef CONFIGURE_MAXIMUM_PROCESSORS
> #error "CONFIGURE_MAXIMUM_PROCESSORS must be defined to \
> configure the EDF SMP scheduler"
>   #endif
> which is worse. It might be possible through some other means though, like
> #ifndef CONFIGURE_MAXIMUM_PROCESSORS
> #error "CONFIGURE_MAXIMUM_PROCESSORS must be defined" ## \
> "to configure the EDF SMP scheduler"
> #endif
> But again, it is probably better just to let it spill over length than
> to make it more complex by this manner.

I understand, Thanks for the clarification.

I think I wrote the first version of this :)
> https://gedare.github.io/pdf/bloom_scheduling_2014.pdf
> Now things are a little bit more complicated by SMP. Capturing this
> knowledge is important.

Got it. Thanks!



On Mon, Mar 22, 2021 at 11:08 PM Gedare Bloom  wrote:

> On Sun, Mar 21, 2021 at 11:48 PM Richi Dubey  wrote:
> >
> > Hi!
> >
> > Thanks for your review.
> >
> >> Why not just copy how SMP EDF deals with this problem?
> >>
> https://github.com/richidubey/rtems/blob/d3d1e4bc8e616738bd5892c59f82b174c399fc0b/cpukit/include/rtems/scheduler.h#L124
> >> I guess that is the "right way" to put the onus on the application to
> >> specify their configuration properly.
> >
> > This seems like the right way and I completely understand. I am going to
> use a similar strategy. btw line 125 exceeds the 80 char limit, is that
> okay for header files?
> >
>
> That depends. I don't think the CPP will handle string concatenation
> so well, so a long string like that can cause an exception to the
> rule. Otherwise you would end up with something like
>   #ifndef CONFIGURE_MAXIMUM_PROCESSORS
> #error "CONFIGURE_MAXIMUM_PROCESSORS must be defined to \
> configure the EDF SMP scheduler"
>   #endif
> which is worse. It might be possible through some other means though, like
> #ifndef CONFIGURE_MAXIMUM_PROCESSORS
> #error "CONFIGURE_MAXIMUM_PROCESSORS must be defined" ## \
> "to configure the EDF SMP scheduler"
> #endif
>
> But again, it is probably better just to let it spill over length than
> to make it more complex by this manner.
>
> >> I never thought too much about this. I guess, it means we expect the
> >> scheduler-specific tests do a sufficient job to exercise the scheduler
> >> implementations that we can rely on the default scheduler to "just
> >> work" in the other tests. Which makes sense although you have stumbled
> >> over some scheduler-specific bugs that are uncovered by these
> >> non-scheduler tests.
> >
> > Yes, some of the errors in our Strong APA scheduler (code or logic) was
> discovered by failing non scheduler tests.
> >>
> >> Documenting your approach to developing and
> >> debugging a scheduler may help the next person who tries. :) We don't
> >> have a great place for that kind of guidance, so I guess clear blog
> >> posts are helpful for now.
> >
> > Okay, I am on it. How to write a scheduler: RTEMS edition coming soon on
> https://rtemswithrichi.wordpress.com/ :p
>
> I think I wrote the first version of this :)
> https://gedare.github.io/pdf/bloom_scheduling_2014.pdf
> Now things are a little bit more complicated by SMP. Capturing this
> knowledge is important.
>
> >
> >
> > On Thu, Mar 18, 2021 at 2:15 AM Gedare Bloom  wrote:
> >>
> >> Hi Richi,
> >>
> >> On Wed, Mar 17, 2021 at 1:06 PM Sebastian Huber
> >>  wrote:
> >> >
> >> > On 17/03/2021 19:00, Richi Dubey wrote:
> >> >
> >> > > Thanks for your quick review.
> >> > >
> >> > > I think this patch is superfluous. In which scenario do you
> think
> >> > > it is
> >> > > necessary?
> >> > >
> >> > > It is from this mail conversation:
> >> > > https://lists.rtems.org/pipermail/devel/2020-September/061845.html
> >> > >  >
> >> > > followed by
> >> > > https://lists.rtems.org/pipermail/devel/2020-September/061846.html
> >> > >  >.
> >> > For the tests you just need a temporary hack.
> >> > >
> >> > > Strong APA uses the value of CONFIGURE_MAXIMUM_PROCESSORS in its
> >> > > declaration
> >> > > <
> https://github.com/richidubey/rtems/blob/d3d1e4bc8e616738bd5892c59f82b174c399fc0b/cpukit/include/rtems/scheduler.h#L260
> >
> >> > > at cpukit/include/rtems/scheduler.h. This addition would be
> necessary
> >> > > to support future SMP schedulers that need to know the number of
> CPUs
> >> > > in the system at the time of configuration.
> >>
> >> Can you use _CONFIGURE_MAXIMUM_PROCESSORS there?
> >>
> >> Why not just copy how SMP EDF deals with this problem?
> >>
> https://github.com/richidubey/rtems/blob/d3d1e4bc8e616738bd5892c59f82b174c399fc0b/cpukit/include/rtems/scheduler.h#L124
> >>
> >> I guess that is the "right way" to put the onus on the 

Regarding cross-reference link in docs

[Ayushman Mishra]

Hello everyone,
I am writing a document page and i am getting stuck in writing reference
link to different manual of docs. I tried using the .. _reference: ,
:ref:`refrence-link` method but it works for document in same manual but
not in different manuals . And I think this is an issue in several
different pages of docs also since while going through other pages I saw
reference-links given in form of complete url of other doc manual which I
don't think will work once the master branch changes to release version (
and url gets changed) .
It would very helpful if someone can please provide some information about
how to refer to different manual so that I can complete the page and
correct other links of docs.
Thanks,
Ayushman
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

About locks and concurrency rules for SMP systems

[Richi Dubey]

Hi,

How does RTEMS handle cases where two different cores access
scheduler-related functions at the same time? For ex., Can they access (or
modify) the Scheduler_strong_APA_Context

at the same time?

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