On Thu, 2014-03-06 at 20:55 +0100, Jan Kratochvil wrote:
On Tue, 04 Mar 2014 11:40:21 +0100, Mark Wielaard wrote:
> +/* Structure used for keeping track of ptrace attaching a thread.
> + Shared by linux-pid-attach and linux-proc-maps. If it has been setup
> + then get the instance through __libdwfl_get_pid_arg. */
> +struct pid_arg
IMO the namespace should be 'struct __libdwfl_pid_arg'.
OK, changed everywhere.
[...]
> --- a/libdwfl/linux-proc-maps.c
> +++ b/libdwfl/linux-proc-maps.c
> @@ -339,34 +339,60 @@ dwfl_linux_proc_find_elf (Dwfl_Module *mod __attribute__
((unused)),
[...]
> + bool detach = false;
> + bool tid_was_stopped = false;
> + struct pid_arg *pid_arg = __libdwfl_get_pid_arg (mod->dwfl);
> + if (pid_arg != NULL && ! pid_arg->assume_ptrace_stopped)
> + {
> + pid_t tid = pid_arg->tid_attached;
> + if (tid != 0)
> + {
> + /* If the pid already is attached we are fine, otherwise
> + just read through the thread that is attached. */
> + if (tid != pid)
> + pid = tid;
The 'if' conditional has no meaning but it is probably meant for possible
better code readability.
No it was just a silly if. I removed it and clarified the comment.
In general I believe reading /proc/PID/maps is safer than the
guessing by
elf_from_remote_memory(), for example because the image may come from mmap()
and not from dlopen(). But I guess it may be the common disagreement.
I agree that using all addresses from /proc/PID/maps, and not just the
base address, would be a good idea. But IMHO elf_from_remote_memory
should still be used for sanity checking the segment mapping addresses
and offsets to make sure the ELF file was mapped in as expected.
I do not mind much using elf_from_remote_memory() instead.
OK, pushed.
Thanks,
Mark