[v2] xtensa: ldso: coalesce dl_mprotect address ranges

This noticeably lowers the number of mprotect calls at program startup,
e.g. for busybox: 7 calls vs 1835 calls.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
Changes v1->v2:
- initialize prev_got_start and prev_got_end in INIT_GOT as well.

 ldso/ldso/xtensa/dl-startup.h | 18 +++++++++++++++++-
 ldso/ldso/xtensa/dl-sysdep.h  | 22 +++++++++++++++++++++-
 2 files changed, 38 insertions(+), 2 deletions(-)

Max,
Are the symbols usually ordered in ascending order? Otherwise, would it
also safe time to check if the new range overlaps at the front?

Cheers!
-Chris

On Fri, Jan 9, 2015 at 5:06 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:

> This noticeably lowers the number of mprotect calls at program startup,
> e.g. for busybox: 7 calls vs 1835 calls.
>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
> Changes v1->v2:
> - initialize prev_got_start and prev_got_end in INIT_GOT as well.
>
>  ldso/ldso/xtensa/dl-startup.h | 18 +++++++++++++++++-
>  ldso/ldso/xtensa/dl-sysdep.h  | 22 +++++++++++++++++++++-
>  2 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/ldso/ldso/xtensa/dl-startup.h b/ldso/ldso/xtensa/dl-startup.h
> index b135a4c..c60c9cf 100644
> --- a/ldso/ldso/xtensa/dl-startup.h
> +++ b/ldso/ldso/xtensa/dl-startup.h
> @@ -83,6 +83,7 @@ do { \
>         unsigned long l_addr = tpnt->loadaddr; \
>         Elf32_Word relative_count; \
>         unsigned long rel_addr; \
> +       Elf32_Addr prev_got_start = 0, prev_got_end = 0; \
>         int x; \
>  \
>         got_loc = (xtensa_got_location *) \
> @@ -93,7 +94,22 @@ do { \
>                 got_start = got_loc[x].offset & ~(PAGE_SIZE - 1); \
>                 got_end = ((got_loc[x].offset + got_loc[x].length +
> PAGE_SIZE - 1) \
>                                    & ~(PAGE_SIZE - 1)); \
> -               _dl_mprotect ((void *)(got_start + l_addr), got_end -
> got_start, \
> +               if (got_start >= prev_got_start && got_start <=
> prev_got_end) { \
> +                       if (got_end > prev_got_end) \
> +                               prev_got_end = got_end; \
> +                       continue; \
> +               } else if (prev_got_start != prev_got_end) { \
> +                       _dl_mprotect ((void *)(prev_got_start + l_addr), \
> +                                                 prev_got_end -
> prev_got_start, \
> +                                                 PROT_READ | PROT_WRITE |
> PROT_EXEC); \
> +               } \
> +               prev_got_start = got_start; \
> +               prev_got_end = got_end; \
> +       } \
> +\
> +       if (prev_got_start != prev_got_end) { \
> +               _dl_mprotect ((void *)(prev_got_start + l_addr), \
> +                                         prev_got_end - prev_got_start, \
>                                           PROT_READ | PROT_WRITE |
> PROT_EXEC); \
>         } \
>  \
> diff --git a/ldso/ldso/xtensa/dl-sysdep.h b/ldso/ldso/xtensa/dl-sysdep.h
> index a0ed4e5..958f7ef 100644
> --- a/ldso/ldso/xtensa/dl-sysdep.h
> +++ b/ldso/ldso/xtensa/dl-sysdep.h
> @@ -36,6 +36,7 @@ typedef struct xtensa_got_location_struct {
>    do {
>    \
>      xtensa_got_location *got_loc;
>     \
>      Elf32_Addr l_addr = MODULE->loadaddr;
>     \
> +    Elf32_Addr prev_got_start = 0, prev_got_end = 0;
>    \
>      int x;
>    \
>
>     \
>      got_loc = (xtensa_got_location *)
>     \
> @@ -47,7 +48,26 @@ typedef struct xtensa_got_location_struct {
>         got_start = got_loc[x].offset & ~(PAGE_SIZE - 1);
>    \
>         got_end = ((got_loc[x].offset + got_loc[x].length + PAGE_SIZE -
> 1)    \
>                    & ~(PAGE_SIZE - 1));
>    \
> -       _dl_mprotect ((void *)(got_start + l_addr) , got_end - got_start,
>    \
> +       if (got_start >= prev_got_start && got_start <= prev_got_end)
>    \
> +         {
>    \
> +           if (got_end > prev_got_end)
>    \
> +               prev_got_end = got_end;
>    \
> +           continue;
>    \
> +         }
>    \
> +        else if (prev_got_start != prev_got_end)
>    \
> +         {
>    \
> +           _dl_mprotect ((void *)(prev_got_start + l_addr),
>     \
> +                         prev_got_end - prev_got_start,
>     \
> +                         PROT_READ | PROT_WRITE | PROT_EXEC);
>     \
> +          }
>     \
> +        prev_got_start = got_start;
>     \
> +        prev_got_end = got_end;
>             \
> +      }
>             \
> +
>    \
> +    if (prev_got_start != prev_got_end)
>             \
> +      {
>             \
> +        _dl_mprotect ((void *)(prev_got_start + l_addr),
>    \
> +                     prev_got_end - prev_got_start,
>     \
>                       PROT_READ | PROT_WRITE | PROT_EXEC);
>     \
>        }
>             \
>
>     \
> --
> 1.8.1.4
>
>

Chris,

On Fri, Jan 9, 2015 at 10:24 PM, Chris Zankel <chris@zankel.net> wrote:
> Are the symbols usually ordered in ascending order? Otherwise, would it also
> safe time to check if the new range overlaps at the front?

In binaries that I have -- yes. But coalescing ranges from both sides takes
only one additional conditional move, so I'll send v3 with that. Thanks!

-- Max