From c285a6ec3d0a558ccf0582d8aa05509ec4bc36d4 Mon Sep 17 00:00:00 2001 From: xermicus Date: Thu, 17 Jul 2025 09:53:54 +0200 Subject: [PATCH] add columns to debug information (#362) - Add column numbers to debug information. - Do not build allocas at entry for now. --------- Signed-off-by: Cyrill Leutwiler --- CHANGELOG.md | 1 + crates/integration/codesize.json | 16 ++++---- .../context/function/runtime/deploy_code.rs | 2 +- .../polkavm/context/function/runtime/entry.rs | 1 + .../context/function/runtime/runtime_code.rs | 1 + .../llvm-context/src/polkavm/context/mod.rs | 37 +++++++++++++------ .../src/polkavm/context/runtime.rs | 1 + .../llvm-context/src/polkavm/context/tests.rs | 5 +++ crates/yul/src/parser/statement/assignment.rs | 4 +- crates/yul/src/parser/statement/block.rs | 13 +++++-- .../statement/expression/function_call/mod.rs | 2 +- crates/yul/src/parser/statement/for_loop.rs | 4 ++ .../parser/statement/function_definition.rs | 8 +++- .../src/parser/statement/if_conditional.rs | 2 +- crates/yul/src/parser/statement/object.rs | 4 +- crates/yul/src/parser/statement/switch/mod.rs | 4 ++ .../parser/statement/variable_declaration.rs | 4 +- 17 files changed, 75 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8028cff..cc49f87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Supported `polkadot-sdk` rev: `2503.0.1` ### Added - Line debug information per YUL builtin and for `if` statements. +- Column numbers in debug information. - Support for the YUL optimizer details in the standard json input definition. ### Fixed diff --git a/crates/integration/codesize.json b/crates/integration/codesize.json index 11e9545..fc47648 100644 --- a/crates/integration/codesize.json +++ b/crates/integration/codesize.json @@ -1,10 +1,10 @@ { - "Baseline": 939, - "Computation": 2282, - "DivisionArithmetics": 8849, - "ERC20": 18308, - "Events": 1640, - "FibonacciIterative": 1497, - "Flipper": 2099, - "SHA1": 8243 + "Baseline": 960, + "Computation": 2367, + "DivisionArithmetics": 9108, + "ERC20": 17655, + "Events": 1680, + "FibonacciIterative": 1536, + "Flipper": 2137, + "SHA1": 8299 } \ No newline at end of file diff --git a/crates/llvm-context/src/polkavm/context/function/runtime/deploy_code.rs b/crates/llvm-context/src/polkavm/context/function/runtime/deploy_code.rs index 279b1e8..e961040 100644 --- a/crates/llvm-context/src/polkavm/context/function/runtime/deploy_code.rs +++ b/crates/llvm-context/src/polkavm/context/function/runtime/deploy_code.rs @@ -48,6 +48,7 @@ where function_type, 0, Some(inkwell::module::Linkage::External), + None, )?; self.inner.declare(context) @@ -74,7 +75,6 @@ where } context.set_basic_block(context.current_function().borrow().return_block()); - context.set_debug_location(0, 0, None)?; context.build_return(None); context.pop_debug_scope(); diff --git a/crates/llvm-context/src/polkavm/context/function/runtime/entry.rs b/crates/llvm-context/src/polkavm/context/function/runtime/entry.rs index ae40a99..0724d98 100644 --- a/crates/llvm-context/src/polkavm/context/function/runtime/entry.rs +++ b/crates/llvm-context/src/polkavm/context/function/runtime/entry.rs @@ -145,6 +145,7 @@ where entry_function_type, 0, Some(inkwell::module::Linkage::External), + None, )?; context.declare_global( diff --git a/crates/llvm-context/src/polkavm/context/function/runtime/runtime_code.rs b/crates/llvm-context/src/polkavm/context/function/runtime/runtime_code.rs index 8cf1435..1718392 100644 --- a/crates/llvm-context/src/polkavm/context/function/runtime/runtime_code.rs +++ b/crates/llvm-context/src/polkavm/context/function/runtime/runtime_code.rs @@ -48,6 +48,7 @@ where function_type, 0, Some(inkwell::module::Linkage::External), + None, )?; self.inner.declare(context) diff --git a/crates/llvm-context/src/polkavm/context/mod.rs b/crates/llvm-context/src/polkavm/context/mod.rs index 4560bc2..5a48ddc 100644 --- a/crates/llvm-context/src/polkavm/context/mod.rs +++ b/crates/llvm-context/src/polkavm/context/mod.rs @@ -463,6 +463,7 @@ where r#type: inkwell::types::FunctionType<'ctx>, return_values_length: usize, linkage: Option, + location: Option<(u32, u32)>, ) -> anyhow::Result>>> { let value = self.module().add_function(name, r#type, linkage); @@ -478,7 +479,8 @@ where Some(scp) => scp, }; self.push_debug_scope(func_scope.as_debug_info_scope()); - self.set_debug_location(0, 0, Some(func_scope.as_debug_info_scope()))?; + let (line, column) = location.unwrap_or((0, 0)); + self.set_debug_location(line, column, Some(func_scope.as_debug_info_scope()))?; } let entry_block = self.llvm.append_basic_block(value, "entry"); @@ -533,7 +535,11 @@ where /// Sets the current active function. If debug-info generation is enabled, /// constructs a debug-scope and pushes in on the scope-stack. - pub fn set_current_function(&mut self, name: &str, line: Option) -> anyhow::Result<()> { + pub fn set_current_function( + &mut self, + name: &str, + location: Option<(u32, u32)>, + ) -> anyhow::Result<()> { let function = self.functions.get(name).cloned().ok_or_else(|| { anyhow::anyhow!("Failed to activate an undeclared function `{}`", name) })?; @@ -542,7 +548,8 @@ where if let Some(scope) = self.current_function().borrow().get_debug_scope() { self.push_debug_scope(scope); } - self.set_debug_location(line.unwrap_or_default(), 0, None)?; + let (line, column) = location.unwrap_or_default(); + self.set_debug_location(line, column, None)?; Ok(()) } @@ -723,17 +730,23 @@ where r#type: T, name: &str, ) -> Pointer<'ctx> { - let current_block = self.basic_block(); - let entry_block = self.current_function().borrow().entry_block(); + // TODO: Revisit. While at entry should be preferred in theory: + // - It has negligible code size impact on real word contracts. + // - Sometimes has negative impact on code size. + // - Messes up debug information used to analyze code size issues. + self.build_alloca(r#type, name) - match entry_block.get_first_instruction() { - Some(instruction) => self.builder().position_before(&instruction), - None => self.builder().position_at_end(entry_block), - } + // let current_block = self.basic_block(); + // let entry_block = self.current_function().borrow().entry_block(); - let pointer = self.build_alloca(r#type, name); - self.set_basic_block(current_block); - pointer + // match entry_block.get_first_instruction() { + // Some(instruction) => self.builder().position_before(&instruction), + // None => self.builder().position_at_end(entry_block), + // } + + // let pointer = self.build_alloca(r#type, name); + // self.set_basic_block(current_block); + // pointer } /// Builds an aligned stack allocation at the current position. diff --git a/crates/llvm-context/src/polkavm/context/runtime.rs b/crates/llvm-context/src/polkavm/context/runtime.rs index 1617eb0..315a92a 100644 --- a/crates/llvm-context/src/polkavm/context/runtime.rs +++ b/crates/llvm-context/src/polkavm/context/runtime.rs @@ -35,6 +35,7 @@ where Self::r#type(context), 0, Some(inkwell::module::Linkage::External), + None, )?; let mut attributes = Self::ATTRIBUTES.to_vec(); diff --git a/crates/llvm-context/src/polkavm/context/tests.rs b/crates/llvm-context/src/polkavm/context/tests.rs index 9060a3d..721a967 100644 --- a/crates/llvm-context/src/polkavm/context/tests.rs +++ b/crates/llvm-context/src/polkavm/context/tests.rs @@ -40,6 +40,7 @@ pub fn check_attribute_null_pointer_is_invalid() { .fn_type(&[context.word_type().into()], false), 1, Some(inkwell::module::Linkage::External), + None, ) .expect("Failed to add function"); assert!(!function @@ -63,6 +64,7 @@ pub fn check_attribute_optimize_for_size_mode_3() { .fn_type(&[context.word_type().into()], false), 1, Some(inkwell::module::Linkage::External), + None, ) .expect("Failed to add function"); assert!(!function @@ -86,6 +88,7 @@ pub fn check_attribute_optimize_for_size_mode_z() { .fn_type(&[context.word_type().into()], false), 1, Some(inkwell::module::Linkage::External), + None, ) .expect("Failed to add function"); assert!(function @@ -109,6 +112,7 @@ pub fn check_attribute_min_size_mode_3() { .fn_type(&[context.word_type().into()], false), 1, Some(inkwell::module::Linkage::External), + None, ) .expect("Failed to add function"); assert!(!function @@ -132,6 +136,7 @@ pub fn check_attribute_min_size_mode_z() { .fn_type(&[context.word_type().into()], false), 1, Some(inkwell::module::Linkage::External), + None, ) .expect("Failed to add function"); assert!(function diff --git a/crates/yul/src/parser/statement/assignment.rs b/crates/yul/src/parser/statement/assignment.rs index c3079e2..77ccd8f 100644 --- a/crates/yul/src/parser/statement/assignment.rs +++ b/crates/yul/src/parser/statement/assignment.rs @@ -119,7 +119,7 @@ where mut self, context: &mut revive_llvm_context::PolkaVMContext, ) -> anyhow::Result<()> { - context.set_debug_location(self.location.line, 0, None)?; + context.set_debug_location(self.location.line, self.location.column, None)?; let value = match self.initializer.into_llvm(context)? { Some(value) => value, @@ -149,7 +149,7 @@ where context.build_store(tuple_pointer, value)?; for (index, binding) in self.bindings.into_iter().enumerate() { - context.set_debug_location(self.location.line, 0, None)?; + context.set_debug_location(self.location.line, self.location.column, None)?; let field_pointer = context.build_gep( tuple_pointer, diff --git a/crates/yul/src/parser/statement/block.rs b/crates/yul/src/parser/statement/block.rs index 07d38c2..f7c3fb4 100644 --- a/crates/yul/src/parser/statement/block.rs +++ b/crates/yul/src/parser/statement/block.rs @@ -155,7 +155,10 @@ where function.into_llvm(context)?; } - context.set_current_function(current_function.as_str(), Some(self.location.line))?; + context.set_current_function( + current_function.as_str(), + Some((self.location.line, self.location.column)), + )?; if let Some(debug_info) = context.debug_info() { let di_builder = debug_info.builder(); @@ -169,12 +172,16 @@ where ) .as_debug_info_scope(); context.push_debug_scope(di_block_scope); - context.set_debug_location(self.location.line, 0, None)?; + context.set_debug_location(self.location.line, self.location.column, None)?; } context.set_basic_block(current_block); for statement in local_statements.into_iter() { - context.set_debug_location(statement.location().line, 0, None)?; + context.set_debug_location( + statement.location().line, + statement.location().column, + None, + )?; if context.basic_block().get_terminator().is_some() { break; } diff --git a/crates/yul/src/parser/statement/expression/function_call/mod.rs b/crates/yul/src/parser/statement/expression/function_call/mod.rs index b825ca4..7f1378d 100644 --- a/crates/yul/src/parser/statement/expression/function_call/mod.rs +++ b/crates/yul/src/parser/statement/expression/function_call/mod.rs @@ -123,7 +123,7 @@ impl FunctionCall { D: revive_llvm_context::PolkaVMDependency + Clone, { let location = self.location; - context.set_debug_location(location.line, 0, None)?; + context.set_debug_location(location.line, location.column, None)?; match self.name { Name::UserDefined(name) => { diff --git a/crates/yul/src/parser/statement/for_loop.rs b/crates/yul/src/parser/statement/for_loop.rs index 822e9b9..3923125 100644 --- a/crates/yul/src/parser/statement/for_loop.rs +++ b/crates/yul/src/parser/statement/for_loop.rs @@ -72,6 +72,7 @@ where let increment_block = context.append_basic_block("for_increment"); let join_block = context.append_basic_block("for_join"); + context.set_debug_location(self.location.line, self.location.column, None)?; context.build_unconditional_branch(condition_block); context.set_basic_block(condition_block); let condition = self @@ -80,6 +81,7 @@ where .expect("Always exists") .access(context)? .into_int_value(); + context.set_debug_location(self.location.line, self.location.column, None)?; let condition = context.builder().build_int_z_extend_or_bit_cast( condition, context.word_type(), @@ -98,10 +100,12 @@ where context.set_basic_block(body_block); self.body.into_llvm(context)?; context.build_unconditional_branch(increment_block); + context.set_debug_location(self.location.line, self.location.column, None)?; context.set_basic_block(increment_block); self.finalizer.into_llvm(context)?; context.build_unconditional_branch(condition_block); + context.set_debug_location(self.location.line, self.location.column, None)?; context.pop_loop(); context.set_basic_block(join_block); diff --git a/crates/yul/src/parser/statement/function_definition.rs b/crates/yul/src/parser/statement/function_definition.rs index e776e4b..d3b2b4e 100644 --- a/crates/yul/src/parser/statement/function_definition.rs +++ b/crates/yul/src/parser/statement/function_definition.rs @@ -212,6 +212,7 @@ where function_type, self.result.len(), Some(inkwell::module::Linkage::External), + Some((self.location.line, self.location.column)), )?; revive_llvm_context::PolkaVMFunction::set_attributes( context.llvm(), @@ -230,7 +231,10 @@ where mut self, context: &mut revive_llvm_context::PolkaVMContext, ) -> anyhow::Result<()> { - context.set_current_function(self.identifier.as_str(), Some(self.location.line))?; + context.set_current_function( + self.identifier.as_str(), + Some((self.location.line, self.location.column)), + )?; context.set_basic_block(context.current_function().borrow().entry_block()); let r#return = context.current_function().borrow().r#return(); @@ -290,7 +294,7 @@ where } self.body.into_llvm(context)?; - context.set_debug_location(self.location.line, 0, None)?; + context.set_debug_location(self.location.line, self.location.column, None)?; match context .basic_block() diff --git a/crates/yul/src/parser/statement/if_conditional.rs b/crates/yul/src/parser/statement/if_conditional.rs index a833b8b..5408a52 100644 --- a/crates/yul/src/parser/statement/if_conditional.rs +++ b/crates/yul/src/parser/statement/if_conditional.rs @@ -53,13 +53,13 @@ where D: revive_llvm_context::PolkaVMDependency + Clone, { fn into_llvm(self, context: &mut revive_llvm_context::PolkaVMContext) -> anyhow::Result<()> { - context.set_debug_location(self.location.line, 0, None)?; let condition = self .condition .into_llvm(context)? .expect("Always exists") .access(context)? .into_int_value(); + context.set_debug_location(self.location.line, self.location.column, None)?; let condition = context.builder().build_int_z_extend_or_bit_cast( condition, context.word_type(), diff --git a/crates/yul/src/parser/statement/object.rs b/crates/yul/src/parser/statement/object.rs index 7af755d..ffd9202 100644 --- a/crates/yul/src/parser/statement/object.rs +++ b/crates/yul/src/parser/statement/object.rs @@ -279,17 +279,17 @@ where context.push_debug_scope(object_scope.as_debug_info_scope()); } + context.set_debug_location(self.location.line, self.location.column, None)?; if self.identifier.ends_with("_deployed") { revive_llvm_context::PolkaVMRuntimeCodeFunction::new(self.code).into_llvm(context)?; } else { revive_llvm_context::PolkaVMDeployCodeFunction::new(self.code).into_llvm(context)?; } - context.set_debug_location(self.location.line, 0, None)?; if let Some(object) = self.inner_object { object.into_llvm(context)?; } - context.set_debug_location(self.location.line, 0, None)?; + context.set_debug_location(self.location.line, self.location.column, None)?; context.pop_debug_scope(); diff --git a/crates/yul/src/parser/statement/switch/mod.rs b/crates/yul/src/parser/statement/switch/mod.rs index 5c63edf..b7d0aad 100644 --- a/crates/yul/src/parser/statement/switch/mod.rs +++ b/crates/yul/src/parser/statement/switch/mod.rs @@ -123,6 +123,7 @@ where D: revive_llvm_context::PolkaVMDependency + Clone, { fn into_llvm(self, context: &mut revive_llvm_context::PolkaVMContext) -> anyhow::Result<()> { + context.set_debug_location(self.location.line, self.location.column, None)?; let scrutinee = self.expression.into_llvm(context)?; if self.cases.is_empty() { @@ -143,6 +144,7 @@ where .append_basic_block(format!("switch_case_branch_{}_block", index + 1).as_str()); context.set_basic_block(expression_block); case.block.into_llvm(context)?; + context.set_debug_location(self.location.line, self.location.column, None)?; context.build_unconditional_branch(join_block); branches.push((constant.into_int_value(), expression_block)); @@ -159,6 +161,7 @@ where None => join_block, }; + context.set_debug_location(self.location.line, self.location.column, None)?; context.set_basic_block(current_block); context.builder().build_switch( scrutinee @@ -169,6 +172,7 @@ where branches.as_slice(), )?; + context.set_debug_location(self.location.line, self.location.column, None)?; context.set_basic_block(join_block); Ok(()) diff --git a/crates/yul/src/parser/statement/variable_declaration.rs b/crates/yul/src/parser/statement/variable_declaration.rs index fdf917e..849b152 100644 --- a/crates/yul/src/parser/statement/variable_declaration.rs +++ b/crates/yul/src/parser/statement/variable_declaration.rs @@ -101,7 +101,7 @@ where ) -> anyhow::Result<()> { if self.bindings.len() == 1 { let identifier = self.bindings.remove(0); - context.set_debug_location(self.location.line, 0, None)?; + context.set_debug_location(self.location.line, self.location.column, None)?; let identifier_type = identifier.r#type.clone().unwrap_or_default(); let r#type = identifier_type.into_llvm(context); let pointer = context.build_alloca(r#type, identifier.inner.as_str()); @@ -133,7 +133,7 @@ where } for (index, binding) in self.bindings.iter().enumerate() { - context.set_debug_location(self.location.line, 0, None)?; + context.set_debug_location(self.location.line, self.location.column, None)?; let yul_type = binding .r#type