This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB 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: Patch : gdbserver get_image_name on CE


On Wednesday 01 July 2009 19:40:48, Danny Backx wrote:
> > And you never did explain what was broken with get_image_name
> > that you had to fix, as I asked $n emails ago, right?
> 
> Err. I am almost certain that the return value of GetProcessMemory isn't
> a reliable way to verify success. 

Thanks!  You know that I couldn't guess what you were thinking or were
almost certain, right?

> Its final parameter is. Or at least 
> that is my experience. Of course, MSDN docs don't mention this.

I'm not convinced of this.  I think you must stumbling on some other bug,
or a bug in the device.  MSDN docs mention this:

http://msdn.microsoft.com/en-us/library/ms680553(VS.85).aspx

"Reads data from an area of memory in a specified process. The entire area to be read
must be accessible or the operation fails."

That's why we read data bytewise to know the length or the string first.
So what you're doing may work for you, but it may fail for someone
else (other device, binary, etc.).

It makes *no sense* to be able to read the data in one go,
but not be able to read it bytewise.  This would mean that ReadProcessMemory
could fail in many other circumstances --- it is used when gdb wants to
read memory of off the inferior...

So, please, now that you have a gdbserver that works, debug gdbserver
with gdbserver, and step through that routine (the old implementation),
and see where/why it is failing.  Posting a log of that debug session
showing the failure would do wonders in convincing me where is the bug.

Hey, you may even have a compiler bug, right?

> I tested this on ARM and on i386. Real devices, not emulators. The ARM
> implementation worked as it was, and continued to work after my fix. The
> i386 version didn't work before, does work after the fix.
> 
> Is this sufficient explanation?

I'm not convinced yet.  :-)

I'll see about cleaning up the bits of your patch that are OK and
apply them (you still aren't following the code conventions).  Then
we can focus on just this bit.

-- 
Pedro Alves


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