[PATCH] Use a proper C tokenizer to implement the obsolete typedefs test.

[Zack Weinberg <zackw at panix dot com>]

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 <bits/stdint-intn.h>
 
-#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
 # <http://www.gnu.org/licenses/>.
 
-# 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
+# <http://www.gnu.org/licenses/>.
+
+"""Verifies that installed headers do not use any obsolete constructs:
+ * legacy BSD typedefs superseded by <stdint.h>:
+   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<STRING>        \"(?:[^\"\\\r\n]|\\(?:[\r\n -~]|\r\n))*\")
+   |(?P<BAD_STRING>    \"(?:[^\"\\\r\n]|\\[ -~])*)
+   |(?P<CHARCONST>     \'(?:[^\'\\\r\n]|\\(?:[\r\n -~]|\r\n))*\')
+   |(?P<BAD_CHARCONST> \'(?:[^\'\\\r\n]|\\[ -~])*)
+   |(?P<BLOCK_COMMENT> /\*(?:\*(?!/)|[^*])*\*/)
+   |(?P<BAD_BLOCK_COM> /\*(?:\*(?!/)|[^*])*\*?)
+   |(?P<LINE_COMMENT>  //[^\r\n]*)
+   |(?P<IDENT>         [_a-zA-Z][_a-zA-Z0-9]*)
+   |(?P<PP_NUMBER>     \.?[0-9](?:[0-9a-df-oq-zA-DF-OQ-Z_.]|[eEpP][+-]?)*)
+   |(?P<PUNCTUATOR>
+       [,;?~(){}\[\]]
+     | [!*/^=]=?
+     | \#\#?
+     | %(?:[=>]|:(?:%:)?)?
+     | &[=&]?
+     |\|[=|]?
+     |\+[=+]?
+     | -[=->]?
+     |\.(?:\.\.)?
+     | :>?
+     | <(?:[%:]|<(?:=|<=?)?)?
+     | >(?:=|>=?)?)
+   |(?P<ESCNL>         \\(?=[\r\n]))
+   |(?P<WHITESPACE>    [ \t\n\r\v\f]+)
+   |(?P<OTHER>         .)
+''', 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()
-- 
2.20.1