Fix value label ranges resolution (#1572)

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

*    Implements FIXME in expression.rs
*    Move TargetIsa from CompiledExpression structure
*    Fix expression format for GDB
*    Add tests for parsing
*    Proper logic in ValueLabelRangesBuilder::process_label
*    Tests for ValueLabelRangesBuilder
*    Refactor build_with_locals to return Iterator instead of Vec<_>
*    Misc comments and magical numbers

cranelift/codegen/src/value_label.rs

@@ -18,9 +18,9 @@ use serde::{Deserialize, Serialize};
 pub struct ValueLocRange {
     /// The ValueLoc containing a ValueLabel during this range.
     pub loc: ValueLoc,
-    /// The start of the range.
+    /// The start of the range. It is an offset in the generated code.
     pub start: u32,
-    /// The end of the range.
+    /// The end of the range. It is an offset in the generated code.
     pub end: u32,
 }
 

crates/debug/src/transform/address_transform.rs

@@ -597,20 +597,6 @@ impl AddressTransform {
         let map = &self.map[index];
         (map.wasm_start, map.wasm_end)
     }
-
-    pub fn convert_to_code_range(
-        &self,
-        addr: write::Address,
-        len: u64,
-    ) -> (GeneratedAddress, GeneratedAddress) {
-        let start = if let write::Address::Symbol { addend, .. } = addr {
-            // TODO subtract self.map[symbol].offset ?
-            addend as GeneratedAddress
-        } else {
-            unreachable!();
-        };
-        (start, start + len as GeneratedAddress)
-    }
 }
 
 #[cfg(test)]

crates/debug/src/transform/attr.rs

@@ -13,11 +13,11 @@ use wasmtime_environ::isa::TargetIsa;
 #[derive(Debug)]
 pub(crate) enum FileAttributeContext<'a> {
     Root(Option<DebugLineOffset>),
-    Children(
+    Children {
-        &'a Vec<write::FileId>,
+        file_map: &'a [write::FileId],
-        u64,
+        file_index_base: u64,
-        Option<&'a CompiledExpression<'a>>,
+        frame_base: Option<&'a CompiledExpression>,
-    ),
+    },
 }
 
 fn is_exprloc_to_loclist_allowed(attr_name: gimli::constants::DwAt) -> bool {
@@ -55,8 +55,6 @@ pub(crate) fn clone_die_attributes<'a, R>(
 where
     R: Reader,
 {
-    let _tag = &entry.tag();
-    let endian = gimli::RunTimeEndian::Little;
     let unit_encoding = unit.encoding();
 
     let range_info = if let Some(subprogram_range_builder) = subprogram_range_builder {
@@ -116,7 +114,12 @@ where
                 }
             }
             AttributeValue::FileIndex(i) => {
-                if let FileAttributeContext::Children(file_map, file_index_base, _) = file_context {
+                if let FileAttributeContext::Children {
+                    file_map,
+                    file_index_base,
+                    ..
+                } = file_context
+                {
                     write::AttributeValue::FileIndex(Some(file_map[(i - file_index_base) as usize]))
                 } else {
                     return Err(TransformError("unexpected file index attribute").into());
@@ -150,34 +153,41 @@ where
                     unit.addr_base,
                 )?;
                 let frame_base =
-                    if let FileAttributeContext::Children(_, _, frame_base) = file_context {
+                    if let FileAttributeContext::Children { frame_base, .. } = file_context {
                         frame_base
                     } else {
                         None
                     };
-                let mut result = None;
+
+                let mut result: Option<Vec<_>> = None;
                 while let Some(loc) = locs.next()? {
-                    if let Some(expr) =
+                    if let Some(expr) = compile_expression(&loc.data, unit_encoding, frame_base)? {
-                        compile_expression(&loc.data, unit_encoding, frame_base, isa)?
+                        let chunk = expr
-                    {
+                            .build_with_locals(
-                        if result.is_none() {
-                            result = Some(Vec::new());
-                        }
-                        for (start, len, expr) in expr.build_with_locals(
                                 &[(loc.range.begin, loc.range.end)],
                                 addr_tr,
                                 frame_info,
-                            endian,
+                                isa,
-                        )? {
+                            )
-                            if len == 0 {
+                            .filter(|i| {
                                 // Ignore empty range
-                                continue;
+                                if let Ok((_, 0, _)) = i {
+                                    false
+                                } else {
+                                    true
                                 }
-                            result.as_mut().unwrap().push(write::Location::StartLength {
+                            })
+                            .map(|i| {
+                                i.map(|(start, len, expr)| write::Location::StartLength {
                                     begin: start,
                                     length: len,
                                     data: expr,
-                            });
+                                })
+                            })
+                            .collect::<Result<Vec<_>, _>>()?;
+                        match &mut result {
+                            Some(r) => r.extend(chunk),
+                            x @ None => *x = Some(chunk),
                         }
                     } else {
                         // FIXME _expr contains invalid expression
@@ -192,12 +202,12 @@ where
             }
             AttributeValue::Exprloc(ref expr) => {
                 let frame_base =
-                    if let FileAttributeContext::Children(_, _, frame_base) = file_context {
+                    if let FileAttributeContext::Children { frame_base, .. } = file_context {
                         frame_base
                     } else {
                         None
                     };
-                if let Some(expr) = compile_expression(expr, unit_encoding, frame_base, isa)? {
+                if let Some(expr) = compile_expression(expr, unit_encoding, frame_base)? {
                     if expr.is_simple() {
                         if let Some(expr) = expr.build() {
                             write::AttributeValue::Exprloc(expr)
@@ -207,8 +217,9 @@ where
                     } else {
                         // Conversion to loclist is required.
                         if let Some(scope_ranges) = scope_ranges {
-                            let exprs =
+                            let exprs = expr
-                                expr.build_with_locals(scope_ranges, addr_tr, frame_info, endian)?;
+                                .build_with_locals(scope_ranges, addr_tr, frame_info, isa)
+                                .collect::<Result<Vec<_>, _>>()?;
                             if exprs.is_empty() {
                                 continue;
                             }

crates/debug/src/transform/expression.rs

@@ -29,76 +29,121 @@ impl<'a> FunctionFrameInfo<'a> {
     }
 }
 
-#[derive(Debug)]
+struct ExpressionWriter(write::EndianVec<gimli::RunTimeEndian>);
+
+impl ExpressionWriter {
+    pub fn new() -> Self {
+        let endian = gimli::RunTimeEndian::Little;
+        let writer = write::EndianVec::new(endian);
+        ExpressionWriter(writer)
+    }
+
+    pub fn write_op(&mut self, op: gimli::DwOp) -> write::Result<()> {
+        self.write_u8(op.0 as u8)
+    }
+
+    pub fn write_op_reg(&mut self, reg: u16) -> write::Result<()> {
+        if reg < 32 {
+            self.write_u8(gimli::constants::DW_OP_reg0.0 as u8 + reg as u8)
+        } else {
+            self.write_op(gimli::constants::DW_OP_regx)?;
+            self.write_uleb128(reg.into())
+        }
+    }
+
+    pub fn write_op_breg(&mut self, reg: u16) -> write::Result<()> {
+        if reg < 32 {
+            self.write_u8(gimli::constants::DW_OP_breg0.0 as u8 + reg as u8)
+        } else {
+            self.write_op(gimli::constants::DW_OP_bregx)?;
+            self.write_uleb128(reg.into())
+        }
+    }
+
+    pub fn write_u8(&mut self, b: u8) -> write::Result<()> {
+        write::Writer::write_u8(&mut self.0, b)
+    }
+
+    pub fn write_uleb128(&mut self, i: u64) -> write::Result<()> {
+        write::Writer::write_uleb128(&mut self.0, i)
+    }
+
+    pub fn write_sleb128(&mut self, i: i64) -> write::Result<()> {
+        write::Writer::write_sleb128(&mut self.0, i)
+    }
+
+    pub fn into_vec(self) -> Vec<u8> {
+        self.0.into_vec()
+    }
+}
+
+#[derive(Debug, Clone, PartialEq)]
 enum CompiledExpressionPart {
+    // Untranslated DWARF expression.
     Code(Vec<u8>),
-    Local(ValueLabel),
+    // The wasm-local DWARF operator. The label points to `ValueLabel`.
+    // The trailing field denotes that the operator was last in sequence,
+    // and it is the DWARF location (not a pointer).
+    Local { label: ValueLabel, trailing: bool },
+    // Dereference is needed.
     Deref,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone, PartialEq)]
-pub struct CompiledExpression<'a> {
+pub struct CompiledExpression {
     parts: Vec<CompiledExpressionPart>,
     need_deref: bool,
-    isa: &'a dyn TargetIsa,
 }
 
-impl Clone for CompiledExpressionPart {
+impl CompiledExpression {
-    fn clone(&self) -> Self {
+    pub fn vmctx() -> CompiledExpression {
-        match self {
+        CompiledExpression::from_label(get_vmctx_value_label())
-            CompiledExpressionPart::Code(c) => CompiledExpressionPart::Code(c.clone()),
-            CompiledExpressionPart::Local(i) => CompiledExpressionPart::Local(*i),
-            CompiledExpressionPart::Deref => CompiledExpressionPart::Deref,
-        }
-    }
     }
 
-impl<'a> CompiledExpression<'a> {
+    pub fn from_label(label: ValueLabel) -> CompiledExpression {
-    pub fn vmctx(isa: &'a dyn TargetIsa) -> CompiledExpression {
-        CompiledExpression::from_label(get_vmctx_value_label(), isa)
-    }
-
-    pub fn from_label(label: ValueLabel, isa: &'a dyn TargetIsa) -> CompiledExpression<'a> {
         CompiledExpression {
-            parts: vec![
+            parts: vec![CompiledExpressionPart::Local {
-                CompiledExpressionPart::Local(label),
+                label,
-                CompiledExpressionPart::Code(vec![gimli::constants::DW_OP_stack_value.0 as u8]),
+                trailing: true,
-            ],
+            }],
             need_deref: false,
-            isa,
         }
     }
 }
 
+const X86_64_STACK_OFFSET: i64 = 16;
+
 fn translate_loc(
     loc: ValueLoc,
     frame_info: Option<&FunctionFrameInfo>,
     isa: &dyn TargetIsa,
+    add_stack_value: bool,
 ) -> Result<Option<Vec<u8>>> {
-    use gimli::write::Writer;
     Ok(match loc {
+        ValueLoc::Reg(reg) if add_stack_value => {
+            let machine_reg = isa.map_dwarf_register(reg)?;
+            let mut writer = ExpressionWriter::new();
+            writer.write_op_reg(machine_reg)?;
+            Some(writer.into_vec())
+        }
         ValueLoc::Reg(reg) => {
-            let machine_reg = isa.map_dwarf_register(reg)? as u8;
+            assert!(!add_stack_value);
-            Some(if machine_reg < 32 {
+            let machine_reg = isa.map_dwarf_register(reg)?;
-                vec![gimli::constants::DW_OP_reg0.0 + machine_reg]
+            let mut writer = ExpressionWriter::new();
-            } else {
+            writer.write_op_breg(machine_reg)?;
-                let endian = gimli::RunTimeEndian::Little;
+            writer.write_sleb128(0)?;
-                let mut writer = write::EndianVec::new(endian);
+            Some(writer.into_vec())
-                writer.write_u8(gimli::constants::DW_OP_regx.0 as u8)?;
-                writer.write_uleb128(machine_reg.into())?;
-                writer.into_vec()
-            })
         }
         ValueLoc::Stack(ss) => {
             if let Some(frame_info) = frame_info {
                 if let Some(ss_offset) = frame_info.stack_slots[ss].offset {
-                    let endian = gimli::RunTimeEndian::Little;
+                    let mut writer = ExpressionWriter::new();
-                    let mut writer = write::EndianVec::new(endian);
+                    writer.write_op_breg(X86_64::RBP.0)?;
-                    writer.write_u8(gimli::constants::DW_OP_breg0.0 + X86_64::RBP.0 as u8)?;
+                    writer.write_sleb128(ss_offset as i64 + X86_64_STACK_OFFSET)?;
-                    writer.write_sleb128(ss_offset as i64 + 16)?;
+                    if !add_stack_value {
-                    writer.write_u8(gimli::constants::DW_OP_deref.0 as u8)?;
+                        writer.write_op(gimli::constants::DW_OP_deref)?;
-                    let buf = writer.into_vec();
+                    }
-                    return Ok(Some(buf));
+                    return Ok(Some(writer.into_vec()));
                 }
             }
             None
@@ -111,11 +156,9 @@ fn append_memory_deref(
     buf: &mut Vec<u8>,
     frame_info: &FunctionFrameInfo,
     vmctx_loc: ValueLoc,
-    endian: gimli::RunTimeEndian,
     isa: &dyn TargetIsa,
 ) -> Result<bool> {
-    use gimli::write::Writer;
+    let mut writer = ExpressionWriter::new();
-    let mut writer = write::EndianVec::new(endian);
     // FIXME for imported memory
     match vmctx_loc {
         ValueLoc::Reg(vmctx_reg) => {
@@ -131,10 +174,10 @@ fn append_memory_deref(
         }
         ValueLoc::Stack(ss) => {
             if let Some(ss_offset) = frame_info.stack_slots[ss].offset {
-                writer.write_u8(gimli::constants::DW_OP_breg0.0 + X86_64::RBP.0 as u8)?;
+                writer.write_op_breg(X86_64::RBP.0)?;
-                writer.write_sleb128(ss_offset as i64 + 16)?;
+                writer.write_sleb128(ss_offset as i64 + X86_64_STACK_OFFSET)?;
-                writer.write_u8(gimli::constants::DW_OP_deref.0 as u8)?;
+                writer.write_op(gimli::constants::DW_OP_deref)?;
-                writer.write_u8(gimli::constants::DW_OP_consts.0 as u8)?;
+                writer.write_op(gimli::constants::DW_OP_consts)?;
                 let memory_offset = match frame_info.vmctx_memory_offset() {
                     Some(offset) => offset,
                     None => {
@@ -142,7 +185,7 @@ fn append_memory_deref(
                     }
                 };
                 writer.write_sleb128(memory_offset)?;
-                writer.write_u8(gimli::constants::DW_OP_plus.0 as u8)?;
+                writer.write_op(gimli::constants::DW_OP_plus)?;
             } else {
                 return Ok(false);
             }
@@ -151,18 +194,17 @@ fn append_memory_deref(
             return Ok(false);
         }
     }
-    writer.write_u8(gimli::constants::DW_OP_deref.0 as u8)?;
+    writer.write_op(gimli::constants::DW_OP_deref)?;
-    writer.write_u8(gimli::constants::DW_OP_swap.0 as u8)?;
+    writer.write_op(gimli::constants::DW_OP_swap)?;
-    writer.write_u8(gimli::constants::DW_OP_stack_value.0 as u8)?;
+    writer.write_op(gimli::constants::DW_OP_constu)?;
-    writer.write_u8(gimli::constants::DW_OP_constu.0 as u8)?;
     writer.write_uleb128(0xffff_ffff)?;
-    writer.write_u8(gimli::constants::DW_OP_and.0 as u8)?;
+    writer.write_op(gimli::constants::DW_OP_and)?;
-    writer.write_u8(gimli::constants::DW_OP_plus.0 as u8)?;
+    writer.write_op(gimli::constants::DW_OP_plus)?;
-    buf.extend_from_slice(writer.slice());
+    buf.extend(writer.into_vec());
     Ok(true)
 }
 
-impl<'a> CompiledExpression<'a> {
+impl CompiledExpression {
     pub fn is_simple(&self) -> bool {
         if let [CompiledExpressionPart::Code(_)] = self.parts.as_slice() {
             true
@@ -179,25 +221,60 @@ impl<'a> CompiledExpression<'a> {
         None
     }
 
-    pub fn build_with_locals(
+    pub fn build_with_locals<'a>(
-        &self,
+        &'a self,
-        scope: &[(u64, u64)], // wasm ranges
+        scope: &'a [(u64, u64)], // wasm ranges
-        addr_tr: &AddressTransform,
+        addr_tr: &'a AddressTransform,
-        frame_info: Option<&FunctionFrameInfo>,
+        frame_info: Option<&'a FunctionFrameInfo>,
-        endian: gimli::RunTimeEndian,
+        isa: &'a dyn TargetIsa,
-    ) -> Result<Vec<(write::Address, u64, write::Expression)>> {
+    ) -> impl Iterator<Item = Result<(write::Address, u64, write::Expression)>> + 'a {
-        if scope.is_empty() {
+        enum BuildWithLocalsResult<'a> {
-            return Ok(vec![]);
+            Empty,
+            Simple(
+                Box<dyn Iterator<Item = (write::Address, u64)> + 'a>,
+                Vec<u8>,
+            ),
+            Ranges(
+                Box<dyn Iterator<Item = Result<(DefinedFuncIndex, usize, usize, Vec<u8>)>> + 'a>,
+            ),
+        }
+        impl Iterator for BuildWithLocalsResult<'_> {
+            type Item = Result<(write::Address, u64, write::Expression)>;
+            fn next(&mut self) -> Option<Self::Item> {
+                match self {
+                    BuildWithLocalsResult::Empty => None,
+                    BuildWithLocalsResult::Simple(it, code) => it
+                        .next()
+                        .map(|(addr, len)| Ok((addr, len, write::Expression(code.to_vec())))),
+                    BuildWithLocalsResult::Ranges(it) => it.next().map(|r| {
+                        r.map(|(func_index, start, end, code_buf)| {
+                            (
+                                write::Address::Symbol {
+                                    symbol: func_index.index(),
+                                    addend: start as i64,
+                                },
+                                (end - start) as u64,
+                                write::Expression(code_buf),
+                            )
+                        })
+                    }),
+                }
+            }
         }
 
+        if scope.is_empty() {
+            return BuildWithLocalsResult::Empty;
+        }
+
+        // If it a simple DWARF code, no need in locals processing. Just translate
+        // the scope ranges.
         if let [CompiledExpressionPart::Code(code)] = self.parts.as_slice() {
-            let mut result_scope = Vec::new();
+            return BuildWithLocalsResult::Simple(
-            for s in scope {
+                Box::new(scope.iter().flat_map(move |(wasm_start, wasm_end)| {
-                for (addr, len) in addr_tr.translate_ranges(s.0, s.1) {
+                    addr_tr.translate_ranges(*wasm_start, *wasm_end)
-                    result_scope.push((addr, len, write::Expression(code.to_vec())));
+                })),
-                }
+                code.clone(),
-            }
+            );
-            return Ok(result_scope);
         }
 
         let vmctx_label = get_vmctx_value_label();
@@ -205,41 +282,32 @@ impl<'a> CompiledExpression<'a> {
         // Some locals are present, preparing and divided ranges based on the scope
         // and frame_info data.
         let mut ranges_builder = ValueLabelRangesBuilder::new(scope, addr_tr, frame_info);
-        for p in &self.parts {
+        for p in self.parts.iter() {
             match p {
                 CompiledExpressionPart::Code(_) => (),
-                CompiledExpressionPart::Local(label) => ranges_builder.process_label(*label),
+                CompiledExpressionPart::Local { label, .. } => ranges_builder.process_label(*label),
                 CompiledExpressionPart::Deref => ranges_builder.process_label(vmctx_label),
             }
         }
         if self.need_deref {
             ranges_builder.process_label(vmctx_label);
         }
-        ranges_builder.remove_incomplete_ranges();
+        let ranges = ranges_builder.into_ranges();
-        let ranges = ranges_builder.ranges;
 
-        let mut result = Vec::new();
+        return BuildWithLocalsResult::Ranges(Box::new(
-        'range: for CachedValueLabelRange {
+            ranges
+                .into_iter()
+                .map(
+                    move |CachedValueLabelRange {
                               func_index,
                               start,
                               end,
                               label_location,
-        } in ranges
+                          }| {
-        {
                         // build expression
                         let mut code_buf = Vec::new();
-            for part in &self.parts {
+                        macro_rules! deref {
-                match part {
+                            () => {
-                    CompiledExpressionPart::Code(c) => code_buf.extend_from_slice(c.as_slice()),
-                    CompiledExpressionPart::Local(label) => {
-                        let loc = *label_location.get(&label).context("label_location")?;
-                        if let Some(expr) = translate_loc(loc, frame_info, self.isa)? {
-                            code_buf.extend_from_slice(&expr)
-                        } else {
-                            continue 'range;
-                        }
-                    }
-                    CompiledExpressionPart::Deref => {
                                 if let (Some(vmctx_loc), Some(frame_info)) =
                                     (label_location.get(&vmctx_label), frame_info)
                                 {
@@ -247,46 +315,43 @@ impl<'a> CompiledExpression<'a> {
                                         &mut code_buf,
                                         frame_info,
                                         *vmctx_loc,
-                                endian,
+                                        isa,
-                                self.isa,
                                     )? {
-                                continue 'range;
+                                        return Ok(None);
                                     }
                                 } else {
-                            continue 'range;
+                                    return Ok(None);
+                                };
                             };
                         }
+                        for part in &self.parts {
+                            match part {
+                                CompiledExpressionPart::Code(c) => {
+                                    code_buf.extend_from_slice(c.as_slice())
+                                }
+                                CompiledExpressionPart::Local { label, trailing } => {
+                                    let loc =
+                                        *label_location.get(&label).context("label_location")?;
+                                    if let Some(expr) =
+                                        translate_loc(loc, frame_info, isa, *trailing)?
+                                    {
+                                        code_buf.extend_from_slice(&expr)
+                                    } else {
+                                        return Ok(None);
+                                    }
+                                }
+                                CompiledExpressionPart::Deref => deref!(),
                             }
                         }
                         if self.need_deref {
-                if let (Some(vmctx_loc), Some(frame_info)) =
+                            deref!();
-                    (label_location.get(&vmctx_label), frame_info)
-                {
-                    if !append_memory_deref(
-                        &mut code_buf,
-                        frame_info,
-                        *vmctx_loc,
-                        endian,
-                        self.isa,
-                    )? {
-                        continue 'range;
                         }
-                } else {
+                        Ok(Some((func_index, start, end, code_buf)))
-                    continue 'range;
-                };
-            }
-            result.push((
-                write::Address::Symbol {
-                    symbol: func_index.index(),
-                    addend: start as i64,
                     },
-                (end - start) as u64,
+                )
-                write::Expression(code_buf),
+                .filter_map(Result::transpose),
         ));
     }
-
-        Ok(result)
-    }
 }
 
 fn is_old_expression_format(buf: &[u8]) -> bool {
@@ -299,12 +364,11 @@ fn is_old_expression_format(buf: &[u8]) -> bool {
     buf.contains(&(gimli::constants::DW_OP_plus_uconst.0 as u8))
 }
 
-pub fn compile_expression<'a, R>(
+pub fn compile_expression<R>(
     expr: &Expression<R>,
     encoding: gimli::Encoding,
     frame_base: Option<&CompiledExpression>,
-    isa: &'a dyn TargetIsa,
+) -> Result<Option<CompiledExpression>, Error>
-) -> Result<Option<CompiledExpression<'a>>, Error>
 where
     R: Reader,
 {
@@ -315,10 +379,21 @@ where
     if is_old_expression_format(&buf) && frame_base.is_some() {
         // Still supporting old DWARF variable expressions without fbreg.
         parts.extend_from_slice(&frame_base.unwrap().parts);
+        if let Some(CompiledExpressionPart::Local { trailing, .. }) = parts.last_mut() {
+            *trailing = false;
+        }
         need_deref = frame_base.unwrap().need_deref;
     }
     let base_len = parts.len();
     let mut code_chunk = Vec::new();
+    macro_rules! flush_code_chunk {
+        () => {
+            if !code_chunk.is_empty() {
+                parts.push(CompiledExpressionPart::Code(code_chunk));
+                code_chunk = Vec::new();
+            }
+        };
+    };
     while !pc.is_empty() {
         let next = buf[pc.offset_from(&expr.0).into_u64() as usize];
         need_deref = true;
@@ -331,54 +406,52 @@ where
                 // TODO support wasm globals?
                 return Ok(None);
             }
-            let index = pc.read_uleb128()?;
+            let index = pc.read_sleb128()?;
-            if pc.read_u8()? != 159 {
+            flush_code_chunk!();
-                // FIXME The following operator is not DW_OP_stack_value, e.g. :
-                // DW_AT_location  (0x00000ea5:
-                //   [0x00001e19, 0x00001e26): DW_OP_WASM_location 0x0 +1, DW_OP_plus_uconst 0x10, DW_OP_stack_value
-                //   [0x00001e5a, 0x00001e72): DW_OP_WASM_location 0x0 +20, DW_OP_stack_value
-                // )
-                return Ok(None);
-            }
-            if !code_chunk.is_empty() {
-                parts.push(CompiledExpressionPart::Code(code_chunk));
-                code_chunk = Vec::new();
-            }
             let label = ValueLabel::from_u32(index as u32);
-            parts.push(CompiledExpressionPart::Local(label));
+            parts.push(CompiledExpressionPart::Local {
+                label,
+                trailing: false,
+            });
         } else {
             let pos = pc.offset_from(&expr.0).into_u64() as usize;
             let op = Operation::parse(&mut pc, &expr.0, encoding)?;
             match op {
                 Operation::FrameOffset { offset } => {
                     // Expand DW_OP_fpreg into frame location and DW_OP_plus_uconst.
-                    use gimli::write::Writer;
                     if frame_base.is_some() {
                         // Add frame base expressions.
-                        if !code_chunk.is_empty() {
+                        flush_code_chunk!();
-                            parts.push(CompiledExpressionPart::Code(code_chunk));
-                            code_chunk = Vec::new();
-                        }
                         parts.extend_from_slice(&frame_base.unwrap().parts);
-                        need_deref = frame_base.unwrap().need_deref;
+                    }
+                    if let Some(CompiledExpressionPart::Local { trailing, .. }) = parts.last_mut() {
+                        // Reset local trailing flag.
+                        *trailing = false;
                     }
                     // Append DW_OP_plus_uconst part.
-                    let endian = gimli::RunTimeEndian::Little;
+                    let mut writer = ExpressionWriter::new();
-                    let mut writer = write::EndianVec::new(endian);
+                    writer.write_op(gimli::constants::DW_OP_plus_uconst)?;
-                    writer.write_u8(gimli::constants::DW_OP_plus_uconst.0 as u8)?;
                     writer.write_uleb128(offset as u64)?;
                     code_chunk.extend(writer.into_vec());
                     continue;
                 }
-                Operation::Literal { .. } | Operation::PlusConstant { .. } => (),
+                Operation::Literal { .. }
+                | Operation::PlusConstant { .. }
+                | Operation::Piece { .. } => (),
                 Operation::StackValue => {
                     need_deref = false;
+
+                    // Find extra stack_value, that follow wasm-local operators,
+                    // and mark such locals with special flag.
+                    if let (Some(CompiledExpressionPart::Local { trailing, .. }), true) =
+                        (parts.last_mut(), code_chunk.is_empty())
+                    {
+                        *trailing = true;
+                        continue;
+                    }
                 }
                 Operation::Deref { .. } => {
-                    if !code_chunk.is_empty() {
+                    flush_code_chunk!();
-                        parts.push(CompiledExpressionPart::Code(code_chunk));
-                        code_chunk = Vec::new();
-                    }
                     parts.push(CompiledExpressionPart::Deref);
                 }
                 _ => {
@@ -406,11 +479,7 @@ where
         }
     }
 
-    Ok(Some(CompiledExpression {
+    Ok(Some(CompiledExpression { parts, need_deref }))
-        parts,
-        need_deref,
-        isa,
-    }))
 }
 
 #[derive(Debug, Clone)]
@@ -423,34 +492,30 @@ struct CachedValueLabelRange {
 
 struct ValueLabelRangesBuilder<'a, 'b> {
     ranges: Vec<CachedValueLabelRange>,
-    addr_tr: &'a AddressTransform,
     frame_info: Option<&'a FunctionFrameInfo<'b>>,
     processed_labels: HashSet<ValueLabel>,
 }
 
 impl<'a, 'b> ValueLabelRangesBuilder<'a, 'b> {
-    fn new(
+    pub fn new(
         scope: &[(u64, u64)], // wasm ranges
         addr_tr: &'a AddressTransform,
         frame_info: Option<&'a FunctionFrameInfo<'b>>,
     ) -> Self {
         let mut ranges = Vec::new();
-        for s in scope {
+        for (wasm_start, wasm_end) in scope {
-            if let Some((func_index, tr)) = addr_tr.translate_ranges_raw(s.0, s.1) {
+            if let Some((func_index, tr)) = addr_tr.translate_ranges_raw(*wasm_start, *wasm_end) {
-                for (start, end) in tr {
+                ranges.extend(tr.into_iter().map(|(start, end)| CachedValueLabelRange {
-                    ranges.push(CachedValueLabelRange {
                     func_index,
                     start,
                     end,
                     label_location: HashMap::new(),
-                    })
+                }));
-                }
             }
         }
         ranges.sort_unstable_by(|a, b| a.start.cmp(&b.start));
         ValueLabelRangesBuilder {
             ranges,
-            addr_tr,
             frame_info,
             processed_labels: HashSet::new(),
         }
@@ -462,28 +527,23 @@ impl<'a, 'b> ValueLabelRangesBuilder<'a, 'b> {
         }
         self.processed_labels.insert(label);
 
-        let value_ranges = if let Some(frame_info) = self.frame_info {
+        let value_ranges = match self.frame_info.and_then(|fi| fi.value_ranges.get(&label)) {
-            &frame_info.value_ranges
+            Some(value_ranges) => value_ranges,
-        } else {
+            None => {
                 return;
+            }
         };
 
         let ranges = &mut self.ranges;
-        if let Some(local_ranges) = value_ranges.get(&label) {
+        for value_range in value_ranges {
-            for local_range in local_ranges {
+            let range_start = value_range.start as usize;
-                let wasm_start = local_range.start;
+            let range_end = value_range.end as usize;
-                let wasm_end = local_range.end;
+            let loc = value_range.loc;
-                let loc = local_range.loc;
-                // Find all native ranges for the value label ranges.
-                for (addr, len) in self
-                    .addr_tr
-                    .translate_ranges(wasm_start as u64, wasm_end as u64)
-                {
-                    let (range_start, range_end) = self.addr_tr.convert_to_code_range(addr, len);
             if range_start == range_end {
                 continue;
             }
             assert_lt!(range_start, range_end);
+
             // Find acceptable scope of ranges to intersect with.
             let i = match ranges.binary_search_by(|s| s.start.cmp(&range_start)) {
                 Ok(i) => i,
@@ -498,7 +558,7 @@ impl<'a, 'b> ValueLabelRangesBuilder<'a, 'b> {
             let j = match ranges.binary_search_by(|s| s.start.cmp(&range_end)) {
                 Ok(i) | Err(i) => i,
             };
-                    // Starting for the end, intersect (range_start..range_end) with
+            // Starting from the end, intersect (range_start..range_end) with
             // self.ranges array.
             for i in (i..j).rev() {
                 if range_end <= ranges[i].start || ranges[i].end <= range_start {
@@ -525,13 +585,224 @@ impl<'a, 'b> ValueLabelRangesBuilder<'a, 'b> {
             }
         }
     }
-        }
-    }
 
-    fn remove_incomplete_ranges(&mut self) {
+    pub fn into_ranges(self) -> impl Iterator<Item = CachedValueLabelRange> {
         // Ranges with not-enough labels are discarded.
         let processed_labels_len = self.processed_labels.len();
         self.ranges
-            .retain(|r| r.label_location.len() == processed_labels_len);
+            .into_iter()
+            .filter(move |r| r.label_location.len() == processed_labels_len)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::compile_expression;
+    use super::{AddressTransform, FunctionFrameInfo, ValueLabel, ValueLabelsRanges};
+    use gimli::{self, Encoding, EndianSlice, Expression, RunTimeEndian};
+
+    macro_rules! expression {
+        ($($i:literal),*) => {
+            Expression(EndianSlice::new(
+                &[$($i),*],
+                RunTimeEndian::Little,
+            ))
+        }
+    }
+
+    static DWARF_ENCODING: Encoding = Encoding {
+        address_size: 4,
+        format: gimli::Format::Dwarf32,
+        version: 4,
+    };
+
+    #[test]
+    fn test_debug_parse_expressions() {
+        use super::{CompiledExpression, CompiledExpressionPart};
+        use wasmtime_environ::entity::EntityRef;
+
+        let (val1, val3, val20) = (ValueLabel::new(1), ValueLabel::new(3), ValueLabel::new(20));
+
+        // DW_OP_WASM_location 0x0 +20, DW_OP_stack_value
+        let e = expression!(0xed, 0x00, 0x14, 0x9f);
+        let ce = compile_expression(&e, DWARF_ENCODING, None)
+            .expect("non-error")
+            .expect("expression");
+        assert_eq!(
+            ce,
+            CompiledExpression {
+                parts: vec![CompiledExpressionPart::Local {
+                    label: val20,
+                    trailing: true
+                }],
+                need_deref: false
+            }
+        );
+
+        //  DW_OP_WASM_location 0x0 +1, DW_OP_plus_uconst 0x10, DW_OP_stack_value
+        let e = expression!(0xed, 0x00, 0x01, 0x23, 0x10, 0x9f);
+        let ce = compile_expression(&e, DWARF_ENCODING, None)
+            .expect("non-error")
+            .expect("expression");
+        assert_eq!(
+            ce,
+            CompiledExpression {
+                parts: vec![
+                    CompiledExpressionPart::Local {
+                        label: val1,
+                        trailing: false
+                    },
+                    CompiledExpressionPart::Code(vec![35, 16, 159])
+                ],
+                need_deref: false
+            }
+        );
+
+        // Frame base: DW_OP_WASM_location 0x0 +3, DW_OP_stack_value
+        let e = expression!(0xed, 0x00, 0x03, 0x9f);
+        let fe = compile_expression(&e, DWARF_ENCODING, None).expect("non-error");
+        // DW_OP_fpreg 0x12
+        let e = expression!(0x91, 0x12);
+        let ce = compile_expression(&e, DWARF_ENCODING, fe.as_ref())
+            .expect("non-error")
+            .expect("expression");
+        assert_eq!(
+            ce,
+            CompiledExpression {
+                parts: vec![
+                    CompiledExpressionPart::Local {
+                        label: val3,
+                        trailing: false
+                    },
+                    CompiledExpressionPart::Code(vec![35, 18])
+                ],
+                need_deref: true
+            }
+        );
+    }
+
+    fn create_mock_address_transform() -> AddressTransform {
+        use crate::read_debuginfo::WasmFileInfo;
+        use wasmtime_environ::entity::PrimaryMap;
+        use wasmtime_environ::ir::SourceLoc;
+        use wasmtime_environ::{FunctionAddressMap, InstructionAddressMap};
+        let mut module_map = PrimaryMap::new();
+        let code_section_offset: u32 = 100;
+        module_map.push(FunctionAddressMap {
+            instructions: vec![
+                InstructionAddressMap {
+                    srcloc: SourceLoc::new(code_section_offset + 12),
+                    code_offset: 5,
+                    code_len: 3,
+                },
+                InstructionAddressMap {
+                    srcloc: SourceLoc::new(code_section_offset + 17),
+                    code_offset: 15,
+                    code_len: 8,
+                },
+            ],
+            start_srcloc: SourceLoc::new(code_section_offset + 10),
+            end_srcloc: SourceLoc::new(code_section_offset + 20),
+            body_offset: 0,
+            body_len: 30,
+        });
+        let fi = WasmFileInfo {
+            code_section_offset: code_section_offset.into(),
+            funcs: Box::new([]),
+            imported_func_count: 0,
+            path: None,
+        };
+        AddressTransform::new(&module_map, &fi)
+    }
+
+    fn create_mock_value_ranges() -> (ValueLabelsRanges, (ValueLabel, ValueLabel, ValueLabel)) {
+        use std::collections::HashMap;
+        use wasmtime_environ::entity::EntityRef;
+        use wasmtime_environ::ir::{ValueLoc, ValueLocRange};
+        let mut value_ranges = HashMap::new();
+        let value_0 = ValueLabel::new(0);
+        let value_1 = ValueLabel::new(1);
+        let value_2 = ValueLabel::new(2);
+        value_ranges.insert(
+            value_0,
+            vec![ValueLocRange {
+                loc: ValueLoc::Unassigned,
+                start: 0,
+                end: 25,
+            }],
+        );
+        value_ranges.insert(
+            value_1,
+            vec![ValueLocRange {
+                loc: ValueLoc::Unassigned,
+                start: 5,
+                end: 30,
+            }],
+        );
+        value_ranges.insert(
+            value_2,
+            vec![
+                ValueLocRange {
+                    loc: ValueLoc::Unassigned,
+                    start: 0,
+                    end: 10,
+                },
+                ValueLocRange {
+                    loc: ValueLoc::Unassigned,
+                    start: 20,
+                    end: 30,
+                },
+            ],
+        );
+        (value_ranges, (value_0, value_1, value_2))
+    }
+
+    #[test]
+    fn test_debug_value_range_builder() {
+        use super::ValueLabelRangesBuilder;
+        use wasmtime_environ::entity::EntityRef;
+        use wasmtime_environ::ir::StackSlots;
+        use wasmtime_environ::wasm::DefinedFuncIndex;
+        use wasmtime_environ::ModuleMemoryOffset;
+
+        let addr_tr = create_mock_address_transform();
+        let stack_slots = StackSlots::new();
+        let (value_ranges, value_labels) = create_mock_value_ranges();
+        let fi = FunctionFrameInfo {
+            memory_offset: ModuleMemoryOffset::None,
+            stack_slots: &stack_slots,
+            value_ranges: &value_ranges,
+        };
+
+        // No value labels, testing if entire function range coming through.
+        let builder = ValueLabelRangesBuilder::new(&[(10, 20)], &addr_tr, Some(&fi));
+        let ranges = builder.into_ranges().collect::<Vec<_>>();
+        assert_eq!(ranges.len(), 1);
+        assert_eq!(ranges[0].func_index, DefinedFuncIndex::new(0));
+        assert_eq!(ranges[0].start, 0);
+        assert_eq!(ranges[0].end, 30);
+
+        // Two labels (val0@0..25 and val1@5..30), their common lifetime intersect at 5..25.
+        let mut builder = ValueLabelRangesBuilder::new(&[(10, 20)], &addr_tr, Some(&fi));
+        builder.process_label(value_labels.0);
+        builder.process_label(value_labels.1);
+        let ranges = builder.into_ranges().collect::<Vec<_>>();
+        assert_eq!(ranges.len(), 1);
+        assert_eq!(ranges[0].start, 5);
+        assert_eq!(ranges[0].end, 25);
+
+        // Adds val2 with complex lifetime @0..10 and @20..30 to the previous test, and
+        // also narrows range.
+        let mut builder = ValueLabelRangesBuilder::new(&[(11, 17)], &addr_tr, Some(&fi));
+        builder.process_label(value_labels.0);
+        builder.process_label(value_labels.1);
+        builder.process_label(value_labels.2);
+        let ranges = builder.into_ranges().collect::<Vec<_>>();
+        // Result is two ranges @5..10 and @20..23
+        assert_eq!(ranges.len(), 2);
+        assert_eq!(ranges[0].start, 5);
+        assert_eq!(ranges[0].end, 10);
+        assert_eq!(ranges[1].start, 20);
+        assert_eq!(ranges[1].end, 23);
     }
 }

crates/debug/src/transform/simulate.rs

@@ -228,19 +228,16 @@ fn generate_vars(
                 };
 
             let loc_list_id = {
-                let endian = gimli::RunTimeEndian::Little;
+                let locs = CompiledExpression::from_label(*label)
-
+                    .build_with_locals(scope_ranges, addr_tr, Some(frame_info), isa)
-                let expr = CompiledExpression::from_label(*label, isa);
+                    .map(|i| {
-                let mut locs = Vec::new();
+                        i.map(|(begin, length, data)| write::Location::StartLength {
-                for (begin, length, data) in
-                    expr.build_with_locals(scope_ranges, addr_tr, Some(frame_info), endian)?
-                {
-                    locs.push(write::Location::StartLength {
                             begin,
                             length,
                             data,
-                    });
+                        })
-                }
+                    })
+                    .collect::<Result<Vec<_>, _>>()?;
                 unit.locations.add(write::LocationList(locs))
             };
 

crates/debug/src/transform/unit.rs

@@ -386,7 +386,7 @@ where
         }
 
         if let Some(AttributeValue::Exprloc(expr)) = entry.attr_value(gimli::DW_AT_frame_base)? {
-            if let Some(expr) = compile_expression(&expr, unit.encoding(), None, isa)? {
+            if let Some(expr) = compile_expression(&expr, unit.encoding(), None)? {
                 current_frame_base.push(new_stack_len, expr);
             }
         }
@@ -432,7 +432,11 @@ where
             out_strings,
             &mut pending_die_refs,
             &mut pending_di_refs,
-            FileAttributeContext::Children(&file_map, file_index_base, current_frame_base.top()),
+            FileAttributeContext::Children {
+                file_map: &file_map,
+                file_index_base,
+                frame_base: current_frame_base.top(),
+            },
             isa,
         )?;
 

crates/debug/src/transform/utils.rs

@@ -134,19 +134,17 @@ pub(crate) fn append_vmctx_info(
     isa: &dyn TargetIsa,
 ) -> Result<(), Error> {
     let loc = {
-        let endian = gimli::RunTimeEndian::Little;
+        let expr = CompiledExpression::vmctx();
-
+        let locs = expr
-        let expr = CompiledExpression::vmctx(isa);
+            .build_with_locals(scope_ranges, addr_tr, frame_info, isa)
-        let mut locs = Vec::new();
+            .map(|i| {
-        for (begin, length, data) in
+                i.map(|(begin, length, data)| write::Location::StartLength {
-            expr.build_with_locals(scope_ranges, addr_tr, frame_info, endian)?
-        {
-            locs.push(write::Location::StartLength {
                     begin,
                     length,
                     data,
-            });
+                })
-        }
+            })
+            .collect::<Result<Vec<_>, _>>()?;
         let list_id = comp_unit.locations.add(write::LocationList(locs));
         write::AttributeValue::LocationListRef(list_id)
     };

crates/environ/src/data_structures.rs

@@ -5,7 +5,7 @@ pub mod ir {
         types, AbiParam, ArgumentPurpose, Signature, SourceLoc, StackSlots, TrapCode, Type,
         ValueLabel, ValueLoc,
     };
-    pub use cranelift_codegen::ValueLabelsRanges;
+    pub use cranelift_codegen::{ValueLabelsRanges, ValueLocRange};
 }
 
 pub mod settings {