[PATCH v2] tests/validation: Fix 64bit test failure
Sebastian Huber
sebastian.huber at embedded-brains.de
Tue Feb 16 05:51:49 UTC 2021
On 15/02/2021 22:34, Martin Erik Werner wrote:
> [...]
> I'm however wondering if this is the right way to fix this...
>
> I'm guessing that the failure mentioned is based on this specification
> in rtems-central : spec/rtems/message/req/construct-errors.yml
>
> 393 - enabled-by: true
> 394 post-conditions:
> 395 Status: InvNum
> 396 pre-conditions:
> 397 Area: all
> 398 AreaSize: all
> 399 Id:
> 400 - Id
> 401 MaxPending:
> 402 - Big
> 403 MaxSize:
> 404 - Valid
> 405 Name:
> 406 - Valid
> 407 Queues:
> 408 - Avail
>
> Which in practice seems to specify that
>
> rtems_message_queue_create
> (
> name,
> UINT32_MAX /* count */,
> 1 /* size */,
> attribute_set,
> &id
> );
>
> must fail with RTEMS_INVALID_NUMBER due to
>
> 117 - name: Big
> 118 test-code: |
> 119 ctx->config.maximum_pending_messages = UINT32_MAX;
> 120 text: |
> 121 The maximum number of pending messages of the message queue configuration
> 122 shall be big enough so that a calculation to get the message buffer
> 123 storage area size overflows.
>
> which in the code looks like
>
> /* Make sure the memory allocation size computation does not overflow */
> if ( maximum_pending_messages > SIZE_MAX / buffer_size ) {
> return STATUS_MESSAGE_QUEUE_INVALID_NUMBER;
> }
>
> But when the SIZE_MAX is a 64bit size_t, then UINT32_MAX * (1 + buffer
> overhead) cannot reasonably overflow SIZE_MAX, so this will report
> success instead of the expected invalid number which is the failure
> seen in the validation test, is that correct?
Yes, this is correct.
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.
>
>
> If so, it seems very odd to change the interface just to allow this
> failure to occur.
>
> Would it be possible to instead specify that if
>
> SIZE_MAX >= UINT32_MAX * (1 + buffer overhead)
>
> then this case should be skipped, or expects success?
I would have probably fixed the issue without changing the API.
If we change the API, it should be consistent and all unnecessary cast
should be removed. For example
rtems_status_code rtems_message_queue_create(
rtems_name name,
uint32_t count,
size_t max_message_size,
rtems_attribute attribute_set,
rtems_id *id
);
should change as well.
--
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/
More information about the devel
mailing list