about summary refs log tree commit diff stats
path: root/subx/031check_operands.cc
diff options
context:
space:
mode:
authorKartik Agaram <vc@akkartik.com>2019-03-12 18:56:55 -0700
committerKartik Agaram <vc@akkartik.com>2019-03-12 19:14:12 -0700
commit4a943d4ed313eff001504c2b5c472266e86a38af (patch)
treea5757233a8c81b303a808f251180c7344071ed51 /subx/031check_operands.cc
parent43711b0e9f18e0225ce14687fb6ea0902aa6fc61 (diff)
downloadmu-4a943d4ed313eff001504c2b5c472266e86a38af.tar.gz
5001 - drop the :(scenario) DSL
I've been saying for a while[1][2][3] that adding extra abstractions makes
things harder for newcomers, and adding new notations doubly so. And then
I notice this DSL in my own backyard. Makes me feel like a hypocrite.

[1] https://news.ycombinator.com/item?id=13565743#13570092
[2] https://lobste.rs/s/to8wpr/configuration_files_are_canary_warning
[3] https://lobste.rs/s/mdmcdi/little_languages_by_jon_bentley_1986#c_3miuf2

The implementation of the DSL was also highly hacky:

a) It was happening in the tangle/ tool, but was utterly unrelated to tangling
layers.

b) There were several persnickety constraints on the different kinds of
lines and the specific order they were expected in. I kept finding bugs
where the translator would silently do the wrong thing. Or the error messages
sucked, and readers may be stuck looking at the generated code to figure
out what happened. Fixing error messages would require a lot more code,
which is one of my arguments against DSLs in the first place: they may
be easy to implement, but they're hard to design to go with the grain of
the underlying platform. They require lots of iteration. Is that effort
worth prioritizing in this project?

On the other hand, the DSL did make at least some readers' life easier,
the ones who weren't immediately put off by having to learn a strange syntax.
There were fewer quotes to parse, fewer backslash escapes.

Anyway, since there are also people who dislike having to put up with strange
syntaxes, we'll call that consideration a wash and tear this DSL out.

---

This commit was sheer drudgery. Hopefully it won't need to be redone with
a new DSL because I grow sick of backslashes.
Diffstat (limited to 'subx/031check_operands.cc')
-rw-r--r--subx/031check_operands.cc300
1 files changed, 189 insertions, 111 deletions
diff --git a/subx/031check_operands.cc b/subx/031check_operands.cc
index bc76fca5..475cda5c 100644
--- a/subx/031check_operands.cc
+++ b/subx/031check_operands.cc
@@ -1,11 +1,16 @@
 //: Since we're tagging operands with their types, let's start checking these
 //: operand types for each instruction.
 
-:(scenario check_missing_imm8_operand)
-% Hide_errors = true;
-== 0x1
-cd  # int ??
-+error: 'cd' (software interrupt): missing imm8 operand
+void test_check_missing_imm8_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "cd\n"  // interrupt ??
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: 'cd' (software interrupt): missing imm8 operand\n"
+  );
+}
 
 :(before "Pack Operands(segment code)")
 check_operands(code);
@@ -239,7 +244,6 @@ void init_permitted_operands() {
   // End Init Permitted Operands
 }
 
-:(code)
 #define HAS(bitvector, bit)  ((bitvector) & (1 << (bit)))
 #define SET(bitvector, bit)  ((bitvector) | (1 << (bit)))
 #define CLEAR(bitvector, bit)  ((bitvector) & (~(1 << (bit))))
@@ -335,22 +339,31 @@ uint32_t expected_bit_for_received_operand(const word& w, set<string>& instructi
   return bv;
 }
 
-:(scenario conflicting_operand_type)
-% Hide_errors = true;
-== 0x1
-cd/software-interrupt 80/imm8/imm32
-+error: '80/imm8/imm32' has conflicting operand types; it should have only one
+void test_conflicting_operand_type() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "cd/software-interrupt 80/imm8/imm32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '80/imm8/imm32' has conflicting operand types; it should have only one\n"
+  );
+}
 
 //: Instructions computing effective addresses have more complex rules, so
 //: we'll hard-code a common set of instruction-decoding rules.
 
-:(scenario check_missing_mod_operand)
-% Hide_errors = true;
-== 0x1
-81 0/add/subop       3/rm32/ebx 1/imm32
-+error: '81 0/add/subop 3/rm32/ebx 1/imm32' (combine rm32 with imm32 based on subop): missing mod operand
+void test_check_missing_mod_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "81 0/add/subop       3/rm32/ebx 1/imm32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '81 0/add/subop 3/rm32/ebx 1/imm32' (combine rm32 with imm32 based on subop): missing mod operand\n"
+  );
+}
 
-:(code)
 void check_operands_modrm(const line& inst, const word& op) {
   if (all_hex_bytes(inst)) return;  // deliberately programming in raw hex; we'll raise a warning elsewhere
   check_operand_metadata_present(inst, "mod", op);
@@ -421,95 +434,158 @@ void check_operand_metadata_absent(const line& inst, const string& type, const w
     raise << "'" << to_string(inst) << "'" << maybe_name(op) << ": unexpected " << type << " operand (" << msg << ")\n" << end();
 }
 
-:(scenarios transform)
-:(scenario modrm_with_displacement)
-% Reg[EAX].u = 0x1;
-== 0x1
-# just avoid null pointer
-8b/copy 1/mod/lookup+disp8 0/rm32/EAX 2/r32/EDX 4/disp8  # copy *(EAX+4) to EDX
-$error: 0
-
-:(scenario check_missing_disp8)
-% Hide_errors = true;
-== 0x1
-89/copy 1/mod/lookup+disp8 0/rm32/EAX 1/r32/ECX  # missing disp8
-+error: '89/copy 1/mod/lookup+disp8 0/rm32/EAX 1/r32/ECX' (copy r32 to rm32): missing disp8 operand
-
-:(scenario check_missing_disp32)
-% Hide_errors = true;
-== 0x1
-8b/copy 0/mod/indirect 5/rm32/.disp32 2/r32/EDX  # missing disp32
-+error: '8b/copy 0/mod/indirect 5/rm32/.disp32 2/r32/EDX' (copy rm32 to r32): missing disp32 operand
-:(scenarios run)
-
-:(scenario conflicting_operands_in_modrm_instruction)
-% Hide_errors = true;
-== 0x1
-01/add 0/mod 3/mod
-+error: '01/add 0/mod 3/mod' has conflicting mod operands
-
-:(scenario conflicting_operand_type_modrm)
-% Hide_errors = true;
-== 0x1
-01/add 0/mod 3/rm32/r32
-+error: '3/rm32/r32' has conflicting operand types; it should have only one
-
-:(scenario check_missing_rm32_operand)
-% Hide_errors = true;
-== 0x1
-81 0/add/subop 0/mod            1/imm32
-+error: '81 0/add/subop 0/mod 1/imm32' (combine rm32 with imm32 based on subop): missing rm32 operand
-
-:(scenario check_missing_subop_operand)
-% Hide_errors = true;
-== 0x1
-81             0/mod 3/rm32/ebx 1/imm32
-+error: '81 0/mod 3/rm32/ebx 1/imm32' (combine rm32 with imm32 based on subop): missing subop operand
-
-:(scenario check_missing_base_operand)
-% Hide_errors = true;
-== 0x1
-81 0/add/subop 0/mod/indirect 4/rm32/use-sib 1/imm32
-+error: '81 0/add/subop 0/mod/indirect 4/rm32/use-sib 1/imm32' (combine rm32 with imm32 based on subop): missing base operand
-
-:(scenario check_missing_index_operand)
-% Hide_errors = true;
-== 0x1
-81 0/add/subop 0/mod/indirect 4/rm32/use-sib 0/base 1/imm32
-+error: '81 0/add/subop 0/mod/indirect 4/rm32/use-sib 0/base 1/imm32' (combine rm32 with imm32 based on subop): missing index operand
-
-:(scenario check_missing_base_operand_2)
-% Hide_errors = true;
-== 0x1
-81 0/add/subop 0/mod/indirect 4/rm32/use-sib 2/index 3/scale 1/imm32
-+error: '81 0/add/subop 0/mod/indirect 4/rm32/use-sib 2/index 3/scale 1/imm32' (combine rm32 with imm32 based on subop): missing base operand
-
-:(scenario check_extra_displacement)
-% Hide_errors = true;
-== 0x1
-89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 4/disp8
-+error: '89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 4/disp8' (copy r32 to rm32): unexpected disp8 operand
-
-:(scenario check_duplicate_operand)
-% Hide_errors = true;
-== 0x1
-89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 1/r32
-+error: '89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 1/r32': duplicate r32 operand
-
-:(scenario check_base_operand_not_needed_in_direct_mode)
-== 0x1
-81 0/add/subop 3/mod/indirect 4/rm32/use-sib 1/imm32
-$error: 0
-
-:(scenario extra_modrm)
-% Hide_errors = true;
-== 0x1
-59/pop-to-ECX  3/mod/direct 1/rm32/ECX 4/r32/ESP
-+error: '59/pop-to-ECX 3/mod/direct 1/rm32/ECX 4/r32/ESP' (pop top of stack to ECX): unexpected modrm operand
+void test_modrm_with_displacement() {
+  Reg[EAX].u = 0x1;
+  transform(
+      "== 0x1\n"
+      // just avoid null pointer
+      "8b/copy 1/mod/lookup+disp8 0/rm32/EAX 2/r32/EDX 4/disp8\n"  // copy *(EAX+4) to EDX
+  );
+  CHECK_TRACE_COUNT("error", 0);
+}
+
+void test_check_missing_disp8() {
+  Hide_errors = true;
+  transform(
+      "== 0x1\n"  // code segment
+      "89/copy 1/mod/lookup+disp8 0/rm32/EAX 1/r32/ECX\n"  // missing disp8
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '89/copy 1/mod/lookup+disp8 0/rm32/EAX 1/r32/ECX' (copy r32 to rm32): missing disp8 operand\n"
+  );
+}
+
+void test_check_missing_disp32() {
+  Hide_errors = true;
+  transform(
+      "== 0x1\n"  // code segment
+      "8b/copy 0/mod/indirect 5/rm32/.disp32 2/r32/EDX\n"  // missing disp32
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '8b/copy 0/mod/indirect 5/rm32/.disp32 2/r32/EDX' (copy rm32 to r32): missing disp32 operand\n"
+  );
+}
+
+void test_conflicting_operands_in_modrm_instruction() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "01/add 0/mod 3/mod\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '01/add 0/mod 3/mod' has conflicting mod operands\n"
+  );
+}
+
+void test_conflicting_operand_type_modrm() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "01/add 0/mod 3/rm32/r32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '3/rm32/r32' has conflicting operand types; it should have only one\n"
+  );
+}
+
+void test_check_missing_rm32_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "81 0/add/subop 0/mod            1/imm32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '81 0/add/subop 0/mod 1/imm32' (combine rm32 with imm32 based on subop): missing rm32 operand\n"
+  );
+}
+
+void test_check_missing_subop_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "81             0/mod 3/rm32/ebx 1/imm32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '81 0/mod 3/rm32/ebx 1/imm32' (combine rm32 with imm32 based on subop): missing subop operand\n"
+  );
+}
+
+void test_check_missing_base_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "81 0/add/subop 0/mod/indirect 4/rm32/use-sib 1/imm32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '81 0/add/subop 0/mod/indirect 4/rm32/use-sib 1/imm32' (combine rm32 with imm32 based on subop): missing base operand\n"
+  );
+}
+
+void test_check_missing_index_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "81 0/add/subop 0/mod/indirect 4/rm32/use-sib 0/base 1/imm32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '81 0/add/subop 0/mod/indirect 4/rm32/use-sib 0/base 1/imm32' (combine rm32 with imm32 based on subop): missing index operand\n"
+  );
+}
+
+void test_check_missing_base_operand_2() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "81 0/add/subop 0/mod/indirect 4/rm32/use-sib 2/index 3/scale 1/imm32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '81 0/add/subop 0/mod/indirect 4/rm32/use-sib 2/index 3/scale 1/imm32' (combine rm32 with imm32 based on subop): missing base operand\n"
+  );
+}
+
+void test_check_extra_displacement() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 4/disp8\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 4/disp8' (copy r32 to rm32): unexpected disp8 operand\n"
+  );
+}
+
+void test_check_duplicate_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 1/r32\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '89/copy 0/mod/indirect 0/rm32/EAX 1/r32/ECX 1/r32': duplicate r32 operand\n"
+  );
+}
+
+void test_check_base_operand_not_needed_in_direct_mode() {
+  run(
+      "== 0x1\n"  // code segment
+      "81 0/add/subop 3/mod/indirect 4/rm32/use-sib 1/imm32\n"
+  );
+  CHECK_TRACE_COUNT("error", 0);
+}
+
+void test_extra_modrm() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "59/pop-to-ECX  3/mod/direct 1/rm32/ECX 4/r32/ESP\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '59/pop-to-ECX 3/mod/direct 1/rm32/ECX 4/r32/ESP' (pop top of stack to ECX): unexpected modrm operand\n"
+  );
+}
 
 //:: similarly handle multi-byte opcodes
 
-:(code)
 void check_operands_0f(const line& inst) {
   assert(inst.words.at(0).data == "0f");
   if (SIZE(inst.words) == 1) {
@@ -528,14 +604,16 @@ void check_operands_f3(const line& /*unused*/) {
   raise << "no supported opcodes starting with f3\n" << end();
 }
 
-:(scenario check_missing_disp32_operand)
-% Hide_errors = true;
-== 0x1
-# instruction                     effective address                                                   operand     displacement    immediate
-# op          subop               mod             rm32          base        index         scale       r32
-# 1-3 bytes   3 bits              2 bits          3 bits        3 bits      3 bits        2 bits      2 bits      0/1/2/4 bytes   0/1/2/4 bytes
-  0f 84                                                                                                                                             # jmp if ZF to ??
-+error: '0f 84' (jump disp32 bytes away if equal, if ZF is set): missing disp32 operand
+void test_check_missing_disp32_operand() {
+  Hide_errors = true;
+  run(
+      "== 0x1\n"  // code segment
+      "  0f 84                                                                                                                                             # jmp if ZF to ??\n"
+  );
+  CHECK_TRACE_CONTENTS(
+      "error: '0f 84' (jump disp32 bytes away if equal, if ZF is set): missing disp32 operand\n"
+  );
+}
 
 :(before "End Globals")
 map</*op*/string, /*bitvector*/uint8_t> Permitted_operands_0f;