On Tue, Feb 25, 2020 at 3:06 PM William Roberts <bill.c.roberts@xxxxxxxxx> wrote: > > On Tue, Feb 25, 2020 at 2:49 PM Stephen Smalley > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > On Tue, Feb 25, 2020 at 3:03 PM <bill.c.roberts@xxxxxxxxx> wrote: > > > The annoying part is internal users of the routines. We could always make > > > a v2 version of the function for internal callers, and leave the old > > > interfaces intact to work around the warnings, or just pragma them out. > > > This series pragma's them out. > > > > > > diagnostic push has been supported since GCC v4.6. Earlier versions will > > > warn on this, and the sideffect is that the diagnostic ignored pragma > > > will be valid for the rest of the file. Clang has similair support thats > > > been around *at least* since clang 6.0. > > > > My inclination (and others are free to disagree) would be to rename > > the deprecated functions for internal users (and mark them hidden), > > and add stubs for the old interfaces that call the hidden functions to > > avoid the need for this pragma. > > I'm actually leaning that way myself after thinking about it more. It doesn't get us around all the issues, anything marked deprecated in selinux.h and marked hidden_proto in selinux_internal.h, will still need a pragma. This would be much simpler if we drop the whole hidden_proto stuff, I guess kloczek is proposing patches? Who is that, I see the Github name as: Tomasz Kłoczko But I can't find any matching author in the git logs to CC them. I'm looking at those macros: hidden_def hidden_proto They both seem to take a function and create an _internal symbol, and set the global symbol equal to the internal one. Essentially, both the normal and _internal functions are pointing to the same address. $ readelf -s ./src/libselinux.so | grep setexeccon_raw 176: 0000000000017c26 63 FUNC GLOBAL DEFAULT 12 setexeccon_raw 279: 0000000000017c26 63 FUNC LOCAL DEFAULT 12 setexeccon_raw_internal 606: 0000000000017c26 63 FUNC GLOBAL DEFAULT 12 setexeccon_raw The hidden one, additionally sets the visibility to hidden, which if IIUC, just requires one to statically link. We can see above the visibility is local. What I don't understand, is why one would do this? What actual performance enhancement do we get? > > > Also, FWIW, there is a push to remove > > the hidden_def/hidden_proto stuff for LTO, see > > https://github.com/SELinuxProject/selinux/issues/204, although I don't > > yet know how that will turn out for libselinux. > > Good to know.
