Re: [PATCH 5/6] refactor and simplify ELF loader in objcodes.c

[Ludovic Courtès]

Andy Wingo <address@hidden> skribis:

> * libguile/objcodes.c (sniff_elf_alignment, alloc_aligned)
>   (copy_and_align_elf_data): New helpers for portably re-aligning ELF
>   data from read(2) or from a bytevector.
>   (load_thunk_from_memory): Simplify!  Now there is only one procedure
>   that loads ELF, and it does less: it simply receives the whole image
>   in one array, hopefully from mmap.
>
>   (scm_load_thunk_from_file): Use new map_file_contents helper, and go
>   through load_thunk_from_memory.
>   (scm_load_thunk_from_memory): Pass load_thunk_from_memory a piece of
>   memory that it owns, and that is appropriately aligned.
> ---
>  libguile/objcodes.c |  435 
> +++++++++++++++++++++++----------------------------
>  1 file changed, 195 insertions(+), 240 deletions(-)

Lines removed, cool!  ;-)

> +static unsigned
> +sniff_elf_alignment (const char *data, size_t len)

What about:

  /* Return the alignment required by the ELF at DATA,
     of LEN bytes.  */
  static size_t
  elf_alignment (const char *elf, size_t len)

> +/* This function leaks the memory that it allocates.  */
> +static char*
> +alloc_aligned (size_t len, unsigned alignment)

What about using posix_memalign or similar?

Alternatively, using scm_gc_malloc_pointerless, which is known to return
8-byte-aligned boundary?

Or using ALIGN (malloc (size + 8), 8)?

> +  /* Given that we are using the read fallback, optimistically assume
> +     that the .go files were made with 8-byte alignment.
> +     alignment.  */
> +  data = malloc (end);
> +  if (!data)

scm_malloc would save you from the if (!data).
Would it be OK to use scm_gc_malloc_pointerless here?

> +  /* If our optimism failed, fall back.  */
> +  {
> +    unsigned alignment = sniff_elf_alignment (data, end);
> +
> +    if (alignment != 8)

Since .go are only produced by Guile, can it really happen to have .go
files without 8-byte alignment?

Ludo’.