On Wed, Mar 28, 2012 at 02:54:49PM -0700, Roland McGrath wrote:
> On Wed, Mar 28, 2012 at 01:36:26PM -0700, Roland McGrath wrote:
> > As mentioned before, if we merge and finish up the relocate branch
> > then the problem goes away.
> But we currently don't have an estimate for when this will happen.
IIRC it was pretty well ready when I left it. I just merged the trunk
into the branch. It still builds except for strip.c where you copied
some of the libdwfl/relocate.c code. That needs to get the
corresponding updates to use ebl_reloc_simple_types instead of
ebl_reloc_simple_type. Do you want to make those fixes on the branch?
Thanks for that. I'll take a look.
I think the main barrier was wanting some more consideration on the
API additions (look at 'git diff master..roland/relocate -- libdw.h')
before we enshrined them in the permanent libdw ABI. So one option is
just to decide we think they are good enough as they are.
OK. I'll start a new review thread for that. I admit I didn't know it
was already that close.
Another option is to do an intermediate version of the branch with
the internals changes, but without making any new interfaces public.
Then libdwfl-based uses like systemtap's get the (presumed, measurable)
performance benefits of reloc-on-demand and the compressed-section
problem goes away for libdwfl. But we don't commit to any new APIs or
I can try it against some systemtap testcases to see how it goes.
> Until we do finish that I like to not apply relocations to
> sections, since we know that just doesn't work (and the corruption might
> not even be detected if we do). That is really all the patch does.
I have several concerns here. Even if we did just that, then I thought
the patch looked ugly enough that it can probably be done in a way
somewhat cleaner than that. But that's just the trivial concern.
Since I thought the patch was as clean as I could come up with please
do let me know what you thought was the ugly part. I might have to
adjust my "beautiful code" filter :) The patch really was minimal
diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index 95206f4..effef44 100644
@@ -319,6 +319,12 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr
if (tdata == NULL)
+ if (tname == '.' && tname == 'z' &&
tdata->d_size >= 4 + 8
+ && memcmp (tdata->d_buf, "ZLIB", 4) == 0)
+ /* Compressed section can only be relocated after decompression,
+ don't touch it or we will corrupt it. */
+ return DWFL_E_NOERROR;
/* Apply one relocation. Returns true for any invalid data. */
Dwfl_Error relocate (GElf_Addr offset, const GElf_Sxword *addend,
int rtype, int symndx)
It concerns me more that we would be papering over our lack of
support for .zdebug_* sections while making it appear that we might
handle them. Giving such a false impression might be worse than an
"obviously broken" state.
OK, I was more concerned with the possibility of silently corrupting
the sections libdwfl prepared.
The support I added in commit 725aad5d was pretty minimal and I
overlooked multiple subtleties at the time. (We weren't near a release
at the time, and I probably figured we'd have more close attention
before the next release than we actually had in the year--exactly one
year, it so happens--that we had from then until 0.153.) The libdwfl
interaction for ET_REL that's been noticed here is just one. Another
that pops to mind now is that ebl_debugscn_p doesn't match .zdebug_*
sections. What effects does that have?
Some strip variants would get confused, I can write some tests for that.
Funnily relocate () has a debug flag which checks ebl_debugscn_p and doesn't
relocate if the target isn't a ebl_debugscn_p (). But we never use that