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:
- Is there interest in having "full" parameter space tests like https://github.com/capstone-engine/capstone/pull/2790?
- 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 fordefault. 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) likestringops_XXXinstead ofdefault_XXXto avoid jostling the naming conventions too much. - 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 fromZydisInfoeffectively 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!
Activity
4
hainest1y agoIt 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).
athre0z1y agoPersonally 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
hainest11mo agoPersonally 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
0x4Xregion is REX in x86-64.
I'm referring to the parameter space of 16-, 32-, and 64-bit decode mode and any available prefixes: REP, REX, 0x67, etc. The string comparison tests (1, 2, 3) I added to Capstone would be the level of detail I'm aiming for. Of course, it would be nice but a bit unreasonable to try to test all ModR/M bytes for any given opcode.
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.
It would also guard against incompleteness in the source tables (e.g., https://github.com/zyantific/zydis-db/issues/42).
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.
Capstone currently uses LLVM TableGen for x86, and it's pretty spartan in terms of semantics. I've found Zydis and bdisassem to be much more accurate. But there are still bugs and some items I disagree with. For example, I've found that Xed suggests using 'eax' in 16-bit mode which makes no sense.
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?
I've employed hand-written automatable tests in Capstone and Dyninst. I'd like to start with a direct port of those, if possible, using the techniques I suggested in my top post.
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.
Good to know.
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
Indeed. I've recommended adding diffuzing to Capstone using Meshegos. The project team I work with has also done some work on this in the past (https://github.com/dyninst/tools/tree/master/fleece).
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 ?