Skip to content

Commit

Permalink
Fix another move-native fmt::Debug crash (vector-of-structs). (anza-x…
Browse files Browse the repository at this point in the history
…yz#194)

This patch fixes another occurrence of the move-native runtime 
defect where a debug formatter-- this time in the vector-of-structs case--
dereferenced and called a rogue pointer. This results in a callx out of
the address space.  anza-xyz#191.
  
Also enhanced the fmt::Debug routines for structs to output the struct name
and otherwise make the output more like what a default rust debug formatter
for structs would do.
    
Other minor NFC clean-ups.
    
Added a runnable rbpf test cases demonstrating vector-of-struct operations
and debug-printing. Updated the log messages of the other rtty tests since
the output now includes names.
  • Loading branch information
nvjle authored and ksolana committed Jul 6, 2023
1 parent 72e8bb8 commit e2085de
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 33 deletions.
31 changes: 19 additions & 12 deletions language/move-native/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,7 @@ pub(crate) mod conv {
Address(&'mv MoveAddress),
Signer(&'mv MoveSigner),
Vector(MoveType, &'mv MoveUntypedVector),
Struct(StructTypeInfo, &'mv AnyValue),
Struct(MoveType, &'mv AnyValue),
Reference(MoveType, &'mv MoveUntypedReference),
// todo
}
Expand All @@ -1588,8 +1588,11 @@ pub(crate) mod conv {
BorrowedTypedMoveValue::Vector(element_type, move_ref)
}
TypeDesc::Struct => {
let struct_info = (*type_.type_info).struct_;
BorrowedTypedMoveValue::Struct(struct_info, value)
// Previously we stored the StructTypeInfo here. But passing the enclosing
// MoveType instead gives routines access to the struct name (i.e., more
// context). Otherwise we would need an uplevel pointer in StructTypeInfo or
// to redundantly store the name there.
BorrowedTypedMoveValue::Struct(*type_, value)
}
TypeDesc::Reference => {
let element_type = *(*type_.type_info).reference.element_type;
Expand Down Expand Up @@ -1784,11 +1787,12 @@ pub(crate) mod conv {
let rv = borrow_typed_move_vec_as_rust_vec(mt, mv);
rv.serialize(serializer)
},
BorrowedTypedMoveValue::Struct(st, mv) => unsafe {
BorrowedTypedMoveValue::Struct(t, mv) => unsafe {
// fixme: probably need serialize_struct here
let st = (*(t.type_info)).struct_;
let len = usize::try_from(st.field_array_len).expect("overflow");
let mut seq = serializer.serialize_seq(Some(len))?;
let fields = walk_struct_fields(st, mv);
let fields = walk_struct_fields(&st, mv);
for (type_, ref_) in fields {
let rv = borrow_move_value_as_rust_value(type_, ref_);
seq.serialize_element(&rv);
Expand Down Expand Up @@ -1928,15 +1932,16 @@ pub(crate) mod conv {
rv.fmt(f)
},
BorrowedTypedMoveValue::Struct(t, v) => unsafe {
// fixme struct / field names
f.write_str("[");
let fields = walk_struct_fields(t, v);
// fixme field names
let st = (*(t.type_info)).struct_;
write!(f, "{} {{", t.name.as_ascii_str());
let fields = walk_struct_fields(&st, v);
for (type_, ref_) in fields {
let rv = borrow_move_value_as_rust_value(type_, ref_);
rv.fmt(f);
f.write_str(", ");
}
f.write_str("]");
f.write_str("}");
Ok(())
},
BorrowedTypedMoveValue::Reference(t, v) => unsafe {
Expand Down Expand Up @@ -1970,7 +1975,7 @@ pub(crate) mod conv {
dbg.finish()
}
TypedMoveBorrowedRustVec::Struct(s) => {
let mut dbg = f.debug_list();
f.write_str("[");
unsafe {
for vref in s.iter() {
let type_ = MoveType {
Expand All @@ -1979,10 +1984,12 @@ pub(crate) mod conv {
type_info: &TypeInfo { struct_: *s.type_ },
};
let e = borrow_move_value_as_rust_value(&type_, vref);
dbg.entry(&e);
e.fmt(f);
f.write_str(", ");
}
}
dbg.finish()
f.write_str("]");
Ok(())
}
TypedMoveBorrowedRustVec::Reference(t, v) => {
let mut dbg = f.debug_list();
Expand Down
24 changes: 7 additions & 17 deletions language/tools/move-mv-llvm-compiler/src/stackless/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,24 +830,14 @@ impl Type {
pub fn dump_properties_to_str(&self, data_layout: LLVMTargetDataRef) -> String {
unsafe {
let ty = self.0;
let mut s = "".to_string();
s += &format!(
"StoreSizeOfType: {}\n",
LLVMStoreSizeOfType(data_layout, ty) as u32
let s = &format!(
"StoreSizeOfType: {}\nABISizeOfType: {}\nABIAlignmnetOfType: {}\nSizeOfTypeInBits: {}\n",
LLVMStoreSizeOfType(data_layout, ty) as u32,
LLVMABISizeOfType(data_layout, ty) as u32,
LLVMABIAlignmentOfType(data_layout, ty),
LLVMSizeOfTypeInBits(data_layout, ty) as u32,
);
s += &format!(
"ABISizeOfType: {}\n",
LLVMABISizeOfType(data_layout, ty) as u32
);
s += &format!(
"ABIAlignmnetOfType: {}\n",
LLVMABIAlignmentOfType(data_layout, ty)
);
s += &format!(
"SizeOfTypeInBits: {}\n",
LLVMSizeOfTypeInBits(data_layout, ty) as u32
);
s
s.to_string()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ fn define_type_info_global_struct(
ll_fld_array.set_unnamed_addr();
ll_fld_array.set_initializer(aval.as_const());

// Create the overall `ll_struct_info_ty` runtime descriptor global. This LLVM type
// corresponds to `move_native::rt_types::StructFieldInfo`:
// Create the overall `ll_struct_type_info_ty` runtime descriptor global. This LLVM type
// corresponds to `move_native::rt_types::StructTypeInfo`:
// pub struct StructTypeInfo {
// pub field_array_ptr: *const StructFieldInfo,
// pub field_array_len: u64,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// log [123000, 66000, 33000, @000000000000000000000000000000000000000000000000000000000000CAFE, 0, ]
// log ST::A1 {123000, 66000, 33000, @000000000000000000000000000000000000000000000000000000000000CAFE, 0, }
// log 123000
// log 66000
// log 33000
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// log [456, [true, 0, ], 0, ]
// log ST::A1 {456, ST::A0 {true, 0, }, 0, }
// log 456
// log true

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// log ST::A1 {254, 36893488147419103232, 0, }
// log [ST::A1 {254, 36893488147419103232, 0, }, ST::A1 {123, 456, 0, }, ]
// log 123
// log 456


module 0x10::debug {
native public fun print<T>(x: &T);
}

module 0x10::vector {
native public fun empty<Element>(): vector<Element>;
native public fun length<Element>(v: &vector<Element>): u64;
native public fun push_back<Element>(v: &mut vector<Element>, e: Element);
native public fun pop_back<Element>(v: &mut vector<Element>): Element;
native public fun destroy_empty<Element>(v: vector<Element>);
native public fun swap<Element>(v: &mut vector<Element>, i: u64, j: u64);
native public fun borrow<Element>(v: &vector<Element>, i: u64): &Element;
native public fun borrow_mut<Element>(v: &mut vector<Element>, i: u64): &mut Element;
}

module 0x200::ST {
struct A1 has drop, copy {
f1: u8,
f2: u128
}

public fun new(a1: u8, a2: u128): A1 {
A1 { f1: a1, f2: a2 }
}

public fun get(s: &A1): (u8, u128) {
let A1 { f1: x, f2: y } = *s;
(x, y)
}
}

script {
use 0x10::debug;
use 0x10::vector;
use 0x200::ST;

fun main() {
let s1 = ST::new(254, 36893488147419103232_u128);
let s2 = ST::new(123, 456);
debug::print(&s1);

// Now we're really going gonzo and operating on and debug-printing
// a vector-of-structs.
let v: vector<ST::A1> = vector::empty();
vector::push_back(&mut v, s1);
vector::push_back(&mut v, s2);
debug::print(&v);

let sref = vector::borrow<ST::A1>(&v, 1);
let (f1, f2) = ST::get(sref);
debug::print(&f1);
debug::print(&f2);
}
}

0 comments on commit e2085de

Please sign in to comment.