From ade414139872237390f5011631ad717334bc738d Mon Sep 17 00:00:00 2001 From: WyattBlue Date: Wed, 2 Sep 2026 01:32:25 -0400 Subject: [PATCH 1/3] Add pxdpad, a .pxd layout linter Reports padding holes in cdef class and cdef struct declarations, and the field order that removes them. A .pxd has one field order for every platform we ship to, so all four ABIs are checked at once: `long` is 4 bytes under LLP64 and pointers halve on 32-bit, which means a layout that packs tightly on lp64 can still have holes on Windows. The suggested order is the one that measures smallest across every ABI, not the best for any single one, and the declared order wins ties so a reshuffle is only proposed when it buys something. Sizing a cdef class needs two things that are not in the .pxd: PyObject_HEAD, and the hidden __pyx_vtab pointer Cython places after it in the topmost class that declares a cdef method. With both modelled, the computed sizes match tp_basicsize for all 58 cdef classes in av. Run it with `make pxdpad`, also wired into the smoke workflow. --- .github/workflows/smoke.yml | 7 + Makefile | 9 +- tools/pxdpad/.gitignore | 4 + tools/pxdpad/src/abi.nim | 120 ++++++++ tools/pxdpad/src/layout.nim | 251 +++++++++++++++++ tools/pxdpad/src/main.nim | 258 ++++++++++++++++++ tools/pxdpad/src/model.nim | 97 +++++++ tools/pxdpad/src/parser.nim | 372 +++++++++++++++++++++++++ tools/pxdpad/tests/test_pxdpad.nim | 423 +++++++++++++++++++++++++++++ 9 files changed, 1539 insertions(+), 2 deletions(-) create mode 100644 tools/pxdpad/.gitignore create mode 100644 tools/pxdpad/src/abi.nim create mode 100644 tools/pxdpad/src/layout.nim create mode 100644 tools/pxdpad/src/main.nim create mode 100644 tools/pxdpad/src/model.nim create mode 100644 tools/pxdpad/src/parser.nim create mode 100644 tools/pxdpad/tests/test_pxdpad.nim diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index c09c1b038..74cd42ad0 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -26,8 +26,15 @@ jobs: with: activate-environment: true python-version: "3.14" + - name: Nim + uses: jiro4989/setup-nim-action@v2 + with: + nim-version: "stable" + repo-token: ${{ secrets.GITHUB_TOKEN }} - name: Linters run: make lint + - name: pxdpad + run: make pxdpad nix: name: "py-${{ matrix.config.python }} lib-${{ matrix.config.ffmpeg }} ${{matrix.config.os}}" diff --git a/Makefile b/Makefile index 60edd9a98..ac0ca761b 100644 --- a/Makefile +++ b/Makefile @@ -7,11 +7,10 @@ PYTHON := $(PYAV_PYTHON) PIP := $(PYAV_PIP) -.PHONY: default build clean fate-suite lint test +.PHONY: default build clean fate-suite lint test pxdpad pxdpad-build default: build - build: $(PIP) install -U --pre cython setuptools CFLAGS=$(CFLAGS) LDFLAGS=$(LDFLAGS) $(PYTHON) setup.py build_ext --inplace --debug @@ -34,6 +33,12 @@ lint: isort --check-only --diff av examples tests mypy av tests +pxdpad-build: + nim c -d:danger --hints:off -o:tools/pxdpad/bin/pxdpad tools/pxdpad/src/main.nim + +pxdpad: pxdpad-build + tools/pxdpad/bin/pxdpad av include + test: $(PIP) install --group test $(PYTHON) -m pytest diff --git a/tools/pxdpad/.gitignore b/tools/pxdpad/.gitignore new file mode 100644 index 000000000..76791781d --- /dev/null +++ b/tools/pxdpad/.gitignore @@ -0,0 +1,4 @@ +bin/ +nimcache/ +*.exe +tests/test_pxdpad diff --git a/tools/pxdpad/src/abi.nim b/tools/pxdpad/src/abi.nim new file mode 100644 index 000000000..c2998cd61 --- /dev/null +++ b/tools/pxdpad/src/abi.nim @@ -0,0 +1,120 @@ +## Target ABIs and the built-in type table. + +import std/[strutils, sets] +import model + +const + targetNames* = ["lp64", "darwin-arm64", "llp64", "ilp32"] + +proc getTarget*(name: string): Target = + ## `lp64` is the x86-64 System V ABI, `llp64` is 64-bit Windows. + case name.toLowerAscii + of "lp64", "linux", "sysv": # x86-64 System V + Target(name: "lp64", ptrSize: 8, ptrAlign: 8, longSize: 8, longAlign: 8, + ldSize: 16, ldAlign: 16, maxScalarAlign: 8, headSize: 16) + of "darwin-arm64", "macos", "arm64": + Target(name: "darwin-arm64", ptrSize: 8, ptrAlign: 8, longSize: 8, longAlign: 8, + ldSize: 8, ldAlign: 8, maxScalarAlign: 8, headSize: 16) + of "llp64", "windows", "win64": + Target(name: "llp64", ptrSize: 8, ptrAlign: 8, longSize: 4, longAlign: 4, + ldSize: 8, ldAlign: 8, maxScalarAlign: 8, headSize: 16) + of "ilp32", "x86": # 32-bit x86 System V + Target(name: "ilp32", ptrSize: 4, ptrAlign: 4, longSize: 4, longAlign: 4, + ldSize: 12, ldAlign: 4, maxScalarAlign: 4, headSize: 8) + else: + raise newException(ValueError, "unknown target '" & name & "', expected one of " & + targetNames.join(", ")) + +proc defaultTarget*(): Target = + when defined(windows): getTarget("llp64") + elif defined(macosx) and defined(arm64): getTarget("darwin-arm64") + elif sizeof(pointer) == 4: getTarget("ilp32") + else: getTarget("lp64") + +const pyObjectTypes* = toHashSet([ + "object", "str", "bytes", "unicode", "bytearray", "dict", "list", "tuple", + "set", "frozenset", "type", "slice", "complex", "BaseException", "Exception", + "basestring", "memoryview", "array"]) + +## Multi-word C base types, keyed by their normalized spelling. +proc cScalar*(name: string, t: Target): tuple[size, align: int, ok: bool] = + template r(s: int): untyped = (s, min(s, t.maxScalarAlign), true) + case name + of "char", "signed char", "unsigned char", "_Bool", "bool": r(1) + of "short", "short int", "unsigned short", "unsigned short int", + "signed short", "signed short int": r(2) + of "int", "signed", "signed int", "unsigned", "unsigned int": r(4) + of "long", "long int", "unsigned long", "unsigned long int", + "signed long", "signed long int": (t.longSize, t.longAlign, true) + of "long long", "long long int", "unsigned long long", + "unsigned long long int", "signed long long", "signed long long int": r(8) + of "float": r(4) + of "double": r(8) + of "long double": (t.ldSize, t.ldAlign, true) + of "float complex": r(8) + of "double complex": r(16) + # Fixed-width and semantic integers. + of "int8_t", "uint8_t": r(1) + of "int16_t", "uint16_t", "char16_t": r(2) + of "int32_t", "uint32_t", "char32_t": r(4) + of "int64_t", "uint64_t": r(8) + of "size_t", "ssize_t", "Py_ssize_t", "ptrdiff_t", "intptr_t", "uintptr_t", + "Py_hash_t", "Py_uintptr_t", "uintmax_t", "intmax_t": + (t.ptrSize, t.ptrAlign, true) + of "Py_UCS4", "wchar_t": r(4) + of "Py_UCS2": r(2) + of "Py_UCS1": r(1) + of "bint": r(4) # Cython lowers bint to int + of "void": (0, 1, false) # only legal behind a pointer + else: (0, 0, false) + +## Structs whose layout is fixed by an ABI we can rely on, written as ordinary +## declarations so that every target computes them itself instead of trusting a +## number measured on one machine. A real definition in the sources being linted +## takes precedence over these. +const builtinDecls* = """ +cdef struct AVRational: + int num + int den + +cdef union AVChannelLayoutU: + uint64_t mask + void *map + +cdef struct AVChannelLayout: + int order + int nb_channels + AVChannelLayoutU u + void *opaque + +cdef struct AVIndexEntry: + int64_t pos + int64_t timestamp + int flags_and_size + int min_distance + +cdef struct AVSubtitle: + uint16_t format + uint32_t start_display_time + uint32_t end_display_time + unsigned int num_rects + void *rects + int64_t pts + +cdef struct PyObject: + Py_ssize_t ob_refcnt + void *ob_type + +cdef struct Py_buffer: + void *buf + void *obj + Py_ssize_t len + Py_ssize_t itemsize + int readonly + int ndim + char *format + Py_ssize_t *shape + Py_ssize_t *strides + Py_ssize_t *suboffsets + void *internal +""" diff --git a/tools/pxdpad/src/layout.nim b/tools/pxdpad/src/layout.nim new file mode 100644 index 000000000..38cbc922e --- /dev/null +++ b/tools/pxdpad/src/layout.nim @@ -0,0 +1,251 @@ +## Resolves field types to sizes and lays aggregates out per the target ABI. + +import std/[strutils, tables, sets, sequtils, algorithm] +import model, abi, parser + +type + Resolver* = ref object + target*: Target + aggs*: Table[string, Aggregate] ## structs and unions + classAggs*: Table[string, Aggregate] ## cdef classes, a separate namespace + classes*: HashSet[string] + enums*: HashSet[string] + aliases*: Table[string, string] + assumed*: Table[string, tuple[size, align: int]] + memo: Table[string, Layout] + inProgress: HashSet[string] + +func simpleName*(s: string): string = + let dot = s.rfind('.') + if dot >= 0: s[dot + 1 .. ^1] else: s + +proc newResolver*(pr: ParseResult, t: Target): Resolver = + result = Resolver(target: t, classes: pr.classes, enums: pr.enums, + aliases: pr.aliases) + for a in pr.aggs: + if not a.hasBody: continue + # A cdef class and a C struct may share a name (`AVRational` does), so they + # are kept apart: a qualified type name means the struct, a bare one the class. + if a.kind == akClass: + if a.name notin result.classAggs: result.classAggs[a.name] = a + continue + # First definition wins, but a non-extern one always beats an extern one. + if a.name in result.aggs and not (result.aggs[a.name].isExtern and not a.isExtern): + continue + result.aggs[a.name] = a + + # Fill in the ABI-defined structs, but never over a real definition: an + # extern declaration lists only the members Cython was told about, so the + # built-in wins over that, and a full definition in the sources wins over + # the built-in. + var builtins = ParseResult() + parseFile("", builtinDecls, builtins) + for a in builtins.aggs: + if a.name notin result.aggs or result.aggs[a.name].isExtern: + result.aggs[a.name] = a + +proc layoutAgg*(r: Resolver, a: Aggregate): Layout + +func key(a: Aggregate): string = $a.kind & " " & a.name + +proc sizeOf(r: Resolver, a: Aggregate): tuple[size, align: int, ok: bool] = + if key(a) in r.inProgress: return (0, 0, false) # recursive by value + let lay = r.layoutAgg(a) + if lay.complete: (lay.size, lay.align, true) else: (0, 0, false) + +proc sizeOfStruct(r: Resolver, name: string): tuple[size, align: int, ok: bool] = + if not r.aggs.hasKey(name): return (0, 0, false) + let a = r.aggs[name] + # A struct declared inside `cdef extern` lists only the members Cython was + # told about, so its field list cannot be trusted for a size. + if a.isExtern: return (0, 0, false) + r.sizeOf(a) + +proc sizeOfClass(r: Resolver, name: string): tuple[size, align: int, ok: bool] = + if not r.classAggs.hasKey(name): return (0, 0, false) + r.sizeOf(r.classAggs[name]) + +proc sizeOfType*(r: Resolver, typeName: string, ptrDepth, arrayLen: int): + tuple[size, align: int, ok: bool] = + var base: tuple[size, align: int, ok: bool] + if ptrDepth > 0: + base = (r.target.ptrSize, r.target.ptrAlign, true) + else: + var name = simpleName(typeName) + var hops = 0 + while r.aliases.hasKey(name) and hops < 8: + name = simpleName(r.aliases[name]) + if name.endsWith("*"): return (r.target.ptrSize, r.target.ptrAlign, true) + inc hops + # `lib.AVRational` is FFmpeg's two-int struct; a bare `AVRational` is the + # cdef class of the same name. Qualified names take the C meaning first. + let qualified = typeName.contains('.') + template asPyObject(): untyped = + (name in pyObjectTypes or name in r.classes) + if r.assumed.hasKey(name): + let a = r.assumed[name] + base = (a.size, a.align, true) + else: + base = cScalar(name, r.target) + if not base.ok and not qualified and asPyObject(): + base = (r.target.ptrSize, r.target.ptrAlign, true) + if not base.ok and name in r.enums: + base = (4, 4, true) + if not base.ok: + base = r.sizeOfStruct(name) + if not base.ok and qualified and asPyObject(): + base = (r.target.ptrSize, r.target.ptrAlign, true) + if not base.ok: return base + if arrayLen < 0: return (0, 0, false) + (base.size * arrayLen, base.align, true) + +proc baseOfClass(r: Resolver, a: Aggregate): tuple[size, align: int, ok: bool] = + if a.base.len == 0: + return (r.target.headSize, r.target.ptrAlign, true) + r.sizeOfClass(a.base) + +proc ancestorHasVtab(r: Resolver, a: Aggregate): bool = + ## Cython puts the `__pyx_vtab` pointer in the topmost class that declares a + ## cdef method; every subclass inherits that slot rather than adding its own. + var name = a.base + var hops = 0 + while name.len > 0 and hops < 16: + if not r.classAggs.hasKey(name): return false + let b = r.classAggs[name] + if b.hasCMethods: return true + name = b.base + inc hops + false + +proc classBase(r: Resolver, a: Aggregate): tuple[size, align: int, ok, vtab: bool] = + if a.kind != akClass: return (0, 1, true, false) + let b = r.baseOfClass(a) + var size = b.size + var align = b.align + var vtab = false + if a.hasCMethods and not r.ancestorHasVtab(a): + size += r.target.ptrSize + align = max(align, r.target.ptrAlign) + vtab = true + (size, align, b.ok, vtab) + +proc layoutFields(r: Resolver, a: Aggregate, fields: seq[FieldDecl], + baseSize, baseAlign: int, collectHoles: bool): Layout = + result.baseSize = baseSize + result.align = max(1, baseAlign) + result.complete = true + var offset = baseSize + + for f in fields: + let sz = r.sizeOfType(f.typeName, f.ptrDepth, f.arrayLen) + if not sz.ok: + result.complete = false + result.unresolved.add f.name & ": " & f.typeName & + (if f.arrayLen < 0: "[]" else: "") + continue + let align = if a.packed: 1 else: sz.align + result.align = max(result.align, align) + if a.kind == akUnion: + result.fields.add LaidField(decl: f, offset: baseSize, size: sz.size, + align: align) + offset = max(offset, baseSize + sz.size) + continue + let pad = (align - (offset mod align)) mod align + if pad > 0 and collectHoles: + result.holes.add Hole(offset: offset, size: pad, + after: (if result.fields.len > 0: result.fields[^1].decl.name else: "")) + offset += pad + result.fields.add LaidField(decl: f, offset: offset, size: sz.size, + align: align) + offset += sz.size + + let tail = (result.align - (offset mod result.align)) mod result.align + if tail > 0 and collectHoles: + result.holes.add Hole(offset: offset, size: tail, tail: true, + after: (if result.fields.len > 0: result.fields[^1].decl.name else: "")) + result.size = offset + tail + +proc layoutAgg*(r: Resolver, a: Aggregate): Layout = + if r.memo.hasKey(key(a)): return r.memo[key(a)] + r.inProgress.incl key(a) + defer: r.inProgress.excl key(a) + + let b = r.classBase(a) + result = r.layoutFields(a, a.fields, b.size, b.align, true) + result.vtab = b.vtab + if not b.ok: + result.complete = false + result.unresolved.insert("base class: " & a.base, 0) + if result.complete: + r.memo[key(a)] = result + +func orderKey(f: LaidField): (int, int) = (-f.align, -f.size) + +proc sizeUnder*(r: Resolver, a: Aggregate, order: seq[FieldDecl]): + tuple[size: int, ok: bool] = + ## What this aggregate would measure with its fields in the given order. + let b = r.classBase(a) + let lay = r.layoutFields(a, order, b.size, b.align, false) + (lay.size, lay.complete and b.ok) + +proc optimize*(r: Resolver, a: Aggregate, lay: Layout): tuple[size: int, order: seq[FieldDecl]] = + ## Descending alignment, then descending size, ties broken by original order. + if a.kind == akUnion or a.packed or not lay.complete: + return (lay.size, lay.fields.mapIt(it.decl)) + var sorted = lay.fields + sorted.sort(proc (x, y: LaidField): int = cmp(orderKey(x), orderKey(y))) + (r.sizeUnder(a, sorted.mapIt(it.decl)).size, sorted.mapIt(it.decl)) + +proc analyze*(r: Resolver, a: Aggregate): Finding = + let lay = r.layoutAgg(a) + let opt = r.optimize(a, lay) + Finding(agg: a, layout: lay, optimalSize: opt.size, order: opt.order) + +func names(order: seq[FieldDecl]): seq[string] = order.mapIt(it.name) + +proc analyzeAll*(resolvers: seq[Resolver], a: Aggregate): Report = + ## Measures `a` against every ABI and picks one field order for all of them. + ## A `.pxd` has a single declaration, so an order that is perfect on LP64 but + ## poor on LLP64 is not a fix; the candidates are each ABI's own best order + ## plus the declared one, scored by their total size everywhere. + result.agg = a + var candidates = @[a.fields] + for r in resolvers: + let f = r.analyze(a) + result.results.add TargetResult(target: r.target.name, layout: f.layout, + optimalSize: f.optimalSize, suggested: f.layout.size, + unresolved: f.layout.unresolved) + if not f.layout.complete: continue + inc result.sized + if a.kind == akUnion or a.packed: continue + if not candidates.anyIt(names(it) == names(f.order)): candidates.add f.order + + if result.sized == 0: + result.order = a.fields + return + + var best = 0 + var bestScore = (high(int), high(int)) + for ci, cand in candidates: + var total = 0 + var first = high(int) + var ok = true + for ti, r in resolvers: + if not result.results[ti].layout.complete: continue + let s = r.sizeUnder(a, cand) + if not s.ok: + ok = false + break + total += s.size + if first == high(int): first = s.size + # Ties keep the earliest candidate, and the declared order comes first, so + # a reshuffle is only ever suggested when it actually buys something. + if ok and (total, first) < bestScore: + bestScore = (total, first) + best = ci + + result.order = candidates[best] + result.changed = names(result.order) != names(a.fields) + for ti, r in resolvers: + if result.results[ti].layout.complete: + result.results[ti].suggested = r.sizeUnder(a, result.order).size diff --git a/tools/pxdpad/src/main.nim b/tools/pxdpad/src/main.nim new file mode 100644 index 000000000..d4101cb74 --- /dev/null +++ b/tools/pxdpad/src/main.nim @@ -0,0 +1,258 @@ +import std/[strutils, sequtils, os, parseopt, sets, tables, terminal] +import model, abi, parser, layout + +const + version = "pxdpad 0.1.0" + skipDirs = ["venvs", "venv", ".venv", "build", "dist", ".git", "vendor", + ".eggs", "node_modules", "__pycache__"] + usage = """ +$1 + +Usage: pxdpad [options] [path ...] + +Reports padding holes in `cdef class` and `cdef struct` declarations, and the +field order that removes them. Paths may be .pxd files or directories (searched +recursively); the default is the working directory. + +Every ABI is checked by default ($2), because a +declaration that packs tightly on one platform can have holes on another: `long` +is 4 bytes on 64-bit Windows, and pointers are 4 bytes on 32-bit. A .pxd has one +field order for all of them, so the suggested order is the one that measures +smallest across every ABI checked, not the best for any single one. + +Options: + -t, --target:NAMES restrict to these ABIs, comma-separated. One of + $2 + -m, --min-waste:N only report types wasting at least N bytes (default: 1) + -a, --all list every type, including those with no waste + --assume:T=SZ[:AL] give type T a size (and alignment); repeatable + --include-extern also analyze declarations inside `cdef extern` blocks + --no-suggest omit the suggested field order + -v, --verbose list types that could not be sized + --exit-zero always exit 0, even when findings are reported + -h, --help show this help + -V, --version show the version + +Exit status is 1 when a type wastes --min-waste bytes or more on any ABI +checked, unless --exit-zero is given. +""" + +type Options = object + paths: seq[string] + targets: seq[string] + minWaste: int + all, includeExtern, noSuggest, verbose, exitZero: bool + assumed: Table[string, tuple[size, align: int]] + +proc parseAssume(spec: string, opts: var Options) = + let eq = spec.find('=') + if eq < 0: + quit "--assume needs TYPE=SIZE[:ALIGN], got '" & spec & "'", 2 + let name = spec[0 ..< eq].strip() + let rest = spec[eq + 1 .. ^1].split(':') + try: + let size = parseInt(rest[0].strip()) + let align = if rest.len > 1: parseInt(rest[1].strip()) else: min(size, 8) + if size < 0 or align <= 0: raise newException(ValueError, "") + opts.assumed[simpleName(name)] = (size, align) + except ValueError: + quit "--assume needs integer values, got '" & spec & "'", 2 + +proc collect(paths: seq[string]): seq[string] = + var seen = initHashSet[string]() + for p in paths: + if fileExists(p): + if not seen.containsOrIncl(p): result.add p + elif dirExists(p): + for path in walkDirRec(p, relative = false): + if path.splitFile().ext != ".pxd": continue + if path.split(DirSep).anyIt(it in skipDirs): continue + if not seen.containsOrIncl(path): result.add path + else: + stderr.writeLine "pxdpad: no such file or directory: " & p + +proc rel(p: string): string = + ## Paths under the working directory read better relative; others do not. + try: + let r = p.relativePath(getCurrentDir()) + if r.startsWith(".."): p else: r + except OSError: + p + +proc holeLines(lay: Layout): string = + for h in lay.holes: + if h.tail: + result.add " " & $h.size & " bytes of tail padding at offset " & + $h.offset & "\n" + elif h.after.len == 0: + result.add " " & $h.size & " bytes of padding after the object header " & + "(offset " & $h.offset & ")\n" + else: + result.add " " & $h.size & " bytes of padding after `" & h.after & + "` (offset " & $h.offset & ")\n" + +proc sig(t: TargetResult): string = + ## Two ABIs are worth one row when they measure the type identically. lp64 and + ## darwin-arm64 differ only in `long double`, so they usually collapse. + if not t.layout.complete: return "n/a" + result = $t.layout.size & "/" & $t.suggested & "/" & $t.layout.align + for h in t.layout.holes: + result.add "|" & $h.offset & ":" & $h.size + +proc grouped(rep: Report): seq[tuple[label: string, res: TargetResult]] = + ## ABIs that measure alike, merged and labelled together, in the order given. + var index = initTable[string, int]() + for t in rep.results: + let s = sig(t) + if index.hasKey(s): + result[index[s]].label.add ", " & t.target + else: + index[s] = result.len + result.add (t.target, t) + +proc describe(rep: Report, opts: Options, color: bool): string = + let bold = if color: "\e[1m" else: "" + let dim = if color: "\e[2m" else: "" + let red = if color: "\e[31m" else: "" + let off = if color: "\e[0m" else: "" + let waste = rep.worstWaste + let single = rep.results.len == 1 + + result = dim & rel(rep.agg.file) & ":" & $rep.agg.line & off & " " & bold & + $rep.agg.kind & " " & rep.agg.name & off + if single: + let t = rep.results[0] + result.add " — " & $t.layout.size & " bytes" + if waste > 0: + result.add ", " & red & $waste & " wasted" & off & " (" & $t.suggested & + " optimal)" + result.add "\n" + else: + let groups = grouped(rep) + var worstGroup = 0 + for i, g in groups: + if g.res.wasted > groups[worstGroup].res.wasted: worstGroup = i + if waste > 0: + result.add " — " & red & $waste & " bytes wasted" & off & " on " & + groups[worstGroup].label & "\n" + else: + result.add "\n" + var cells: seq[string] = @[] + for g in groups: + if not g.res.layout.complete: + cells.add g.label & " " & dim & "n/a" & off + elif g.res.wasted > 0: + cells.add g.label & " " & $g.res.layout.size & red & "→" & + $g.res.suggested & off + else: + cells.add g.label & " " & $g.res.layout.size + result.add " " & cells.join(" ") & "\n" + + # Holes differ per ABI; show them for the one with the most to gain. + var worst = 0 + for i, t in rep.results: + if t.wasted > rep.results[worst].wasted: worst = i + let lay = rep.results[worst].layout + if lay.holes.len > 0: + if not single: + let groups = grouped(rep) + var label = rep.results[worst].target + for g in groups: + if sig(g.res) == sig(rep.results[worst]): label = g.label + result.add " padding on " & label & ":\n" + result.add holeLines(lay) + + if rep.changed and not opts.noSuggest: + result.add " suggested order" + if single: + result.add " (" & $rep.results[0].suggested & " bytes)" + else: + result.add " — " & grouped(rep).filterIt(it.res.layout.complete) + .mapIt(it.label & " " & $it.res.suggested).join("; ") + result.add ":\n" + for d in rep.order: + result.add " " & d.display & "\n" + +proc main() = + var opts = Options(minWaste: 1) + for kind, key, val in getopt(): + case kind + of cmdArgument: opts.paths.add key + of cmdLongOption, cmdShortOption: + case key + of "t", "target": + for name in val.split(','): + if name.strip().len > 0: opts.targets.add name.strip() + of "m", "min-waste": + try: opts.minWaste = parseInt(val) + except ValueError: quit "pxdpad: --min-waste needs an integer", 2 + of "a", "all": opts.all = true + of "assume": parseAssume(val, opts) + of "include-extern": opts.includeExtern = true + of "no-suggest": opts.noSuggest = true + of "v", "verbose": opts.verbose = true + of "exit-zero": opts.exitZero = true + of "h", "help": quit usage % [version, targetNames.join(", ")], 0 + of "V", "version": quit version, 0 + else: quit "pxdpad: unknown option --" & key, 2 + of cmdEnd: discard + if opts.paths.len == 0: opts.paths = @[getCurrentDir()] + if opts.targets.len == 0: opts.targets = @targetNames + + var targets: seq[Target] = @[] + var seenTarget = initHashSet[string]() + for name in opts.targets: + var t: Target + try: t = getTarget(name) + except ValueError as e: quit "pxdpad: " & e.msg, 2 + if not seenTarget.containsOrIncl(t.name): targets.add t + + let files = collect(opts.paths) + var pr = ParseResult() + for f in files: + try: + parseFile(f, readFile(f), pr) + except IOError: + stderr.writeLine "pxdpad: cannot read " & f + + var resolvers: seq[Resolver] = @[] + for t in targets: + let r = newResolver(pr, t) + r.assumed = opts.assumed + resolvers.add r + + var findings, skipped: seq[Report] = @[] + var checked, reportable = 0 + for a in pr.aggs: + if not a.hasBody: continue + if a.fields.len == 0 and not opts.all: continue + if a.isExtern and not opts.includeExtern: continue + let rep = analyzeAll(resolvers, a) + if rep.sized == 0: + skipped.add rep + continue + inc checked + let notable = rep.worstWaste >= max(1, opts.minWaste) + if notable: inc reportable + if opts.all or notable: findings.add rep + + let color = isatty(stdout) + for f in findings: + echo describe(f, opts, color) + var total = 0 + for f in findings: total += f.worstWaste + echo "$1 type(s) checked across $2 ABI(s) ($3), $4 with waste, $5 byte(s) wasted" % + [$checked, $targets.len, targets.mapIt(it.name).join(", "), $reportable, $total] + if skipped.len > 0: + echo "$1 type(s) skipped: a field's size is unknown (--verbose to list)" % + [$skipped.len] + if opts.verbose: + for f in skipped: + echo " " & rel(f.agg.file) & ":" & $f.agg.line & " " & f.agg.name & + " — " & f.results[0].unresolved.join(", ") + + if reportable > 0 and not opts.exitZero: quit 1 + quit 0 + +when isMainModule: + main() diff --git a/tools/pxdpad/src/model.nim b/tools/pxdpad/src/model.nim new file mode 100644 index 000000000..71071ecd2 --- /dev/null +++ b/tools/pxdpad/src/model.nim @@ -0,0 +1,97 @@ +## Data types shared by the parser, the layout engine and the CLI. + +type + AggKind* = enum + akClass = "cdef class" + akStruct = "struct" + akUnion = "union" + + Target* = object + name*: string + ptrSize*, ptrAlign*: int + longSize*, longAlign*: int + ldSize*, ldAlign*: int ## long double + maxScalarAlign*: int ## i386 aligns double and long long to 4 + headSize*: int ## sizeof(PyObject_HEAD) + + FieldDecl* = object + name*: string + typeName*: string ## base type as written, qualifiers and subscript removed + mods*: string ## "readonly ", "public " or "" + display*: string ## paste-ready declaration line + ptrDepth*: int + arrayLen*: int ## 1 when scalar, -1 when the extent is not a literal + line*: int + + Aggregate* = object + kind*: AggKind + name*: string + base*: string ## base class for akClass, "" when object + packed*: bool + isExtern*: bool + hasBody*: bool ## false for forward declarations + hasCMethods*: bool ## declares a cdef/cpdef method, so it needs a vtable + file*: string + line*: int + fields*: seq[FieldDecl] + + Hole* = object + offset*, size*: int + after*: string ## field the hole follows ("" = the object header) + tail*: bool + + LaidField* = object + decl*: FieldDecl + offset*, size*, align*: int + + Layout* = object + size*, align*, baseSize*: int + vtab*: bool ## a hidden __pyx_vtab pointer is part of baseSize + complete*: bool ## every field resolved to a concrete size + fields*: seq[LaidField] + holes*: seq[Hole] + unresolved*: seq[string] + + Finding* = object + agg*: Aggregate + layout*: Layout + optimalSize*: int + order*: seq[FieldDecl] ## fields in the suggested order + + TargetResult* = object + target*: string + layout*: Layout + optimalSize*: int ## best this ABI could do on its own + suggested*: int ## what the shared suggested order costs here + unresolved*: seq[string] + + Report* = object + ## One aggregate measured against several ABIs at once. The declaration has + ## a single field order, so the suggestion has to suit all of them. + agg*: Aggregate + results*: seq[TargetResult] + order*: seq[FieldDecl] + changed*: bool ## the suggestion differs from the declared order + sized*: int ## how many targets could be measured + +func wasted*(f: Finding): int = + f.layout.size - f.optimalSize + +func complete*(t: TargetResult): bool = t.layout.complete + +func wasted*(t: TargetResult): int = + ## What the suggested order would actually save on this ABI. + if t.layout.complete: t.layout.size - t.suggested else: 0 + +func worstWaste*(r: Report): int = + for t in r.results: result = max(result, t.wasted) + +func worstTarget*(r: Report): string = + var worst = -1 + for t in r.results: + if t.wasted > worst: + worst = t.wasted + result = t.target + +func isArray*(f: FieldDecl): bool = + f.arrayLen != 1 diff --git a/tools/pxdpad/src/parser.nim b/tools/pxdpad/src/parser.nim new file mode 100644 index 000000000..a06617a07 --- /dev/null +++ b/tools/pxdpad/src/parser.nim @@ -0,0 +1,372 @@ +## A tolerant, line-oriented reader for Cython .pxd declarations. +## +## It is deliberately not a real Cython parser: it recognizes aggregate headers +## and member declarations, and skips anything it does not understand rather +## than failing. + +import std/[strutils, sequtils, tables, sets] +import model + +type + ParseResult* = object + aggs*: seq[Aggregate] + classes*: HashSet[string] ## every name declared as `cdef class` + enums*: HashSet[string] ## enum tags (int-sized) + aliases*: Table[string, string] + + LogicalLine = object + indent: int + text: string + line: int + + CtxKind = enum + ckExtern, ckAgg, ckOther + + Ctx = object + kind: CtxKind + indent: int + agg: int + +const + cKeywords = ["unsigned", "signed", "long", "short", "int", "char", "float", + "double", "void", "_Bool", "complex"] + qualifiers = ["const", "volatile", "static", "struct", "union", "enum"] + quotePrefixes = {'r', 'b', 'u', 'f', 'R', 'B', 'U', 'F'} + +func leadingIndent(s: string): int = + for c in s: + if c == ' ': inc result + elif c == '\t': result += 8 - (result mod 8) + else: break + +func stripComment(s: string): string = + var quote = '\0' + var i = 0 + while i < s.len: + let c = s[i] + if quote != '\0': + if c == '\\': inc i + elif c == quote: quote = '\0' + elif c in {'\'', '"'}: quote = c + elif c == '#': return s[0 ..< i] + inc i + s + +func bracketDelta(s: string): int = + var quote = '\0' + var i = 0 + while i < s.len: + let c = s[i] + if quote != '\0': + if c == '\\': inc i + elif c == quote: quote = '\0' + elif c in {'\'', '"'}: quote = c + elif c in {'(', '[', '{'}: inc result + elif c in {')', ']', '}'}: dec result + inc i + +func logicalLines(src: string): seq[LogicalLine] = + ## Drops blank lines, comments and docstrings; joins bracket and backslash + ## continuations into one entry. + let raw = src.splitLines() + var i = 0 + var docDelim = "" + while i < raw.len: + let stripped = raw[i].strip() + if docDelim.len > 0: + if stripped.contains(docDelim): docDelim = "" + inc i + continue + var body = stripped + while body.len > 0 and body[0] in quotePrefixes: body = body[1 .. ^1] + if body.startsWith("\"\"\"") or body.startsWith("'''"): + let d = body[0 .. 2] + if not body[3 .. ^1].contains(d): docDelim = d + inc i + continue + var text = stripComment(raw[i]) + if text.strip().len == 0: + inc i + continue + let indent = leadingIndent(text) + let startLine = i + 1 + var acc = text.strip() + var depth = bracketDelta(acc) + while (depth > 0 or acc.endsWith("\\")) and i + 1 < raw.len: + if acc.endsWith("\\"): acc.setLen(acc.len - 1) + inc i + let nxt = stripComment(raw[i]).strip() + depth += bracketDelta(nxt) + acc = acc.strip() & " " & nxt + result.add LogicalLine(indent: indent, text: acc.strip(), line: startLine) + inc i + +func words(s: string): seq[string] = + s.split({' ', '\t'}).filterIt(it.len > 0) + +func tokenize(s: string): seq[string] = + ## Splits a declaration into identifiers (with any `[...]` subscript attached), + ## `*` and `,`. + var i = 0 + while i < s.len: + let c = s[i] + if c in {' ', '\t'}: + inc i + elif c == '*': + result.add "*" + inc i + elif c == ',': + result.add "," + inc i + elif c == '[': + let start = i + var depth = 0 + while i < s.len: + if s[i] == '[': inc depth + elif s[i] == ']': + dec depth + if depth == 0: + inc i + break + inc i + let group = s[start ..< i] + if result.len > 0 and result[^1] notin ["*", ","]: + result[^1] = result[^1] & group + else: + result.add group + else: + let start = i + while i < s.len and (s[i].isAlphaNumeric or s[i] in {'_', '.'}): inc i + if i == start: inc i + else: result.add s[start ..< i] + +func parseExtent(sub: string): int = + ## `[8]` -> 8, `[4][2]` -> 8, anything non-literal -> -1. + result = 1 + var i = 0 + while i < sub.len: + if sub[i] == '[': + let close = sub.find(']', i) + if close < 0: return -1 + let inner = sub[i + 1 ..< close].strip() + var n: int + try: + n = parseInt(inner) + except ValueError: + return -1 + result *= n + i = close + 1 + else: + inc i + +proc parseMember(decl, srcLine: string, isClass: bool, lineNo: int): seq[FieldDecl] = + ## `decl` is the declaration with any leading `cdef` already removed. + var rest = decl.strip() + var mods = "" + while true: + let w = rest.words() + if w.len == 0: return + if isClass and w[0] in ["readonly", "public"]: + mods = w[0] & " " + rest = rest[w[0].len .. ^1].strip() + elif w[0] in ["const", "volatile", "static"]: + rest = rest[w[0].len .. ^1].strip() + else: + break + + let toks = tokenize(rest) + if toks.len == 0: return + + var parts: seq[string] + var idx = 0 + while idx < toks.len: + let t = toks[idx] + if t in qualifiers: + inc idx + elif t in cKeywords: + parts.add t + inc idx + elif parts.len == 0: + parts.add t + inc idx + break + else: + break + if parts.len == 0: return + if idx >= toks.len: + # `cdef readonly planes` — an untyped class attribute is an object. + if isClass and parts.len == 1 and parts[0] notin cKeywords and + not parts[0].contains('.') and not parts[0].contains('['): + return @[FieldDecl(name: parts[0], typeName: "object", mods: mods, + display: srcLine.strip(), arrayLen: 1, line: lineNo)] + return + + var typeName = parts.join(" ") + let sub = typeName.find('[') + if sub >= 0: typeName = typeName[0 ..< sub] + + var groups: seq[seq[string]] = @[@[]] + for t in toks[idx .. ^1]: + if t == ",": groups.add @[] + else: groups[^1].add t + + var res: seq[FieldDecl] = @[] + for g in groups: + var f = FieldDecl(typeName: typeName, mods: mods, arrayLen: 1, line: lineNo) + for t in g: + if t == "*": inc f.ptrDepth + elif f.name.len == 0: + let br = t.find('[') + if br >= 0: + f.name = t[0 ..< br] + f.arrayLen = parseExtent(t[br .. ^1]) + else: + f.name = t + if f.name.len == 0: return @[] + res.add f + + for i in 0 ..< res.len: + if res.len == 1: + res[i].display = srcLine.strip() + else: + let stars = repeat('*', res[i].ptrDepth) + let arr = if res[i].arrayLen == 1: "" else: "[" & $res[i].arrayLen & "]" + res[i].display = (if isClass: "cdef " else: "") & res[i].mods & + parts.join(" ") & " " & stars & res[i].name & arr + res + +func headerName(tokens: seq[string], at: int): string = + ## Name of an aggregate header, cut at `(` or `:`. + if at >= tokens.len: return "" + result = tokens[at] + for stop in ['(', ':']: + let i = result.find(stop) + if i >= 0: result = result[0 ..< i] + +func baseName(text: string): string = + ## `cdef class VideoFrame(Frame):` -> `Frame` + let o = text.find('(') + if o < 0: return "" + let c = text.find(')', o) + if c < 0: return "" + result = text[o + 1 ..< c].strip() + let dot = result.rfind('.') + if dot >= 0: result = result[dot + 1 .. ^1] + +proc parseFile*(path, src: string, pr: var ParseResult) = + var stack: seq[Ctx] = @[] + for ll in logicalLines(src): + while stack.len > 0 and ll.indent <= stack[^1].indent: + discard stack.pop() + + let text = ll.text + let w = text.words() + if w.len == 0: continue + let opensBlock = text.endsWith(":") + let inExtern = stack.anyIt(it.kind == ckExtern) + + # `cdef extern from "libavutil/x.h" nogil:` + if w.len >= 3 and w[0] == "cdef" and w[1] == "extern": + stack.add Ctx(kind: ckExtern, indent: ll.indent, agg: -1) + continue + + var head = w + var packed = false + if head[0] == "ctypedef" or head[0] == "cdef" or head[0] == "cpdef": + head = head[1 .. ^1] + if head.len > 0 and head[0] == "api": + head = head[1 .. ^1] + if head.len > 0 and head[0] == "packed": + packed = true + head = head[1 .. ^1] + if head.len == 0: continue + + case head[0] + of "class": + let name = headerName(head, 1) + if name.len == 0: continue + pr.classes.incl name + pr.aggs.add Aggregate(kind: akClass, name: name, base: baseName(text), + isExtern: inExtern, hasBody: opensBlock, file: path, + line: ll.line) + if opensBlock: + stack.add Ctx(kind: ckAgg, indent: ll.indent, agg: pr.aggs.high) + continue + of "struct", "union": + let name = headerName(head, 1) + if name.len == 0: continue + let kind = if head[0] == "struct": akStruct else: akUnion + pr.aggs.add Aggregate(kind: kind, name: name, packed: packed, + isExtern: inExtern, hasBody: opensBlock, file: path, + line: ll.line) + if opensBlock: + stack.add Ctx(kind: ckAgg, indent: ll.indent, agg: pr.aggs.high) + continue + of "enum": + let name = headerName(head, 1) + if name.len > 0: pr.enums.incl name + if opensBlock: + stack.add Ctx(kind: ckOther, indent: ll.indent, agg: -1) + continue + else: + discard + + # `ctypedef void (*Deleter)(Foo*) nogil` + if w[0] == "ctypedef" and text.contains("(*"): + let open = text.find("(*") + var i = open + 2 + var name = "" + while i < text.len and (text[i].isAlphaNumeric or text[i] == '_'): + name.add text[i] + inc i + if name.len > 0: pr.aliases[name] = "void *" + continue + + # `ctypedef int64_t Timestamp` + if w[0] == "ctypedef" and not opensBlock and w.len >= 3 and not text.contains("("): + let toks = tokenize(text[w[0].len .. ^1]) + if toks.len >= 2 and toks[^1] notin ["*", ","]: + let alias = toks[^1] + var aliased = toks[0 ..< toks.high].filterIt(it != "*").join(" ") + if toks.anyIt(it == "*"): aliased = "void *" + pr.aliases[alias] = aliased + continue + + if opensBlock: + stack.add Ctx(kind: ckOther, indent: ll.indent, agg: -1) + continue + + if stack.len == 0 or stack[^1].kind != ckAgg: continue + let ai = stack[^1].agg + let isClass = pr.aggs[ai].kind == akClass + + var decl = text + if isClass: + if w[0] notin ["cdef", "cpdef"]: continue + decl = text[w[0].len .. ^1] + if decl.strip() == "pass": continue + # Bitfields change the layout rules; refuse to guess rather than be wrong. + if decl.contains(':'): + pr.aggs[ai].fields.add FieldDecl(name: decl.strip(), typeName: "", + display: text.strip(), arrayLen: 1, line: ll.line) + continue + # Methods and function declarations; `(*name)(...)` is a function pointer field. + if decl.contains('(') and not decl.contains("(*"): + # A cdef/cpdef method puts the class in a vtable; `def` methods do not. + if isClass and w[0] in ["cdef", "cpdef"]: + pr.aggs[ai].hasCMethods = true + continue + if decl.contains("(*"): + let open = decl.find("(*") + var i = open + 2 + var name = "" + while i < decl.len and (decl[i].isAlphaNumeric or decl[i] == '_'): + name.add decl[i] + inc i + if name.len > 0: + pr.aggs[ai].fields.add FieldDecl(name: name, typeName: "void", + mods: "", display: text.strip(), ptrDepth: 1, arrayLen: 1, line: ll.line) + continue + + for f in parseMember(decl, text, isClass, ll.line): + pr.aggs[ai].fields.add f diff --git a/tools/pxdpad/tests/test_pxdpad.nim b/tools/pxdpad/tests/test_pxdpad.nim new file mode 100644 index 000000000..ddda11363 --- /dev/null +++ b/tools/pxdpad/tests/test_pxdpad.nim @@ -0,0 +1,423 @@ +import std/[unittest, tables, strutils] +import model, abi, parser, layout + +proc analyzeMulti(src: string, targetNames: seq[string]): Table[string, Report] = + var pr = ParseResult() + parseFile("test.pxd", src, pr) + var resolvers: seq[Resolver] = @[] + for name in targetNames: + resolvers.add newResolver(pr, getTarget(name)) + for a in pr.aggs: + if a.hasBody: + result[a.name] = analyzeAll(resolvers, a) + +proc analyze(src: string, targetName = "lp64"): Table[string, Finding] = + var pr = ParseResult() + parseFile("test.pxd", src, pr) + let r = newResolver(pr, getTarget(targetName)) + for a in pr.aggs: + if a.hasBody: + result[a.name] = r.analyze(a) + +suite "struct layout": + test "padding between and after members": + let f = analyze(""" +cdef struct S: + char a + int b + char c +""")["S"] + check f.layout.size == 12 + check f.optimalSize == 8 + check f.wasted == 4 + check f.layout.holes.len == 2 + check f.layout.holes[0].offset == 1 + check f.layout.holes[0].size == 3 + check f.layout.holes[0].after == "a" + check f.layout.holes[1].tail + check f.order[0].name == "b" + + test "already optimal structs report nothing": + let f = analyze(""" +cdef struct S: + int b + char a + char c +""")["S"] + check f.layout.size == 8 + check f.wasted == 0 + + test "packed structs are never reordered": + let f = analyze(""" +cdef packed struct S: + char a + int b +""")["S"] + check f.layout.size == 5 + check f.wasted == 0 + + test "unions take the widest member": + let f = analyze(""" +cdef union U: + char a + double b + int c +""")["U"] + check f.layout.size == 8 + check f.layout.align == 8 + check f.wasted == 0 + + test "arrays and nested structs": + let f = analyze(""" +cdef struct Inner: + double d + char c + +cdef struct S: + char flags[3] + Inner inner + int counts[4] +""") + check f["Inner"].layout.size == 16 + check f["S"].layout.size == 40 # 3 +5 pad, 16, 16 + check f["S"].optimalSize == 40 + check f["S"].layout.fields[2].size == 16 + + test "unknown extents are not guessed": + let f = analyze(""" +cdef struct S: + int n + char buf[MAX] +""")["S"] + check not f.layout.complete + check f.layout.unresolved[0].startsWith("buf") + +suite "cdef class layout": + test "PyObject_HEAD only, no vtable without cdef methods": + let f = analyze(""" +cdef class C: + cdef int a + cdef void *p + cdef int b +""")["C"] + check f.layout.baseSize == 16 + check not f.layout.vtab + check f.layout.size == 40 # a, 4 pad, p, b, 4 tail + check f.optimalSize == 32 # p, a, b + check f.wasted == 8 + + test "a cdef method adds the hidden vtable pointer": + let f = analyze(""" +cdef class C: + cdef int a + cdef void run(self) +""")["C"] + check f.layout.vtab + check f.layout.baseSize == 24 + check f.layout.size == 32 + + test "subclasses inherit the vtable slot, not a second one": + let f = analyze(""" +cdef class Base: + cdef void *p + cdef void run(self) + +cdef class Sub(Base): + cdef int x + cdef void go(self) +""") + check f["Base"].layout.size == 32 # head + vtab + p + check f["Sub"].layout.baseSize == 32 + check not f["Sub"].layout.vtab + check f["Sub"].layout.size == 40 + + test "multi-line method declarations are not fields": + let f = analyze(""" +cdef class C: + cdef int a + cdef void go( + self, int x, int y + ) + cdef int b +""")["C"] + check f.layout.fields.len == 2 + check f.layout.vtab + + test "docstrings and attribute docstrings are skipped": + let f = analyze(""" +cdef class C: + ''' + cdef int not_a_field + ''' + cdef int a + '''The a value.''' + cdef int b +""")["C"] + check f.layout.fields.len == 2 + check f.layout.size == 24 + + test "an unknown base class is reported, not assumed": + let f = analyze(""" +cdef class C(SomethingElse): + cdef int a +""")["C"] + check not f.layout.complete + check f.layout.unresolved[0].contains("SomethingElse") + +suite "declaration parsing": + test "one line, several fields": + let f = analyze(""" +cdef class C: + cdef unsigned int width, height + cdef char *a, *b +""")["C"] + check f.layout.fields.len == 4 + check f.layout.fields[0].size == 4 + check f.layout.fields[2].size == 8 + check f.layout.fields[2].decl.ptrDepth == 1 + + test "pointer stars bind either way": + let f = analyze(""" +cdef class C: + cdef lib.AVFrame* a + cdef lib.AVFrame *b + cdef const lib.AVCodec *c +""")["C"] + check f.layout.size == 40 + for lf in f.layout.fields: check lf.size == 8 + + test "python containers and generics are one pointer": + let f = analyze(""" +cdef class C: + cdef dict[str, int] a + cdef readonly list[Stream] b + cdef public tuple c +""")["C"] + check f.layout.size == 40 + check f.layout.fields[0].decl.typeName == "dict" + check f.layout.fields[1].decl.mods == "readonly " + + test "a qualified C struct is not confused with a class of the same name": + # av/rational.pxd declares `cdef class AVRational` next to lib.AVRational. + let f = analyze(""" +cdef class AVRational: + cdef readonly int num + cdef readonly int den + +cdef struct S: + int a + lib.AVRational q + +cdef class C: + cdef int a + cdef lib.AVRational q + cdef AVRational obj +""") + check f["S"].layout.size == 12 # not 16: the struct aligns to 4 + check f["S"].layout.align == 4 + check f["S"].layout.fields[1].offset == 4 + check f["C"].layout.fields[1].align == 4 + check f["C"].layout.fields[1].offset == 20 + check f["C"].layout.fields[2].align == 8 # the class, one pointer + check f["C"].layout.fields[2].size == 8 + + test "an untyped class attribute is an object": + let f = analyze(""" +cdef class C: + cdef readonly planes + cdef void *p +""")["C"] + check f.layout.fields.len == 2 + check f.layout.fields[0].decl.name == "planes" + check f.layout.fields[0].decl.typeName == "object" + check f.layout.size == 32 + + test "a field may be named like a keyword": + let f = analyze(""" +cdef class C: + cdef lib.AVRational struct +""")["C"] + check f.layout.fields.len == 1 + check f.layout.fields[0].decl.name == "struct" + check f.layout.fields[0].size == 8 + + test "enums declared in extern blocks size as int": + let f = analyze(""" +cdef extern from "libavutil/pixfmt.h": + enum AVPixelFormat: + AV_PIX_FMT_NONE + +cdef class C: + cdef AVPixelFormat fmt + cdef void *p +""")["C"] + check f.layout.size == 32 + check f.optimalSize == 32 + + test "extern structs are not sized from partial declarations": + let f = analyze(""" +cdef extern from "x.h": + ctypedef struct AVCodecContext: + int width + +cdef class C: + cdef AVCodecContext ctx +""")["C"] + check not f.layout.complete + + test "function pointer typedefs and members are pointers": + let f = analyze(""" +ctypedef void (*Deleter)(void *) noexcept nogil + +cdef struct S: + Deleter d + void (*cb)(int) + int n +""")["S"] + check f.layout.size == 24 + check f.layout.fields[0].size == 8 + check f.layout.fields[1].size == 8 + + test "typedef aliases resolve": + let f = analyze(""" +ctypedef int64_t Timestamp + +cdef struct S: + Timestamp t + int n +""")["S"] + check f.layout.size == 16 + +suite "targets": + test "long is 4 bytes on llp64": + let src = """ +cdef struct S: + long a + int b +""" + check analyze(src, "lp64")["S"].layout.size == 16 + check analyze(src, "llp64")["S"].layout.size == 8 + + test "ilp32 shrinks pointers and the object header": + let f = analyze(""" +cdef class C: + cdef void *p + cdef int n +""", "ilp32")["C"] + check f.layout.baseSize == 8 + check f.layout.size == 16 + +suite "regression: PyAV declarations": + test "HWAccel matches the compiled extension": + let f = analyze(""" +cdef class HWAccel: + cdef str _device + cdef readonly Codec codec + cdef readonly HWConfig config + cdef lib.AVBufferRef *ptr + cdef public dict options + cdef int _device_type + cdef readonly int device_id + cdef public int flags + cdef readonly bint is_hw_owned + cdef public bint allow_software_fallback + +cdef class Codec: + cdef const lib.AVCodec *ptr + +cdef class HWConfig: + cdef object __weakref__ + cdef const lib.AVCodecHWConfig *ptr + cdef void _init(self, const lib.AVCodecHWConfig *ptr) +""") + check f["HWAccel"].layout.size == 80 + check f["HWAccel"].wasted == 0 + check f["HWConfig"].layout.size == 40 # head + vtab + __weakref__ + ptr + check f["HWConfig"].layout.vtab + + test "the suggested order really is smaller": + let f = analyze(""" +cdef class C: + cdef char tag + cdef void *p + cdef int n + cdef double d + cdef bint on +""")["C"] + check f.layout.size == 56 # tag, 7 pad, p, n, 4 pad, d, on, 4 tail + check f.optimalSize == 48 # p, d, n, on, tag, 7 tail + check f.wasted == 8 + check f.order[0].name == "p" + check f.order[^1].name == "tag" + +suite "across ABIs": + test "a layout clean on lp64 can have a hole on llp64": + # `long` is 8 bytes on lp64 and 4 on 64-bit Windows, so this packs on one + # and leaves a hole on the other. Checking only the host would miss it. + let r = analyzeMulti(""" +cdef struct S: + long a + void *p + long b +""", @["lp64", "llp64"])["S"] + check r.results[0].layout.size == 24 + check r.results[0].wasted == 0 + check r.results[1].layout.size == 24 + check r.results[1].wasted == 8 + check r.worstWaste == 8 + check r.worstTarget == "llp64" + check r.changed + check r.order[0].name == "p" + + test "waste on one ABI is reported even when another is fine": + let r = analyzeMulti(""" +cdef struct S: + int a + void *p + long b + int c +""", @["lp64", "llp64"])["S"] + check r.results[0].wasted == 8 # lp64: 32 -> 24 + check r.results[1].wasted == 0 # llp64: already 24 + check r.worstTarget == "lp64" + + test "the suggested order never costs an ABI more than it had": + let r = analyzeMulti(""" +cdef struct S: + char tag + long a + void *p + double d + int n +""", @["lp64", "darwin-arm64", "llp64", "ilp32"])["S"] + for t in r.results: + check t.suggested <= t.layout.size + + test "a declaration that is already best everywhere suggests nothing": + let r = analyzeMulti(""" +cdef struct S: + void *p + int a + int b +""", @["lp64", "llp64", "ilp32"])["S"] + check not r.changed + check r.worstWaste == 0 + + test "ABI-defined structs are sized per target, not from a table": + # Py_buffer is 7 pointers, 2 Py_ssize_t and 2 int. + let r = analyzeMulti(""" +cdef class C: + cdef Py_buffer view +""", @["lp64", "ilp32"])["C"] + check r.results[0].layout.size == 96 # 16 head + 80 + check r.results[1].layout.size == 52 # 8 head + 44 + + test "unions and packed structs are never reordered": + let r = analyzeMulti(""" +cdef packed struct S: + char a + int b + char c +""", @["lp64", "llp64"])["S"] + check not r.changed + check r.results[0].layout.size == 6 From bee8540d81fcfd3d56b88526c319cb7292706c21 Mon Sep 17 00:00:00 2001 From: WyattBlue Date: Wed, 2 Sep 2026 01:32:30 -0400 Subject: [PATCH 2/3] Remove padding from HWDevice --- av/codec/hwaccel.pxd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/av/codec/hwaccel.pxd b/av/codec/hwaccel.pxd index c6aaa15b4..9764d9414 100644 --- a/av/codec/hwaccel.pxd +++ b/av/codec/hwaccel.pxd @@ -11,9 +11,9 @@ cdef class HWConfig: cdef HWConfig wrap_hwconfig(const lib.AVCodecHWConfig *ptr) cdef class HWDevice: - cdef int _device_type cdef lib.AVBufferRef *ptr cdef readonly dict options + cdef int _device_type cdef readonly int flags cdef class HWAccel: From 01a81e6d45e4b8a208705b2f447326d3cb73af35 Mon Sep 17 00:00:00 2001 From: WyattBlue Date: Wed, 2 Sep 2026 01:44:03 -0400 Subject: [PATCH 3/3] Add armv7 abi in pxdpad --- tools/pxdpad/src/abi.nim | 13 ++++++------- tools/pxdpad/tests/test_pxdpad.nim | 12 ++++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/tools/pxdpad/src/abi.nim b/tools/pxdpad/src/abi.nim index c2998cd61..21371022e 100644 --- a/tools/pxdpad/src/abi.nim +++ b/tools/pxdpad/src/abi.nim @@ -4,7 +4,7 @@ import std/[strutils, sets] import model const - targetNames* = ["lp64", "darwin-arm64", "llp64", "ilp32"] + targetNames* = ["lp64", "darwin-arm64", "llp64", "armv7", "ilp32"] proc getTarget*(name: string): Target = ## `lp64` is the x86-64 System V ABI, `llp64` is 64-bit Windows. @@ -18,6 +18,11 @@ proc getTarget*(name: string): Target = of "llp64", "windows", "win64": Target(name: "llp64", ptrSize: 8, ptrAlign: 8, longSize: 4, longAlign: 4, ldSize: 8, ldAlign: 8, maxScalarAlign: 8, headSize: 16) + of "armv7", "armhf", "armv7l": # 32-bit ARM, arm-linux-gnueabihf + # ILP32 like i386, but AAPCS aligns double and long long to 8, and + # long double is just double. + Target(name: "armv7", ptrSize: 4, ptrAlign: 4, longSize: 4, longAlign: 4, + ldSize: 8, ldAlign: 8, maxScalarAlign: 8, headSize: 8) of "ilp32", "x86": # 32-bit x86 System V Target(name: "ilp32", ptrSize: 4, ptrAlign: 4, longSize: 4, longAlign: 4, ldSize: 12, ldAlign: 4, maxScalarAlign: 4, headSize: 8) @@ -25,12 +30,6 @@ proc getTarget*(name: string): Target = raise newException(ValueError, "unknown target '" & name & "', expected one of " & targetNames.join(", ")) -proc defaultTarget*(): Target = - when defined(windows): getTarget("llp64") - elif defined(macosx) and defined(arm64): getTarget("darwin-arm64") - elif sizeof(pointer) == 4: getTarget("ilp32") - else: getTarget("lp64") - const pyObjectTypes* = toHashSet([ "object", "str", "bytes", "unicode", "bytearray", "dict", "list", "tuple", "set", "frozenset", "type", "slice", "complex", "BaseException", "Exception", diff --git a/tools/pxdpad/tests/test_pxdpad.nim b/tools/pxdpad/tests/test_pxdpad.nim index ddda11363..d3fd77d6f 100644 --- a/tools/pxdpad/tests/test_pxdpad.nim +++ b/tools/pxdpad/tests/test_pxdpad.nim @@ -298,6 +298,18 @@ cdef struct S: check analyze(src, "lp64")["S"].layout.size == 16 check analyze(src, "llp64")["S"].layout.size == 8 + test "armv7 is ILP32 but aligns double and long long to 8": + let src = """ +cdef struct S: + int a + double d +""" + # i386 packs d at offset 4; AAPCS pads to 8. + check analyze(src, "ilp32")["S"].layout.size == 12 + check analyze(src, "armv7")["S"].layout.size == 16 + check analyze(src, "armv7")["S"].layout.fields[1].offset == 8 + check analyze(src, "armv7")["S"].layout.holes.len == 1 + test "ilp32 shrinks pointers and the object header": let f = analyze(""" cdef class C: