Re: [Qemu-devel] [ARM/FDPIC v3 3/4] linux-user: ARM-FDPIC: Add support of FDPIC for ARM.

[Peter Maydell]

On 24 April 2018 at 08:27, Christophe Lyon <address@hidden> wrote:
> Add FDPIC info into image_info structure since interpreter info is on
> stack and needs to be saved to be accessed later on.
>
> Compared to v2: Do not add the is_fdpic field to TaskState, as the
> information can be retrieved from the 'info' data in TaskState.
>
> Co-Authored-By:  Mickaël Guêné <address@hidden>
> Signed-off-by: Christophe Lyon <address@hidden>
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 76d7718..fad3c42 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -78,6 +78,11 @@ enum {
>   */
>  #define personality(pers)       (pers & PER_MASK)
>
> +int info_is_fdpic(struct image_info *info)
> +{
> +    return (info->personality == PER_LINUX_FDPIC);

The () here aren't required.

> +}
> +
>  /* this flag is uneffective under linux too, should be deleted */
>  #ifndef MAP_DENYWRITE
>  #define MAP_DENYWRITE 0
> @@ -287,6 +292,25 @@ static inline void init_thread(struct target_pt_regs 
> *regs,
>      /* For uClinux PIC binaries.  */
>      /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
>      regs->uregs[10] = infop->start_data;
> +
> +    /* Support ARM FDPIC.  */
> +    if (info_is_fdpic(infop)) {
> +        /* As described in the ABI document, r7 points to the loadmap info
> +         * prepared by the kernel. If an interpreter is needed, r8 points
> +         * to the interpreter loadmap and r9 points to the interpreter
> +         * PT_DYNAMIC info. If no interpreter is needed, r8 is zero, and
> +         * r9 points to the main program PT_DYNAMIC info.
> +         */
> +        regs->uregs[7] = infop->loadmap_addr;
> +        if (infop->interpreter_loadmap_addr) {
> +            /* Executable is dynamically loaded.  */
> +            regs->uregs[8] = infop->interpreter_loadmap_addr;
> +            regs->uregs[9] = infop->interpreter_pt_dynamic_addr;
> +        } else {
> +            regs->uregs[8] = 0;
> +            regs->uregs[9] = infop->pt_dynamic_addr;
> +        }
> +    }
>  }
>
>  #define ELF_NREG    18
> @@ -1745,6 +1769,11 @@ static abi_ulong create_elf_tables(abi_ulong p, int 
> argc, int envc,
>          if (interp_info) {
>              interp_info->other_info = info;
>              sp = loader_build_fdpic_loadmap(interp_info, sp);
> +            info->interpreter_loadmap_addr = interp_info->loadmap_addr;
> +            info->interpreter_pt_dynamic_addr = interp_info->pt_dynamic_addr;
> +        } else {
> +            info->interpreter_loadmap_addr = 0;
> +            info->interpreter_pt_dynamic_addr = 0;
>          }
>      }
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index da3b517..f8b9896 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -57,6 +57,8 @@ struct image_info {
>          uint16_t        nsegs;
>          void           *loadsegs;
>          abi_ulong       pt_dynamic_addr;
> +        abi_ulong       interpreter_loadmap_addr;
> +        abi_ulong       interpreter_pt_dynamic_addr;
>          struct image_info *other_info;
>  };
>
> @@ -144,7 +146,6 @@ typedef struct TaskState {
>       * from multiple threads.)
>       */
>      int signal_pending;
> -
>  } __attribute__((aligned(16))) TaskState;

Stray whitespace change.

>
>  extern char *exec_path;
> @@ -182,6 +183,7 @@ abi_ulong loader_build_argptr(int envc, int argc, 
> abi_ulong sp,
>  int loader_exec(int fdexec, const char *filename, char **argv, char **envp,
>               struct target_pt_regs * regs, struct image_info *infop,
>               struct linux_binprm *);
> +int info_is_fdpic(struct image_info *info);

Could we have a doc comment for a new global function, please?

>
>  uint32_t get_elf_eflags(int fd);
>  int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
> --
> 2.6.3

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM