Skip to content

Commit

Permalink
i#6662 public traces: allow one missing virtual_address in v2p.textpr…
Browse files Browse the repository at this point in the history
…oto (#7174)

The textproto format does not have fields with value == 0.
In some cases a trace can use virtual address 0x0, which
would result in a missing virtual_address field in `v2p.textproto`.
Previously we did not allow any missing fields in `v2p.textproto`,
so this case needs to be handled.
We do so relying on the fact that multiple missing virtual_address
fields would try to map 0x0 multiple times, resulting in an error.

We add the missing virtual_address case to the unit tests.

Issue #6662
  • Loading branch information
edeiana authored Jan 7, 2025
1 parent 7922cfa commit 4dfb878
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 15 deletions.
18 changes: 8 additions & 10 deletions clients/drcachesim/reader/v2p_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ v2p_reader_t::create_v2p_info_from_file(std::istream &v2p_file, v2p_info_t &v2p_
const std::string bytes_mapped_key = "bytes_mapped";
const std::string virtual_address_key = "virtual_address";
const std::string physical_address_key = "physical_address";
// Assumes virtual_address 0 is not in the v2p file.
addr_t virtual_address = 0;
uint64_t value = 0;
std::string error_str;
Expand All @@ -103,20 +102,19 @@ v2p_reader_t::create_v2p_info_from_file(std::istream &v2p_file, v2p_info_t &v2p_

found = line.find(physical_address_key);
if (found != std::string::npos) {
error_str = get_value_from_line(line, value);
if (!error_str.empty())
return error_str;
addr_t physical_address = static_cast<addr_t>(value);
if (virtual_address == 0) {
error_ss << "ERROR: no corresponding " << virtual_address_key << " for "
<< physical_address_key << " " << physical_address << ".";
return error_ss.str();
}
// Two missing virtual_address fields in the textproto will hit this
// condition, as virtual_address will be 0 for both.
// We do allow one missing virtual_address in case the trace uses
// virtual_address 0.
if (v2p_info.v2p_map.count(virtual_address) > 0) {
error_ss << "ERROR: " << virtual_address_key << " " << virtual_address
<< " is already present in v2p_map.";
return error_ss.str();
}
error_str = get_value_from_line(line, value);
if (!error_str.empty())
return error_str;
addr_t physical_address = static_cast<addr_t>(value);
v2p_info.v2p_map[virtual_address] = physical_address;
}
virtual_address = 0;
Expand Down
3 changes: 3 additions & 0 deletions clients/drcachesim/reader/v2p_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
* }
* In create_v2p_info_from_file() we rely on the fact that virtual_address and
* physical_address are one after the other on two different lines.
* The only exception is a single missing virtual_address field, which indicates
* virtual_address = 0x0 ("no presence" == "0" in a textproto). This can happen if the
* trace makes use of that virtual address.
* The virtual-to-physical mapping along with the page size, page count, and number of
* bytes mapped is stored in memory in a v2p_info_t object.
*/
Expand Down
4 changes: 4 additions & 0 deletions clients/drcachesim/tests/v2p_example.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ address_mapping_group {
virtual_address: 0x456
physical_address: 0x4
}
address_mapping {
# Missing virtual_address field on purpose (== virtual_address: 0x0)
physical_address: 0x6
}
address_mapping {
virtual_address: 0x789
physical_address: 0x5
Expand Down
11 changes: 6 additions & 5 deletions clients/drcachesim/tests/v2p_reader_unit_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,19 @@ namespace drmemtrace {
static void
check_v2p_info(const v2p_info_t &v2p_info)
{
// Change the number of entries if v2p_map_example.textproto is updated.
// Change NUM_ENTRIES if clients/drcachesim/tests/v2p_example.textproto is updated.
// Must be equal to the number of "address_mapping {...}" blocks in the textproto.
constexpr size_t NUM_ENTRIES = 3;
constexpr size_t NUM_ENTRIES = 4;
if (v2p_info.v2p_map.size() != NUM_ENTRIES) {
std::cerr << "v2p_map incorrect number of entries. Expected " << NUM_ENTRIES
<< " got " << v2p_info.v2p_map.size() << ".\n";
exit(1);
}

// Virtual and physical addresses must be aligned with v2p_example.textproto.
const std::vector<addr_t> virtual_addresses = { 0x123, 0x456, 0x789 };
const std::vector<addr_t> physical_addresses = { 0x3, 0x4, 0x5 };
// Virtual and physical addresses must be aligned with
// clients/drcachesim/tests/v2p_example.textproto.
const std::vector<addr_t> virtual_addresses = { 0x123, 0x456, 0x0, 0x789 };
const std::vector<addr_t> physical_addresses = { 0x3, 0x4, 0x6, 0x5 };
for (int i = 0; i < virtual_addresses.size(); ++i) {
auto key_val = v2p_info.v2p_map.find(virtual_addresses[i]);
if (key_val != v2p_info.v2p_map.end()) {
Expand Down

0 comments on commit 4dfb878

Please sign in to comment.