On Sat, Jul 26, 2008 at 1:59 AM, Duane Ellis <[EMAIL PROTECTED]> wrote:
> Hello - I 've come across a few things that make no sense and seem wrong.
There is a lot of history here and I think that there could be some cleanup
and simplifications now that a targets responsibilities have crystalized more.
Also, there is some desire to make targets more instruction set independent
in their definition.
There is a desire to stay away from C/C++, but of course object oriented
ideas w/inheritance comes to mind w/polymorphism.
A target can be a CPU(different instruction sets and subtypes of instruction
sets), non-CPU families, flash chips, FPGAs, etc.
Other contributors are *not* keen on C/C++. See earlier discussion.
Here is a thought: lets try to strip down the target definitions and
push more into scripting :-) That way we at least get a crisper
definition of targets. This should be good in terms of adding
support for non-ARM targets too.
>
> ===========================
> (a) example - top part of "target.c' - target_init()
>
> does this:
> target->type->examined = 0;
>
> Generally, the idea of "type" is a C version of a C++ method table.
> (or what ever its you call it)
>
> That I would think - should be *READONLY* - because
> target->type->examined
> is for *ALL* targets of that type.
>
> *NOT* the specific target in question.
>
> I would have expected: target->examined instead.
>
> In effect, "target_type_t" should be a CONST... should it not?
I would like target to be a straight C++ class, so we could use
inheritance, resource tracking & exceptions. :-)
>
> ===========================
> (b) I have been burned by this a few times - GDB goes out in La-La Land..
>
> In src/server/gdbserver.c - gdb_read_memory_packet()
>
> If the requested address + the length causes it to wrap... GDB is
> generally lost and should be told to pound sand.
>
> This is really true @ startup - if the program counter is some how
> reported as -2
> (Why it is - I do not know.... but it happens some times for me - I
> wish i knew)
>
> And at that point - GCC appears to proceed to try to disassemble
> 4gig of memory...
Nice... GDB is clearly busted here.
Perhaps GDB will work, if we fail to read that memory?
The right place to fix this would be in target_read/write_memory though...
How's the attached patch?
>
> I added this - it seems to help - but I am sure about it. If it is
> obvious - I suggest someone fix it.
>
> // about line 1050 in gdb_server.c
> if( (addr+len) < addr ){
> /* memory wraps! */
> retval == ERROR_TARGET_DATA_ABORT;
> } else {
> retval = target_read_buffer(target, addr, len, buffer);
> }
I want this fixed in target_read/write_buffer()...
Note that we *have to* fail the packet if the requested amount of memory
is more than a couple of k's. GDB has code to split up memory read/write
packets, so anything above this fragmentation limit really is a pathological
sign.
--
Øyvind Harboe
http://www.zylin.com/zy1000.html
ARM7 ARM9 XScale Cortex
JTAG debugger and flash programmer
Index: C:/workspace/openocd/src/target/target.c
===================================================================
--- C:/workspace/openocd/src/target/target.c (revision 873)
+++ C:/workspace/openocd/src/target/target.c (working copy)
@@ -967,6 +967,8 @@
int target_write_buffer(struct target_s *target, u32 address, u32 size, u8
*buffer)
{
int retval;
+ LOG_DEBUG("writing buffer of %i byte at 0x%8.8x", size, address);
+
if (!target->type->examined)
{
LOG_ERROR("Target not examined yet");
@@ -973,7 +975,12 @@
return ERROR_FAIL;
}
- LOG_DEBUG("writing buffer of %i byte at 0x%8.8x", size, address);
+ if (address+size<address)
+ {
+ /* GDB can request this when e.g. PC is 0xfffffffc*/
+ LOG_ERROR("address+size wrapped(0x%08x, 0x%08x)", address,
size);
+ return ERROR_FAIL;
+ }
if (((address % 2) == 0) && (size == 2))
{
@@ -1036,6 +1043,8 @@
int target_read_buffer(struct target_s *target, u32 address, u32 size, u8
*buffer)
{
int retval;
+ LOG_DEBUG("reading buffer of %i byte at 0x%8.8x", size, address);
+
if (!target->type->examined)
{
LOG_ERROR("Target not examined yet");
@@ -1041,8 +1050,13 @@
LOG_ERROR("Target not examined yet");
return ERROR_FAIL;
}
-
- LOG_DEBUG("reading buffer of %i byte at 0x%8.8x", size, address);
+
+ if (address+size<address)
+ {
+ /* GDB can request this when e.g. PC is 0xfffffffc*/
+ LOG_ERROR("address+size wrapped(0x%08x, 0x%08x)", address,
size);
+ return ERROR_FAIL;
+ }
if (((address % 2) == 0) && (size == 2))
{
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development
