[Lldb-commits] [lldb] r351376 - Change TypeSystem::GetBitSize() to return an optional result.

Adrian Prantl via lldb-commits lldb-commits at lists.llvm.org
Wed Jan 16 13:19:20 PST 2019


Author: adrian
Date: Wed Jan 16 13:19:20 2019
New Revision: 351376

URL: http://llvm.org/viewvc/llvm-project?rev=351376&view=rev
Log:
Change TypeSystem::GetBitSize() to return an optional result.

This patch changes the behavior when printing C++ function references:
where we previously would get a <could not determine size>, there is
now a <no summary available>. It's not clear to me whether this is a
bug or an omission, but it's one step further than LLDB previously
got.

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

Added:
    lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/
    lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/Makefile
    lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/TestFunctionRefs.py
    lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/main.cpp
Modified:
    lldb/trunk/include/lldb/Symbol/ClangASTContext.h
    lldb/trunk/include/lldb/Symbol/TypeSystem.h
    lldb/trunk/lit/SymbolFile/NativePDB/function-types-builtins.cpp
    lldb/trunk/packages/Python/lldbsuite/test/lang/c/struct_types/main.c
    lldb/trunk/source/Core/Value.cpp
    lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=351376&r1=351375&r2=351376&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Wed Jan 16 13:19:20 2019
@@ -726,13 +726,16 @@ public:
   // Exploring the type
   //----------------------------------------------------------------------
 
-  uint64_t GetByteSize(lldb::opaque_compiler_type_t type,
+  llvm::Optional<uint64_t> GetByteSize(lldb::opaque_compiler_type_t type,
                        ExecutionContextScope *exe_scope) {
-    return (GetBitSize(type, exe_scope) + 7) / 8;
+    if (llvm::Optional<uint64_t> bit_size = GetBitSize(type, exe_scope))
+      return (*bit_size + 7) / 8;
+    return llvm::None;
   }
 
-  uint64_t GetBitSize(lldb::opaque_compiler_type_t type,
-                      ExecutionContextScope *exe_scope) override;
+  llvm::Optional<uint64_t>
+  GetBitSize(lldb::opaque_compiler_type_t type,
+             ExecutionContextScope *exe_scope) override;
 
   lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
                              uint64_t &count) override;

Modified: lldb/trunk/include/lldb/Symbol/TypeSystem.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/TypeSystem.h?rev=351376&r1=351375&r2=351376&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/TypeSystem.h (original)
+++ lldb/trunk/include/lldb/Symbol/TypeSystem.h Wed Jan 16 13:19:20 2019
@@ -271,8 +271,9 @@ public:
   // Exploring the type
   //----------------------------------------------------------------------
 
-  virtual uint64_t GetBitSize(lldb::opaque_compiler_type_t type,
-                              ExecutionContextScope *exe_scope) = 0;
+  virtual llvm::Optional<uint64_t>
+  GetBitSize(lldb::opaque_compiler_type_t type,
+             ExecutionContextScope *exe_scope) = 0;
 
   virtual lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
                                      uint64_t &count) = 0;

Modified: lldb/trunk/lit/SymbolFile/NativePDB/function-types-builtins.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/NativePDB/function-types-builtins.cpp?rev=351376&r1=351375&r2=351376&view=diff
==============================================================================
--- lldb/trunk/lit/SymbolFile/NativePDB/function-types-builtins.cpp (original)
+++ lldb/trunk/lit/SymbolFile/NativePDB/function-types-builtins.cpp Wed Jan 16 13:19:20 2019
@@ -186,17 +186,17 @@ auto rad = &unaryret<int(*const)[5], 28>
 // Function references, we only need a couple of these since most of the
 // interesting cases are already tested.
 auto &ref = unary<bool>;
-// CHECK: (void (&)(bool)) ref = {{.*}} (&::ref = <Unable to determine byte size.>)
+// CHECK: (void (&)(bool)) ref = {{.*}} (&::ref = <no summary available>)
 auto &ref2 = unary<volatile int*>;
-// CHECK: (void (&)(volatile int *)) ref2 = {{.*}} (&::ref2 = <Unable to determine byte size.>)
+// CHECK: (void (&)(volatile int *)) ref2 = {{.*}} (&::ref2 = <no summary available>)
 auto &ref3 = varargs;
-// CHECK: (int (&)(int, int, ...)) ref3 = {{.*}} (&::ref3 = <Unable to determine byte size.>)
+// CHECK: (int (&)(int, int, ...)) ref3 = {{.*}} (&::ref3 = <no summary available>)
 
 // Multiple arguments, as before, just something to make sure it works.
 auto binp = &binary<int*, const int*>;
 // CHECK: (void (*)(int *, const int *)) binp = {{.*}}
 auto &binr = binary<int*, const int*>;
-// CHECK: (void (&)(int *, const int *)) binr = {{.*}} (&::binr = <Unable to determine byte size.>)
+// CHECK: (void (&)(int *, const int *)) binr = {{.*}} (&::binr = <no summary available>)
 
 // And finally, a function with no arguments.
 auto null = &nullary;

Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/struct_types/main.c
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/struct_types/main.c?rev=351376&r1=351375&r2=351376&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/struct_types/main.c (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/struct_types/main.c Wed Jan 16 13:19:20 2019
@@ -29,6 +29,10 @@ int main (int argc, char const *argv[])
        //% self.expect("expression -- (pt.padding[0])", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["(char)", " = "])
        //% self.expect("image lookup -t point_tag", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['padding[]']) # Once rdar://problem/12566646 is fixed, this should display correctly
 
+    struct {} empty;
+    //% self.expect("frame variable empty", substrs = ["empty = {}"])
+    //% self.expect("expression -- sizeof(empty)", substrs = ["= 0"])
+
     struct rect_tag {
         struct point_tag bottom_left;
         struct point_tag top_right;

Added: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/Makefile?rev=351376&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/Makefile (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/Makefile Wed Jan 16 13:19:20 2019
@@ -0,0 +1,3 @@
+LEVEL = ../../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules

Added: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/TestFunctionRefs.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/TestFunctionRefs.py?rev=351376&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/TestFunctionRefs.py (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/TestFunctionRefs.py Wed Jan 16 13:19:20 2019
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals())

Added: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/main.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/main.cpp?rev=351376&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/main.cpp (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function_refs/main.cpp Wed Jan 16 13:19:20 2019
@@ -0,0 +1,32 @@
+// This is plagiarized from lit/SymbolFile/NativePDB/function-types-builtin.cpp.
+void nullary() {}
+
+template<typename Arg>
+void unary(Arg) { }
+
+template<typename A1, typename A2>
+void binary(A1, A2) { }
+
+int varargs(int, int, ...) { return 0; }
+
+auto &ref = unary<bool>;
+auto &ref2 = unary<volatile int*>;
+auto &ref3 = varargs;
+auto binp = &binary<int*, const int*>;
+auto &binr = binary<int*, const int*>;
+auto null = &nullary;
+int main(int argc, char **argv) {
+//% self.expect("target var ref", substrs=["(void (&)(bool))", "ref = 0x",
+//%             "&::ref = <no summary available>"])
+//% self.expect("target var ref2",
+//%              substrs=["(void (&)(volatile int *))", "ref2 = 0x"])
+//% self.expect("target var ref3",
+//%              substrs=["(int (&)(int, int, ...))", "ref3 = 0x"])
+//% self.expect("target var binp",
+//%              substrs=["(void (*)(int *, const int *))", "binp = 0x"])
+//% self.expect("target var binr",
+//%              substrs=["(void (&)(int *, const int *))", "binr = 0x"])
+//% self.expect("target var null",
+//%              substrs=["(void (*)())", "null = 0x"])
+  return 0;
+}

Modified: lldb/trunk/source/Core/Value.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Value.cpp?rev=351376&r1=351375&r2=351376&view=diff
==============================================================================
--- lldb/trunk/source/Core/Value.cpp (original)
+++ lldb/trunk/source/Core/Value.cpp Wed Jan 16 13:19:20 2019
@@ -210,35 +210,31 @@ bool Value::ValueOf(ExecutionContext *ex
 }
 
 uint64_t Value::GetValueByteSize(Status *error_ptr, ExecutionContext *exe_ctx) {
-  uint64_t byte_size = 0;
-
   switch (m_context_type) {
   case eContextTypeRegisterInfo: // RegisterInfo *
-    if (GetRegisterInfo())
-      byte_size = GetRegisterInfo()->byte_size;
+    if (GetRegisterInfo()) {
+      if (error_ptr)
+        error_ptr->Clear();
+      return GetRegisterInfo()->byte_size;
+    }
     break;
 
   case eContextTypeInvalid:
   case eContextTypeLLDBType: // Type *
   case eContextTypeVariable: // Variable *
   {
-    const CompilerType &ast_type = GetCompilerType();
-    if (ast_type.IsValid())
-      if (llvm::Optional<uint64_t> size = ast_type.GetByteSize(
-              exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
-        byte_size = *size;
-  } break;
-  }
-
-  if (error_ptr) {
-    if (byte_size == 0) {
-      if (error_ptr->Success())
-        error_ptr->SetErrorString("Unable to determine byte size.");
-    } else {
-      error_ptr->Clear();
+    auto *scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
+    if (llvm::Optional<uint64_t> size = GetCompilerType().GetByteSize(scope)) {
+      if (error_ptr)
+        error_ptr->Clear();
+      return *size;
     }
+    break;
   }
-  return byte_size;
+  }
+  if (error_ptr && error_ptr->Success())
+    error_ptr->SetErrorString("Unable to determine byte size.");
+  return 0;
 }
 
 const CompilerType &Value::GetCompilerType() {
@@ -519,6 +515,10 @@ Status Value::GetValueAsData(ExecutionCo
   if (error.Fail())
     return error;
 
+  // No memory to read for zero-sized types.
+  if (byte_size == 0)
+    return error;
+
   // Make sure we have enough room within "data", and if we don't make
   // something large enough that does
   if (!data.ValidOffsetForDataOfSize(data_offset, byte_size)) {

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=351376&r1=351375&r2=351376&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Wed Jan 16 13:19:20 2019
@@ -5010,8 +5010,9 @@ CompilerType ClangASTContext::GetBasicTy
 // Exploring the type
 //----------------------------------------------------------------------
 
-uint64_t ClangASTContext::GetBitSize(lldb::opaque_compiler_type_t type,
-                                     ExecutionContextScope *exe_scope) {
+Optional<uint64_t>
+ClangASTContext::GetBitSize(lldb::opaque_compiler_type_t type,
+                            ExecutionContextScope *exe_scope) {
   if (GetCompleteType(type)) {
     clang::QualType qual_type(GetCanonicalQualType(type));
     const clang::Type::TypeClass type_class = qual_type->getTypeClass();
@@ -5020,7 +5021,7 @@ uint64_t ClangASTContext::GetBitSize(lld
       if (GetCompleteType(type))
         return getASTContext()->getTypeSize(qual_type);
       else
-        return 0;
+        return None;
       break;
 
     case clang::Type::ObjCInterface:
@@ -5065,10 +5066,14 @@ uint64_t ClangASTContext::GetBitSize(lld
         return bit_size +
                getASTContext()->getTypeSize(
                    getASTContext()->ObjCBuiltinClassTy);
-      return bit_size;
+      // Function types actually have a size of 0, that's not an error.
+      if (qual_type->isFunctionProtoType())
+        return bit_size;
+      if (bit_size)
+        return bit_size;
     }
   }
-  return 0;
+  return None;
 }
 
 size_t ClangASTContext::GetTypeBitAlign(lldb::opaque_compiler_type_t type) {




More information about the lldb-commits mailing list