From patchwork Sat Jan 19 16:46:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Zack Weinberg X-Patchwork-Id: 31128 Received: (qmail 53956 invoked by alias); 19 Jan 2019 16:46:46 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 53853 invoked by uid 89); 19 Jan 2019 16:46:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_PASS autolearn=ham version=3.3.2 spammy=deeply, junk, claiming, functional X-HELO: mailbackend.panix.com From: Zack Weinberg To: libc-alpha@sourceware.org Cc: Carlos O'Donell , Florian Weimer , DJ Delorie , Arjun Shankar Subject: [PATCH] Use a proper C tokenizer to implement the obsolete typedefs test. Date: Sat, 19 Jan 2019 11:46:27 -0500 Message-Id: <20190119164627.14259-1-zackw@panix.com> MIME-Version: 1.0 The test for obsolete typedefs in installed headers was implemented using “grep” and could therefore get false positives on e.g. “ulong” in a comment. It was also scanning all of the headers included by our headers, and therefore testing headers we don’t control, e.g. Linux kernel headers. This patch splits the obsolete-typedef test from scripts/check-installed-headers.sh to a separate program, scripts/check-obsolete-constructs.py. Being implemented in Python, it is feasible to make it tokenize C accurately enough to avoid false positives on the contents of comments and strings. It also only examines $(headers) in each subdirectory--all the headers we install, but not any external dependencies of those headers. It is also feasible to make the new test understand the difference between _defining_ the obsolete typedefs and _using_ the obsolete typedefs, which means posix/{bits,sys}/types.h no longer need to be exempted. This uncovered an actual bug in bits/types.h: __quad_t and __u_quad_t were being used to define __S64_TYPE, __U64_TYPE, __SQUAD_TYPE and __UQUAD_TYPE. These are changed to __int64_t and __uint64_t respectively. (__SQUAD_TYPE and __UQUAD_TYPE should be considered obsolete as well, but that is more of a change than I feel is safe during the freeze. Note that the comments in bits/types.h claiming a difference between *QUAD_TYPE and *64_TYPE were also in error: supposedly QUAD_TYPE is ‘long long’ in all ABIs whereas 64_TYPE is ‘long’ in LP64 ABIs, but that appears never to have been true; both are defined as ‘long’ in LP64 ABIs. I made a minimal change to make the comments not completely wrong and will revisit this whole area for the next release.) The change to sys/types.h removes a construct that was too complicated for the new script (which lexes C but does not attempt to parse it) to understand. It should have absolutely no functional effect. We might want to consider limiting sys/types.h’s definition of intN_t and u_intN_t to __USE_MISC, and we might also want to consider adding register_t to the set of obsolete typedefs, but those changes are much too risky during the freeze (even within our own headers there are places where we assume sys/types.h defines intN_t unconditionally). * scripts/check-obsolete-constructs.py: New test script. * scripts/check-installed-headers.sh: Don’t test for obsolete typedefs. * Rules: Run scripts/check-obsolete-constructs.py over $(headers) as a special test. Update commentary. * bits/types.h (__SQUAD_TYPE, __S64_TYPE): Define as __int64_t. (__UQUAD_TYPE, __U64_TYPE): Define as __uint64_t. Update commentary. * sys/types.h (__u_intN_t): Remove. (u_int8_t): Typedef using __uint8_t. (u_int16_t): Typedef using __uint16_t. (u_int32_t): Typedef using __uint32_t. (u_int64_t): Typedef using __uint64_t. --- Rules | 15 +- posix/bits/types.h | 10 +- posix/sys/types.h | 33 +--- scripts/check-installed-headers.sh | 37 +---- scripts/check-obsolete-constructs.py | 225 +++++++++++++++++++++++++++ 5 files changed, 257 insertions(+), 63 deletions(-) create mode 100755 scripts/check-obsolete-constructs.py diff --git a/Rules b/Rules index 1562f2ce6d..26d3f0dca0 100644 --- a/Rules +++ b/Rules @@ -82,7 +82,8 @@ $(common-objpfx)dummy.c: common-generated += dummy.o dummy.c ifneq "$(headers)" "" -# Special test of all the installed headers in this directory. +# Test that all of the headers installed by this directory can be compiled +# in isolation. tests-special += $(objpfx)check-installed-headers-c.out libof-check-installed-headers-c := testsuite $(objpfx)check-installed-headers-c.out: \ @@ -93,6 +94,8 @@ $(objpfx)check-installed-headers-c.out: \ $(evaluate-test) ifneq "$(CXX)" "" +# If a C++ compiler is available, also test that they can be compiled +# in isolation as C++. tests-special += $(objpfx)check-installed-headers-cxx.out libof-check-installed-headers-cxx := testsuite $(objpfx)check-installed-headers-cxx.out: \ @@ -102,6 +105,16 @@ $(objpfx)check-installed-headers-cxx.out: \ $(headers) > $@; \ $(evaluate-test) endif + +# Test that none of the headers installed by this directory use certain +# obsolete constructs (e.g. legacy BSD typedefs superseded by stdint.h). +# This script does not need $(py-env). +tests-special += $(objpfx)check-obsolete-constructs.out +libof-check-obsolete-constructs := testsuite +$(objpfx)check-obsolete-constructs.out: \ + $(..)scripts/check-obsolete-constructs.py $(headers) + $(PYTHON) $^ > $@ 2>&1; \ + $(evaluate-test) endif # This makes all the auxiliary and test programs. diff --git a/posix/bits/types.h b/posix/bits/types.h index 27e065c3be..0de6c59bb4 100644 --- a/posix/bits/types.h +++ b/posix/bits/types.h @@ -87,7 +87,7 @@ __extension__ typedef unsigned long long int __uintmax_t; 32 -- "natural" 32-bit type (always int) 64 -- "natural" 64-bit type (long or long long) LONG32 -- 32-bit type, traditionally long - QUAD -- 64-bit type, always long long + QUAD -- 64-bit type, traditionally long long WORD -- natural type of __WORDSIZE bits (int or long) LONGWORD -- type of __WORDSIZE bits, traditionally long @@ -113,14 +113,14 @@ __extension__ typedef unsigned long long int __uintmax_t; #define __SLONGWORD_TYPE long int #define __ULONGWORD_TYPE unsigned long int #if __WORDSIZE == 32 -# define __SQUAD_TYPE __quad_t -# define __UQUAD_TYPE __u_quad_t +# define __SQUAD_TYPE __int64_t +# define __UQUAD_TYPE __uint64_t # define __SWORD_TYPE int # define __UWORD_TYPE unsigned int # define __SLONG32_TYPE long int # define __ULONG32_TYPE unsigned long int -# define __S64_TYPE __quad_t -# define __U64_TYPE __u_quad_t +# define __S64_TYPE __int64_t +# define __U64_TYPE __uint64_t /* We want __extension__ before typedef's that use nonstandard base types such as `long long' in C89 mode. */ # define __STD_TYPE __extension__ typedef diff --git a/posix/sys/types.h b/posix/sys/types.h index 27129c5c23..0e37b1ce6a 100644 --- a/posix/sys/types.h +++ b/posix/sys/types.h @@ -154,37 +154,20 @@ typedef unsigned int uint; #include -#if !__GNUC_PREREQ (2, 7) - /* These were defined by ISO C without the first `_'. */ -typedef unsigned char u_int8_t; -typedef unsigned short int u_int16_t; -typedef unsigned int u_int32_t; -# if __WORDSIZE == 64 -typedef unsigned long int u_int64_t; -# else -__extension__ typedef unsigned long long int u_int64_t; -# endif - -typedef int register_t; - -#else - -/* For GCC 2.7 and later, we can use specific type-size attributes. */ -# define __u_intN_t(N, MODE) \ - typedef unsigned int u_int##N##_t __attribute__ ((__mode__ (MODE))) - -__u_intN_t (8, __QI__); -__u_intN_t (16, __HI__); -__u_intN_t (32, __SI__); -__u_intN_t (64, __DI__); +typedef __uint8_t u_int8_t; +typedef __uint16_t u_int16_t; +typedef __uint32_t u_int32_t; +typedef __uint64_t u_int64_t; +#if __GNUC_PREREQ (2, 7) typedef int register_t __attribute__ ((__mode__ (__word__))); - +#else +typedef int register_t; +#endif /* Some code from BIND tests this macro to see if the types above are defined. */ -#endif #define __BIT_TYPES_DEFINED__ 1 diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh index 8e7beffd82..63bc8d4fa6 100644 --- a/scripts/check-installed-headers.sh +++ b/scripts/check-installed-headers.sh @@ -16,11 +16,9 @@ # License along with the GNU C Library; if not, see # . -# Check installed headers for cleanliness. For each header, confirm -# that it's possible to compile a file that includes that header and -# does nothing else, in several different compilation modes. Also, -# scan the header for a set of obsolete typedefs that should no longer -# appear. +# For each installed header, confirm that it's possible to compile a +# file that includes that header and does nothing else, in several +# different compilation modes. # These compilation switches assume GCC or compatible, which is probably # fine since we also assume that when _building_ glibc. @@ -31,13 +29,6 @@ cxx_modes="-std=c++98 -std=gnu++98 -std=c++11 -std=gnu++11" # These are probably the most commonly used three. lib_modes="-D_DEFAULT_SOURCE=1 -D_GNU_SOURCE=1 -D_XOPEN_SOURCE=700" -# sys/types.h+bits/types.h have to define the obsolete types. -# rpc(svc)/* have the obsolete types too deeply embedded in their API -# to remove. -skip_obsolete_type_check='*/sys/types.h|*/bits/types.h|*/rpc/*|*/rpcsvc/*' -obsolete_type_re=\ -'\<((__)?(quad_t|u(short|int|long|_(char|short|int([0-9]+_t)?|long|quad_t))))\>' - if [ $# -lt 3 ]; then echo "usage: $0 c|c++ \"compile command\" header header header..." >&2 exit 2 @@ -46,14 +37,10 @@ case "$1" in (c) lang_modes="$c_modes" cih_test_c=$(mktemp ${TMPDIR-/tmp}/cih_test_XXXXXX.c) - already="$skip_obsolete_type_check" ;; (c++) lang_modes="$cxx_modes" cih_test_c=$(mktemp ${TMPDIR-/tmp}/cih_test_XXXXXX.cc) - # The obsolete-type check can be skipped for C++; it is - # sufficient to do it for C. - already="*" ;; (*) echo "usage: $0 c|c++ \"compile command\" header header header..." >&2 @@ -151,22 +138,8 @@ $expanded_lib_mode int avoid_empty_translation_unit; EOF if $cc_cmd -fsyntax-only $lang_mode "$cih_test_c" 2>&1 - then - includes=$($cc_cmd -fsyntax-only -H $lang_mode \ - "$cih_test_c" 2>&1 | sed -ne 's/^[.][.]* //p') - for h in $includes; do - # Don't repeat work. - eval 'case "$h" in ('"$already"') continue;; esac' - - if grep -qE "$obsolete_type_re" "$h"; then - echo "*** Obsolete types detected:" - grep -HE "$obsolete_type_re" "$h" - failed=1 - fi - already="$already|$h" - done - else - failed=1 + then : + else failed=1 fi done done diff --git a/scripts/check-obsolete-constructs.py b/scripts/check-obsolete-constructs.py new file mode 100755 index 0000000000..a0af229dc8 --- /dev/null +++ b/scripts/check-obsolete-constructs.py @@ -0,0 +1,225 @@ +#! /usr/bin/python3 +# Copyright (C) 2019 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 +# . + +"""Verifies that installed headers do not use any obsolete constructs: + * legacy BSD typedefs superseded by : + ushort uint ulong u_short u_int u_long u_intNN_t quad_t u_quad_t + (sys/types.h is allowed to _define_ these types, but not to use them + to define anything else). +""" + +import argparse +import collections +import os +import re +import sys + +# Simplified lexical analyzer for C preprocessing tokens. +# Does not implement trigraphs. +# Does not implement backslash-newline in the middle of any lexical +# item other than a string literal. +# Does not implement universal-character-names in identifiers. +# Does not implement header-names. +# Treats prefixed strings (e.g. L"...") as two tokens (L and "...") +# Accepts non-ASCII characters only within comments and strings. + +# Caution: The order of the outermost alternation matters. +# STRING must be before BAD_STRING, CHARCONST before BAD_CHARCONST, +# BLOCK_COMMENT before BAD_BLOCK_COM before PUNCTUATOR, and OTHER must +# be last. +# Caution: There should be no capturing groups other than the named +# captures in the outermost alternation. + +# For reference, these are all of the C punctuators as of C11: +# [ ] ( ) { } , ; ? ~ +# ! != * *= / /= ^ ^= = == +# # ## +# % %= %> %: %:%: +# & &= && +# | |= || +# + += ++ +# - -= -- -> +# . ... +# : :> +# < <% <: << <<= <= +# > >= >> >>= + +# The BAD_* tokens are not part of the official definition of pp-tokens; +# they match unclosed strings, character constants, and block comments, +# so that the regex engine doesn't have to backtrack all the way to the +# beginning of a broken construct and then emit dozens of junk tokens. + +PP_TOKEN_RE_ = re.compile(r''' + (?P \"(?:[^\"\\\r\n]|\\(?:[\r\n -~]|\r\n))*\") + |(?P \"(?:[^\"\\\r\n]|\\[ -~])*) + |(?P \'(?:[^\'\\\r\n]|\\(?:[\r\n -~]|\r\n))*\') + |(?P \'(?:[^\'\\\r\n]|\\[ -~])*) + |(?P /\*(?:\*(?!/)|[^*])*\*/) + |(?P /\*(?:\*(?!/)|[^*])*\*?) + |(?P //[^\r\n]*) + |(?P [_a-zA-Z][_a-zA-Z0-9]*) + |(?P \.?[0-9](?:[0-9a-df-oq-zA-DF-OQ-Z_.]|[eEpP][+-]?)*) + |(?P + [,;?~(){}\[\]] + | [!*/^=]=? + | \#\#? + | %(?:[=>]|:(?:%:)?)? + | &[=&]? + |\|[=|]? + |\+[=+]? + | -[=->]? + |\.(?:\.\.)? + | :>? + | <(?:[%:]|<(?:=|<=?)?)? + | >(?:=|>=?)?) + |(?P \\(?=[\r\n])) + |(?P [ \t\n\r\v\f]+) + |(?P .) +''', re.DOTALL | re.VERBOSE) +ENDLINE_RE_ = re.compile(r'\r|\n|\r\n') + +# based on the sample code in the Python re documentation +Token_ = collections.namedtuple('Token', ( + 'kind', 'text', 'line', 'column')) +def tokenize_c(file_contents): + Token = Token_ + PP_TOKEN_RE = PP_TOKEN_RE_ + ENDLINE_RE = ENDLINE_RE_ + + line_num = 1 + line_start = 0 + for mo in PP_TOKEN_RE.finditer(file_contents): + kind = mo.lastgroup + text = mo.group() + line = line_num + column = mo.start() - line_start + # only these kinds can contain a newline + if kind in ('WHITESPACE', 'BLOCK_COMMENT', 'LINE_COMMENT', + 'STRING', 'CHARCONST', 'BAD_BLOCK_COM'): + adj_line_start = 0 + for tmo in ENDLINE_RE.finditer(text): + line_num += 1 + adj_line_start = tmo.end() + if adj_line_start: + line_start = mo.start() + adj_line_start + yield Token(kind, text, line, column + 1) + + +class Checker: + def __init__(self): + self.status = 0 + + def error(self, fname, tok, message): + self.status = 1 + if '{!r}' in message: + message = message.format(tok.text) + sys.stderr.write("{}:{}:{}: error: {}\n".format( + fname, tok.line, tok.column, message)) + + # The obsolete type names we're looking for: + OBSOLETE_TYPE_RE_ = re.compile(r'''\A + (__)? + (?: quad_t + | u(?: short | int | long + | _(?: char | short | int([0-9]+_t)? | long | quad_t ))) + \Z''', re.VERBOSE) + + def check_file(self, fname): + OBSOLETE_TYPE_RE = self.OBSOLETE_TYPE_RE_ + + try: + with open(fname, "rt") as fp: + contents = fp.read() + except OSError as e: + sys.stderr.write("{}: {}\n".format(fname, e.strerror)) + self.status = 1 + return + + # The obsolete rpc/ and rpcsvc/ headers are allowed to use the + # obsolete types, because it would be more trouble than it's + # worth to remove them from headers that we intend to stop + # installing eventually anyway. + uses_ok = (fname.startswith("rpc/") or + fname.startswith("rpcsvc/") or + "/rpc/" in fname or + "/rpcsvc/" in fname) + + # sys/types.h and bits/types.h are allowed to define the + # obsolete types, and they're allowed to use the __-versions + # of the obsolete types to define the unprefixed versions of + # the obsolete types. + typedefs_ok = (fname == "sys/types.h" or + fname == "bits/types.h" or + fname.endswith("/sys/types.h") or + fname.endswith("/bits/types.h")) + in_typedef = False + delayed_error_token = None + + for tok in tokenize_c(contents): + kind = tok.kind + if kind == 'BAD_STRING': + self.error(fname, tok, "unclosed string") + elif kind == 'BAD_CHARCONST': + self.error(fname, tok, "unclosed char constant") + elif kind == 'BAD_BLOCK_COM': + self.error(fname, tok, "unclosed block comment") + elif kind == 'OTHER': + self.error(fname, tok, "stray {!r} in program") + + elif kind == 'IDENT': + if tok.text == 'typedef': + in_typedef = True + continue + + mo = OBSOLETE_TYPE_RE.match(tok.text) + if mo: + if uses_ok: + continue + + if typedefs_ok and in_typedef: + # inside a typedef, the underscore versions + # are allowed unconditionally, and the + # non-underscore versions are allowed as the + # final identifier (the one being defined). + if mo.group(1) != '__': + delayed_error_token = tok + continue + + self.error(fname, tok, "use of {!r}") + + elif kind == 'PUNCTUATOR': + if in_typedef and tok.text == ';': + delayed_error_token = None + in_typedef = False + + if delayed_error_token is not None: + self.error(fname, delayed_error_token, "use of {!r}") + delayed_error_token = None + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("headers", metavar="header", nargs="+", + help="one or more headers to scan for obsolete constructs") + args = ap.parse_args() + + checker = Checker() + for fname in args.headers: + checker.check_file(fname) + sys.exit(checker.status) + +main()