diff --git a/e2e-tests/tests/fixtures/scalar_types_program/scalar_types_program.c b/e2e-tests/tests/fixtures/scalar_types_program/scalar_types_program.c index ff55698..f89573e 100644 --- a/e2e-tests/tests/fixtures/scalar_types_program/scalar_types_program.c +++ b/e2e-tests/tests/fixtures/scalar_types_program/scalar_types_program.c @@ -100,6 +100,44 @@ __attribute__((noinline)) static void scalar_by_value( asm volatile("" ::: "memory"); } +__attribute__((noinline)) static void scalar_float_bool_by_value( + _Bool bool_truev, + _Bool bool_falsev, + float f32v, + double f64v) +{ + scalar_sink ^= (uintptr_t)bool_truev; + scalar_sink ^= (uintptr_t)bool_falsev; + scalar_sink ^= (uintptr_t)(int)f32v; + scalar_sink ^= (uintptr_t)(long long)f64v; + asm volatile("" ::: "memory"); +} + +__attribute__((noinline)) static void scalar_stack_by_value( + uint64_t reg1, + uint64_t reg2, + uint64_t reg3, + uint64_t reg4, + uint64_t reg5, + uint64_t reg6, + int8_t stack_i8v, + uint8_t stack_u8v, + int32_t stack_i32v, + uint64_t stack_u64v) +{ + scalar_sink ^= (uintptr_t)reg1; + scalar_sink ^= (uintptr_t)reg2; + scalar_sink ^= (uintptr_t)reg3; + scalar_sink ^= (uintptr_t)reg4; + scalar_sink ^= (uintptr_t)reg5; + scalar_sink ^= (uintptr_t)reg6; + scalar_sink ^= (uintptr_t)(uint8_t)stack_i8v; + scalar_sink ^= (uintptr_t)stack_u8v; + scalar_sink ^= (uintptr_t)(uint32_t)stack_i32v; + scalar_sink ^= (uintptr_t)stack_u64v; + asm volatile("" ::: "memory"); +} + __attribute__((noinline)) static void scalar_probe(int iter) { volatile int8_t i8_neg = (int8_t)-5; @@ -167,6 +205,24 @@ __attribute__((noinline)) static void scalar_probe(int iter) alias_i8_neg, enum_neg, enum_big); + + scalar_float_bool_by_value( + bool_true, + bool_false, + f32_val, + f64_val); + + scalar_stack_by_value( + 11, + 22, + 33, + 44, + 55, + 66, + i8_neg, + u8_big, + i32_neg, + u64_big); } int main(void) diff --git a/e2e-tests/tests/scalar_types_execution.rs b/e2e-tests/tests/scalar_types_execution.rs index e29d60b..31b552a 100644 --- a/e2e-tests/tests/scalar_types_execution.rs +++ b/e2e-tests/tests/scalar_types_execution.rs @@ -81,6 +81,12 @@ trace scalar_anchor { if *ulongp > 8000000000 { print "ULONG_BIG"; } if *u64p > 9223372036854775807 { print "U64_HIGH"; } if *u64p != 0 { print "U64_NONZERO"; } + if *i32p > *u32p { print "MIXED_I32_GT_U32"; } + if *u32p < *i32p { print "MIXED_U32_LT_I32"; } + if *i32p < *u32p { print "MIXED_I32_LT_U32"; } + if *i8p < *u8p { print "MIXED_I8_LT_U8"; } + if *i8p > *u8p { print "MIXED_I8_GT_U8"; } + if *i64p < *u64p { print "MIXED_I64_LT_U64"; } } "#; @@ -101,12 +107,22 @@ trace scalar_anchor { "ULONG_BIG", "U64_HIGH", "U64_NONZERO", + "MIXED_I32_GT_U32", + "MIXED_U32_LT_I32", + "MIXED_I8_LT_U8", + "MIXED_I64_LT_U64", ] { assert!( stdout.contains(marker), "Expected comparison marker {marker}. STDOUT: {stdout}" ); } + for marker in ["MIXED_I32_LT_U32", "MIXED_I8_GT_U8"] { + assert!( + !stdout.contains(marker), + "Unexpected mixed comparison marker {marker}. STDOUT: {stdout}" + ); + } Ok(()) } @@ -195,6 +211,86 @@ trace scalar_by_value { Ok(()) } +#[tokio::test] +async fn test_c_scalar_float_bool_by_value_parameters() -> anyhow::Result<()> { + init(); + + let binary_path = FIXTURES.get_test_binary("scalar_types_program")?; + let target = spawn_scalar_types_binary(&binary_path).await?; + + let script = r#" +trace scalar_float_bool_by_value { + print "BYVAL_BOOL:{}:{}", bool_truev, bool_falsev; + print "BYVAL_FLOAT:{}:{}", f32v, f64v; + if bool_truev != bool_falsev { print "BYVAL_BOOL_DIFF"; } +} +"#; + + let (exit_code, stdout, stderr) = + run_ghostscope_with_script_for_target(script, 4, &target).await?; + target.terminate().await?; + assert_eq!(exit_code, 0, "stderr={stderr} stdout={stdout}"); + + assert!( + stdout.contains("BYVAL_BOOL:true:false"), + "Expected _Bool by-value parameter formatting. STDOUT: {stdout}" + ); + assert!( + stdout.contains("BYVAL_FLOAT:-12.5:123456.25"), + "Expected float and double by-value parameter formatting. STDOUT: {stdout}" + ); + assert!( + stdout.contains("BYVAL_BOOL_DIFF"), + "Expected _Bool by-value comparison marker. STDOUT: {stdout}" + ); + Ok(()) +} + +#[tokio::test] +async fn test_c_scalar_stack_by_value_parameters_preserve_semantics() -> anyhow::Result<()> { + init(); + + let binary_path = FIXTURES.get_test_binary("scalar_types_program")?; + let target = spawn_scalar_types_binary(&binary_path).await?; + + let script = r#" +trace scalar_stack_by_value { + print "STACK_REGS:{}:{}", reg1, reg6; + print "STACK_VALUES:{}:{}:{}:{}", stack_i8v, stack_u8v, stack_i32v, stack_u64v; + if stack_i8v < 0 { print "STACK_I8_NEG"; } + if stack_u8v > 200 { print "STACK_U8_BIG"; } + if stack_i32v < -10000000 { print "STACK_I32_NEG"; } + if stack_u64v > 9223372036854775807 { print "STACK_U64_HIGH"; } +} +"#; + + let (exit_code, stdout, stderr) = + run_ghostscope_with_script_for_target(script, 4, &target).await?; + target.terminate().await?; + assert_eq!(exit_code, 0, "stderr={stderr} stdout={stdout}"); + + assert!( + stdout.contains("STACK_REGS:11:66"), + "Expected register by-value parameters before stack slots. STDOUT: {stdout}" + ); + assert!( + stdout.contains("STACK_VALUES:-5:250:-12345678:18446744073709551610"), + "Expected stack-passed by-value parameters to preserve values. STDOUT: {stdout}" + ); + for marker in [ + "STACK_I8_NEG", + "STACK_U8_BIG", + "STACK_I32_NEG", + "STACK_U64_HIGH", + ] { + assert!( + stdout.contains(marker), + "Expected stack by-value marker {marker}. STDOUT: {stdout}" + ); + } + Ok(()) +} + #[tokio::test] async fn test_c_scalar_typedef_char_enum_and_float_edges() -> anyhow::Result<()> { init(); diff --git a/ghostscope-compiler/src/ebpf/expression.rs b/ghostscope-compiler/src/ebpf/expression.rs index 0294d09..397dbc0 100644 --- a/ghostscope-compiler/src/ebpf/expression.rs +++ b/ghostscope-compiler/src/ebpf/expression.rs @@ -14,6 +14,25 @@ use tracing::debug; // compare cap is provided via compile_options.compare_cap (config: ebpf.compare_cap) +#[derive(Clone, Copy, Debug)] +struct CIntegerComparisonType { + size: u64, + is_unsigned: bool, +} + +impl CIntegerComparisonType { + fn promoted(self) -> Self { + if self.size < 4 { + Self { + size: 4, + is_unsigned: false, + } + } else { + self + } + } +} + impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { pub(crate) fn get_host_pid_tid_values(&mut self) -> Result<(IntValue<'ctx>, IntValue<'ctx>)> { let i32_type = self.context.i32_type(); @@ -245,33 +264,82 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { false } - fn is_unsigned_integer_type(t: &DwarfType) -> bool { + fn integer_comparison_type(t: &DwarfType) -> Option { match t { - DwarfType::BaseType { encoding, .. } => { - *encoding == ghostscope_dwarf::constants::DW_ATE_unsigned.0 as u16 - || *encoding == ghostscope_dwarf::constants::DW_ATE_unsigned_char.0 as u16 + DwarfType::BaseType { encoding, size, .. } => { + let is_unsigned = *encoding + == ghostscope_dwarf::constants::DW_ATE_unsigned.0 as u16 + || *encoding == ghostscope_dwarf::constants::DW_ATE_unsigned_char.0 as u16; + let is_signed = *encoding == ghostscope_dwarf::constants::DW_ATE_signed.0 as u16 + || *encoding == ghostscope_dwarf::constants::DW_ATE_signed_char.0 as u16 + || *encoding == ghostscope_dwarf::constants::DW_ATE_boolean.0 as u16; + if is_unsigned || is_signed { + Some(CIntegerComparisonType { + size: *size, + is_unsigned, + }) + } else { + None + } } - DwarfType::EnumType { base_type, .. } => Self::is_unsigned_integer_type(base_type), + DwarfType::EnumType { + base_type, size, .. + } => Self::integer_comparison_type(base_type).map(|mut ty| { + if ty.size == 0 { + ty.size = *size; + } + ty + }), DwarfType::BitfieldType { - underlying_type, .. - } => Self::is_unsigned_integer_type(underlying_type), + underlying_type, + bit_size, + .. + } => Self::integer_comparison_type(underlying_type).map(|mut ty| { + ty.size = (*bit_size as u64).max(1).div_ceil(8); + ty + }), DwarfType::TypedefType { underlying_type, .. } | DwarfType::QualifiedType { underlying_type, .. - } => Self::is_unsigned_integer_type(underlying_type), - _ => false, + } => Self::integer_comparison_type(underlying_type), + _ => None, } } - fn is_dwarf_unsigned_integer_expr(&mut self, expr: &Expr) -> bool { + fn dwarf_integer_comparison_expr(&mut self, expr: &Expr) -> Option { if let Ok(Some(var)) = self.query_dwarf_for_complex_expr(expr) { if let Some(ref ty) = var.dwarf_type { - return Self::is_unsigned_integer_type(ty); + return Self::integer_comparison_type(ty); } } - false + None + } + + fn usual_arithmetic_prefers_unsigned( + left: CIntegerComparisonType, + right: CIntegerComparisonType, + ) -> bool { + let left = left.promoted(); + let right = right.promoted(); + if left.is_unsigned == right.is_unsigned { + return left.is_unsigned; + } + + let unsigned = if left.is_unsigned { left } else { right }; + let signed = if left.is_unsigned { right } else { left }; + unsigned.size >= signed.size + } + + fn use_unsigned_ordering_for_exprs(&mut self, left: &Expr, right: &Expr) -> bool { + let left_ty = self.dwarf_integer_comparison_expr(left); + let right_ty = self.dwarf_integer_comparison_expr(right); + match (left_ty, right_ty) { + (Some(left), Some(right)) => Self::usual_arithmetic_prefers_unsigned(left, right), + (Some(ty), None) | (None, Some(ty)) => ty.promoted().is_unsigned, + (None, None) => false, + } } /// Ensure that when an expression refers to a DWARF-backed variable (not via address-of), @@ -1694,9 +1762,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { return Ok(phi.as_basic_value()); } - let use_unsigned_ordering = is_ordered - && (self.is_dwarf_unsigned_integer_expr(left) - || self.is_dwarf_unsigned_integer_expr(right)); + let use_unsigned_ordering = + is_ordered && self.use_unsigned_ordering_for_exprs(left, right); // Default eager evaluation for other binary ops let left_val = self.compile_expr(left)?;