This patch adds a new dg-lint subdirectory below contrib, containing a "dg-lint" script for detecting common mistakes made in our DejaGnu tests.
Specifically, DejaGnu's dg.exp's dg-get-options has a regexp for detecting dg- directives https://git.savannah.gnu.org/gitweb/?p=dejagnu.git;a=blob;f=lib/dg.exp here's the current: set tmp [grep $prog "{\[ \t\]\+dg-\[-a-z\]\+\[ \t\]\+.*\[ \t\]\+}" line] which if I'm reading it right requires a "{", then one or more tab/space chars, then a "dg-" directive name, then one of more tab/space characters, then anything (for arguments to the directive), then one of more tab/space character, then a "}". There are numerous places in our testsuite which look like attempts to use a directive, but which don't match this regexp. The script warns about such places, along with a list of misspelled directives (currently just "dg_options" for "dg-options"), and a warning if a dg-do appears after a dg-require-* (as per https://gcc.gnu.org/onlinedocs/gccint/Directives.html "This directive must appear after any dg-do directive in the test and before any dg-additional-sources directive." for dg-require-effective-target. dg-lint uses libgdiagnostics to report its results; the patch adds a new libgdiagnostics.py script below contrib/dg-lint. This uses Python's ctypes module to expose libgdianostics.so to Python via FFI. Hence the warnings have colorization, quote the pertinent parts of the tested file, can have fix-it hints, etc. Here's the output from the tests, run from the top-level directory: $ LD_LIBRARY_PATH=../build/gcc/ ./contrib/dg-lint/dg-lint contrib/dg-lint/test-*.c contrib/dg-lint/test-1.c:6:6: warning: misspelled directive: 'dg_final'; did you mean 'dg-final'? 6 | /* { dg_final { scan_assembler_times "vmsumudm" 2 } } */ | ^~~~~~~~ | dg-final contrib/dg-lint/test-1.c:15:4: warning: directive 'dg-output-file' appears not to match dg.exp's regexp 15 | dg-output-file "m4.out" | ^~~~~~~~~~~~~~ contrib/dg-lint/test-1.c:18:4: warning: directive 'dg-output-file' appears not to match dg.exp's regexp 18 | dg-output-file "m4.out" } | ^~~~~~~~~~~~~~ contrib/dg-lint/test-1.c:21:6: warning: directive 'dg-output-file' appears not to match dg.exp's regexp 21 | { dg-output-file "m4.out" | ^~~~~~~~~~~~~~ contrib/dg-lint/test-1.c:24:5: warning: directive 'dg-output-file' appears not to match dg.exp's regexp 24 | {dg-output-file "m4.out"} | ^~~~~~~~~~~~~~ contrib/dg-lint/test-1.c:27:6: warning: directive 'dg-output-file' appears not to match dg.exp's regexp 27 | { dg-output-file, "m4.out" } | ^~~~~~~~~~~~~~ contrib/dg-lint/test-2.c:4:6: warning: 'dg-do' after 'dg-require-effective-target' 4 | /* { dg-do compile } */ | ^~~~~ contrib/dg-lint/test-2.c:3:6: note: 'dg-require-effective-target' was here 3 | /* { dg-require-effective-target c++11 } */ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ I don't yet have a way to verify these tests (clearly we can't use DejaGnu for this). These Python bindings could be used by other projects, but so far I only implemented what I needed for dg-lint. Running the test on the GCC source tree finds dozens of issues, which followup patches address. Tested with Python 3.8 contrib/ChangeLog: PR testsuite/116163 * dg-lint/dg-lint: New file. * dg-lint/libgdiagnostics.py: New file. * dg-lint/test-1.c: New file. * dg-lint/test-2.c: New file. --- contrib/dg-lint/dg-lint | 210 ++++++++++++++++++++++++ contrib/dg-lint/libgdiagnostics.py | 248 +++++++++++++++++++++++++++++ contrib/dg-lint/test-1.c | 28 ++++ contrib/dg-lint/test-2.c | 8 + 4 files changed, 494 insertions(+) create mode 100755 contrib/dg-lint/dg-lint create mode 100644 contrib/dg-lint/libgdiagnostics.py create mode 100644 contrib/dg-lint/test-1.c create mode 100644 contrib/dg-lint/test-2.c diff --git a/contrib/dg-lint/dg-lint b/contrib/dg-lint/dg-lint new file mode 100755 index 000000000000..20157c304137 --- /dev/null +++ b/contrib/dg-lint/dg-lint @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 + +# Script to detect common mistakes in DejaGnu tests. + +# Contributed by David Malcolm <dmalc...@redhat.com> +# +# Copyright (C) 2024-2025 Free Software Foundation, Inc. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3, or (at your option) +# any later version. +# +# GCC 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING. If not, write to +# the Free Software Foundation, 51 Franklin Street, Fifth Floor, +# Boston, MA 02110-1301, USA. + +import argparse +import os +import pathlib +import re +import sys + +import libgdiagnostics + +COMMON_MISSPELLINGS = {('dg_final', 'dg-final')} + +class Directive: + @staticmethod + def from_match(path, line_num, line, m): + return Directive(path, line_num, + m.group(1), m.span(1), + m.group(2), m.span(2)) + + def __init__(self, path, line_num, + name, name_span, + args_str, args_str_span): + self.path = path + self.line_num = line_num + self.name = name + self.name_span = name_span + self.args_str = args_str + self.args_str_span = args_str_span + +class FileState: + def __init__(self): + # Map from directive name to list of directives + self.directives = {} + + def add_directive(self, d): + if d.name not in self.directives: + self.directives[d.name] = [] + self.directives[d.name].append(d) + +class Context: + def __init__(self): + self.mgr = libgdiagnostics.Manager() + self.mgr.set_tool_name('dg-lint') + self.mgr.add_text_sink() + + def check_file(self, f_in): + file_state = FileState() + try: + for line_idx, line in enumerate(f_in): + self.check_line(file_state, f_in.name, line_idx + 1, line) + except UnicodeDecodeError: + return # FIXME + + def check_line(self, file_state, path, line_num, line): + if 0: + phys_loc = self.get_file_and_line(path, line_num) + self.report_warning(phys_loc, "line = '%s'", line.rstrip()) + + # Look for directives + # Compare with dg.exp's dg-get-options + m = re.search(r"{[ \t]+(dg-[-a-z]+)[ \t]+(.*)[ \t]+}", line) + if m: + d = Directive.from_match(path, line_num, line, m) + self.on_directive(file_state, d) + else: + # Look for things that look like attempts to use directives, + # but didn't match the regexp + m = re.search(r"(dg-[-a-z]+)", line) + if m: + phys_loc = self.get_file_line_and_range(path, + line_num, + m.start() + 1, + m.end()) + self.make_warning() \ + .at(phys_loc) \ + .emit('directive %qs appears not to match dg.exp\'s regexp', + m.group(1)) + + # Complain about common misspellings + for misspelling, correction in COMMON_MISSPELLINGS: + for m in re.finditer(misspelling, line): + phys_loc = self.get_file_line_and_range(path, + line_num, + m.start() + 1, + m.end()) + self.make_warning() \ + .at(phys_loc) \ + .with_fix_it_replace (phys_loc, correction) \ + .emit("misspelled directive: %qs;" + " did you mean %qs?", + misspelling, correction) + + def on_directive(self, file_state, d): + if 0: + phys_loc = self.get_file_and_line(d.path, d.line_num) + self.make_note() \ + .at(phys_loc) \ + .emit('directive %qs with arguments %qs', + d.name, d.args_str) + + # Apparently we can't have a dg-do after a dg-require; + # see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116163#c5 + if d.name == 'dg-do': + for existing_name in file_state.directives: + if existing_name.startswith('dg-require'): + with self.mgr.make_group() as g: + phys_loc = self.get_file_line_and_range(d.path, + d.line_num, + d.name_span[0] + 1, + d.name_span[1]) + self.make_warning() \ + .at(phys_loc) \ + .emit("%qs after %qs", d.name, existing_name) + other_d = file_state.directives[existing_name][0] + phys_loc = self.get_file_line_and_range(other_d.path, + other_d.line_num, + other_d.name_span[0] + 1, + other_d.name_span[1]) + self.make_note() \ + .at(phys_loc) \ + .emit("%qs was here", existing_name) + + file_state.add_directive(d) + + def get_file_and_line(self, + path: str, + line_num: int): + """ + Get a libgdiagnostics.c_diagnostic_physical_location_ptr for the given line. + """ + c_diag_file = self.mgr.get_file(path) + return self.mgr.get_file_and_line(c_diag_file, line_num) + + def get_file_line_and_range(self, + path: str, + line_num: int, + start_column: int, + end_column: int): + """ + Get a libgdiagnostics.c_diagnostic_physical_location_ptr for the range + of columns on the given line. + """ + c_diag_file = self.mgr.get_file(path) + start_loc = self.mgr.get_file_line_and_column(c_diag_file, line_num, start_column) + end_loc = self.mgr.get_file_line_and_column(c_diag_file, line_num, end_column) + return self.mgr.get_range(start_loc, start_loc, end_loc) + + def report_warning(self, phys_loc, msg, *args): + diag = libgdiagnostics.Diagnostic(self.mgr, + libgdiagnostics.DIAGNOSTIC_LEVEL_WARNING) + diag.set_location (phys_loc) + diag.finish(msg, *args) + + def make_warning(self): + return libgdiagnostics.Diagnostic(self.mgr, + libgdiagnostics.DIAGNOSTIC_LEVEL_WARNING) + + def make_note(self): + return libgdiagnostics.Diagnostic(self.mgr, + libgdiagnostics.DIAGNOSTIC_LEVEL_NOTE) + +def main(argv): + parser = argparse.ArgumentParser()#usage=__doc__) + parser.add_argument('paths', nargs='+', type=pathlib.Path) + opts = parser.parse_args(argv[1:]) + + ctxt = Context() + for path in opts.paths: + if path.is_dir(): + for dirpath, dirnames, filenames in os.walk(path): + for f in filenames: + if 'ChangeLog' in f: + continue + if f.endswith('.exp'): + continue + p = os.path.join(dirpath, f) + with open(p) as f: + ctxt.check_file(f) + else: + with open(path) as f: + ctxt.check_file(f) + +# TODO: how to unit test? + +if __name__ == '__main__': + retval = main(sys.argv) + sys.exit(retval) diff --git a/contrib/dg-lint/libgdiagnostics.py b/contrib/dg-lint/libgdiagnostics.py new file mode 100644 index 000000000000..59fe0ea6c8c4 --- /dev/null +++ b/contrib/dg-lint/libgdiagnostics.py @@ -0,0 +1,248 @@ +#!/usr/bin/env python3 + +# Python bindings for libgdiagnostics, using ctypes + +# Contributed by David Malcolm <dmalc...@redhat.com> +# +# Copyright (C) 2025 Free Software Foundation, Inc. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3, or (at your option) +# any later version. +# +# GCC 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING. If not, write to +# the Free Software Foundation, 51 Franklin Street, Fifth Floor, +# Boston, MA 02110-1301, USA. + +from contextlib import contextmanager +import ctypes + +# Lower-level API: use ctypes and FFI to wrap the C API directly + +cdll = ctypes.cdll.LoadLibrary('libgdiagnostics.so') + +libc = ctypes.CDLL(None) +c_stderr = ctypes.c_void_p.in_dll(libc, 'stderr') + +# Opaque structs + +class c_diagnostic_manager(ctypes.Structure): + pass +c_diagnostic_manager_ptr = ctypes.POINTER(c_diagnostic_manager) + +class c_diagnostic(ctypes.Structure): + pass +c_diagnostic_ptr = ctypes.POINTER(c_diagnostic) + +class c_diagnostic_file(ctypes.Structure): + pass +c_diagnostic_file_ptr = ctypes.POINTER(c_diagnostic_file) + +class c_diagnostic_physical_location(ctypes.Structure): + pass +c_diagnostic_physical_location_ptr = ctypes.POINTER(c_diagnostic_physical_location) + +# Enums + +DIAGNOSTIC_COLORIZE_IF_TTY = 0 + +DIAGNOSTIC_LEVEL_ERROR = 0 +DIAGNOSTIC_LEVEL_WARNING = 1 +DIAGNOSTIC_LEVEL_NOTE = 2 +DIAGNOSTIC_LEVEL_SORRY = 3 + +# Entrypoints + +cdll.diagnostic_manager_new.argtypes = [] +cdll.diagnostic_manager_new.restype = c_diagnostic_manager_ptr + +cdll.diagnostic_manager_release.argtypes = [c_diagnostic_manager_ptr] +cdll.diagnostic_manager_release.restype = None + +cdll.diagnostic_manager_set_tool_name.argtypes = [c_diagnostic_manager_ptr, + ctypes.c_char_p] +cdll.diagnostic_manager_set_tool_name.restype = None + +cdll.diagnostic_manager_begin_group.argtypes = [c_diagnostic_manager_ptr] +cdll.diagnostic_manager_begin_group.restype = None + +cdll.diagnostic_manager_end_group.argtypes = [c_diagnostic_manager_ptr] +cdll.diagnostic_manager_end_group.restype = None + +cdll.diagnostic_begin.argtypes = [c_diagnostic_manager_ptr, + ctypes.c_int] +cdll.diagnostic_begin.restype = c_diagnostic_ptr + +cdll.diagnostic_finish.argtypes = [c_diagnostic_ptr, + ctypes.c_char_p]#, ctypes.c_char_p] # FIXME: should be variadic +cdll.diagnostic_finish.restype = None + +cdll.diagnostic_manager_new_file.argtypes = [c_diagnostic_manager_ptr, + ctypes.c_char_p, + ctypes.c_char_p] +cdll.diagnostic_manager_new_file.restype = c_diagnostic_file_ptr + +cdll.diagnostic_manager_new_location_from_file_and_line.argtypes \ + = [c_diagnostic_manager_ptr, + c_diagnostic_file_ptr, + ctypes.c_int] +cdll.diagnostic_manager_new_location_from_file_and_line.restype \ + = c_diagnostic_physical_location_ptr + +cdll.diagnostic_manager_new_location_from_file_line_column.argtypes \ + = [c_diagnostic_manager_ptr, + c_diagnostic_file_ptr, + ctypes.c_int, + ctypes.c_int] +cdll.diagnostic_manager_new_location_from_file_line_column.restype \ + = c_diagnostic_physical_location_ptr + +cdll.diagnostic_manager_new_location_from_range.argtypes\ + = [c_diagnostic_manager_ptr, + c_diagnostic_physical_location_ptr, + c_diagnostic_physical_location_ptr, + c_diagnostic_physical_location_ptr] +cdll.diagnostic_manager_new_location_from_range.restype \ + = c_diagnostic_physical_location_ptr + +cdll.diagnostic_set_location.argtypes = [c_diagnostic_ptr, + c_diagnostic_physical_location_ptr] +cdll.diagnostic_set_location.restype = None + +cdll.diagnostic_add_fix_it_hint_replace.argtypes \ + = [c_diagnostic_ptr, + c_diagnostic_physical_location_ptr, + ctypes.c_char_p] +cdll.diagnostic_add_fix_it_hint_replace.restype = None + +# Helper functions + +def _to_utf8(s: str): + if s is None: + return None + return s.encode('utf-8') + +# Higher-level API, a more pythonic approach, with classes + +class Manager: + def __init__(self): + self.c_mgr = cdll.diagnostic_manager_new() + if 0: + print('__init__: %r' % self.c_mgr) + + def __del__(self): + if 0: + print('__del__: %r' % self.c_mgr) + self.c_mgr = cdll.diagnostic_manager_release(self.c_mgr) + + def set_tool_name(self, name: str): + assert self.c_mgr + assert str + cdll.diagnostic_manager_set_tool_name (self.c_mgr, _to_utf8(name)) + + def add_text_sink(self): + assert self.c_mgr + # FIXME: hardcode the args for now + cdll.diagnostic_manager_add_text_sink (self.c_mgr, + c_stderr, + DIAGNOSTIC_COLORIZE_IF_TTY) + + def get_file(self, path: str, sarif_lang: str = None): + assert self.c_mgr + assert path + c_file = cdll.diagnostic_manager_new_file (self.c_mgr, + _to_utf8(path), + _to_utf8(sarif_lang)) + return c_file + + def get_file_and_line(self, + c_file: c_diagnostic_file_ptr, + line_num: int): + assert self.c_mgr + assert c_file + c_phys_loc = cdll.diagnostic_manager_new_location_from_file_and_line (self.c_mgr, + c_file, + line_num) + return c_phys_loc + + def get_file_line_and_column(self, + c_file: c_diagnostic_file_ptr, + line_num: int, + column_num: int): + assert self.c_mgr + assert c_file + c_phys_loc = cdll.diagnostic_manager_new_location_from_file_line_column (self.c_mgr, + c_file, + line_num, + column_num) + return c_phys_loc + + def get_range(self, + caret: c_diagnostic_physical_location_ptr, + start: c_diagnostic_physical_location_ptr, + finish: c_diagnostic_physical_location_ptr): + assert self.c_mgr + c_phys_loc = cdll.diagnostic_manager_new_location_from_range (self.c_mgr, + caret, + start, + finish) + return c_phys_loc + + @contextmanager + def make_group(self): + assert self.c_mgr + cdll.diagnostic_manager_begin_group (self.c_mgr) + try: + yield + finally: + assert self.c_mgr + cdll.diagnostic_manager_end_group (self.c_mgr) + +class Diagnostic: + def __init__(self, mgr: Manager, level: int): + self.c_diag = cdll.diagnostic_begin (mgr.c_mgr, level) + if 0: + print('__init__: %r' % self.c_diag) + + def finish(self, fmt: str, *args): + assert self.c_diag + c_args = [] + for arg in args: + c_args.append(_to_utf8(arg)) + cdll.diagnostic_finish (self.c_diag, _to_utf8(fmt), *c_args) + self.c_diagnostic = None + + def set_location(self, + phys_loc: c_diagnostic_physical_location_ptr): + assert self.c_diag + cdll.diagnostic_set_location(self.c_diag, phys_loc) + + + # Chaining-style API + + def at(self, + phys_loc: c_diagnostic_physical_location_ptr): + self.set_location(phys_loc) + return self + + def with_fix_it_replace(self, + phys_loc: c_diagnostic_physical_location_ptr, + replacement: str): + assert self.c_diag + assert replacement + cdll.diagnostic_add_fix_it_hint_replace(self.c_diag, + phys_loc, + _to_utf8(replacement)) + return self + + def emit(self, fmt: str, *args): + self.finish(fmt, *args) diff --git a/contrib/dg-lint/test-1.c b/contrib/dg-lint/test-1.c new file mode 100644 index 000000000000..6e179e9bd1cb --- /dev/null +++ b/contrib/dg-lint/test-1.c @@ -0,0 +1,28 @@ +/* Various misspelled DejaGnu directives. */ + +/* Underscore rather than dash. + Example taken from gcc/testsuite/gcc.target/powerpc/vsx-builtin-msum.c + fixed in r15-3878-gd5864b95ce94d9. */ +/* { dg_final { scan_assembler_times "vmsumudm" 2 } } */ + + +/* Correct directive. */ +/* { dg-final { scan_assembler_times "vmsumudm" 2 } } */ + +/* Malformed uses of directives. + + Missing braces + dg-output-file "m4.out" + + Missing leading brace + dg-output-file "m4.out" } + + Missing trailing brace + { dg-output-file "m4.out" + + Missing whitespace. + {dg-output-file "m4.out"} + + Trailing comma. + { dg-output-file, "m4.out" } +*/ diff --git a/contrib/dg-lint/test-2.c b/contrib/dg-lint/test-2.c new file mode 100644 index 000000000000..147f5b02f219 --- /dev/null +++ b/contrib/dg-lint/test-2.c @@ -0,0 +1,8 @@ +/* Bad directive ordering; see + https://gcc.gnu.org/onlinedocs/gccint/Directives.html + "This directive must appear after any dg-do directive in the test and + before any dg-additional-sources directive." for + dg-require-effective-target. */ + +/* { dg-require-effective-target c++11 } */ +/* { dg-do compile } */ -- 2.26.3
