Re: [PATCH, MPX runtime 1/2] Integrate MPX runtime library

[Joseph Myers <joseph at codesourcery dot com>]

On Tue, 11 Nov 2014, Ilya Enkovich wrote:

> Hi,
> 
> This patch integrates MPX runtime library into GCC source tree.  MPX 
> runtime is responsible for initialization of MPX feature in HW, signal 
> handling, reporting etc.  Library is linked to codes compiled with 
> -mmpx.
> 
> Bootstrap is OK for x86_64-unknown-linux-gnu.  OK for trunk?

Please use symbol versioning to control the set of exports for the 
library; only symbols explicitly listed to be exported at a given symbol 
version should be exported.

If the library is intended to be used implicitly by compiler-generated 
code rather than directly by users, all exported symbols should be in the 
implementation namespace - only symbols intended to be called by users 
should not start with "__".

In any case, symbols not intended to be exported should be in the 
implementation namespace, or static, to avoid namespace issues for static 
linking.

The code contains unchecked malloc calls.  You always need to allow for 
errors from library functions.

The code contains unchecked sprintf to static buffers using values from 
the environment.  Use snprintf.

The code contains calls to getenv to choose files to open.  You need to 
use __secure_getenv / secure_getenv when available (probably for all 
getenv calls, unless you have a clear reason certain calls are safe even 
for setuid programs).

You have lots of static writable variables.  Are you sure all those 
variables are handled in a thread-safe way (e.g. only modified before any 
threads start)?

-- 
Joseph S. Myers
joseph@codesourcery.com