Re: [PATCH v3] bsps/shared/ofw: Fix coverity reported defects

2021-02-04 Thread Niteesh G. S. 
On Thu, Feb 4, 2021 at 1:21 AM Gedare Bloom  wrote:

>
>
> On Wed, Feb 3, 2021 at 2:48 AM G S Niteesh Babu 
> wrote:
>
>> Fixed use after free and null pointer dereference defects
>>
>> FIXES:
>> 1) CID 1472601 (NULL_RETURNS)
>> 2) CID 1472600 (USE_AFTER_FREE)
>> 3) CID 1472599 (USE_AFTER_FREE)
>> 4) CID 1472598 (USE_AFTER_FREE)
>> 5) CID 1472596 (USE_AFTER_FREE)
>> 6) CID 1472597 (ARRAY_VS_SINGLETON)
>> 7) CID 1472595 (ARRAY_VS_SINGLETON)
>> ---
>>  bsps/shared/ofw/ofw.c | 36 ++--
>>  1 file changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/bsps/shared/ofw/ofw.c b/bsps/shared/ofw/ofw.c
>> index 82924b2600..ccd57e36af 100644
>> --- a/bsps/shared/ofw/ofw.c
>> +++ b/bsps/shared/ofw/ofw.c
>> @@ -313,7 +313,7 @@ ssize_t rtems_ofw_get_prop_alloc(
>>  }
>>
>>  if (rtems_ofw_get_prop(node, propname, *buf, len) == -1) {
>> -  rtems_ofw_free(buf);
>> +  rtems_ofw_free(*buf);
>>*buf = NULL;
>>return -1;
>>  }
>> @@ -344,7 +344,7 @@ ssize_t rtems_ofw_get_prop_alloc_multi(
>>  }
>>
>>  if (rtems_ofw_get_prop(node, propname, *buf, len) == -1) {
>> -  rtems_ofw_free(buf);
>> +  rtems_ofw_free(*buf);
>>*buf = NULL;
>>return -1;
>>  }
>> @@ -373,7 +373,7 @@ ssize_t rtems_ofw_get_enc_prop_alloc(
>>  }
>>
>>  if (rtems_ofw_get_enc_prop(node, propname, *buf, len) == -1) {
>> -  rtems_ofw_free(buf);
>> +  rtems_ofw_free(*buf);
>>*buf = NULL;
>>return -1;
>>  }
>> @@ -404,7 +404,7 @@ ssize_t rtems_ofw_get_enc_prop_alloc_multi(
>>  }
>>
>>  if (rtems_ofw_get_enc_prop(node, propname, *buf, len) == -1) {
>> -  rtems_ofw_free(buf);
>> +  rtems_ofw_free(*buf);
>>*buf = NULL;
>>return -1;
>>  }
>>
> The above all look fine to me.
>
>
>> @@ -500,21 +500,21 @@ static phandle_t rtems_ofw_get_effective_phandle(
>>  )
>>  {
>>phandle_t child;
>> -  phandle_t ref;
>> +  phandle_t ref[1];
>>
>>
> I don't like this. I know this was suggested, but I think it is a little
> ridiculous. This is a false positive since we know that sizeof(*buf) ==
> len. I don't know if we can make that an assertion. Otherwise, we can just
> mark this as a false positive in coverity. We know the array dereference in
> this case won't overwrite past the bounds of ref.
>
> Instead of using hard-coded values of 4 in rtems_ofw_get_enc_prop() you
> might make it more explicitly using sizeof(pcell_t), since that is what you
> mean.
>
Done.

>
> I would also agree to change the strncpy as Christian identified before in
> rtems_ofw_get_prop().
>
Is the reason to avoid strncpy that it ignores the null byte if len(dst) <=
len(src)?
If so can I do an explicit null byte append?
Or is there any other reason?


>
>>for (child = rtems_ofw_child(node); child != 0; child =
>> rtems_ofw_peer(child)) {
>> -ref = rtems_ofw_get_effective_phandle(child, xref);
>> -if (ref != -1)
>> -  return ref;
>> +ref[0] = rtems_ofw_get_effective_phandle(child, xref);
>>
>
> I didn't notice before, but is this recursion bounded (yes, it is a
> tree, but it might be better to rewrite this function non-recursively).
>
Just curious why is it better? Is it because it might use a lot of stack
frames?
I can only think of using stack or queue to implement it non-recursively.
Is there
any other way?

>
>
>> +if (ref[0] != -1)
>> +  return ref[0];
>>
>> -if (rtems_ofw_get_enc_prop(child, "phandle", &ref, sizeof(ref)) ==
>> -1 &&
>> -rtems_ofw_get_enc_prop(child, "ibm,phandle", &ref, sizeof(ref))
>> == -1 &&
>> -rtems_ofw_get_enc_prop(child, "linux,phandle", &ref,
>> sizeof(ref)) == -1
>> +if (rtems_ofw_get_enc_prop(child, "phandle", ref, sizeof(ref)) == -1
>> &&
>> +rtems_ofw_get_enc_prop(child, "ibm,phandle", ref, sizeof(ref))
>> == -1 &&
>> +rtems_ofw_get_enc_prop(child, "linux,phandle", ref, sizeof(ref))
>> == -1
>>  ) {
>>continue;
>>  }
>>
>> -if (ref == xref)
>> +if (ref[0] == xref)
>>return child;
>>}
>>
>> @@ -533,16 +533,16 @@ phandle_t rtems_ofw_node_from_xref( phandle_t xref )
>>
>>  phandle_t rtems_ofw_xref_from_node( phandle_t node )
>>  {
>> -  phandle_t ref;
>> +  phandle_t ref[1];
>>
>> -if (rtems_ofw_get_enc_prop(node, "phandle", &ref, sizeof(ref)) == -1
>> &&
>> -rtems_ofw_get_enc_prop(node, "ibm,phandle", &ref, sizeof(ref))
>> == -1 &&
>> -rtems_ofw_get_enc_prop(node, "linux,phandle", &ref, sizeof(ref))
>> == -1)
>> +if (rtems_ofw_get_enc_prop(node, "phandle", ref, sizeof(ref)) == -1
>> &&
>> +rtems_ofw_get_enc_prop(node, "ibm,phandle", ref, sizeof(ref)) ==
>> -1 &&
>> +rtems_ofw_get_enc_prop(node, "linux,phandle", ref, sizeof(ref))
>> == -1)
>>  {
>>return node;
>>  }
>>
>> -return ref;
>> +return ref[0];
>>  }
>>
>>  phandle_t rtems_ofw_instance_to_package( ihandle_t instance )
>> @@ -614,7 +614,7 @@ int rtems_ofw_get_reg(
>>  of

Re: [PATCH] Basic lwIP for STM32H7 BSP

2021-02-04 Thread Robin Müller 
I think the DMA descriptors need to be 32 byte (0x20) aligned. That's the
reason the example application was set up like this.

Kind Regards
Robin

On Wed, 3 Feb 2021 at 15:32, Joel Sherrill  wrote:

>
>
> On Wed, Feb 3, 2021, 8:26 AM Robin Müller 
> wrote:
>
>> Also, Andrei Chichak provided a very good explanation:
>>
>> The STM32x7 ethernet controllers need their descriptors and data areas to
>>> be located on 32-byte boundaries with a combination of cache being turned
>>> off and buffering being turned off. This is a side effect of the STM32x7
>>> DMA and data caching.
>>>
>>> The examples that ST have for LWiP under FreeRTOS have some issues with
>>> the linker file having overlapping sections and the stack (above the ETH
>>> data areas) being left with no cache and no buffering. The space above the
>>> rx/tx buffers is also less than the stack space minimum specified in the
>>> linker file as well.  Some rearranging of the ETH descriptors and data
>>> areas would be prudent. Like push them to the top of SRAM and locate the
>>> stack below.
>>>
>>> To get this all going, they set up the MPU for two overlapping sections,
>>> one being the top 16kB of SRAM and the other the 256B that contain the ETH
>>> descriptors. I believe the MPU regions can be set in increments of 32Bytes,
>>> so setting up the cache and buffering should be do-able without affecting
>>> the stack.
>>>
>>
> Just asking. Can this be a non-cacheable region and the variables not in
> special sections? I wonder if changing them to normal pointers to special
> memory would ripple. Do they just use the variables for initialization or
> reference them a lot?
>
> I think the standard linkcmds setup allows for nocache areas already.
>
> --joel
>
>
>> Kind Regards
>> Robin Müller
>>
>> On Wed, 3 Feb 2021 at 14:50, Robin Müller 
>> wrote:
>>
>>> The following link contains more theoretical information about why these
>>> sections were placed at these addresses:
>>> https://community.st.com/s/article/FAQ-DMA-is-not-working-on-STM32H7-devices
>>>
>>> Kind Regards
>>> Robin
>>>
>>> On Wed, 3 Feb 2021 at 14:44, Robin Müller 
>>> wrote:
>>>
 The DMA descriptors need to be placed at the start of the SRAM3 and
 need to be aligned in a certain way. The RX buffer will take up the first
 (slightly less than) 16 kB of SRAM3 but needs to be placed
 behind the DMA descriptors. It also needs to be placed in a way that
 the MPU configuration required for the DMA descriptors will not do
 something with the RX buffers.
 In the example provided by STM32, the first 256 bytes are configured by
 MPU Config.

 Kind Regards
 Robin



 On Wed, 3 Feb 2021 at 13:43, Sebastian Huber <
 sebastian.hu...@embedded-brains.de> wrote:

> On 02/02/2021 20:10, Robin Mueller wrote:
>
> > + /* Not an ideal solution but required for lwIP on the STM32H7
> BSP.
> > + This places the DMA descriptors for lwIP at the start of SRAM3.
> > + The MPU still needs to be configured for the DMA descriptor
> regions to be
> > + bufferable, non-cacheable, non-shareable (first 256 bytes) */
> > + .lwip_sec_stm32h7 (NOLOAD) : ALIGN_WITH_INPUT {
> > + . = ABSOLUTE(0x3004);
> > + *(.RxDecripSection)
> > + . = ABSOLUTE(0x30040060);
> > + *(.TxDecripSection)
> > + . = ABSOLUTE(0x30040200);
> > + *(.RxArraySection)
> > + } >SRAM_3 AT> REGION_TEXT_LOAD
> > +
>
> This is the wrong linker command file. This stuff should be in
>
> spec/build/bsps/arm/stm32h7/linkcmdsmemory.yml
>
> with an output section name like ".stm32h7_sram_3" and corresponding
> input section names. Why do you need absolute addresses here?
>
> --
> 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: [PATCH] Basic lwIP for STM32H7 BSP

2021-02-04 Thread Joel Sherrill 
On Thu, Feb 4, 2021, 6:18 AM Robin Müller  wrote:

> I think the DMA descriptors need to be 32 byte (0x20) aligned. That's the
> reason the example application was set up like this.
>

But you can just as easily turn them into pointers which point to 32 byte
aligned areas in a nocache region.

Just wondering if that's possible with the code because I think it would be
easier with the existing linkcmds to assign a nocache area as you need and
then split it by hand.

Sebastian... ?

>
> Kind Regards
> Robin
>
> On Wed, 3 Feb 2021 at 15:32, Joel Sherrill  wrote:
>
>>
>>
>> On Wed, Feb 3, 2021, 8:26 AM Robin Müller 
>> wrote:
>>
>>> Also, Andrei Chichak provided a very good explanation:
>>>
>>> The STM32x7 ethernet controllers need their descriptors and data areas
 to be located on 32-byte boundaries with a combination of cache being
 turned off and buffering being turned off. This is a side effect of the
 STM32x7 DMA and data caching.

 The examples that ST have for LWiP under FreeRTOS have some issues with
 the linker file having overlapping sections and the stack (above the ETH
 data areas) being left with no cache and no buffering. The space above the
 rx/tx buffers is also less than the stack space minimum specified in the
 linker file as well.  Some rearranging of the ETH descriptors and data
 areas would be prudent. Like push them to the top of SRAM and locate the
 stack below.

 To get this all going, they set up the MPU for two overlapping
 sections, one being the top 16kB of SRAM and the other the 256B that
 contain the ETH descriptors. I believe the MPU regions can be set in
 increments of 32Bytes, so setting up the cache and buffering should be
 do-able without affecting the stack.

>>>
>> Just asking. Can this be a non-cacheable region and the variables not in
>> special sections? I wonder if changing them to normal pointers to special
>> memory would ripple. Do they just use the variables for initialization or
>> reference them a lot?
>>
>> I think the standard linkcmds setup allows for nocache areas already.
>>
>> --joel
>>
>>
>>> Kind Regards
>>> Robin Müller
>>>
>>> On Wed, 3 Feb 2021 at 14:50, Robin Müller 
>>> wrote:
>>>
 The following link contains more theoretical information about why
 these sections were placed at these addresses:
 https://community.st.com/s/article/FAQ-DMA-is-not-working-on-STM32H7-devices

 Kind Regards
 Robin

 On Wed, 3 Feb 2021 at 14:44, Robin Müller 
 wrote:

> The DMA descriptors need to be placed at the start of the SRAM3 and
> need to be aligned in a certain way. The RX buffer will take up the first
> (slightly less than) 16 kB of SRAM3 but needs to be placed
> behind the DMA descriptors. It also needs to be placed in a way that
> the MPU configuration required for the DMA descriptors will not do
> something with the RX buffers.
> In the example provided by STM32, the first 256 bytes are configured
> by MPU Config.
>
> Kind Regards
> Robin
>
>
>
> On Wed, 3 Feb 2021 at 13:43, Sebastian Huber <
> sebastian.hu...@embedded-brains.de> wrote:
>
>> On 02/02/2021 20:10, Robin Mueller wrote:
>>
>> > + /* Not an ideal solution but required for lwIP on the STM32H7
>> BSP.
>> > + This places the DMA descriptors for lwIP at the start of
>> SRAM3.
>> > + The MPU still needs to be configured for the DMA descriptor
>> regions to be
>> > + bufferable, non-cacheable, non-shareable (first 256 bytes) */
>> > + .lwip_sec_stm32h7 (NOLOAD) : ALIGN_WITH_INPUT {
>> > + . = ABSOLUTE(0x3004);
>> > + *(.RxDecripSection)
>> > + . = ABSOLUTE(0x30040060);
>> > + *(.TxDecripSection)
>> > + . = ABSOLUTE(0x30040200);
>> > + *(.RxArraySection)
>> > + } >SRAM_3 AT> REGION_TEXT_LOAD
>> > +
>>
>> This is the wrong linker command file. This stuff should be in
>>
>> spec/build/bsps/arm/stm32h7/linkcmdsmemory.yml
>>
>> with an output section name like ".stm32h7_sram_3" and corresponding
>> input section names. Why do you need absolute addresses here?
>>
>> --
>> 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] Basic lwIP for STM32H7 BSP

2021-02-04 Thread Sebastian Huber 

On 04/02/2021 14:39, Joel Sherrill wrote:

On Thu, Feb 4, 2021, 6:18 AM Robin Müller > wrote:


I think the DMA descriptors need to be 32 byte (0x20) aligned.
That's the reason the example application was set up like this.


But you can just as easily turn them into pointers which point to 32 
byte aligned areas in a nocache region.


Just wondering if that's possible with the code because I think it 
would be easier with the existing linkcmds to assign a nocache area as 
you need and then split it by hand.


Sebastian... ?
I would prefer to use a generic linker section definition and then 
simply define global data structures in the right section with the right 
alignment (see RTEMS_ALIGN()).


--
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 v3] bsps/shared/ofw: Fix coverity reported defects

2021-02-04 Thread Gedare Bloom 
On Thu, Feb 4, 2021 at 1:58 AM Niteesh G. S.  wrote:

>
>
> On Thu, Feb 4, 2021 at 1:21 AM Gedare Bloom  wrote:
>
>>
>>
>> On Wed, Feb 3, 2021 at 2:48 AM G S Niteesh Babu 
>> wrote:
>>
>>> Fixed use after free and null pointer dereference defects
>>>
>>> FIXES:
>>> 1) CID 1472601 (NULL_RETURNS)
>>> 2) CID 1472600 (USE_AFTER_FREE)
>>> 3) CID 1472599 (USE_AFTER_FREE)
>>> 4) CID 1472598 (USE_AFTER_FREE)
>>> 5) CID 1472596 (USE_AFTER_FREE)
>>> 6) CID 1472597 (ARRAY_VS_SINGLETON)
>>> 7) CID 1472595 (ARRAY_VS_SINGLETON)
>>> ---
>>>  bsps/shared/ofw/ofw.c | 36 ++--
>>>  1 file changed, 18 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/bsps/shared/ofw/ofw.c b/bsps/shared/ofw/ofw.c
>>> index 82924b2600..ccd57e36af 100644
>>> --- a/bsps/shared/ofw/ofw.c
>>> +++ b/bsps/shared/ofw/ofw.c
>>> @@ -313,7 +313,7 @@ ssize_t rtems_ofw_get_prop_alloc(
>>>  }
>>>
>>>  if (rtems_ofw_get_prop(node, propname, *buf, len) == -1) {
>>> -  rtems_ofw_free(buf);
>>> +  rtems_ofw_free(*buf);
>>>*buf = NULL;
>>>return -1;
>>>  }
>>> @@ -344,7 +344,7 @@ ssize_t rtems_ofw_get_prop_alloc_multi(
>>>  }
>>>
>>>  if (rtems_ofw_get_prop(node, propname, *buf, len) == -1) {
>>> -  rtems_ofw_free(buf);
>>> +  rtems_ofw_free(*buf);
>>>*buf = NULL;
>>>return -1;
>>>  }
>>> @@ -373,7 +373,7 @@ ssize_t rtems_ofw_get_enc_prop_alloc(
>>>  }
>>>
>>>  if (rtems_ofw_get_enc_prop(node, propname, *buf, len) == -1) {
>>> -  rtems_ofw_free(buf);
>>> +  rtems_ofw_free(*buf);
>>>*buf = NULL;
>>>return -1;
>>>  }
>>> @@ -404,7 +404,7 @@ ssize_t rtems_ofw_get_enc_prop_alloc_multi(
>>>  }
>>>
>>>  if (rtems_ofw_get_enc_prop(node, propname, *buf, len) == -1) {
>>> -  rtems_ofw_free(buf);
>>> +  rtems_ofw_free(*buf);
>>>*buf = NULL;
>>>return -1;
>>>  }
>>>
>> The above all look fine to me.
>>
>>
>>> @@ -500,21 +500,21 @@ static phandle_t rtems_ofw_get_effective_phandle(
>>>  )
>>>  {
>>>phandle_t child;
>>> -  phandle_t ref;
>>> +  phandle_t ref[1];
>>>
>>>
>> I don't like this. I know this was suggested, but I think it is a little
>> ridiculous. This is a false positive since we know that sizeof(*buf) ==
>> len. I don't know if we can make that an assertion. Otherwise, we can just
>> mark this as a false positive in coverity. We know the array dereference in
>> this case won't overwrite past the bounds of ref.
>>
>> Instead of using hard-coded values of 4 in rtems_ofw_get_enc_prop() you
>> might make it more explicitly using sizeof(pcell_t), since that is what you
>> mean.
>>
> Done.
>
>>
>> I would also agree to change the strncpy as Christian identified before
>> in rtems_ofw_get_prop().
>>
> Is the reason to avoid strncpy that it ignores the null byte if len(dst)
> <= len(src)?
> If so can I do an explicit null byte append?
> Or is there any other reason?
>
> The reason is that it passes void* pointers. If these are strings, you
should use char* type. Otherwise, memcpy is more suitable.

It also would be generally safer to overwrite with the NIL character to
guarantee it is a null-terminated string, if that is expected.


>
>>
>>>for (child = rtems_ofw_child(node); child != 0; child =
>>> rtems_ofw_peer(child)) {
>>> -ref = rtems_ofw_get_effective_phandle(child, xref);
>>> -if (ref != -1)
>>> -  return ref;
>>> +ref[0] = rtems_ofw_get_effective_phandle(child, xref);
>>>
>>
>> I didn't notice before, but is this recursion bounded (yes, it is a
>> tree, but it might be better to rewrite this function non-recursively).
>>
> Just curious why is it better? Is it because it might use a lot of stack
> frames?
> I can only think of using stack or queue to implement it non-recursively.
> Is there
> any other way?
>

Recursion causes two potential problems: large stack usage and
hard-to-analyze execution times. These are generally important for an RTOS
to be wary of.

This looks like a depth-first search to find xref? But the tree traversal
order doesn't matter. In fact, I would check if the FDT can be iterated
directly. I don't know enough about the FDT structure to say whether that
is simple to do. If you start at the root and repeatedly call
fdt_next_node()  do you traverse all the nodes?

You can implement non-recursive tree searches using nested loops when you
have sibling, child, and parent pointers. Probably, you can find code
examples of how to do this. The basic idea is pretty simple though, here is
a DFS:
node = root
do {
  visit(node)
  next_node = child_of(node)
  if ( ! next_node ) {
while ( !has_sibling(node) && node != root) {
node = parent_of(node) /* back up */
}
next_node = sibling_of(node)
  }
  node = next_node;
} while (node)

This pseudocode assumes the root has a NULL-value sibling and leaves have
NULL-value children. I also didn't test it, but the rough idea should work.
You can do something similar with BFS.



>
>>
>>> +i

psim autoconf build w/C++ enabled fails oddly

2021-02-04 Thread Joel Sherrill 
Hi

I will have to doublecheck waf but based on build list posts, autoconf w/o
C++ built OK yesterday. The cdtest.exe fails to link like this:

gmake[5]: Entering directory
`/home/joel/rtems-work/b-psim/powerpc-rtems6/c/psim/testsuites/samples'
powerpc-rtems6-g++  -meabi -mcpu=603e -msdata=sysv -fno-common -O2 -g
-fno-keep-inline-functions -ffunction-sections -fdata-sections -Wall
-B./../../lib/libbsp/powerpc/psim
-B/home/joel/rtems-work/rtems/bsps/powerpc/psim/start -specs bsp_specs
-qrtems -L./../../cpukit
-L/home/joel/rtems-work/rtems/bsps/powerpc/shared/start -Wl,--wrap=printf
-Wl,--wrap=puts -Wl,--wrap=putchar -Wl,--gc-sections -o cdtest.exe
cdtest/cdtest-init.o cdtest/cdtest-main.o
./../../lib/libbsp/powerpc/psim/librtemsbsp.a ./../../cpukit/librtemscpu.a
./../../cpukit/librtemstest.a
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
warning: cannot find entry symbol _start; defaulting to 01800100
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
./../../lib/libbsp/powerpc/psim/librtemsbsp.a(cpu.o): in function
`_TLS_Get_thread_control_block_area_size':
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:151: undefined
reference to `_TLS_Alignment'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:151: undefined
reference to `_TLS_Alignment'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
./../../lib/libbsp/powerpc/psim/librtemsbsp.a(cpu.o): in function
`_TLS_Heap_align_up':
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:135: undefined
reference to `_TLS_Size'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:135: undefined
reference to `_TLS_Size'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
./../../lib/libbsp/powerpc/psim/librtemsbsp.a(cpu.o): in function
`_TLS_Copy_and_clear':
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:171: undefined
reference to `_TLS_Data_begin'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:171: undefined
reference to `_TLS_Data_begin'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:171: undefined
reference to `_TLS_Data_size'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:171: undefined
reference to `_TLS_Data_size'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:179: undefined
reference to `_TLS_BSS_begin'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:179: undefined
reference to `_TLS_BSS_begin'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:178: undefined
reference to `_TLS_BSS_size'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
/home/joel/rtems-work/rtems/cpukit/include/rtems/score/tls.h:178: undefined
reference to `_TLS_BSS_size'
/home/joel/rtems-cron-6/tools/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
./../../cpukit/librtemscpu.a(newlibc_exit.o): in function `_exit':
/home/joel/rtems-work/b-psim/powerpc-rtems6/c/psim/cpukit/../../../../../rtems/c/src/../../cpukit/libcsupport/src/newlibc_exit.c:37:
undefined reference to `_fini'

Any ideas?

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

Re: [PATCH] Basic lwIP for STM32H7 BSP

2021-02-04 Thread Robin Müller 
Could you look into this, based on the existing patch? I don't really have
the time to look (deeply) into it in the next few weeks,
and the changes are probably easier  for you to implement. I can offer to
test it though :-)

Kind Regards
Robin Müller

On Thu, 4 Feb 2021 at 14:41, Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> On 04/02/2021 14:39, Joel Sherrill wrote:
>
> > On Thu, Feb 4, 2021, 6:18 AM Robin Müller  > > wrote:
> >
> > I think the DMA descriptors need to be 32 byte (0x20) aligned.
> > That's the reason the example application was set up like this.
> >
> >
> > But you can just as easily turn them into pointers which point to 32
> > byte aligned areas in a nocache region.
> >
> > Just wondering if that's possible with the code because I think it
> > would be easier with the existing linkcmds to assign a nocache area as
> > you need and then split it by hand.
> >
> > Sebastian... ?
> I would prefer to use a generic linker section definition and then
> simply define global data structures in the right section with the right
> alignment (see RTEMS_ALIGN()).
>
> --
> 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] Basic lwIP for STM32H7 BSP

2021-02-04 Thread Sebastian Huber 

On 04/02/2021 18:45, Robin Müller wrote:

Could you look into this, based on the existing patch? I don't really 
have the time to look (deeply) into it in the next few weeks,
and the changes are probably easier  for you to implement. I can offer 
to test it though :-)

Ok, please remind me in a week. I am currently quite busy.

--
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: psim autoconf build w/C++ enabled fails oddly

2021-02-04 Thread Sebastian Huber 

On 04/02/2021 18:15, Joel Sherrill wrote:

I will have to doublecheck waf but based on build list posts, autoconf 
w/o C++ built OK yesterday. 


Is this with the latest tools? One of the recent RSB commits was bad.

There is also this issue:

https://lists.rtems.org/pipermail/devel/2021-February/064173.html

--
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 v3] bsps/shared/ofw: Fix coverity reported defects

2021-02-04 Thread Christian Mauderer 



On 04/02/2021 19:52, Christian Mauderer wrote:



On 04/02/2021 17:34, Gedare Bloom wrote:



On Thu, Feb 4, 2021 at 1:58 AM Niteesh G. S. > wrote:




    On Thu, Feb 4, 2021 at 1:21 AM Gedare Bloom mailto:ged...@rtems.org>> wrote:



    On Wed, Feb 3, 2021 at 2:48 AM G S Niteesh Babu
    mailto:niteesh...@gmail.com>> wrote:

    Fixed use after free and null pointer dereference defects

    FIXES:
    1) CID 1472601 (NULL_RETURNS)
    2) CID 1472600 (USE_AFTER_FREE)
    3) CID 1472599 (USE_AFTER_FREE)
    4) CID 1472598 (USE_AFTER_FREE)
    5) CID 1472596 (USE_AFTER_FREE)
    6) CID 1472597 (ARRAY_VS_SINGLETON)
    7) CID 1472595 (ARRAY_VS_SINGLETON)
    ---
  bsps/shared/ofw/ofw.c | 36
    ++--
  1 file changed, 18 insertions(+), 18 deletions(-)

    diff --git a/bsps/shared/ofw/ofw.c b/bsps/shared/ofw/ofw.c
    index 82924b2600..ccd57e36af 100644
    --- a/bsps/shared/ofw/ofw.c
    +++ b/bsps/shared/ofw/ofw.c
    @@ -313,7 +313,7 @@ ssize_t rtems_ofw_get_prop_alloc(
      }

      if (rtems_ofw_get_prop(node, propname, *buf, len) == 
-1) {

    -      rtems_ofw_free(buf);
    +      rtems_ofw_free(*buf);
        *buf = NULL;
        return -1;
      }
    @@ -344,7 +344,7 @@ ssize_t rtems_ofw_get_prop_alloc_multi(
      }

      if (rtems_ofw_get_prop(node, propname, *buf, len) == 
-1) {

    -      rtems_ofw_free(buf);
    +      rtems_ofw_free(*buf);
        *buf = NULL;
        return -1;
      }
    @@ -373,7 +373,7 @@ ssize_t rtems_ofw_get_enc_prop_alloc(
      }

      if (rtems_ofw_get_enc_prop(node, propname, *buf, len)
    == -1) {
    -      rtems_ofw_free(buf);
    +      rtems_ofw_free(*buf);
        *buf = NULL;
        return -1;
      }
    @@ -404,7 +404,7 @@ ssize_t 
rtems_ofw_get_enc_prop_alloc_multi(

      }

      if (rtems_ofw_get_enc_prop(node, propname, *buf, len)
    == -1) {
    -      rtems_ofw_free(buf);
    +      rtems_ofw_free(*buf);
        *buf = NULL;
        return -1;
      }

    The above all look fine to me.

    @@ -500,21 +500,21 @@ static phandle_t
    rtems_ofw_get_effective_phandle(
  )
  {
    phandle_t child;
    -  phandle_t ref;
    +  phandle_t ref[1];


    I don't like this. I know this was suggested, but I think it is
    a little ridiculous. This is a false positive since we know that
    sizeof(*buf) == len. I don't know if we can make that an
    assertion. Otherwise, we can just mark this as a false positive
    in coverity. We know the array dereference in this case won't
    overwrite past the bounds of ref.

    Instead of using hard-coded values of 4
    in rtems_ofw_get_enc_prop() you might make it more explicitly
    using sizeof(pcell_t), since that is what you mean.

    Done.


    I would also agree to change the strncpy as Christian identified
    before in rtems_ofw_get_prop().

    Is the reason to avoid strncpy that it ignores the null byte if
    len(dst) <= len(src)?
    If so can I do an explicit null byte append?
    Or is there any other reason?

The reason is that it passes void* pointers. If these are strings, you 
should use char* type. Otherwise, memcpy is more suitable.


It also would be generally safer to overwrite with the NIL character 
to guarantee it is a null-terminated string, if that is expected.


That was not the only reason. Let me pull the relevant lines together 
(reordered and pulled from multiple files):



typedef uint32_t  pcell_t;

rtems_ofw_get_enc_prop(
   phandle_t    node,
   const char  *prop,
   pcell_t *buf,
   size_t   len
)
{
   ...
   rv = rtems_ofw_get_prop(node, prop, buf, len);
   ...
}

rtems_ofw_get_prop(
   phandle_t    node,
   const char  *propname,
   void    *buf,
   size_t   bufsize
{
   ...
     strncpy(buf, prop, bufsize);
   ...
}

Let's say I do the following call:

rtems_ofw_get_enc_prop(node, "name", &foo, sizeof(foo));

In that case the code is using a strncpy to copy into a uint32_t. That's 
not a good idea. What if there is (for example) a value of 0x00110011 in 
the property? strncpy will find one of these two 0 bytes and stop there. 
I'm not sure which one because endianess will have an influence on that 
too. Note that I'm not sure whether using rtems_ofw_get_enc_prob with 
these parameters is a useful call. But it's possible and it's a bad idea 
if it results in an undefined behavior.

spcontext01 failure

2021-02-04 Thread Richi Dubey 
Hi,

When I try to run spcontext01.exe with the master as the source and the
latest build, it fails.

I built a realview_pbx_a9_qemu bsp and ran the program using gdb and qemu
system arm.

Can someone please verify that it is actually failing on the current master?

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

Re: spcontext01 failure

2021-02-04 Thread Richi Dubey 
Also, I forgot to mention that I ran the qemu system arm like this:



:~/quick-start/CurrentMaster/rtems/6/bin$ qemu-system-arm -net none
-nographic -M realview-pbx-a9 -m 256M -kernel
~/quick-start/CurrentMaster/build/bsp/arm-rtems6/c/realview_pbx_a9_qemu/testsuites/samples/unlimited.exe
-smp 4 -s -S

So I believe it is invoking the Scheduler EDF SMP to run the test. Does
this scheduler fail for spcontext01?

On Fri, Feb 5, 2021 at 10:17 AM Richi Dubey  wrote:

> Hi,
>
> When I try to run spcontext01.exe with the master as the source and the
> latest build, it fails.
>
> I built a realview_pbx_a9_qemu bsp and ran the program using gdb and qemu
> system arm.
>
> Can someone please verify that it is actually failing on the current
> master?
>
> Thanks,
> Richi.
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: spcontext01 failure

2021-02-04 Thread Sebastian Huber 

On 05/02/2021 06:26, Richi Dubey wrote:


Also, I forgot to mention that I ran the qemu system arm like this:

:~/quick-start/CurrentMaster/rtems/6/bin$ qemu-system-arm -net none 
-nographic -M realview-pbx-a9 -m 256M -kernel 
~/quick-start/CurrentMaster/build/bsp/arm-rtems6/c/realview_pbx_a9_qemu/testsuites/samples/unlimited.exe 
-smp 4 -s -S


If  you start applications configured for one processor on more than one 
processor on arm, then you have currently an undefined behaviour.


--
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