unwind branch review
by Roland McGrath
I've been poking at it again a bit lately, and, oddly enough, I think the
roland/unwind branch might be more or less ready to merge on the trunk
(perhaps as soon as 0.142). I'd appreciate some review of the branch code.
The branch is really a misnomer, of course. It's not the "unwinder"
branch, it's the CFI support branch. Like everything else in libdw, it is
meant to be the right granular interface at the right level of abstraction
for what the data is really about. So it does not do any "unwinding" at
all. What this piece exposes is the "PC => {register #, location} map"
mapping. That is, the static semantics view of CFI, using DWARF location
expressions (or value expressions) in Dwarf_Op[] form.
The "show me a backtrace in the core file/process" level of "unwinding"
that everyone thinks about is in fact a composition of CFI interpretation,
location expression evaluation, and register/memory gathering. What we
have here is the first of those.
This is the only piece we need for static DWARF tools like dwarfcmp and
writer/compression support. I think the new libdw interfaces here are
decent enough in C for simple support, and a sufficient foundation for the
C++ interfaces to read and write this data.
This is work I originally did a couple of years ago; I have updated it
along the way, and cleaned up a bit more lately around the edges. But I
haven't done a thorough review of all the code from scratch myself lately.
So anything XXX-commented or unfinished-looking or otherwise questionable
merits some fresh hashing out.
Thanks,
Roland
$ git diff --stat origin/master..roland/unwind
backends/ChangeLog | 10 +
backends/Makefile.am | 4 +-
backends/i386_cfi.c | 65 +++++
backends/i386_init.c | 1 +
backends/x86_64_cfi.c | 60 +++++
backends/x86_64_init.c | 1 +
libdw/ChangeLog | 54 +++++
libdw/Makefile.am | 10 +-
libdw/cfi.c | 498 +++++++++++++++++++++++++++++++++++++++
libdw/cfi.h | 249 +++++++++++++++++++
libdw/cie.c | 193 +++++++++++++++
libdw/dwarf_begin_elf.c | 3 +-
libdw/dwarf_cfi_addrframe.c | 78 ++++++
libdw/dwarf_cfi_end.c | 70 ++++++
libdw/dwarf_end.c | 8 +-
libdw/dwarf_error.c | 1 +
libdw/dwarf_frame_cfa.c | 100 ++++++++
libdw/dwarf_frame_info.c | 74 ++++++
libdw/dwarf_frame_register.c | 143 +++++++++++
libdw/dwarf_getcfi.c | 92 +++++++
libdw/dwarf_getcfi_elf.c | 331 ++++++++++++++++++++++++++
libdw/dwarf_getlocation.c | 75 +++++--
libdw/dwarf_next_cfi.c | 234 ++++++++++++++++++
libdw/encoded-value.h | 174 ++++++++++++++
libdw/fde.c | 297 +++++++++++++++++++++++
libdw/frame-cache.c | 87 +++++++
libdw/libdw.h | 158 ++++++++++++-
libdw/libdw.map | 16 ++
libdw/libdwP.h | 15 +-
libdw/memory-access.h | 37 ++-
libdwfl/ChangeLog | 11 +-
libdwfl/Makefile.am | 2 +
libdwfl/dwfl_addrframe.c | 93 ++++++++
libdwfl/dwfl_module_dwarf_cfi.c | 89 +++++++
libdwfl/dwfl_module_eh_cfi.c | 75 ++++++
libdwfl/libdwfl.h | 23 ++
libdwfl/libdwflP.h | 10 +
libebl/ChangeLog | 9 +
libebl/Makefile.am | 2 +-
libebl/ebl-hooks.h | 2 +
libebl/eblabicfi.c | 63 +++++
libebl/eblopenbackend.c | 9 +
libebl/libebl.h | 34 +++-
libebl/libeblP.h | 6 +
tests/ChangeLog | 6 +
tests/Makefile.am | 3 +-
tests/addrcfi.c | 191 +++++++++++++++
47 files changed, 3720 insertions(+), 46 deletions(-)
15 years
release soon?
by Roland McGrath
I think we should do 0.141 after another day or two for testing and
verification. It has some needed fixes, and makes two Fedora secondary
arch ports newly happy.
All looks good on my end. I'd like to let mjw verify that current git has
the problems he hit in stap fixed (RHBZ#489439 and dwfl_module_getsym).
"sed -i s/140/141/ configure.ac; autreconf" to test _ELFUTILS_PREREQ code.
$ git log --abbrev-commit --pretty=oneline --reverse elfutils-0.140..master
8651c5e... Fix RHBZ#489439: dwfl_module_build_id yields wrong vaddr for prelinked DSO
329ed9f... __libdwfl_find_build_id optimization nit
6a4bdd9... Add DW_CFA_GNU_negative_offset_extended.
d5ff667... Update elf.h and some tables; fill in sparc backend reloc table.
dae7be7... Sending translation for po/es.po
4e1b7c9... Sending translation for po/de.po
243fa26... dwfl_module_getsym should not adjust symbols from non-SHF_ALLOC sections.
785dbb1... Fix double-adjust for ET_REL.
68f29f9... Make dwfl_module_getsym yield shndx -1 for any non-allocated section.
ec84323... Sending translation for Spanish
04a1416... Fix strip -f infinite loop when coping with some extra kooky object sections.
fa25326... Fix RHBZ#494858: fix bad address checks in core file support.
54eb890... NEWS items for notable changes since 0.140
94f4383... Fix error message.
eb9ba47... Nominal ARM support.
Thanks,
Roland
15 years
Re: [RFC]: Feature to display local arguments and variables
by Dave Anderson
----- "Sharyathi Nagesh" <sharyath(a)in.ibm.com> wrote:
> >> How to Build/Requirements
> >> 1. Enable compiling extends option in main Makefile
> >
> > You mean to just enter "make extensions", right?
> Yes, found the Makefile under extensions is not called due to this, we
> faced this issue with crash 4.0.7.7 didn't try with latest crash
> though.
It's not a new concept. If the extensions/Makefile sees a "local.c" file,
it will then look for a "local.mk" file and invoke it. I applied your
patch, and "make extensions" from the top-level directory works as expected
(although the compilation fails as expected):
# make extensions
gcc -nostartfiles -shared -rdynamic -o local.so local.c -fPIC -ldw -L ../../elfutils-0.137/libdw -I ../../elfutils-0.137/libdw -I ../../elfutils-0.137/libelf/ -DX86_64 -Wall;
local.c:8:23: error: libdw.h: No such file or directory
local.c:9:19: error: dwarf.h: No such file or directory
local.c:11:21: error: netdump.h: No such file or directory
local.c:33: error: expected ‘)’ before ‘*’ token
...
> >
> >> 2. create a symbolic link to netdump.h under extensions/
> >
> > You should be able to do that in your local.mk file.
> Ok, sure will take care of that. Please let us know of any reference
> *.mk file that is used. will help as this will need to take care of
> library dependencies in this file
The extensions/sial.mk file is the only example in the upstream tree.
Actually there are two alternatives:
(1) as you are doing now, put your local.c and local.mk and whatever other
files you need into an existing extensions subdirectory, or
(2) build a standalone package that only requires the crash-devel
subpackage.
Alternative (2) allows you to create a standalone package that
uses the crash-devel subpackage, which only consists of the "defs.h"
file. Doing it that way you don't need the complete crash source tree.
Your package is unique in that it currently needs "netdump.h"
for the vmcore_data structure. I am not averse to moving that
structure declaration into "defs.h" if you want to go that route.
Check out the IBM crash-spu-commands-1.1-1.src.rpm package as an example
of alternative (2):
http://riksun.riken.go.jp/pub/pub/Linux/cern/updates/slc5X/SRPMS/crash-sp...
In any case, that's easy enough to deal with in the future.
I'd just continue working in the extensions subdirectory for now.
> >> 3. Context switching via -r command still needs to implemented
> >
> > BTW, the "placeholder" code in cmd_local() for -p and -r would be
> > disastrous because they modify a context structure with a totally
> > different pid. What you'll want to do is update local->tc with a
> > pointer to the context of the entered pid argument. (There's helper
> > functions to do that)
> yes you are right initialize_local() function needs to handle it.
> For -p opion we need
> 1. function to provide task context for the pid passed
That would be pid_to_context().
But more on that below...
> 2. Current register context for the process, for Active process we
> could get from ELF Notes, if there is a way to access register set for
> these process it would be helpful
The only register sets are found in the per-cpu NT_PRSTATUS notes in
the ELF header. They don't exist for the other non-active tasks, and
in fact, the older netdump format only contains a single ELF NT_PRSTATUS
for the crashing cpu task.
Furthermore, if the kdump vmcore is compressed with "makedumpfile -c",
then the resultant dumpfile's unique non-ELF header doesn't even contain
those register sets.
> -c option allows user to shift context. If user changes active CPU via
> set -c <new CPU>, local -c will allow user to move current local context
> to that particular cpu context.
>
> Please Let us know your thoughts on this implementation
I don't know why that would be necessary. When your cmd_local() function
is called without "-p <pid>", it should just default to the current context
which would have been set with the prior "set -c <cpu>", "set <task>" or
"set <pid>". For that matter, why do you even need "-p <pid>"? I'm not
clear on why you wouldn't be using the current context all of the time?
It appears that your mechanism will only work if the target task is
an active task whose register set is contained in the ELF header.
So when your command is called, it seems like it would simply need to
first determine "can I handle this context?", and if not, just bail out.
Dave
15 years
Re: [RFC]: Feature to display local arguments and variables
by Dave Anderson
----- "Sharyathi Nagesh" <sharyath(a)in.ibm.com> wrote:
> Hi
> We have implemented this piece of code to provide, crash tool,
> capability to display local variables and arguments. We would kindly
> request you to provide your feedback and guidance on this code so that
> we can take it further
When I get a chance I'll reserve a ppc64 machine to give this a go.
But for now, I've just got a few questions and suggestions.
>
> How to Build/Requirements
> 1. Enable compiling extends option in main Makefile
You mean to just enter "make extensions", right?
> 2. create a symbolic link to netdump.h under extensions/
You should be able to do that in your local.mk file.
> 3. This feature makes use of libelf and libdw libraries provided by elfutils package.
> 4, This feature is implemented as an extend, local.so library needs to
> be loaded once crash prompt is available
>
> Features
> 1. Currently it has feature to display arguments/locals of top most
> stack frame. ex usage: local params or local locals
What function would be considered the "top most" stack frame, say if
panic() were called, i.e., when crash_kexec() gets called with a NULL
"regs" argument? It appears that it would be crash_kexec() itself.
On the other hand, most dumps are generated from die(), but the regs
have been passed through several functions, so it's not clear to me
what you would see.
For those of us who are ppc64-machine-challenged, a few examples would
have been helpful...
> 2. Currently code can analyze only ppc64 dumps
> 3. It displays the current address of the local variables ( and some
> times direct values when the variables are stored in register and not in
> stack frame)
> 4. In case of code optimization, variable information is not available
> at that time printing "Failed to fetch information" (This is in
> accordance with gdb out put)
>
> TBD
> 1. Stack unwinding code is still need to be implemented
> 2. Support for x86 dumps is still not provided
BTW, have you explicitly left out x86_64?
> 3. Context switching via -r command still needs to implemented
BTW, the "placeholder" code in cmd_local() for -p and -r would be
disastrous because they modify a context structure with a totally
different pid. What you'll want to do is update local->tc with a
pointer to the context of the entered pid argument. (There's helper
functions to do that)
>
> Note: We were planning to display the variable values instead of
> addresses as we are doing currently, but it adds up to additional
> challenges in terms of typecasting the variables, in case it is array or
> structure, so for the time being we are printing only address and expect
> user to dump contents using rd command. Suggestions here will be very helpful.
Presumably the user will know what the local data types are, and can
simply display them using the "struct <datatype> <address>" command.
Using rd is fine, but may be unnecessarily primative. But anyway,
are you saying that the local variable data types cannot be determined
from some dwarf interface?
>
> Attaching the code...but the Code is not extensively tested
> please let us know your thoughts
Another minor nit -- I'd prefer that a new function be added to
netdump.c similar to get_kdump_vmcore_data() that you can use
for "real" netdump dumpfiles -- if you are planning to actually
planning to support netdump dumpfiles. Just create a new
get_netdump_vmcore_data() instead of making the structure global.
Anyway, it looks promising on paper!
Thanks,
Dave
15 years
DW_CIE_ID
by Roland McGrath
I'm not sure that DW_CIE_ID belongs in dwarf.h as it is.
The CIE_id value is 0xffffffff only in 32-bit DWARF.
In 64-bit DWARF, it's 0xffffffffffffffff.
So there isn't a single value that is correct for DW_CIE_ID.
In my unwind branch, I didn't add any CIE_id constant to dwarf.h at all.
Instead, I only use a header parser (dwarf_next_cfi) that canonicalizes
the CIE_id/CIE_pointer field to a Dwarf_Off (i.e. 64 bits) and so the
single CIE_ID constant I provide is the 64-bit one. (That's all in
unwind.h on the branch, but would go in libdw.h when merged.)
But dwarf.h is for the literal format constants in raw form, not for
canonicalized constants in the libdw interface. So if anything is in
dwarf.h for this, I think it has to be both of the two variants with
variant names. e.g. DW32_CIE_ID and DW64_CIE_ID. (Or maybe it's better
to keep the uniform DW_ prefix and make them DW_CIE_ID_{32,64} or some
other names.)
i.e. like the patch below. This works fine for me, but I am a little
worried that the 64-bit constant in an enum might upset the compiler
(version or anality switch-set) used for some application's build.
What say you?
Thanks,
Roland
======
diff --git a/libdw/dwarf.h b/libdw/dwarf.h
index 3b9d40c..0000000 100644
--- a/libdw/dwarf.h
+++ b/libdw/dwarf.h
@@ -677,7 +677,8 @@ enum
/* ID indicating CIE as opposed to FDE in .debug_frame. */
enum
{
- DW_CIE_ID = 0xffffffff
+ DW32_CIE_ID = 0xffffffff,
+ DW64_CIE_ID = 0xffffffffffffffff
};
diff --git a/src/readelf.c b/src/readelf.c
index f4ed033..0000000 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -4673,9 +4673,13 @@ print_debug_frame_section (Dwfl_Module *
if (unlikely (cieend > dataend || readp + 8 > dataend))
goto invalid_data;
- Dwarf_Word cie_id;
+ Dwarf_Off cie_id;
if (length == 4)
- cie_id = read_4ubyte_unaligned_inc (dbg, readp);
+ {
+ cie_id = read_4ubyte_unaligned_inc (dbg, readp);
+ if (!is_eh_frame && cie_id == DW32_CIE_ID)
+ cie_id = DW64_CIE_ID;
+ }
else
cie_id = read_8ubyte_unaligned_inc (dbg, readp);
@@ -4686,7 +4690,7 @@ print_debug_frame_section (Dwfl_Module *
Dwarf_Word initial_location = 0;
Dwarf_Word vma_base = 0;
- if (cie_id == (is_eh_frame ? 0 : DW_CIE_ID))
+ if (cie_id == (is_eh_frame ? 0 : DW64_CIE_ID))
{
uint_fast8_t version = *readp++;
const char *const augmentation = (const char *) readp;
15 years
[RFC]: Feature to display local arguments and variables
by Sharyathi Nagesh
Hi
We have implemented this piece of code to provide, crash tool,
capability to display local variables and arguments. We would kindly
request you to provide your feedback and guidance on this code so that
we can take it further
How to Build/Requirements
1. Enable compiling extends option in main Makefile
2. create a symbolic link to netdump.h under extensions/
3. This feature makes use of libelf and libdw libraries provided by
elfutils package.
4, This feature is implemented as an extend, local.so library needs to
be loaded once crash prompt is available
Features
1. Currently it has feature to display arguments/locals of top most
stack frame. ex usage: local params or local locals
2. Currently code can analyze only ppc64 dumps
3. It displays the current address of the local variables ( and some
times direct values when the variables are stored in register and not in
stack frame)
4. In case of code optimization, variable information is not available
at that time printing "Failed to fetch information" (This is in
accordance with gdb out put)
TBD
1. Stack unwinding code is still need to be implemented
2. Support for x86 dumps is still not provided
3. Context switching via -r command still needs to implemented
Note: We were planning to display the variable values instead of
addresses as we are doing currently, but it adds up to additional
challenges in terms of typecasting the variables, in case it is array or
structure, so for the time being we are printing only address and expect
user to dump contents using rd command. Suggestions here will be very
helpful.
Attaching the code...but the Code is not extensively tested
please let us know your thoughts
Thanks
Sharyathi Nagesh
15 years
elfutils status 2009-4-13
by Roland McGrath
The past is gone. I have nothing to report for my woeful lack of elfutils
status reports since February. Some this and that here and there, not much.
By next week I hope we'll get back to a coherent set of tasks for the near
term and weekly progress tracking.
This week:
Expected time on elfutils: 50%
* dwfl_module_getsym vs !SHF_ALLOC change (stap report)
* RHBZ#495213: eu-strip misbehavior
* RHBZ#494858: crash in core-file support
* DwarfTasks update
** reloc-aware reader items
** C++ reloc support items
** reference equality plan
** writer components
* post wrt writer component design
15 years
relocation of DW_AT_low_pc against non-exec and/or non-alloc section
by Petr Machata
Hi,
I'm getting a couple cases where a relocation of DW_AT_low_pc and
DW_AT_high_pc attribute is formed against a section that's either
non-EXEC, or even non-ALLOC.
The case of non-EXEC is in .rodata of vmlinux. A single die that comes
from arch/x86/kernel/trampoline_32.S has this property. So I shift my
understanding of low_pc and high_pc from "we can expect PC to have this
address" to "it's a place in address space".
Another case is basically any GRUB module. These have relocations of
low_pc and high_pc formed against .moddeps, which is non-ALLOC section
(which, if I understand things correctly, means it doesn't end up in
address space at all).
So, is my ALLOC & EXEC expectation valid? If it's too strict, is any
ALLOC section kosher, and GRUB modules are erroneous, or is any section
at all kosher?
PM
15 years