Stream: git-wasmtime

Topic: wasmtime / PR #1572 Fix value label ranges resolution


view this post on Zulip Wasmtime GitHub notifications bot (Apr 21 2020 at 22:38):

yurydelendik opened PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 15:19):

yurydelendik updated PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 16:54):

yurydelendik updated PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:18):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:18):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:18):

ggreif created PR Review Comment:

                        .flat_map(move |(wasm_start, wasm_end)| {
                            addr_tr.translate_ranges(*wasm_start, *wasm_end)
                        })

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:18):

ggreif created PR Review Comment:

            // Starting from the end, intersect (range_start..range_end) with

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:18):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:22):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:22):

ggreif created PR Review Comment:

Courtesy of Haskell's concatMap :-)
Potentially a bit faster.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:33):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:35):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:35):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:35):

bjorn3 created PR Review Comment:

Why not &'a [write::FileId]? Saves an indirection.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 20:59):

yurydelendik updated PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 21:22):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 21:22):

ggreif created PR Review Comment:

        let value_ranges = self.frame_info
        .and_then(|fi| fi.value_ranges.get(&label))
        .unwrap_or({return});

This might be a little more clear. Matter of taste.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 28 2020 at 21:24):

ggreif edited PR Review Comment.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 01:28):

yurydelendik updated PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 14:43):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 14:43):

ggreif created PR Review Comment:

It is probably evil. Sorry for this :-/
The only reference a search turned up was https://www.reddit.com/r/rust/comments/86x1fj/unwrap_orreturn/
and I am now convinced, that an eagerly evaluated argument return when passed to unwrap_or will exit the function in any case. I will learn a bit more about this antipattern, but don't apply it :skull:!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 14:56):

yurydelendik edited PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 14:58):

ggreif deleted PR Review Comment.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 14:58):

ggreif deleted PR Review Comment.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 15:01):

yurydelendik edited PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 15:13):

yurydelendik updated PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 18:25):

fitzgen submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 18:25):

fitzgen submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 18:25):

fitzgen created PR Review Comment:

Tangent: Why aren't we doing #[derive(Clone)] for CompiledExpressionPart? I don't see anything here that is different from what the derive would do.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 18:25):

fitzgen created PR Review Comment:

nit: rm commented out code

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 18:25):

fitzgen created PR Review Comment:

Why asserting the debug formatting and not the value itself?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 18:25):

fitzgen created PR Review Comment:

Why not move this if machine_reg < 32 logic into GimliWriter::write_op_reg? That could remove a precondition assertion from that method and some duplication here and below.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 18:25):

fitzgen created PR Review Comment:

    // Dereference is needed.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 23:17):

yurydelendik submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 29 2020 at 23:17):

yurydelendik created PR Review Comment:

Not sure. But it is gone: we have [derive(Clone)] now.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2020 at 01:52):

yurydelendik updated PR #1572 from fix-value-label-resolve to master:

There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2020 at 08:33):

ggreif submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2020 at 13:07):

yurydelendik merged PR #1572.


Last updated: Dec 23 2024 at 12:05 UTC