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

[Andy Wingo]

On Wed 22 May 2013 23:06, address@hidden (Ludovic Courtès) writes:

>> +/* This function leaks the memory that it allocates.  */
>> +static char*
>> +alloc_aligned (size_t len, unsigned alignment)
>
> What about using posix_memalign or similar?

Not present on many systems, and gnulib can't make a useful wrapper.  If
you do the normal trick of allocating more with malloc then aligning it,
you make it impossible to free().

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

For two reasons.  One is that this memory is not collectable.  Static
allocation of constants means that you can get pointers into the middle
of the image from the heap.  I wanted to avoid pressuring the GC.

The other reason is that you can get pointers out of the image *to* the
heap -- for data that is statically allocated, but fixed up at runtime.
For example, keywords.  Their stringbuf is allocated statically in
read-only memory, but there is also a cell allocated for the keyword
itself, and that lives on the heap.

The best is to use mmap.  But if you fall back, malloc is OK, and we can
manually align it.  In any case the loader ends up adding part of the
image to the GC roots, so it's probably best that the memory not be
managed by the GC itself.

>> +  /* 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?

Yes!  If the system supports mmap, .go files should be produced with
4096-byte alignment so that the whole thing can be mapped read-only, and
write permissions only need be given to the writable pages.  (Mprotect
only works on a page level.)

Cheers,

Andy
-- 
http://wingolog.org/