This is the mail archive of the ecos-discuss@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Stack access violations in eCos


>>>>> "Jifl" == Jonathan Larmour <jifl at eCosCentric dot com> writes:

    Jifl> Erik Aagaard Knudsen wrote:
    >> I would like to ask the administrators of the official eCos repository if
    >> they have a timeframe for applying Larice Roberts patch
    >> (http://sources.redhat.com/ml/ecos-discuss/2003-03/msg00304.html) to the
    >> repository. The errors that this patch corrects cause very nasty and
    >> difficult to find bugs.

    Jifl> I'm sure. I will check it over and (hopefully) apply it
    Jifl> right now.

Hold on. See
http://sources.redhat.com/ml/ecos-discuss/2003-03/msg00287.html, I am
not convinced that this patch is a sensible way to address alignment
issues.

Any change that aligns the base of the stack is insufficient. The
stack pointer can still end up misaligned because of e.g.
CYGNUM_KERNEL_THREADS_STACK_CHECK_DATA_SIZE. Offhand I can think of
three ways of addressing the problem:

1) invent a new way of defining stacks, e.g.:

       CYG_STACK(thread1_stack, 2048);

   This would expand to something like

        unsigned char thread1_stack[...] (possibly an alignment directive);

   using some expression for the array size that takes into account
   stack checking data, per-thread variables, architectural
   restrictions, and possibly minimum stack requirements. For some but
   not all architectures there may be an alignment attribute of some
   sort.

   This would be fine for globals, but unlikely to work well for
   stacks embedded in larger data structures.

   Introducing something like this would require changing all existing
   stack definitions in eCos, including all the testcases.

2) adjust the stack pointer during thread creation, e.g.

    register CYG_WORD* _sp_ = ((CYG_WORD*)((_sparg_) &~15));

   This could be done in certain HAL's, e.g. I already do this in
   HAL_THREAD_INIT_CONTEXT() in the synthetic target. Or possibly it
   could be done more centrally in the kernel's
   Cyg_HardwareThread::attach_stack(), using an alignment #define
   provided by the HAL. Or optionally provided by the HAL, with the
   kernel using a #ifdef. Or possibly we have a default alignment of
   e.g. 4.

   This would be a much smaller change than patching all the testcases
   with CYG_STACK macros or alignment directives, and probably more
   robust. There is a risk that some bytes of the provided stack will
   be wasted, but if the application developer is careful then the
   size will be just right and aligning the stack pointer has no
   effect. There are also a couple of extra cycles to do this
   alignment, but only during thread creation.

3) ignore the problem and force application developers to provide
   sensible stacks. That is fine in theory, but does not work well for
   code that is supposed to be portable. e.g. the testcases.

Personally I favour option 2. For carefully written applications it
involves no wasted memory, just a couple of extra cycles during thread
creation. However other maintainers may disagree, so further comments
are invited.

Regarding other aspects of the patch, aligning arrays that will end up
being used for structures/objects, see
http://sources.redhat.com/ml/ecos-discuss/2003-03/msg00299.html
In most cases it should be possible to have a much cleaner solution
than adding alignment directives, and let the compiler do the work.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts

-- 
Before posting, please read the FAQ: http://sources.redhat.com/fom/ecos
and search the list archive: http://sources.redhat.com/ml/ecos-discuss


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]