Request: guidance for adding tests

[hainest]

I would like to port the decoder tests I recently added to Capstone to here. Based on the main GitHub CI workflow, I see there are a set of regression tests in tests/cases and fuzzing is done with tools/ZydisFuzzDecoder.c. Am I missing any for the decoder?

A few questions:

1. Is there interest in having "full" parameter space tests like Update read/written registers for x86 string instructions capstone-engine/capstone#2790?
2. The naming convention for regression tests is currently <type>_XXX.{in,out}. The stringop tests I created will eat up more than 10% of the available namespace for default. Would it be ok to extend the number of digits? I was also considering using a convention with a more specific type based on the instruction's category (ZydisInstructionCategory) like stringops_XXX instead of default_XXX to avoid jostling the naming conventions too much.
3. Is there interest in having configuration-driven testing for the decoder like there is for the encoder (e.g., tests/enc_test_cases.json)? The current diffing of the output from ZydisInfo effectively precludes certain types of updates to its output. Although clearly hasn't been an issue since they were added four years ago. I'd be happy to put in some serious cycles into building the needed testing framework for a big update like this.

Many thanks!

[flobernd]

Hi @hainest ,

interesting question! Yes, the regression tests and the fuzzing are the only tests specifically for the decoder / formatter. I'm definitely open to improve the tests, however I'm not sure how far we want to go with this.

The regression tests are kinda "random". We added a few specific cases that do trigger certain paths in the decoder (e.g. gather instructions) for which we often experienced edge cases or broken behavior after changes.

For stuff like the affected flags, written registers like in your example with the "string" instructions, a "full" test would mostly validate that the instruction definitions are correct, since this kind of data is completely processed by the code generator. Hardcoding such tests feels quite redundant to me. It implies that we do not trust the instruction definitions (they of course can be incorrect from time to time) but on the other hand we trust the hardcoded test cases.

WDYT @athre0z @mappzor ?

[hainest]

It implies that we do not trust the instruction definitions (they of course can be incorrect from time to time)

I don't mean to be pithy, but you shouldn't trust the definitions. Capstone uses the LLVM TableGen outputs for their x86 definitions, but they are often woefully incomplete and occassionally just wrong. And that's coming from a compiler vendor... There is also extended information like implicit memory writes[1].

on the other hand we trust the hardcoded test cases.

This is a hard tradeoff that I completely understand is a tough decision. My argument tends to be that hardcoded tests should be added in small sets so they can be very carefully verified. That's what I've been doing with Capstone. Although I freely admit the example I gave for the string instructions is very much on the side of too big (there are just a huge number of permutations for those instructions).

[athre0z]

Personally I wouldn't be opposed to more test cases, though I'd definitely want to go with an approach where we sample a subset from the "full parameter space". Not sure what space you are referring to here -- if it's just "one test for each opcode in the default opcode space, I'd also be OK with that. Note that the opcodes can have different meanings depending on decoder mode, e.g. the 0x4X region is REX in x86-64.

From my POV, the primary purpose of such tests would be to do regression tests that protect us from bugs in the table generator and/or the "routing"/selection logic in the decoder.

Our tables are a mixture of being sourced from Intel XED, multiple manual passes through both Intel and AMD documentation as well as many fixups that we've applied manually over time. We've also occasionally checked LLVM's tables, but in our experience they tended to be less reliable than other sources.

What methodology would you like to apply to validate the results? Generate N tests and then manually check the outputs against Intel and/or AMD documentation?

Would it be ok to extend the number of digits? I was also considering using a convention with a more specific type based on the instruction's category

No strong pref about the naming at all, as long as all the tooling relying on the old names is updated in the same process. Not sure if there is much reliance on the names -- don't quite recall how it works.

Edit: there's also some prior art by Trail Of Bits to validate the tables by means of diffing outputs of different disassembler libraries here -- might be interesting to you as well