|
From: Josef W. <Jos...@gm...> - 2010-05-31 13:42:38
|
Hi Philippe, what I personally find very interesting in your patch to implement a gdbserver in VG is your solution for external notifcations using ptrace. I would be very much interested to have this to replace my broken polling scheme for a command file in callgrind. Is there a possiblity to extract this feature from your bigger patch? Do I understand it right, that I would need a stripped-down version of vgdb for callgrind_control? Thanks, Josef |
|
From: Philippe W. <phi...@sk...> - 2010-05-31 17:49:43
|
Hello Josef,
> Is there a possiblity to extract this feature from your bigger patch?
For sure, it would be possible to extract the external notification from the gdbserver patch.
This implies to subset vgdb.c but also subset the "valgrind side" of the gdbserver patch.
However, this is quite some changes, not compatible with the full gdbserver patch,
which is a better solution:
* a better integration with gdb is a 'much wanted' feature of the 2005 user survey
(3rd 'wanted' feature, ex-aequo with GUI)
* there are multiple bugzilla entries about weaknesses/bugs of the current --db-attach feature
* the gdbserver patch gives an external notification framework for all tools (not only for callgrind)
* 100% of the current user basis (i.e 3 known users :) of the gdbserver patch have expressed
their enthusiastic satisfaction about the interactive debugging and control provided.
For sure, if despite the above sales speach, there are good technical reasons for which the gdbserver
patch can't be improved and/or integrated, I can subset (and/or help to subset) the vgdb.c + VG side
to the minimum needed for external notifications.
But, for the reasons explained above, my initial preferred approach is an integration of the whole
(or most) of the gdbserver patch, for which I am ready to do the additional work needed as soon
as I get feedback.
Philippe
|
|
From: Philippe W. <phi...@sk...> - 2010-05-31 22:09:09
|
> However, I confess that at the moment, I mainly am interested in the way you did the external notification. > I think the current solution in Callgrind is a hack, and I always was > searching for a better one. Thus, a better solution "just" would be a bug fix (with me > trying to minimize the patch size). Merging a 150k gzipped patch is a completely different > topic. Tom Hughes said he would look at the patch when complete (which it is now, at least functionally) but for sure, integrating gdbserver patch is far of a small bug fix, even if it is only 108k bzipped :). But I took care to limit the impact on the VG core, both in terms of runtime cost and avoiding nasty interactions. > > Hmm. While looking at the patch, I see the discussion that syscall restart after ptrace > does only work on newer Linux kernels. How could gdb work at all if syscall restart did not? > In that case, there would be the need for a reasonable fallback. And I wonder what > that fallback could be, to be better than the current callgrind solution... I tested on an older linux kernel (2.6.18 on a red hat 5.3 IIRC). In this case, the ptraced process can have its syscall interrupted. The behaviour with vgdb.c ptracing or gdb ptracing was similar. The test was to have a process doing an infinite loop of "read" syscall on stdin, with a printf before and after. With an old kernel, both gdb and vgdb were interrupting the read system call. In vgdb.c, giving a 0 value to --max-invoke-ms parameter disables the ptrace technique and so, only the polling technique via the shared memory remains active. => on a newer kernel version (>= 2.6.25 I believe) or with an application that properly handles interrupted system calls, ptrace can be used. Otherwise the fallback is to give --max-invoke-ms=0. With this, the polling by gdbserver is just more efficient than the callgrind polling, but suffers from the same limitation as of today: a syscall blocked process will not be waken up. I have also looked at ptrace on darwin. From what I could see from reading on the net, the ptrace technique should also work on darwin but I think some small changes to the ptrace calls and the "fake call stack" build by vgdb.c must be done. Philippe |
|
From: Julian S. <js...@ac...> - 2010-06-01 07:42:56
|
On Tuesday 01 June 2010, Philippe Waroquiers wrote: > > However, I confess that at the moment, I mainly am interested in the way > > you did the external notification. I think the current solution in > > Callgrind is a hack, and I always was searching for a better one. Thus, a > > better solution "just" would be a bug fix (with me trying to minimize the > > patch size). Merging a 150k gzipped patch is a completely different > > topic. Josef, very approximately, how much of Philippe's patch is the external notification part? 5% ? 50% ? I must say I am in favour of your approach, first to look at and maybe merge just that part, and then see where we are for the full gdbserver thing. Philippe, you asked about licensing. It needs to be GPL2+. We ask for all contribs to be licensed as 2+. J |
|
From: Philippe W. <phi...@sk...> - 2010-06-01 19:44:21
|
> Actually, how does this work at all? I had problems finding a man page where it is exactly described > what should happen when a process waiting in a syscall is "interrupted" by ptrace. The man page > on my system says that on PTRACE_ATTACH/TRACEME, a SIGSTOP signal is sent to the process. Why > is this different to other signals? Why does the kernel need to do any special handling? SIGSTOP and SIGKILL are two special signals : the kernel never passes these signals to the process. If after SIGSTOP, the only actions done by gdb (or vgdb) is to read data, and then continue the process, then nothing special is to be done by the kernel: it just "continues" the syscall that the process was busy doing, because the ptraced process did not execute anything, and "stayed in the same kernel context". But in our case, after SIGSTOP, vgdb (or gdb, if you use gdb to "call code" in the debugged process) will "force" the process to execute some code (e.g. the gdbserver code) a.o. by setting the program counter register and then continuing the process. In this case, the ptraced process has thus left the kernel context in which it was busy executing a syscall. vgdb (or gdb), once the process has finished executed the "forced" code, will set back the program counter to the instruction where the syscall was "busy being done". But that is not exactly the same as just having SIGSTOPped and SIGCONTinued the process: to fully restore the "syscall context" implies the kernel to do something special only handled properly by a recent linux kernel. > > For PTRACE_ATTACH, the man page states "The calling process actually becomes the parent of the > child process for most purposes". I really would like external notifications to always work. > However, the previous sentences suggests that ptrace modifies the environment in a way which > is not always acceptable (?). The parent is changed only during the time the process is being ptraced, which in our case is during the execution of the "forced" code to be invoked (e.g. gdbserver code). Once vgdb (or gdb) detaches from the process, the original parent is restored. I believe all that is quite transparent, as this is used by gdb, strace + I did hundreds of "vgdb 'interrupts'" of firefox, without seeing anything special. > Just for my understanding: "more efficient" because it polls a memory address instead of doing > a system call? Effectively, that is the difference of efficiency. |
|
From: Josef W. <Jos...@gm...> - 2010-06-02 00:42:51
|
On Tuesday 01 June 2010, Philippe Waroquiers wrote: > > Actually, how does this work at all? I had problems finding a man page where it is exactly described > > what should happen when a process waiting in a syscall is "interrupted" by ptrace. The man page > > on my system says that on PTRACE_ATTACH/TRACEME, a SIGSTOP signal is sent to the process. Why > > is this different to other signals? Why does the kernel need to do any special handling? > > SIGSTOP and SIGKILL are two special signals : the kernel never passes these signals to the process. Ok. > If after SIGSTOP, the only actions done by gdb (or vgdb) is to read data, and then continue the process, > then nothing special is to be done by the kernel: it just "continues" the syscall that the process was busy > doing, because the ptraced process did not execute anything, and "stayed in the same kernel context". > > But in our case, after SIGSTOP, vgdb (or gdb, if you use gdb to "call code" in the debugged process) > will "force" the process to execute some code (e.g. the gdbserver code) a.o. by setting > the program counter register and then continuing the process. I still do not understand why vgdb can "force" the process to execute code by just changing the program counter. Is this PC change detected by the kernel and interpreted as a request to put the process back into user context (if it was waiting in a syscall), as it is needed to result in execution of the code? > > For PTRACE_ATTACH, the man page states "The calling process actually becomes the parent of the > > child process for most purposes". I really would like external notifications to always work. > > However, the previous sentences suggests that ptrace modifies the environment in a way which > > is not always acceptable (?). > > The parent is changed only during the time the process is being ptraced, which in our case is during the > execution of the "forced" code to be invoked (e.g. gdbserver code). Once vgdb (or gdb) detaches > from the process, the original parent is restored. I believe all that is quite transparent, as this is used by > gdb, strace + I did hundreds of "vgdb 'interrupts'" of firefox, without seeing anything special. Ok, that sounds good. Josef |
|
From: Philippe W. <phi...@sk...> - 2010-06-01 20:28:46
|
>> Josef, very approximately, how much of Philippe's patch is the external
>> notification part? 5% ?
>
> I would estimate more like 2%. By far the biggest part is the standalone gdb
> remote server (vgdb).
vgdb.c is about 1500 lines (new file)
m_gdbserver.c, pub_core_gdbserver.h, pub_tool_gdbserver.h : about 1500 lines (3 new files).
These 3 files are providing the interface between the rest of valgrind and the gdbserver "protocol box"
(including the "instrument an SBB block for gdbserver").
The protocol box (recuperated from gdb, with small modifications) is a bunch of files, total about 9000 lines.
valgrind*low*.[hc] is about 1500 lines, implementing the interface between the protocol box
and the "VG platform/architecture" (i.e. find the list of threads; read/write memory or registers, ...).
For this, only x86 and amd64 are done, ppc32 and ppc64 files are just stubs. For each new
processor, a file of about 300 lines is needed.
There are about 900 lines of xml files to describe the valgrind shadow registers so that gdb
is made aware of these VG shadow registers (xml also currently only done for x86 and amd64).
The "pure" ptrace external notification and code invocation part is not big, but makes usage of the
infrastructure provided by the rest of the gdbserver patch (e.g. the protocol, the shared memory).
This is e.g. what ensures that an external notification can be done either from a
"standalone vgdb" or from gdb (via the "qRcmd" monitor gdbserver protocol packet).
For example, the memcheck mc.leak_check can be executed from inside gdb (when the
VG process is being debugged) or by doing in a shell the command "vgdb mc.leak_check".
Ths same code in VG will be executed in both cases, the last one only uses a very small
part of the gdbserver protocol.
>
> As far as I understand, there are 2 mechanisms used for the notification:
> (1) regular polling while VG instrumented code is running (similar to callgrind)
> (2) ptrace calling a handler function in the VG process, which forwards a
> request to the polling mechanism, if it sees that a VG thread is active.
> Otherwise, the "ptrace handler" reacts on the request itself.
> For vgdb notifying VG, first (1) is tried, and if there is no answer after 100ms, then
> (2) is used.
> Philippe, is my understanding correct?
>
> Why not directly always do (2)?
Your understanding is correct.
(2) is not the only mechanism kept (and not always done directly) because
* as discussed in the other mail, the ptrace mechanism does only work on recent linux kernel.
So, a fallback is needed for older linux kernel.
* it is only possible to directly invoke some VG core code with ptrace if VG code is currently
not being executed as the VG core code is not (always) re-entrant code.
So, some sort of polling is any case needed for a process always executing code : no way
we can always directly invoke some code, as the current piece of code might not be
re-entrant.
* finally, I guess not all OS will have the same sophistication as the linux syscall restart after
ptrace. So, for such OS, keeping a functional polling fallback is a good idea.
And if ever in very special cases, the ptrace mechanism would not be transparent, then the
"simple" polling (but which is efficient as this is just reading and comparing a counter from
time to time) will work properly for most applications.
Philippe
|
|
From: Josef W. <Jos...@gm...> - 2010-06-01 23:28:08
|
On Tuesday 01 June 2010, Philippe Waroquiers wrote: > >> Josef, very approximately, how much of Philippe's patch is the external > >> notification part? 5% ? > > > > I would estimate more like 2%. By far the biggest part is the standalone gdb > > remote server (vgdb). > > vgdb.c is about 1500 lines (new file) > ... Ah, thanks for the very detailed correction! I more or less only counted the low-level features needed. Why can't we start off with the external notifications be a feature on top of the low-level services, independent from gdbserver? As this is an internal protocol, we can change it when we decide to merge gdbserver later (even with VG releases inbetween). > > As far as I understand, there are 2 mechanisms used for the notification: > > (1) regular polling while VG instrumented code is running (similar to callgrind) > > (2) ptrace calling a handler function in the VG process, which forwards a > > request to the polling mechanism, if it sees that a VG thread is active. > > Otherwise, the "ptrace handler" reacts on the request itself. > > For vgdb notifying VG, first (1) is tried, and if there is no answer after 100ms, then > > (2) is used. > > Philippe, is my understanding correct? > > > > Why not directly always do (2)? > > Your understanding is correct. > (2) is not the only mechanism kept (and not always done directly) because > * as discussed in the other mail, the ptrace mechanism does only work on recent linux kernel. > So, a fallback is needed for older linux kernel. My suggestion was not to get rid of (1), as (2) sometimes forwards the request to (1). The question was more: if I know that I am on a system with ptrace working (simply check the version of the running linux kernel), there is no reason to first do (1), and after 100ms (2). I always could directly start with (2), getting rid of the 100ms latency (?). Josef |
|
From: Philippe W. <phi...@sk...> - 2010-06-01 20:38:37
|
> Philippe, you asked about licensing. It needs to be GPL2+. We ask for all > contribs to be licensed as 2+. Any plan to have VG GPL3+ at short term ? If VG can't switch to GPL3+ and FSF does not give an exception for the gdbserver 7.0 code for valgrind, then I will replace the protocol box by the gdbserver code 6.6, which is GPL2+. For all the code I have written, it is contributed under GPL2+ (I see I have forgotten to put the GPL header in vgdb.c but that is easy to fix :). Philippe |
|
From: Philippe W. <phi...@sk...> - 2010-06-02 05:24:43
|
> I more or less only counted the low-level features needed. > Why can't we start off with the external notifications be a feature > on top of the low-level services, independent from gdbserver? > As this is an internal protocol, we can change it when we decide to merge > gdbserver later (even with VG releases inbetween). For sure, the framework provided by gdbserver stuff is too heavy for just the objective of improving (at short term) the way callgrind is waken up. The ptrace basis can be kept for that, but the current way with which VG gives info to vgdb (FIFOs, shared mem) can probably be replaced by a simpler mechanism (e.g. callgrind could write in a file the needed info such as the address to invoke, where the list of VG threads are, etc). > My suggestion was not to get rid of (1), as (2) sometimes forwards the request to (1). > > The question was more: if I know that I am on a system with ptrace working (simply check the > version of the running linux kernel), there is no reason to first do (1), and after 100ms (2). > I always could directly start with (2), getting rid of the 100ms latency (?). For gdbserver (gdb debugging VG through vgdb relay), I think that in most cases, VG will read the data sent by vgdb faster than if trying to waking it up via ptrace, SIGSTOP, suspend all threads, invoke reading code, resume the threads (as all this are heavy operations) : the "lightweight" polling can be done often enough so that a "running" VG (which is what happens most of the time) will react directly. If the latency is annoying for some applications, effectively, your suggestion is worth trying. What you suggest above can be somewhat tested by giving to VG a huge --vgdb-poll parameter and giving --max-invoke-ms=1 to vgdb. |
|
From: Philippe W. <phi...@sk...> - 2010-06-02 21:19:19
|
> I still do not understand why vgdb can "force" the process to execute code by just changing > the program counter. Is this PC change detected by the kernel and interpreted as a request > to put the process back into user context (if it was waiting in a syscall), as it is needed > to result in execution of the code? My understanding: the kernel scheduler job is to make a process sleep or run basically by saving or restoring the registers of the process to put asleep or to resume. When the process is stopped by ptrace, the SETREGS ptrace modifies the saved registers of the stopped process. When the is process is continued, the kernel restores these saved registers and ensures the processor starts executing instructions at the restored program counter. But this just a guess, I did not look at the linux kernel code. Philippe |