[ECOS] stack base pointer not align to 4 bytes

[ECOS] stack base pointer not align to 4 bytes

[Xiaochen Zhou]

Hi, All

I want to add CYGFUN_KERNEL_THREADS_STACK_CHECKING to check stack overflow, but found eCos ASSERT about "ASSERT FAIL: <2>thread.inl[212]void Cyg_HardwareThread::attach_stack() stack base alignment"

I view the map file, 

00133f4c b cli_stack
0013bf4c b cli_thread_h
0013bf50 b cli_thread
0013c044 b ulCmdLen
0013c048 b ioBuf             <----------- the C code is "static uint8 ioBuf;"
0013c049 b g_IC_Stack        <---------- not align to 4 bytes
0013d04c b g_pIC_Handle
0013d050 b g_sIC_ThreadObj

Here is my C file,

#define IC_PROCESS_NAME "Syslog_Deamon"
#define IC_PRIORITY     20
#define IC_STACK_SIZE   4096

static char g_IC_Stack[IC_STACK_SIZE]; 
static cyg_handle_t g_pIC_Handle;
static cyg_thread g_sIC_ThreadObj;
cyg_handle_t g_pIC_MailboxIf;
cyg_mbox g_sIC_Mailbox;

void IC_X_Startup(void)
{
    cyg_thread_create(
        IC_PRIORITY,
        IC_Main, 
        0, 
        IC_PROCESS_NAME, 
        g_IC_Stack, 
        IC_STACK_SIZE, 
        &g_pIC_Handle, 
        &g_sIC_ThreadObj);

    cyg_thread_resume(g_pIC_Handle); 

}

If I change the line static char g_IC_Stack[IC_STACK_SIZE]; ----------> static int g_IC_Stack[IC_STACK_SIZE/4]; everything is OK.

I think this is my fault, but when I grep the eCos source tree, thera are many files using stack definition like me( for example: net/tcpip/current/src/ecos/support.c). And the kernel source code(Cyg_HardwareThread::attach_stack()) did not adjust the alignment automaticly. 

My fault or eCos fault? Thanks!

BTW:
My platform is ARM9 and arm-elf-gcc version is 3.2.1.

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

RE: [ECOS] stack base pointer not align to 4 bytes

[Chris Zimman]

> If I change the line static char g_IC_Stack[IC_STACK_SIZE]; ---------->
> static int g_IC_Stack[IC_STACK_SIZE/4]; everything is OK.
> 
> I think this is my fault, but when I grep the eCos source tree, thera
> are many files using stack definition like me( for example:
> net/tcpip/current/src/ecos/support.c). And the kernel source
> code(Cyg_HardwareThread::attach_stack()) did not adjust the alignment
> automaticly.
> 
> My fault or eCos fault? Thanks!

It's not really a 'fault' either way, but if you have it declared as char,
it's definitely not guaranteed to be word aligned.  I think the examples you
point out in eCos are probably bugs, at least on platforms that need aligned
access.

An alternative means for specifying alignment is to use
__attribute__((aligned)), eg.

static char g_IC_Stack[IC_STACK_SIZE] __attribute__((aligned(4)));

--Chris


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

Re: [ECOS] stack base pointer not align to 4 bytes

[Xiaochen Zhou]

Hi Chris,

I think the best way to solve the issue should adjust the stack alignment automaticly by kernel. For most of ecos users do not care the stack alignment. If they create their own applications with the template of package/example/twothreads.c(or serial.c, simple-alarm.c), I don't how many hidden bugs in ecos world!

Xiaochen Zhou

----- Original Message ----- 
From: "Chris Zimman" <czimman@bloomberg.com>
To: "Xiaochen Zhou" <zhouxiaochen@h3c.com>; <ecos-discuss@ecos.sourceware.org>
Sent: Friday, May 23, 2008 6:32 PM
Subject: RE: [ECOS] stack base pointer not align to 4 bytes


> If I change the line static char g_IC_Stack[IC_STACK_SIZE]; ---------->
> static int g_IC_Stack[IC_STACK_SIZE/4]; everything is OK.
> 
> I think this is my fault, but when I grep the eCos source tree, thera
> are many files using stack definition like me( for example:
> net/tcpip/current/src/ecos/support.c). And the kernel source
> code(Cyg_HardwareThread::attach_stack()) did not adjust the alignment
> automaticly.
> 
> My fault or eCos fault? Thanks!

It's not really a 'fault' either way, but if you have it declared as char,
it's definitely not guaranteed to be word aligned.  I think the examples you
point out in eCos are probably bugs, at least on platforms that need aligned
access.

An alternative means for specifying alignment is to use
__attribute__((aligned)), eg.

static char g_IC_Stack[IC_STACK_SIZE] __attribute__((aligned(4)));

--Chris

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

Re: [ECOS] stack base pointer not align to 4 bytes

[Xiaochen Zhou]

Here is the modified kernel/current/include/thread.inl fiile to check and asjust stack alignment automaticly:

inline void Cyg_HardwareThread::attach_stack(CYG_ADDRESS s_base_p, cyg_uint32 s_size_p)
{
    CYG_ADDRESS s_base = s_base_p;
    cyg_uint32 s_size = s_size_p;

    // Check and adjust stack alignment automaticly
    if( 0 != ((sizeof(CYG_WORD)-1) & (cyg_uint32)s_base)) {
        s_base += sizeof(cyg_uint32);
        s_base &= ~(sizeof(CYG_WORD)-1);
        s_size -= sizeof(cyg_uint32);
    }
    if( 0 != ((sizeof(CYG_WORD)-1) & (cyg_uint32)s_size)) {
        s_size &= ~(sizeof(CYG_WORD)-1);
    }

    ....
    ....
    ....
}

Xiaochen Zhou

----- Original Message ----- 
From: "Chris Zimman" <czimman@bloomberg.com>
To: "Xiaochen Zhou" <zhouxiaochen@h3c.com>; <ecos-discuss@ecos.sourceware.org>
Sent: Friday, May 23, 2008 6:32 PM
Subject: RE: [ECOS] stack base pointer not align to 4 bytes


> If I change the line static char g_IC_Stack[IC_STACK_SIZE]; ---------->
> static int g_IC_Stack[IC_STACK_SIZE/4]; everything is OK.
> 
> I think this is my fault, but when I grep the eCos source tree, thera
> are many files using stack definition like me( for example:
> net/tcpip/current/src/ecos/support.c). And the kernel source
> code(Cyg_HardwareThread::attach_stack()) did not adjust the alignment
> automaticly.
> 
> My fault or eCos fault? Thanks!

It's not really a 'fault' either way, but if you have it declared as char,
it's definitely not guaranteed to be word aligned.  I think the examples you
point out in eCos are probably bugs, at least on platforms that need aligned
access.

An alternative means for specifying alignment is to use
__attribute__((aligned)), eg.

static char g_IC_Stack[IC_STACK_SIZE] __attribute__((aligned(4)));

--Chris


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

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

RE: [ECOS] stack base pointer not align to 4 bytes

[Paul D. DeRocco]

> From: Xiaochen Zhou
>
> Here is the modified kernel/current/include/thread.inl fiile to
> check and asjust stack alignment automaticly:
>
> inline void Cyg_HardwareThread::attach_stack(CYG_ADDRESS
> s_base_p, cyg_uint32 s_size_p)
> {
>     CYG_ADDRESS s_base = s_base_p;
>     cyg_uint32 s_size = s_size_p;
>
>     // Check and adjust stack alignment automaticly
>     if( 0 != ((sizeof(CYG_WORD)-1) & (cyg_uint32)s_base)) {
>         s_base += sizeof(cyg_uint32);
>         s_base &= ~(sizeof(CYG_WORD)-1);
>         s_size -= sizeof(cyg_uint32);
>     }
>     if( 0 != ((sizeof(CYG_WORD)-1) & (cyg_uint32)s_size)) {
>         s_size &= ~(sizeof(CYG_WORD)-1);
>     }
>
>     ....
>     ....
>     ....
> }

It seems to me that this is the wrong place to fix this, because the need
for a particular alignment is hardware-dependent. The comment on this
function indicates that there is a macro that will be used instead, if the
HAL defines it. That seems like the place to do this sort of thing.

--

Ciao,               Paul D. DeRocco
Paul                mailto:pderocco@ix.netcom.com


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

Re: [ECOS] stack base pointer not align to 4 bytes

[Xiaochen Zhou]

>> From: Xiaochen Zhou
>>
>> Here is the modified kernel/current/include/thread.inl fiile to
>> check and asjust stack alignment automaticly:
>>
>> inline void Cyg_HardwareThread::attach_stack(CYG_ADDRESS
>> s_base_p, cyg_uint32 s_size_p)
>> {
>>     CYG_ADDRESS s_base = s_base_p;
>>     cyg_uint32 s_size = s_size_p;
>>
>>     // Check and adjust stack alignment automaticly
>>     if( 0 != ((sizeof(CYG_WORD)-1) & (cyg_uint32)s_base)) {
>>         s_base += sizeof(cyg_uint32);
>>         s_base &= ~(sizeof(CYG_WORD)-1);
>>         s_size -= sizeof(cyg_uint32);
>>     }
>>     if( 0 != ((sizeof(CYG_WORD)-1) & (cyg_uint32)s_size)) {
>>         s_size &= ~(sizeof(CYG_WORD)-1);
>>     }
>>
>>     ....
>>     ....
>>     ....
>> }
> 
> It seems to me that this is the wrong place to fix this, because the need
> for a particular alignment is hardware-dependent. The comment on this
> function indicates that there is a macro that will be used instead, if the
> HAL defines it. That seems like the place to do this sort of thing.
> 
> --
> 
> Ciao,               Paul D. DeRocco
> Paul                mailto:pderocco@ix.netcom.com
> 
> 

The comment on the function point the codes below, it support stack grown up and down architecture.

// -------------------------------------------------------------------------
// Attach a stack to this thread. If there is a HAL defined macro to
// do this, then we use that, otherwise assume a falling stack.
inline void Cyg_HardwareThread::attach_stack()
{
    .....
#ifdef HAL_THREAD_ATTACH_STACK

    HAL_THREAD_ATTACH_STACK(stack_ptr, stack_base, stack_size);
   
#else

    stack_ptr = stack_base + stack_size;

#endif

    .....
}

Xiaochen Zhou

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