| Summary: | [mips] PAGE_SIZE defined constant on MIPS, but size is configurable | ||
|---|---|---|---|
| Product: | glibc | Reporter: | Andrew Stubbs <ams> |
| Component: | libc | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | carlos, drepper.fsp, pinskia, roland |
| Priority: | P2 | Flags: | fweimer:
security-
|
| Version: | 2.18 | ||
| Target Milestone: | --- | ||
| Host: | mips*-*-linux* | Target: | |
| Build: | Last reconfirmed: | ||
IMHO the PAGE_SIZE macro should be removed from all architectures. BFD doesn't need it, which is the only purpose of <sys/user.h>. It is a mistake to use it for anything else. What about the other macros - PAGE_SHIFT, PAGE_MASK, NBPG, UPAGES, HOST_TEXT_START_ADDR, HOST_DATA_START_ADDR, HOST_STACK_END_ADDR? Should some or all of those also be removed from <sys/user.h> for all architectures? (In reply to Andrew Stubbs from comment #0) > The sys/user.h header defines PAGE_SIZE to a constant (4096), but that > figure is not always accurate; the page size is a kernel configuration > option. ... > According to Andrew Pinski, the constant PAGE_SIZE is not appropriate on any > MIPS variant, so at least some of the above macros can be removed. Other > architectures appear to remove all of them, but it's not clear that that's > correct on MIPS. From the standards point of view if we define PAGE_SIZE we must also define PAGESIZE and in limits.h. We don't define PAGESIZE or PAGE_SIZE in limits.h, and therefore there are no compile-time constants for these values in standards compliant headers. The question of what to do with sys/user.h PAGE_SIZE define is less clear. We still define PAGE_SIZE for x86, x86_64, MIPS, MicroBlaze, s390, s390x, and Alpha. It isn't clear what user application compatibility issues will arise if we remove the PAGE_SIZE definition for MIPS. I agree that if the kernel built-in page size is larger then user applications making use of PAGE_SIZE won't work correctly. However, they will work correctly and continue to compile correctly if the kernel page size matches. The other problem is that there are several kernel headers that use PAGE_SIZE (/usr/include/linux/binfmts.h, /usr/include/linux/resource.h, /usr/include/linux/kvm.h), what happens to those if glibc stops defining PAGE_SIZE for MIPS? What of the applications that use those headers? How do you propose we tackle the issue of: (a) Compatibility with applications expecting PAGE_SIZE to be defined? and (b) Kernel headers that require PAGE_SIZE? Note: In glibc we use EXEC_PAGESIZE which must be as big as the largest kernel page size, and that's your limiting factor to increasing the page size in the kernel (otherwise mapping anything including dlopen'd libraries won't work reliably). Once we've started up sufficiently we obviously use AT_PAGESZ to determine the correct page size value and that's what is returned via sysconf (_SC_PAGESIZE). BFD's trad-core wants NBPG, UPAGES, HOST_TEXT_START_ADDR, HOST_STACK_END_ADDR, HOST_DATA_START_ADDR, HOST_STACK_START_ADDR (the latter two being optional). The other macros are only defined because the kernel header <asm/user.h> defines NBPG in terms of PAGE_SIZE for most architectures. New architectures should not use trad-core, so have no need to defined these macros at all. Judging by the thread on libc-ports https://sourceware.org/ml/libc-ports/2013-11/msg00028.html and what's written here, I think the consensus is that we can probably just apply Andrew Pinski's patch. https://sourceware.org/ml/libc-ports/2013-11/msg00035.html Based on the discussion so far I think we can conclude: * tst-limits.c should, if PAGE_SIZE is kept at all, include <sys/user.h> to get and test the definition. * If a platform has variable page size in the kernel, PAGE_SIZE, PAGE_SHIFT and PAGE_MASK should be removed. * IA64, MIPS and MicroBlaze (a) have variable page size and (b) do not use trad-core, so the full set of these macros should be removed from their headers (not just the subset removed by Andrew Pinski's patch). (IA64 doesn't define PAGE_SIZE, so isn't actually affected by this bug, but defines NBPG in terms of PAGE_SIZE.) Does someone wish to produce a patch removing all the relevant macros for MIPS, MicroBlaze and IA64, and send it to libc-ports, CC:ing the maintainers for those ports? Then, if not also fixing tst-limits and working out whether PAGE_SIZE should be removed completely, file separate issues for those or put them on the wiki todo list <https://sourceware.org/glibc/wiki/Development_Todo/Master> so they don't get lost. Andrew, as I said in https://sourceware.org/ml/libc-ports/2013-12/msg00021.html the MIPS changes from https://sourceware.org/ml/libc-ports/2013-12/msg00017.html are OK and should be committed (of course, now they won't involve ports at all, just the MIPS file in libc, the NEWS file and the toplevel ChangeLog) - though MicroBlaze and IA64 will need separate bugs and weekly patch pings until those are fixed too. This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".
The branch, master has been updated
via b1efe3bbcfd94941556ffd5b60539207023c8224 (commit)
from b93c2205ec57af3c0491cdb608b53a9b4d7ff57e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=b1efe3bbcfd94941556ffd5b60539207023c8224
commit b1efe3bbcfd94941556ffd5b60539207023c8224
Author: Matthew Fortune <matthew.fortune@imgtec.com>
Date: Mon Jan 5 13:34:41 2015 +0000
Fix MIPS variable PAGE_SIZE bug (16191)
MIPS supports a variable page size but glibc defines a constant.
This causes at least two glibc tests to fail when the page size
does not match the hard-coded size:
inet/test-ifaddrs
inet/test_ifindex
[BZ #16191]
* NEWS: Mention bug fix.
* sysdeps/unix/sysv/linux/mips/sys/user.h (PAGE_SHIFT): Remove.
(PAGE_SIZE, PAGE_MASK, NBPG, UPAGES): Likewise.
(HOST_TEXT_START_ADDR, HOST_DATA_START_ADDR): Likewise.
(HOST_STACK_END_ADDR): Likewise.
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 9 +++++++++
NEWS | 14 +++++++-------
sysdeps/unix/sysv/linux/mips/sys/user.h | 9 ---------
3 files changed, 16 insertions(+), 16 deletions(-)
The fix was checked in for 2.21. |
The sys/user.h header defines PAGE_SIZE to a constant (4096), but that figure is not always accurate; the page size is a kernel configuration option. #define PAGE_SHIFT 12 #define PAGE_SIZE (1UL << PAGE_SHIFT) #define PAGE_MASK (~(PAGE_SIZE-1)) #define NBPG PAGE_SIZE #define UPAGES 1 #define HOST_TEXT_START_ADDR (u.start_code) #define HOST_DATA_START_ADDR (u.start_data) #define HOST_STACK_END_ADDR (u.start_stack + u.u_ssize * NBPG) A knock-on effect of this bug is that sysdeps/unix/sysv/linux/ifaddrs.c uses an "optimization" when PAGE_SIZE is constant that means getifaddrs will always fail when the real page size does not match PAGE_SIZE. #ifdef PAGE_SIZE /* Help the compiler optimize out the malloc call if PAGE_SIZE is constant and smaller or equal to PTHREAD_STACK_MIN/4. */ const size_t buf_size = PAGE_SIZE; #else const size_t buf_size = __getpagesize (); #endif According to Andrew Pinski, the constant PAGE_SIZE is not appropriate on any MIPS variant, so at least some of the above macros can be removed. Other architectures appear to remove all of them, but it's not clear that that's correct on MIPS. This issue probably applied to other architectures also. Joseph Myers has mentioned MicroBlaze.