[U-Boot] [PATCH V2 1/2] malloc: use hidden visibility

[Stephen Warren]

On 03/05/2016 10:44 AM, Tom Rini wrote:
> On Sat, Mar 05, 2016 at 10:30:52AM -0700, Stephen Warren wrote:
>
>> When running sandbox, the following phases occur, each with different
>> malloc implementations or behaviors:
>>
>> 1) Dynamic linker execution, using the dynamic linker's own malloc()
>> implementation. This is fully functional.
>>
>> 2) After U-Boot's malloc symbol has been hooked into the GOT, but before
>> any U-Boot code has run. This phase is entirely non-functional, since
>> U-Boot's gd symbol is NULL and U-Boot's initf_malloc() and
>> mem_malloc_init() have not been called.
>>
>> At least on Ubuntu Xenial, the dynamic linker does make both malloc() and
>> free() calls during this phase. Currently these free() calls crash since
>> they dereference gd, which is NULL.
>>
>> U-Boot itself makes no use of malloc() during this phase.
>>
>> 3) U-Boot execution after gd is set and initf_malloc() has been called.
>> This is fully functional, albeit via a very simple malloc()
>> implementation.
>>
>> 4) U-Boot execution after mem_malloc_init() has been called. This is fully
>> functional with a complete malloc() implementation.
>>
>> Furthermore, if code that called malloc() during phase 1 calls free() in
>> phase 3 or later, it is likely that heap corruption will occur, since
>> U-Boot's malloc implementation will assume the pointer is part of its own
>> heap, although it isn't. I have not actively observed this happening.
>>
>> To prevent phase 2 from happening, this patch makes all of U-Boot's malloc
>> library public symbols have hidden visibility. This prevents them from
>> being hooked into the GOT, so only code in the U-Boot binary itself
>> actually calls them; any other code will call into the standard C library
>> malloc(). This also avoids the "furthermore" issue mentioned above.
>>
>> I have seen references to this GCC pragma in blog posts from 2008, and
>> RHEL5's ancient gcc appears to accept it fine, so I believe it's quite
>> safe to use it without checking gcc version.
>>
>> Cc: Rabin Vincent <rabin at rab.in>
>> Signed-off-by: Stephen Warren <swarren at wwwdotorg.org>
>
> Reviewed-by: Tom Rini <trini at konsulko.com>
>
> But have you tried with clang instead of gcc?  I suspect it's got code
> to pretend to be gcc and do the right thing here but it's worth
> confirming.

Well, clang for sandbox doesn't compile for reasons other than these 
patches on either Ubuntu 14.04 or 16.04 pre-release, nor using the 
instructions in doc/README.clang for rpi on 16.04. However, it did work 
for rpi on Ubuntu 14.04.

So in summary, I think there's no issue using this pragma with clang, 
but I can't test that clang actually implements it and hence solves the 
problem these patches fix.