Re: [PATCH] In _IEEE_LIBM mode, use weak symbols instead of wrapper funcs [v2]

"Keith Packard" <[email protected]> Fri, 21 Sep 2018 12:34:18 -0700
Newsgroups	gmane.comp.lib.newlib
Message-ID	<[email protected]>
Craig Howland <[email protected]> writes:

> Unless I'm missing something, there seems to be a fundamental problem with the 
> approach in that the regular function definitions are unconditionally eliminated 
> when the weak aliases are defined. So while this will work fine for targets 
> which can do the aliases, the needed function wrappers are missing for when 
> aliases do not work.  One example shown.

Hrm. I note that __weak_reference is only used in the sys/linux/net
code currently, which means the definition of that macro could well be
missing on any non-linux platform and we would never know. Of course,
we'd get a compiler error if __weak_reference weren't defined, but
that's not a lot of help to a user interested in building the library.

I think I should make the use of this feature depend on whether there is a
definition of __weak_reference.

> But then the entire acos() function is deleted when defined(_IEEE_LIBM).  This 
> is fine when the alias works, but not when the alias does not work.  That is, 
> this gate also needs to know if the alias will work, rather than being based 
> only on _IEEE_LIBM.

Yup, I think it would be sufficient to check for the definition of
__weak_reference in both places; both to make the creation of the
reference work and to ensure that the wrapper is still defined when necessary.

> In addition, this method can potentially introduce a change in link-time 
> behavior, because the library now has the primary function names as weak aliases 
> instead of definite functions.  This won't necessarily make a difference while 
> linking, but it could.  (While you could guess that the vast majority of uses 
> would have no trouble at all, the question is if there are any which would.)  
> This may or may not be acceptable, but the possibility needs to be considered 
> when evaluating this approach.  (I'm pretty sure it would not affect any of my 
> targets.)  Even ignoring that possibility, just the principle of having primary 
> C library functions in the library as weak aliases seems like a bad
> idea.

That's another good point -- in looking at cdefs.h, I've found that
there's another macro, __strong_reference, which creates non-weak
symbols and should eliminate this issue.

> Why not just abandon the alias approach and instead just re-name the ieee 
> functions?  It solves both of the problems mentioned.  These edits are half-way 
> there, as they get rid of the wrapper functions. But instead of adding the weak 
> references, map the ieee names with the preprocessor.  It would have to be done 
> in something like fdlibm.h because there are some internal calls from ieee to 
> ieee, but the basic name-mapping collection could readily be
> retargeted.

I didn't want to change the ABI of the library to ensure that existing
applications would still work properly. If I switch from
__weak_reference to __strong_reference, and make use of this feature
conditional upon whether that macro is defined for the platform.

> Taking a step back, there's a higher-level question.  The Newlib math libary is 
> already quite goofy with the variations in error handing, making multiple 
> flavors, etc.  Is it perhaps time to give this strangeness up and just make them 
> so that they are C/POSIX compliant?  This too would achieve the desired goal of 
> getting rid of the wrappers.

I would actually prefer the IEEE behavior as that doesn't return errors
in 'errno'. I was happy to find this mode already available in the
library as it matches what I feel are the right semantics for a deeply
embedded system.

Let me re-spin the patch to use __strong_reference, and to only perform
this operation when that macro is available. That will provide a smaller
implementation of the existing feature where possible and without
introducing a difference in the exported symbols.

-- 
-keith
signature.asc (application/pgp-signature, 832 B)