Make CompilerType::getBitSize() / getByteSize() return an optional re…

…sult. NFC

The code in LLDB assumes that CompilerType and friends use the size 0
as a sentinel value to signal an error. This works for C++, where no
zero-sized type exists, but in many other programming languages
(including I believe C) types of size zero are possible and even
common. This is a particular pain point in swift-lldb, where extra
code exists to double-check that a type is *really* of size zero and
not an error at various locations.

To remedy this situation, this patch starts by converting
CompilerType::getBitSize() and getByteSize() to return an optional
result. To avoid wasting space, I hand-rolled my own optional data
type assuming that no type is larger than what fits into 63
bits. Follow-up patches would make similar changes to the ValueObject
hierarchy.

rdar://problem/47178964

Differential Revision: https://reviews.llvm.org/D56688

llvm-svn: 351214

lldb/include/lldb/Symbol/CompilerType.h

@@ -287,9 +287,8 @@ class CompilerType {
 
   struct IntegralTemplateArgument;
 
-  uint64_t GetByteSize(ExecutionContextScope *exe_scope) const;
-
-  uint64_t GetBitSize(ExecutionContextScope *exe_scope) const;
+  llvm::Optional<uint64_t> GetByteSize(ExecutionContextScope *exe_scope) const;
+  llvm::Optional<uint64_t> GetBitSize(ExecutionContextScope *exe_scope) const;
 
   lldb::Encoding GetEncoding(uint64_t &count) const;
 

lldb/include/lldb/Target/ProcessStructReader.h

@@ -60,18 +60,20 @@ class ProcessStructReader {
         return;
       auto size = field_type.GetByteSize(nullptr);
       // no support for things larger than a uint64_t (yet)
-      if (size > 8)
+      if (!size || *size > 8)
         return;
       ConstString const_name = ConstString(name.c_str());
       size_t byte_index = static_cast<size_t>(bit_offset / 8);
       m_fields[const_name] =
-          FieldImpl{field_type, byte_index, static_cast<size_t>(size)};
+          FieldImpl{field_type, byte_index, static_cast<size_t>(*size)};
     }
-    size_t total_size = struct_type.GetByteSize(nullptr);
-    lldb::DataBufferSP buffer_sp(new DataBufferHeap(total_size, 0));
+    auto total_size = struct_type.GetByteSize(nullptr);
+    if (!total_size)
+      return;
+    lldb::DataBufferSP buffer_sp(new DataBufferHeap(*total_size, 0));
     Status error;
     process->ReadMemoryFromInferior(base_addr, buffer_sp->GetBytes(),
-                                    total_size, error);
+                                    *total_size, error);
     if (error.Fail())
       return;
     m_data = DataExtractor(buffer_sp, m_byte_order, m_addr_byte_size);

lldb/source/API/SBType.cpp

@@ -103,10 +103,10 @@ bool SBType::IsValid() const {
 }
 
 uint64_t SBType::GetByteSize() {
-  if (!IsValid())
-    return 0;
-
-  return m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr);
+  if (IsValid())
+    if (auto size = m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))
+      return *size;
+  return 0;
 }
 
 bool SBType::IsPointerType() {

lldb/source/Commands/CommandObjectMemory.cpp

@@ -519,15 +519,15 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
         --pointer_count;
       }
 
-      m_format_options.GetByteSizeValue() = clang_ast_type.GetByteSize(nullptr);
-
-      if (m_format_options.GetByteSizeValue() == 0) {
+      auto size = clang_ast_type.GetByteSize(nullptr);
+      if (!size) {
         result.AppendErrorWithFormat(
             "unable to get the byte size of the type '%s'\n",
             view_as_type_cstr);
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
+      m_format_options.GetByteSizeValue() = *size;
 
       if (!m_format_options.GetCountValue().OptionWasSet())
         m_format_options.GetCountValue() = 1;
@@ -642,12 +642,15 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
       if (!m_format_options.GetFormatValue().OptionWasSet())
         m_format_options.GetFormatValue().SetCurrentValue(eFormatDefault);
 
-      bytes_read = clang_ast_type.GetByteSize(nullptr) *
-                   m_format_options.GetCountValue().GetCurrentValue();
+      auto size = clang_ast_type.GetByteSize(nullptr);
+      if (!size) {
+        result.AppendError("can't get size of type");
+        return false;
+      }
+      bytes_read = *size * m_format_options.GetCountValue().GetCurrentValue();
 
       if (argc > 0)
-        addr = addr + (clang_ast_type.GetByteSize(nullptr) *
-                       m_memory_options.m_offset.GetCurrentValue());
+        addr = addr + (*size * m_memory_options.m_offset.GetCurrentValue());
     } else if (m_format_options.GetFormatValue().GetCurrentValue() !=
                eFormatCString) {
       data_sp.reset(new DataBufferHeap(total_byte_size, '\0'));
@@ -1061,7 +1064,10 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
                m_memory_options.m_expr.GetStringValue(), frame, result_sp)) &&
           result_sp) {
         uint64_t value = result_sp->GetValueAsUnsigned(0);
-        switch (result_sp->GetCompilerType().GetByteSize(nullptr)) {
+        auto size = result_sp->GetCompilerType().GetByteSize(nullptr);
+        if (!size)
+          return false;
+        switch (*size) {
         case 1: {
           uint8_t byte = (uint8_t)value;
           buffer.CopyData(&byte, 1);

lldb/source/Core/Value.cpp

@@ -224,8 +224,9 @@ uint64_t Value::GetValueByteSize(Status *error_ptr, ExecutionContext *exe_ctx) {
   {
     const CompilerType &ast_type = GetCompilerType();
     if (ast_type.IsValid())
-      byte_size = ast_type.GetByteSize(
-          exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
+      if (auto size = ast_type.GetByteSize(
+              exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
+        byte_size = *size;
   } break;
   }
 
@@ -345,8 +346,9 @@ Status Value::GetValueAsData(ExecutionContext *exe_ctx, DataExtractor &data,
     uint32_t limit_byte_size = UINT32_MAX;
 
     if (ast_type.IsValid()) {
-      limit_byte_size = ast_type.GetByteSize(
-          exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
+      if (auto size = ast_type.GetByteSize(
+              exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
+        limit_byte_size = *size;
     }
 
     if (limit_byte_size <= m_value.GetByteSize()) {

lldb/source/Core/ValueObject.cpp

@@ -756,10 +756,12 @@ size_t ValueObject::GetPointeeData(DataExtractor &data, uint32_t item_idx,
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
 
-  const uint64_t item_type_size = pointee_or_element_compiler_type.GetByteSize(
+  auto item_type_size = pointee_or_element_compiler_type.GetByteSize(
       exe_ctx.GetBestExecutionContextScope());
-  const uint64_t bytes = item_count * item_type_size;
-  const uint64_t offset = item_idx * item_type_size;
+  if (!item_type_size)
+    return 0;
+  const uint64_t bytes = item_count * *item_type_size;
+  const uint64_t offset = item_idx * *item_type_size;
 
   if (item_idx == 0 && item_count == 1) // simply a deref
   {
@@ -822,10 +824,10 @@ size_t ValueObject::GetPointeeData(DataExtractor &data, uint32_t item_idx,
       }
     } break;
     case eAddressTypeHost: {
-      const uint64_t max_bytes =
+      auto max_bytes =
           GetCompilerType().GetByteSize(exe_ctx.GetBestExecutionContextScope());
-      if (max_bytes > offset) {
-        size_t bytes_read = std::min<uint64_t>(max_bytes - offset, bytes);
+      if (max_bytes && *max_bytes > offset) {
+        size_t bytes_read = std::min<uint64_t>(*max_bytes - offset, bytes);
         addr = m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
         if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
           break;
@@ -1818,14 +1820,15 @@ ValueObjectSP ValueObject::GetSyntheticChildAtOffset(
     return synthetic_child_sp;
 
   if (!can_create)
-    return ValueObjectSP();
+    return {};
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
-
-  ValueObjectChild *synthetic_child = new ValueObjectChild(
-      *this, type, name_const_str,
-      type.GetByteSize(exe_ctx.GetBestExecutionContextScope()), offset, 0, 0,
-      false, false, eAddressTypeInvalid, 0);
+  auto size = type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
+  if (!size)
+    return {};
+  ValueObjectChild *synthetic_child =
+      new ValueObjectChild(*this, type, name_const_str, *size, offset, 0, 0,
+                           false, false, eAddressTypeInvalid, 0);
   if (synthetic_child) {
     AddSyntheticChild(name_const_str, synthetic_child);
     synthetic_child_sp = synthetic_child->GetSP();
@@ -1856,16 +1859,17 @@ ValueObjectSP ValueObject::GetSyntheticBase(uint32_t offset,
     return synthetic_child_sp;
 
   if (!can_create)
-    return ValueObjectSP();
+    return {};
 
   const bool is_base_class = true;
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
-
-  ValueObjectChild *synthetic_child = new ValueObjectChild(
-      *this, type, name_const_str,
-      type.GetByteSize(exe_ctx.GetBestExecutionContextScope()), offset, 0, 0,
-      is_base_class, false, eAddressTypeInvalid, 0);
+  auto size = type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
+  if (!size)
+    return {};
+  ValueObjectChild *synthetic_child =
+      new ValueObjectChild(*this, type, name_const_str, *size, offset, 0, 0,
+                           is_base_class, false, eAddressTypeInvalid, 0);
   if (synthetic_child) {
     AddSyntheticChild(name_const_str, synthetic_child);
     synthetic_child_sp = synthetic_child->GetSP();

lldb/source/Core/ValueObjectConstResult.cpp

@@ -198,10 +198,11 @@ lldb::ValueType ValueObjectConstResult::GetValueType() const {
 
 uint64_t ValueObjectConstResult::GetByteSize() {
   ExecutionContext exe_ctx(GetExecutionContextRef());
-
-  if (m_byte_size == 0)
-    SetByteSize(
-        GetCompilerType().GetByteSize(exe_ctx.GetBestExecutionContextScope()));
+  if (m_byte_size == 0) {
+    if (auto size =
+        GetCompilerType().GetByteSize(exe_ctx.GetBestExecutionContextScope()))
+      SetByteSize(*size);
+  }
   return m_byte_size;
 }
 

lldb/source/Core/ValueObjectMemory.cpp

@@ -138,7 +138,9 @@ size_t ValueObjectMemory::CalculateNumChildren(uint32_t max) {
 uint64_t ValueObjectMemory::GetByteSize() {
   if (m_type_sp)
     return m_type_sp->GetByteSize();
-  return m_compiler_type.GetByteSize(nullptr);
+  if (auto size = m_compiler_type.GetByteSize(nullptr))
+    return *size;
+  return 0;
 }
 
 lldb::ValueType ValueObjectMemory::GetValueType() const {

lldb/source/Core/ValueObjectVariable.cpp

@@ -112,7 +112,8 @@ uint64_t ValueObjectVariable::GetByteSize() {
   if (!type.IsValid())
     return 0;
 
-  return type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
+  auto size = type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
+  return size ? *size : 0;
 }
 
 lldb::ValueType ValueObjectVariable::GetValueType() const {

lldb/source/DataFormatters/TypeFormat.cpp

@@ -94,16 +94,18 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
             return false;
         }
 
+        ExecutionContextScope *exe_scope =
+            exe_ctx.GetBestExecutionContextScope();
+        auto size = compiler_type.GetByteSize(exe_scope);
+        if (!size)
+          return false;
         StreamString sstr;
-        ExecutionContextScope *exe_scope(
-            exe_ctx.GetBestExecutionContextScope());
         compiler_type.DumpTypeValue(
-            &sstr,       // The stream to use for display
-            GetFormat(), // Format to display this type with
-            data,        // Data to extract from
-            0,           // Byte offset into "m_data"
-            compiler_type.GetByteSize(
-                exe_scope),                 // Byte size of item in "m_data"
+            &sstr,                          // The stream to use for display
+            GetFormat(),                    // Format to display this type with
+            data,                           // Data to extract from
+            0,                              // Byte offset into "m_data"
+            *size,                          // Byte size of item in "m_data"
             valobj->GetBitfieldBitSize(),   // Bitfield bit size
             valobj->GetBitfieldBitOffset(), // Bitfield bit offset
             exe_scope);

lldb/source/DataFormatters/VectorType.cpp

@@ -174,10 +174,10 @@ static size_t CalculateNumChildren(
   auto container_size = container_type.GetByteSize(exe_scope);
   auto element_size = element_type.GetByteSize(exe_scope);
 
-  if (element_size) {
-    if (container_size % element_size)
+  if (container_size && element_size && *element_size) {
+    if (*container_size % *element_size)
       return 0;
-    return container_size / element_size;
+    return *container_size / *element_size;
   }
   return 0;
 }
@@ -197,8 +197,11 @@ class VectorTypeSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 
   lldb::ValueObjectSP GetChildAtIndex(size_t idx) override {
     if (idx >= CalculateNumChildren())
-      return lldb::ValueObjectSP();
-    auto offset = idx * m_child_type.GetByteSize(nullptr);
+      return {};
+    auto size = m_child_type.GetByteSize(nullptr);
+    if (!size)
+      return {};
+    auto offset = idx * *size;
     StreamString idx_name;
     idx_name.Printf("[%" PRIu64 "]", (uint64_t)idx);
     ValueObjectSP child_sp(m_backend.GetSyntheticChildAtOffset(

lldb/source/Expression/Materializer.cpp

@@ -46,7 +46,8 @@ uint32_t Materializer::AddStructMember(Entity &entity) {
 }
 
 void Materializer::Entity::SetSizeAndAlignmentFromType(CompilerType &type) {
-  m_size = type.GetByteSize(nullptr);
+  if (auto size = type.GetByteSize(nullptr))
+    m_size = *size;
 
   uint32_t bit_alignment = type.GetTypeBitAlign();
 
@@ -794,7 +795,11 @@ class EntityResultVariable : public Materializer::Entity {
 
       ExecutionContextScope *exe_scope = map.GetBestExecutionContextScope();
 
-      size_t byte_size = m_type.GetByteSize(exe_scope);
+      auto byte_size = m_type.GetByteSize(exe_scope);
+      if (!byte_size) {
+        err.SetErrorString("can't get size of type");
+        return;
+      }
       size_t bit_align = m_type.GetTypeBitAlign();
       size_t byte_align = (bit_align + 7) / 8;
 
@@ -805,10 +810,10 @@ class EntityResultVariable : public Materializer::Entity {
       const bool zero_memory = true;
 
       m_temporary_allocation = map.Malloc(
-          byte_size, byte_align,
+          *byte_size, byte_align,
           lldb::ePermissionsReadable | lldb::ePermissionsWritable,
           IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);
-      m_temporary_allocation_size = byte_size;
+      m_temporary_allocation_size = *byte_size;
 
       if (!alloc_error.Success()) {
         err.SetErrorStringWithFormat(

lldb/source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp

@@ -1470,14 +1470,16 @@ bool ABIMacOSX_arm::GetArgumentValues(Thread &thread, ValueList &values) const {
     if (compiler_type) {
       bool is_signed = false;
       size_t bit_width = 0;
-      if (compiler_type.IsIntegerOrEnumerationType(is_signed)) {
-        bit_width = compiler_type.GetBitSize(&thread);
-      } else if (compiler_type.IsPointerOrReferenceType()) {
-        bit_width = compiler_type.GetBitSize(&thread);
-      } else {
+      auto bit_size = compiler_type.GetBitSize(&thread);
+      if (!bit_size)
+        return false;
+      if (compiler_type.IsIntegerOrEnumerationType(is_signed))
+        bit_width = *bit_size;
+      else if (compiler_type.IsPointerOrReferenceType())
+        bit_width = *bit_size;
+      else
         // We only handle integer, pointer and reference types currently...
         return false;
-      }
 
       if (bit_width <= (exe_ctx.GetProcessRef().GetAddressByteSize() * 8)) {
         if (value_idx < 4) {
@@ -1574,9 +1576,11 @@ ValueObjectSP ABIMacOSX_arm::GetReturnValueObjectImpl(
 
   const RegisterInfo *r0_reg_info = reg_ctx->GetRegisterInfoByName("r0", 0);
   if (compiler_type.IsIntegerOrEnumerationType(is_signed)) {
-    size_t bit_width = compiler_type.GetBitSize(&thread);
+    auto bit_width = compiler_type.GetBitSize(&thread);
+    if (!bit_width)
+      return return_valobj_sp;
 
-    switch (bit_width) {
+    switch (*bit_width) {
     default:
       return return_valobj_sp;
     case 128:
@@ -1592,14 +1596,16 @@ ValueObjectSP ABIMacOSX_arm::GetReturnValueObjectImpl(
           const RegisterInfo *r3_reg_info =
               reg_ctx->GetRegisterInfoByName("r3", 0);
           if (r1_reg_info && r2_reg_info && r3_reg_info) {
-            const size_t byte_size = compiler_type.GetByteSize(&thread);
+            auto byte_size = compiler_type.GetByteSize(&thread);
+            if (!byte_size)
+              return return_valobj_sp;
             ProcessSP process_sp(thread.GetProcess());
-            if (byte_size <= r0_reg_info->byte_size + r1_reg_info->byte_size +
-                                 r2_reg_info->byte_size +
-                                 r3_reg_info->byte_size &&
+            if (*byte_size <= r0_reg_info->byte_size + r1_reg_info->byte_size +
+                                  r2_reg_info->byte_size +
+                                  r3_reg_info->byte_size &&
                 process_sp) {
               std::unique_ptr<DataBufferHeap> heap_data_ap(
-                  new DataBufferHeap(byte_size, 0));
+                  new DataBufferHeap(*byte_size, 0));
               const ByteOrder byte_order = process_sp->GetByteOrder();
               RegisterValue r0_reg_value;
               RegisterValue r1_reg_value;