[CRIU] [PATCH] pie: Give VDSO symbol table local scope

[Laurent Dufour]

On 05/10/2015 17:30, Christopher Covington wrote:
> Hi Laurent,
> 
> On 10/05/2015 10:54 AM, Laurent Dufour wrote:
>> On 03/10/2015 14:47, Christopher Covington wrote:
>>> In commit c2271198, Laurent Dufour kindly reunified the VDSO code
>>> that had become duplicated between architectures. Unfortunately
>>> this introduced a regression in AArch64 where apparently due to
>>> the scope of vdso_symbols array of pointers to characters changing
>>> from local to global, load-time relocations became necessary.
>>>
>>> The following thread on the GCC mailing list discusses why
>>> load-time relocations can be necessary when pointers are used,
>>> although it doesn't mention the potential for locally scoped
>>> arrays to be handled differently:
>>> https://gcc.gnu.org/ml/gcc/2004-05/msg01016.html
>>>
>>> Because the alternatives, such as porting piegen to AArch64, are
>>> far more involved, simply revert the change in scope.
>>
>> Hi Christopher,
>>
>> This breaks the build on powerpc due to the gcc optimisation done on power.
>>
>> On power the compiler is calling C library memcpy to copy the
>> pre-initialised. This leads to an undefined memcpy symbol when building
>> the parasite part.
>> I'm looking at the best way to get around that.
> 
> Sorry about that. Here are some thoughts:
> 
> A) #ifdefs
> 
> Ideally with some comments and only temporary until we work out a more
> satisfactory solution.
> 
> B) Keep the declaration local but make it static const instead of just const
> 
> You've probably already seen this, but this approach is mentioned in the
> comments here:
> http://stackoverflow.com/questions/6410595/getting-gcc-to-compile-without-inserting-call-to-memcpy

This works for me, but I would be surprised that it works on ARM without
any relocation in place. Isn't it ?

> 
> C) Pack multiple strings into a single char* that's double-null terminated
> 
> Use while() instead of for(), etc.

Not very sexy.

> 
> D) Port piegen to AArch64
> 
> I'd like to do this at some point, if only as an exercise to learn more about
> ELF and AArch64, but I'm not sure I'll have to the time to do it in the next
> few weeks.

I guess at one time this will be required, because there are other place
where such a declaration could be made.

There is another option :
E) Pn powerPC, since the compiler relies on memcpy to set the data in
the stack, define a memcpy service in vdso-pie.c. This way even if there
are other similar declaration, this should work.

I think I will implement the option E) even if B) works fine on power,
this may be convenient in the future.

Laurent.


> 
> Christopher Covington
> 
>>> diff --git a/include/util-vdso.h b/include/util-vdso.h
>>> index 2942337..c8dfa05 100644
>>> --- a/include/util-vdso.h
>>> +++ b/include/util-vdso.h
>>> @@ -60,10 +60,6 @@ static inline unsigned long vvar_vma_size(struct vdso_symtable *t)
>>>  	return t->vvar_end - t->vvar_start;
>>>  }
>>>
>>> -extern const char *vdso_symbols[VDSO_SYMBOL_MAX];
>>> -
>>>  extern int vdso_fill_symtable(char *mem, size_t size, struct vdso_symtable *t);
>>>
>>> -
>>> -
>>>  #endif /* __CR_UTIL_VDSO_H__ */
>>> diff --git a/pie/util-vdso.c b/pie/util-vdso.c
>>> index 9711aaa..65746d9 100644
>>> --- a/pie/util-vdso.c
>>> +++ b/pie/util-vdso.c
>>> @@ -24,10 +24,6 @@
>>>  #endif
>>>  #define LOG_PREFIX "vdso: "
>>>
>>> -const char *vdso_symbols[VDSO_SYMBOL_MAX] = {
>>> -	ARCH_VDSO_SYMBOLS
>>> -};
>>> -
>>>  /* Check if pointer is out-of-bound */
>>>  static bool __ptr_oob(void *ptr, void *start, size_t size)
>>>  {
>>> @@ -54,6 +50,10 @@ static unsigned long elf_hash(const unsigned char *name)
>>>
>>>  int vdso_fill_symtable(char *mem, size_t size, struct vdso_symtable *t)
>>>  {
>>> +	const char *vdso_symbols[VDSO_SYMBOL_MAX] = {
>>> +		ARCH_VDSO_SYMBOLS
>>> +	};
>>> +
>>>  	Elf64_Phdr *dynamic = NULL, *load = NULL;
>>>  	Elf64_Ehdr *ehdr = (void *)mem;
>>>  	Elf64_Dyn *dyn_strtab = NULL;
>>>
>>
> 
> 

  * Anglais - détecté
  * Français
  * Anglais

  * Français
  * Anglais

 <javascript:void(0);>

 <#>