[PATCH v2] tests/validation: Fix 64bit test failure

[Joel Sherrill]

On Wed, Mar 3, 2021 at 3:10 AM Sebastian Huber <
sebastian.huber at embedded-brains.de> wrote:

> On 24/02/2021 18:30, Gedare Bloom wrote:
>
> > On Wed, Feb 24, 2021 at 9:58 AM Sebastian Huber
> > <sebastian.huber at embedded-brains.de>  wrote:
> >> On 24/02/2021 17:54, Gedare Bloom wrote:
> >>
> >>> On Wed, Feb 24, 2021 at 2:52 AM Sebastian Huber
> >>> <sebastian.huber at embedded-brains.de>   wrote:
> >>>> On 16/02/2021 06:51, Sebastian Huber wrote:
> >>>>
> >>>>> The issue can be fixed in two ways.
> >>>>>
> >>>>> 1. We change the API, so that the error condition can happen also on
> >>>>> 64-bit architectures (current patch).
> >>>>>
> >>>>> 2. We change the specification and implementation, so that this error
> >>>>> condition is removed on 64-bit architectures. In the implementation,
> >>>>> this is easy. In the specification, this is a bit more difficult
> since
> >>>>> I would have to introduce a new option which enables or disables
> parts
> >>>>> of the specification based on the word size of the architecture
> >>>>> (similar to RTEMS_SMP). This is the main reason why I didn't fix the
> >>>>> issue immediately
> >>>> How do we want to address this issue? Change the API or the
> >>>> specification, etc.?
> >>>>
> >>> I'd lean toward 1, except without digging in the details I think the
> >>> proposed patch is wrong to use size_t, and the implementation check of
> >>> SIZE_MAX. We shouldn't use size_t for a counting variable, that is
> >>> wrong. size_t has an implied type of number of bytes. The check should
> >>> be against UINT32_MAX if the variable type is uint32_t.
> >> This is exactly the problem. Internally, this uint32_t variable is used
> >> to compute the allocation size in size_t, which is uint64_t on 64-bit
> >> architectures and an overflow cannot happen.
> >>
> > These limits are ridiculous, even on a 32-bit machine no one is going
> > to be allocated a 4 GiB message.
> >
> > We should consider specifying the API more tightly. The maximum
> > message sizes are currently only constrained by the Workspace Size or
> > (for Global) the MPCI message limits. Maybe we should make the
> > maximum_pending_messages as uint16_t or even uint8_t, then the
> > overflow can't happen on 32-bit machine either.
>
> The message size is already size_t. The problem is the
> maximum_pending_messages. I think that limiting the maximum for the
> pending messages to 65535 would be a bit too restrictive.
>

I know of one satellite that used a message queue to enqueue all data to
be downlinked when it was out of site of a ground station. It only was
visible
to the ground station 6 hours a day and the queue could get over 100K
messages.

>
> I will fix the error condition in the specification and adjust the
> implementation accordingly without changing the API.
>

Can the calculation of allocated size be done in terms of uint64_t and
then we can easily check if the allocation is to exceed UINT32_MAX.

Then there are no changes to the API and the code doing this should
be a lot simpler and less prone to error.

--joel

>
> >
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.huber at 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 at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210304/372d23fd/attachment.html>