Skip to content

Commit

Permalink
More rust cleanup
Browse files Browse the repository at this point in the history
- Fixed MANY memory leaks (most due to QualifiedName)
- Made StructureBuilder more builder and less structure
- Fixed CMakeLists.txt that were globbing entire api, resulting in 100 second slowdown on cmake generation
- Added more Debug impl's
- Added some more tests
- Fixed TypeParserResult UB
- Moved the From impls to blank impl for clarity, we have multiple different variants of core to rust for some structures, it should be explicit which one you are choosing.
- PossibleValueSet should now be able to allocate so we can go from rust to core with those variants that require allocation
- Misc doc code formatting
  • Loading branch information
emesare committed Jan 13, 2025
1 parent 12fe44d commit 807526d
Show file tree
Hide file tree
Showing 47 changed files with 1,841 additions and 1,513 deletions.
2 changes: 1 addition & 1 deletion arch/riscv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl<D: 'static + RiscVDisassembler> PartialEq for Register<D> {

impl<D: 'static + RiscVDisassembler> Eq for Register<D> {}

impl<D: 'static + RiscVDisassembler + Send + Sync> fmt::Debug for Register<D> {
impl<D: 'static + RiscVDisassembler> fmt::Debug for Register<D> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(self.name().as_ref())
}
Expand Down
12 changes: 6 additions & 6 deletions plugins/dwarf/dwarf_export/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ file(GLOB PLUGIN_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/../shared/src/*.rs)

file(GLOB_RECURSE API_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/../../../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../../binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../../binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../../binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../../Cargo.toml
${PROJECT_SOURCE_DIR}/../../../src/*.rs)
${PROJECT_SOURCE_DIR}/../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../rust/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/src/*.rs)

if(CMAKE_BUILD_TYPE MATCHES Debug)
set(TARGET_DIR ${PROJECT_BINARY_DIR}/target/debug)
Expand Down
12 changes: 6 additions & 6 deletions plugins/dwarf/dwarf_import/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ file(GLOB PLUGIN_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/../shared/src/*.rs)

file(GLOB_RECURSE API_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/../../../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../../binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../../binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../../binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../../Cargo.toml
${PROJECT_SOURCE_DIR}/../../../src/*.rs)
${PROJECT_SOURCE_DIR}/../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../rust/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/src/*.rs)

if(CMAKE_BUILD_TYPE MATCHES Debug)
set(TARGET_DIR ${PROJECT_BINARY_DIR}/target/debug)
Expand Down
4 changes: 2 additions & 2 deletions plugins/dwarf/dwarf_import/src/die_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ pub(crate) fn handle_function<R: ReaderType>(
if let Some(name) = debug_info_builder_context.get_name(dwarf, unit, entry) {
let ntr = Type::named_type_from_type(
&name,
&Type::function(return_type.as_ref(), &[], false),
&Type::function(return_type.as_ref(), vec![], false),
);
debug_info_builder.add_type(
get_uid(dwarf, unit, entry),
Expand Down Expand Up @@ -350,7 +350,7 @@ pub(crate) fn handle_function<R: ReaderType>(

Some(Type::function(
return_type.as_ref(),
&parameters,
parameters,
variable_arguments,
))
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/dwarf/dwarf_import/src/dwarfdebuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ impl DebugInfoBuilder {
})
.collect();

Type::function(&return_type, &parameters, function.variable_arguments)
Type::function(&return_type, parameters, function.variable_arguments)
}

fn commit_functions(&self, debug_info: &mut DebugInfo) {
Expand Down
8 changes: 4 additions & 4 deletions plugins/dwarf/dwarf_import/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ fn do_structure_parse<R: ReaderType>(

// Create structure with proper size
let size = get_size_as_u64(entry).unwrap_or(0);
let structure_builder: StructureBuilder = StructureBuilder::new();
let mut structure_builder = StructureBuilder::new();
structure_builder
.set_packed(true)
.set_width(size)
.set_structure_type(structure_type);
.packed(true)
.width(size)
.structure_type(structure_type);

// This reference type will be used by any children to grab while we're still building this type
// it will also be how any other types refer to this struct
Expand Down
12 changes: 6 additions & 6 deletions plugins/idb_import/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ file(GLOB_RECURSE PLUGIN_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/src/*.rs)

file(GLOB_RECURSE API_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/../../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../Cargo.toml
${PROJECT_SOURCE_DIR}/../../src/*.rs)
${PROJECT_SOURCE_DIR}/../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../rust/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/src/*.rs)

if(CMAKE_BUILD_TYPE MATCHES Debug)
set(TARGET_DIR ${PROJECT_BINARY_DIR}/target/debug)
Expand Down
18 changes: 9 additions & 9 deletions plugins/idb_import/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ impl<F: Fn(usize, usize) -> Result<(), ()>> TranslateIDBTypes<'_, F> {
bn_args.push(FunctionParameter::new(arg, name, loc));
}

let ty = Type::function(&return_ty, &bn_args, false);
let ty = Type::function(&return_ty, bn_args, false);
if is_partial {
let error = (errors.ret.is_some() || !errors.args.is_empty())
.then(|| BnTypeError::Function(errors));
Expand Down Expand Up @@ -284,7 +284,7 @@ impl<F: Fn(usize, usize) -> Result<(), ()>> TranslateIDBTypes<'_, F> {
&self,
offset: usize,
members_slice: &[TILStructMember],
struct_builder: &StructureBuilder,
struct_builder: &mut StructureBuilder,
) {
if members_slice.is_empty() {
unreachable!()
Expand All @@ -301,7 +301,7 @@ impl<F: Fn(usize, usize) -> Result<(), ()>> TranslateIDBTypes<'_, F> {
let mut current_field_bits: u32 = first_field.width.into();
let mut start_idx = 0;

let create_field = |start_idx, i, bytes| {
let mut create_field = |start_idx, i, bytes| {
let name = if start_idx == i - 1 {
let member: &TILStructMember = &members_slice[i - 1];
member
Expand Down Expand Up @@ -354,8 +354,8 @@ impl<F: Fn(usize, usize) -> Result<(), ()>> TranslateIDBTypes<'_, F> {
return TranslateTypeResult::Translated(Type::void());
}
let mut is_partial = false;
let structure = StructureBuilder::new();
structure.set_alignment(effective_alignment.into());
let mut structure = StructureBuilder::new();
structure.alignment(effective_alignment.into());

let mut errors = vec![];
let mut first_bitfield_seq = None;
Expand All @@ -372,7 +372,7 @@ impl<F: Fn(usize, usize) -> Result<(), ()>> TranslateIDBTypes<'_, F> {
(_, Some(start_idx)) => {
first_bitfield_seq = None;
let members_bitrange = &members[start_idx..i];
self.condensate_bitfields_from_struct(start_idx, members_bitrange, &structure);
self.condensate_bitfields_from_struct(start_idx, members_bitrange, &mut structure);
}

(_, None) => {}
Expand Down Expand Up @@ -402,7 +402,7 @@ impl<F: Fn(usize, usize) -> Result<(), ()>> TranslateIDBTypes<'_, F> {
}
if let Some(start_idx) = first_bitfield_seq {
let members_bitrange = &members[start_idx..];
self.condensate_bitfields_from_struct(start_idx, members_bitrange, &structure);
self.condensate_bitfields_from_struct(start_idx, members_bitrange, &mut structure);
}
let bn_ty = Type::structure(&structure.finalize());
if is_partial {
Expand All @@ -420,8 +420,8 @@ impl<F: Fn(usize, usize) -> Result<(), ()>> TranslateIDBTypes<'_, F> {
_effective_alignment: u16,
) -> TranslateTypeResult {
let mut is_partial = false;
let structure = StructureBuilder::new();
structure.set_structure_type(StructureType::UnionStructureType);
let mut structure = StructureBuilder::new();
structure.structure_type(StructureType::UnionStructureType);
let mut errors = vec![];
for (i, (member_name, member_type)) in members.iter().enumerate() {
// bitfields can be translated into complete fields
Expand Down
12 changes: 6 additions & 6 deletions plugins/pdb-ng/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ file(GLOB PLUGIN_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/src/*.rs)

file(GLOB_RECURSE API_SOURCES CONFIGURE_DEPENDS
${PROJECT_SOURCE_DIR}/../../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../Cargo.toml
${PROJECT_SOURCE_DIR}/../../src/*.rs)
${PROJECT_SOURCE_DIR}/../../binaryninjacore.h
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/build.rs
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/binaryninjacore-sys/src/*
${PROJECT_SOURCE_DIR}/../../rust/Cargo.toml
${PROJECT_SOURCE_DIR}/../../rust/src/*.rs)

if(CMAKE_BUILD_TYPE MATCHES Debug)
set(TARGET_DIR ${PROJECT_BINARY_DIR}/target/debug)
Expand Down
12 changes: 6 additions & 6 deletions plugins/pdb-ng/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,21 +407,21 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
NamedTypeReferenceClass::ClassNamedTypeClass
| NamedTypeReferenceClass::StructNamedTypeClass
| NamedTypeReferenceClass::UnionNamedTypeClass => {
let structure = StructureBuilder::new();
let mut structure = StructureBuilder::new();
match class {
NamedTypeReferenceClass::ClassNamedTypeClass => {
structure.set_structure_type(StructureType::ClassStructureType);
structure.structure_type(StructureType::ClassStructureType);
}
NamedTypeReferenceClass::StructNamedTypeClass => {
structure.set_structure_type(StructureType::StructStructureType);
structure.structure_type(StructureType::StructStructureType);
}
NamedTypeReferenceClass::UnionNamedTypeClass => {
structure.set_structure_type(StructureType::UnionStructureType);
structure.structure_type(StructureType::UnionStructureType);
}
_ => {}
}
structure.set_width(1);
structure.set_alignment(1);
structure.width(1);
structure.alignment(1);

self.debug_info.add_type(
&name,
Expand Down
8 changes: 4 additions & 4 deletions plugins/pdb-ng/src/struct_grouper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ pub fn group_structure(
warn!("{} Could not resolve structure groups: {}", name, e);
for member in members {
structure.insert(
&member.ty.clone(),
&member.ty,
member.name.clone(),
member.offset,
false,
Expand Down Expand Up @@ -391,7 +391,7 @@ fn apply_groups(

if offset > member.offset {
structure.insert(
&member.ty.clone(),
&member.ty,
member.name.clone(),
0,
false,
Expand All @@ -400,7 +400,7 @@ fn apply_groups(
);
} else {
structure.insert(
&member.ty.clone(),
&member.ty,
member.name.clone(),
member.offset - offset,
false,
Expand All @@ -423,7 +423,7 @@ fn apply_groups(
}
ResolvedGroup::Union(inner_offset, children) => {
let mut inner = StructureBuilder::new();
inner.set_structure_type(StructureType::UnionStructureType);
inner.structure_type(StructureType::UnionStructureType);
apply_groups(members, &mut inner, children, inner_offset);
structure.insert(
&Conf::new(Type::structure(inner.finalize().as_ref()), MAX_CONFIDENCE),
Expand Down
8 changes: 4 additions & 4 deletions plugins/pdb-ng/src/symbol_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
.ok_or(anyhow!("no ret"))?,
fancy_params.as_slice(),
fancy_type.contents.has_variable_arguments().contents,
&cc,
cc,
fancy_type.contents.stack_adjustment(),
),
MAX_CONFIDENCE,
Expand Down Expand Up @@ -1801,11 +1801,11 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
if let [p] = parameters.as_slice() {
if p.ty.contents.type_class() == TypeClass::VoidTypeClass {
t = Some(Conf::new(
Type::function::<_>(
Type::function(
&ty.contents
.return_value()
.ok_or(anyhow!("no return value"))?,
&[],
vec![],
ty.contents.has_variable_arguments().contents,
),
ty.confidence,
Expand Down Expand Up @@ -2022,7 +2022,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
}

// Make a new copy of the type with the correct element count
last_member.ty.contents = Type::array(member_element.as_ref(), element_count);
last_member.ty.contents = Type::array(&member_element, element_count);

Ok(Some((
Type::structure(StructureBuilder::from(members).finalize().as_ref()),
Expand Down
Loading

0 comments on commit 807526d

Please sign in to comment.