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

[Kinsey Moore]

On Mon, 2021-02-15 at 3:35pm, Martin Erik Werner wrote:
> On Mon, 2021-02-15 at 09:19 -0600, Kinsey Moore wrote:
>> From: Ryan Long <ryan.long at oarcorp.com>
>>
>> The ts-validation-0 test currently fails on 64bit BSPs due to a
>> limitation of the message structure. Changing the max message size to a
>> size_t type and adjusting the expected value in the test resolves this.
>
> This talks about the message size, but the change is to the pending
> count?

You're correct, the change is for maximum_pending_messages, not
maximum_message_size. I was looking at other parts of the code when I
rewrote parts of this commit message this morning.

>> Closes #4179.
> This seems to be the wrong bug reference, is #4197 the correct one?

Also correct, I'll fix this.

> If this change is correct there are a couple of casts left over now
> which maybe should be adjusted as well?:
>
> cpukit/score/src/coremsg.c:64:    (size_t) maximum_pending_messages * buffer_size,
> cpukit/score/src/coremsg.c:89:    (size_t) maximum_pending_messages,

I'll fix this pending the results of commentary below.

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

That's correct.

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

Expecting success would be the easier tack given how the test is configured. It just seemed
odd to expect a different result of a unit test because the size of void* changed, especially
so when I'm comparing the behavior of two mulitilibs on the same hardware.

Kinsey