Skip to content

Commit

Permalink
Consistency cleanup
Browse files Browse the repository at this point in the history 
Signed-off-by: Dave Thaler <[email protected]>
  • Loading branch information
@dthaler
dthaler committed Jan 4, 2024
1 parent 8357d59 commit 8f10aef
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 73 deletions.
16 changes: 8 additions & 8 deletions src/asm_ostream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,17 +327,17 @@ struct InstructionPrinterVisitor {
print(b.access);
os_ << " ";
bool showfetch = true;
switch ((AtomicOp)((uint32_t)b.op & (uint32_t)AtomicOp::INST_ATOMIC_BASE_MASK)) {
case AtomicOp::INST_ATOMIC_ADD: os_ << "+"; break;
case AtomicOp::INST_ATOMIC_OR: os_ << "|"; break;
case AtomicOp::INST_ATOMIC_AND: os_ << "&"; break;
case AtomicOp::INST_ATOMIC_XOR: os_ << "^"; break;
case AtomicOp::INST_ATOMIC_XCHG_BASE: os_ << "x"; showfetch = false; break;
case AtomicOp::INST_ATOMIC_CMPXCHG_BASE: os_ << "cx"; showfetch = false; break;
switch ((Atomic::Op)((uint32_t)b.op & (uint32_t)Atomic::Op::BASE_MASK)) {
case Atomic::Op::ADD: os_ << "+"; break;
case Atomic::Op::OR : os_ << "|"; break;
case Atomic::Op::AND: os_ << "&"; break;
case Atomic::Op::XOR: os_ << "^"; break;
case Atomic::Op::XCHG_BASE: os_ << "x"; showfetch = false; break;
case Atomic::Op::CMPXCHG_BASE: os_ << "cx"; showfetch = false; break;
}
os_ << "= " << b.valreg;

if (showfetch && ((uint32_t)b.op & (uint32_t)AtomicOp::INST_ATOMIC_FETCH)) {
if (showfetch && ((uint32_t)b.op & (uint32_t)Atomic::Op::FETCH)) {
os_ << " fetch";
}
}
Expand Down
24 changes: 13 additions & 11 deletions src/asm_parse.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,19 @@ static const std::map<std::string, Condition::Op> str_to_cmpop = {
{"s<", Condition::Op::SLT}, {"s<=", Condition::Op::SLE}, {"s>", Condition::Op::SGT}, {"s>=", Condition::Op::SGE},
};

static const std::map<std::string, AtomicOp> str_to_atomicop = {{"+", AtomicOp::INST_ATOMIC_ADD},
{"|", AtomicOp::INST_ATOMIC_OR},
{"&", AtomicOp::INST_ATOMIC_AND},
{"^", AtomicOp::INST_ATOMIC_XOR},
{"x", AtomicOp::INST_ATOMIC_XCHG},
{"cx", AtomicOp::INST_ATOMIC_CMPXCHG}};

static const std::map<std::string, AtomicOp> str_to_atomicfetchop = {{"+", AtomicOp::INST_ATOMIC_ADD_FETCH},
{"|", AtomicOp::INST_ATOMIC_OR_FETCH},
{"&", AtomicOp::INST_ATOMIC_AND_FETCH},
{"^", AtomicOp::INST_ATOMIC_XOR_FETCH}};
static const std::map<std::string, Atomic::Op> str_to_atomicop = {
{"+", Atomic::Op::ADD},
{"|", Atomic::Op::OR},
{"&", Atomic::Op::AND},
{"^", Atomic::Op::XOR},
{"x", Atomic::Op::XCHG},
{"cx", Atomic::Op::CMPXCHG}};

static const std::map<std::string, Atomic::Op> str_to_atomicfetchop = {
{"+", Atomic::Op::ADD_FETCH},
{"|", Atomic::Op::OR_FETCH},
{"&", Atomic::Op::AND_FETCH},
{"^", Atomic::Op::XOR_FETCH}};

static const std::map<std::string, int> str_to_width = {
{"8", 1},
Expand Down
21 changes: 20 additions & 1 deletion src/asm_syntax.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,26 @@ struct Packet {

/// Special instruction for atomically updating values inside shared memory.
struct Atomic {
AtomicOp op;
enum class Op {
FETCH = 0x01,
BASE_MASK = 0xf0, // Not including the FETCH flag.

ADD = 0x00,
ADD_FETCH = (ADD | FETCH),
OR = 0x40,
OR_FETCH = (OR | FETCH),
AND = 0x50,
AND_FETCH = (AND | FETCH),
XOR = 0xa0,
XOR_FETCH = (XOR | FETCH),
XCHG_BASE = 0xe0, // Not valid by itself
CMPXCHG_BASE = 0xf0, // Not valid by itself

XCHG = (XCHG_BASE | FETCH),
CMPXCHG = (CMPXCHG_BASE | FETCH),
};

Op op;
Deref access;
Reg valreg;
};
Expand Down
24 changes: 12 additions & 12 deletions src/asm_unmarshal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,18 +145,18 @@ struct Unmarshaller {
return {};
}

auto getAtomicOp(size_t pc, ebpf_inst inst) -> AtomicOp {
switch ((AtomicOp)inst.imm) {
case AtomicOp::INST_ATOMIC_ADD:
case AtomicOp::INST_ATOMIC_ADD_FETCH:
case AtomicOp::INST_ATOMIC_OR:
case AtomicOp::INST_ATOMIC_OR_FETCH:
case AtomicOp::INST_ATOMIC_AND:
case AtomicOp::INST_ATOMIC_AND_FETCH:
case AtomicOp::INST_ATOMIC_XOR:
case AtomicOp::INST_ATOMIC_XOR_FETCH:
case AtomicOp::INST_ATOMIC_XCHG:
case AtomicOp::INST_ATOMIC_CMPXCHG: return (AtomicOp)inst.imm;
auto getAtomicOp(size_t pc, ebpf_inst inst) -> Atomic::Op {
switch ((Atomic::Op)inst.imm) {
case Atomic::Op::ADD:
case Atomic::Op::ADD_FETCH:
case Atomic::Op::OR:
case Atomic::Op::OR_FETCH:
case Atomic::Op::AND:
case Atomic::Op::AND_FETCH:
case Atomic::Op::XOR:
case Atomic::Op::XOR_FETCH:
case Atomic::Op::XCHG:
case Atomic::Op::CMPXCHG: return (Atomic::Op)inst.imm;
}
throw InvalidInstruction(pc, "Unsupported atomic instruction");
}
Expand Down
2 changes: 1 addition & 1 deletion src/assertions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class AssertExtractor {
res.emplace_back(TypeConstraint{ins.access.basereg, TypeGroup::shared});
res.emplace_back(ValidAccess{ins.access.basereg, ins.access.offset,
Imm{static_cast<uint32_t>(ins.access.width)}, false});
if (ins.op == AtomicOp::INST_ATOMIC_CMPXCHG) {
if (ins.op == Atomic::Op::CMPXCHG) {
// The shared memory contents pointed to by ins.access will be compared
// against the value of the ins.valreg register. Since shared memory
// must contain a number, so must ins.valreg.
Expand Down
18 changes: 9 additions & 9 deletions src/crab/ebpf_domain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2134,26 +2134,26 @@ void ebpf_domain_t::operator()(const Atomic& a) {

// Compute the new value in R11.
Bin bin {.dst = r11, .v = a.valreg, .is64 = (a.access.width == 64), .lddw = false};
switch ((AtomicOp)((uint32_t)a.op & (uint32_t)AtomicOp::INST_ATOMIC_BASE_MASK)) {
case AtomicOp::INST_ATOMIC_ADD: bin.op = Bin::Op::ADD; break;
case AtomicOp::INST_ATOMIC_OR: bin.op = Bin::Op::OR; break;
case AtomicOp::INST_ATOMIC_AND: bin.op = Bin::Op::AND; break;
case AtomicOp::INST_ATOMIC_XOR: bin.op = Bin::Op::XOR; break;
case AtomicOp::INST_ATOMIC_XCHG_BASE:
case AtomicOp::INST_ATOMIC_CMPXCHG_BASE: bin.op = Bin::Op::MOV; break;
switch ((Atomic::Op)((uint32_t)a.op & (uint32_t)Atomic::Op::BASE_MASK)) {
case Atomic::Op::ADD: bin.op = Bin::Op::ADD; break;
case Atomic::Op::OR: bin.op = Bin::Op::OR; break;
case Atomic::Op::AND: bin.op = Bin::Op::AND; break;
case Atomic::Op::XOR: bin.op = Bin::Op::XOR; break;
case Atomic::Op::XCHG_BASE:
case Atomic::Op::CMPXCHG_BASE: bin.op = Bin::Op::MOV; break;
default: throw std::exception();
}
(*this)(bin);

if (a.op == AtomicOp::INST_ATOMIC_CMPXCHG) {
if (a.op == Atomic::Op::CMPXCHG) {
// For CMPXCHG, store the original value in r0 if and only if the value matches
// what is in the src register. Until we track shared memory values,
// we just havoc R0. If we did track shared memory values, we'd need more
// complex logic here.
Reg r0{R0_RETURN_VALUE};
havoc_register(m_inv, r0);
type_inv.havoc_type(m_inv, r0);
} else if ((uint32_t)a.op & (uint32_t)AtomicOp::INST_ATOMIC_FETCH) {
} else if ((uint32_t)a.op & (uint32_t)Atomic::Op::FETCH) {
// For other FETCH operations, store the original value in the src register.
mem.value = a.valreg;
mem.is_load = true;
Expand Down
21 changes: 0 additions & 21 deletions src/ebpf_vm_isa.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,27 +90,6 @@ enum {
R11_ATOMIC_SCRATCH = 11, // Pseudo-register used internally for atomic instructions.
};

enum class AtomicOp {
INST_ATOMIC_FETCH = 0x01,
INST_ATOMIC_BASE_MASK = 0xf0, // Not including the FETCH flag.

// The following 'imm' values are the same as the corresponding
// 'code' values for arithmetic instructions.
INST_ATOMIC_ADD = 0x00,
INST_ATOMIC_ADD_FETCH = (INST_ATOMIC_ADD | INST_ATOMIC_FETCH),
INST_ATOMIC_OR = 0x40,
INST_ATOMIC_OR_FETCH = (INST_ATOMIC_OR | INST_ATOMIC_FETCH),
INST_ATOMIC_AND = 0x50,
INST_ATOMIC_AND_FETCH = (INST_ATOMIC_AND | INST_ATOMIC_FETCH),
INST_ATOMIC_XOR = 0xa0,
INST_ATOMIC_XOR_FETCH = (INST_ATOMIC_XOR | INST_ATOMIC_FETCH),
INST_ATOMIC_XCHG_BASE = 0xe0, // Not valid by itself
INST_ATOMIC_CMPXCHG_BASE = 0xf0, // Not valid by itself

INST_ATOMIC_XCHG = (INST_ATOMIC_XCHG_BASE | INST_ATOMIC_FETCH),
INST_ATOMIC_CMPXCHG = (INST_ATOMIC_CMPXCHG_BASE | INST_ATOMIC_FETCH),
};

int opcode_to_width(uint8_t opcode);
uint8_t width_to_opcode(int width);

Expand Down
20 changes: 10 additions & 10 deletions src/test/test_marshal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,16 @@ TEST_CASE("disasm_marshal", "[disasm][marshal]") {
for (int w : ws) {
if (w == 4 || w == 8) {
Deref access{.width = w, .basereg = Reg{2}, .offset = 17};
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_ADD, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_ADD_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_OR, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_OR_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_AND, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_AND_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_XOR, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_XOR_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_XCHG, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = AtomicOp::INST_ATOMIC_CMPXCHG, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::ADD, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::ADD_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::OR, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::OR_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::AND, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::AND_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::XOR, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::XOR_FETCH, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::XCHG, .access = access, .valreg = Reg{1}});
compare_marshal_unmarshal(Atomic{.op = Atomic::Op::CMPXCHG, .access = access, .valreg = Reg{1}});
}
}
}
Expand Down

0 comments on commit 8f10aef

Please sign in to comment.