Skip to content

Commit

Permalink
Fixed vtable duplication for binary annotator
Browse files Browse the repository at this point in the history
  • Loading branch information
@dbaileychess
dbaileychess committed Feb 2, 2023
1 parent f838017 commit e376ddf
Show file tree
Hide file tree
Showing 28 changed files with 6,772 additions and 6,753 deletions.
2 changes: 1 addition & 1 deletion src/annotated_binary_text_gen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ static std::string ToValueString(const BinaryRegion &region,
// value.
// TODO(dbaileychess): It might be nicer to put this in the comment field.
if (IsOffset(region.type)) {
s += " Loc: +0x";
s += " Loc: 0x";
s += ToHex(region.points_to_offset, output_config.offset_max_char);
}
return s;
Expand Down
92 changes: 55 additions & 37 deletions src/binary_annotator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,22 @@ uint64_t BinaryAnnotator::BuildHeader(const uint64_t header_offset) {
return root_table_offset.value();
}

void BinaryAnnotator::BuildVTable(const uint64_t vtable_offset,
const reflection::Object *const table,
const uint64_t offset_of_referring_table) {
// First see if we have used this vtable before, if so skip building it again.
auto it = vtables_.find(vtable_offset);
if (it != vtables_.end()) { return; }
BinaryAnnotator::VTable *BinaryAnnotator::GetOrBuildVTable(
const uint64_t vtable_offset, const reflection::Object *const table,
const uint64_t offset_of_referring_table) {
// Get a list of vtables (if any) already defined at this offset.
std::list<VTable> &vtables = vtables_[vtable_offset];

// See if this vtable for the table type has been generated before.
for (VTable &vtable : vtables) {
if (vtable.referring_table == table) { return &vtable; }
}

if (ContainsSection(vtable_offset)) { return; }
// If we are trying to make a new vtable and it is already encompassed by
// another binary section, something is corrupted.
if (vtables.empty() && ContainsSection(vtable_offset)) { return nullptr; }

const std::string referring_table_name = table->name()->str();

BinaryRegionComment vtable_size_comment;
vtable_size_comment.type = BinaryRegionCommentType::VTableSize;
Expand All @@ -217,11 +225,11 @@ void BinaryAnnotator::BuildVTable(const uint64_t vtable_offset,

AddSection(vtable_offset,
MakeSingleRegionBinarySection(
table->name()->str(), BinarySectionType::VTable,
referring_table_name, BinarySectionType::VTable,
MakeBinaryRegion(vtable_offset, remaining,
BinaryRegionType::Unknown, remaining, 0,
vtable_size_comment)));
return;
return nullptr;
}

// Vtables start with the size of the vtable
Expand All @@ -232,23 +240,23 @@ void BinaryAnnotator::BuildVTable(const uint64_t vtable_offset,
// The vtable_size points to off the end of the binary.
AddSection(vtable_offset,
MakeSingleRegionBinarySection(
table->name()->str(), BinarySectionType::VTable,
referring_table_name, BinarySectionType::VTable,
MakeBinaryRegion(vtable_offset, sizeof(uint16_t),
BinaryRegionType::Uint16, 0, 0,
vtable_size_comment)));

return;
return nullptr;
} else if (vtable_size < 2 * sizeof(uint16_t)) {
SetError(vtable_size_comment, BinaryRegionStatus::ERROR_LENGTH_TOO_SHORT,
"4");
// The size includes itself and the table size which are both uint16_t.
AddSection(vtable_offset,
MakeSingleRegionBinarySection(
table->name()->str(), BinarySectionType::VTable,
referring_table_name, BinarySectionType::VTable,
MakeBinaryRegion(vtable_offset, sizeof(uint16_t),
BinaryRegionType::Uint16, 0, 0,
vtable_size_comment)));
return;
return nullptr;
}

std::vector<BinaryRegion> regions;
Expand All @@ -272,11 +280,11 @@ void BinaryAnnotator::BuildVTable(const uint64_t vtable_offset,
"2");

AddSection(offset, MakeSingleRegionBinarySection(
table->name()->str(), BinarySectionType::VTable,
referring_table_name, BinarySectionType::VTable,
MakeBinaryRegion(
offset, remaining, BinaryRegionType::Unknown,
remaining, 0, ref_table_len_comment)));
return;
return nullptr;
}

// Then they have the size of the table they reference.
Expand Down Expand Up @@ -395,7 +403,7 @@ void BinaryAnnotator::BuildVTable(const uint64_t vtable_offset,
(vtable_size - sizeof(uint16_t) - sizeof(uint16_t)) / sizeof(uint16_t);

// Prevent a bad binary from declaring a really large vtable_size, that we can
// not indpendently verify.
// not independently verify.
expectant_vtable_fields = std::min(
static_cast<uint16_t>(fields_processed * 3), expectant_vtable_fields);

Expand Down Expand Up @@ -427,15 +435,26 @@ void BinaryAnnotator::BuildVTable(const uint64_t vtable_offset,
field_comment));
}

sections_[vtable_offset] = MakeBinarySection(
table->name()->str(), BinarySectionType::VTable, std::move(regions));
// If we have never added this vtable before record the Binary section.
if (vtables.empty()) {
sections_[vtable_offset] = MakeBinarySection(
referring_table_name, BinarySectionType::VTable, std::move(regions));
} else {
// Add the current table name to the name of the section.
sections_[vtable_offset].name += ", " + referring_table_name;
}

VTable vtable;
vtable.referring_table = table;
vtable.fields = std::move(fields);
vtable.table_size = table_size;
vtable.vtable_size = vtable_size;

vtables_[vtable_offset] = vtable;
// Add this vtable to the collection of vtables at this offset.
vtables.push_back(std::move(vtable));

// Return the vtable we just added.
return &vtables.back();
}

void BinaryAnnotator::BuildTable(const uint64_t table_offset,
Expand Down Expand Up @@ -491,19 +510,17 @@ void BinaryAnnotator::BuildTable(const uint64_t table_offset,

// Parse the vtable first so we know what the rest of the fields in the table
// are.
BuildVTable(vtable_offset, table, table_offset);
const VTable *const vtable =
GetOrBuildVTable(vtable_offset, table, table_offset);

auto vtable_entry = vtables_.find(vtable_offset);
if (vtable_entry == vtables_.end()) {
if (vtable == nullptr) {
// There is no valid vtable for this table, so we cannot process the rest of
// the table entries.
return;
}

const VTable &vtable = vtable_entry->second;

// This is the size and length of this table.
const uint16_t table_size = vtable.table_size;
const uint16_t table_size = vtable->table_size;
uint64_t table_end_offset = table_offset + table_size;

if (!IsValidOffset(table_end_offset - 1)) {
Expand All @@ -516,7 +533,7 @@ void BinaryAnnotator::BuildTable(const uint64_t table_offset,
// not by their IDs. So copy them over to another vector that we can sort on
// the offset_from_table property.
std::vector<VTable::Entry> fields;
for (const auto &vtable_field : vtable.fields) {
for (const auto &vtable_field : vtable->fields) {
fields.push_back(vtable_field.second);
}

Expand Down Expand Up @@ -707,7 +724,8 @@ void BinaryAnnotator::BuildTable(const uint64_t table_offset,
regions.push_back(MakeBinaryRegion(
field_offset, sizeof(uint32_t), BinaryRegionType::UOffset, 0,
offset_of_next_item, offset_field_comment));
BuildVector(offset_of_next_item, table, field, table_offset, vtable);
BuildVector(offset_of_next_item, table, field, table_offset,
vtable->fields);
} break;

case reflection::BaseType::Union: {
Expand All @@ -716,8 +734,8 @@ void BinaryAnnotator::BuildTable(const uint64_t table_offset,
// The union type field is always one less than the union itself.
const uint16_t union_type_id = field->id() - 1;

auto vtable_field = vtable.fields.find(union_type_id);
if (vtable_field == vtable.fields.end()) {
auto vtable_field = vtable->fields.find(union_type_id);
if (vtable_field == vtable->fields.end()) {
// TODO(dbaileychess): need to capture this error condition.
break;
}
Expand Down Expand Up @@ -959,11 +977,10 @@ void BinaryAnnotator::BuildString(const uint64_t string_offset,
BinarySectionType::String, std::move(regions)));
}

void BinaryAnnotator::BuildVector(const uint64_t vector_offset,
const reflection::Object *const table,
const reflection::Field *const field,
const uint64_t parent_table_offset,
const VTable &vtable) {
void BinaryAnnotator::BuildVector(
const uint64_t vector_offset, const reflection::Object *const table,
const reflection::Field *const field, const uint64_t parent_table_offset,
const std::map<uint16_t, VTable::Entry> vtable_fields) {
if (ContainsSection(vector_offset)) { return; }

BinaryRegionComment vector_length_comment;
Expand Down Expand Up @@ -1011,7 +1028,7 @@ void BinaryAnnotator::BuildVector(const uint64_t vector_offset,
regions.push_back(MakeBinaryRegion(vector_offset, sizeof(uint32_t),
BinaryRegionType::Uint32, 0, 0,
vector_length_comment));

// Consume the vector length offset.
uint64_t offset = vector_offset + sizeof(uint32_t);

switch (field->type()->element()) {
Expand Down Expand Up @@ -1079,6 +1096,7 @@ void BinaryAnnotator::BuildVector(const uint64_t vector_offset,
offset, sizeof(uint32_t), BinaryRegionType::UOffset, 0,
table_offset, vector_object_comment));

// Consume the offset to the table.
offset += sizeof(uint32_t);

BuildTable(table_offset, BinarySectionType::Table, object);
Expand Down Expand Up @@ -1135,8 +1153,8 @@ void BinaryAnnotator::BuildVector(const uint64_t vector_offset,
// location.
const uint16_t union_type_vector_id = field->id() - 1;

auto vtable_entry = vtable.fields.find(union_type_vector_id);
if (vtable_entry == vtable.fields.end()) {
auto vtable_entry = vtable_fields.find(union_type_vector_id);
if (vtable_entry == vtable_fields.end()) {
// TODO(dbaileychess): need to capture this error condition.
break;
}
Expand Down
19 changes: 13 additions & 6 deletions src/binary_annotator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#ifndef FLATBUFFERS_BINARY_ANNOTATOR_H_
#define FLATBUFFERS_BINARY_ANNOTATOR_H_

#include <list>
#include <map>
#include <string>
#include <vector>
Expand Down Expand Up @@ -52,8 +53,8 @@ enum class BinaryRegionType {
template<typename T>
static inline std::string ToHex(T i, size_t width = sizeof(T)) {
std::stringstream stream;
stream << std::hex << std::uppercase << std::setfill('0') << std::setw(static_cast<int>(width))
<< i;
stream << std::hex << std::uppercase << std::setfill('0')
<< std::setw(static_cast<int>(width)) << i;
return stream.str();
}

Expand Down Expand Up @@ -257,6 +258,8 @@ class BinaryAnnotator {
uint16_t offset_from_table = 0;
};

const reflection::Object *referring_table;

// Field ID -> {field def, offset from table}
std::map<uint16_t, Entry> fields;

Expand All @@ -266,8 +269,12 @@ class BinaryAnnotator {

uint64_t BuildHeader(uint64_t offset);

void BuildVTable(uint64_t offset, const reflection::Object *table,
uint64_t offset_of_referring_table);
// VTables can be shared across instances or even across objects. This
// attempts to get an existing vtable given the offset and table type,
// otherwise it will built the vtable, memorize it, and return the built
// VTable. Returns nullptr if building the VTable fails.
VTable *GetOrBuildVTable(uint64_t offset, const reflection::Object *table,
uint64_t offset_of_referring_table);

void BuildTable(uint64_t offset, const BinarySectionType type,
const reflection::Object *table);
Expand All @@ -281,7 +288,7 @@ class BinaryAnnotator {

void BuildVector(uint64_t offset, const reflection::Object *table,
const reflection::Field *field, uint64_t parent_table_offset,
const VTable &vtable);
const std::map<uint16_t, VTable::Entry> vtable_fields);

std::string BuildUnion(uint64_t offset, uint8_t realized_type,
const reflection::Field *field);
Expand Down Expand Up @@ -382,7 +389,7 @@ class BinaryAnnotator {
const uint64_t binary_length_;

// Map of binary offset to vtables, to dedupe vtables.
std::map<uint64_t, VTable> vtables_;
std::map<uint64_t, std::list<VTable>> vtables_;

// The annotated binary sections, index by their absolute offset.
std::map<uint64_t, BinarySection> sections_;
Expand Down
Loading

0 comments on commit e376ddf

Please sign in to comment.