[PATCH v1 1/2] score: Enforce stack_end alignment

[Kinsey Moore]

I think this got lost in the deluge. For the second comment, were you suggesting that the allocated size isn't guaranteed to be correctly aligned?

Kinsey

-----Original Message-----
From: devel <devel-bounces at rtems.org> On Behalf Of Kinsey Moore
Sent: Friday, February 19, 2021 11:56
To: Sebastian Huber <sebastian.huber at embedded-brains.de>; devel at rtems.org
Subject: RE: [PATCH v1 1/2] score: Enforce stack_end alignment

On 2021/02/19 01:08, Sebastian Huber wrote:
> On 19/02/2021 07:12, Sebastian Huber wrote:
>> I think the bug is in _TLS_Get_allocation_size(). It assumes
>> CPU_HEAP_ALIGNMENT >= CPU_STACK_ALIGNMENT. It should probably use the
>> maximum of these two values.

The only usage of CPU_HEAP_ALIGNMENT in _TLS_Get_allocation_size() doesn't enforce alignment of the TLS allocation size, this pads the allocation to ensure that the space used by TLS can be aligned properly. In the case of AArch64, both are specified to be 16 bytes, so changing this wouldn't affect the issue that caused this patch to be necessary.

> There is also an issue in the stack allocation, for example
>
> static rtems_status_code _RTEMS_tasks_Allocate_and_prepare_stack(
>    Thread_Configuration    *thread_config,
>    const rtems_task_config *config
> )
> {
>    size_t size;
>
>    thread_config->stack_free = _Stack_Free;
>    size = _Stack_Ensure_minimum( config->storage_size );
>    size = _Stack_Extend_size( size, thread_config->is_fp );
>    thread_config->stack_size = size;
>    thread_config->stack_area = _Stack_Allocate( size );
>
>    if ( thread_config->stack_area == NULL ) {
>      return RTEMS_UNSATISFIED;
>    }
>
>    return RTEMS_SUCCESSFUL;
> }
>
> and
>
> /* Generated from spec:/rtems/task/if/storage-alignment */
>
> /**
>   * @ingroup RTEMSAPIClassicTasks
>   *
>   * @brief This constant defines the recommended alignment of a task storage
>   *   area in bytes.
>   *
>   * @par Notes
>   * Use it with RTEMS_ALIGNED() to define the alignment of a statically
>   * allocated task storage area.
>   */
> #define RTEMS_TASK_STORAGE_ALIGNMENT CPU_HEAP_ALIGNMENT

I'm not sure I follow on this part.

Kinsey