Repository navigation
feat: assembler expressions, NASM/MASM/GAS dialect gaps, and Motorola 68000 syntax - #41
Open
AtomicGaryBusey wants to merge 1 commit into
Open
AtomicGaryBusey wants to merge 1 commit into
AtomicGaryBusey wants to merge 1 commit into
Conversation
…yntax
Parsing real-world x86 sources surfaced a set of constructs the grammar could
not represent. Across four projects — a NASM kernel, GAS CPU-errata probes, and
the Quake 2 software renderer, which ships the *same* routines in both MASM and
GAS — every file failed to parse. All now parse; the MASM/GAS twins let each
fix be checked against a known-good counterpart.
Expressions and literals:
- `expr`: assembler-time arithmetic — `-(A + B)`, `(end - start) / 4`,
`511*8`, `sym+8`. Precedence mirrors the C operators an assembler accepts.
This was the single largest cause of failures; operands are everywhere.
- `int`: MASM radix-suffix literals — `040000000h`, `1010b`, `777o`, `42d`.
MASM writes hex with a trailing `h` and a leading digit, not `0x`.
- `address`: bare `$` and `$$` (NASM here / section start).
Directives and declarations:
- `meta_ident`: `/\.[a-z_]+/` could match neither a digit nor a trailing
uppercase letter, so `.code32` and MASM's `.386P` were unparseable. Widened
to a lowercase-or-digit start; an *uppercase* start is deliberately still
excluded so GAS local labels (`.L5`) stay plain idents — the existing
"Dotted label arg" test covers exactly that.
- `meta`: operands were homogeneous lists (all int, or all string, or one
ident), so `.lcomm env, 32` was rejected. Now a list of expressions, which
also admits NASM's `.msg_len equ $ - .msg`.
- `masm_data`: new rule. MASM names data with no colon
(`reciprocal_table_16 dd 040000000h, ...`), which is neither a `label` nor an
`instruction` — the latter's operands are comma-separated, and there is no
comma between the name and the directive.
- `preproc`: new rule for NASM preprocessor lines. The keyword list is
explicit so a GAS register (`%eax`) can never be mistaken for one.
- `label`: an instruction may follow on the same line (`sym: resq 1`).
Motorola 68000 (vasm / Devpac / ASM-One), which Amiga and Genesis sources use:
- `word`: a trailing `.b`/`.w`/`.l`/`.s` is a size suffix on the mnemonic, not a
new token — `bsr.w`, `bra.s`, `move.l`, `dc.b`. Without this every 68000
instruction with a suffix produced an ERROR node and the mnemonic was lost,
which is most instructions in a real source file.
- `size_suffix`: 68000 absolute addressing writes the size AFTER the operand —
`jsr (Name).l`. A dedicated token, lexed ahead of `meta_ident`, so `.l` binds
to the operand while `.loop` stays a local label (longest match).
Memory operands:
- `ptr`: MASM segment override with a size prefix
(`mov esi, ds:dword ptr[_bbextents]`) — the largest remaining cause after
expressions, and one `far_ptr` would otherwise mis-claim as `ds:dword`.
Also NASM `[rel sym]`, a size prefix without the MASM `ptr` keyword,
arithmetic inside the brackets, and a symbol or register before the paren
for GAS RIP-relative `ltab(%rip)` and `%st(0)`.
- `far_ptr`: `jmp 0x08:long_start`.
- `ptr`, 68000 addressing modes. Validated against a 26,000-line vasm source,
where these five shapes were every remaining parse error — 834 ERROR nodes on
761 lines, now zero:
- post-increment `(a2)+` and pre-decrement `-(sp)`;
- indexed with a size on the index register — `2(a2,d0.w)`, and
`Table(pc,d0.w)`, which is how a 68000 program dispatches a jump table;
- a displacement that is itself an expression — `Handler+$10E(pc,d0.w)`;
- an absolute address written with a size and no parentheses, which is how
these sources reach hardware and scratch RAM — `lea $ffffdb58.w,a2`;
- an absolute address computed at assembly time — `lea (Table+2).w,a4`,
`lea (Data&$FFFFFF).l,a6`, carried by an optional `size_suffix` on
`expr`'s parenthesised form rather than a new alternative, so a bare
`(sym)` keeps parsing as the expression it was.
The post-increment `+` is admitted ONLY when the parenthesis has no
displacement in front of it. `d(An)+` is not a 68000 mode, while GAS's
`C(sym)+4` — the `C()` macro plus a struct offset — is everywhere in the
Quake sources, and taking that `+` as post-increment regressed five files
that had parsed cleanly before.
And one fix that is not about a dialect at all:
- `program`: allow leading newlines. `sep()` required an item before the first
separator, so a file opening with a comment line — which is most files —
forced the parser to insert a MISSING node. This made
`tree.rootNode.hasError` true for essentially every real-world sample even
when nothing else was wrong, which masks genuine errors and breaks any tool
that gates on `hasError`.
Amiga/Devpac forms, validated on 2,087 real files (1.28M lines: the AmigaOS
NDK 3.2 include set, ASM-One/Devpac sources and hand-written 68000 programs).
Error nodes across that corpus: 49,676 -> 1,253, and 1,892 of 2,087 files now
parse with none at all.
- `star_comment`: a `*` in column 0 is a comment, and banner lines of `*` are
the house style. An `_item` rather than an `extras` token, so it only matches
where a line can begin — `511*8` stays multiplication. Without it the comment
TEXT parsed as instructions and operands.
- `address`: Motorola `*` (the current address, `bra *-$10`) and a bare `=`
(`_LVOOpen = -$1e`, the Amiga library vector-offset idiom).
- `immediate`: `#` followed by an expression, a character literal or a string —
`#Sym+4`, `#(SIZEX+SIZEY+2)/4-1`, `#'0'`, `#"ACAP"`. A bare `#42` is still an
`int`, whose `#` form takes an immediate literal token none of these match, so
the two never overlap.
- `expr` / `tc_infix`: `!` is bitwise OR in the Motorola assemblers
(`EQU (CMD_WRITE!TDF_EXTCOM)`), and the space-separated operand path needed
`<<`/`>>` as well (`ADIOF_PERVOL EQU 1<<4`).
- `paren_expr`: a space-separated operand that is parenthesised —
`Label equ (Other+4)`. Only the parenthesised form; a full `expr` there makes
`word _tc_expr '-'` ambiguous with `tc_infix`.
- `angle_arg`: `<(PMB_AWM+7)/8>`, how the NDK passes a computed size to a macro.
- `bitfield`: the 68020 `{offset:width}` operand (`bfextu d0{d1:4},d2`). The two
parts are a register or a literal, never a general expression, which keeps the
`:` from colliding with `far_ptr`.
- `ptr`: the 68020 scaled index — `8(a0,d1.w*4)`.
- `macro_arg`: macro parameter substitution inside a macro body (`\1`, `\@`),
part of `reg` so it works wherever an operand or base register can appear.
- `label`: a label declared through a macro — `C(d_zistepv): .single 0`, how the
GAS side of a hand-written x86 renderer names every symbol so one source
serves platforms that differ on the leading underscore. The symbol is the
macro's ARGUMENT, so it is a `name` field. On Quake 2's MASM/GAS twins this
took `r_varsa.s` from 49 extracted symbols to 69, against the MASM twin's 70.
- `extras`: form feed and vertical tab are whitespace (page breaks in old
sources).
Two tokens are deliberately `token.immediate`, because only the absence of a
space tells the forms apart: the displaced parenthesis in `d(An)` (so
`Label equ (Other+4)` is not addressing) and the AArch64 write-back `[x0,RubixDev#8]!`
(so `!` can be bitwise OR).
Not handled, and visible in what is left of that corpus: a macro parameter in
the mnemonic or label slot (`\1 move.w ...`), `=(` with no space, and
`ENDC !SYMBOL`. Also NOT handled, on purpose: a trailing `*` comment after code
(`STRUCT x,MP_SIZE * a comment`). There is no lexical rule for it that does not
also claim the `*` in `[rsi + rcx * 8]` and `equ 8 * SCALE`, both of which are
real x86 lines in the test corpora; it needs per-file dialect detection, which
this grammar deliberately does not do.
39 corpus tests added across the existing categories plus `expressions` and
`preproc`. All 74 tests pass; the 36 pre-existing tests are unchanged.
`src/parser.c` and the vendored `src/tree_sitter/*.h` runtime headers are
regenerated together, with the tree-sitter CLI current at the time of writing.
They cannot be split: a `parser.c` from a newer CLI initializes `TSLanguage`
with more fields than an older `parser.h` declares, and the build fails with
"excess elements in struct initializer". I tried shipping only `parser.c` to
keep the diff focused and it does not compile — flagging it in case you would
rather regenerate both with your own CLI version and take only `grammar.js`
and the corpus tests from this branch.
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.
Parsing real-world assembly surfaced constructs the grammar can't represent. I hit this
across several projects — NASM/Intel (a hobby kernel), GAS/AT&T (CPU errata probes), MASM
and GAS copies of the Quake 2 software renderer, and a 26,000-line vasm 68000 disassembly.
The largest single cause was that
_exprhad no arithmetic. Operands are everywhere, so onemissing rule failed a large fraction of real lines:
dd -(MB2_MAGIC + MB2_ARCH),mov ecx, (end - start) / 4,[pml4 + 511*8].code32,.386P—/\.[a-z_]+/can match neither.lcomm env, 32(ident + int was not an accepted shape)%include "isr_stubs.inc"relmov [rel syscall_kernel_rsp], rspptrmov dword [edi + 4], 0jmp 0x08:long_start$/$$.msg_len equ $ - .msgdd 040000000h(not0x…)mov esi, ds:dword ptr[_bbextents]reciprocal_table_16 dd 0, Entry2_16— names data with no colonlea ltab(%rip), %r12fstp %st(0)sym: resq 1It also adds the five Motorola 68000 addressing-mode families a real vasm source is written
in. On that disassembly the grammar goes from 834 ERROR nodes across 761 lines to zero,
and no file in a 62-file local x86/ARM corpus parses worse (error nodes 5,843 → 5,818).
Corpus tests: 74 — the 36 existing ones unchanged, 38 added. Generated with tree-sitter
CLI 0.27.1.
A natural A/B for the dialect work
The Quake 2 software renderer ships the same routines in both MASM and GAS
(
r_draw16.asm/r_draw16.s, and seven more pairs). That makes each MASM fix checkableagainst a known-good counterpart rather than against my own judgement. Extracted symbol
counts per pair now agree within ~10% across all eight, where before the MASM side of the
worst pair produced 1 symbol against the GAS side's 50.
One fix worth calling out separately
programwassep(repeat1('\n'), $._item), which requires an_itembefore the firstseparator. A file that opens with a comment line therefore forced the parser to insert a
MISSING node. Since most assembly files open with a comment header,
tree.rootNode.hasErrorwas
truefor essentially every real file even when nothing else was wrong — which masksgenuine errors and breaks any tooling that gates on
hasError. The fix allows leadingnewlines; the separator discipline between items is unchanged.
Notes for review
preproc_identuses an explicit keyword list rather than/%[a-z]+/on purpose: thelatter would swallow GAS registers like
%eax.meta_identaccepts a lowercase-or-digit start but not an uppercase one. Widening itfurther makes
.L5ameta_identand breaks the existing "Dotted label arg" test — I hitthat and narrowed it back; the real MASM directives in the wild are
.386Pand.model.ptr's MASM segment-override branch isprec(11)so it outranksfar_ptr, which wouldotherwise match
ds:dwordand strand theptr[...]behind it.ptr's paren branch isprec(10)so AT&T(%reg)still wins over a parenthesisedexpression.
metaandpreprocareprec.rightso operands bind to their own line.(An)+is admitted only when the parenthesis has no displacement infront of it.
d(An)+is not a 68000 addressing mode, whereas GAS'sC(sym)+4— theC()macro plus a struct offset — is common (78 sites in the Quake 2 software renderer), and
taking that
+as post-increment regressed five files that had parsed cleanly. A declared[$.ptr, $.expr]conflict does not help: the generator reports it as unnecessary andstill resolves statically in
ptr's favour.src/parser.candsrc/tree_sitter/*.hare regenerated together and cannot be split.I first shipped only
parser.cto keep the diff focused on the grammar; that does notcompile — a
parser.cfrom a newer CLI initializesTSLanguagewith more fields than anolder
parser.hdeclares ("excess elements in struct initializer"). If you would rathernot take the runtime-header churn, regenerate both with your own CLI and take only
grammar.jsand the corpus tests from this branch; the grammar is the whole contributionand the generated files are reproducible from it.
Also worth knowing:
tree-sitter build --wasmin 0.27 uses the cached wasi-sdk, while0.24/0.25 shell out to the
emscripten/emsdkdocker image, which has no arm64 build andfails with
exec /emsdk/docker/entrypoint.sh: no such file or directory.