RE: [ECOS] Multi thread Debugging

[Fabrice Gautier <Fabrice_Gautier at sdesigns dot com>]

A bit more on the subject...

> -----Original Message-----
> From: Fabrice Gautier 
> Subject: RE: [ECOS] Multi thread Debugging
> 
> > -----Original Message-----
> > From: Jonathan Larmour [mailto:jlarmour@redhat.co.uk]
> > Subject: Re: [ECOS] Multi thread Debugging
> > 
> > The stack_ptr member is there for a different purpose than 
> > saved_context.
> > saved_context is for GDB debugging purposes only and 
> provides the full
> > context. AFAIK interrupt_end is the only place that stores 
> > the context in
> > saved_context.
> 
> In fact I've seen that when saved_context member is NULL, the 
> stack_ptr is used instead.

However If I compare the context saved in interrupt_end and the one on
HAL_TRHEAD_SWITCH_CONTEXT are quite different:

In interrupt_end the reg parameter point to the somewhere in the stack where
saved values appears in this order: (regs[0] should be edi if i read
corretly, the register from edi to eax are pushed by a pusha instruction)

edi 
esi
ebp
esp  (the one when entering the irq ie: regs+0x20 ) 
ebx
edx
ecx
eax 
Vector number
eip 
cs

At this point we can see this does not look like  a HAL_SavedRegister
structure while interrupt end does a set_save_context of that.

The registers saved by the HAL_THREAD_SWITCH_CONTEXT are also saved on the
stack.
The stack_ptr point to a HAL_SavedRegister structure so we have:

esp
next_context
ebp
ebx
esi
edi

If I'm not wrong in anything this means that :

1/ The HAL_SavedRegister structure does not contain all the information
needed by the debugger. 

2/ In any case the real thread switch occurs in the
HAL_THREAD_SWITCH_CONTEXT. But when this thread switch is caused by an irq,
we would like the debugger to report the point in the thread where the IRQ
occured. In the case of a thread switch not caused by an IRQ we may consider
reporting the thread as "interrupted" by the call to Cyg_Scheduler::unlock.
In this later case the information saved by the HAL_THREAD_SWITCH_CONTEXT
macro is not enought but many registers(eax,ebx,ecx,edx) do not need to be
saved in a C function call, and eip can be found on the stack (as the
Cyg_Sheduler::unlock return eip value) 

3/ The get_saved_context function is buggy: when the saved_context member is
null (means the thread switch was not caused by an IRQ) it should
reconstruct a valid context using stack_ptr. In fact this is the
HAL_THREAD_GET_SAVED_REGISTERS macro that is buggy. So this is a i386
specific bug. (Good news!)

4/ The HAL_GET_GDB_REGISTERS macro (in fact the corresponding function) is
buggy : it should transmit to gdb the saved context passed as the second
argument. A priori another i386 pc specific bug (Good news!)

Any thought about this? 

-- 
Fabrice Gautier
fabrice_gautier@sdesigns.com 





> > This is interesting - it's not clear to me either how 
> > saved_context is set
> > if threads get rescheduled for any reason other than an interrupt.
> 
> It is not set at all. But get_saved_context get the registers 
> from stack_ptr.
>  
> > Is this a bug?
> 
> It doesn't seem 
>  
> > I wouldn't have thought this would have caused a problem on 
> the scale
> > Fabrice is seeing at any rate.
> 
> The problem may be elsewhere.
> 
> May be here: (Warning, this is i386 PC specific stuff now)
> 
> void hal_get_gdb_registers(target_register_t * d, 
> HAL_SavedRegisters * s)
> {
> 	int i ;
> 
> 	for (i = 0; i < NUMREGS; i++)
> 		d[i] = hal_pc_trap_registers[i] ;
> } 
> 
> This is seen in hal/i386/pc/current/src/plf_stub.c. Note how 
> this function does not use the 's' parameter while all we 
> need is in there. This is called in dbg_getthreadreg and 's' 
> would be either saved_context or stack_ptr.
> 
> I found another version of this function in 
> hal/i386/arch/current/src/i386_stub.c
> 
> #if 0
> void hal_get_gdb_registers(target_register_t * d, 
> HAL_SavedRegisters * s)
> {
> 	d[ESP] = s->esp ;
> 	d[ESP] = s->esp ;
> 	d[EBP] = s->ebp ;
> 	d[EBX] = s->ebx ;
> 	d[ESI] = s->esi ;
> 	d[EDI] = s->edi ;
> 	d[PC] = s->eip ;
> }
> #endif
> 
> This one looks better, but unfortunately there is no s->eip. 
> The HAL_SavedRegister doesn't have one. Unfortunate.
> 
> If someone know why this version has been discarded and what 
> the other is intented to do it sould be fun.
> 
> Thanks
> 
> A+
> 
> -- 
> Fabrice Gautier
> fabrice_gautier@sdesigns.com 
>