Bug 70082 - Attribute ifunc marked functions should not be allowed to call other functions.
Summary: Attribute ifunc marked functions should not be allowed to call other functions.
Status: WAITING
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: documentation
Depends on:
Blocks:
 
Reported: 2016-03-04 15:38 UTC by Carlos O'Donell
Modified: 2024-04-01 00:08 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-09-14 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos O'Donell 2016-03-04 15:38:18 UTC
In 2005 the GNU IFUNC support was documented and added to GCC via the ifunc attribute. To be honest this was a mistake, the feature is not documented and the implementation has so many caveats that it is incredibly difficult to use.

Recently gperftools added IFUNC usage in tcmalloc and they did a lot of things which break the IFUNC implementation in general, but happy to work by luck on x86_64.

I have documented some of this here:
https://sourceware.org/glibc/wiki/GNU_IFUNC

The IFUNC support is salvageable, but will require some reworking across the various architectures to organize the relocations in a sensible way, and even then it's going to be hard to allow everything. Particularly with -Wl,-z,now which needs to complete all relocations early.

To that end, and until we fix IFUNC, the use of the ifunc attribute in a function should immediately cause compilation to fail if that function calls any other function. There is simply no way to guarantee you can make that function call safely at early ld.so startup. You might allow calls to functions in the same translation unit, but then you'd have to make sure those functions don't also call other external functions. It's simply safer to limit the use to something that works.

Is such a fix feasible?
Comment 1 Carlos O'Donell 2016-03-04 15:50:21 UTC
... and non-local variable access should be disallowed, as well as TLS variables, and anything that needs a constructor to be initialized (non-POD).
Comment 2 Carlos O'Donell 2016-03-04 15:52:18 UTC
There might be the case where it is argued that documentation is all that is needed, but that doesn't yield a robust implementation. My biggest worry after seeing gperftools/tcmalloc use IFUNC is that it works on x86_64 by chance, but nowhere else. This poses a serious portability problem for ARM and IBM architectures which differ in the order of relocation processing. Thus I'd like to see ifunc attribute support the common case of code which does some computation based on AT_HWCAP (we'll pass this to all x86_64 ifunc resolvers like newer arches do) and then return a result.
Comment 3 Jakub Jelinek 2016-03-04 15:54:34 UTC
I don't think the compiler should do the policeman here, it is enough if glibc documents what it does and doesn't support in ifunc (which is of course generally arch dependent, it really depends on if you need e.g. RELATIVE relocations relocated in the current library/binary already, or if you need other data relocations, etc.  E.g. calling hidden function or even non-hidden, as long as you hide it in symbol version script, is fine.  Calling function from other library very likely not.
Comment 4 Carlos O'Donell 2016-03-04 16:23:01 UTC
(In reply to Jakub Jelinek from comment #3)
> I don't think the compiler should do the policeman here, it is enough if
> glibc documents what it does and doesn't support in ifunc (which is of
> course generally arch dependent, it really depends on if you need e.g.
> RELATIVE relocations relocated in the current library/binary already, or if
> you need other data relocations, etc.  E.g. calling hidden function or even
> non-hidden, as long as you hide it in symbol version script, is fine. 
> Calling function from other library very likely not.

The compiler is already a policeperson :-) The compiler enforces language standards. The compiler enforces that ifunc attribute functions can't be declared weak or weakref. Why not enforce the rest of the safe behaviour of the IFUNC extension? 

Is it hard to enforce the rest of the requirements? Are we worried that because we haven't enforced these rules over the last 6 years (typo, the implementation was added in 2010 not 2005 for IFUNC attribute usage) that people are using attribute ifunc with all their caveats in ways which we will now restrict? I don't particularly see how this is any different from tightening a language standard. However, I agree it's a concern.

Packages are only just starting to use IFUNC, and we should improve what we have at both the documentation and implementation level.

Filed this glibc bug to bring i386 and x86_64 up the level of all the other arches:
"Bug 19766 - All machines should pass dl_hwcap to ifunc resolvers."
https://sourceware.org/bugzilla/show_bug.cgi?id=19766

With bug 19766 fixed the x86_64 ifunc resolvers should be able to do something safely (though it will require a gcc change to generate the dep on a glibc version to ensure it works without error).
Comment 5 Carlos O'Donell 2016-03-04 17:28:59 UTC
Would it be a better solution to:

(a) Fix PLT initialization on arm and ppc64 (allows some function calls to be made).

(b) Document safe functions in glibc manual.

(c) Have gcc manual reference glibc manual safe functions list.

This way we avoid the compiler enforcement and allow developers, particularly compiler developers, to continue using ifunc in ways which are known to work (particularly look at the i386 multiversioning support which uses ifunc to call out to libgcc's __cpu_indicator_init).
Comment 6 Martin Sebor 2018-09-14 19:07:06 UTC
Carlos, do you still feel diagnosing some of the [mis]uses would be helpful, e.g., by a warning?  (I ask because I've been doing some work in this area -- pr81824 -- and I might be able to take care of this at some point as well, perhaps for GCC 10).
Comment 7 Carlos O'Donell 2018-09-16 01:37:26 UTC
(In reply to Martin Sebor from comment #6)
> Carlos, do you still feel diagnosing some of the [mis]uses would be helpful,
> e.g., by a warning?  (I ask because I've been doing some work in this area
> -- pr81824 -- and I might be able to take care of this at some point as
> well, perhaps for GCC 10).

I do still believe that detection and warning of misuses of IFUNC would be helpful. The glibc solutions required to make IFUNC robust are not in place, and so we should do what we can to warn users that it might break. They can then disable the warnings with pragmas (in the case of -Werror) or ignore them.