[PATCH v2 03/15] RISC-V: Startup and Dynamic Loading Code

[Palmer Dabbelt]

On Wed, 20 Dec 2017 08:38:48 PST (-0800), joseph@codesourcery.com wrote:
> On Tue, 19 Dec 2017, Palmer Dabbelt wrote:
>
>> +extern ElfW(Addr) la_riscv_gnu_pltenter (ElfW(Sym) *__sym, unsigned int __ndx,
>
> With reference to my previous comments, ElfW is an exception to the rule
> of spaces before '(' in macro and function calls, because it locally
> expands to a single identifier.  So this code is correct not to have a
> space there.
>
>> +  extern ElfW(Addr) _GLOBAL_OFFSET_TABLE_ __attribute__((visibility("hidden")));

Ah, luckily I missed those when going through you last mail :)

>
> However, __attribute__ is *not* an exception.  This should use:
> __attribute__ ((visibility ("hidden"))) - and likewise for other
> attributes, elsewhere in this patch or the patch series as a whole.

Ah, thanks -- I'd missed these too.

> General question in the area of this patch: static PIE support was
> recently added to glibc.  Do you support it for RISC-V?  It's not so far a
> requirement for adding a port, but if not supported, when the port goes in
> it would need adding to the list at
> <https://sourceware.org/glibc/wiki/PortStatus> of ports for which static
> PIE is not yet known to work.

I haven't tried it, so I assume it does not work.  It looks like the static PIE 
section's header is broken on that site, I'm seeing

    == Static PIE ===

in normal text.  I assume it's supposed to look more like "=== Static PIE ===" 
to make the text large?

Based on looking through the requirement list, there's enough things there in 
toolchain land that "should just work" that I'm sure something will end up 
being broken.  I assume it's OK to just add the various RISC-V targets to the 
unsupported list for now?