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: [RFC] New gdbserver Win32 interrupt code



Yes, please.  Independent fixes should be split into different
patches.  It is easier to review, and if it turns out
something brakes, it should be possible to easilly revert
the independant changes.  There are tools that make the
management of patch series easier.  I use quilt myself
for that.

Ok, acknowledged. :)

I'd like to split that playing with the priorities out of the
first patch, because it will be different in CE.  Focusing a
patch on just that will be clearer.

Okay, but it's important to have that priority stuff in Win32 final version to avoid gdbserver interfere with child, if it for example pauses/resumes its own threads while gdbserver tries to interrupt.
I think it's also important to have gdbserver to go real-time the least possible amount of time. Go real-time, pause child and restore priority, as quick as possible.


I'd like to know what you are referring to when you say clean up the rest of my patch. I took care of properly formatting code, full stops, and even fixed some previous erroneous code indentation and formatting. I'd like to know, what is it I missed that you are talking about?

Oh, nothing really serious, don't worry. A few spaces around operators, and before '(' here and there, and usage of /* * * * for comment blocks. After a while of staring at GNU code, it starts to really stand out. What's not conforming to the standards will just look weird. :-)

Oops.
I did just see some of those, even though I checked it before sending patch.
Truth is I don't usually code in GNU style, just for this. At work use one style and my personal coding another.


I'd also like to know what ifdefing in the patch I submitted that you don't like. The NEW_INT macro is the only one I sent, and I left it
there just in case this new interrupt code isn't suitable for all, but can be removed easily.



There is no need for this macro. We either entirely switch to this mechanism, or we add it as a fallback if the current ones fail.

Ok, then just it's up to removing the old code, if necessary.
One thing I'd like to comment that I have noticed about this new method is that when I interrupt now it seems that child threads when paused in system calls seem to be at different locations than with old method.
If you get the chance please take a look at the Win32 versio, as I am not completely sure, but it seems that this method is like more precise in where exactly the child gets interrupted, like if it works better in the sense that child gets interrupted precisely where child is running, and not where Windows wants to interrupt it. Not that this is usefull for anything, but it's curious.



Also, note that I did not include this synthetic_child_interrupt into separate architecture files (i386/arm) because of one of the benefits of this pausing method, which is portability among Windows versions, and this new code is compatible from Win95 and newer, however I don't know about WinCE.

Well, WinCE runs on i386 too, so splitting by machine isn't exactly right. (although mingw32ce only supports ARM currently).

So, the version I wrote is not so straightforward to use for WinCE.
I thought it would be better to interrupt WinCE also this way.


I'm interested in this for WinCE.  Whoever needs this for
native debugging should submit a patch and it will be
reviewed.  We can't expect the maintainers to scratch our
own itches :-)
I think that if you look back a long way in the history of
win32-nat.c you'll see that once a similar method was used.

...and maybe it got dumped for some reason??
That would be nice to know.
Maybe it has to do with the fact that the child being interrupted this way is not actually interrupted but paused, so that any other app running in the system could resume the child even if gdbserver has interrupted it.


Anyway, do your WinCE stuff, because no matter what I do here, you'll have to come with a solution that'll work for both Win32 and WinCE.
Once it's done on gdbserver, then maybe we can think about making a patch for native gdb.





  of 1.  I got a chance to look at the logs of the native
  WinCE debugger, and could infer that it also takes care of
  this correctly.  The way they do it, is to read the current
  suspend count by always doing a SuspendThread + ResumeThread
  sequence on a debug event (ResumeThread return the current
  suspend count).
But isn't this wrong too, according to my explanation in my previous message?
The problem actually is that when resuming a thread in win32-nat.c, in function continue_one_thread, which resumes a thread suspend_count times, which itself was being set to the number of suspends of the thread as reported by Windows, so for example if the client app had a thread, say suspended 2 times, and then gdbserver suspends the thread again, the internal gdbserver count would be 3, and when resuming it, gdbserver would wake it up three times, which is wrong because the child wanted it to be suspended 2 times.



They must be storing the current suspend count, and not resuming past it. What's important is that they also though about it, or so it seemed from the logs.

But there's no doubt in that when resuming a child thread, suspend_count is the number of times it gets resumed. This suspend_count as I see it is the number of child suspends, plus the number of gdb suspends, which is wrong, unless there's something I missed in the code.

Regards,
Leo.



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