This is the mail archive of the ecos-patches@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: Fix for setting/clearing breaks take 2


John Newlin wrote:
My previous patch had a small bug in it. It really works this time. ;)

Can you give a bit more background on this? This patch doesn't seem right to me.... surely you should be defining things so that "*(t_inst*)pc = (t_inst)HAL_BREAKINST;" will work?


If there is a problem setting the breakpoint due to something not being aligned appropriately, then the __read/write_mem_safe will not necessarily have completed.

Can you tell me what the size of HAL_BREAKINST is? And the alignment requirements?

Jifl

------------------------------------------------------------------------

Index: hal/common/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/common/current/ChangeLog,v
retrieving revision 1.103
diff -c -r1.103 ChangeLog
*** hal/common/current/ChangeLog 22 Apr 2004 15:26:33 -0000 1.103
--- hal/common/current/ChangeLog 21 May 2004 22:18:31 -0000
***************
*** 1,3 ****
--- 1,13 ----
+ 2004-05-19 John Newlin <jnewlin@stretchinc.com>
+ + * src/hal_stub.c:
+ (cyg_hal_gdb_interrupt) + (cyg_hal_gdb_remove_break): Changed both to use
+ _read_mem_safe/__write_mem_safe for inserting a breakpoint, and
+ restoring the original instruction.
+ The Xtensa architecture (and others maybe?) can have unaligned
+ instructions, which caused unaligned load/store exception.
+
2004-04-22 Jani Monoses <jani@iv.ro>
* cdl/hal.cdl :
Index: hal/common/current/src/hal_stub.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/common/current/src/hal_stub.c,v
retrieving revision 1.38
diff -c -r1.38 hal_stub.c
*** hal/common/current/src/hal_stub.c 6 May 2003 21:00:20 -0000 1.38
--- hal/common/current/src/hal_stub.c 21 May 2004 22:18:31 -0000
***************
*** 281,286 ****
--- 281,288 ----
void cyg_hal_gdb_interrupt (target_register_t pc)
{
+ t_inst break_inst = HAL_BREAKINST;
+ CYGARC_HAL_SAVE_GP();
// Clear flag that we Continued instead of Stepping
***************
*** 290,298 ****
cyg_hal_gdb_remove_break( (target_register_t)break_buffer.targetAddr );
if (NULL == break_buffer.targetAddr) {
! break_buffer.targetAddr = (t_inst*) pc;
! break_buffer.savedInstr = *(t_inst*)pc;
! *(t_inst*)pc = (t_inst)HAL_BREAKINST;
__data_cache(CACHE_FLUSH);
__instruction_cache(CACHE_FLUSH);
--- 292,306 ----
cyg_hal_gdb_remove_break( (target_register_t)break_buffer.targetAddr );
if (NULL == break_buffer.targetAddr) {
! // Not always safe to read/write directly to program
! // memory due to possibly unaligned instruction, use the
! // provided memory functions instead.
! __read_mem_safe(&break_buffer.savedInstr, (t_inst*)pc, HAL_BREAKINST_SIZE);
! __write_mem_safe(&break_inst, (t_inst*)pc, HAL_BREAKINST_SIZE);
! ! // Save the PC where we put the break, so we can remove
! // it after the target takes the break.
! break_buffer.targetAddr = (t_inst*)pc;
__data_cache(CACHE_FLUSH);
__instruction_cache(CACHE_FLUSH);
***************
*** 308,314 ****
return 0;
if ((t_inst*)pc == break_buffer.targetAddr) {
! *(t_inst*)pc = break_buffer.savedInstr;
break_buffer.targetAddr = NULL;
__data_cache(CACHE_FLUSH);
--- 316,323 ----
return 0;
if ((t_inst*)pc == break_buffer.targetAddr) {
! ! __write_mem_safe(&break_buffer.savedInstr, (t_inst*)pc, HAL_BREAKINST_SIZE);
break_buffer.targetAddr = NULL;
__data_cache(CACHE_FLUSH);


--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine


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