This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]