This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Remove no longer applicable assertion.
On Fri, Apr 6, 2012 at 5:27 AM, David Miller <davem@davemloft.net> wrote:
> From: "Carlos O'Donell" <carlos@systemhalted.org>
> Date: Fri, 6 Apr 2012 05:17:17 -0400
>
>> Would the following be more optimal?
>
> Well, it's taking another step. ?My changes accomodated the case where
> there is no DT_REL(A) present at all, as did the existing code.
>
> But with your change, we are assuming that a DT_REL(A) is always
> present.
>
> And if you think it's OK to assume that, then you can also turn the
> first if() statement in this macro:
>
> ? ?if ((map)->l_info[DT_##RELOC])
>
> into an assertion and just cons up ranges[0] unconditionally.
So you can, but only only if ELF_DURING_STARTUP. Note that if
ELF_DURING_STARTUP then that appears to guarantee you a DT_REL*
because the code always merges ranges in that case.
In the general case you're right, I don't think we can assume DT_REL*
is present in all cases. You could conceivably have no non-PLT
relocations. In which case there is still an optimziation to be had.
You need only check for a zero sized range[0]?
diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
index 310ad5e..9eb25d0 100644
--- a/elf/dynamic-link.h
+++ b/elf/dynamic-link.h
@@ -278,13 +278,7 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
\
if (__builtin_expect (ranges[0].size, 1)) \
ranges[0].size = (start - ranges[0].start); \
- if (! ELF_DURING_STARTUP \
- && ((do_lazy) \
- /* This test does not only detect whether the relocation \
- sections are in the right order, it also checks whether \
- there is a DT_REL/DT_RELA section. */ \
- || __builtin_expect (ranges[0].start + ranges[0].size \
- != start, 0))) \
+ if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0) \
{ \
ranges[1].start = start; \
ranges[1].size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val; \
@@ -293,7 +287,6 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
else \
{ \
/* Combine processing the sections. */ \
- assert (ranges[0].start + ranges[0].size == start); \
ranges[0].size += (map)->l_info[DT_PLTRELSZ]->d_un.d_val; \
} \
}
~~~
Cheers,
Carlos.