Put callx register into dst field

The mailing list discussion at
https://mailarchive.ietf.org/arch/msg/bpf/Vx1H3ViPUWoGKNssCO22lOIjyXU/
agreed that inst.dst is where to put the register to use going forward
and clang v.19 will do so but earlier versions of clang used inst.imm.

Signed-off-by: Dave Thaler <[email protected]>

Author: dthaler

external/bpf_conformance

@@ -180,9 +180,9 @@ struct MarshalVisitor {
     }
 
     vector<ebpf_inst> operator()(Callx const& b) {
-        // callx is defined to have the source register in 'imm' not in 'src'.
+        // callx is defined to have the register in 'dst' not in 'src'.
         return {
-            ebpf_inst{.opcode = static_cast<uint8_t>(INST_OP_CALL | INST_SRC_REG), .dst = 0, .src = 0, .offset = 0, .imm = b.func.v}};
+            ebpf_inst{.opcode = static_cast<uint8_t>(INST_OP_CALL | INST_SRC_REG), .dst = b.func.v, .src = 0, .offset = 0}};
     }
 
     vector<ebpf_inst> operator()(Exit const& b) {

src/asm_marshal.cpp

@@ -406,10 +406,18 @@ struct Unmarshaller {
     }
 
     auto makeCallx(ebpf_inst inst, pc_t pc) const {
-        // callx puts the register number in the 'imm' field rather than the 'src' field.
-        if (inst.imm < 0 || inst.imm > R10_STACK_POINTER)
+        // callx puts the register number in the 'dst' field rather than the 'src' field.
+        if (inst.dst > R10_STACK_POINTER)
             throw InvalidInstruction(pc, "Bad register");
-        return Callx{(uint8_t)inst.imm};
+        if (inst.imm != 0) {
+            // Clang prior to v19 put the register number into the 'imm' field.
+            if (inst.dst > 0)
+                throw InvalidInstruction(pc, make_opcode_message("Nonzero imm for", inst.opcode));
+            if (inst.imm < 0 || inst.imm > R10_STACK_POINTER)
+                throw InvalidInstruction(pc, "Bad register");
+            return Callx{(uint8_t)inst.imm};
+        }
+        return Callx{inst.dst};
     }
 
     auto makeJmp(ebpf_inst inst, const vector<ebpf_inst>& insts, pc_t pc) -> Instruction {

src/asm_unmarshal.cpp

@@ -177,11 +177,19 @@ TEST_CASE("disasm_marshal", "[disasm][marshal]") {
         oss << "0: Bad instruction op 0x" << std::hex << INST_OP_CALLX << std::endl;
         check_unmarshal_fail(ebpf_inst{.opcode = INST_OP_CALLX}, oss.str().c_str());
 
-        // Test callx with support.  Note that callx puts the register number in 'imm' not 'src'.
+        // Test callx with support.  Note that callx puts the register number in 'dst' not 'src'.
         ebpf_platform_t platform = g_ebpf_platform_linux;
         platform.callx = true;
         compare_marshal_unmarshal(Callx{8}, false, &platform);
-        check_unmarshal_fail(ebpf_inst{.opcode = /* 0x8d */ INST_OP_CALLX, .imm = 11}, "0: Bad register\n", &platform);
+        ebpf_inst callx{.opcode = INST_OP_CALLX, .dst = 8};
+        compare_unmarshal_marshal(callx, callx, &platform);
+        check_unmarshal_fail({.opcode = INST_OP_CALLX, .dst = 11}, "0: Bad register\n", &platform);
+        check_unmarshal_fail({.opcode = INST_OP_CALLX, .dst = 8, .imm = 8}, "0: Nonzero imm for op 0x8d\n", &platform);
+
+        // clang prior to v19 put the register into 'imm' instead of 'dst' so we treat it as equivalent.
+        compare_unmarshal_marshal(ebpf_inst{.opcode = /* 0x8d */ INST_OP_CALLX, .imm = 8}, callx, &platform);
+        check_unmarshal_fail({.opcode = INST_OP_CALLX, .imm = 11}, "0: Bad register\n", &platform);
+        check_unmarshal_fail({.opcode = INST_OP_CALLX, .imm = -1}, "0: Bad register\n", &platform);
     }
 
     SECTION("Exit") { compare_marshal_unmarshal(Exit{}); }