X++: add extractor and QL library generated from the compiler's own AST - #392
Draft
Giulia Stocco (gfs) wants to merge 8 commits into
Draft
X++: add extractor and QL library generated from the compiler's own AST#392Giulia Stocco (gfs) wants to merge 8 commits into
Giulia Stocco (gfs) wants to merge 8 commits into
Conversation
Adds the first stage of X++ CodeQL support: a schema generated directly from the X++ compiler's own AST hierarchy, rather than a hand-maintained mapping. Xpp.SchemaGenerator reflects over the 243 types deriving from Microsoft.Dynamics.AX.Metadata.XppCompiler.Ast in Microsoft.Dynamics.AX.Framework.Xlnt.XppCore.dll, using MetadataLoadContext so no code from the proprietary compiler package is executed. It emits xpp/schema/ast.py, which misc/codegen turns into the dbscheme, the QL classes and their hand-editable stubs. The generator resolves several mismatches between the CLR hierarchy and the schema: - CLR tuples (try/catch pairs, switch cases, call arguments) have no schema equivalent, so a named class is synthesized per tuple-valued property. - Dictionaries of AST values are represented by their values, since the key is always the value's own name. - Subclasses that re-declare an inherited property are collapsed onto the ancestor's declaration, which would otherwise emit conflicting QL predicates. - HasX booleans that duplicate the hasX() predicate generated for an optional X are dropped. - Table names that would collide with a class table get an explicit override. The compiler package is proprietary and is not committed. Point XPP_COMPILER_PACKAGE at an extracted copy and run xpp/tools/generate-schema.sh; the generated output is committed so the QL layer can be worked on without it. Verified: generation is deterministic across runs and the full generated library compiles against the dbscheme. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1f6fb880-dfd8-4fba-91a2-903f43dc287d
Options were scanned from index 1 to length-1 and the `type` command read its operand from a fixed position, so a type name followed by `--package` was shadowed by the option itself. Parse options and positionals separately. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1f6fb880-dfd8-4fba-91a2-903f43dc287d
Adds the second stage of X++ support: a generated TRAP writer, and an extractor that turns a D365 F&O metadata object into a CodeQL database. The TRAP writer is generated from the same reflected AST model as the schema rather than from a Python generator reading schema/ast.py. The schema keeps only snake_case names and loses the CLR binding needed to read a property back off a node, so the generator has to be where the model already lives. Relation names, however, are produced by inflection.tableize inside misc/codegen, so the generator reimplements those naming rules and validates every relation it emits against the committed dbscheme. A divergence fails generation instead of silently dropping tuples, and that check already caught two naming bugs: plural rules applied in the wrong order, and predicate tables using underscore rather than tableize. Two further mismatches between the CLR hierarchy and the dbscheme surfaced: - A class that is instantiable and also extended gets only a union in the dbscheme, leaving instances of exactly that class with nowhere to bind. Seven X++ classes are in that position, so the generator emits an `...Internal` leaf for each, as the Rust schema does. - Open generic AST classes cannot be named in generated code, and a closed instance's runtime name would not match a dispatch case. They are treated as abstract and their members bound through `dynamic`. Extraction reads the X++ out of the CDATA blocks in an object's XML and passes each block's line offset to the parser, so positions come back relative to the file rather than the fragment. Method bodies parse without metadata; class and interface headers need a provider to resolve `extends`, so an empty one is supplied through DispatchProxy rather than hand-writing eighty-odd members. The compiler package stays out of build output and is resolved at run time from XPP_COMPILER_PACKAGE. Verified end to end. Three AxClass objects from TrudAX/XppTools yield 63 source blocks and 2181 nodes with no unsupported nodes and no errors; the resulting TRAP imports cleanly and queries return the expected method names and counts. A fixture covering `while select`, ttsbegin/ttscommit/ttsabort, try/catch and changecompany confirms the X++-specific constructs reach the database. Regeneration remains deterministic. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1f6fb880-dfd8-4fba-91a2-903f43dc287d
`codeql database create --language=xpp` now produces a queryable database. Adds the tool scripts the CLI invokes and a script that assembles the extractor pack. X++ has no build step, so as with the PowerShell extractor the work happens in autobuild rather than in a build wrapper. The extractor now understands the environment the CLI sets up: it reads `--file-list`, writes one TRAP file per source file under CODEQL_EXTRACTOR_XPP_TRAP_DIR, and copies each object into CODEQL_EXTRACTOR_XPP_SOURCE_ARCHIVE_DIR so alerts have something to display. Passing `--trap` or leaving the environment unset keeps the single-stream behaviour that is convenient outside the CLI. Also stops generating the per-node-type test placeholders. They were 209 empty MISSING_SOURCE.txt markers with no value until real fixtures exist, and per-node-type generated tests are the wrong shape for this extractor anyway. The already-committed placeholder files still need removing separately. Verified with `codeql database create --language=xpp` over four AxClass objects: 65 source blocks, 2224 nodes, no unsupported nodes and no errors, TRAP imported and source archive written. Queries against the resulting database return the expected counts, including `while select`, ttsbegin and try/catch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1f6fb880-dfd8-4fba-91a2-903f43dc287d
Adds the first QL test, covering the class and its methods, the control-flow statements, and the database statements that have no equivalent in other languages. Writing it immediately found a bug. The compiler groups some children in CLR tuples, such as a `catch` with its handler body and a switch case with its statements. The tree walk only followed properties that were an AST node or a sequence of them, so it stepped over those tuples and never visited the subtrees inside. A `ttsabort` in a catch block was extracted as zero statements. Traversal now looks through tuple slots, and the test pins the counts for both shapes. Extraction during `codeql test run` uses the qltest protocol, which needs `legacy_qltest_extraction` in the extractor config. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1f6fb880-dfd8-4fba-91a2-903f43dc287d
The check-implicit-this workflow requires the property on every qlpack.yml, and the newly added test pack was missing it. The library pack already had it. Enabling it produces no warnings in the generated or hand-written QL, and the library test still passes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1f6fb880-dfd8-4fba-91a2-903f43dc287d
These were 209 empty MISSING_SOURCE.txt markers, one per AST node type, generated before any fixtures existed. codegen.conf no longer produces them. The hand-written library test under xpp/ql/test/library-tests covers this ground far better - writing it found a real traversal bug that the generated placeholders never would have.
All five reported issues reproduced, so all five are fixed. Dictionary-held children were dropped. The compiler keeps some children in dictionaries keyed by name, such as a class's fields and methods. Iterating one as a bare IEnumerable yields KeyValuePair rather than the child, so the emitter labelled the pair and the tree walk, seeing neither an Ast nor a tuple, skipped the subtree entirely. Both now go through AstSequence, which reduces entries to their values. A class with two fields extracted zero FieldDeclarations before and two after. Source positions were wrong on CRLF files. The line offset was found by searching the raw file for the CDATA payload, but an XML parser normalises CRLF to LF inside element values, so on the Windows-authored files that F&O actually produces the search never matched and every multiline block silently fell back to offset zero. Offsets now come from XML line information. That fix was unverifiable at first because locations were never written at all, so this also emits them: `Ast.Position` supplies the extent for each node. Nodes the parser synthesises have no extent and are left without a location rather than being pinned to the top of the file. Normal indexing processed every XML file. The extractor declares the .xml extension, so the CLI's file list covers the whole tree; entries were used unfiltered, letting an unrelated or malformed XML file fail an otherwise valid build. File-list entries are now filtered the same way directory walks are. Pack creation could delete the source tree. Passing the repository root made the target `<repo>/xpp` and the script removed it. It now refuses to build into the source tree, and only removes a directory that is itself a generated pack. Combined TRAP output reused labels. A writer was created per input while sharing one stream, so each restarted at #1 and collided. One writer is now created per stream. Extracting three files to a single file yields 5943 label definitions, all distinct and monotonic. Removing the codegen test output also turned out to break regeneration, since qlgen requires the path. It now points at a gitignored scratch directory. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1f6fb880-dfd8-4fba-91a2-903f43dc287d
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
CodeQL has no support for X++, the language behind Dynamics 365 Finance & Operations. The only existing option is running the C# extractor over compiled X++, which captures CIL semantics but loses everything X++-specific:
selectstatements,ttsbeginblocks, tables, forms, chain-of-command. Those are exactly where the interesting security questions live.This adds a first-class X++ extractor. The guiding decision is that the dbscheme and QL classes are generated 1:1 from the X++ compiler's own AST hierarchy, so there is no hand-maintained mapping to keep in sync and no second-guessing the first-party model.
About the file count
Most of this diff is generated and marked
linguist-generated, so it collapses in review. For calibration,xpp/ql/libis about 1000 files;rust/ql/libis 1019 andswift/ql/libis 1578. Per schema class we are slightly leaner than rust. The stubs cannot be dropped without patchingmisc/codegen, which is shared with rust and swift, because generated classes importXImpl::ImplandconstructX.The genuinely hand-written surface is about 20 files, all under
xpp/extractor/**(excludingGenerated/), plusxpp/schema/prelude.py,xpp/codegen.conf,xpp/codeql-extractor.ymlandxpp/tools/.One cleanup is still outstanding:
xpp/ql/test/extractor-tests/holds 209 emptyMISSING_SOURCE.txtplaceholders, one per node type, generated before any fixtures existed.codegen.confno longer produces them, but the committed files still need deleting:Approach
Xpp.SchemaGeneratorreflects over the 243 types deriving fromMicrosoft.Dynamics.AX.Metadata.XppCompiler.Ast, usingMetadataLoadContextso no code from the proprietary compiler package is executed. The resultingschema/ast.pyfeedsmisc/codegen, the same suite Swift and Rust use.Extraction calls
Pass1, the compiler's own parser, in process. There is no reimplemented grammar anywhere in this change.Reconciling the CLR hierarchy with the dbscheme
Most of the interesting work was in the mismatches between what the compiler's class model expresses and what a dbscheme can:
catchwith its handler, a switch case with its body, a call argument with its by-ref flag). The schema has no tuple type, so the generator synthesizes a named class per tuple-valued property.HasXbooleans that duplicate thehasX()predicate generated for an optionalXare dropped....Internalleaf for each, following the Rust schema.dynamic.Five properties remain unmapped, all reasonably so: two open generic parameters, two
object-typed accessors that have typed equivalents on subclasses, and oneSystem.Type.The TRAP writer is checked against the dbscheme
The TRAP writer is generated from the reflected model rather than from
schema/ast.py, because the schema keeps only snake_case names and loses the CLR binding needed to read a property back off a node. Relation names, though, come frominflection.tableizeinsidemisc/codegen, so the generator reimplements those rules and then validates every relation it emits against the committed dbscheme. A divergence fails generation rather than silently producing a database with missing tuples.That check earned its keep immediately, catching two naming bugs: plural rules applied in the wrong order, and predicate tables using
underscorerather thantableize.Verification
codeql database create --language=xppover fourAxClassobjects: 65 source blocks, 2224 nodes, no unsupported nodes, no errors. TRAP imported, source archive written, and queries return the expected counts.ttsabortinside acatchextracted as zero statements. Traversal now looks through tuple slots and the test pins counts for both tuple shapes.AxClassobjects from the MIT-licensedTrudAX/XppToolsparse to 2181 nodes with no unsupported nodes and no errors.Worth noting:
Pass1runs standalone on .NET 8 including macOS, so the extractor is not tied to Windows. Method bodies parse with no metadata provider; only class and interface headers need one to resolveextends, and an empty one supplied throughDispatchProxyis sufficient.Notes for reviewers
XPP_COMPILER_PACKAGE. The generated schema, dbscheme and QL are committed so the QL layer can be worked on without it.misc/codegenrequires Python 3.13 or earlier. On 3.14, PEP 649 lazy annotations causecls.__dict__["__annotations__"]to come back empty and every property is silently dropped from the schema.ql/lib/codeql/xpp/elements/internal/ElementImpl.qllis hand-edited to supply the defaulttoStringImpl. Its "generated" marker comment has been removed deliberately; leave it off, and do not delete the generated tree wholesale when regenerating.Not included
Still to come: the base QL layer over the
shared/packs (CFG, SSA, dataflow), Concepts and MaD models, CI and packaging wiring, and the security queries themselves.