[lldb] Fix "frame var" for large bitfields

[labath]

Summary

The problem here is in the "sliding" code in
ValueObjectChild::UpdateValue. It modifies m_bitfield_bit_offset and
m_value to ensure the bitfield value fits the window given by the
underlying type.

However, this is broken next time UpdateValue is called, because it
updates the m_value value from the parent. However, the value cannot be
slid again because the m_bitfield_bit_offset is already modified.

It seems this can happen only under specific circumstances. One way to
trigger is is to run an expression which can be interpreted (jitting it
causes a new StackFrame and ValueObject variables to be created).

I fix this bug by modifying m_byte_offset instead of m_scalar, and
ensuring the changes are folded into m_scalar regardless of how many
times UpdateValue is called.

[shafik]

LGTM I notice that we are using m_byte_offset directly a little above the line you fixed instead of GetByteOffset().

lldb/source/Core/ValueObjectChild.cpp

if (parent->UpdateValueIfNeeded(false)) {
         case Value::eValueTypeHostAddress: {
           lldb::addr_t addr =
               m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
           if (addr == LLDB_INVALID_ADDRESS) {
             m_error.SetErrorString("parent address is invalid.");
           } else if (addr == 0) {
             m_error.SetErrorString("parent is NULL");
           } else {
-            // Set this object's scalar value to the address of its value by
-            // adding its byte offset to the parent address
-            m_value.GetScalar() += GetByteOffset();
-
             // If a bitfield doesn't fit into the child_byte_size'd
             // window at child_byte_offset, move the window forward
             // until it fits.  The problem here is that Value has no
             // notion of bitfields and thus the Value's DataExtractor
             // is sized like the bitfields CompilerType; a sequence of
             // bitfields, however, can be larger than their underlying
             // type.
             if (m_bitfield_bit_offset) {
               const bool thread_and_frame_only_if_stopped = true;
               ExecutionContext exe_ctx(GetExecutionContextRef().Lock(
                   thread_and_frame_only_if_stopped));
               if (auto type_bit_size = GetCompilerType().GetBitSize(
                       exe_ctx.GetBestExecutionContextScope())) {
                 uint64_t bitfield_end =
                     m_bitfield_bit_size + m_bitfield_bit_offset;
                 if (bitfield_end > *type_bit_size) {
                   uint64_t overhang_bytes =
                       (bitfield_end - *type_bit_size + 7) / 8;
-                  m_value.GetScalar() += overhang_bytes;
+                  m_byte_offset += overhang_bytes;
                   m_bitfield_bit_offset -= overhang_bytes * 8;
                 }
               }
             }
+
+            // Set this object's scalar value to the address of its value by
+            // adding its byte offset to the parent address
+            m_value.GetScalar() += m_byte_offset;
           }
         } break;
 
         case Value::eValueTypeScalar:
           // try to extract the child value from the parent's scalar value
           {
             Scalar scalar(m_value.GetScalar());
             scalar.ExtractBitfield(8 * m_byte_size, 8 * m_byte_offset);

lldb/test/API/lang/c/bitfields/TestBitfields.py

def test_and_run_command(self):
                 '(uint8_t:1) b15 = \'\\0\'',
                 '(uint8_t:1) b16 = \'\\0\'',
                 '(uint8_t:1) b17 = \'\\0\'',
                 ])
 
         self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
                     substrs=["a = 0x0000000cbbbbaaaa", "b = 0x0000000dffffeee"])
 
+    # BitFields exhibit crashes in record layout on Windows
+    # (http://llvm.org/pr21800)
+    @skipIfWindows
+    def test_expression_bug(self):
+        # Ensure evaluating (emulating) an expression does not break bitfield
+        # values for already parsed variables. The expression is run twice
+        # because the very first expression can resume a target (to allocate
+        # memory, etc.) even if it is not being jitted.
+        self.build()
+        lldbutil.run_to_line_breakpoint(self, lldb.SBFileSpec("main.c"),
+                self.line)
+        self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=["a = 0x0000000cbbbbaaaa", "b = 0x0000000dffffeee"])
+        self.expect("expr --allow-jit false  -- more_bits.a", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['uint32_t', '3'])
+        self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=["a = 0x0000000cbbbbaaaa", "b = 0x0000000dffffeee"])
+        self.expect("expr --allow-jit false  -- more_bits.a", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['uint32_t', '3'])
+        self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=["a = 0x0000000cbbbbaaaa", "b = 0x0000000dffffeee"])
 
     @add_test_categories(['pyapi'])
     # BitFields exhibit crashes in record layout on Windows
     # (http://llvm.org/pr21800)
     @skipIfWindows
     def test_and_python_api(self):
         """Use Python APIs to inspect a bitfields variable."""
         self.build()