Commit fcaf5f6c authored by shafik's avatar shafik
Browse files

[LLDB] Fix the handling of unnamed bit-fields when parsing DWARF

We ran into an assert when debugging clang and performing an expression on a class derived from DeclContext. The assert was indicating we were getting the offsets wrong for RecordDeclBitfields. We were getting both the size and offset of unnamed bit-field members wrong. We could fix this case with a quick change but as I extended the test suite to include more combinations we kept finding more cases that were being handled incorrectly. A fix that handled all the new cases as well as the cases already covered required a refactor of the existing technique.

Differential Revision: https://reviews.llvm.org/D72953
parent 765b37ab
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
CXX_SOURCES := main.cpp

include Makefile.rules
+105 −0
Original line number Diff line number Diff line
"""Show bitfields and check that they display correctly."""

import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil


class CppBitfieldsTestCase(TestBase):

    mydir = TestBase.compute_mydir(__file__)

    def setUp(self):
        # Call super's setUp().
        TestBase.setUp(self)
        # Find the line number to break inside main().
        self.line = line_number('main.cpp', '// Set break point at this line.')

    # BitFields exhibit crashes in record layout on Windows
    # (http://llvm.org/pr21800)
    @skipIfWindows
    def test_and_run_command(self):
        """Test 'frame variable ...' on a variable with bitfields."""
        self.build()

        lldbutil.run_to_source_breakpoint(self, '// Set break point at this line.',
          lldb.SBFileSpec("main.cpp", False))

        # The stop reason of the thread should be breakpoint.
        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                    substrs=['stopped',
                             'stop reason = breakpoint'])

        # The breakpoint should have a hit count of 1.
        self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
                    substrs=[' resolved, hit count = 1'])

        self.expect("expr (lba.a)", VARIABLES_DISPLAYED_CORRECTLY,
                    substrs=['unsigned int', '2'])
        self.expect("expr (lbb.b)", VARIABLES_DISPLAYED_CORRECTLY,
                    substrs=['unsigned int', '3'])
        self.expect("expr (lbc.c)", VARIABLES_DISPLAYED_CORRECTLY,
                    substrs=['unsigned int', '4'])
        self.expect("expr (lbd.a)", VARIABLES_DISPLAYED_CORRECTLY,
                    substrs=['unsigned int', '5'])
        self.expect("expr (clang_example.f.a)", VARIABLES_DISPLAYED_CORRECTLY,
                    substrs=['uint64_t', '1'])

        self.expect(
            "frame variable --show-types lba",
            VARIABLES_DISPLAYED_CORRECTLY,
            substrs=[
                '(int:32)  = ',
                '(unsigned int:20) a = 2',
                ])

        self.expect(
            "frame variable --show-types lbb",
            VARIABLES_DISPLAYED_CORRECTLY,
            substrs=[
                '(unsigned int:1) a = 1',
                '(int:31)  =',
                '(unsigned int:20) b = 3',
                ])

        self.expect(
            "frame variable --show-types lbc",
            VARIABLES_DISPLAYED_CORRECTLY,
            substrs=[
                '(int:22)  =',
                '(unsigned int:1) a = 1',
                '(unsigned int:1) b = 0',
                '(unsigned int:5) c = 4',
                '(unsigned int:1) d = 1',
                '(int:2)  =',
                '(unsigned int:20) e = 20',
                ])

        self.expect(
            "frame variable --show-types lbd",
            VARIABLES_DISPLAYED_CORRECTLY,
            substrs=[
                '(char [3]) arr = "abc"',
                '(int:32)  =',
                '(unsigned int:20) a = 5',
                ])

        self.expect(
            "frame variable --show-types clang_example",
            VARIABLES_DISPLAYED_CORRECTLY,
            substrs=[
                   '(int:22)  =',
                   '(uint64_t:1) a = 1',
                   '(uint64_t:1) b = 0',
                   '(uint64_t:1) c = 1',
                   '(uint64_t:1) d = 0',
                   '(uint64_t:1) e = 1',
                   '(uint64_t:1) f = 0',
                   '(uint64_t:1) g = 1',
                   '(uint64_t:1) h = 0',
                   '(uint64_t:1) i = 1',
                   '(uint64_t:1) j = 0',
                   '(uint64_t:1) k = 1',
                ])
+81 −0
Original line number Diff line number Diff line
#include <stdint.h>

int main(int argc, char const *argv[]) {
  struct LargeBitsA {
    unsigned int : 30, a : 20;
  } lba;

  struct LargeBitsB {
    unsigned int a : 1, : 11, : 12, b : 20;
  } lbb;

  struct LargeBitsC {
    unsigned int : 13, : 9, a : 1, b : 1, c : 5, d : 1, e : 20;
  } lbc;

  struct LargeBitsD {
    char arr[3];
    unsigned int : 30, a : 20;
  } lbd;

  // This case came up when debugging clang and models RecordDeclBits
  struct BitExampleFromClangDeclContext {
    class fields {
      uint64_t : 13;
      uint64_t : 9;

      uint64_t a: 1;
      uint64_t b: 1;
      uint64_t c: 1;
      uint64_t d: 1;
      uint64_t e: 1;
      uint64_t f: 1;
      uint64_t g: 1;
      uint64_t h: 1;
      uint64_t i: 1;
      uint64_t j: 1;
      uint64_t k: 1;

      // In order to reproduce the crash for this case we need the
      // members of fields to stay private :-(
      friend struct BitExampleFromClangDeclContext;
    };

    union {
      struct fields f;
    };

    BitExampleFromClangDeclContext() {
  f.a = 1;
  f.b = 0;
  f.c = 1;
  f.d = 0;
  f.e = 1;
  f.f = 0;
  f.g = 1;
  f.h = 0;
  f.i = 1;
  f.j = 0;
  f.k = 1;
    } 
  } clang_example;

  lba.a = 2;

  lbb.a = 1;
  lbb.b = 3;

  lbc.a = 1;
  lbc.b = 0;
  lbc.c = 4;
  lbc.d = 1;
  lbc.e = 20;

  lbd.arr[0] = 'a';
  lbd.arr[1] = 'b';
  lbd.arr[2] = 'c';
  lbd.a = 5;


  return 0; // Set break point at this line.
}
+55 −111
Original line number Diff line number Diff line
@@ -85,35 +85,6 @@ static bool DeclKindIsCXXClass(clang::Decl::Kind decl_kind) {
  return false;
}

struct BitfieldInfo {
  uint64_t bit_size;
  uint64_t bit_offset;

  BitfieldInfo()
      : bit_size(LLDB_INVALID_ADDRESS), bit_offset(LLDB_INVALID_ADDRESS) {}

  void Clear() {
    bit_size = LLDB_INVALID_ADDRESS;
    bit_offset = LLDB_INVALID_ADDRESS;
  }

  bool IsValid() const {
    return (bit_size != LLDB_INVALID_ADDRESS) &&
           (bit_offset != LLDB_INVALID_ADDRESS);
  }

  bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
    if (IsValid()) {
      // This bitfield info is valid, so any subsequent bitfields must not
      // overlap and must be at a higher bit offset than any previous bitfield
      // + size.
      return (bit_size + bit_offset) <= next_bit_offset;
    } else {
      // If the this BitfieldInfo is not valid, then any offset isOK
      return true;
    }
  }
};

ClangASTImporter &DWARFASTParserClang::GetClangASTImporter() {
  if (!m_clang_ast_importer_up) {
@@ -2419,7 +2390,7 @@ void DWARFASTParserClang::ParseSingleMember(
    lldb::AccessType &default_accessibility,
    DelayedPropertyList &delayed_properties,
    lldb_private::ClangASTImporter::LayoutInfo &layout_info,
    BitfieldInfo &last_field_info) {
    FieldInfo &last_field_info) {
  ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
  const dw_tag_t tag = die.Tag();
  // Get the parent byte size so we can verify any members will fit
@@ -2453,6 +2424,14 @@ void DWARFASTParserClang::ParseSingleMember(
      const dw_attr_t attr = attributes.AttributeAtIndex(i);
      DWARFFormValue form_value;
      if (attributes.ExtractFormValueAtIndex(i, form_value)) {
        // DW_AT_data_member_location indicates the byte offset of the
        // word from the base address of the structure.
        //
        // DW_AT_bit_offset indicates how many bits into the word
        // (according to the host endianness) the low-order bit of the
        // field starts.  AT_bit_offset can be negative.
        //
        // DW_AT_bit_size indicates the size of the field in bits.
        switch (attr) {
        case DW_AT_name:
          name = form_value.AsCString();
@@ -2603,36 +2582,24 @@ void DWARFASTParserClang::ParseSingleMember(
      Type *member_type = die.ResolveTypeUID(encoding_form.Reference());

      clang::FieldDecl *field_decl = nullptr;
      const uint64_t character_width = 8;
      const uint64_t word_width = 32;
      if (tag == DW_TAG_member) {
        if (member_type) {
          CompilerType member_clang_type = member_type->GetLayoutCompilerType();

          if (accessibility == eAccessNone)
            accessibility = default_accessibility;
          member_accessibilities.push_back(accessibility);

          uint64_t field_bit_offset =
              (member_byte_offset == UINT32_MAX ? 0 : (member_byte_offset * 8));
          if (bit_size > 0) {

            BitfieldInfo this_field_info;
          if (bit_size > 0) {
            FieldInfo this_field_info;
            this_field_info.bit_offset = field_bit_offset;
            this_field_info.bit_size = bit_size;

            /////////////////////////////////////////////////////////////
            // How to locate a field given the DWARF debug information
            //
            // AT_byte_size indicates the size of the word in which the bit
            // offset must be interpreted.
            //
            // AT_data_member_location indicates the byte offset of the
            // word from the base address of the structure.
            //
            // AT_bit_offset indicates how many bits into the word
            // (according to the host endianness) the low-order bit of the
            // field starts.  AT_bit_offset can be negative.
            //
            // AT_bit_size indicates the size of the field in bits.
            /////////////////////////////////////////////////////////////

            if (data_bit_offset != UINT64_MAX) {
              this_field_info.bit_offset = data_bit_offset;
            } else {
@@ -2649,8 +2616,9 @@ void DWARFASTParserClang::ParseSingleMember(
            }

            if ((this_field_info.bit_offset >= parent_bit_size) ||
                (last_field_info.IsBitfield() &&
                 !last_field_info.NextBitfieldOffsetIsValid(
                    this_field_info.bit_offset)) {
                     this_field_info.bit_offset))) {
              ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
              objfile->GetModule()->ReportWarning(
                  "0x%8.8" PRIx64 ": %s bitfield named \"%s\" has invalid "
@@ -2659,40 +2627,12 @@ void DWARFASTParserClang::ParseSingleMember(
                  "compiler and include the preprocessed output for %s\n",
                  die.GetID(), DW_TAG_value_to_name(tag), name,
                  this_field_info.bit_offset, GetUnitName(parent_die).c_str());
              this_field_info.Clear();
              return;
            }

            // Update the field bit offset we will report for layout
            field_bit_offset = this_field_info.bit_offset;

            // If the member to be emitted did not start on a character
            // boundary and there is empty space between the last field and
            // this one, then we need to emit an anonymous member filling
            // up the space up to its start.  There are three cases here:
            //
            // 1 If the previous member ended on a character boundary, then
            // we can emit an
            //   anonymous member starting at the most recent character
            //   boundary.
            //
            // 2 If the previous member did not end on a character boundary
            // and the distance
            //   from the end of the previous member to the current member
            //   is less than a
            //   word width, then we can emit an anonymous member starting
            //   right after the
            //   previous member and right before this member.
            //
            // 3 If the previous member did not end on a character boundary
            // and the distance
            //   from the end of the previous member to the current member
            //   is greater than
            //   or equal a word width, then we act as in Case 1.

            const uint64_t character_width = 8;
            const uint64_t word_width = 32;

            // Objective-C has invalid DW_AT_bit_offset values in older
            // versions of clang, so we have to be careful and only insert
            // unnamed bitfields if we have a new enough clang.
@@ -2704,53 +2644,57 @@ void DWARFASTParserClang::ParseSingleMember(
                  die.GetCU()->Supports_unnamed_objc_bitfields();

            if (detect_unnamed_bitfields) {
              BitfieldInfo anon_field_info;

              if ((this_field_info.bit_offset % character_width) !=
                  0) // not char aligned
              {
              clang::Optional<FieldInfo> unnamed_field_info;
              uint64_t last_field_end = 0;

                if (last_field_info.IsValid())
              last_field_end =
                  last_field_info.bit_offset + last_field_info.bit_size;

                if (this_field_info.bit_offset != last_field_end) {
                  if (((last_field_end % character_width) == 0) || // case 1
                      (this_field_info.bit_offset - last_field_end >=
                       word_width)) // case 3
                  {
                    anon_field_info.bit_size =
                        this_field_info.bit_offset % character_width;
                    anon_field_info.bit_offset =
                        this_field_info.bit_offset - anon_field_info.bit_size;
                  } else // case 2
                  {
                    anon_field_info.bit_size =
              if (!last_field_info.IsBitfield()) {
                // The last field was not a bit-field...
                // but if it did take up the entire word then we need to extend
                // last_field_end so the bit-field does not step into the last
                // fields padding.
                if (last_field_end != 0 && ((last_field_end % word_width) != 0))
                  last_field_end += word_width - (last_field_end % word_width);
              }

              // If we have a gap between the last_field_end and the current
              // field we have an unnamed bit-field
              if (this_field_info.bit_offset != last_field_end &&
                  !(this_field_info.bit_offset < last_field_end)) {
                unnamed_field_info = FieldInfo{};
                unnamed_field_info->bit_size =
                    this_field_info.bit_offset - last_field_end;
                    anon_field_info.bit_offset = last_field_end;
                  }
                }
                unnamed_field_info->bit_offset = last_field_end;
              }

              if (anon_field_info.IsValid()) {
              if (unnamed_field_info) {
                clang::FieldDecl *unnamed_bitfield_decl =
                    TypeSystemClang::AddFieldToRecordType(
                        class_clang_type, llvm::StringRef(),
                        m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
                                                                  word_width),
                        accessibility, anon_field_info.bit_size);
                        accessibility, unnamed_field_info->bit_size);

                layout_info.field_offsets.insert(std::make_pair(
                    unnamed_bitfield_decl, anon_field_info.bit_offset));
                    unnamed_bitfield_decl, unnamed_field_info->bit_offset));
              }
            }

            last_field_info = this_field_info;
            last_field_info.SetIsBitfield(true);
          } else {
            last_field_info.Clear();
            last_field_info.bit_offset = field_bit_offset;

            if (llvm::Optional<uint64_t> clang_type_size =
                    member_clang_type.GetByteSize(nullptr)) {
              last_field_info.bit_size = *clang_type_size * character_width;
            }

            last_field_info.SetIsBitfield(false);
          }

          CompilerType member_clang_type = member_type->GetLayoutCompilerType();
          if (!member_clang_type.IsCompleteType())
            member_clang_type.GetCompleteType();

@@ -2885,7 +2829,7 @@ bool DWARFASTParserClang::ParseChildMembers(
  if (!parent_die)
    return false;

  BitfieldInfo last_field_info;
  FieldInfo last_field_info;

  ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
  TypeSystemClang *ast =
+11 −24
Original line number Diff line number Diff line
@@ -170,33 +170,20 @@ protected:
  lldb::ModuleSP GetModuleForType(const DWARFDIE &die);

private:
  struct BitfieldInfo {
    uint64_t bit_size;
    uint64_t bit_offset;
  struct FieldInfo {
    uint64_t bit_size = 0;
    uint64_t bit_offset = 0;
    bool is_bitfield = false;

    BitfieldInfo()
        : bit_size(LLDB_INVALID_ADDRESS), bit_offset(LLDB_INVALID_ADDRESS) {}
    FieldInfo() = default;

    void Clear() {
      bit_size = LLDB_INVALID_ADDRESS;
      bit_offset = LLDB_INVALID_ADDRESS;
    }

    bool IsValid() const {
      return (bit_size != LLDB_INVALID_ADDRESS) &&
             (bit_offset != LLDB_INVALID_ADDRESS);
    }
    void SetIsBitfield(bool flag) { is_bitfield = flag; }
    bool IsBitfield() { return is_bitfield; }

    bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
      if (IsValid()) {
        // This bitfield info is valid, so any subsequent bitfields must not
        // overlap and must be at a higher bit offset than any previous bitfield
        // + size.
      // Any subsequent bitfields must not overlap and must be at a higher
      // bit offset than any previous bitfield + size.
      return (bit_size + bit_offset) <= next_bit_offset;
      } else {
        // If the this BitfieldInfo is not valid, then any offset isOK
        return true;
      }
    }
  };

@@ -208,7 +195,7 @@ private:
                    lldb::AccessType &default_accessibility,
                    DelayedPropertyList &delayed_properties,
                    lldb_private::ClangASTImporter::LayoutInfo &layout_info,
                    BitfieldInfo &last_field_info);
                    FieldInfo &last_field_info);

  bool CompleteRecordType(const DWARFDIE &die, lldb_private::Type *type,
                          lldb_private::CompilerType &clang_type);