On Wed, Mar 3, 2021 at 10:56 AM Sebastian Huber <sebastian.hu...@embedded-brains.de> wrote: > > > On 03/03/2021 17:04, Gedare Bloom wrote: > >> +#if CPU_STACK_ALIGNMENT > CPU_HEAP_ALIGNMENT > >> + stack_align = CPU_STACK_ALIGNMENT; > >> + stack_begin = (char *) RTEMS_ALIGN_UP( (uintptr_t) stack_begin, > >> stack_align ); > >> + stack_end = (char *) RTEMS_ALIGN_DOWN( (uintptr_t) stack_end, > >> stack_align ); > > Why align down here? Why align the end at all? > Most stacks grow down, so the alignment of the end address is important.
OK, I think we had this discussion recently also on related patches. It might be good to include a comment here that this code aims to accommodate stacks growing either direction. > > Is there a requirement > > for the stack area to be a multiple of the stack alignment? I guess we > > can lose some bytes and return less than config->stack_size as a > > result of this? > > The CPU_STACK_ALIGNMENT should be set to the stack alignment required by > the ABI (in SMP configurations, it should be probably at least cache > aligned, but this is another topic). In this case all stack frames will > be on such a boundary and you can't use these extra bytes. Independent > of this, you should probably not calculate your stack size so that you > actually use the last byte of your stack. > OK, but it's not necessary to waste those bytes, when we can align the stack_begin area of the stack before we calculate the stack_end, if a stack size is requested that is a multiple of the CPU_STACK_ALIGNMENT (which is likely). It's fine either way, pretty minor space optimization/waste problem. I could see someone doing something like putting an application CANARY value at the end (bottom/begin) of their stack. mystack[end - size] = CANARY; that will give some unexpected results when the stack end has been aligned down, and the size is larger than the end-begin due to alignment. Just a hunch. Maybe I'm making a big deal out of nothing. > Also the changed _Stack_Extend_size() includes the alignment overhead. > > > > > Shouldn't we better do... > > #if CPU_STACK_ALIGNMENT > CPU_HEAP_ALIGNMENT > >> + stack_align = CPU_STACK_ALIGNMENT; > >> + stack_begin = (char *) RTEMS_ALIGN_UP( (uintptr_t) config->stack_area, > >> stack_align ); > > #else > > stack_begin = config->stack_area; > No, I think the code in the patch is fine, however, I will add an assert > after the block to ensure that begin and end are aligned. ok Is there no guarantee that stack_size == config->stack_size? > > #endif > > stack_end = stack_begin + config->stack_size; > > > > So we always return the requested stack size? > > > -- > 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
