On Mon, 21 Oct 2013 00:23:34 +0200, Mark Wielaard wrote:
> On Sun, Oct 13, 2013 at 12:24:55PM +0200, Jan Kratochvil wrote:
> > * dwfl_frame_pid.c: New file.
> > * dwfl_frame_core.c: New file.
>
> Nitpick. Since these files don't actually contain the public
> functions dwfl_frame_pid or dwfl_frame_core they might be better
> named something without the dwfl_ prefix (linux-pid-attach.c and
> linux-core-attach.c, or libdwfl_attach_pid/core.c maybe?)
done:
dwfl_frame_core.c -> linux-core-attach.c
dwfl_frame_pid.c -> linux-pid-attach.c
> > diff --git a/libdwfl/dwfl_frame_core.c b/libdwfl/dwfl_frame_core.c
> > +struct core_arg
> > +{
> > + Elf *core;
> > + Elf_Data *note_data;
> > + size_t thread_note_offset;
> > + Ebl *ebl;
> > +};
> > +
> > +struct thread_arg
> > +{
> > + struct core_arg *core_arg;
> > + size_t note_offset;
> > +};
> > +
> > +static bool
> > +core_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word *result,
> > + void *dwfl_arg)
> > +{
> > + Dwfl_Process *process = dwfl->process;
> > + struct core_arg *core_arg = dwfl_arg;
> > + Elf *core = core_arg->core;
> > + assert (core != NULL);
> > + static size_t phnum;
> > + if (elf_getphdrnum (core, &phnum) < 0)
> > + return false;
>
> __libdwfl_seterrno (DWFL_E_LIBELF) ?
Yes, fixed.
> > + for (size_t cnt = 0; cnt < phnum; ++cnt)
> > + {
> > + GElf_Phdr phdr_mem, *phdr = gelf_getphdr (core, cnt, &phdr_mem);
> > + if (phdr == NULL || phdr->p_type != PT_LOAD)
> > + continue;
> > + /* Bias is zero here, a core file itself has no bias. */
> > + GElf_Addr start = __libdwfl_segment_start (dwfl, phdr->p_vaddr);
> > + GElf_Addr end = __libdwfl_segment_end (dwfl,
> > + phdr->p_vaddr + phdr->p_memsz);
> > + unsigned bytes = process->ebl->class == ELFCLASS64 ? 8 : 4;
> > + if (addr < start || addr + bytes > end)
> > + continue;
> > + Elf_Data *data;
> > + data = elf_getdata_rawchunk (core, phdr->p_offset + addr - start,
> > + bytes, ELF_T_ADDR);
> > + if (data == NULL)
> > + return false;
>
> __libdwfl_seterrno (DWFL_E_LIBELF) ?
Yes.
> > + assert (data->d_size == bytes);
>
> Maybe assert is too strong here? Could there be corrupt core files?
No. In such case elf_getdata_rawchunk would return NULL.
I do not think this assert can fail.
> > + /* FIXME: Currently any arch supported for unwinding supports
> > + unaligned access. */
> > + if (bytes == 8)
> > + *result = *(const uint64_t *) data->d_buf;
> > + else
> > + *result = *(const uint32_t *) data->d_buf;
> > + return true;
> > + }
> > + return false;
> > +}
>
> It would be nice to set dwfl_errno at the end to indicate the core file
> just didn't contain that memory. DWFL_E_ADDR_OUTOFRANGE or DWFL_E_NO_MATCH?
Put there DWFL_E_ADDR_OUTOFRANGE.
> > +static pid_t
> > +core_next_thread (Dwfl *dwfl __attribute__ ((unused)),
> > + Dwfl_Thread *nthread __attribute__ ((unused)), void *dwfl_arg,
> > + void **thread_argp)
> > +{
> > + struct core_arg *core_arg = dwfl_arg;
> > + Elf *core = core_arg->core;
> > + GElf_Nhdr nhdr;
> > + size_t name_offset;
> > + size_t desc_offset;
> > + Elf_Data *note_data = core_arg->note_data;
> > + size_t offset;
> > + while (offset = core_arg->thread_note_offset, offset < note_data->d_size
> > + && (core_arg->thread_note_offset = gelf_getnote (note_data, offset,
> > + &nhdr, &name_offset,
> > + &desc_offset)) > 0)
> > + {
> > + /* Do not check NAME for now, help broken Linux kernels. */
> > + const char *name = note_data->d_buf + name_offset;
> > + const char *desc = note_data->d_buf + desc_offset;
> > + GElf_Word regs_offset;
> > + size_t nregloc;
> > + const Ebl_Register_Location *reglocs;
> > + size_t nitems;
> > + const Ebl_Core_Item *items;
> > + if (! ebl_core_note (core_arg->ebl, &nhdr, name,
> > + ®s_offset, &nregloc, ®locs, &nitems, &items))
> > + {
> > + /* This note may be just not recognized, skip it. */
> > + continue;
> > + }
> > + if (nhdr.n_type != NT_PRSTATUS)
> > + continue;
> > + const Ebl_Core_Item *item;
> > + for (item = items; item < items + nitems; item++)
> > + if (strcmp (item->name, "pid") == 0)
> > + break;
> > + if (item == items + nitems)
> > + continue;
> > + uint32_t val32 = *(const uint32_t *) (desc + item->offset);
> > + val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
> > + ? be32toh (val32) : le32toh (val32));
> > + pid_t tid = (int32_t) val32;
> > + eu_static_assert (sizeof val32 <= sizeof tid);
> > + struct thread_arg *thread_arg = malloc (sizeof (*thread_arg));
> > + if (thread_arg == NULL)
> > + {
> > + __libdwfl_seterrno (DWFL_E_NOMEM);
> > + return 0;
> > + }
>
> Error should be return -1.
OK, yes.
> > + thread_arg->core_arg = core_arg;
> > + thread_arg->note_offset = offset;
> > + *thread_argp = thread_arg;
> > + return tid;
> > + }
> > + return 0;
> > +}
>
> > +static bool
> > +core_set_initial_registers (Dwfl_Thread *thread, void *thread_arg_voidp)
> > +{
> > + struct thread_arg *thread_arg = thread_arg_voidp;
> > + struct core_arg *core_arg = thread_arg->core_arg;
> > + Elf *core = core_arg->core;
> > + size_t offset = thread_arg->note_offset;
> > + GElf_Nhdr nhdr;
> > + size_t name_offset;
> > + size_t desc_offset;
> > + Elf_Data *note_data = core_arg->note_data;
> > + size_t nregs = ebl_frame_nregs (core_arg->ebl);
> > + assert (offset < note_data->d_size);
> > + size_t getnote_err = gelf_getnote (note_data, offset, &nhdr,
> > &name_offset,
> > + &desc_offset);
> > + assert (getnote_err != 0);
>
> Too strong IMHO. Just __libdwfl_seterrno (DWFL_E_LIBELF) return false?
> core files can be corrupted.
In such case __libdwfl_attach_state_for_core would already fail the and
execution would not get here at all. As the data is already read in memory it
is IMO really rather an assert(). Or do I miss something?
> > + /* Do not check NAME for now, help broken Linux kernels. */
> > + const char *name = note_data->d_buf + name_offset;
> > + const char *desc = note_data->d_buf + desc_offset;
> > + GElf_Word regs_offset;
> > + size_t nregloc;
> > + const Ebl_Register_Location *reglocs;
> > + size_t nitems;
> > + const Ebl_Core_Item *items;
> > + int core_note_err = ebl_core_note (core_arg->ebl, &nhdr, name,
> > ®s_offset,
> > + &nregloc, ®locs, &nitems, &items);
> > + assert (core_note_err != 0);
> > + assert (nhdr.n_type == NT_PRSTATUS);
>
> As above too strong IMHO.
The same reason also here.
> > + const Ebl_Core_Item *item;
> > + for (item = items; item < items + nitems; item++)
> > + if (strcmp (item->name, "pid") == 0)
> > + break;
> > + assert (item < items + nitems);
> > + pid_t tid;
> > + {
> > + uint32_t val32 = *(const uint32_t *) (desc + item->offset);
> > + val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
> > + ? be32toh (val32) : le32toh (val32));
> > + tid = (int32_t) val32;
> > + eu_static_assert (sizeof val32 <= sizeof tid);
> > + }
> > + assert (tid == INTUSE(dwfl_thread_tid) (thread));
>
> Can this happen with a corrupt core file? Then it is too strong IMHO.
No. core_next_thread has already read the same number to figure out the TID.
Here the same core number is just read a second time.
> > + desc += regs_offset;
> > + for (size_t regloci = 0; regloci < nregloc; regloci++)
> > + {
> > + const Ebl_Register_Location *regloc = reglocs + regloci;
> > + if (regloc->regno >= nregs)
> > + continue;
> > + assert (regloc->bits == 32 || regloc->bits == 64);
> > + const char *reg_desc = desc + regloc->offset;
> > + for (unsigned regno = regloc->regno;
> > + regno < MIN (regloc->regno + (regloc->count ?: 1U), nregs);
> > + regno++)
> > + {
> > + /* PPC provides DWARF register 65 irrelevant for
> > + CFI which clashes with register 108 (LR) we need.
> > + LR (108) is provided earlier (in NT_PRSTATUS) than the # 65.
> > + FIXME: It depends now on their order in core notes.
> > + FIXME: It uses private function. */
> > + if (dwfl_frame_reg_get (thread->unwound, regno, NULL))
> > + continue;
>
> I dunno how (or if it is important) to fix this issue.
> The dwfl_frame_reg_get check does look irrelevant for other arches.
> But should be harmless.
This is an ugly hack which is only relevant PPC. It could be moved at least
to the second branch for non-x86* archs. But I somehow found it too small to
virtualize it to backends/libebl when it does not hurt other archs.
> > + Dwarf_Word val;
> > + switch (regloc->bits)
> > + {
> > + case 32:;
> > + uint32_t val32 = *(const uint32_t *) reg_desc;
> > + reg_desc += sizeof val32;
> > + val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
> > + ? be32toh (val32) : le32toh (val32));
> > + /* Do a host width conversion. */
> > + val = val32;
> > + break;
> > + case 64:;
> > + uint64_t val64 = *(const uint64_t *) reg_desc;
> > + reg_desc += sizeof val64;
> > + val64 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
> > + ? be64toh (val64) : le64toh (val64));
> > + assert (sizeof (*thread->unwound->regs) == sizeof val64);
> > + val = val64;
> > + break;
> > + default:
> > + abort ();
> > + }
> > + /* Registers not valid for CFI are just ignored. */
> > + INTUSE(dwfl_thread_state_registers) (thread, regno, 1, &val);
> > + reg_desc += regloc->pad;
> > + }
> > + }
> > + return true;
> > +}
[...]
> > diff --git a/libdwfl/dwfl_frame_pid.c b/libdwfl/dwfl_frame_pid.c
> > +struct pid_arg
> > +{
> > + DIR *dir;
> > + size_t tids_attached_size, tids_attached_used;
> > + pid_t *tids_attached;
> > +};
>
> This might be overkill. I think the way we use the callbacks makes
> sure there is only one tid at a time attached.
Now, it wasn't...
I have deleted it now when at most one TID can be backtraced at a time.
> Except when there is a multi-threaded app that uses the same Dwfl in
> multiple threads. I am not sure we fully support that.
At least from what IIRC Roland said despite some thread locking in elfutils it
does not really support multithreaded use.
> > +static bool
> > +ptrace_attach (pid_t tid)
> > +{
> > + if (ptrace (PTRACE_ATTACH, tid, NULL, NULL) != 0)
> > + return false;
> > + /* FIXME: Handle missing SIGSTOP on old Linux kernels. */
>
> How old are these kernels?
> If this is before say 2.6.18 kernels (7 years ago now) then I
> think it isn't too urgent.
It happens in RHEL-6. It is only very recent change where PTRACE_ATTACH can
no longer hang.
> > + for (;;)
> > + {
> > + int status;
> > + if (waitpid (tid, &status, __WALL) != tid || !WIFSTOPPED (status))
> > + {
> > + ptrace (PTRACE_DETACH, tid, NULL, NULL);
> > + return false;
> > + }
> > + if (WSTOPSIG (status) == SIGSTOP)
> > + break;
> > + if (ptrace (PTRACE_CONT, tid, NULL,
> > + (void *) (uintptr_t) WSTOPSIG (status)) != 0)
> > + {
> > + ptrace (PTRACE_DETACH, tid, NULL, NULL);
> > + return false;
> > + }
> > + }
> > + return true;
> > +}
>
> When used in pid_set_initial_registers the return value is also just
> false to indicate failure. And dwfl_thread_getframes will then just
> return -1 and not set dwfl_errno. Can we do a little better?
> Maybe use __libdwfl_seterrno (DWFL_E_ERRNO) when ptrace or waitpid fails?
Done with DWFL_E_ERRNO with blocks like:
if (waitpid (tid, &status, __WALL) != tid || !WIFSTOPPED (status))
{
int saved_errno = errno;
ptrace (PTRACE_DETACH, tid, NULL, NULL);
errno = saved_errno;
__libdwfl_seterrno (DWFL_E_ERRNO);
return false;
}
> > +static bool
> > +pid_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word *result, void
> > *arg)
> > +{
> > + struct pid_arg *pid_arg = arg;
> > + assert (pid_arg->tids_attached_used > 0);
> > + pid_t tid = pid_arg->tids_attached[0];
>
> Aha, that works because you need an arbitrary tid of the pid that is
> attached. And you know there must be at least one or this callback
> wouldn't be called.
Yes.
> But is this really why the whole tids_attached datastructure is kept?
Yes.
> If so, then maybe our memory_read callback really should have a
> tid or Dwfl_Thread argument to indicate for which thread the memory
> read was intended.
It should not, it does not depend on the TID. It would be wrong API.
> This thread must be attached already and then
> you could just use it here instead of having to keep track of all
> the tids_attached. In theory memory_read is generic per process,
> but in practice it does matter which thread the request was for
> it seems.
I think the logic is now easy with single 'tid_attached' variable when at most
one thread can be PTRACE_ATTACH-ed.
> > + Dwfl_Process *process = dwfl->process;
> > + if (process->ebl->class == ELFCLASS64)
> > + {
> > + errno = 0;
> > + *result = ptrace (PTRACE_PEEKDATA, tid, (void *) (uintptr_t) addr,
> > NULL);
> > + return errno == 0;
> > + }
> > +#if SIZEOF_LONG == 8
> > + /* We do not care about reads unaliged to 4 bytes boundary.
> > + But 0x...ffc read of 8 bytes could overrun a page. */
> > + bool lowered = (addr & 4) != 0;
> > + if (lowered)
> > + addr -= 4;
> > +#endif /* SIZEOF_LONG == 8 */
> > + errno = 0;
> > + *result = ptrace (PTRACE_PEEKDATA, tid, (void *) (uintptr_t) addr, NULL);
> > + if (errno != 0)
> > + return false;
> > +#if SIZEOF_LONG == 8
> > +# if BYTE_ORDER == BIG_ENDIAN
> > + if (! lowered)
> > + *result >>= 32;
> > +# else
> > + if (lowered)
> > + *result >>= 32;
> > +# endif
> > +#endif /* SIZEOF_LONG == 8 */
> > + *result &= 0xffffffff;
> > + return true;
> > +}
>
> > +static pid_t
> > +pid_next_thread (Dwfl *dwfl __attribute__ ((unused)),
> > + Dwfl_Thread *nthread __attribute__ ((unused)), void *dwfl_arg,
> > + void **thread_argp)
> > +{
> > + struct pid_arg *pid_arg = dwfl_arg;
> > + struct dirent *dirent;
> > + do
> > + {
> > + errno = 0;
> > + dirent = readdir (pid_arg->dir);
> > + if (dirent == NULL)
> > + return errno == 0 ? 0 : -1;
>
> if (errno != 0) __libdwfl_seterrno (DWFL_E_ERRNO) ?
Yes.
if (dirent == NULL)
{
if (errno != 0)
{
__libdwfl_seterrno (DWFL_E_ERRNO);
return -1;
}
return 0;
}
> > + }
> > + while (strcmp (dirent->d_name, ".") == 0
> > + || strcmp (dirent->d_name, "..") == 0);
>
> I assume readdir makes sure there two are always first?
I do not see such guarantee in Fedora man 3 readdir and neither in:
http://pubs.opengroup.org/onlinepubs/007908799/xsh/readdir.html
> > + char *end;
> > + errno = 0;
> > + long tidl = strtol (dirent->d_name, &end, 10);
> > + if (errno != 0)
> > + return -1;
>
> __libdwfl_seterrno (DWFL_E_ERRNO) ?
Yes.
> > + pid_t tid = tidl;
> > + if (tidl <= 0 || (end && *end) || tid != tidl)
> > + return -1;
>
> O, weird... Yeah, that would not be good, but an assert is
> probably not good either in case something weird shows up in the dir.
> Maybe DWFL_E_PARSE_PROC?
Yes.
> > + *thread_argp = dwfl_arg;
> > + return tid;
> > +}
>
> > +static bool
> > +pid_thread_state_registers_cb (const int firstreg,
> > + unsigned nregs,
> > + const Dwarf_Word *regs,
> > + void *arg)
> > +{
> > + Dwfl_Thread *thread = (Dwfl_Thread *) arg;
> > + return INTUSE(dwfl_thread_state_registers) (thread, firstreg, nregs,
> > regs);
> > +}
>
> Might want to add a comment this is the ebl_set_initial_registers_tid
> callback.
Added:
/* Implement the ebl_set_initial_registers_tid setfunc callback. */
> > +static bool
> > +pid_set_initial_registers (Dwfl_Thread *thread, void *thread_arg)
> > +{
> > + struct pid_arg *pid_arg = thread_arg;
> > + pid_t tid = INTUSE(dwfl_thread_tid) (thread);
> > + if (! ptrace_attach (tid))
> > + return false;
> > + if (pid_arg->tids_attached_used == pid_arg->tids_attached_size)
> > + {
> > + pid_arg->tids_attached_size *= 2;
> > + pid_arg->tids_attached_size = MAX (64, pid_arg->tids_attached_size);
> > + pid_arg->tids_attached = realloc (pid_arg->tids_attached,
> > + (pid_arg->tids_attached_size
> > + * sizeof *pid_arg->tids_attached));
>
> In theory the realloc could fail and return NULL.
The code is no longer present.
> > + }
> > + pid_arg->tids_attached[pid_arg->tids_attached_used++] = tid;
> > + Dwfl_Process *process = thread->process;
> > + Ebl *ebl = process->ebl;
> > + return ebl_set_initial_registers_tid (ebl, tid,
> > + pid_thread_state_registers_cb, thread);
> > +}
[...]
> > +bool
> > +internal_function
> > +__libdwfl_attach_state_for_pid (Dwfl *dwfl, pid_t pid)
> > +{
> > + char dirname[64];
> > + int i = snprintf (dirname, sizeof (dirname), "/proc/%ld/task", (long)
> > pid);
> > + assert (i > 0 && i < (ssize_t) sizeof (dirname) - 1);
>
> 30 chars is enough if pids are actually long, otherwise 21 should be enough
> for everybody. (MAX_INT is 2147483647, MAX_LONG is 9223372036854775807.)
I think at least dirname[32] is required for:
/proc/-9223372036854775808/taskZ
Also stack frame is 16-bytes aligned. It could probably be [32] but I really
do not think it matters to discuss this.
> > + DIR *dir = opendir (dirname);
> > + if (dir == NULL)
> > + {
> > + __libdwfl_seterrno (DWFL_E_PARSE_PROC);
> > + return NULL;
> > + }
>
> That should be return false.
Yes.
> Do you need a special dwfl_errno for this or can you use DWFL_E_ERRNO?
Used DWFL_E_ERRNO.
> > + struct pid_arg *pid_arg = malloc (sizeof *pid_arg);
> > + if (pid_arg == NULL)
> > + {
> > + closedir (dir);
> > + __libdwfl_seterrno (DWFL_E_NOMEM);
> > + return false;
> > + }
> > + pid_arg->dir = dir;
> > + pid_arg->tids_attached_size = 0;
> > + pid_arg->tids_attached_used = 0;
> > + pid_arg->tids_attached = NULL;
> > + if (! INTUSE(dwfl_attach_state) (dwfl, EM_NONE, pid,
> > &pid_thread_callbacks,
> > + pid_arg))
> > + {
> > + free (pid_arg);
> > + return false;
> > + }
>
> Missing closedir (dir)?
Right.
> > + return true;
> > +}
Thanks,
Jan
diff --git a/libdwfl/linux-core-attach.c b/libdwfl/linux-core-attach.c
index c04f0e9..567d891 100644
--- a/libdwfl/linux-core-attach.c
+++ b/libdwfl/linux-core-attach.c
@@ -58,7 +58,10 @@ core_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word
*result,
assert (core != NULL);
static size_t phnum;
if (elf_getphdrnum (core, &phnum) < 0)
- return false;
+ {
+ __libdwfl_seterrno (DWFL_E_LIBELF);
+ return false;
+ }
for (size_t cnt = 0; cnt < phnum; ++cnt)
{
GElf_Phdr phdr_mem, *phdr = gelf_getphdr (core, cnt, &phdr_mem);
@@ -75,7 +78,10 @@ core_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word
*result,
data = elf_getdata_rawchunk (core, phdr->p_offset + addr - start,
bytes, ELF_T_ADDR);
if (data == NULL)
- return false;
+ {
+ __libdwfl_seterrno (DWFL_E_LIBELF);
+ return false;
+ }
assert (data->d_size == bytes);
/* FIXME: Currently any arch supported for unwinding supports
unaligned access. */
@@ -85,6 +91,7 @@ core_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word
*result,
*result = *(const uint32_t *) data->d_buf;
return true;
}
+ __libdwfl_seterrno (DWFL_E_ADDR_OUTOFRANGE);
return false;
}
@@ -136,7 +143,7 @@ core_next_thread (Dwfl *dwfl __attribute__ ((unused)),
if (thread_arg == NULL)
{
__libdwfl_seterrno (DWFL_E_NOMEM);
- return 0;
+ return -1;
}
thread_arg->core_arg = core_arg;
thread_arg->note_offset = offset;
@@ -158,6 +165,7 @@ core_set_initial_registers (Dwfl_Thread *thread, void
*thread_arg_voidp)
size_t desc_offset;
Elf_Data *note_data = core_arg->note_data;
size_t nregs = ebl_frame_nregs (core_arg->ebl);
+ assert (nregs > 0);
assert (offset < note_data->d_size);
size_t getnote_err = gelf_getnote (note_data, offset, &nhdr, &name_offset,
&desc_offset);
diff --git a/libdwfl/linux-pid-attach.c b/libdwfl/linux-pid-attach.c
index 22101b7..2892052 100644
--- a/libdwfl/linux-pid-attach.c
+++ b/libdwfl/linux-pid-attach.c
@@ -38,22 +38,28 @@
struct pid_arg
{
DIR *dir;
- size_t tids_attached_size, tids_attached_used;
- pid_t *tids_attached;
+ /* It is 0 if not used. */
+ pid_t tid_attached;
};
static bool
ptrace_attach (pid_t tid)
{
if (ptrace (PTRACE_ATTACH, tid, NULL, NULL) != 0)
- return false;
+ {
+ __libdwfl_seterrno (DWFL_E_ERRNO);
+ return false;
+ }
/* FIXME: Handle missing SIGSTOP on old Linux kernels. */
for (;;)
{
int status;
if (waitpid (tid, &status, __WALL) != tid || !WIFSTOPPED (status))
{
+ int saved_errno = errno;
ptrace (PTRACE_DETACH, tid, NULL, NULL);
+ errno = saved_errno;
+ __libdwfl_seterrno (DWFL_E_ERRNO);
return false;
}
if (WSTOPSIG (status) == SIGSTOP)
@@ -61,7 +67,10 @@ ptrace_attach (pid_t tid)
if (ptrace (PTRACE_CONT, tid, NULL,
(void *) (uintptr_t) WSTOPSIG (status)) != 0)
{
+ int saved_errno = errno;
ptrace (PTRACE_DETACH, tid, NULL, NULL);
+ errno = saved_errno;
+ __libdwfl_seterrno (DWFL_E_ERRNO);
return false;
}
}
@@ -72,8 +81,8 @@ static bool
pid_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word *result, void *arg)
{
struct pid_arg *pid_arg = arg;
- assert (pid_arg->tids_attached_used > 0);
- pid_t tid = pid_arg->tids_attached[0];
+ pid_t tid = pid_arg->tid_attached;
+ assert (tid > 0);
Dwfl_Process *process = dwfl->process;
if (process->ebl->class == ELFCLASS64)
{
@@ -117,7 +126,14 @@ pid_next_thread (Dwfl *dwfl __attribute__ ((unused)),
errno = 0;
dirent = readdir (pid_arg->dir);
if (dirent == NULL)
- return errno == 0 ? 0 : -1;
+ {
+ if (errno != 0)
+ {
+ __libdwfl_seterrno (DWFL_E_ERRNO);
+ return -1;
+ }
+ return 0;
+ }
}
while (strcmp (dirent->d_name, ".") == 0
|| strcmp (dirent->d_name, "..") == 0);
@@ -125,14 +141,22 @@ pid_next_thread (Dwfl *dwfl __attribute__ ((unused)),
errno = 0;
long tidl = strtol (dirent->d_name, &end, 10);
if (errno != 0)
- return -1;
+ {
+ __libdwfl_seterrno (DWFL_E_ERRNO);
+ return -1;
+ }
pid_t tid = tidl;
if (tidl <= 0 || (end && *end) || tid != tidl)
- return -1;
+ {
+ __libdwfl_seterrno (DWFL_E_PARSE_PROC);
+ return -1;
+ }
*thread_argp = dwfl_arg;
return tid;
}
+/* Implement the ebl_set_initial_registers_tid setfunc callback. */
+
static bool
pid_thread_state_registers_cb (const int firstreg,
unsigned nregs,
@@ -147,18 +171,11 @@ static bool
pid_set_initial_registers (Dwfl_Thread *thread, void *thread_arg)
{
struct pid_arg *pid_arg = thread_arg;
+ assert (! pid_arg->tid_attached);
pid_t tid = INTUSE(dwfl_thread_tid) (thread);
if (! ptrace_attach (tid))
return false;
- if (pid_arg->tids_attached_used == pid_arg->tids_attached_size)
- {
- pid_arg->tids_attached_size *= 2;
- pid_arg->tids_attached_size = MAX (64, pid_arg->tids_attached_size);
- pid_arg->tids_attached = realloc (pid_arg->tids_attached,
- (pid_arg->tids_attached_size
- * sizeof *pid_arg->tids_attached));
- }
- pid_arg->tids_attached[pid_arg->tids_attached_used++] = tid;
+ pid_arg->tid_attached = tid;
Dwfl_Process *process = thread->process;
Ebl *ebl = process->ebl;
return ebl_set_initial_registers_tid (ebl, tid,
@@ -170,7 +187,6 @@ pid_detach (Dwfl *dwfl __attribute__ ((unused)), void
*dwfl_arg)
{
struct pid_arg *pid_arg = dwfl_arg;
closedir (pid_arg->dir);
- free (pid_arg->tids_attached);
free (pid_arg);
}
@@ -179,13 +195,8 @@ pid_thread_detach (Dwfl_Thread *thread, void *thread_arg)
{
struct pid_arg *pid_arg = thread_arg;
pid_t tid = INTUSE(dwfl_thread_tid) (thread);
- size_t ix;
- for (ix = 0; ix < pid_arg->tids_attached_used; ix++)
- if (pid_arg->tids_attached[ix] == tid)
- break;
- assert (ix < pid_arg->tids_attached_used);
- pid_arg->tids_attached[ix]
- = pid_arg->tids_attached[--pid_arg->tids_attached_used];
+ assert (pid_arg->tid_attached == tid);
+ pid_arg->tid_attached = 0;
ptrace (PTRACE_DETACH, tid, NULL, NULL);
}
@@ -208,8 +219,8 @@ __libdwfl_attach_state_for_pid (Dwfl *dwfl, pid_t pid)
DIR *dir = opendir (dirname);
if (dir == NULL)
{
- __libdwfl_seterrno (DWFL_E_PARSE_PROC);
- return NULL;
+ __libdwfl_seterrno (DWFL_E_ERRNO);
+ return false;
}
struct pid_arg *pid_arg = malloc (sizeof *pid_arg);
if (pid_arg == NULL)
@@ -219,12 +230,11 @@ __libdwfl_attach_state_for_pid (Dwfl *dwfl, pid_t pid)
return false;
}
pid_arg->dir = dir;
- pid_arg->tids_attached_size = 0;
- pid_arg->tids_attached_used = 0;
- pid_arg->tids_attached = NULL;
+ pid_arg->tid_attached = 0;
if (! INTUSE(dwfl_attach_state) (dwfl, EM_NONE, pid, &pid_thread_callbacks,
pid_arg))
{
+ closedir (dir);
free (pid_arg);
return false;
}
