[RFC] Using protected for math symbols
Commit Message
GLIBC uses non-standard names internally for math calls. As a result
even basic functions like sqrt and fabs are not inlined by default.
I recently posted patches to inline sqrt and fabs. In order for this to work
in cases GCC doesn't inline, asm redirects and conditional defines
are needed.
An alternative approach to this is to completely remove the __sqrt
variants and use protected symbol visibility. This means math functions
are defined using their real name, internal calls use the real name and
don't generate PLT calls, and no more weak/redirect magic is needed.
It seems much cleaner to do it this way, so my question is, are there
any issues with this approach?
The patch below shows how it might look like for sqrt as the difference
between the current approach and the proposed one. Only generic code
has been changed - target specific assembly code may need similar
edits:
--
Comments
On Fri, 29 Sep 2017, Wilco Dijkstra wrote:
> An alternative approach to this is to completely remove the __sqrt
> variants and use protected symbol visibility. This means math functions
> are defined using their real name, internal calls use the real name and
> don't generate PLT calls, and no more weak/redirect magic is needed.
Protected visibility is best avoided in general.
https://www.airs.com/blog/archives/307
> -__sqrt (double x)
> +sqrt (double x)
> {
> if (__builtin_expect (isless (x, 0.0), 0) && _LIB_VERSION != _IEEE_)
> return __kernel_standard (x, x, 26); /* sqrt(negative) */
>
> return __ieee754_sqrt (x);
> }
> -libm_alias_double (__sqrt, sqrt)
Having names such as __sqrt is convenient even if they aren't called
anywhere, as it allows libm_alias_double to be used as a uniform way of
getting sqrt, sqrtl (in long double = double case), sqrtl compat symbol
(in case where long double used to = double), sqrtf64 (in future) and
sqrtf32x (in future) all defined at once.
On Fri, 29 Sep 2017, Wilco Dijkstra wrote:
> GLIBC uses non-standard names internally for math calls. As a result
> even basic functions like sqrt and fabs are not inlined by default.
> I recently posted patches to inline sqrt and fabs. In order for this to work
> in cases GCC doesn't inline, asm redirects and conditional defines
> are needed.
>
> An alternative approach to this is to completely remove the __sqrt
> variants and use protected symbol visibility. This means math functions
> are defined using their real name, internal calls use the real name and
> don't generate PLT calls, and no more weak/redirect magic is needed.
>
> It seems much cleaner to do it this way, so my question is, are there
> any issues with this approach?
Yes, you would need to post-process libc.so to change protected
visibility to default, otherwise LLD will fail to link non-PIE
executables that need PLT slots for such protected functions. Their
position is that otherwise those functions may have two different addresses:
from the point of view of libc.so (the address of the implementation)
and any other loaded module (the address of the PLT slot in the executable),
and the linker should not allow that by default.
https://bugs.llvm.org/show_bug.cgi?id=32425
Alexander
On 09/29/2017 06:46 PM, Wilco Dijkstra wrote:
> An alternative approach to this is to completely remove the __sqrt
> variants and use protected symbol visibility. This means math functions
> are defined using their real name, internal calls use the real name and
> don't generate PLT calls, and no more weak/redirect magic is needed.
I don't think this allows us to address namespace issues with static
linking.
Thanks,
Florian
@@ -55,14 +55,10 @@ libm_hidden_proto (__expf128)
libm_hidden_proto (__expm1f128)
# endif
-# ifndef NO_SQRT_REDIRECT
-/* Declare sqrt for use within GLIBC. Sqrt will typically inline into a
- single instruction. Use an asm to avoid use of PLTs if it doesn't. */
-float sqrtf (float) asm ("__ieee754_sqrtf");
-double sqrt (double) asm ("__ieee754_sqrt");
-# ifndef __NO_LONG_DOUBLE_MATH
-long double sqrtl (long double) asm ("__ieee754_sqrtl");
-# endif
+float sqrtf (float) __attribute__ ((visibility ("protected")));
+double sqrt (double) __attribute__ ((visibility ("protected")));
+# ifndef __NO_LONG_DOUBLE_MATH
+long double sqrtl (long double) __attribute__ ((visibility ("protected")));
# endif
#endif
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#define NO_SQRT_REDIRECT
#include <math.h>
#include <math_private.h>
#include <math-svid-compat.h>
@@ -26,12 +25,11 @@
#if LIBM_SVID_COMPAT
/* wrapper sqrt */
double
-__sqrt (double x)
+sqrt (double x)
{
if (__builtin_expect (isless (x, 0.0), 0) && _LIB_VERSION != _IEEE_)
return __kernel_standard (x, x, 26); /* sqrt(negative) */
return __ieee754_sqrt (x);
}
-libm_alias_double (__sqrt, sqrt)
#endif
@@ -21,20 +21,18 @@
for each floating-point type. */
#if __USE_WRAPPER_TEMPLATE
-#define NO_SQRT_REDIRECT
# include <errno.h>
# include <fenv.h>
# include <math.h>
# include <math_private.h>
FLOAT
-M_DECL_FUNC (__sqrt) (FLOAT x)
+M_DECL_FUNC (sqrt) (FLOAT x)
{
if (__glibc_unlikely (isless (x, M_LIT (0.0))))
/* Domain error: sqrt(x<-0). */
__set_errno (EDOM);
return M_SUF (__ieee754_sqrt) (x);
}
-declare_mgen_alias (__sqrt, sqrt)
#endif /* __USE_WRAPPER_TEMPLATE. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#define NO_SQRT_REDIRECT
#include <math.h>
#include <math_private.h>
#include <math-svid-compat.h>
@@ -26,12 +25,11 @@
#if LIBM_SVID_COMPAT
/* wrapper sqrtf */
float