Skip to content

Commit 8f73067

Browse files
committed
refactor: add unique_mmap smart pointer
1 parent 62d14a7 commit 8f73067

15 files changed

+68
-58
lines changed

src/jsonrpc-remote-machine.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,7 +1138,7 @@ static json jsonrpc_machine_read_memory_handler(const json &j, const std::shared
11381138
auto args = parse_args<uint64_t, uint64_t>(j, param_name);
11391139
auto address = std::get<0>(args);
11401140
auto length = std::get<1>(args);
1141-
auto data = cartesi::unique_calloc<unsigned char>(length);
1141+
auto data = cartesi::make_unique_calloc<unsigned char>(length);
11421142
session->handler->machine->read_memory(address, data.get(), length);
11431143
return jsonrpc_response_ok(j, cartesi::encode_base64(data.get(), length));
11441144
}
@@ -1172,7 +1172,7 @@ static json jsonrpc_machine_read_virtual_memory_handler(const json &j, const std
11721172
auto args = parse_args<uint64_t, uint64_t>(j, param_name);
11731173
auto address = std::get<0>(args);
11741174
auto length = std::get<1>(args);
1175-
auto data = cartesi::unique_calloc<unsigned char>(length);
1175+
auto data = cartesi::make_unique_calloc<unsigned char>(length);
11761176
session->handler->machine->read_virtual_memory(address, data.get(), length);
11771177
return jsonrpc_response_ok(j, cartesi::encode_base64(data.get(), length));
11781178
}

src/machine.cpp

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
#include "machine-memory-range-descr.h"
5252
#include "machine-reg.h"
5353
#include "machine-runtime-config.h"
54-
#include "os.h"
5554
#include "plic-factory.h"
5655
#include "pma-constants.h"
5756
#include "pma-defines.h"
@@ -384,7 +383,7 @@ machine::machine(machine_config c, machine_runtime_config r) : m_c{std::move(c)}
384383
}
385384
// Auto detect flash drive image length
386385
if (f.length == UINT64_C(-1)) {
387-
auto fp = unique_fopen(f.image_filename.c_str(), "rb");
386+
auto fp = make_unique_fopen(f.image_filename.c_str(), "rb");
388387
if (fseek(fp.get(), 0, SEEK_END) != 0) {
389388
throw std::system_error{errno, std::generic_category(),
390389
"unable to obtain length of image file '"s + f.image_filename + "' when initializing "s +
@@ -524,11 +523,8 @@ machine::machine(machine_config c, machine_runtime_config r) : m_c{std::move(c)}
524523
// Initialize TLB device.
525524
// This must be done after all PMA entries are already registered, so we can lookup page addresses
526525
if (!m_c.tlb.image_filename.empty()) {
527-
unsigned char *buf = os_map_file(m_c.tlb.image_filename.c_str(), PMA_SHADOW_TLB_LENGTH, false);
528-
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
529-
const auto &shadow_tlb = *reinterpret_cast<const shadow_tlb_state *>(buf);
530-
init_tlb(shadow_tlb);
531-
os_unmap_file(buf, PMA_SHADOW_TLB_LENGTH);
526+
auto shadow_tlb = make_unique_mmap<shadow_tlb_state>(m_c.tlb.image_filename.c_str(), 1, false /* not shared */);
527+
init_tlb(*shadow_tlb);
532528
} else {
533529
init_tlb();
534530
}
@@ -573,7 +569,7 @@ machine::machine(machine_config c, machine_runtime_config r) : m_c{std::move(c)}
573569

574570
static void load_hash(const std::string &dir, machine::hash_type &h) {
575571
auto name = dir + "/hash";
576-
auto fp = unique_fopen(name.c_str(), "rb");
572+
auto fp = make_unique_fopen(name.c_str(), "rb");
577573
if (fread(h.data(), 1, h.size(), fp.get()) != h.size()) {
578574
throw std::runtime_error{"error reading from '" + name + "'"};
579575
}
@@ -737,9 +733,9 @@ static void store_device_pma(const machine &m, const pma_entry &pma, const std::
737733
if (!pma.get_istart_IO()) {
738734
throw std::runtime_error{"attempt to save non-device PMA"};
739735
}
740-
auto scratch = unique_calloc<unsigned char>(PMA_PAGE_SIZE); // will throw if it fails
736+
auto scratch = make_unique_calloc<unsigned char>(PMA_PAGE_SIZE); // will throw if it fails
741737
auto name = machine_config::get_image_filename(dir, pma.get_start(), pma.get_length());
742-
auto fp = unique_fopen(name.c_str(), "wb");
738+
auto fp = make_unique_fopen(name.c_str(), "wb");
743739
for (uint64_t page_start_in_range = 0; page_start_in_range < pma.get_length();
744740
page_start_in_range += PMA_PAGE_SIZE) {
745741
const unsigned char *page_data = nullptr;
@@ -762,7 +758,7 @@ static void store_memory_pma(const pma_entry &pma, const std::string &dir) {
762758
throw std::runtime_error{"attempt to save non-memory PMA"};
763759
}
764760
auto name = machine_config::get_image_filename(dir, pma.get_start(), pma.get_length());
765-
auto fp = unique_fopen(name.c_str(), "wb");
761+
auto fp = make_unique_fopen(name.c_str(), "wb");
766762
const pma_memory &mem = pma.get_memory();
767763
if (fwrite(mem.get_host_memory(), 1, pma.get_length(), fp.get()) != pma.get_length()) {
768764
throw std::runtime_error{"error writing to '" + name + "'"};
@@ -938,7 +934,7 @@ void machine::store_pmas(const machine_config &c, const std::string &dir) const
938934

939935
static void store_hash(const machine::hash_type &h, const std::string &dir) {
940936
auto name = dir + "/hash";
941-
auto fp = unique_fopen(name.c_str(), "wb");
937+
auto fp = make_unique_fopen(name.c_str(), "wb");
942938
if (fwrite(h.data(), 1, h.size(), fp.get()) != h.size()) {
943939
throw std::runtime_error{"error writing to '" + name + "'"};
944940
}
@@ -1820,7 +1816,7 @@ bool machine::verify_dirty_page_maps() const {
18201816
static_assert(PMA_PAGE_SIZE == machine_merkle_tree::get_page_size(),
18211817
"PMA and machine_merkle_tree page sizes must match");
18221818
machine_merkle_tree::hasher_type h;
1823-
auto scratch = unique_calloc<unsigned char>(PMA_PAGE_SIZE, std::nothrow_t{});
1819+
auto scratch = make_unique_calloc<unsigned char>(PMA_PAGE_SIZE, std::nothrow_t{});
18241820
if (!scratch) {
18251821
return false;
18261822
}
@@ -1885,7 +1881,7 @@ bool machine::update_merkle_tree() const {
18851881
// runtime config or as the hardware supports.
18861882
const uint64_t n = get_task_concurrency(m_r.concurrency.update_merkle_tree);
18871883
const bool succeeded = os_parallel_for(n, [&](int j, const parallel_for_mutex &mutex) -> bool {
1888-
auto scratch = unique_calloc<unsigned char>(PMA_PAGE_SIZE, std::nothrow_t{});
1884+
auto scratch = make_unique_calloc<unsigned char>(PMA_PAGE_SIZE, std::nothrow_t{});
18891885
if (!scratch) {
18901886
return false;
18911887
}
@@ -1949,7 +1945,7 @@ bool machine::update_merkle_tree_page(uint64_t address) {
19491945
pma_entry &pma = find_pma_entry(m_merkle_pmas, address, sizeof(uint64_t));
19501946
const uint64_t page_start_in_range = address - pma.get_start();
19511947
machine_merkle_tree::hasher_type h;
1952-
auto scratch = unique_calloc<unsigned char>(PMA_PAGE_SIZE, std::nothrow_t{});
1948+
auto scratch = make_unique_calloc<unsigned char>(PMA_PAGE_SIZE, std::nothrow_t{});
19531949
if (!scratch) {
19541950
return false;
19551951
}
@@ -1999,7 +1995,7 @@ machine::hash_type machine::get_merkle_tree_node_hash(uint64_t address, int log2
19991995
throw std::domain_error{"address not aligned to log2_size"};
20001996
}
20011997
const auto size = UINT64_C(1) << log2_size;
2002-
auto scratch = unique_calloc<unsigned char>(size);
1998+
auto scratch = make_unique_calloc<unsigned char>(size);
20031999
read_memory(address, scratch.get(), size);
20042000
machine_merkle_tree::hasher_type h;
20052001
machine_merkle_tree::hash_type hash;
@@ -2064,7 +2060,7 @@ machine_merkle_tree::proof_type machine::get_proof(uint64_t address, int log2_si
20642060
if (log2_size < machine_merkle_tree::get_log2_page_size()) {
20652061
const uint64_t length = UINT64_C(1) << log2_size;
20662062
const pma_entry &pma = find_pma_entry(m_merkle_pmas, address, length);
2067-
auto scratch = unique_calloc<unsigned char>(PMA_PAGE_SIZE);
2063+
auto scratch = make_unique_calloc<unsigned char>(PMA_PAGE_SIZE);
20682064
const unsigned char *page_data = nullptr;
20692065
// If the PMA range is empty, we know the desired range is
20702066
// entirely outside of any non-pristine PMA.
@@ -2491,16 +2487,15 @@ interpreter_break_reason machine::log_step(uint64_t mcycle_count, const std::str
24912487
interpreter_break_reason machine::verify_step(const hash_type &root_hash_before, const std::string &filename,
24922488
uint64_t mcycle_count, const hash_type &root_hash_after) {
24932489
auto data_length = os_get_file_length(filename.c_str(), "step log file");
2494-
auto *data = os_map_file(filename.c_str(), data_length, false /* not shared */);
2490+
auto data = make_unique_mmap<unsigned char>(filename.c_str(), data_length, false /* not shared */);
24952491
replay_step_state_access::context context;
2496-
replay_step_state_access a(context, data, data_length, root_hash_before);
2492+
replay_step_state_access a(context, data.get(), data_length, root_hash_before);
24972493
uint64_t mcycle_end{};
24982494
if (__builtin_add_overflow(a.read_mcycle(), mcycle_count, &mcycle_end)) {
24992495
mcycle_end = UINT64_MAX;
25002496
}
25012497
auto break_reason = interpret(a, mcycle_end);
25022498
a.finish(root_hash_after);
2503-
os_unmap_file(data, data_length);
25042499
return break_reason;
25052500
}
25062501

src/merkle-tree-hash.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ int main(int argc, char *argv[]) {
231231
// Read from stdin if no input name was given
232232
auto input_file = unique_file_ptr{stdin};
233233
if (input_name != nullptr) {
234-
input_file = unique_fopen(input_name, "ro", std::nothrow_t{});
234+
input_file = make_unique_fopen(input_name, "ro", std::nothrow_t{});
235235
if (!input_file) {
236236
error("unable to open input file '%s'\n", input_name);
237237
return 1;
@@ -240,7 +240,7 @@ int main(int argc, char *argv[]) {
240240

241241
// Allocate buffer for leaf data
242242
const uint64_t leaf_size = UINT64_C(1) << log2_leaf_size;
243-
auto leaf_buf = unique_calloc<unsigned char>(leaf_size, std::nothrow_t{});
243+
auto leaf_buf = make_unique_calloc<unsigned char>(leaf_size, std::nothrow_t{});
244244
if (!leaf_buf) {
245245
error("unable to allocate leaf buffer\n");
246246
return 1;

src/os.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ int os_mkdir(const char *path, [[maybe_unused]] int mode) {
557557
#endif // HAVE_MKDIR
558558
}
559559

560-
unsigned char *os_map_file(const char *path, uint64_t length, bool shared) {
560+
void *os_map_file(const char *path, uint64_t length, bool shared) {
561561
if ((path == nullptr) || *path == '\0') {
562562
throw std::runtime_error{"image file path must be specified"s};
563563
}
@@ -589,8 +589,7 @@ unsigned char *os_map_file(const char *path, uint64_t length, bool shared) {
589589

590590
// Try to map image file to host memory
591591
const int mflag = shared ? MAP_SHARED : MAP_PRIVATE;
592-
auto *host_memory =
593-
static_cast<unsigned char *>(mmap(nullptr, length, PROT_READ | PROT_WRITE, mflag, backing_file, 0));
592+
void *host_memory = mmap(nullptr, length, PROT_READ | PROT_WRITE, mflag, backing_file, 0);
594593
if (host_memory == MAP_FAILED) { // NOLINT(cppcoreguidelines-pro-type-cstyle-cast,performance-no-int-to-ptr)
595594
close(backing_file);
596595
throw std::system_error{errno, std::generic_category(), "could not map image file '"s + path + "' to memory"s};
@@ -636,7 +635,7 @@ unsigned char *os_map_file(const char *path, uint64_t length, bool shared) {
636635
}
637636

638637
DWORD dwDesiredAccess = shared ? FILE_MAP_WRITE : FILE_MAP_COPY;
639-
auto *host_memory = static_cast<unsigned char *>(MapViewOfFile(hFileMappingObject, dwDesiredAccess, 0, 0, length));
638+
void *host_memory = MapViewOfFile(hFileMappingObject, dwDesiredAccess, 0, 0, length);
640639
if (!host_memory) {
641640
_close(backing_file);
642641
throw std::system_error{errno, std::generic_category(), "could not map image file '"s + path + "' to memory"s};
@@ -687,7 +686,7 @@ unsigned char *os_map_file(const char *path, uint64_t length, bool shared) {
687686
#endif // HAVE_MMAP
688687
}
689688

690-
void os_unmap_file(unsigned char *host_memory, [[maybe_unused]] uint64_t length) {
689+
void os_unmap_file(void *host_memory, [[maybe_unused]] uint64_t length) {
691690
#ifdef HAVE_MMAP
692691
munmap(host_memory, length);
693692

@@ -981,7 +980,7 @@ int os_double_fork([[maybe_unused]] bool emancipate, [[maybe_unused]] const char
981980
}
982981

983982
int64_t os_get_file_length(const char *filename, const char *text) {
984-
auto fp = unique_fopen(filename, "rb");
983+
auto fp = make_unique_fopen(filename, "rb");
985984
if (fseek(fp.get(), 0, SEEK_END) != 0) {
986985
throw std::system_error{errno, std::generic_category(),
987986
"unable to obtain length of file '"s + filename + "' "s + text};

src/os.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,10 @@ void os_silence_putchar(bool yes);
9494
int os_mkdir(const char *path, int mode);
9595

9696
/// \brief Maps a file to memory
97-
unsigned char *os_map_file(const char *path, uint64_t length, bool shared);
97+
void *os_map_file(const char *path, uint64_t length, bool shared);
9898

9999
/// \brief Unmaps a file from memory
100-
void os_unmap_file(unsigned char *host_memory, uint64_t length);
100+
void os_unmap_file(void *host_memory, uint64_t length);
101101

102102
/// \brief Get time elapsed since its first call with microsecond precision
103103
int64_t os_now_us();

src/pma-constants.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ enum PMA_ranges : uint64_t {
6969
enum PMA_constants : uint64_t {
7070
PMA_PAGE_SIZE_LOG2 = EXPAND_UINT64_C(PMA_PAGE_SIZE_LOG2_DEF), ///< log<sub>2</sub> of physical memory page size.
7171
PMA_PAGE_SIZE = (UINT64_C(1) << PMA_PAGE_SIZE_LOG2_DEF), ///< Physical memory page size.
72-
PMA_WORD_SIZE = EXPAND_UINT64_C(PMA_WORD_SIZE_DEF), ///< Physical memory word size.
7372
PMA_MAX = EXPAND_UINT64_C(PMA_MAX_DEF) ///< Maximum number of PMAs
7473
};
7574

src/pma-defines.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
#define PMA_RAM_START_DEF 0x80000000 ///< RAM start address
4949

5050
#define PMA_PAGE_SIZE_LOG2_DEF 12 ///< log<sub>2</sub> of physical memory page size.
51-
#define PMA_WORD_SIZE_DEF 8 ///< Physical memory word size.
5251
#define PMA_MAX_DEF 32 ///< Maximum number of PMAs
5352
#define PMA_PLIC_MAX_IRQ_DEF 31 ///< Maximum PLIC interrupt
5453

src/pma.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ pma_memory::pma_memory(const std::string &description, uint64_t length, const st
7878
pma_memory{description, length, c} {
7979
// Try to load image file, if any
8080
if (!path.empty()) {
81-
auto fp = unique_fopen(path.c_str(), "rb", std::nothrow_t{});
81+
auto fp = make_unique_fopen(path.c_str(), "rb", std::nothrow_t{});
8282
if (!fp) {
8383
throw std::system_error{errno, std::generic_category(),
8484
"error opening image file '"s + path + "' when initializing "s + description};
@@ -111,7 +111,7 @@ pma_memory::pma_memory(const std::string &description, uint64_t length, const st
111111
m_host_memory{nullptr},
112112
m_mmapped{false} {
113113
try {
114-
m_host_memory = os_map_file(path.c_str(), length, m.shared);
114+
m_host_memory = static_cast<unsigned char *>(os_map_file(path.c_str(), length, m.shared));
115115
m_mmapped = true;
116116
} catch (std::exception &e) {
117117
throw std::runtime_error{e.what() + " when initializing "s + description};

src/pma.h

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,7 @@ class pma_device final {
8585

8686
/// \brief Returns context to pass to callbacks.
8787
void *get_context() const {
88-
// Discard qualifier on purpose because the context
89-
// is none of our business.
90-
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
91-
return const_cast<void *>(m_context);
88+
return m_context;
9289
}
9390

9491
/// \brief Returns pointer to driver with callbacks
@@ -165,11 +162,6 @@ class pma_memory final {
165162
const unsigned char *get_host_memory() const {
166163
return m_host_memory;
167164
}
168-
169-
/// \brief Returns copy of PMA length field (needed for munmap).
170-
uint64_t get_length() const {
171-
return m_length;
172-
}
173165
};
174166

175167
/// \brief Data for empty memory ranges (nothing, really)

src/record-step-state-access.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ class record_step_state_access :
103103
// We store the page index, instead of the page address.
104104
// Scratch area is used by the replay to store page hashes, which change during replay
105105
// This is to work around the lack of dynamic memory allocation when replaying the log in microarchitectures
106-
auto fp = unique_fopen(m_context.filename.c_str(), "wb");
106+
auto fp = make_unique_fopen(m_context.filename.c_str(), "wb");
107107
if (fwrite(&page_count, sizeof(page_count), 1, fp.get()) != 1) {
108108
throw std::runtime_error("Could not write page count to log file");
109109
}

src/replay-send-cmio-state-access.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ class replay_send_cmio_state_access : public i_state_access<replay_send_cmio_sta
326326
const auto &written_hash = access.get_written_hash().value();
327327
// compute hash of data argument padded with zeroes
328328
hash_type computed_data_hash{};
329-
auto scratch = unique_calloc<unsigned char>(write_length, std::nothrow_t{});
329+
auto scratch = make_unique_calloc<unsigned char>(write_length, std::nothrow_t{});
330330
if (!scratch) {
331331
throw std::runtime_error("Could not allocate scratch memory");
332332
}

src/replay-step-state-access.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class replay_step_state_access :
122122
if (!validate_and_advance_offset(log_size, 0, sizeof(m_context.page_count), 1, &first_page_offset)) {
123123
interop_throw_runtime_error("page count past end of step log");
124124
}
125-
memcpy(&m_context.page_count, log_image, sizeof(m_context.page_count));
125+
m_context.page_count = aliased_aligned_read<uint64_t, uint8_t>(log_image);
126126
if (m_context.page_count == 0) {
127127
interop_throw_runtime_error("page count is zero");
128128
}
@@ -140,7 +140,7 @@ class replay_step_state_access :
140140
&first_sibling_offset)) {
141141
interop_throw_runtime_error("sibling count past end of step log");
142142
}
143-
memcpy(&m_context.sibling_count, log_image + sibling_count_offset, sizeof(m_context.sibling_count));
143+
m_context.sibling_count = aliased_aligned_read<uint64_t, uint8_t>(log_image + sibling_count_offset);
144144

145145
// set sibling hashes
146146
if (!validate_and_advance_offset(log_size, first_sibling_offset, sizeof(hash_type), m_context.sibling_count,

0 commit comments

Comments
 (0)