Re: [PATCH v3 09/19] RISC-V: Startup and Dynamic Loading Code

[Joseph Myers <joseph at codesourcery dot com>]

On Tue, 26 Dec 2017, Palmer Dabbelt wrote:

> +/* Return the link-time address of _DYNAMIC.  */
> +static inline ElfW(Addr)
> +elf_machine_dynamic (void)
> +{
> +  extern ElfW(Addr) _GLOBAL_OFFSET_TABLE_ __attribute__ ((visibility("hidden")));

Missing space between "visibility" and "(".  Please check for and fix such 
issues throughout the patch series.

> +/* Return the run-time load address of the shared object.  */
> +static inline ElfW(Addr)
> +elf_machine_load_address (void)
> +{
> +  ElfW(Addr) load_addr;
> +  asm ("lla %0, _DYNAMIC" : "=r"(load_addr));

Missing space before "(load_addr)".  Again, please check for and fix such 
issues in asms throughout the patch series.

> +  /* Check for unexpected PLT reloc type.  */
> +  if (__builtin_expect (r_type == R_RISCV_JUMP_SLOT, 1))
> +    {
> +      if (__builtin_expect (map->l_mach.plt, 0) == 0)

When __builtin_expect is being used for a boolean condition, please use 
__glibc_likely and __glibc_unlikely instead throughout this patch series.  
So that's __glibc_likely (r_type == R_RISCV_JUMP_SLOT) and __glibc_likely 
(map->l_mach.plt == 0), in this case.

> +auto inline int
> +__attribute__ ((always_inline))
> +elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
> +{
> +#ifndef RTLD_BOOTSTRAP
> +  /* If using PLTs, fill in the first two entries of .got.plt.  */
> +  if (l->l_info[DT_JMPREL])
> +    {
> +      extern void _dl_runtime_resolve (void) __attribute__ ((visibility("hidden")));

Another missing space in visibility attribute.

-- 
Joseph S. Myers
joseph@codesourcery.com