Stack checker broken on PowerPC/virtex BSP - 4.7.99.1 or later - RESOLVED

[Robert S. Grimes]

see below

Joel Sherrill wrote:
>> The only overlap I see is the initial and interrupt stacks overlap with
>> the beginning of the work space, which may be normal?  If not, why is
>> this wrong? Is there some configuration setting wrong here?
>>   
>
> No configuration mistake.  This is a BSP
Didn't think so, but I'm still the rookie here...
>
> The BSP initialization and rtems_initialize_execute_XXX runs using
> the starting stack.  Object control blocks, task stacks, internal
> data structures, etc are allocated from that.  While initializing
> all that, the memory for the Init stack is going to get trashed.
> You are seeing this happen when the first task stack is doped but
> this is just the most brutal overwrite of the memory.
Just to clarify, the two (init and interrupt) stacks need to be outside
of the RTEMS workspace, right?
>
> The interrupt stack also overlaps this and is getting clobbered also
> but this doesn't matter until you get an interrupt that uses stack
> memory that is thought by RTEMS to be a task stack or a data structure
> and clobbers that.
>
> The BSP needs to be fixed to the workspace memory doesn't conflict
> with the interrupt or initial stacks.  Reserve them.
>
> Make sure the BSP is starting the workspace after the high address
> symbol for the interrupt stack.
>
> Look for this in bspstart.c:
>
>    /* round _end up to next 64k boundary for start of workspace */
>    BSP_Configuration.work_space_start = (void *)((((uint32_t)&_end) +
> 0x18000) & 0xffff0000);
>
> It looks like _end in linkcmds should be defined after the definition
> of intrStackPtr not before those stacks.
Yes.  And it is, and is not. Here's the relevant portion of linkcmds:

    __SBSS_END__ = .;
   
    PROVIDE(_bss_start   = ADDR(.bss));
    PROVIDE(_bss_size    = SIZEOF(.bss));
    PROVIDE(bss.size    = SIZEOF(.bss));
    PROVIDE(_data_start  = ADDR(.data));
    PROVIDE(_data_size   = SIZEOF(.data));
    PROVIDE(_text_start  = ADDR(.text));
    PROVIDE(_text_size   = SIZEOF(.text));
    PROVIDE(_end = data.end);                        <<<=== WRONG?!

    .gzipmalloc : {
    . = ALIGN (16);
        _startmalloc = .;
     } >RAM
             

    /*
     * Interrupt stack setup
     */
    IntrStack_start = ALIGN(0x10);
    . += 0x4000;
    intrStack = .;
    PROVIDE(intrStackPtr = intrStack);

    /* Sections for compressed .text and .data         */
    /* after the .datarom section is an int specifying */
    /* the length of the following compressed image    */
    /* Executes once then could get overwritten        */
    .textrom 0x100000 :
    {
        *(.textrom)
        _endloader = .;
    } >RAM

    .datarom :
    {
        _dr_start = .;
        *(.datarom)
        _dr_end = .;
    } >RAM
    dr_len = _dr_end - _dr_start;

  /* align bottom of 32k init stack at a 32k boundary */
  . = . + 0x4000;
  . = ALIGN( 0x8000 );
  stack.start = .;
  . = . + 0x8000;
  stack.end   = .;
  /*
   * Interrupt stack setup
   */
  IntrStack_start = ALIGN(0x10);
  . += 0x4000;
  intrStack = .;
  PROVIDE(intrStackPtr = intrStack);

  PROVIDE(_end = intrStack);                     <<<===  RIGHT?!

Note the two lines marked with email comments starting with <<<===.  For
whatever reason, _end is being defined twice!  Furthermore, the first
(bad) value is being used.  I commented out the first one, and it works
properly!
>
> Then the BSP better be careful to make sure that the
> heap starts BSP_Configuration.work_space_size bytes after that and the
> rest of memory goes to the Heap.
> But right now, the linkcmds says
> _HeapSize defaults to 8M and the code appears to ignore it.
No, the code doesn't - here it is:
    heap_end = heap_start + (uint32_t)&_HeapSize;
    heap_size = (heap_end - heap_start);

> Look at libbsp/shared/m68k/m68kpretaskinghook.c for a way to default
> to all of memory (_HeapSize == 0) or let the user specify a maximum
> size by defining _HeapSize on the command line.
There is a comment to this effect at the end of bsp_start, with
indications that someone had been thinking of doing something like
this.  Right now, it is low priority for me - though perhaps I could do
something, now that it is fresh in my mind.  I do want to wait to hear
from the vacationing Thomas, as he's been working this.
>>  
> I know this doesn't help you but I cover this specific
> problem in the BSP part of the RTEMS Class.  I plan
> to pull up the mailing list archives next week and show
> everyone how weird the results can be from not getting
> the memory map right.  This is such a simple mistake
> that results in weird errors.
Well, here's another example, then!  I certainly would have enjoyed
taking part in the class, but my schedule hasn't really allowed for
it...  ;-(

As for this issue, I'm going to proceed with my fix for the time being,
and continue my active development with 4.7.99.2; this is the extent to
which I can contribute with testing of it.  So what I'm really working
with is 4.7.99.2, plus Chris John's object.h macro fix, and this
linkcmds change;  this is not precisely the 4.8 candidate, but is
clearly better...

As for the linkcmds fix, we should get Thomas' blessing on it, as he's
been putting together the virtex BSP; after all, it is entirely possible
I'm missing something here...

Thanks, Joel!

-Bob