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;
}