Hi Jan,
On Mon, 2013-09-02 at 15:26 +0200, Jan Kratochvil wrote:
here is the refactored version addressing the paragraph from my last
post
[patch v3] unwinder: The unwinder (x86* only)x
https://lists.fedorahosted.org/pipermail/elfutils-devel/2013-June/003109....
Message-ID: <20130623183057.GA9934(a)host2.jankratochvil.net>
This is taking way too long for a review. Sorry.
But it is a fairly big patch. So lets concentrate on some smaller parts
to get things moving. Just looking at the ebl/backend change now.
backends/
2013-06-23 Jan Kratochvil <jan.kratochvil(a)redhat.com>
* Makefile.am (AM_CPPFLAGS): Add ../libdwfl.
(i386_SRCS): Add i386_initreg.c.
(x86_64_SRCS): Add x86_64_initreg.c.
* i386_initreg.c: New file.
* i386_init.c (i386_init): Initialize frame_nregs and
set_initial_registers_tid.
* x86_64_initreg.c: New file.
* x86_64_init.c (x86_64_init): Initialize frame_nregs and
set_initial_registers_tid.
[...]
libebl/
2013-06-23 Jan Kratochvil <jan.kratochvil(a)redhat.com>
* Makefile.am (AM_CPPFLAGS): Add ../libdwfl.
(gen_SOURCES): Add eblinitreg.c.
* ebl-hooks.h (set_initial_registers_tid): New entry.
* eblinitreg.c: New file.
* libebl.h (dwfl_thread_state_registers_t): New definition.
(ebl_set_initial_registers_tid, ebl_frame_nregs): New declarations.
* libeblP.h (Dwfl_Module): New declaration.
(struct ebl): New entry frame_nregs.
I don't really like that ebl/backends now depends on dwfl. If only
because it makes just reviewing the ebl/backends changes harder :)
It feels like the code layering is breached by having the low-level
library code depend on the high-level library definitions.
This is needed because the new ebl_set_initial_registers_tid function is
defined as:
diff --git a/libebl/libebl.h b/libebl/libebl.h
index cae31c9..b11c4b3 100644
--- a/libebl/libebl.h
+++ b/libebl/libebl.h
[...]
+/* Fetch process data from live TID into THREAD->unwound. */
+struct Dwfl_Thread;
+typedef bool (dwfl_thread_state_registers_t) (struct Dwfl_Thread *thread,
+ const int firstreg,
+ unsigned nregs,
+ const Dwarf_Word *regs)
+ __nonnull_attribute__ (1, 4);
+extern bool ebl_set_initial_registers_tid (struct Dwfl_Thread *thread,
+ pid_t tid,
+ dwfl_thread_state_registers_t *setfunc)
+ __nonnull_attribute__ (1);
And Dwfl_Thread is defined in libdwfl.h.
ebl_set_initial_registers does two things. First it determines (from the
Dwfl_Thread) which tid it wants, and fetches the registers for it for
that tid. Then it puts the registers that will be needed for frame
unwinding into a Dwarf_Word[] and calls the setfunc for on it.
I was wondering if you had considered doing this more like is done for
core files to make it a bit more generic?
For core files we have ebl_core_note () that gives you a pointer to the
register "block", the number of registers and an array of
Ebl_Register_Location describing the registers (offset in the block,
DWARF register number, bits, etc).
The advantage would be that things are a little bit more flexible for
the caller and that more code could be shared between things that
process registers in core notes and those that grab them from a process.
And that we don't have a strict reliance on the libdwfl data structures.
The disadvantage of course is that it is a bit more work to extract all
that information.
Does that make sense? If so I can try to rewrite the code a bit to do
that.
Thanks,
Mark
--- a/libebl/ebl-hooks.h
+++ b/libebl/ebl-hooks.h
@@ -1,5 +1,5 @@
/* Backend hook signatures internal interface for libebl.
- Copyright (C) 2000-2011 Red Hat, Inc.
+ Copyright (C) 2000-2011, 2013 Red Hat, Inc.
This file is part of elfutils.
This file is free software; you can redistribute it and/or modify
@@ -155,5 +155,15 @@ int EBLHOOK(disasm) (const uint8_t **startp, const uint8_t *end,
Function returns 0 on success and -1 on error. */
int EBLHOOK(abi_cfi) (Ebl *ebl, Dwarf_CIE *abi_info);
+/* *SYM must be STT_FUNC. Then if it describes a function descriptor (PPC64)
+ convert in-place its data and return a possibly different new name for it.
+ The name is valid as long as EBL is valid. */
+const char *EBLHOOK(get_func_pc) (Ebl *ebl, struct Dwfl_Module *mod,
+ GElf_Sym *sym);
BTW. This hook isn't actually implemented in this patch.