[llvm-context, solidity] Add CLI option '-g' to generate debug inform… #74

ghost · 2024-10-10T14:13:09Z

…ation.

Add command line option '-g' to generate source level debug information in the output code. This only works with the LLVM-IR code generator.

Add flag 'emit_debug_info' to the llvm-context optimizer/code-gen settings structure, to record the setting of the '-g' CLI option.

Generate source level debug information for the functions defined when YUL is lowered to LLVM-IR. This includes the deploy_code and runtime functions generated by the compiler.

Generate debug-location information for other constructs that may appear in a contract.

ghost · 2024-10-10T15:20:24Z

Add debug-info to the immutables-load function.

xermicus

As a rule of thumb, repeated code-gen logic should be implemented as methods to the context and this applies to debug info as well. I suggest basically any if let Some(dinfo) = context.debug_info() be a method context.debug_info_something instead. They can all return early using let else if debug info isn't requested. This will 1) solve the issue of the heavy code duplication, 2) greatly ease the reading flow of the implementation as all the debug info related stuff can be skipped by skipping just over a single line of code and 3) make it consistent with the regular code-gen related code.

Can you please also enable debug info in the integration test suite by default to get test coverage?

Thanks!

crates/llvm-context/src/polkavm/context/function/runtime/deploy_code.rs

crates/llvm-context/src/polkavm/context/function/runtime/immutable_data_load.rs

crates/llvm-context/src/polkavm/context/function/runtime/runtime_code.rs

crates/solidity/src/project/contract/mod.rs

crates/solidity/src/yul/parser/statement/function_definition.rs

crates/llvm-context/src/polkavm/context/function/runtime/entry.rs

crates/solidity/src/yul/parser/statement/function_definition.rs

crates/solidity/src/project/contract/mod.rs

ghost · 2024-10-15T09:41:32Z

Reworked 'set_debug_location' to make it easier to set debug locations.

xermicus

Reworked 'set_debug_location' to make it easier to set debug locations.

Thanks! I really like it. Half of the lines added are gone :) However there are still some inconsistencies I'd like to iron out.

Also the EVMLA is rather untouched yet, I think it requires some changes to emit correct debug info too.

crates/llvm-context/src/polkavm/context/function/runtime/immutable_data_load.rs

crates/llvm-context/src/polkavm/context/function/runtime/entry.rs

xermicus · 2024-10-15T10:55:48Z

crates/llvm-context/src/polkavm/context/mod.rs

+        scope: Option<DIScope<'ctx>>,
+    ) -> anyhow::Result<()> {
+        if let Some(dinfo) = self.debug_info() {
+            let line_num: u32 = std::cmp::min(line, u32::MAX as usize) as u32;


We have a lot of those. A line number greater than u32::MAX should never occur. I doubt solc does emit this. So this should be try_into?

However, the parser must handle this, i.e. error out if reported line and column numbers can not be parsed. During codegen we expect a syntactically correct program. And then this is no longer necessary.

…ation. Add command line option '-g' to generate source level debug information in the output code. This only works with the LLVM-IR code generator. Add flag 'emit_debug_info' to the llvm-context optimizer/code-gen settings structure, to record the setting of the '-g' CLI option. Generate source level debug information for the functions defined when YUL is lowered to LLVM-IR. This includes the deploy_code and runtime functions generated by the compiler. Generate debug-location information for other constructs that may appear in a contract.

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

…ation. Add command line option '-g' to generate source level debug information in the output code. This only works with the LLVM-IR code generator. Add flag 'emit_debug_info' to the llvm-context optimizer/code-gen settings structure, to record the setting of the '-g' CLI option. Generate source level debug information for the functions defined when YUL is lowered to LLVM-IR. This includes the deploy_code and runtime functions generated by the compiler. Generate debug-location information for other constructs that may appear in a contract.

xermicus · 2024-10-17T07:36:14Z

Cargo.toml

-polkavm-linker = "0.10"
-polkavm-disassembler = "0.10"
-polkavm = "0.10"
+polkavm-common = "0.12"


I don't get it - this is already in paritytech:main? :D

maybe this goes away after a rebase against main?

This is part of a commit on the main branch that was accidentally included.

Ok, can you please rebase so we get a clean diff?

…ation. Add command line option '-g' to generate source level debug information in the output code. This only works with the LLVM-IR code generator. Add flag 'emit_debug_info' to the llvm-context optimizer/code-gen settings structure, to record the setting of the '-g' CLI option. Generate source level debug information for the functions defined when YUL is lowered to LLVM-IR. This includes the deploy_code and runtime functions generated by the compiler. Generate debug-location information for other constructs that may appear in a contract.

xermicus · 2024-10-18T10:12:00Z

crates/llvm-context/src/polkavm/context/mod.rs

+        name: &str,
+        line_no: u32,
+    ) -> anyhow::Result<inkwell::debug_info::DISubprogram<'ctx>> {
+        let dinfo = self.debug_info().expect("expected debug-info builder");


Why is this not a let else with an early return?

xermicus · 2024-10-18T10:17:55Z

crates/llvm-context/src/polkavm/context/mod.rs

        Ok(())
    }

+    /// Pushes a debug-info scope to the stack.
+    pub fn push_debug_scope(&self, scope: DIScope<'ctx>) {
+        if self.debug_info().is_some() {


Should prefer if let instead of is_some() followed by unwraps when we operate on optional values only if they are Some (noticed this in other places too).

xermicus · 2024-10-18T10:29:53Z

crates/llvm-context/src/polkavm/context/function/runtime/immutable_data_load.rs

+        if context.debug_info().is_some() {
+            context.builder().unset_current_debug_location();
+            let func_scope = context
+                .set_current_function_debug_info(runtime::FUNCTION_LOAD_IMMUTABLE_DATA, 0)?
+                .as_debug_info_scope();
+            context.push_debug_scope(func_scope);
+        }
+


This code is duplicated 3x. Since it is needed every time we lower functions it should be a helper on context instead.

…ation. Add command line option '-g' to generate source level debug information in the output code. This only works with the LLVM-IR code generator. Add flag 'emit_debug_info' to the llvm-context optimizer/code-gen settings structure, to record the setting of the '-g' CLI option. Generate source level debug information for the functions defined when YUL is lowered to LLVM-IR. This includes the deploy_code and runtime functions generated by the compiler. Generate debug-location information for other constructs that may appear in a contract.

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

- remove unused runtime API imports and constants - move runtime api symbols into the revive-runtime-api crate Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Signed-off-by: xermicus <cyrill@parity.io>

…l object (#52) Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

- The storage pointer values will no longer be truncated to the register size, allowing for the use of arbitrary storage keys - Failed storage value reads will now guarantee to return the zero value

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Signed-off-by: xermicus <cyrill@parity.io>

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

…ation. Add command line option '-g' to generate source level debug information in the output code. This only works with the LLVM-IR code generator. Add flag 'emit_debug_info' to the llvm-context optimizer/code-gen settings structure, to record the setting of the '-g' CLI option. Generate source level debug information for the functions defined when YUL is lowered to LLVM-IR. This includes the deploy_code and runtime functions generated by the compiler. Generate debug-location information for other constructs that may appear in a contract.

xermicus

Debug info generation is not enabled by default in our integration test right? All test pass when I switch the default behavior to always generate debug info, so we should be good to enable it.

xermicus · 2024-11-09T07:35:32Z

crates/solidity/src/tests/cli-tests/tests/common.test.ts

@@ -76,3 +76,57 @@ describe("Default run a command from the help", () => {
        expect(result.output).not.toMatch(/([Ee]rror|[Ww]arning|[Ff]ail)/i);
    });
 });
+
+describe("Run resolc with source debug information enabled", () => {


Can we verify in this test that some of the expected debug info is in the actual final code blob? Doesn't need to be fancy, a string match or something we expect only to appear with debug info would be sufficient.

…ation. Add command line option '-g' to generate source level debug information in the output code. This only works with the LLVM-IR code generator. Add flag 'emit_debug_info' to the llvm-context optimizer/code-gen settings structure, to record the setting of the '-g' CLI option. Generate source level debug information for the functions defined when YUL is lowered to LLVM-IR. This includes the deploy_code and runtime functions generated by the compiler. Generate debug-location information for other constructs that may appear in a contract.

ghost force-pushed the debug-info branch from 39a94b2 to 5a919a2 Compare October 10, 2024 15:19

xermicus requested changes Oct 10, 2024

View reviewed changes

xermicus reviewed Oct 11, 2024

View reviewed changes

crates/solidity/src/project/contract/mod.rs Outdated Show resolved Hide resolved

ghost force-pushed the debug-info branch 6 times, most recently from 6620874 to 38e8e3b Compare October 15, 2024 09:40

xermicus reviewed Oct 15, 2024

View reviewed changes

ghost force-pushed the debug-info branch from 38e8e3b to 84ab968 Compare October 15, 2024 13:15

wpt967 and others added 3 commits October 15, 2024 16:30

bump polkavm and polkadot-sdk (#87)

65aa72e

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

xermicus reviewed Oct 17, 2024

View reviewed changes

wpt967 added 3 commits October 17, 2024 10:16

xermicus reviewed Oct 18, 2024

View reviewed changes

wpt967 added 3 commits October 18, 2024 11:56

ghost marked this pull request as draft October 31, 2024 10:27

xermicus and others added 4 commits October 31, 2024 12:07

Makefile: remove duplicate workspace tests from the test target (#90)

cedd6bc

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

tidy up the runtime API crate (#85)

2989e71

- remove unused runtime API imports and constants - move runtime api symbols into the revive-runtime-api crate Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Implement extcodehash (#77)

eaabae5

bump polkadot-sdk and polkavm (#94)

fbdac01

Signed-off-by: xermicus <cyrill@parity.io>

xermicus and others added 12 commits October 31, 2024 12:07

standard json output: skip serializing keys instead of emitting a nul…

4d44d1a

…l object (#52) Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

Set evm.deployedBytecode to the value of evm.bytecode (#95)

6eaa692

support full storage key space (#100)

fe6fdf0

- The storage pointer values will no longer be truncated to the register size, allowing for the use of arbitrary storage keys - Failed storage value reads will now guarantee to return the zero value

update polkadot-sdk and inkwell dependencies (#101)

083fb85

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

update LLVM target features (#102)

c4db2d7

Signed-off-by: xermicus <cyrill@parity.io>

support the origin opcode (#103)

d78aded

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

update the polkadot-sdk and polkavm dependencies (#104)

eb8d102

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

display the git revision version information (#105)

677a440

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

ghost marked this pull request as ready for review November 4, 2024 11:52

Merge branch 'main' into debug-info

84055ef

xermicus reviewed Nov 9, 2024

View reviewed changes

Repository owner closed this by deleting the head repository Nov 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[llvm-context, solidity] Add CLI option '-g' to generate debug inform… #74

[llvm-context, solidity] Add CLI option '-g' to generate debug inform… #74

ghost commented Oct 10, 2024

ghost commented Oct 10, 2024

xermicus left a comment •

edited

Loading

ghost commented Oct 15, 2024

xermicus left a comment •

edited

Loading

xermicus Oct 15, 2024 •

edited

Loading

xermicus Oct 17, 2024

xermicus Oct 17, 2024

ghost Oct 17, 2024

xermicus Oct 18, 2024

xermicus Oct 18, 2024

xermicus Oct 18, 2024

xermicus Oct 18, 2024

xermicus left a comment

xermicus Nov 9, 2024

[llvm-context, solidity] Add CLI option '-g' to generate debug inform… #74

[llvm-context, solidity] Add CLI option '-g' to generate debug inform… #74

Conversation

ghost commented Oct 10, 2024

ghost commented Oct 10, 2024

xermicus left a comment • edited Loading

Choose a reason for hiding this comment

ghost commented Oct 15, 2024

xermicus left a comment • edited Loading

Choose a reason for hiding this comment

xermicus Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xermicus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xermicus left a comment •

edited

Loading

xermicus left a comment •

edited

Loading

xermicus Oct 15, 2024 •

edited

Loading