wasmtime / Issue #1410 support a few DWARF-5 only features · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / Issue #1410 support a few DWARF-5 only features

Wasmtime GitHub notifications bot (Mar 26 2020 at 13:31):

ggreif commented on Issue #1410:

@yurydelendik does your self-assignment mean I shouldn't add any more commits?

Wasmtime GitHub notifications bot (Mar 26 2020 at 13:37):

bjorn3 commented on Issue #1410:

No, it means that he will review this PR once it is ready.

Wasmtime GitHub notifications bot (Apr 22 2020 at 16:03):

ggreif commented on Issue #1410:

@yurydelendik I have now added DW_FORM_addrx* in two spots, and I would expect that frames for subprograms would fill with parameters in lldb. They do for fib-was.c but not for my fib-was.mo.wasm on this branch. I wonder what I am missing here. Where should I start debugging this? Any tips?

Wasmtime GitHub notifications bot (Apr 22 2020 at 17:13):

yurydelendik commented on Issue #1410:

They do for fib-was.c but not for my fib-was.mo.wasm on this branch. I wonder what I am missing here.

@ggreif I don't see DW_AT_high_pc defined for any subprogram in fib-was.mo.wasm. I'm sure how the transform will behave for subprogram without code range defined.

Wasmtime GitHub notifications bot (Apr 22 2020 at 17:13):

yurydelendik edited a comment on Issue #1410:

They do for fib-was.c but not for my fib-was.mo.wasm on this branch. I wonder what I am missing here.

@ggreif I don't see DW_AT_high_pc defined for any subprogram in fib-was.mo.wasm. I'm not sure how the transform will behave for subprogram without code range defined.

Wasmtime GitHub notifications bot (Apr 22 2020 at 17:20):

ggreif commented on Issue #1410:

Correct. I didn't bother adding DW_AT_high_pc yet. But I have seen that the garbage collector does mark my subprograms as live even without it, so I should be safe.

I am mostly wondering if there are any gotchas about the frame structure that I should know. I guess the type definitions are passed through unchanged.

Wasmtime GitHub notifications bot (Apr 22 2020 at 17:23):

ggreif edited a comment on Issue #1410:

Correct. I didn't bother adding DW_AT_high_pc yet. But I have seen that the garbage collector does mark my subprograms as live even without it, so I should be safe. I should say that I can set breakpoints, but (lldb) frame var will be empty a.t.m.

I am mostly wondering if there are any gotchas about the frame structure that I should know. I guess the type definitions are passed through unchanged.

Wasmtime GitHub notifications bot (Apr 22 2020 at 17:30):

yurydelendik commented on Issue #1410:

I didn't bother adding DW_AT_high_pc yet.

Can you add support for DW_AT_high_pc? That's one of main differences between fib-was.c and fib-was.mo.

Wasmtime GitHub notifications bot (Apr 22 2020 at 17:31):

yurydelendik edited a comment on Issue #1410:

I didn't bother adding DW_AT_high_pc yet.

Can you add support for DW_AT_high_pc? That's one of main differences between fib-was.c and fib-was.mo.

P.S. that might be affecting frame var

Wasmtime GitHub notifications bot (Apr 22 2020 at 19:49):

ggreif commented on Issue #1410:

I have updated the binary with DW_AT_high_pc, but I still cannot see the formal parameter in lldb. I am beginning to think that a DWARF-5-ism is the reason, and the code bails out somewhere. I guess the DW_AT_language is not a significant difference?

Wasmtime GitHub notifications bot (Apr 22 2020 at 20:16):

yurydelendik commented on Issue #1410:

Hard to say, I just tried running your branch and it failed at clone_line_program -- the .debug_line format can differ between v4 and v5.

Also DWARF expressions look rather weird: just DW_OP_plus_uconst is old and it also will not work without DW_AT_frame_base.

Try to use cargo run -- wasm2obj to generate object file and inspect the result file using dwarfdump.

Wasmtime GitHub notifications bot (Apr 22 2020 at 20:43):

ggreif commented on Issue #1410:

Hard to say, I just tried running your branch and it failed at clone_line_program -- the .debug_line format can differ between v4 and v5.

I have a quick hack for that. The reason is that file number 0 is allowed in DWARF-5, but not in 4.

Here is the diff
diff --git a/crates/debug/src/transform/line_program.rs b/crates/debug/src/transform/line_program.rs
index 499a1300..862bf5d3 100644
--- a/crates/debug/src/transform/line_program.rs
+++ b/crates/debug/src/transform/line_program.rs
@@ -238,7 +238,8 @@ where
                         };
                         out_program.row().address_offset = address_offset;
                         out_program.row().op_index = *op_index;

-                        out_program.row().file = files[(file_index - 1) as usize];
+                        out_program.row().file = files[(if *file_index as i64 <= 0 {0} else {file_index - 1}) as usize];
+                        // out_program.row().file = if *file_index as i64 <= 0 {write::line::id::zero()} else {files[(file_index - 1) as usize]};
                         out_program.row().line = *line;
                         out_program.row().column = *column;
                         out_program.row().discriminator = *discriminator;
Sorry!

Regarding DW_OP_plus_uconst, yes this is a provocation to lldb which would emit warnings when frame var is processed. Alas they don't appear with my program. They do with the fib-wasm.wasm one.

Which brings me to the question: Are the loc programs in DW_AT_frame_base and DW_AT_location supposed to be concatenated for each argument/local? I am a bit puzzled to see what clang-11 seems to emit. (Those binaries live in the main repo, I temporarily reverted them to clang-10 on my branch.)

Wasmtime GitHub notifications bot (Apr 22 2020 at 20:50):

ggreif commented on Issue #1410:

Wow, thanks for the tip!

cargo run -- wasm2obj -g tests/debug/testsuite/fib-wasm.mo.wasm  tests/debug/testsuite/fib-wasm.mo.out

llvm-dwarfdump tests/debug/testsuite/fib-wasm.mo.out

totally helps! :first_place: :rocket:

Wasmtime GitHub notifications bot (Apr 22 2020 at 20:50):

ggreif edited a comment on Issue #1410:

Wow, thanks for the tip!

cargo run -- wasm2obj -g tests/debug/testsuite/fib-wasm.mo.wasm tests/debug/testsuite/fib-wasm.mo.out

llvm-dwarfdump tests/debug/testsuite/fib-wasm.mo.out

totally helps! :first_place: :rocket:

Wasmtime GitHub notifications bot (Apr 22 2020 at 21:07):

yurydelendik commented on Issue #1410:

Are the loc programs in DW_AT_frame_base and DW_AT_location supposed to be concatenated for each argument/local?

Nope. That was the case before DW_AT_frame_base patch landed. DW_OP_fpreg shall be used from this point on to access frame base.

Wasmtime GitHub notifications bot (Apr 22 2020 at 21:19):

ggreif commented on Issue #1410:

A quick before/after analysis shows

0x0000082b:   DW_TAG_subprogram [2] *
                DW_AT_low_pc [DW_FORM_addrx]    (indexed (00000064) address = 0x00000000000019e5)
                DW_AT_high_pc [DW_FORM_data4]   (0x0000005e)
                DW_AT_name [DW_FORM_strp]   ( .debug_str[0x000004af] = "fib")
                DW_AT_decl_line [DW_FORM_data1] (4)
                DW_AT_decl_column [DW_FORM_data1]   (0x00)
                DW_AT_prototyped [DW_FORM_flag_present] (true)
                DW_AT_external [DW_FORM_flag]   (0x00)

0x00000838:     DW_TAG_formal_parameter [3]
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000145] = "n")
                  DW_AT_decl_line [DW_FORM_data1]   (4)
                  DW_AT_decl_column [DW_FORM_data1] (0x09)
                  DW_AT_type [DW_FORM_ref_udata]    (cu + 0x67 => {0x00000067} "Nat16")
                  DW_AT_location [DW_FORM_exprloc]  (DW_OP_plus_uconst 0x10)

goes in, and the below comes out:

0x00000a9b:   DW_TAG_subprogram [13] *
                DW_AT_low_pc [DW_FORM_addr] (0x0000000000000000 ".text._wasm_function_100")
                DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0000051a] = "fib")
                DW_AT_decl_line [DW_FORM_udata] (4)
                DW_AT_decl_column [DW_FORM_udata]   (0)
                DW_AT_prototyped [DW_FORM_flag] (0x01)
                DW_AT_external [DW_FORM_flag]   (0x00)

0x00000aac:     DW_TAG_formal_parameter [14]
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x000001f9] = "n")
                  DW_AT_decl_line [DW_FORM_udata]   (4)
                  DW_AT_decl_column [DW_FORM_udata] (9)
                  DW_AT_location [DW_FORM_exprloc]  (DW_OP_plus_uconst 0x10)
                  DW_AT_type [DW_FORM_ref4] (cu + 0x00a9 => {0x000000a9} "Nat16")

Note the disappearance of DW_AT_high_pc. This looks suspiciously the effect of this line:
https://github.com/ggreif/wasmtime/blob/master/crates/debug/src/transform/attr.rs#L71

It is explicitly discarded. I wonder why...

Wasmtime GitHub notifications bot (Apr 22 2020 at 21:31):

yurydelendik commented on Issue #1410:

It is explicitly discarded

The before says DW_AT_high_pc [DW_FORM_data4] (0x0000005e) which is normally Udata for DW_AT_high_pc. Other forms of constants as not supported yet.

Wasmtime GitHub notifications bot (Apr 22 2020 at 21:32):

yurydelendik edited a comment on Issue #1410:

It is explicitly discarded

The before says DW_AT_high_pc [DW_FORM_data4] (0x0000005e) which is normally Udata for DW_AT_high_pc. Other forms of constants is not supported yet.

Wasmtime GitHub notifications bot (Apr 22 2020 at 21:33):

yurydelendik edited a comment on Issue #1410:

It is explicitly discarded

The before says DW_AT_high_pc [DW_FORM_data4] (0x0000005e) which is normally Udata for DW_AT_high_pc. Other forms of constants are not supported yet.

Wasmtime GitHub notifications bot (Apr 22 2020 at 22:10):

ggreif commented on Issue #1410:

Right. It gets read as Udata and skipped over it in lines
https://github.com/ggreif/wasmtime/blob/master/crates/debug/src/transform/attr.rs#L71-L73

Similarly for DW_AT_low_pc for Addr in lines
https://github.com/ggreif/wasmtime/blob/master/crates/debug/src/transform/attr.rs#L68-L70.

Both won't get writen. But here is the twist: I use DW_FORM_addrx (maps to DebugAddrIndex), which ends up written (this is new code added by me) in line
https://github.com/ggreif/wasmtime/blob/master/crates/debug/src/transform/attr.rs#L88!

Should I also suppress DebugAddrIndex with DW_AT_low_pc?

This makes me wonder how fib-wasm.wasm is different. The before/after picture seems to have both DW_AT_low_pc and DW_AT_high_pc:

0x00000026:   DW_TAG_subprogram [2] *
                DW_AT_low_pc [DW_FORM_addr] (0x0000000000000003)
                DW_AT_high_pc [DW_FORM_data4]   (0x000000cb)
                DW_AT_name [DW_FORM_strp]   ( .debug_str[0x00000023] = "fib")
                DW_AT_decl_file [DW_FORM_data1] ("./fib-wasm.c")
                DW_AT_decl_line [DW_FORM_data1] (8)
                DW_AT_prototyped [DW_FORM_flag_present] (true)
                DW_AT_type [DW_FORM_ref4]   (cu + 0x008a => {0x0000008a} "int")
                DW_AT_external [DW_FORM_flag_present]   (true)

0x00000039:     DW_TAG_formal_parameter [3]
                  DW_AT_location [DW_FORM_exprloc]  (DW_OP_plus_uconst 0x1c)
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000002b] = "n")
                  DW_AT_decl_file [DW_FORM_data1]   ("./fib-wasm.c")
                  DW_AT_decl_line [DW_FORM_data1]   (8)
                  DW_AT_type [DW_FORM_ref4] (cu + 0x008a => {0x0000008a} "int")

After:

0x00000069:   DW_TAG_subprogram [9] *
                DW_AT_low_pc [DW_FORM_addr] (0x0000000000000000)
                DW_AT_high_pc [DW_FORM_udata]   (142)
                DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0000007a] = "fib")
                DW_AT_decl_file [DW_FORM_udata] ("./fib-wasm.c")
                DW_AT_decl_line [DW_FORM_udata] (8)
                DW_AT_prototyped [DW_FORM_flag] (0x01)
                DW_AT_external [DW_FORM_flag]   (0x01)
                DW_AT_type [DW_FORM_ref4]   (cu + 0x00df => {0x000000df} "int")

...

0x0000008d:     DW_TAG_formal_parameter [11]
                  DW_AT_location [DW_FORM_exprloc]  (DW_OP_plus_uconst 0x1c)
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000086] = "n")
                  DW_AT_decl_file [DW_FORM_udata]   ("./fib-wasm.c")
                  DW_AT_decl_line [DW_FORM_udata]   (8)
                  DW_AT_type [DW_FORM_ref4] (cu + 0x00df => {0x000000df} "int")

Wasmtime GitHub notifications bot (Apr 22 2020 at 22:23):

yurydelendik commented on Issue #1410:

Should I also suppress DebugAddrIndex with DW_AT_low_pc?

I guess so.

Wasmtime GitHub notifications bot (Apr 23 2020 at 04:52):

ggreif commented on Issue #1410:

@yurydelendik I am getting closer. Now I could use some guidance how to compile operand stack Exprloc s (https://yurydelendik.github.io/webassembly-dwarf/#location-descriptions-operand-stack) to CompiledExpression (compile_expression), so that it matches cranelift 's calling convention.

Wasmtime GitHub notifications bot (Apr 23 2020 at 04:55):

ggreif edited a comment on Issue #1410:

@yurydelendik I am getting closer. Now I could use some guidance how to compile operand stack Exprloc s (https://yurydelendik.github.io/webassembly-dwarf/#location-descriptions-operand-stack) to CompiledExpression (compile_expression), so that it matches cranelift 's calling convention. N.B.: I am assuming that formal parameters are passed this way, and are _not_ considered locals.

Wasmtime GitHub notifications bot (Apr 23 2020 at 12:00):

yurydelendik commented on Issue #1410:

I am assuming that formal parameters are passed this way, and are not considered locals.

That's incorrect. WebAssembly function parameters are locals (per specification).

I could use some guidance how to compile operand stack

It is not possible atm. Currently no toolset generate expressions for operand stack, and cranelift does not provide data for it -- thus such expressions are silently ignored. It is possible to use only locals atm.

Wasmtime GitHub notifications bot (Apr 23 2020 at 14:30):

ggreif commented on Issue #1410:

I am marking this as ready for first review, then. I think the changes are mostly benign and incremental, so they pose no elevated risk.
I still have my doubts about the DW_AT_addr_base = 8 thing, and will establish a mutable variable in the CU context.

any guidance how to add sustainable and meaningful tests is welcome.

.debug_line_str will be filled in soon (today?)

Wasmtime GitHub notifications bot (Apr 23 2020 at 15:43):

ggreif edited a comment on Issue #1410:

I am marking this as ready for first review, then. I think the changes are mostly benign and incremental, so they pose no elevated risk.
I still have my doubts about the DW_AT_addr_base = 8 thing, and will establish a mutable variable in the CU context.

any guidance how to add sustainable and meaningful tests is welcome.

.debug_line_str will be filled in soon (today?)

revert clang-10 stuff, remove moc stuff

Wasmtime GitHub notifications bot (Apr 24 2020 at 15:31):

ggreif edited a comment on Issue #1410:

Hard to say, I just tried running your branch and it failed at clone_line_program -- the .debug_line format can differ between v4 and v5.

I have a quick hack for that. The reason is that file number 0 is allowed in DWARF-5, but not in 4.

Here is the diff
diff --git a/crates/debug/src/transform/line_program.rs b/crates/debug/src/transform/line_program.rs
index 499a1300..862bf5d3 100644
--- a/crates/debug/src/transform/line_program.rs
+++ b/crates/debug/src/transform/line_program.rs
@@ -238,7 +238,8 @@ where
                         };
                         out_program.row().address_offset = address_offset;
                         out_program.row().op_index = *op_index;

-                        out_program.row().file = files[(file_index - 1) as usize];
+                        out_program.row().file = files[(if *file_index as i64 <= 0 {0} else {file_index - 1}) as usize];
+                        // out_program.row().file = if *file_index as i64 <= 0 {write::line::id::zero()} else {files[(file_index - 1) as usize]};
                         out_program.row().line = *line;
                         out_program.row().column = *column;
                         out_program.row().discriminator = *discriminator;
Sorry!

Regarding DW_OP_plus_uconst, yes this is a provocation to lldb which would emit warnings when frame var is processed. Alas they don't appear with my program. They do with the fib-wasm.wasm one.

Which brings me to the question: Are the loc programs in DW_AT_frame_base and DW_AT_location supposed to be concatenated for each argument/local? I am a bit puzzled to see what clang-11 seems to emit. (Those binaries live in the main repo, I temporarily reverted them to clang-10 on my branch, because nix doesn't yet have clang-11.)

It boils down to indexing. DWARF-4 is one-based for file and directory lists
file_names[  1]:
           name: "fib-wasm.c"
      dir_index: 0
       mod_time: 0x00000000
         length: 0x00000000
while DWARF-5 is zero-based:
include_directories[  0] =  .debug_line_str[0x00000000] = "."
file_names[  0]:
           name:  .debug_line_str[0x00000002] = "fib-wasm.c"
      dir_index: 0
   md5_checksum: 3444f0aa42db6d50ac2e4e57ac752c75

Wasmtime GitHub notifications bot (Apr 24 2020 at 18:39):

ggreif commented on Issue #1410:

@yurydelendik what about enabling these?

index 1fba1c35c..bba6aef1e 100644
--- a/tests/all/debug/translate.rs
+++ b/tests/all/debug/translate.rs
@@ -25,7 +25,6 @@ fn check_wasm(wasm_path: &str, directives: &str) -> Result<()> {
 }

 #[test]
-#[ignore]
 #[cfg(all(
     any(target_os = "linux", target_os = "macos"),
     target_pointer_width = "64"
@@ -58,7 +57,6 @@ check:        DW_AT_decl_line (10)
 }

 #[test]
-#[ignore]
 #[cfg(all(
     any(target_os = "linux", target_os = "macos"),
     target_pointer_width = "64"

Wasmtime GitHub notifications bot (Apr 24 2020 at 18:47):

yurydelendik commented on Issue #1410:

what about enabling these?

It requires from a developer to have lldb and dwarfdump installed: wasmtime is an umbrella for multiple (sub)project, so we decided to run these tests only on CI.

Wasmtime GitHub notifications bot (Apr 24 2020 at 18:55):

ggreif commented on Issue #1410:

Got it, let's have a look how the _ignore nothing_ flag is passed.

On 4/24/20, Yury Delendik <notifications@github.com> wrote:

what about enabling these?

It requires from a developer to have lldb and dwarfdump installed: wasmtime
is an umbrella for multiple (sub)project, so we decided to run these tests
only on CI.

--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/bytecodealliance/wasmtime/pull/1410#issuecomment-619183127

Wasmtime GitHub notifications bot (Apr 24 2020 at 19:31):

ggreif commented on Issue #1410:

@yurydelendik can you do me a favour and refresh these on my branch with clang-11 (prerelease):
tests/all/debug/testsuite/fib-wasm-dwarf5.wasm tests/all/debug/testsuite/fib-wasm.wasm

The instructions are at the top of the file tests/all/debug/testsuite/fib-wasm.c.

Wasmtime GitHub notifications bot (Apr 24 2020 at 19:33):

ggreif edited a comment on Issue #1410:

@yurydelendik can you do me a favour and refresh these on my branch with clang-11 (prerelease):
tests/all/debug/testsuite/fib-wasm-dwarf5.wasm tests/all/debug/testsuite/fib-wasm.wasm

The instructions are at the top of the file tests/all/debug/testsuite/fib-wasm.c.

UPDATE: I added you to my repo collabs.

Wasmtime GitHub notifications bot (Apr 24 2020 at 19:34):

ggreif edited a comment on Issue #1410:

@yurydelendik can you do me a favour and refresh these on my branch with clang-11 (prerelease):
tests/all/debug/testsuite/fib-wasm-dwarf5.wasm tests/all/debug/testsuite/fib-wasm.wasm

The instructions are at the top of the file tests/all/debug/testsuite/fib-wasm.c.

Reason: I don't have a bleeding edge clang.

UPDATE: I added you to my repo collabs.

Wasmtime GitHub notifications bot (Apr 24 2020 at 19:36):

ggreif edited a comment on Issue #1410:

@yurydelendik can you do me a favour and refresh these on my branch with clang-11 (prerelease):
tests/all/debug/testsuite/fib-wasm-dwarf5.wasm tests/all/debug/testsuite/fib-wasm.wasm

The instructions are at the top of the file tests/all/debug/testsuite/fib-wasm.c.

Reason: I don't have a bleeding edge clang, and using older clang-10 doesn't support DW_AT_frame_base yet. This causes test failures.

UPDATE: I added you to my repo collabs.

Wasmtime GitHub notifications bot (Apr 24 2020 at 20:01):

ggreif edited a comment on Issue #1410:

@yurydelendik can you do me a favour and refresh these on my branch with clang-11 (prerelease):
tests/all/debug/testsuite/fib-wasm-dwarf5.wasm tests/all/debug/testsuite/fib-wasm.wasm

The instructions are at the top of the file tests/all/debug/testsuite/fib-wasm.c.

Reason: I don't have a bleeding edge clang, and using older clang-10 doesn't support DW_AT_frame_base yet. This causes test failures.

UPDATE: I added you to my repo collabs.

Wasmtime GitHub notifications bot (Apr 25 2020 at 14:49):

ggreif edited a comment on Issue #1410:

Got it, let's have a look how the _ignore nothing_ flag is passed.

Like this: cargo test translate -- --ignored

On 4/24/20, Yury Delendik <notifications@github.com> wrote:

what about enabling these?

It requires from a developer to have lldb and dwarfdump installed: wasmtime
is an umbrella for multiple (sub)project, so we decided to run these tests
only on CI.

--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/bytecodealliance/wasmtime/pull/1410#issuecomment-619183127

Wasmtime GitHub notifications bot (Apr 25 2020 at 15:21):

ggreif edited a comment on Issue #1410:

Hard to say, I just tried running your branch and it failed at clone_line_program -- the .debug_line format can differ between v4 and v5.

I have a quick hack for that. The reason is that file number 0 is allowed in DWARF-5, but not in 4.

Here is the diff
diff --git a/crates/debug/src/transform/line_program.rs b/crates/debug/src/transform/line_program.rs
index 499a1300..862bf5d3 100644
--- a/crates/debug/src/transform/line_program.rs
+++ b/crates/debug/src/transform/line_program.rs
@@ -238,7 +238,8 @@ where
                         };
                         out_program.row().address_offset = address_offset;
                         out_program.row().op_index = *op_index;

-                        out_program.row().file = files[(file_index - 1) as usize];
+                        out_program.row().file = files[(if *file_index as i64 <= 0 {0} else {file_index - 1}) as usize];
+                        // out_program.row().file = if *file_index as i64 <= 0 {write::line::id::zero()} else {files[(file_index - 1) as usize]};
                         out_program.row().line = *line;
                         out_program.row().column = *column;
                         out_program.row().discriminator = *discriminator;
Sorry!

Regarding DW_OP_plus_uconst, yes this is a provocation to lldb which would emit warnings when frame var is processed. Alas they don't appear with my program. They do with the fib-wasm.wasm one.

Which brings me to the question: Are the loc programs in DW_AT_frame_base and DW_AT_location supposed to be concatenated for each argument/local? I am a bit puzzled to see what clang-11 seems to emit. (Those binaries live in the main repo, I temporarily reverted them to clang-10 on my branch, because nix doesn't yet have clang-11.)

It boils down to indexing. DWARF-4 is one-based for file and directory lists
file_names[  1]:
           name: "fib-wasm.c"
      dir_index: 0
       mod_time: 0x00000000
         length: 0x00000000
while DWARF-5 is zero-based:
include_directories[  0] =  .debug_line_str[0x00000000] = "."
file_names[  0]:
           name:  .debug_line_str[0x00000002] = "fib-wasm.c"
      dir_index: 0
   md5_checksum: 3444f0aa42db6d50ac2e4e57ac752c75
EDIT: this is fixed in d117d74.

Wasmtime GitHub notifications bot (Apr 27 2020 at 14:32):

ggreif commented on Issue #1410:

@yurydelendik I am grappling with remove_incomplete_ranges in expression.rs. Mostly because I cannot find a definition of range and label. This looks like a (boolean) matrix operation where labels must cover ranges, i.e. the whole row.

But somehow labels correspond to local variables, and ranges are in terms of code offsets (IIUC). I end up with all ranges discarded, because

self.processed_labels = {val1, val4294967294} with .len == 2
and p's label_location only mentions one of the variables:
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 0, end: 62, label_location: {} }  ### 0 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 62, end: 96, label_location: {val1: Stack(ss1)} }  ### 1 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 96, end: 120, label_location: {} }  ### 0 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 120, end: 140, label_location: {val4294967294: Stack(ss0)} }  ### 1 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 140, end: 175, label_location: {} }  ### 0 vs. 2
I am obviously doing something wrong, but since I barely understand what is going on here, I am hard-pressed fixing it.

Wasmtime GitHub notifications bot (Apr 27 2020 at 14:47):

yurydelendik commented on Issue #1410:

I am grappling with remove_incomplete_ranges in expression.rs

Please notice I fixed lots of issues for expression.rs in https://github.com/bytecodealliance/wasmtime/pull/1572 -- currently waiting on review.

ValueLabelRangesBuilder is used during DWARF expression transformation, and if not all value labels (in your case 2 labels) present in particular item of ranges, DWARF expression will not be transformed successfully.

Wasmtime GitHub notifications bot (Apr 27 2020 at 14:47):

ggreif edited a comment on Issue #1410:

@yurydelendik I am grappling with remove_incomplete_ranges in expression.rs. Mostly because I cannot find a definition of range and label. This looks like a (boolean) matrix operation where labels must cover ranges, i.e. the whole row.

But somehow labels correspond to local variables, and ranges are in terms of code offsets (IIUC). I end up with all ranges discarded, because

self.processed_labels = {val1, val4294967294} with .len == 2
and p's label_location only mentions one of the variables:
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 0, end: 62, label_location: {} }  ### 0 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 62, end: 96, label_location: {val1: Stack(ss1)} }  ### 1 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 96, end: 120, label_location: {} }  ### 0 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 120, end: 140, label_location: {val4294967294: Stack(ss0)} }  ### 1 vs. 2
remove_incomplete_ranges iter CachedValueLabelRange { func_index: DefinedFuncIndex(6), start: 140, end: 175, label_location: {} }  ### 0 vs. 2
I am obviously doing something wrong, but since I barely understand what is going on here, I am hard-pressed fixing it.

I am also happy to accept a _zoom_ lecture if that is more convenient for you.

Wasmtime GitHub notifications bot (Apr 27 2020 at 14:51):

ggreif commented on Issue #1410:

I am grappling with remove_incomplete_ranges in expression.rs

Please notice I fixed _lots_ of issues for expression.rs in #1572 -- currently waiting on review.

ValueLabelRangesBuilder is used during DWARF expression transformation, and if not all value labels (in your case 2 labels) present in particular item of ranges, DWARF expression will not be transformed successfully.

So, next step for me to figure out why not both labels show up in CachedValueLabelRange, right?

Wasmtime GitHub notifications bot (Apr 27 2020 at 15:09):

yurydelendik commented on Issue #1410:

So, next step for me to figure out why not both labels show up in CachedValueLabelRange, right?

Correct. Try to use --opt-level 0 maybe it is just cranelift optimization or bug in value label range tracking there.

(BTW, would you like to join https://bytecodealliance.zulipchat.com/ for chat?)

Wasmtime GitHub notifications bot (Apr 27 2020 at 15:20):

yurydelendik commented on Issue #1410:

Let's freeze scope of this PR to the original description. I noticed you trying to work on DWARF expressions related stuff. I'll update .wasm tests and will try to finalize the review.

Wasmtime GitHub notifications bot (Apr 27 2020 at 15:39):

ggreif commented on Issue #1410:

I was trying to figure out if the Exprloc problems I am experiencing are DWARF-5 related. That's the background of my last Q. Never mind, I think this PR is ready. Please go ahead, and I'll iterate on the Expr stuff with follow-up PRs.

Wasmtime GitHub notifications bot (Apr 27 2020 at 16:08):

yurydelendik commented on Issue #1410:

Updated wasm files. fwiw you don't have to provide access to entire repo, submitted as PR branch (option selected by default) gives push access for a reviewer.

Wasmtime GitHub notifications bot (Apr 27 2020 at 20:19):

ggreif commented on Issue #1410:

This looks good. Will you squash?

Wasmtime GitHub notifications bot (Apr 27 2020 at 22:27):

yurydelendik commented on Issue #1410:

Thank you for the patch

Last updated: Nov 22 2024 at 17:03 UTC