Skip to content

Commit

Permalink
fix multiple memory leaks related to CStr/BnString
Browse files Browse the repository at this point in the history
  • Loading branch information
rbran committed May 27, 2024
1 parent 331d226 commit 6f62fb5
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 69 deletions.
4 changes: 2 additions & 2 deletions rust/src/architecture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -983,8 +983,8 @@ impl FlagGroup for CoreFlagGroup {
}
}

#[derive(Copy, Clone, Eq, PartialEq)]
pub struct CoreIntrinsic(*mut BNArchitecture, u32);
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub struct CoreIntrinsic(pub(crate) *mut BNArchitecture, pub(crate) u32);

impl Intrinsic for crate::architecture::CoreIntrinsic {
fn name(&self) -> Cow<str> {
Expand Down
9 changes: 4 additions & 5 deletions rust/src/hlil/instruction.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use binaryninjacore_sys::BNGetHighLevelILByIndex;
use binaryninjacore_sys::BNHighLevelILOperation;

use crate::architecture::CoreIntrinsic;
use crate::operand_iter::OperandIter;
use crate::rc::Ref;
use crate::types::{
ConstantData, ILIntrinsic, RegisterValue, RegisterValueType, SSAVariable, Variable,
};
use crate::types::{ConstantData, RegisterValue, RegisterValueType, SSAVariable, Variable};

use super::operation::*;
use super::{HighLevelILFunction, HighLevelILLiftedInstruction, HighLevelILLiftedInstructionKind};
Expand Down Expand Up @@ -812,11 +811,11 @@ impl HighLevelILInstruction {
cond_false: self.lift_operand(op.cond_false),
}),
Intrinsic(op) => Lifted::Intrinsic(LiftedIntrinsic {
intrinsic: ILIntrinsic::new(self.function.get_function().arch(), op.intrinsic),
intrinsic: CoreIntrinsic(self.function.get_function().arch().0, op.intrinsic),
params: self.lift_instruction_list(op.first_param, op.num_params),
}),
IntrinsicSsa(op) => Lifted::IntrinsicSsa(LiftedIntrinsicSsa {
intrinsic: ILIntrinsic::new(self.function.get_function().arch(), op.intrinsic),
intrinsic: CoreIntrinsic(self.function.get_function().arch().0, op.intrinsic),
params: self.lift_instruction_list(op.first_param, op.num_params),
dest_memory: op.dest_memory,
src_memory: op.src_memory,
Expand Down
8 changes: 5 additions & 3 deletions rust/src/hlil/lift.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use super::{operation::*, HighLevelILFunction};
use super::operation::*;
use super::HighLevelILFunction;

use crate::architecture::CoreIntrinsic;
use crate::rc::Ref;
use crate::types::{ConstantData, ILIntrinsic, SSAVariable, Variable};
use crate::types::{ConstantData, SSAVariable, Variable};

#[derive(Clone)]
pub enum HighLevelILLiftedOperand {
Expand All @@ -11,7 +13,7 @@ pub enum HighLevelILLiftedOperand {
Float(f64),
Int(u64),
IntList(Vec<u64>),
Intrinsic(ILIntrinsic),
Intrinsic(CoreIntrinsic),
Label(GotoLabel),
MemberIndex(Option<usize>),
Var(Variable),
Expand Down
14 changes: 7 additions & 7 deletions rust/src/hlil/operation.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use binaryninjacore_sys::BNGetGotoLabelName;

use crate::architecture::CoreIntrinsic;
use crate::function::Function;
use crate::rc::Ref;
use crate::types::{ConstantData, ILIntrinsic, SSAVariable, Variable};
use crate::string::BnString;
use crate::types::{ConstantData, SSAVariable, Variable};

use super::HighLevelILLiftedInstruction;

Expand All @@ -13,10 +15,8 @@ pub struct GotoLabel {
}

impl GotoLabel {
pub fn name(&self) -> &str {
let raw_str = unsafe { BNGetGotoLabelName(self.function.handle, self.target) };
let c_str = unsafe { core::ffi::CStr::from_ptr(raw_str) };
c_str.to_str().unwrap()
pub fn name(&self) -> BnString {
unsafe { BnString::from_raw(BNGetGotoLabelName(self.function.handle, self.target)) }
}
}

Expand Down Expand Up @@ -320,7 +320,7 @@ pub struct Intrinsic {
}
#[derive(Clone, Debug, PartialEq)]
pub struct LiftedIntrinsic {
pub intrinsic: ILIntrinsic,
pub intrinsic: CoreIntrinsic,
pub params: Vec<HighLevelILLiftedInstruction>,
}

Expand All @@ -335,7 +335,7 @@ pub struct IntrinsicSsa {
}
#[derive(Clone, Debug, PartialEq)]
pub struct LiftedIntrinsicSsa {
pub intrinsic: ILIntrinsic,
pub intrinsic: CoreIntrinsic,
pub params: Vec<HighLevelILLiftedInstruction>,
pub dest_memory: u64,
pub src_memory: u64,
Expand Down
9 changes: 4 additions & 5 deletions rust/src/mlil/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ use binaryninjacore_sys::BNGetMediumLevelILByIndex;
use binaryninjacore_sys::BNMediumLevelILInstruction;
use binaryninjacore_sys::BNMediumLevelILOperation;

use crate::architecture::CoreIntrinsic;
use crate::operand_iter::OperandIter;
use crate::rc::Ref;
use crate::types::{
ConstantData, ILIntrinsic, RegisterValue, RegisterValueType, SSAVariable, Variable,
};
use crate::types::{ConstantData, RegisterValue, RegisterValueType, SSAVariable, Variable};

use super::lift::*;
use super::operation::*;
Expand Down Expand Up @@ -906,7 +905,7 @@ impl MediumLevelILInstruction {
output: OperandIter::new(&*self.function, op.first_output, op.num_outputs)
.vars()
.collect(),
intrinsic: ILIntrinsic::new(self.function.get_function().arch(), op.intrinsic),
intrinsic: CoreIntrinsic(self.function.get_function().arch().0, op.intrinsic),
params: OperandIter::new(&*self.function, op.first_param, op.num_params)
.exprs()
.map(|expr| expr.lift())
Expand All @@ -925,7 +924,7 @@ impl MediumLevelILInstruction {
output: OperandIter::new(&*self.function, op.first_output, op.num_outputs)
.ssa_vars()
.collect(),
intrinsic: ILIntrinsic::new(self.function.get_function().arch(), op.intrinsic),
intrinsic: CoreIntrinsic(self.function.get_function().arch().0, op.intrinsic),
params: OperandIter::new(&*self.function, op.first_param, op.num_params)
.exprs()
.map(|expr| expr.lift())
Expand Down
5 changes: 3 additions & 2 deletions rust/src/mlil/lift.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use std::collections::BTreeMap;

use crate::architecture::CoreIntrinsic;
use crate::rc::Ref;
use crate::types::{ConstantData, ILIntrinsic, SSAVariable, Variable};
use crate::types::{ConstantData, SSAVariable, Variable};

use super::operation::*;
use super::MediumLevelILFunction;

#[derive(Clone)]
pub enum MediumLevelILLiftedOperand {
ConstantData(ConstantData),
Intrinsic(ILIntrinsic),
Intrinsic(CoreIntrinsic),
Expr(MediumLevelILLiftedInstruction),
ExprList(Vec<MediumLevelILLiftedInstruction>),
Float(f64),
Expand Down
6 changes: 3 additions & 3 deletions rust/src/mlil/operation.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::collections::BTreeMap;

use crate::types::{ConstantData, ILIntrinsic, SSAVariable, Variable};
use crate::{architecture::CoreIntrinsic, types::{ConstantData, SSAVariable, Variable}};

use super::MediumLevelILLiftedInstruction;

Expand Down Expand Up @@ -355,7 +355,7 @@ pub struct Intrinsic {
#[derive(Clone, Debug, PartialEq)]
pub struct LiftedIntrinsic {
pub output: Vec<Variable>,
pub intrinsic: ILIntrinsic,
pub intrinsic: CoreIntrinsic,
pub params: Vec<MediumLevelILLiftedInstruction>,
}

Expand All @@ -371,7 +371,7 @@ pub struct IntrinsicSsa {
#[derive(Clone, Debug, PartialEq)]
pub struct LiftedIntrinsicSsa {
pub output: Vec<SSAVariable>,
pub intrinsic: ILIntrinsic,
pub intrinsic: CoreIntrinsic,
pub params: Vec<MediumLevelILLiftedInstruction>,
}

Expand Down
20 changes: 5 additions & 15 deletions rust/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

//! Interfaces for the various kinds of symbols in a binary.

use std::ffi::CStr;
use std::fmt;
use std::hash::{Hash, Hasher};
use std::ptr;
Expand Down Expand Up @@ -249,24 +248,15 @@ impl Symbol {
}

pub fn full_name(&self) -> BnString {
unsafe {
let name = BNGetSymbolFullName(self.handle);
BnString::from_raw(name)
}
unsafe { BnString::from_raw(BNGetSymbolFullName(self.handle)) }
}

pub fn short_name(&self) -> &str {
unsafe {
let name = BNGetSymbolShortName(self.handle);
CStr::from_ptr(name).to_str().unwrap()
}
pub fn short_name(&self) -> BnString {
unsafe { BnString::from_raw(BNGetSymbolShortName(self.handle)) }
}

pub fn raw_name(&self) -> &str {
unsafe {
let name = BNGetSymbolRawName(self.handle);
CStr::from_ptr(name).to_str().unwrap()
}
pub fn raw_name(&self) -> BnString {
unsafe { BnString::from_raw(BNGetSymbolRawName(self.handle)) }
}

pub fn address(&self) -> u64 {
Expand Down
29 changes: 2 additions & 27 deletions rust/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1194,11 +1194,9 @@ impl Type {
}
}

pub fn generate_auto_demangled_type_id<'a, S: BnStrCompatible>(name: S) -> &'a str {
pub fn generate_auto_demangled_type_id<S: BnStrCompatible>(name: S) -> BnString {
let mut name = QualifiedName::from(name);
unsafe { CStr::from_ptr(BNGenerateAutoDemangledTypeId(&mut name.0)) }
.to_str()
.unwrap()
unsafe { BnString::from_raw(BNGenerateAutoDemangledTypeId(&mut name.0)) }
}
}

Expand Down Expand Up @@ -2717,29 +2715,6 @@ impl<S: BnStrCompatible> DataVariableAndName<S> {
}
}

/////////////////////////
// ILIntrinsic

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct ILIntrinsic {
arch: CoreArchitecture,
index: u32,
}

impl ILIntrinsic {
pub(crate) fn new(arch: CoreArchitecture, index: u32) -> Self {
Self { arch, index }
}

pub fn name(&self) -> &str {
let name_ptr = unsafe { BNGetArchitectureIntrinsicName(self.arch.0, self.index) };
let name_raw = unsafe { core::ffi::CStr::from_ptr(name_ptr) };
name_raw.to_str().unwrap()
}

// TODO impl inputs and outputs function
}

/////////////////////////
// RegisterValueType

Expand Down

0 comments on commit 6f62fb5

Please sign in to comment.