Rewrite lexer and parser

[Schamper]

Closes #85, partially #142, and will make #86 and #138 a lot easier to implement. Fixes #149.

This PR will (finally) replace the shoddy C syntax parser I originally wrote many moons ago, when I discovered the existence of re.Scanner and ran with it. This PR aims to add a somewhat decent lexer and separate parser. I'm still not a compsci 1337coder, so this is just what I came up with (with some help) and definitely not a textbook implementation. All feedback is welcome.

• New lexer
• New C syntax parser that utilizes the new lexer
• Expression parser re-uses the new lexer
• Reworked how sizeof works in the expression parser, and added offsetof

The new parser has made changing parsing behavior a lot easier. As such, this PR already makes the following changes:

• The new parser is slightly stricter, requiring proper semicolon endings for example. We'll need to fix this in any dissect code that has this.

• An important semantic change is how named nested structures are handled. In my infinite wisdom, I originally figured that named nested structures do not "exist" in the top level scope. That's not true, so now named nested structures get properly registered with the cstruct instance:

struct a {
    struct b {
        ...
    };
};

// Will register both `a` and `b`

• Another important change is how we deal with struct { ... } name;. We used to parse this first as an anonymous struct, then capture name as the structure type name. That's not strictly correct, name is a variable of an anonymous unnamed struct, so we now treat it as such. We don't error on this, but rather we silently ignore name and skip until we reach a ;
• typedef enum ... is now allowed
• Probably some other things I'm forgetting

This probably warrants a major version bump, so maybe good to pair this with #114, #144 and what we discussed in #142.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 761 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (29652dd) to head (3a37ab0).

Files with missing lines	Patch %	Lines
dissect/cstruct/lexer.py	0.00%	341 Missing ⚠️
dissect/cstruct/parser.py	0.00%	293 Missing ⚠️
dissect/cstruct/expression.py	0.00%	111 Missing ⚠️
dissect/cstruct/utils.py	0.00%	12 Missing ⚠️
dissect/cstruct/cstruct.py	0.00%	2 Missing ⚠️
dissect/cstruct/exceptions.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main    #146    +/-   ##
======================================
  Coverage   0.00%   0.00%            
======================================
  Files         21      22     +1     
  Lines       2470    2582   +112     
======================================
- Misses      2470    2582   +112     

Flag	Coverage Δ
unittests	0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codspeed-hq]

Merging this PR will improve performance by 11.81%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

#### 🎉 Hooray! `pytest-codspeed` just leveled up to 5.0.2!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

⚡ 8 improved benchmarks
❌ 1 regressed benchmark
✅ 3 untouched benchmarks
🆕 2 new benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	test_benchmark_expression_evaluate	81.9 µs	127.7 µs	-35.89%
⚡	test_benchmark_attribute_access	15.3 µs	11.3 µs	+35.53%
⚡	test_benchmark_expression_parse	345.4 µs	264.2 µs	+30.73%
⚡	test_benchmark_basic[compiled]	81.2 µs	73.3 µs	+10.7%
⚡	test_benchmark_expression_parse_and_evaluate	391.6 µs	353.7 µs	+10.73%
⚡	test_benchmark_getattr_constants	17 µs	13.6 µs	+25.48%
⚡	test_benchmark_getattr_typedefs	27.5 µs	24.3 µs	+13.42%
⚡	test_benchmark_getattr_types	27 µs	23.8 µs	+13.69%
🆕	test_benchmark_lexer	N/A	2.4 ms	N/A
⚡	test_benchmark_lexer_and_parser	15.7 ms	13 ms	+21.21%
🆕	test_benchmark_parser	N/A	10.6 ms	N/A

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing rewrite-parser (3a37ab0) with main (29652dd)}

[Schamper]

@sMezaOrellana would be interested in your thoughts on these changes!

[twiggler]

Architecure looks ok, found 2 possible issues TBC

[twiggler]

This PR has been migrated to the dissect monorepo: twiggler/dissect-monorepo-test#5

The original diff and commit history have been preserved on the migrate/dissect.cstruct/pr-146 branch.

[Schamper]

@twiggler I was a bit sad that your message was just this test, as I thought maybe you left more comments 😉.

[twiggler]

LGTM

Since it is an impactful rewrite I asked @Miauwkeru to QA

[Miauwkeru]

I did some checking on our other projects to see what goes wrong, and found some regressions regarding to where it crashed unexpectedly.

1. it cannot evaluate a ternary operation, which occurred in dissect.vmfs and gives a LexerError when trying to evaluate it as it doesn't understand the ? token. While I am aware that those definitions don't function yet, it probably shouldn't crash when trying to evaluate it

#define func(x) ( x ? 1 : 0 )

Without a ternary it makes it into a string:

# define func(x)    ( x == 0)
>>> c_struct.func
' ( x ) ( x = = 0)`

2. Another issue I found was when it was trying to create define a flag in dissect.apfs. Here it couldn't evaluate the . after the INODE in the definition.

flag INODE {
  ...
};

#define APFS_INODE_PINNED_MASK            INODE.PINNED_TO_MAIN | INODE.PINNED_TO_TIER2

I'll look through the code now to see whether I can find some additional issues.

[Schamper]

@Miauwkeru fixed.

[Miauwkeru]

Only found one more thing that might be an oddity:

#define ADCRYPT_MAGIC ADCRYPT\00

This now fails to parse due to the \00 at the end. Which is fixed by using quotes. I don't know if this was an intended change tho

[Schamper]

Not necessarily intended. What do you think would be reasonable behavior in this case? (also ping @JSCU-CNI)

[Miauwkeru]

Not necessarily intended. What do you think would be reasonable behavior in this case? (also ping @JSCU-CNI)

I think it makes most sense to go the C route. In the example I gave:

#define ADCRYPT_MAGIC ADCRYPT\00

C wouldn't compile it as it would expect ADCRYPT to be another definition or something else it can resolve. (Besides not knowing what to do with the \0). So I think it would be better if we require strings to be explicitly quoted.

What do you think? @Schamper @JSCU-CNI

[Schamper]

I was leaning in that direction too.

[JSCU-CNI]

Agreed. Something like #define ADCRYPT_MAGIC "ADCRYPT\00" or #define ADCRYPT_MAGIC b"ADCRYPT\x00" would make sense.

[Schamper]

I changed a bit how #define values are handled with 3a37ab0. Feedback is welcome.

Both ways now actually work.

[Miauwkeru]

Don't we want it to be explicitly quoted so that it fails on this kind of definition?

[Schamper]

My "new approach" (which is basically, don't tokenize anything after #define NAME, but just take its raw value until the end of the line) allows this to work again. I think as long as it's unit tested, it should be fine to keep in.

The reason why I slightly prefer this new approach is so that in the parser, we get a more "true" representation of that the define value actually is, including spacing and such. The downside being that the parser now has to deal a little bit with some basic string parsing.