diff --git a/dune-project b/dune-project index b59caa9..b5c9ed8 100644 --- a/dune-project +++ b/dune-project @@ -10,7 +10,7 @@ (package (name libdash) (synopsis "Bindings to the dash shell's parser") - (version 0.5.0) + (version 0.5.1) (depends ("ctypes" (>= "0.21.1")) ("ctypes-foreign" (>= "0.21.1")) diff --git a/libdash/_dash.py b/libdash/_dash.py index bb15190..2261b50 100644 --- a/libdash/_dash.py +++ b/libdash/_dash.py @@ -223,8 +223,8 @@ class strpush (Structure): # struct alias *ap; /* if push was associated with an alias */ # char *string; /* remember the string since it may change */ # -# /* Remember last two characters for pungetc. */ -# int lastc[2]; +# /* Delay freeing so we can stop nested aliases. */ +# struct strpush *spfree; # # /* Number of outstanding calls to pungetc. */ # int unget; @@ -234,7 +234,7 @@ class strpush (Structure): ("prevnleft", c_int), ("ap", c_void_p), ("string", c_char_p), - ("lastc", 2 * c_int), + ("spfree", POINTER (strpush)), ("unget", c_int)]; class parsefile (Structure): @@ -245,14 +245,18 @@ class parsefile (Structure): # int linno; /* current line */ # int fd; /* file descriptor (or -1 if string) */ # int nleft; /* number of chars left in this line */ -# int lleft; /* number of chars left in this buffer */ +# int eof; /* do not read again once we hit EOF */ # char *nextc; /* next char in buffer */ # char *buf; /* input buffer */ # struct strpush *strpush; /* for pushing strings at this level */ # struct strpush basestrpush; /* so pushing one is fast */ # -# /* Remember last two characters for pungetc. */ -# int lastc[2]; +# /* Delay freeing so we can stop nested aliases. */ +# struct strpush *spfree; +# +# #ifndef SMALL +# int lleft; /* number of chars left in this buffer */ +# #endif # # /* Number of outstanding calls to pungetc. */ # int unget; @@ -261,12 +265,13 @@ class parsefile (Structure): ("linno", c_int), ("fd", c_int), ("nleft", c_int), - ("lleft", c_int), + ("eof", c_int), ("nextc", POINTER (c_char)), # NOT c_char_p! ("buf", c_char_p), ("strpush", POINTER (strpush)), ("basestrpush", strpush), - ("lastc", 2 * c_int), + ("spfree", POINTER (strpush)), + ("lleft", c_int), ("unget", c_int)]; diff --git a/libdash/parser.py b/libdash/parser.py index 4db7808..4fcd30e 100644 --- a/libdash/parser.py +++ b/libdash/parser.py @@ -16,7 +16,8 @@ def libdash_library_path(): LIBDASH_LIBRARY_PATH = os.path.join(FILE_PATH, "libdash.so") return LIBDASH_LIBRARY_PATH -EOF_NLEFT = -99; # libdash/src/input.c +# parsefile->eof bit 1: set by preadbuffer at EOF, never cleared by pungetc. +PARSEFILE_EOF = 2; class ParsingException(Exception): def __init__(self, message='ParseError'): @@ -64,7 +65,7 @@ def parse(inputPath, init=True): n_ptr_C = parsecmd_safe (libdash, False) linno_after = parsefile_var.contents.linno - 1; # libdash is 1-indexed - nleft_after = parsefile_var.contents.nleft + eof_after = parsefile_var.contents.eof if (n_ptr_C == None): # Dash.Null pass @@ -73,19 +74,18 @@ def parse(inputPath, init=True): elif (n_ptr_C == NERR): # Dash.Error raise ParsingException() else: - if (nleft_after == EOF_NLEFT): - linno_after = linno_after + 1; # The last line wasn't counted - - if (inputPath != "-"): - ## Both of these assertions check "our" assumption with respect to the final parser state - ## and are therefore not necessary if they become an issue. - assert ((linno_after == len (lines)) or (linno_after == len (lines) + 1)) - - # Last line did not have a newline - assert (len (lines [-1]) > 0 and (lines [-1][-1] != '\n')) - elif nleft_after != 0: - # we formerly asserted that `nleft_after != 0`, but this no longer holds - linno_after = linno_after + 1; # The last line wasn't counted + ## dash bumps `linno` on newline, so an unterminated last line is + ## never counted. Only adjust at EOF; otherwise every node would + ## swallow the next one's first line. + if (eof_after & PARSEFILE_EOF): + if (inputPath == "-"): + ## no lines to check against; trust the flag + linno_after = linno_after + 1 + elif (lines and (lines [-1][-1:] != '\n')): + linno_after = len (lines) + + if (inputPath != "-"): + linno_after = min (linno_after, len (lines)) n_ptr = cast (n_ptr_C, POINTER (union_node)) new_ast = of_node (n_ptr) diff --git a/pyproject.toml b/pyproject.toml index 8904bfd..3e35d97 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "libdash" -version = "0.5.0" +version = "0.5.1" authors = [ { name="Michael Greenberg", email="michael@greenberg.science" }, { name="PaSh contributors" }, diff --git a/python/Makefile b/python/Makefile index b446cce..9e5091c 100644 --- a/python/Makefile +++ b/python/Makefile @@ -1,9 +1,19 @@ -.PHONY: test clean +.PHONY: test test-structs test-roundtrip test-line-mapping clean -test: rt.py ../libdash/*.py +test: test-structs test-roundtrip test-line-mapping + +# _dash.py hand-copies dash's struct layouts; nothing else checks them. +test-structs: ../test/check_structs.py ../libdash/_dash.py + @../test/check_structs.py + +test-roundtrip: rt.py ../libdash/*.py @find ../test/tests ../test/pash_tests -type f | while read f; do ../test/round_trip.sh ./rt.py "$$f"; done | tee python.log @cat python.log | egrep '^[A-Z0-9_]+:' | cut -d ':' -f 1 | sort | uniq -c @grep ':' python.log && echo "FAILED" && exit 1 || exit 0 +# Per-node line mapping; round_trip.sh cannot see it (rt.py uses only the AST). +test-line-mapping: dump.py ../libdash/*.py + @../test/line_mapping.sh + clean: rm *.o *.so *.log diff --git a/python/dump.py b/python/dump.py new file mode 100755 index 0000000..8874ab0 --- /dev/null +++ b/python/dump.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 +"""Dump the parser nodes libdash detects in a script. + +Per node: attributed line range, source lines, printed form, and the AST (-a). + +Also audits attribution across nodes, reporting OVERLAP (range runs into the +next node), GAP (lines belong to no node), EMPTY, INVERTED, OVERRUN and +UNCOVERED. Exits nonzero on any, so it doubles as a regression test. +""" + +import argparse +import os +import pprint +import sys + +try: + import libdash +except ModuleNotFoundError: + # fall back to the working tree, so libdash/*.py edits need no reinstall + sys.path.insert (0, os.path.dirname (os.path.dirname (os.path.abspath (__file__)))) + import libdash + +sys.setrecursionlimit (9001) + + +class Problem: + def __init__ (self, kind, index, detail): + self.kind = kind + self.index = index + self.detail = detail + + def __str__ (self): + return "{}: node {}: {}".format (self.kind, self.index, self.detail) + + +def read_lines (path): + with open (path, 'r') as fp: + return fp.readlines () + + +def collect (path, init): + """Parse `path`, returning (nodes, error) where nodes is a list of + (ast, lines, linno_before, linno_after).""" + nodes = [] + + try: + for node in libdash.parse (path, init): + nodes.append (node) + except Exception as e: + return (nodes, e) + + return (nodes, None) + + +def is_inert (line): + """A blank line or a whole-line comment yields no node, so it may legitimately + fall between two nodes.""" + stripped = line.strip () + return stripped == "" or stripped.startswith ("#") + + +def audit (nodes, lines): + """Check the line ranges the parser handed back for overlaps and gaps.""" + problems = [] + prev_after = 0 + + for (i, (_ast, _text, before, after)) in enumerate (nodes): + if after < before: + problems.append (Problem ("INVERTED", i, + "range [{}, {}) runs backwards".format (before, after))) + elif after == before: + problems.append (Problem ("EMPTY", i, + "range [{}, {}) covers no lines".format (before, after))) + + if before < prev_after: + problems.append (Problem ("OVERLAP", i, + "starts at line {} but the previous node ran through line {}" + .format (before, prev_after - 1))) + elif before > prev_after: + # Blank lines and comments produce no node, so only flag a gap that + # swallowed something the parser should have accounted for. + skipped = lines [prev_after:before] + + if not all (is_inert (line) for line in skipped): + problems.append (Problem ("GAP", i, + "lines {}..{} belong to no node: {}" + .format (prev_after + 1, before, + ", ".join (repr (l) for l in skipped + if not is_inert (l))))) + + prev_after = max (prev_after, after) + + if nodes and prev_after > len (lines): + problems.append (Problem ("OVERRUN", len (nodes) - 1, + "last node ends at line {} but the file has {} lines" + .format (prev_after, len (lines)))) + + trailing = lines [prev_after:] + + if nodes and not all (is_inert (line) for line in trailing): + problems.append (Problem ("UNCOVERED", len (nodes) - 1, + "lines {}..{} after the last node belong to no node: {}" + .format (prev_after + 1, len (lines), + ", ".join (repr (l) for l in trailing + if not is_inert (l))))) + + return problems + + +def show (path, nodes, err, problems, args): + lines = read_lines (path) + + print ("=== {} ({} lines) ===".format (path, len (lines))) + + for (i, (ast, text, before, after)) in enumerate (nodes): + print ("--- node {} lines [{}, {}) {}" + .format (i, before, after, + "src lines {}..{}".format (before + 1, after) if after > before + else "no source lines")) + + if text is None: + print (" src | ") + else: + for (n, line) in enumerate (text.splitlines ()): + print (" src | {:>4} | {}".format (before + n + 1, line)) + + for line in libdash.to_string (ast).splitlines (): + print (" print | {}".format (line)) + + if args.ast: + for line in pprint.pformat (ast, width = 100).splitlines (): + print (" ast | {}".format (line)) + + if err is not None: + print ("!!! {}: {}".format (type (err).__name__, err)) + + for p in problems: + print ("!!! {}".format (p)) + + print ("--- {} node(s), {} problem(s)".format (len (nodes), len (problems))) + print () + + +def show_ranges (path, nodes, err): + """One line per node, stable enough to diff against a golden file.""" + for (i, (_ast, text, before, after)) in enumerate (nodes): + print ("node {} [{}, {}) {!r}".format (i, before, after, text)) + + if err is not None: + print ("error {}: {}".format (type (err).__name__, err)) + + +def main (): + ap = argparse.ArgumentParser (description = __doc__, + formatter_class = argparse.RawDescriptionHelpFormatter) + ap.add_argument ("files", nargs = "*", help = "shell scripts to dump") + ap.add_argument ("-a", "--ast", action = "store_true", + help = "also dump the raw AST for each node") + ap.add_argument ("-q", "--quiet", action = "store_true", + help = "only show files with problems") + ap.add_argument ("-r", "--ranges", action = "store_true", + help = "terse one-line-per-node output, for golden-file comparison") + ap.add_argument ("-w", "--which", action = "store_true", + help = "print which libdash was imported, then exit") + args = ap.parse_args () + + if args.which: + print (libdash.__file__) + return 0 + + if not args.files: + ap.error ("no files given") + + init = True + bad = 0 + + for path in args.files: + try: + lines = read_lines (path) + except OSError as e: + print ("=== {}: cannot read: {}".format (path, e)) + bad += 1 + continue + + (nodes, err) = collect (path, init) + init = False + + if args.ranges: + show_ranges (path, nodes, err) + continue + + problems = audit (nodes, lines) + + if problems: + bad += 1 + + if problems or not args.quiet: + show (path, nodes, err, problems, args) + elif err is not None: + print ("=== {}: {}: {}".format (path, type (err).__name__, err)) + print () + + if bad: + print ("{} file(s) with line-attribution problems".format (bad)) + + return 1 if bad else 0 + + +if __name__ == "__main__": + sys.exit (main ()) diff --git a/setup.py b/setup.py index f9c8387..eaf7397 100644 --- a/setup.py +++ b/setup.py @@ -61,7 +61,7 @@ def run(self): setup(name='libdash', packages=['libdash'], cmdclass={'build_py': libdash_build_py}, - version='0.5.0', + version='0.5.1', long_description=long_description, long_description_content_type='text/markdown', include_package_data=True, diff --git a/test/README.md b/test/README.md index c3b7f08..f958e28 100644 --- a/test/README.md +++ b/test/README.md @@ -1,7 +1,24 @@ -There are three directories of tests: +There are four directories of tests: - `tests` are the original libdash tests, mostly handwritten - `pash_tests` are shell scripts taken from [`pash`](https://github.com/binpash/pash) - `failing` are shell scripts that aren't working right now (which is probably a bug) + - `line_mapping` holds fixtures for the per-node source line mapping, each with a + `.expected` golden -Both OCaml and Python bindings use the `round_trip.sh` to test round tripping. The `test_ocaml_python.sh` script compares the output from Python and OCaml. +Both OCaml and Python bindings use the `round_trip.sh` to test round tripping, i.e., that `print . parse` reaches a fixpoint. +It cannot compare against the original source, since the AST drops comments, whitespace and quoting style. +The `test_ocaml_python.sh` script ensures that Python and OCaml round-trip to the same output (or fail together). + +Round trip tests, however, can hide important bugs. +Two other tests check parses more concretely. + +`check_structs.py` checks the ctypes mirrors in `libdash/_dash.py` against +dash's real structs, comparing `sizeof` and every field offset via a probe +compiled from `src/*.h`. +This test needs a C compiler and a built tree. +This test only runs on the Python code, as OCaml's ctypes automatically computes offsets, but `_dash.py` hand-writes them. + +`line_mapping.sh` covers source line mapping (`parsedLines`, `linno_before`, `linno_after`). +It runs `python/dump.py --ranges` over each fixture and diffs against the `.expected` output. +Run `REGEN=1 ./line_mapping.sh` to automatically update the expected output after an intentional change. diff --git a/test/check_structs.py b/test/check_structs.py new file mode 100755 index 0000000..9d2bc57 --- /dev/null +++ b/test/check_structs.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +"""Check libdash/_dash.py's ctypes mirrors against dash's real C structs. + +_dash.py hand-copies dash's struct layouts and nothing else verifies the copy, +so a moved field is read at the old offset and silently returns a different +member -- as happened when parsefile gained `eof` and Python's `lleft` started +returning it. + +Compiles a probe against src/*.h and compares sizeof and every field offset +with ctypes. Needs a built tree (src/nodes.h is generated). + +The OCaml bindings need no equivalent: ocaml/dune's ctypes stanza computes +offsets from the headers at build time. +""" + +import os +import subprocess +import sys +import tempfile + +from ctypes import sizeof + +ROOT = os.path.dirname (os.path.dirname (os.path.abspath (__file__))) +sys.path.insert (0, ROOT) + +from libdash import _dash + +# (python class, C type, check field offsets too?) +# stackmark is opaque in _dash.py -- only its size is claimed correct. +STRUCTS = [ + ("stackmark", "struct stackmark", False), + ("nodelist", "struct nodelist", True), + ("union_node", "union node", True), + ("ncmd", "struct ncmd", True), + ("npipe", "struct npipe", True), + ("nredir", "struct nredir", True), + ("nbinary", "struct nbinary", True), + ("nif", "struct nif", True), + ("nfor", "struct nfor", True), + ("ncase", "struct ncase", True), + ("nclist", "struct nclist", True), + ("ndefun", "struct ndefun", True), + ("narg", "struct narg", True), + ("nfile", "struct nfile", True), + ("ndup", "struct ndup", True), + ("nhere", "struct nhere", True), + ("nnot", "struct nnot", True), + ("strpush", "struct strpush", True), + ("parsefile", "struct parsefile", True), +] + +PROBE_HEADER = """\ +#include +#include +#include "shell.h" +#include "nodes.h" +#include "input.h" +#include "memalloc.h" + +int main (void) +{ +""" + + +def probe_source (ctype, fields): + body = [' printf ("sizeof %zu\\n", sizeof ({}));'.format (ctype)] + + for name in fields: + body.append (' printf ("{0} %zu\\n", offsetof ({1}, {0}));' + .format (name, ctype)) + + return PROBE_HEADER + "\n".join (body) + "\n return 0;\n}\n" + + +def run_probe (ctype, fields, workdir): + """Returns (offsets dict, None) or (None, error string).""" + src = os.path.join (workdir, "probe.c") + exe = os.path.join (workdir, "probe") + + with open (src, "w") as f: + f.write (probe_source (ctype, fields)) + + cc = os.environ.get ("CC", "cc") + compiled = subprocess.run ([cc, "-I", os.path.join (ROOT, "src"), "-I", ROOT, + "-o", exe, src], + capture_output = True, text = True) + + if compiled.returncode != 0: + # a field C no longer has lands here, e.g. "no member named 'lastc'" + notes = [l.strip () for l in compiled.stderr.splitlines () if "error:" in l] + return (None, "; ".join (notes) or "probe failed to compile") + + ran = subprocess.run ([exe], capture_output = True, text = True) + + if ran.returncode != 0: + return (None, "probe failed to run") + + out = {} + + for line in ran.stdout.split ("\n"): + if line.strip (): + (key, value) = line.rsplit (" ", 1) + out [key] = int (value) + + return (out, None) + + +def resolve (pyname): + """_dash.py's `def nodelist` shadows `class nodelist`; reach the class + through a field pointing at it.""" + found = getattr (_dash, pyname) + + if pyname == "nodelist" and not hasattr (found, "_fields_"): + return dict (_dash.narg._fields_) ["backquote"]._type_ + + return found + + +def main (): + if not os.path.exists (os.path.join (ROOT, "src", "nodes.h")): + print ("SKIP: src/nodes.h not generated; build libdash first") + return 0 + + status = 0 + + with tempfile.TemporaryDirectory () as workdir: + for (pyname, ctype, check_fields) in STRUCTS: + cls = resolve (pyname) + fields = [n for (n, _t) in cls._fields_] if check_fields else [] + + (c, err) = run_probe (ctype, fields, workdir) + + if err is not None: + print ("STRUCT_MISMATCH: '{}' ({}): {}".format (ctype, pyname, err)) + status = 1 + continue + + problems = [] + + if sizeof (cls) != c ["sizeof"]: + problems.append ("sizeof: python {}, C {}" + .format (sizeof (cls), c ["sizeof"])) + + for name in fields: + mine = getattr (cls, name).offset + + if mine != c [name]: + problems.append ("field {}: python offset {}, C offset {}" + .format (name, mine, c [name])) + + if problems: + for p in problems: + print ("STRUCT_MISMATCH: '{}' ({}): {}".format (ctype, pyname, p)) + status = 1 + else: + print ("PASS '{}' (sizeof {}, {} field(s))" + .format (ctype, c ["sizeof"], len (fields))) + + return status + + +if __name__ == "__main__": + sys.exit (main ()) diff --git a/test/line_mapping.sh b/test/line_mapping.sh new file mode 100755 index 0000000..aea56ed --- /dev/null +++ b/test/line_mapping.sh @@ -0,0 +1,61 @@ +#!/bin/sh +# +# Checks the per-node source line mapping (parsedLines, linno_before, +# linno_after) -- invisible to round_trip.sh, which only sees the AST. +# +# Each fixture in line_mapping/ has a .expected golden, one line per node. +# REGEN=1 rewrites the goldens after an intentional change. + +: ${DUMP=../python/dump.py} + +cd "$(dirname "$0")" || exit 2 + +status=0 + +# dump.py falls back to the working tree, so a stale PYTHONPATH can swap the +# library under test; say which one we got. +echo "# libdash: $("$DUMP" --which 2>/dev/null || echo '')" + +for tgt in line_mapping/*.sh +do + expected="${tgt%.sh}.expected" + actual=$(mktemp) + + if ! "$DUMP" --ranges "$tgt" >"$actual" 2>&1 + then + echo "LINE_MAPPING_ABORT: '$tgt'" + cat "$actual" >&2 + rm -f "$actual" + status=1 + continue + fi + + if [ "$REGEN" ] + then + cp "$actual" "$expected" + echo "REGEN '$tgt'" + rm -f "$actual" + continue + fi + + if [ ! -f "$expected" ] + then + echo "LINE_MAPPING_NO_GOLDEN: '$tgt' (run REGEN=1 $0)" + rm -f "$actual" + status=1 + continue + fi + + if diff "$expected" "$actual" >/dev/null + then + echo "PASS '$tgt'" + else + echo "LINE_MAPPING_FAIL: '$tgt'" + diff -u "$expected" "$actual" + status=1 + fi + + rm -f "$actual" +done + +exit $status diff --git a/test/line_mapping/cases.expected b/test/line_mapping/cases.expected new file mode 100644 index 0000000..ed461b2 --- /dev/null +++ b/test/line_mapping/cases.expected @@ -0,0 +1,5 @@ +node 0 [0, 1) 'echo A\n' +node 1 [1, 2) 'echo B; echo C\n' +node 2 [2, 5) 'for i in 1 2; do\n echo "loop $i"\ndone\n' +node 3 [5, 7) 'echo D | \\\n cat\n' +node 4 [7, 8) 'echo E\n' diff --git a/test/line_mapping/cases.sh b/test/line_mapping/cases.sh new file mode 100644 index 0000000..983fddc --- /dev/null +++ b/test/line_mapping/cases.sh @@ -0,0 +1,8 @@ +echo A +echo B; echo C +for i in 1 2; do + echo "loop $i" +done +echo D | \ + cat +echo E diff --git a/test/line_mapping/comments_and_blanks.expected b/test/line_mapping/comments_and_blanks.expected new file mode 100644 index 0000000..561ca7b --- /dev/null +++ b/test/line_mapping/comments_and_blanks.expected @@ -0,0 +1,2 @@ +node 0 [2, 3) 'echo first\n' +node 1 [5, 6) 'echo second\n' diff --git a/test/line_mapping/comments_and_blanks.sh b/test/line_mapping/comments_and_blanks.sh new file mode 100644 index 0000000..988ca95 --- /dev/null +++ b/test/line_mapping/comments_and_blanks.sh @@ -0,0 +1,7 @@ +# leading comment + +echo first + +# middle comment +echo second +# trailing comment diff --git a/test/line_mapping/continuation.expected b/test/line_mapping/continuation.expected new file mode 100644 index 0000000..53bd341 --- /dev/null +++ b/test/line_mapping/continuation.expected @@ -0,0 +1,2 @@ +node 0 [0, 3) 'echo one \\\n two \\\n three\n' +node 1 [3, 4) 'echo next\n' diff --git a/test/line_mapping/continuation.sh b/test/line_mapping/continuation.sh new file mode 100644 index 0000000..d74f2f4 --- /dev/null +++ b/test/line_mapping/continuation.sh @@ -0,0 +1,4 @@ +echo one \ + two \ + three +echo next diff --git a/test/line_mapping/function.expected b/test/line_mapping/function.expected new file mode 100644 index 0000000..394ef50 --- /dev/null +++ b/test/line_mapping/function.expected @@ -0,0 +1,2 @@ +node 0 [0, 3) 'greet() {\n echo hello\n}\n' +node 1 [3, 4) 'greet\n' diff --git a/test/line_mapping/function.sh b/test/line_mapping/function.sh new file mode 100644 index 0000000..a25a7f7 --- /dev/null +++ b/test/line_mapping/function.sh @@ -0,0 +1,4 @@ +greet() { + echo hello +} +greet diff --git a/test/line_mapping/heredoc.expected b/test/line_mapping/heredoc.expected new file mode 100644 index 0000000..d373214 --- /dev/null +++ b/test/line_mapping/heredoc.expected @@ -0,0 +1,2 @@ +node 0 [0, 4) 'cat <