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.

Sign up for GitHub

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

No transform when checking + check code reorg (second attempt) #796

Merged
merged 4 commits into from
Nov 17, 2024
Merged

No transform when checking + check code reorg (second attempt) #796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e699599
no transform when checking + check code reorg
elazarg Nov 17, 2024
835e315
no abstract classes and no unique_ptr
elazarg Nov 17, 2024
03ca56c
no higher-order timed_execution()
elazarg Nov 17, 2024
e8dc77c
CR comment
elazarg Nov 17, 2024
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
13 changes: 7 additions & 6 deletions src/asm_cfg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// Copyright (c) Prevail Verifier contributors.
// SPDX-License-Identifier: MIT
#include <cassert>

#include <algorithm>
#include <cassert>
#include <map>
#include <optional>
#include <string>
Expand Down Expand Up @@ -87,7 +86,7 @@ static void add_cfg_nodes(cfg_t& cfg, const label_t& caller_label, const label_t
macro_labels.erase(macro_label);

if (stack_frame_prefix == macro_label.stack_frame_prefix) {
throw std::runtime_error{stack_frame_prefix + ": illegal recursion"};
throw crab::InvalidControlFlow{stack_frame_prefix + ": illegal recursion"};
}

// Clone the macro block into a new block with the new stack frame prefix.
Expand Down Expand Up @@ -143,7 +142,7 @@ static void add_cfg_nodes(cfg_t& cfg, const label_t& caller_label, const label_t
const label_t label(macro_label.from, macro_label.to, caller_label_str);
if (const auto pins = std::get_if<CallLocal>(&cfg.at(label).cmd)) {
if (stack_frame_depth >= MAX_CALL_STACK_FRAMES) {
throw std::runtime_error{"too many call stack frames"};
throw crab::InvalidControlFlow{"too many call stack frames"};
}
add_cfg_nodes(cfg, label, pins->target);
}
Expand All @@ -163,7 +162,7 @@ static cfg_t instruction_seq_to_cfg(const InstructionSeq& insts, const bool must
}

if (insts.size() == 0) {
throw std::invalid_argument{"empty instruction sequence"};
throw crab::InvalidControlFlow{"empty instruction sequence"};
elazarg marked this conversation as resolved.
Show resolved Hide resolved
} else {
const auto& [label, inst, _0] = insts[0];
cfg.get_node(cfg.entry_label()) >> cfg.get_node(label);
Expand All @@ -183,7 +182,7 @@ static cfg_t instruction_seq_to_cfg(const InstructionSeq& insts, const bool must
fallthrough = std::get<0>(insts[i + 1]);
} else {
if (has_fall(inst) && must_have_exit) {
throw std::invalid_argument{"fallthrough in last instruction"};
throw crab::InvalidControlFlow{"fallthrough in last instruction"};
}
}
if (const auto jmp = std::get_if<Jmp>(&inst)) {
Expand Down Expand Up @@ -230,6 +229,8 @@ static cfg_t instruction_seq_to_cfg(const InstructionSeq& insts, const bool must
}

cfg_t prepare_cfg(const InstructionSeq& prog, const program_info& info, const prepare_cfg_options& options) {
thread_local_program_info.set(info);

// Convert the instruction sequence to a deterministic control-flow graph.
cfg_t cfg = instruction_seq_to_cfg(prog, options.must_have_exit);

Expand Down
74 changes: 37 additions & 37 deletions src/asm_files.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ template <typename T>
requires std::is_trivially_copyable_v<T>
static vector<T> vector_of(const char* data, const ELFIO::Elf_Xword size) {
if (size % sizeof(T) != 0 || size > std::numeric_limits<uint32_t>::max() || !data) {
throw std::runtime_error("Invalid argument to vector_of");
throw UnmarshalError("Invalid argument to vector_of");
}
return {reinterpret_cast<const T*>(data), reinterpret_cast<const T*>(data + size)};
}
Expand All @@ -38,15 +38,15 @@ static vector<T> vector_of(const ELFIO::section& sec) {
int create_map_crab(const EbpfMapType& map_type, const uint32_t key_size, const uint32_t value_size,
const uint32_t max_entries, ebpf_verifier_options_t) {
const EquivalenceKey equiv{map_type.value_type, key_size, value_size, map_type.is_array ? max_entries : 0};
if (!global_program_info->cache.contains(equiv)) {
if (!thread_local_program_info->cache.contains(equiv)) {
// +1 so 0 is the null FD
global_program_info->cache[equiv] = gsl::narrow<int>(global_program_info->cache.size()) + 1;
thread_local_program_info->cache[equiv] = gsl::narrow<int>(thread_local_program_info->cache.size()) + 1;
}
return global_program_info->cache.at(equiv);
return thread_local_program_info->cache.at(equiv);
}

EbpfMapDescriptor* find_map_descriptor(const int map_fd) {
for (EbpfMapDescriptor& map : global_program_info->map_descriptors) {
for (EbpfMapDescriptor& map : thread_local_program_info->map_descriptors) {
if (map.original_fd == map_fd) {
return &map;
}
Expand Down Expand Up @@ -111,10 +111,10 @@ static size_t parse_map_sections(const ebpf_verifier_options_t& options, const e
if (map_count > 0) {
map_record_size = s->get_size() / map_count;
if (s->get_data() == nullptr || map_record_size == 0) {
throw std::runtime_error("bad maps section");
throw UnmarshalError("bad maps section");
}
if (s->get_size() % map_record_size != 0) {
throw std::runtime_error("bad maps section size");
throw UnmarshalError("bad maps section size");
}
platform->parse_maps_section(map_descriptors, s->get_data(), map_record_size, map_count, platform, options);
}
Expand All @@ -131,9 +131,9 @@ vector<raw_program> read_elf(const string& path, const string& desired_section,
}
struct stat st;
if (stat(path.c_str(), &st)) {
throw std::runtime_error(string(strerror(errno)) + " opening " + path);
throw UnmarshalError(string(strerror(errno)) + " opening " + path);
elazarg marked this conversation as resolved.
Show resolved Hide resolved
}
throw std::runtime_error("Can't process ELF file " + path);
throw UnmarshalError("Can't process ELF file " + path);
}

static std::tuple<string, ELFIO::Elf_Xword>
Expand Down Expand Up @@ -168,8 +168,8 @@ void relocate_map(ebpf_inst& inst, const std::string& symbol_name,
const ELFIO::const_symbol_section_accessor& symbols) {
// Only permit loading the address of the map.
if ((inst.opcode & INST_CLS_MASK) != INST_CLS_LD) {
throw std::runtime_error("Illegal operation on symbol " + symbol_name + " at location " +
std::to_string(offset / sizeof(ebpf_inst)));
throw UnmarshalError("Illegal operation on symbol " + symbol_name + " at location " +
std::to_string(offset / sizeof(ebpf_inst)));
}
inst.src = 1; // magic number for LoadFd

Expand All @@ -190,8 +190,8 @@ void relocate_map(ebpf_inst& inst, const std::string& symbol_name,
}
}
if (reloc_value >= info.map_descriptors.size()) {
throw std::runtime_error("Bad reloc value (" + std::to_string(reloc_value) + "). " +
"Make sure to compile with -O2.");
throw UnmarshalError("Bad reloc value (" + std::to_string(reloc_value) + "). " +
"Make sure to compile with -O2.");
}
inst.imm = info.map_descriptors.at(reloc_value).original_fd;
}
Expand All @@ -213,17 +213,17 @@ static vector<ebpf_inst> read_subprogram(const ELFIO::section& subprogram_sectio
auto [subprogram_name, subprogram_size] =
get_program_name_and_size(subprogram_section, subprogram_offset, symbols);
if (subprogram_size == 0) {
throw std::runtime_error("Zero-size subprogram '" + subprogram_name + "' in section '" +
subprogram_section.get_name() + "'");
throw UnmarshalError("Zero-size subprogram '" + subprogram_name + "' in section '" +
subprogram_section.get_name() + "'");
}
if (subprogram_name == symbol_name) {
// Append subprogram instructions to the main program.
return vector_of<ebpf_inst>(subprogram_section.get_data() + subprogram_offset, subprogram_size);
}
subprogram_offset += subprogram_size;
}
throw std::runtime_error("Subprogram '" + symbol_name + "' not found in section '" + subprogram_section.get_name() +
"'");
throw UnmarshalError("Subprogram '" + symbol_name + "' not found in section '" + subprogram_section.get_name() +
"'");
}

static void append_subprograms(raw_program& prog, const vector<raw_program>& programs,
Expand Down Expand Up @@ -254,8 +254,8 @@ static void append_subprograms(raw_program& prog, const vector<raw_program>& pro
const int64_t target_offset = gsl::narrow_cast<int64_t>(subprogram_offsets[reloc.target_function_name]);
const auto offset_diff = target_offset - gsl::narrow<int64_t>(reloc.source_offset) - 1;
if (offset_diff < std::numeric_limits<int32_t>::min() || offset_diff > std::numeric_limits<int32_t>::max()) {
throw std::runtime_error("Offset difference out of int32_t range for instruction at source offset " +
std::to_string(reloc.source_offset));
throw UnmarshalError("Offset difference out of int32_t range for instruction at source offset " +
std::to_string(reloc.source_offset));
}
prog.prog[reloc.source_offset].imm = gsl::narrow_cast<int32_t>(offset_diff);
}
Expand All @@ -282,20 +282,20 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
const ebpf_verifier_options_t& options, const ebpf_platform_t* platform) {
ELFIO::elfio reader;
if (!reader.load(input_stream)) {
throw std::runtime_error("Can't process ELF file " + path);
throw UnmarshalError("Can't process ELF file " + path);
}

auto symbol_section = reader.sections[".symtab"];
if (!symbol_section) {
throw std::runtime_error("No symbol section found in ELF file " + path);
throw UnmarshalError("No symbol section found in ELF file " + path);
}

// Make sure the ELFIO library will be able to parse the symbol section correctly.
auto expected_entry_size =
reader.get_class() == ELFIO::ELFCLASS32 ? sizeof(ELFIO::Elf32_Sym) : sizeof(ELFIO::Elf64_Sym);

if (symbol_section->get_entry_size() != expected_entry_size) {
throw std::runtime_error("Invalid symbol section found in ELF file " + path);
throw UnmarshalError("Invalid symbol section found in ELF file " + path);
}

program_info info{platform};
Expand All @@ -307,7 +307,7 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
if (btf) {
// Parse the BTF type data.
btf_data = vector_of<std::byte>(*btf);
if (options.dump_btf_types_json) {
if (options.verbosity_opts.dump_btf_types_json) {
std::stringstream output;
std::cout << "Dumping BTF data for" << path << std::endl;
// Dump the BTF data to cout for debugging purposes.
Expand All @@ -324,7 +324,7 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
parse_map_sections(options, platform, reader, info.map_descriptors, map_section_indices, symbols);
} else {
if (!btf_data.has_value()) {
throw std::runtime_error("No BTF section found in ELF file " + path);
throw UnmarshalError("No BTF section found in ELF file " + path);
}
map_record_size_or_map_offsets = parse_map_section(*btf_data, info.map_descriptors);
// Prevail requires:
Expand Down Expand Up @@ -390,7 +390,7 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path

if (prelocs) {
if (!prelocs->get_data()) {
throw std::runtime_error("Malformed relocation data");
throw UnmarshalError("Malformed relocation data");
}
ELFIO::const_relocation_section_accessor reloc{reader, prelocs};

Expand All @@ -410,7 +410,7 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
}
offset -= program_offset;
if (offset / sizeof(ebpf_inst) >= prog.prog.size()) {
throw std::runtime_error("Invalid relocation data");
throw UnmarshalError("Invalid relocation data");
}
ebpf_inst& inst = prog.prog[offset / sizeof(ebpf_inst)];

Expand All @@ -437,7 +437,6 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
unresolved_symbols_errors.push_back(unresolved_symbol);
}
}
prog.line_info.resize(prog.prog.size());
res.push_back(prog);
program_offset += program_size;
}
Expand All @@ -457,8 +456,8 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
for (const auto& unresolved_symbol : unresolved_symbols_errors) {
std::cerr << unresolved_symbol << std::endl;
}
throw std::runtime_error("There are relocations in section but no maps sections in file " + path +
"\nMake sure to inline all function calls.");
throw UnmarshalError("There are relocations in section but no maps sections in file " + path +
"\nMake sure to inline all function calls.");
}

auto btf_ext = reader.sections[".BTF.ext"];
Expand All @@ -469,10 +468,11 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path
if (program.section_name == section && instruction_offset >= program.insn_off &&
instruction_offset < program.insn_off + program.prog.size() * sizeof(ebpf_inst)) {
const size_t inst_index = (instruction_offset - program.insn_off) / sizeof(ebpf_inst);
if (inst_index >= program.line_info.size()) {
throw std::runtime_error("Invalid BTF data");
if (inst_index >= program.prog.size()) {
throw UnmarshalError("Invalid BTF data");
}
program.line_info[inst_index] = {file_name, source, line_number, column_number};
program.info.line_info.insert_or_assign(
inst_index, btf_line_info_t{file_name, source, line_number, column_number});
return;
}
}
Expand All @@ -482,20 +482,20 @@ vector<raw_program> read_elf(std::istream& input_stream, const std::string& path

// BTF doesn't include line info for every instruction, only on the first instruction per source line.
for (auto& program : res) {
for (size_t i = 1; i < program.line_info.size(); i++) {
for (size_t i = 1; i < program.info.line_info.size(); i++) {
// If the previous PC has line info, copy it.
if (program.line_info[i].line_number == 0 && program.line_info[i - 1].line_number != 0) {
program.line_info[i] = program.line_info[i - 1];
if (program.info.line_info[i].line_number == 0 && program.info.line_info[i - 1].line_number != 0) {
program.info.line_info[i] = program.info.line_info[i - 1];
}
}
}
}

if (res.empty()) {
if (desired_section.empty()) {
throw std::runtime_error("Can't find any non-empty TEXT sections in file " + path);
throw UnmarshalError("Can't find any non-empty TEXT sections in file " + path);
}
throw std::runtime_error("Can't find section " + desired_section + " in file " + path);
throw UnmarshalError("Can't find section " + desired_section + " in file " + path);
}
return res;
}
5 changes: 5 additions & 0 deletions src/asm_files.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
#include "asm_syntax.hpp"
#include "platform.hpp"

class UnmarshalError final : public std::runtime_error {
public:
explicit UnmarshalError(const std::string& what) : std::runtime_error(what) {}
};
elazarg marked this conversation as resolved.
Show resolved Hide resolved

std::vector<raw_program> read_raw(std::string path, program_info info);
std::vector<raw_program> read_elf(const std::string& path, const std::string& desired_section,
const ebpf_verifier_options_t& options, const ebpf_platform_t* platform);
Expand Down
2 changes: 1 addition & 1 deletion src/asm_ostream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ struct AssertionPrinterVisitor {
}

void operator()(ValidCall const& a) {
const EbpfHelperPrototype proto = global_program_info->platform->get_helper_prototype(a.func);
const EbpfHelperPrototype proto = thread_local_program_info->platform->get_helper_prototype(a.func);
_os << "valid call(" << proto.name << ")";
}

Expand Down
10 changes: 5 additions & 5 deletions src/asm_unmarshal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ struct Unmarshaller {
}
}

vector<LabeledInstruction> unmarshal(vector<ebpf_inst> const& insts, vector<btf_line_info_t> const& line_info) {
vector<LabeledInstruction> unmarshal(vector<ebpf_inst> const& insts) {
vector<LabeledInstruction> prog;
int exit_count = 0;
if (insts.empty()) {
Expand Down Expand Up @@ -773,8 +773,8 @@ struct Unmarshaller {

std::optional<btf_line_info_t> current_line_info = {};

if (pc < line_info.size()) {
current_line_info = line_info[pc];
if (pc < info.line_info.size()) {
current_line_info = info.line_info.at(pc);
elazarg marked this conversation as resolved.
Show resolved Hide resolved
}

prog.emplace_back(label_t(gsl::narrow<int>(pc)), new_ins, current_line_info);
Expand All @@ -794,9 +794,9 @@ struct Unmarshaller {
};

std::variant<InstructionSeq, std::string> unmarshal(const raw_program& raw_prog, vector<vector<string>>& notes) {
global_program_info = raw_prog.info;
thread_local_program_info = raw_prog.info;
try {
return Unmarshaller{notes, raw_prog.info}.unmarshal(raw_prog.prog, raw_prog.line_info);
return Unmarshaller{notes, raw_prog.info}.unmarshal(raw_prog.prog);
elazarg marked this conversation as resolved.
Show resolved Hide resolved
} catch (InvalidInstruction& arg) {
std::ostringstream ss;
ss << arg.pc << ": " << arg.what() << "\n";
Expand Down
6 changes: 5 additions & 1 deletion src/assertions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,18 @@ class AssertExtractor {
vector<Assertion> operator()(const Mem& ins) const {
vector<Assertion> res;
const Reg basereg = ins.access.basereg;
Imm width{static_cast<uint32_t>(ins.access.width)};
Imm width{gsl::narrow<uint32_t>(ins.access.width)};
const int offset = ins.access.offset;
if (basereg.v == R10_STACK_POINTER) {
// We know we are accessing the stack.
if (offset < -EBPF_SUBPROGRAM_STACK_SIZE || offset + static_cast<int>(width.v) > 0) {
// This assertion will fail
res.emplace_back(make_valid_access(basereg, offset, width, false,
ins.is_load ? AccessType::read : AccessType::write));
} else if (ins.is_load) {
// This assertion is not for bound checks but for reading initialized memory.
// TODO: avoid load assertions: use post-assertion to check that the loaded value is initialized
// res.emplace_back(make_valid_access(basereg, offset, width, false, AccessType::read));
elazarg marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
res.emplace_back(TypeConstraint{basereg, TypeGroup::pointer});
Expand Down
Loading
Loading