Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use gsl::narrow in asm_marshal.cpp #712

Merged
merged 1 commit into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 23 additions & 22 deletions src/asm_marshal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "asm_marshal.hpp"
#include "asm_ostream.hpp"
#include "crab_utils/num_safety.hpp"

using std::vector;

Expand Down Expand Up @@ -88,9 +89,9 @@ static uint8_t imm_endian(const Un::Op op) {
struct MarshalVisitor {
private:
static vector<ebpf_inst> makeLddw(const Reg dst, const bool isFd, const int32_t imm, const int32_t next_imm) {
return {ebpf_inst{.opcode = static_cast<uint8_t>(INST_CLS_LD | width_to_opcode(8)),
return {ebpf_inst{.opcode = gsl::narrow<uint8_t>(INST_CLS_LD | width_to_opcode(8)),
.dst = dst.v,
.src = static_cast<uint8_t>(isFd ? 1 : 0),
.src = gsl::narrow<uint8_t>(isFd ? 1 : 0),
.offset = 0,
.imm = imm},
ebpf_inst{.opcode = 0, .dst = 0, .src = 0, .offset = 0, .imm = next_imm}};
Expand All @@ -115,7 +116,7 @@ struct MarshalVisitor {
return makeLddw(b.dst, false, imm, next_imm);
}

ebpf_inst res{.opcode = static_cast<uint8_t>((b.is64 ? INST_CLS_ALU64 : INST_CLS_ALU) | (op(b.op) << 4)),
ebpf_inst res{.opcode = gsl::narrow<uint8_t>((b.is64 ? INST_CLS_ALU64 : INST_CLS_ALU) | (op(b.op) << 4)),
elazarg marked this conversation as resolved.
Show resolved Hide resolved
.dst = b.dst.v,
.src = 0,
.offset = offset(b.op),
Expand All @@ -124,7 +125,7 @@ struct MarshalVisitor {
res.opcode |= INST_SRC_REG;
res.src = right.v;
},
[&](const Imm right) { res.imm = static_cast<int32_t>(right.v); }},
[&](const Imm right) { res.imm = gsl::narrow<int32_t>(right.v); }},
b.v);
return {res};
}
Expand All @@ -133,7 +134,7 @@ struct MarshalVisitor {
switch (b.op) {
case Un::Op::NEG:
return {ebpf_inst{
.opcode = static_cast<uint8_t>((b.is64 ? INST_CLS_ALU64 : INST_CLS_ALU) | INST_ALU_OP_NEG),
.opcode = gsl::narrow<uint8_t>((b.is64 ? INST_CLS_ALU64 : INST_CLS_ALU) | INST_ALU_OP_NEG),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand All @@ -143,7 +144,7 @@ struct MarshalVisitor {
case Un::Op::LE32:
case Un::Op::LE64:
return {ebpf_inst{
.opcode = static_cast<uint8_t>(INST_CLS_ALU | INST_END_LE | INST_ALU_OP_END),
.opcode = gsl::narrow<uint8_t>(INST_CLS_ALU | INST_END_LE | INST_ALU_OP_END),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand All @@ -153,7 +154,7 @@ struct MarshalVisitor {
case Un::Op::BE32:
case Un::Op::BE64:
return {ebpf_inst{
.opcode = static_cast<uint8_t>(INST_CLS_ALU | INST_END_BE | INST_ALU_OP_END),
.opcode = gsl::narrow<uint8_t>(INST_CLS_ALU | INST_END_BE | INST_ALU_OP_END),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand All @@ -163,7 +164,7 @@ struct MarshalVisitor {
case Un::Op::SWAP32:
case Un::Op::SWAP64:
return {ebpf_inst{
.opcode = static_cast<uint8_t>(INST_CLS_ALU64 | INST_ALU_OP_END),
.opcode = gsl::narrow<uint8_t>(INST_CLS_ALU64 | INST_ALU_OP_END),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand All @@ -175,15 +176,15 @@ struct MarshalVisitor {
}

vector<ebpf_inst> operator()(Call const& b) const {
return {ebpf_inst{.opcode = static_cast<uint8_t>(INST_OP_CALL),
return {ebpf_inst{.opcode = gsl::narrow<uint8_t>(INST_OP_CALL),
.dst = 0,
.src = INST_CALL_STATIC_HELPER,
.offset = 0,
.imm = b.func}};
}

vector<ebpf_inst> operator()(CallLocal const& b) const {
return {ebpf_inst{.opcode = static_cast<uint8_t>(INST_OP_CALL),
return {ebpf_inst{.opcode = gsl::narrow<uint8_t>(INST_OP_CALL),
.dst = 0,
.src = INST_CALL_LOCAL,
.offset = 0,
Expand All @@ -192,7 +193,7 @@ struct MarshalVisitor {

vector<ebpf_inst> operator()(Callx const& b) const {
// callx is defined to have the register in 'dst' not in 'src'.
return {ebpf_inst{.opcode = static_cast<uint8_t>(INST_OP_CALLX),
return {ebpf_inst{.opcode = gsl::narrow<uint8_t>(INST_OP_CALLX),
.dst = b.func.v,
.src = INST_CALL_STATIC_HELPER,
.offset = 0}};
Expand All @@ -209,7 +210,7 @@ struct MarshalVisitor {
vector<ebpf_inst> operator()(Jmp const& b) const {
if (b.cond) {
ebpf_inst res{
.opcode = static_cast<uint8_t>(INST_CLS_JMP | (op(b.cond->op) << 4)),
.opcode = gsl::narrow<uint8_t>(INST_CLS_JMP | (op(b.cond->op) << 4)),
.dst = b.cond->left.v,
.src = 0,
.offset = label_to_offset16(b.target),
Expand All @@ -218,7 +219,7 @@ struct MarshalVisitor {
res.opcode |= INST_SRC_REG;
res.src = right.v;
},
[&](const Imm right) { res.imm = static_cast<int32_t>(right.v); }},
[&](const Imm right) { res.imm = gsl::narrow<int32_t>(right.v); }},
b.cond->right);
return {res};
} else {
Expand All @@ -234,17 +235,17 @@ struct MarshalVisitor {
vector<ebpf_inst> operator()(Mem const& b) const {
const Deref access = b.access;
ebpf_inst res{
.opcode = static_cast<uint8_t>(INST_MODE_MEM | width_to_opcode(access.width)),
.opcode = gsl::narrow<uint8_t>(INST_MODE_MEM | width_to_opcode(access.width)),
.dst = 0,
.src = 0,
.offset = static_cast<int16_t>(access.offset),
.offset = gsl::narrow<int16_t>(access.offset),
};
if (b.is_load) {
if (!std::holds_alternative<Reg>(b.value)) {
throw std::runtime_error(std::string("LD IMM: ") + to_string(b));
}
res.opcode |= INST_CLS_LD | 0x1;
res.dst = static_cast<uint8_t>(std::get<Reg>(b.value).v);
res.dst = gsl::narrow<uint8_t>(std::get<Reg>(b.value).v);
res.src = access.basereg.v;
} else {
res.opcode |= INST_CLS_ST;
Expand All @@ -254,19 +255,19 @@ struct MarshalVisitor {
res.src = preg->v;
} else {
res.opcode |= 0x0;
res.imm = static_cast<int32_t>(std::get<Imm>(b.value).v);
res.imm = gsl::narrow<int32_t>(std::get<Imm>(b.value).v);
}
}
return {res};
}

vector<ebpf_inst> operator()(Packet const& b) const {
ebpf_inst res{
.opcode = static_cast<uint8_t>(INST_CLS_LD | width_to_opcode(b.width)),
.opcode = gsl::narrow<uint8_t>(INST_CLS_LD | width_to_opcode(b.width)),
.dst = 0,
.src = 0,
.offset = 0,
.imm = static_cast<int32_t>(b.offset),
.imm = gsl::narrow<int32_t>(b.offset),
};
if (b.regoffset) {
res.opcode |= INST_MODE_IND;
Expand All @@ -278,15 +279,15 @@ struct MarshalVisitor {
}

vector<ebpf_inst> operator()(Atomic const& b) const {
auto imm = static_cast<int32_t>(b.op);
auto imm = gsl::narrow<int32_t>(b.op);
if (b.fetch) {
imm |= INST_FETCH;
}
return {
ebpf_inst{.opcode = static_cast<uint8_t>(INST_CLS_STX | INST_MODE_ATOMIC | width_to_opcode(b.access.width)),
ebpf_inst{.opcode = gsl::narrow<uint8_t>(INST_CLS_STX | INST_MODE_ATOMIC | width_to_opcode(b.access.width)),
.dst = b.access.basereg.v,
.src = b.valreg.v,
.offset = static_cast<int16_t>(b.access.offset),
.offset = gsl::narrow<int16_t>(b.access.offset),
.imm = imm}};
}

Expand Down
2 changes: 1 addition & 1 deletion src/asm_ostream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ std::ostream& operator<<(std::ostream& os, ValidAccess const& a) {
os << a.offset;
}

if (a.width == static_cast<Value>(Imm{0})) {
if (a.width == Value{Imm{0}}) {
// a.width == 0, meaning we only care it's an in-bound pointer,
// so it can be compared with another pointer to the same region.
os << ") for comparison/subtraction";
Expand Down
2 changes: 1 addition & 1 deletion src/crab_utils/graph_ops.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ class GraphOps {
++qtail;
}

for (vert_id v : scc) {
for (vert_id _ : scc) {
elazarg marked this conversation as resolved.
Show resolved Hide resolved
while (qtail != qhead) {
vert_id s = *--qtail;
// If it _was_ on the queue, it must be in the SCC
Expand Down
9 changes: 4 additions & 5 deletions src/crab_utils/num_safeint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
namespace crab {

class safe_i64 {

// Current implementation is based on
// https://blog.regehr.org/archives/1139 using wider integers.

Expand Down Expand Up @@ -70,13 +69,13 @@ class safe_i64 {
}

public:
safe_i64() : m_num(0) {}
safe_i64() : m_num{0} {}

safe_i64(const int64_t num) : m_num(num) {}
safe_i64(const int64_t num) : m_num{num} {}

safe_i64(const number_t& n) : m_num(n.narrow<int64_t>()) {}
safe_i64(const number_t& n) : m_num{n.narrow<int64_t>()} {}
elazarg marked this conversation as resolved.
Show resolved Hide resolved

operator int64_t() const { return (int64_t)m_num; }
operator int64_t() const { return m_num; }

// TODO: output parameters whether operation overflows
safe_i64 operator+(const safe_i64 x) const {
Expand Down
3 changes: 2 additions & 1 deletion src/crab_utils/stats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ long Stopwatch::systemTime() const {
}

// Convert from 100ns intervals to microseconds.
uint64_t total_us = (((uint64_t)user_time.dwHighDateTime << 32) | (uint64_t)user_time.dwLowDateTime) / 10;
uint64_t total_us =
((static_cast<uint64_t>(user_time.dwHighDateTime) << 32) | static_cast<uint64_t>(user_time.dwLowDateTime)) / 10;

return (long)total_us;
#else
Expand Down
Loading