Fix -Os gnu_dev_* linknamespace, localplt issues (bug 15105, bug 19463)
Commit Message
Building with -Os produces linknamespace and localplt failures for,
among other functions, gnu_dev_major, gnu_dev_minor and
gnu_dev_makedev.
The issue is that those functions are not inlined when building with
-Os. While one could force them to be inlined in that case, it seems
more natural to fix this issue similarly to other namespace issues.
Thus, this patch makes gnu_dev_* into weak aliases for hidden symbols
__gnu_dev_*; __gnu_dev_* are then defined as inlines in the internal
include/sys/sysmacros.h, and uses of gnu_dev_* (often via the macros
major, minor and makedev) for which there are namespace issues are
changed to use __gnu_dev_*; where there are no namespace issues, use
of libc_hidden_proto serves to avoid unnecessary local PLT entry use.
Tested for x86_64, (a) without -Os, to verify the testsuite continues
to pass without problems and that the functions called under their new
names continue to be inlined as expected in that case; (b) with -Os,
together with
<https://sourceware.org/ml/libc-alpha/2018-02/msg00038.html> (pending
review) to fix the build, to verify that the linknamespace and
localplt failures in question go away (but because of other such
failures present, neither of the relevant bugs can yet be closed).
2018-02-02 Joseph Myers <joseph@codesourcery.com>
[BZ #15105]
[BZ #19463]
* include/sys/sysmacros.h [!_ISOMAC]
(__SYSMACROS_NEED_IMPLEMENTATION): Define macro.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC]
(_SYS_SYSMACROS_H_WRAPPER): Likewise.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (gnu_dev_major): Use
libc_hidden_proto.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (gnu_dev_minor): Likewise.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (gnu_dev_makedev):
Likewise.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (__SYSMACROS_DECL_TEMPL):
Undefine and redefine to add use __gnu_dev_ prefix.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (__SYSMACROS_IMPL_TEMPL):
Likewise.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (__gnu_dev_major): Declare
and define as hidden inline function.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (__gnu_dev_minor):
Likewise.
[!_SYS_SYSMACROS_H_WRAPPER && !_ISOMAC] (__gnu_dev_makedev):
Likewise.
* misc/makedev.c (OUT_OF_LINE_IMPL_TEMPL): Use __gnu_dev_ prefix.
(gnu_dev_major): Use weak_alias and libc_hidden_weak.
(gnu_dev_minor): Likewise.
(gnu_dev_makedev): Likewise.
* csu/check_fds.c (check_one_fd): Use __gnu_dev_makedev instead of
makedev.
* posix/wordexp.c (exec_comm_child): Likewise.
* sysdeps/mach/hurd/xmknodat.c (__xmknodat): Use __gnu_dev_minor
instead of minor and __gnu_dev_major instead of major.
* sysdeps/unix/sysv/linux/device-nrs.h (DEV_TTY_P): Use
__gnu_dev_major instead of major.
* sysdeps/unix/sysv/linux/pathconf.c (distinguish_extX): Use
__gnu_dev_major instead of gnu_dev_major and __gnu_dev_minor
instead of gnu_dev_minor.
* sysdeps/unix/sysv/linux/ptsname.c (MASTER_P): Likewise.
(SLAVE_P): Likewise.
(__ptsname_internal): Use __gnu_dev_minor instead of minor.
* sysdeps/unix/sysv/linux/ttyname.h (is_pty): Use __gnu_dev_major
instead of major.
Comments
Ping. This patch
<https://sourceware.org/ml/libc-alpha/2018-02/msg00076.html> is pending
review.
On 02/02/2018 06:43 PM, Joseph Myers wrote:
> Building with -Os produces linknamespace and localplt failures for,
> among other functions, gnu_dev_major, gnu_dev_minor and
> gnu_dev_makedev.
>
> The issue is that those functions are not inlined when building with
> -Os. While one could force them to be inlined in that case, it seems
> more natural to fix this issue similarly to other namespace issues.
> Thus, this patch makes gnu_dev_* into weak aliases for hidden symbols
> __gnu_dev_*; __gnu_dev_* are then defined as inlines in the internal
> include/sys/sysmacros.h, and uses of gnu_dev_* (often via the macros
> major, minor and makedev) for which there are namespace issues are
> changed to use __gnu_dev_*; where there are no namespace issues, use
> of libc_hidden_proto serves to avoid unnecessary local PLT entry use.
Patch looks reasonable. Thanks.
Florian
This isn't directly related, but what do you think of completing the
removal of sys/sysmacros.h from sys/types.h in this release cycle?
On 02/07/2018 04:15 PM, Zack Weinberg wrote:
> This isn't directly related, but what do you think of completing the
> removal of sys/sysmacros.h from sys/types.h in this release cycle?
I'm in favor.
Thanks,
Florian
On Wed, 7 Feb 2018, Zack Weinberg wrote:
> This isn't directly related, but what do you think of completing the
> removal of sys/sysmacros.h from sys/types.h in this release cycle?
I support that.
@@ -50,12 +50,12 @@ check_one_fd (int fd, int mode)
if ((mode & O_ACCMODE) == O_WRONLY)
{
name = _PATH_DEV "full";
- dev = makedev (DEV_FULL_MAJOR, DEV_FULL_MINOR);
+ dev = __gnu_dev_makedev (DEV_FULL_MAJOR, DEV_FULL_MINOR);
}
else
{
name = _PATH_DEVNULL;
- dev = makedev (DEV_NULL_MAJOR, DEV_NULL_MINOR);
+ dev = __gnu_dev_makedev (DEV_NULL_MAJOR, DEV_NULL_MINOR);
}
/* Something is wrong with this descriptor, it's probably not
@@ -1 +1,52 @@
+/* Definitions of macros to access 'dev_t' values. Internal header.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _ISOMAC
+# define __SYSMACROS_NEED_IMPLEMENTATION
+#endif
+
#include <misc/sys/sysmacros.h>
+
+#if !defined _SYS_SYSMACROS_H_WRAPPER && !defined _ISOMAC
+# define _SYS_SYSMACROS_H_WRAPPER 1
+
+libc_hidden_proto (gnu_dev_major)
+libc_hidden_proto (gnu_dev_minor)
+libc_hidden_proto (gnu_dev_makedev)
+
+# undef __SYSMACROS_DECL_TEMPL
+# define __SYSMACROS_DECL_TEMPL(rtype, name, proto) \
+ extern rtype __gnu_dev_##name proto \
+ __THROW __attribute_const__ attribute_hidden;
+
+# undef __SYSMACROS_IMPL_TEMPL
+# define __SYSMACROS_IMPL_TEMPL(rtype, name, proto) \
+ __extension__ __extern_inline __attribute_const__ rtype \
+ __NTH (__gnu_dev_##name proto)
+
+__SYSMACROS_DECLARE_MAJOR (__SYSMACROS_DECL_TEMPL)
+__SYSMACROS_DECLARE_MINOR (__SYSMACROS_DECL_TEMPL)
+__SYSMACROS_DECLARE_MAKEDEV (__SYSMACROS_DECL_TEMPL)
+
+# ifdef __USE_EXTERN_INLINES
+__SYSMACROS_DEFINE_MAJOR (__SYSMACROS_IMPL_TEMPL)
+__SYSMACROS_DEFINE_MINOR (__SYSMACROS_IMPL_TEMPL)
+__SYSMACROS_DEFINE_MAKEDEV (__SYSMACROS_IMPL_TEMPL)
+# endif
+
+#endif
@@ -23,8 +23,14 @@
#include <sys/sysmacros.h>
#define OUT_OF_LINE_IMPL_TEMPL(rtype, name, proto) \
- rtype gnu_dev_##name proto
+ rtype __gnu_dev_##name proto
__SYSMACROS_DEFINE_MAJOR(OUT_OF_LINE_IMPL_TEMPL)
+weak_alias (__gnu_dev_major, gnu_dev_major)
+libc_hidden_weak (gnu_dev_major)
__SYSMACROS_DEFINE_MINOR(OUT_OF_LINE_IMPL_TEMPL)
+weak_alias (__gnu_dev_minor, gnu_dev_minor)
+libc_hidden_weak (gnu_dev_minor)
__SYSMACROS_DEFINE_MAKEDEV(OUT_OF_LINE_IMPL_TEMPL)
+weak_alias (__gnu_dev_makedev, gnu_dev_makedev)
+libc_hidden_weak (gnu_dev_makedev)
@@ -837,7 +837,7 @@ exec_comm_child (char *comm, int *fildes, int showerr, int noexec)
if (__builtin_expect (__fxstat64 (_STAT_VER, STDERR_FILENO, &st), 0) != 0
|| __builtin_expect (S_ISCHR (st.st_mode), 1) == 0
#if defined DEV_NULL_MAJOR && defined DEV_NULL_MINOR
- || st.st_rdev != makedev (DEV_NULL_MAJOR, DEV_NULL_MINOR)
+ || st.st_rdev != __gnu_dev_makedev (DEV_NULL_MAJOR, DEV_NULL_MINOR)
#endif
)
/* It's not the /dev/null device. Stop right here. The
@@ -81,9 +81,9 @@ __xmknodat (int vers, int fd, const char *path, mode_t mode, dev_t *dev)
bp = buf + sizeof (buf);
*--bp = '\0';
- bp = _itoa (minor (*dev), bp, 10, 0);
+ bp = _itoa (__gnu_dev_minor (*dev), bp, 10, 0);
*--bp = '\0';
- bp = _itoa (major (*dev), bp, 10, 0);
+ bp = _itoa (__gnu_dev_major (*dev), bp, 10, 0);
memcpy (bp - len, translator, len);
translator = bp - len;
len = buf + sizeof (buf) - translator;
@@ -39,7 +39,7 @@
/* Test whether given device is a tty. */
#define DEV_TTY_P(statp) \
- ({ int __dev_major = major ((statp)->st_rdev); \
+ ({ int __dev_major = __gnu_dev_major ((statp)->st_rdev); \
__dev_major >= DEV_TTY_LOW_MAJOR && __dev_major <= DEV_TTY_HIGH_MAJOR; })
#endif /* device-nrs.h */
@@ -73,7 +73,7 @@ distinguish_extX (const struct statfs *fsbuf, const char *file, int fd)
return EXT2_LINK_MAX;
__snprintf (buf, sizeof (buf), "/sys/dev/block/%u:%u",
- gnu_dev_major (st.st_dev), gnu_dev_minor (st.st_dev));
+ __gnu_dev_major (st.st_dev), __gnu_dev_minor (st.st_dev));
ssize_t n = __readlink (buf, path, sizeof (path));
if (n != -1 && n < sizeof (path))
@@ -29,16 +29,18 @@
#include <_itoa.h>
/* Check if DEV corresponds to a master pseudo terminal device. */
-#define MASTER_P(Dev) \
- (major ((Dev)) == 2 \
- || (major ((Dev)) == 4 && minor ((Dev)) >= 128 && minor ((Dev)) < 192) \
- || (major ((Dev)) >= 128 && major ((Dev)) < 136))
+#define MASTER_P(Dev) \
+ (__gnu_dev_major ((Dev)) == 2 \
+ || (__gnu_dev_major ((Dev)) == 4 \
+ && __gnu_dev_minor ((Dev)) >= 128 && __gnu_dev_minor ((Dev)) < 192) \
+ || (__gnu_dev_major ((Dev)) >= 128 && __gnu_dev_major ((Dev)) < 136))
/* Check if DEV corresponds to a slave pseudo terminal device. */
-#define SLAVE_P(Dev) \
- (major ((Dev)) == 3 \
- || (major ((Dev)) == 4 && minor ((Dev)) >= 192 && minor ((Dev)) < 256) \
- || (major ((Dev)) >= 136 && major ((Dev)) < 144))
+#define SLAVE_P(Dev) \
+ (__gnu_dev_major ((Dev)) == 3 \
+ || (__gnu_dev_major ((Dev)) == 4 \
+ && __gnu_dev_minor ((Dev)) >= 192 && __gnu_dev_minor ((Dev)) < 256) \
+ || (__gnu_dev_major ((Dev)) >= 136 && __gnu_dev_major ((Dev)) < 144))
/* Note that major number 4 corresponds to the old BSD style pseudo
terminal devices. As of Linux 2.1.115 these are no longer
@@ -122,7 +124,7 @@ __ptsname_internal (int fd, char *buf, size_t buflen, struct stat64 *stp)
return ENOTTY;
}
- ptyno = minor (stp->st_rdev);
+ ptyno = __gnu_dev_minor (stp->st_rdev);
if (ptyno / 16 >= strlen (__libc_ptyname1))
{
@@ -28,7 +28,7 @@ static inline bool
is_pty (struct stat64 *sb)
{
#ifdef _STATBUF_ST_RDEV
- int m = major (sb->st_rdev);
+ int m = __gnu_dev_major (sb->st_rdev);
return (136 <= m && m <= 143);
#else
return false;