support a few DWARF-5 only features #1410

ggreif · 2020-03-26T00:51:33Z

See #932.

accept and pass DebugAddrIndex, DebugStrOffsetsIndex attributes
skip DebugAddrBase, DebugStrOffsetsBase attribute when transforming, these are managed by the compilation unit elsewhere
accept and resolve DebugLineStrRef in line programs
read .debug_addr
read .debug_rnglists
read .debug_loclists
read .debug_line_str
read .debug_str_offsets
perform the DebugAddrIndex and DebugStrOffsetsIndex indirections

TODO:

tests (added DWARF-5 test, but it needs a refresh, lldb test also needed).

ggreif · 2020-03-26T13:31:54Z

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

bjorn3 · 2020-03-26T13:37:06Z

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

* skip DebugAddressBase attribute when transforming * read .debug_addr * read .debug_rnglists

need to skip the header

* skip DebugAddressBase attribute when transforming * read .debug_addr * read .debug_rnglists

need to skip the header

in order to support these, one has to pass around the references to their sections too, and to the `unit`, because the str_offsets_base lives there

crates/debug/src/transform/attr.rs

in the line program now these tests pass: test debug::translate::test_debug_dwarf_translate ... ok test debug::translate::test_debug_dwarf5_translate ... ok

ggreif · 2020-04-24T18:39:58Z

@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"

yurydelendik · 2020-04-24T18:47:09Z

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.

ggreif · 2020-04-24T18:55:24Z

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 ***@***.***> 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: #1410 (comment)

ggreif · 2020-04-24T19:31:23Z

@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.

ggreif · 2020-04-27T14:32:58Z

@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.

yurydelendik · 2020-04-27T14:47:35Z

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.

ggreif · 2020-04-27T14:51:09Z

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?

yurydelendik · 2020-04-27T15:09:39Z

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?)

yurydelendik · 2020-04-27T15:20:35Z

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.

ggreif · 2020-04-27T15:38:59Z

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.

yurydelendik · 2020-04-27T16:08:00Z

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.

clang version 11.0.0 (https://github.com/llvm/llvm-project.git 1090a830692a863ccb091e3fad8cc1a287417493)

yurydelendik

Looks good. Few items to address before merge.

crates/debug/src/transform/attr.rs

crates/debug/src/transform/unit.rs

review feedback: don't cross-check index bases

ggreif · 2020-04-27T20:19:23Z

This looks good. Will you squash?

yurydelendik · 2020-04-27T22:27:36Z

Thank you for the patch

yurydelendik self-assigned this Mar 26, 2020

ggreif mentioned this pull request Mar 26, 2020

Step-through debugging dfinity/motoko#1244

Closed

7 tasks

ggreif force-pushed the master branch 2 times, most recently from d8812f4 to 19f932d Compare April 17, 2020 19:35

ggreif added 7 commits April 20, 2020 13:10

support a few DWARF-5 only features

c0110c8

* skip DebugAddressBase attribute when transforming * read .debug_addr * read .debug_rnglists

cargo fmt --all

a6137b0

read .debug_loclists when present

bf1cdd7

tmp: enforce addr base to be zero

c382447

fixup: expect 8 as DebugAddrBase

f48c00d

need to skip the header

add an early DWARF-5 example

683b365

add dwarf-5 test

8b681f8

ggreif force-pushed the master branch from ab552d2 to 22b3944 Compare April 20, 2020 11:11

fix encoding of DW_OP_WASM_location

7d925db

ggreif force-pushed the master branch 2 times, most recently from 7d925db to b26be14 Compare April 20, 2020 21:10

ggreif added 9 commits April 22, 2020 13:22

support a few DWARF-5 only features

9540084

* skip DebugAddressBase attribute when transforming * read .debug_addr * read .debug_rnglists

cargo fmt --all

7067dff

read .debug_loclists when present

281948e

tmp: enforce addr base to be zero

d328903

fixup: expect 8 as DebugAddrBase

33c699a

need to skip the header

add an early DWARF-5 example

ca5a79b

add dwarf-5 test

51c9ea4

fix encoding of DW_OP_WASM_location

186f697

handle DebugAddrIndex in two places

67c83f7

ggreif force-pushed the master branch from 56682c9 to 67c83f7 Compare April 22, 2020 11:23

ggreif and others added 3 commits April 22, 2020 12:45

Merge branch 'master' of github.com:ggreif/wasmtime

6d12636

detabify

fa85b61

Merge branch 'master' of github.com:bytecodealliance/wasmtime

efc7aaa

ggreif added 2 commits April 24, 2020 00:26

cleanups, fmt

8fa26c8

support .debug_line_str and .debug_str_offsets

b01999e

in order to support these, one has to pass around the references to their sections too, and to the `unit`, because the str_offsets_base lives there

ggreif commented Apr 24, 2020

View reviewed changes

crates/debug/src/transform/attr.rs Outdated Show resolved Hide resolved

ggreif added 4 commits April 24, 2020 19:42

accomodate for the fact that DWARF-5 uses zero-based file indexing

d117d74

in the line program now these tests pass: test debug::translate::test_debug_dwarf_translate ... ok test debug::translate::test_debug_dwarf5_translate ... ok

cargo fmt

c65ea34

don't bail for inconsistency

2e0b30a

drop redundant imports

6e465cf

ggreif requested a review from yurydelendik April 25, 2020 15:24

remove local tests

cfa0ea9

Update lldb wasm test

b82d7d8

clang version 11.0.0 (https://github.com/llvm/llvm-project.git 1090a830692a863ccb091e3fad8cc1a287417493)

yurydelendik force-pushed the master branch from dc6994d to b82d7d8 Compare April 27, 2020 17:45

yurydelendik approved these changes Apr 27, 2020

View reviewed changes

crates/debug/src/transform/attr.rs Outdated Show resolved Hide resolved

crates/debug/src/transform/unit.rs Outdated Show resolved Hide resolved

ggreif added 2 commits April 27, 2020 21:39

cleanups

5942490

review feedback: don't cross-check index bases

cargo fmt

903fcb4

restore CU FIXME comment

589b8c2

yurydelendik merged commit 1639ed0 into bytecodealliance:master Apr 27, 2020

ggreif mentioned this pull request Apr 28, 2020

DWARF-5 support #932

Closed

1 task

support a few DWARF-5 only features #1410

support a few DWARF-5 only features #1410

Uh oh!

Conversation

ggreif commented Mar 26, 2020 • edited by yurydelendik Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ggreif commented Mar 26, 2020

Uh oh!

bjorn3 commented Mar 26, 2020

Uh oh!

Uh oh!

ggreif commented Apr 24, 2020

Uh oh!

yurydelendik commented Apr 24, 2020

Uh oh!

ggreif commented Apr 24, 2020 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ggreif commented Apr 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ggreif commented Apr 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurydelendik commented Apr 27, 2020

Uh oh!

ggreif commented Apr 27, 2020

Uh oh!

yurydelendik commented Apr 27, 2020

Uh oh!

yurydelendik commented Apr 27, 2020

Uh oh!

ggreif commented Apr 27, 2020

Uh oh!

yurydelendik commented Apr 27, 2020

Uh oh!

yurydelendik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ggreif commented Apr 27, 2020

Uh oh!

yurydelendik commented Apr 27, 2020

Uh oh!

Uh oh!

ggreif commented Mar 26, 2020 •

edited by yurydelendik

Loading

ggreif commented Apr 24, 2020 via email •

edited

Loading

ggreif commented Apr 24, 2020 •

edited

Loading

ggreif commented Apr 27, 2020 •

edited

Loading