diff --git a/ghostscope-compiler/src/ebpf/codegen/args.rs b/ghostscope-compiler/src/ebpf/codegen/args.rs index 3420725e..16b73d24 100644 --- a/ghostscope-compiler/src/ebpf/codegen/args.rs +++ b/ghostscope-compiler/src/ebpf/codegen/args.rs @@ -19,8 +19,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { name: materialized.name.clone(), }; Ok(ComplexArg { - var_name_index: self.trace_context.add_variable_name(display_name), - type_index: self.trace_context.add_type(optimized_type), + var_name_index: self.trace_context.add_variable_name(display_name)?, + type_index: self.trace_context.add_type(optimized_type)?, access_path: Vec::new(), data_len: 0, source: ComplexArgSource::ImmediateBytes { bytes: Vec::new() }, @@ -46,8 +46,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { let module_hint = Self::module_path_for_offsets(materialized.module_path.as_deref()); Ok(ComplexArg { - var_name_index: self.trace_context.add_variable_name(display_name), - type_index: self.trace_context.add_type(dwarf_type.clone()), + var_name_index: self.trace_context.add_variable_name(display_name)?, + type_index: self.trace_context.add_type(dwarf_type.clone())?, access_path: Vec::new(), data_len, source: ComplexArgSource::RuntimeRead { @@ -80,8 +80,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { }; let data_len = Self::compute_read_size_for_type(&dwarf_type).clamp(1, 8); Ok(ComplexArg { - var_name_index: self.trace_context.add_variable_name(display_name), - type_index: self.trace_context.add_type(dwarf_type), + var_name_index: self.trace_context.add_variable_name(display_name)?, + type_index: self.trace_context.add_type(dwarf_type)?, access_path: Vec::new(), data_len, source: ComplexArgSource::ComputedInt { value, byte_len: data_len }, @@ -110,8 +110,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.trace_context.add_type(dwarf_type), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.trace_context.add_type(dwarf_type)?, access_path: Vec::new(), data_len, source: ComplexArgSource::MemDump { @@ -132,8 +132,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { self.expr_to_name(source_expr), target_type ); - let var_name_index = self.trace_context.add_variable_name(display_name.clone()); - let type_index = self.trace_context.add_type(dwarf_type.clone()); + let var_name_index = self.trace_context.add_variable_name(display_name.clone())?; + let type_index = self.trace_context.add_type(dwarf_type.clone())?; if matches!( ghostscope_dwarf::strip_type_aliases(&dwarf_type), @@ -206,10 +206,10 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { E::Variable(name) if self.alias_variable_exists(name) => { let aliased = self.get_alias_variable(name).expect("alias exists"); let addr_i64 = self.resolve_ptr_i64_from_expr(&aliased)?; - let var_name_index = self.trace_context.add_variable_name(name.clone()); + let var_name_index = self.trace_context.add_variable_name(name.clone())?; Ok(ComplexArg { var_name_index, - type_index: self.add_synthesized_type_index_for_kind(TypeKind::Pointer), + type_index: self.add_synthesized_type_index_for_kind(TypeKind::Pointer)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::ComputedInt { @@ -221,7 +221,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { // 1) Script variables first E::Variable(name) if self.variable_exists(name) => { let val = self.load_variable(name)?; - let var_name_index = self.trace_context.add_variable_name(name.clone()); + let var_name_index = self.trace_context.add_variable_name(name.clone())?; // If this is a string variable, print its contents instead of address if self .get_variable_type(name) @@ -242,7 +242,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { }; return Ok(ComplexArg { var_name_index, - type_index: self.trace_context.add_type(array_type), + type_index: self.trace_context.add_type(array_type)?, access_path: Vec::new(), data_len: bytes.len(), source: ComplexArgSource::ImmediateBytes { bytes }, @@ -266,7 +266,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { }; Ok(ComplexArg { var_name_index, - type_index: self.add_synthesized_type_index_for_kind(kind), + type_index: self.add_synthesized_type_index_for_kind(kind)?, access_path: Vec::new(), data_len: byte_len, source: ComplexArgSource::ComputedInt { @@ -283,7 +283,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { .map_err(|e| CodeGenError::Builder(e.to_string()))?; Ok(ComplexArg { var_name_index, - type_index: self.add_synthesized_type_index_for_kind(TypeKind::Pointer), + type_index: self + .add_synthesized_type_index_for_kind(TypeKind::Pointer)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::ComputedInt { @@ -315,8 +316,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name("__str_literal".to_string()), - type_index: self.trace_context.add_type(array_type), + .add_variable_name("__str_literal".to_string())?, + type_index: self.trace_context.add_type(array_type)?, access_path: Vec::new(), data_len: bytes.len(), source: ComplexArgSource::ImmediateBytes { bytes }, @@ -335,8 +336,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name("__int_literal".to_string()), - type_index: self.trace_context.add_type(int_type), + .add_variable_name("__int_literal".to_string())?, + type_index: self.trace_context.add_type(int_type)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::ImmediateBytes { bytes }, @@ -356,8 +357,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { return Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.trace_context.add_type(ptr_ty), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.trace_context.add_type(ptr_ty)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::ComputedAddress { @@ -401,8 +402,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.trace_context.add_type(ptr_ty), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.trace_context.add_type(ptr_ty)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::AddressValue { @@ -440,8 +441,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { return Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.add_synthesized_type_index_for_kind(kind), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.add_synthesized_type_index_for_kind(kind)?, access_path: Vec::new(), data_len: byte_len, source: ComplexArgSource::ComputedInt { value, byte_len }, @@ -467,8 +468,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { return Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.add_synthesized_type_index_for_kind(kind), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.add_synthesized_type_index_for_kind(kind)?, access_path: Vec::new(), data_len: byte_len, source: ComplexArgSource::ComputedInt { value, byte_len }, @@ -557,8 +558,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { return Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.trace_context.add_type(ptr_ti), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.trace_context.add_type(ptr_ti)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::AddressValue { @@ -570,8 +571,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { return Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.trace_context.add_type(elem_ty.clone()), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.trace_context.add_type(elem_ty.clone())?, access_path: Vec::new(), data_len, source: ComplexArgSource::RuntimeRead { @@ -602,8 +603,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.add_synthesized_type_index_for_kind(kind), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.add_synthesized_type_index_for_kind(kind)?, access_path: Vec::new(), data_len: byte_len, source: ComplexArgSource::ComputedInt { @@ -637,8 +638,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { Ok(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(other)), - type_index: self.add_synthesized_type_index_for_kind(kind), + .add_variable_name(self.expr_to_name(other))?, + type_index: self.add_synthesized_type_index_for_kind(kind)?, access_path: Vec::new(), data_len: byte_len, source: ComplexArgSource::ComputedInt { @@ -690,13 +691,13 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { | ComplexArgSource::ComputedAddress { .. } | ComplexArgSource::ImmediateBytes { .. } => { // Use ComplexFormat with "{}" to render address/immediate nicely - let fmt_idx = self.trace_context.add_string("{}".to_string()); + let fmt_idx = self.trace_context.add_string("{}".to_string())?; self.generate_print_complex_format_instruction(fmt_idx, &[arg])?; Ok(1) } ComplexArgSource::MemDump { .. } | ComplexArgSource::MemDumpDynamic { .. } => { // Use ComplexFormat with "{}"; generate_print_complex_format_instruction handles MemDump - let fmt_idx = self.trace_context.add_string("{}".to_string()); + let fmt_idx = self.trace_context.add_string("{}".to_string())?; self.generate_print_complex_format_instruction(fmt_idx, &[arg])?; Ok(1) } @@ -899,7 +900,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { return compile(self); } - let expr_index = self.trace_context.add_string(self.expr_to_name(expr)); + let expr_index = self.trace_context.add_string(self.expr_to_name(expr))?; let entry_event_bytes = self.compile_time_event_bytes_upper_bound; self.reset_condition_error()?; diff --git a/ghostscope-compiler/src/ebpf/codegen/backtrace.rs b/ghostscope-compiler/src/ebpf/codegen/backtrace.rs index 596995cb..6f9932fc 100644 --- a/ghostscope-compiler/src/ebpf/codegen/backtrace.rs +++ b/ghostscope-compiler/src/ebpf/codegen/backtrace.rs @@ -3663,7 +3663,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { (&hit_value, hit_end), (&ctx.context.i32_type().const_zero(), miss_end), ]); - Ok(phi.as_basic_value().into_int_value()) + Ok::<_, CodeGenError>(phi.as_basic_value().into_int_value()) }; Ok(BtModuleRangeMeta { found: found_phi.as_basic_value().into_int_value(), @@ -3922,7 +3922,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { .build_phi(ctx.context.i64_type(), name) .map_err(|e| CodeGenError::LLVMError(e.to_string()))?; phi.add_incoming(&[(&hit_value, hit_end), (&zero_i64, miss_end)]); - Ok(phi.as_basic_value().into_int_value()) + Ok::<_, CodeGenError>(phi.as_basic_value().into_int_value()) }; Ok(BtModuleRangeValue { found: found_phi.as_basic_value().into_int_value(), diff --git a/ghostscope-compiler/src/ebpf/codegen/format.rs b/ghostscope-compiler/src/ebpf/codegen/format.rs index 0a6ab41b..4aa198cc 100644 --- a/ghostscope-compiler/src/ebpf/codegen/format.rs +++ b/ghostscope-compiler/src/ebpf/codegen/format.rs @@ -155,7 +155,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { format, args.len() ); - let format_string_index = self.trace_context.add_string(format.to_string()); + let format_string_index = self.trace_context.add_string(format.to_string())?; let mut complex_args: Vec> = Vec::with_capacity(args.len()); // Parse placeholders from the format string to support extended specifiers @@ -274,8 +274,9 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { complex_args.push(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.add_synthesized_type_index_for_kind(TypeKind::Pointer), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self + .add_synthesized_type_index_for_kind(TypeKind::Pointer)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::ComputedAddress { address }, @@ -306,8 +307,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { complex_args.push(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), - type_index: self.add_synthesized_type_index_for_kind(TypeKind::Pointer), + .add_variable_name(self.expr_to_name(expr))?, + type_index: self.add_synthesized_type_index_for_kind(TypeKind::Pointer)?, access_path: Vec::new(), data_len: 8, source: ComplexArgSource::ComputedInt { @@ -329,7 +330,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { complex_args.push(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(expr)), + .add_variable_name(self.expr_to_name(expr))?, type_index: self .trace_context .add_type(ghostscope_dwarf::TypeInfo::ArrayType { @@ -342,7 +343,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { }), element_count: Some(n as u64), total_size: Some(n as u64), - }), + })?, access_path: Vec::new(), data_len: n, source: ComplexArgSource::MemDump { @@ -371,8 +372,9 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { complex_args.push(ComplexArg { var_name_index: self .trace_context - .add_variable_name("__len".into()), - type_index: self.add_synthesized_type_index_for_kind(TypeKind::U64), + .add_variable_name("__len".into())?, + type_index: self + .add_synthesized_type_index_for_kind(TypeKind::U64)?, access_path: Vec::new(), data_len: byte_len, source: ComplexArgSource::ComputedInt { @@ -389,7 +391,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { complex_args.push(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(val_expr)), + .add_variable_name(self.expr_to_name(val_expr))?, type_index: self .trace_context .add_type(ghostscope_dwarf::TypeInfo::ArrayType { @@ -402,7 +404,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { }), element_count: Some(cap as u64), total_size: Some(cap as u64), - }), + })?, access_path: Vec::new(), data_len: cap, source: ComplexArgSource::MemDumpDynamic { @@ -444,8 +446,11 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { } }; complex_args.push(ComplexArg { - var_name_index: self.trace_context.add_variable_name(name.clone()), - type_index: self.add_synthesized_type_index_for_kind(TypeKind::U64), + var_name_index: self + .trace_context + .add_variable_name(name.clone())?, + type_index: self + .add_synthesized_type_index_for_kind(TypeKind::U64)?, access_path: Vec::new(), data_len: byte_len, source: ComplexArgSource::ComputedInt { @@ -461,7 +466,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { complex_args.push(ComplexArg { var_name_index: self .trace_context - .add_variable_name(self.expr_to_name(val_expr)), + .add_variable_name(self.expr_to_name(val_expr))?, type_index: self .trace_context .add_type(ghostscope_dwarf::TypeInfo::ArrayType { @@ -474,7 +479,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { }), element_count: Some(cap as u64), total_size: Some(cap as u64), - }), + })?, access_path: Vec::new(), data_len: cap, source: ComplexArgSource::MemDumpDynamic { diff --git a/ghostscope-compiler/src/ebpf/codegen/print_variable_index.rs b/ghostscope-compiler/src/ebpf/codegen/print_variable_index.rs index f4a1d0bd..8785492a 100644 --- a/ghostscope-compiler/src/ebpf/codegen/print_variable_index.rs +++ b/ghostscope-compiler/src/ebpf/codegen/print_variable_index.rs @@ -16,12 +16,12 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { // Resolve type_index from DWARF if available; otherwise synthesize from TypeKind let type_index = match self.query_dwarf_for_variable(var_name)? { Some(var) => match var.dwarf_type { - Some(ref t) => self.trace_context.add_type(t.clone()), - None => self.add_synthesized_type_index_for_kind(type_encoding), + Some(ref t) => self.trace_context.add_type(t.clone())?, + None => self.add_synthesized_type_index_for_kind(type_encoding)?, }, None => { // Variable not found via DWARF; fall back to synthesized type info based on TypeKind - self.add_synthesized_type_index_for_kind(type_encoding) + self.add_synthesized_type_index_for_kind(type_encoding)? } }; diff --git a/ghostscope-compiler/src/ebpf/codegen/statements.rs b/ghostscope-compiler/src/ebpf/codegen/statements.rs index cc71fd29..f73d315f 100644 --- a/ghostscope-compiler/src/ebpf/codegen/statements.rs +++ b/ghostscope-compiler/src/ebpf/codegen/statements.rs @@ -118,7 +118,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { // Prepare condition context (runtime error capture) // Pretty expression text for warning let expr_text = self.expr_to_name(condition); - let expr_index = self.trace_context.add_string(expr_text); + let expr_index = self.trace_context.add_string(expr_text)?; // Activate condition context (compile-time flag) and reset runtime error byte self.condition_context_active = true; self.reset_condition_error()?; @@ -302,7 +302,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { PrintStatement::String(s) => { info!("Processing string literal: {}", s); // 1. Add string to TraceContext - let string_index = self.trace_context.add_string(s.to_string()); + let string_index = self.trace_context.add_string(s.to_string())?; // 2. Generate eBPF code for PrintStringIndex self.generate_print_string_index(string_index)?; Ok(1) // Generated 1 instruction diff --git a/ghostscope-compiler/src/ebpf/codegen/types.rs b/ghostscope-compiler/src/ebpf/codegen/types.rs index 1d6ccd94..64acc2c0 100644 --- a/ghostscope-compiler/src/ebpf/codegen/types.rs +++ b/ghostscope-compiler/src/ebpf/codegen/types.rs @@ -15,7 +15,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { let type_encoding = self.infer_type_from_llvm_value(&loaded_value); // Add to TraceContext - let var_name_index = self.trace_context.add_variable_name(var_name.to_string()); + let var_name_index = self.trace_context.add_variable_name(var_name.to_string())?; return Ok((var_name_index, type_encoding)); } @@ -44,7 +44,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { let type_encoding = TypeKind::from(dwarf_type); // Add to StringTable - let var_name_index = self.trace_context.add_variable_name(var_name.to_string()); + let var_name_index = self.trace_context.add_variable_name(var_name.to_string())?; info!( "DWARF variable '{}' resolved successfully with type: {:?}", @@ -143,9 +143,9 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { } } - pub(super) fn add_synthesized_type_index_for_kind(&mut self, kind: TypeKind) -> u16 { + pub(super) fn add_synthesized_type_index_for_kind(&mut self, kind: TypeKind) -> Result { let ti = self.synthesize_typeinfo_for_typekind(kind); - self.trace_context.add_type(ti) + Ok(self.trace_context.add_type(ti)?) } /// Infer TypeKind from LLVM value type diff --git a/ghostscope-compiler/src/ebpf/context.rs b/ghostscope-compiler/src/ebpf/context.rs index b2dc8aca..7d338db4 100644 --- a/ghostscope-compiler/src/ebpf/context.rs +++ b/ghostscope-compiler/src/ebpf/context.rs @@ -72,6 +72,8 @@ pub enum CodeGenError { DwarfError(String), #[error("Type size not available for variable: {0}")] TypeSizeNotAvailable(String), + #[error("Trace context error: {0}")] + TraceContext(#[from] ghostscope_protocol::TraceContextOverflow), } pub type Result = std::result::Result; diff --git a/ghostscope-process/Cargo.toml b/ghostscope-process/Cargo.toml index 9210f7e6..f4ee8b60 100644 --- a/ghostscope-process/Cargo.toml +++ b/ghostscope-process/Cargo.toml @@ -20,7 +20,7 @@ log = { workspace = true } object = { workspace = true } aya = { workspace = true } aya-obj = { workspace = true } -ghostscope-protocol = { version = "0.1.4", path = "../ghostscope-protocol", features = ["aya-pod"] } +ghostscope-protocol = { version = "0.1.5", path = "../ghostscope-protocol", features = ["aya-pod"] } libc = "0.2" bytes = "1" memmap2 = "0.9" diff --git a/ghostscope-protocol/src/format_printer.rs b/ghostscope-protocol/src/format_printer.rs index 19f5f218..b71b3da2 100644 --- a/ghostscope-protocol/src/format_printer.rs +++ b/ghostscope-protocol/src/format_printer.rs @@ -1213,7 +1213,9 @@ mod tests { #[test] fn test_format_print_data_with_trace_context() { let mut trace_context = TraceContext::new(); - let format_index = trace_context.add_string("Hello {}, you are {} years old!".to_string()); + let format_index = trace_context + .add_string("Hello {}, you are {} years old!".to_string()) + .expect("add format string"); let rendered: Vec = vec!["Alice".to_string(), "25".to_string()]; let fmt = trace_context.get_string(format_index).unwrap(); let result = FormatPrinter::apply_format_strings(fmt, &rendered); @@ -1225,7 +1227,9 @@ mod tests { use crate::type_info::{StructMember, TypeInfo}; let mut trace_context = TraceContext::new(); - let var_name_idx = trace_context.add_variable_name("person".to_string()); + let var_name_idx = trace_context + .add_variable_name("person".to_string()) + .expect("add variable name"); let person_type = TypeInfo::StructType { name: "Person".to_string(), @@ -1256,7 +1260,9 @@ mod tests { ], }; - let type_idx = trace_context.add_type(person_type); + let type_idx = trace_context + .add_type(person_type) + .expect("add person type"); // Data: age=25 (4 bytes) + id=12345 (8 bytes) let data = vec![ @@ -1282,7 +1288,9 @@ mod tests { use crate::type_info::TypeInfo; let mut trace_context = TraceContext::new(); - let var_name_idx = trace_context.add_variable_name("name".to_string()); + let var_name_idx = trace_context + .add_variable_name("name".to_string()) + .expect("add variable name"); // Define char array type: char name[16] let char_type = TypeInfo::BaseType { name: "char".to_string(), @@ -1294,13 +1302,15 @@ mod tests { element_count: Some(16), total_size: Some(16), }; - let type_idx = trace_context.add_type(arr_type); + let type_idx = trace_context.add_type(arr_type).expect("add array type"); // Data buffer with "Alice\0" and padding let mut data = b"Alice\0".to_vec(); data.resize(16, 0u8); - let fmt_idx = trace_context.add_string("{}".to_string()); + let fmt_idx = trace_context + .add_string("{}".to_string()) + .expect("add format string"); let complex_vars = vec![ParsedComplexVariable { var_name_index: var_name_idx, type_index: type_idx, @@ -1437,7 +1447,9 @@ mod tests { #[test] fn test_ext_hex_preserves_null_deref_error() { let mut trace_context = TraceContext::new(); - let fmt_idx = trace_context.add_string("{:x.16}".to_string()); + let fmt_idx = trace_context + .add_string("{:x.16}".to_string()) + .expect("add format string"); // Array as the value type let arr_type = TypeInfo::ArrayType { @@ -1449,8 +1461,10 @@ mod tests { element_count: Some(16), total_size: Some(16), }; - let type_idx = trace_context.add_type(arr_type); - let var_name_idx = trace_context.add_variable_name("buf".to_string()); + let type_idx = trace_context.add_type(arr_type).expect("add array type"); + let var_name_idx = trace_context + .add_variable_name("buf".to_string()) + .expect("add variable name"); let vars = vec![ParsedComplexVariable { var_name_index: var_name_idx, @@ -1474,7 +1488,9 @@ mod tests { #[test] fn test_ext_s_preserves_read_error_errno_addr() { let mut trace_context = TraceContext::new(); - let fmt_idx = trace_context.add_string("{:s.16}".to_string()); + let fmt_idx = trace_context + .add_string("{:s.16}".to_string()) + .expect("add format string"); // Array let arr_type = TypeInfo::ArrayType { @@ -1486,8 +1502,10 @@ mod tests { element_count: Some(16), total_size: Some(16), }; - let type_idx = trace_context.add_type(arr_type); - let var_name_idx = trace_context.add_variable_name("buf".to_string()); + let type_idx = trace_context.add_type(arr_type).expect("add array type"); + let var_name_idx = trace_context + .add_variable_name("buf".to_string()) + .expect("add variable name"); // Encode errno:i32 + addr:u64 into data let errno: i32 = -14; // EFAULT-like @@ -1515,7 +1533,9 @@ mod tests { #[test] fn test_ext_p_preserves_offsets_unavailable() { let mut trace_context = TraceContext::new(); - let fmt_idx = trace_context.add_string("P={:p}".to_string()); + let fmt_idx = trace_context + .add_string("P={:p}".to_string()) + .expect("add format string"); let ptr_type = TypeInfo::PointerType { target_type: Box::new(TypeInfo::BaseType { @@ -1525,8 +1545,10 @@ mod tests { }), size: 8, }; - let type_idx = trace_context.add_type(ptr_type); - let var_name_idx = trace_context.add_variable_name("ptr".to_string()); + let type_idx = trace_context.add_type(ptr_type).expect("add pointer type"); + let var_name_idx = trace_context + .add_variable_name("ptr".to_string()) + .expect("add variable name"); let vars = vec![ParsedComplexVariable { var_name_index: var_name_idx, @@ -1547,7 +1569,9 @@ mod tests { #[test] fn test_ext_star_len_error_precedence() { let mut trace_context = TraceContext::new(); - let fmt_idx = trace_context.add_string("S={:x.*}".to_string()); + let fmt_idx = trace_context + .add_string("S={:x.*}".to_string()) + .expect("add format string"); // length argument (will surface its error), use base type for simplicity let len_type = TypeInfo::BaseType { @@ -1555,8 +1579,10 @@ mod tests { size: 8, encoding: gimli::constants::DW_ATE_signed.0 as u16, }; - let len_ty_idx = trace_context.add_type(len_type); - let len_name_idx = trace_context.add_variable_name("len".to_string()); + let len_ty_idx = trace_context.add_type(len_type).expect("add length type"); + let len_name_idx = trace_context + .add_variable_name("len".to_string()) + .expect("add length variable"); // value argument (OK) let arr_type = TypeInfo::ArrayType { @@ -1568,8 +1594,10 @@ mod tests { element_count: Some(16), total_size: Some(16), }; - let val_ty_idx = trace_context.add_type(arr_type); - let val_name_idx = trace_context.add_variable_name("buf".to_string()); + let val_ty_idx = trace_context.add_type(arr_type).expect("add array type"); + let val_name_idx = trace_context + .add_variable_name("buf".to_string()) + .expect("add variable name"); let val_data: Vec = (0u8..16).collect(); let vars = vec![ diff --git a/ghostscope-protocol/src/lib.rs b/ghostscope-protocol/src/lib.rs index 62de7e51..358d3f95 100644 --- a/ghostscope-protocol/src/lib.rs +++ b/ghostscope-protocol/src/lib.rs @@ -74,7 +74,9 @@ pub use bpf_abi::{ PROC_MODULE_RANGE_VALUE_SIZE, PROC_MODULE_RANGE_VALUE_TEXT_OFFSET, }; -pub use trace_context::TraceContext; +pub use trace_context::{ + TraceContext, TraceContextOverflow, TraceContextResult, TraceContextTable, +}; pub use format_printer::FormatPrinter; diff --git a/ghostscope-protocol/src/streaming_parser.rs b/ghostscope-protocol/src/streaming_parser.rs index 4e0967a3..5050e43a 100644 --- a/ghostscope-protocol/src/streaming_parser.rs +++ b/ghostscope-protocol/src/streaming_parser.rs @@ -864,7 +864,9 @@ mod tests { #[test] fn test_streaming_parser() { let mut trace_context = TraceContext::new(); - let _str_idx = trace_context.add_string("hello world".to_string()); + let _str_idx = trace_context + .add_string("hello world".to_string()) + .expect("add test string"); let mut parser = StreamingTraceParser::new(); @@ -902,7 +904,9 @@ mod tests { #[test] fn test_parse_exprerror_instruction() { let mut trace_context = TraceContext::new(); - let expr_idx = trace_context.add_string("memcmp(buf, hex(\"504f\"), 2)".to_string()); + let expr_idx = trace_context + .add_string("memcmp(buf, hex(\"504f\"), 2)".to_string()) + .expect("add test expression"); let mut parser = StreamingTraceParser::new(); diff --git a/ghostscope-protocol/src/trace_context.rs b/ghostscope-protocol/src/trace_context.rs index 134efc01..d611e95d 100644 --- a/ghostscope-protocol/src/trace_context.rs +++ b/ghostscope-protocol/src/trace_context.rs @@ -5,6 +5,63 @@ use crate::type_info::TypeInfo; use serde::{Deserialize, Serialize}; +use std::fmt; + +/// Trace context table identifiers used in overflow errors. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TraceContextTable { + Strings, + Types, + VariableNames, +} + +impl fmt::Display for TraceContextTable { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Strings => write!(f, "string"), + Self::Types => write!(f, "type"), + Self::VariableNames => write!(f, "variable name"), + } + } +} + +/// Error returned when a trace context table can no longer be indexed by u16. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct TraceContextOverflow { + table: TraceContextTable, + attempted_index: usize, +} + +impl TraceContextOverflow { + fn new(table: TraceContextTable, attempted_index: usize) -> Self { + Self { + table, + attempted_index, + } + } + + pub fn table(&self) -> TraceContextTable { + self.table + } + + pub fn attempted_index(&self) -> usize { + self.attempted_index + } +} + +impl fmt::Display for TraceContextOverflow { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "trace context {} table overflow: index {} exceeds u16::MAX", + self.table, self.attempted_index + ) + } +} + +impl std::error::Error for TraceContextOverflow {} + +pub type TraceContextResult = std::result::Result; /// Unified context for trace execution containing strings, types, and variable names #[derive(Debug, Clone, Serialize, Deserialize)] @@ -30,55 +87,58 @@ impl TraceContext { } /// Add a string to the context and return its index - pub fn add_string(&mut self, s: String) -> u16 { + pub fn add_string(&mut self, s: String) -> TraceContextResult { // Check if string already exists to avoid duplicates if let Some(index) = self.strings.iter().position(|existing| existing == &s) { - return index as u16; + return Ok(index as u16); } let index = self.strings.len(); if index > u16::MAX as usize { - panic!("String table overflow: too many strings"); + return Err(TraceContextOverflow::new(TraceContextTable::Strings, index)); } self.strings.push(s); - index as u16 + Ok(index as u16) } /// Add a type to the context and return its index - pub fn add_type(&mut self, type_info: TypeInfo) -> u16 { + pub fn add_type(&mut self, type_info: TypeInfo) -> TraceContextResult { // For now, don't deduplicate types as they can be complex to compare // TODO: Implement type deduplication for performance optimization let index = self.types.len(); if index > u16::MAX as usize { - panic!("Type table overflow: too many types"); + return Err(TraceContextOverflow::new(TraceContextTable::Types, index)); } // Debug: Log the type being added tracing::debug!("Adding type at index {}: {:#?}", index, type_info); self.types.push(type_info); - index as u16 + Ok(index as u16) } /// Add a variable name to the context and return its index - pub fn add_variable_name(&mut self, name: String) -> u16 { + pub fn add_variable_name(&mut self, name: String) -> TraceContextResult { // Check if variable name already exists to avoid duplicates if let Some(index) = self .variable_names .iter() .position(|existing| existing == &name) { - return index as u16; + return Ok(index as u16); } let index = self.variable_names.len(); if index > u16::MAX as usize { - panic!("Variable name table overflow: too many variable names"); + return Err(TraceContextOverflow::new( + TraceContextTable::VariableNames, + index, + )); } self.variable_names.push(name); - index as u16 + Ok(index as u16) } /// Get a string by index @@ -146,9 +206,9 @@ mod tests { fn test_trace_context_strings() { let mut ctx = TraceContext::new(); - let idx1 = ctx.add_string("Hello".to_string()); - let idx2 = ctx.add_string("World".to_string()); - let idx3 = ctx.add_string("Hello".to_string()); // Duplicate + let idx1 = ctx.add_string("Hello".to_string()).unwrap(); + let idx2 = ctx.add_string("World".to_string()).unwrap(); + let idx3 = ctx.add_string("Hello".to_string()).unwrap(); // Duplicate assert_eq!(idx1, 0); assert_eq!(idx2, 1); @@ -163,9 +223,9 @@ mod tests { fn test_trace_context_variable_names() { let mut ctx = TraceContext::new(); - let idx1 = ctx.add_variable_name("person".to_string()); - let idx2 = ctx.add_variable_name("name".to_string()); - let idx3 = ctx.add_variable_name("person".to_string()); // Duplicate + let idx1 = ctx.add_variable_name("person".to_string()).unwrap(); + let idx2 = ctx.add_variable_name("name".to_string()).unwrap(); + let idx3 = ctx.add_variable_name("person".to_string()).unwrap(); // Duplicate assert_eq!(idx1, 0); assert_eq!(idx2, 1); @@ -202,8 +262,8 @@ mod tests { }], }; - let idx1 = ctx.add_type(type1.clone()); - let idx2 = ctx.add_type(type2.clone()); + let idx1 = ctx.add_type(type1.clone()).unwrap(); + let idx2 = ctx.add_type(type2.clone()).unwrap(); assert_eq!(idx1, 0); assert_eq!(idx2, 1); @@ -217,13 +277,15 @@ mod tests { fn test_trace_context_combined() { let mut ctx = TraceContext::new(); - let str_idx = ctx.add_string("format: {} = {}".to_string()); - let var_idx = ctx.add_variable_name("person.name".to_string()); - let type_idx = ctx.add_type(TypeInfo::BaseType { - name: "char[32]".to_string(), - size: 32, - encoding: gimli::constants::DW_ATE_signed_char.0 as u16, - }); + let str_idx = ctx.add_string("format: {} = {}".to_string()).unwrap(); + let var_idx = ctx.add_variable_name("person.name".to_string()).unwrap(); + let type_idx = ctx + .add_type(TypeInfo::BaseType { + name: "char[32]".to_string(), + size: 32, + encoding: gimli::constants::DW_ATE_signed_char.0 as u16, + }) + .unwrap(); assert_eq!(str_idx, 0); assert_eq!(var_idx, 0); @@ -239,15 +301,29 @@ mod tests { fn test_trace_context_memory_usage() { let mut ctx = TraceContext::new(); - ctx.add_string("Hello World".to_string()); // 11 bytes - ctx.add_variable_name("variable".to_string()); // 8 bytes + ctx.add_string("Hello World".to_string()).unwrap(); // 11 bytes + ctx.add_variable_name("variable".to_string()).unwrap(); // 8 bytes ctx.add_type(TypeInfo::BaseType { name: "int".to_string(), size: 4, encoding: gimli::constants::DW_ATE_signed.0 as u16, - }); // ~100 bytes estimate + }) + .unwrap(); // ~100 bytes estimate let usage = ctx.estimated_memory_usage(); assert!(usage >= 119); // At least the string sizes + type estimate } + + #[test] + fn test_trace_context_reports_table_overflow() { + let mut ctx = TraceContext::new(); + ctx.variable_names = (0..=u16::MAX).map(|i| format!("v{i}")).collect(); + + let err = ctx + .add_variable_name("overflow".to_string()) + .expect_err("variable name table should overflow"); + + assert_eq!(err.table(), TraceContextTable::VariableNames); + assert_eq!(err.attempted_index(), usize::from(u16::MAX) + 1); + } } diff --git a/ghostscope-ui/src/components/command_panel/command_parser.rs b/ghostscope-ui/src/components/command_panel/command_parser.rs index c1d4c890..e6693407 100644 --- a/ghostscope-ui/src/components/command_panel/command_parser.rs +++ b/ghostscope-ui/src/components/command_panel/command_parser.rs @@ -1238,7 +1238,7 @@ impl CommandParser { let filename = if command.starts_with("source ") { command.strip_prefix("source ").unwrap().trim() } else if command.starts_with("s ") - && !command.starts_with("s t") + && !Self::matches_shortcut(command, "s t") && !command.starts_with("save") { // Handle "s " but not "s t" (save traces) or "save" @@ -1445,9 +1445,9 @@ impl CommandParser { // Handle "s " -> "source " (but not "s t", "s o", "s s") if command.starts_with("s ") - && !command.starts_with("s t") - && !command.starts_with("s o") - && !command.starts_with("s s") + && !Self::matches_shortcut(command, "s t") + && !Self::matches_shortcut(command, "s o") + && !Self::matches_shortcut(command, "s s") { return Self::parse_source_command(state, command); } @@ -1600,12 +1600,110 @@ impl CommandParser { } } } + + fn matches_shortcut(command: &str, shortcut: &str) -> bool { + command == shortcut + || command + .strip_prefix(shortcut) + .is_some_and(|rest| rest.starts_with(' ')) + } } #[cfg(test)] mod tests { + use crate::components::command_panel::trace_persistence::SaveFilter; + use std::path::PathBuf; + use std::sync::atomic::{AtomicUsize, Ordering}; + use super::*; + static SOURCE_SHORTCUT_TEST_ID: AtomicUsize = AtomicUsize::new(0); + + fn write_temp_trace_file(name: &str) -> PathBuf { + let id = SOURCE_SHORTCUT_TEST_ID.fetch_add(1, Ordering::Relaxed); + let path = std::env::temp_dir().join(format!( + "ghostscope-command-parser-{name}-{}-{id}.gs", + std::process::id() + )); + + std::fs::write( + &path, + "trace main {\n print \"loaded from shortcut\";\n}\n", + ) + .expect("write trace fixture"); + path + } + + #[test] + fn source_shortcut_treats_t_prefix_filename_as_source_file() { + let path = write_temp_trace_file("test"); + let path_str = path.to_string_lossy().to_string(); + let mut state = CommandPanelState::new(); + + let actions = CommandParser::parse_command(&mut state, &format!("s {path_str}")); + + let _ = std::fs::remove_file(&path); + assert!(matches!( + state.input_state, + InputState::WaitingResponse { + command_type: CommandType::LoadTraces, + .. + } + )); + assert!(matches!( + actions.as_slice(), + [Action::SendRuntimeCommand(RuntimeCommand::LoadTraces { filename, traces })] + if filename == &path_str && traces.len() == 1 + )); + } + + #[test] + fn source_shortcut_treats_o_and_s_prefix_filenames_as_source_files() { + for name in ["output", "script"] { + let path = write_temp_trace_file(name); + let path_str = path.to_string_lossy().to_string(); + let mut state = CommandPanelState::new(); + + let actions = CommandParser::parse_command(&mut state, &format!("s {path_str}")); + + let _ = std::fs::remove_file(&path); + assert!(matches!( + state.input_state, + InputState::WaitingResponse { + command_type: CommandType::LoadTraces, + .. + } + )); + assert!(matches!( + actions.as_slice(), + [Action::SendRuntimeCommand(RuntimeCommand::LoadTraces { filename, traces })] + if filename == &path_str && traces.len() == 1 + )); + } + } + + #[test] + fn save_traces_shortcut_still_matches_complete_token() { + let mut state = CommandPanelState::new(); + + let actions = CommandParser::parse_command(&mut state, "s t session.gs"); + + assert!(matches!( + state.input_state, + InputState::WaitingResponse { + command_type: CommandType::SaveTraces, + .. + } + )); + assert!(matches!( + actions.as_slice(), + [Action::SendRuntimeCommand(RuntimeCommand::SaveTraces { + filename: Some(filename), + filter: SaveFilter::All + })] if filename == "session.gs" + )); + } + #[test] fn test_command_completion_exact_match() { // Test exact single match