kuilpd updated this revision to Diff 509080.
kuilpd added a comment.

Added asserts in default cases, added checks for architecture instead of 
address size, added a unit test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146965/new/

https://reviews.llvm.org/D146965

Files:
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h
  lldb/unittests/Utility/ArchSpecTest.cpp

Index: lldb/unittests/Utility/ArchSpecTest.cpp
===================================================================
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -123,6 +123,12 @@
   EXPECT_STREQ("i686", AS.GetArchitectureName());
   EXPECT_EQ(ArchSpec::eCore_x86_32_i686, AS.GetCore());
 
+  AS = ArchSpec();
+  EXPECT_TRUE(AS.SetTriple("msp430---elf"));
+  EXPECT_EQ(llvm::Triple::msp430, AS.GetTriple().getArch());
+  EXPECT_STREQ("msp430", AS.GetArchitectureName());
+  EXPECT_EQ(ArchSpec::eCore_msp430, AS.GetCore());
+
   // Various flavors of invalid triples.
   AS = ArchSpec();
   EXPECT_FALSE(AS.SetTriple("unknown-unknown-unknown"));
Index: lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h
===================================================================
--- lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h
+++ lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h
@@ -10,10 +10,6 @@
 #ifndef LLDB_SOURCE_PLUGINS_ABI_MSP430_ABISYSV_MSP430_H
 #define LLDB_SOURCE_PLUGINS_ABI_MSP430_ABISYSV_MSP430_H
 
-// C Includes
-// C++ Includes
-// Other libraries and framework includes
-// Project includes
 #include "lldb/Target/ABI.h"
 #include "lldb/lldb-private.h"
 
@@ -48,10 +44,9 @@
 
   bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
     // Make sure the stack call frame addresses are 2 byte aligned
-    if (cfa & 0x01)
-      return false; // Not 2 byte aligned
-    if (cfa == 0)
-      return false; // Zero is not a valid stack address
+    // and not zero
+    if (cfa & 0x01 || cfa == 0)
+      return false;
     return true;
   }
 
Index: lldb/source/Expression/LLVMUserExpression.cpp
===================================================================
--- lldb/source/Expression/LLVMUserExpression.cpp
+++ lldb/source/Expression/LLVMUserExpression.cpp
@@ -333,9 +333,9 @@
     if (m_can_interpret && m_stack_frame_bottom == LLDB_INVALID_ADDRESS) {
       Status alloc_error;
 
+      auto arch = target->GetArchitecture().GetTriple().getArch();
       const size_t stack_frame_size =
-          target->GetArchitecture().GetAddressByteSize() == 2 ? 512
-                                                              : 512 * 1024;
+          arch == llvm::Triple::msp430 ? 512 : 512 * 1024;
 
       const bool zero_memory = false;
 
Index: lldb/source/Expression/IRMemoryMap.cpp
===================================================================
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -97,22 +97,21 @@
   // adequate space for our allocation.
   if (process_is_alive) {
     uint64_t end_of_memory;
-    uint32_t address_byte_size = process_sp->GetAddressByteSize();
-    switch (address_byte_size) {
+    switch (process_sp->GetAddressByteSize()) {
     case 2:
       end_of_memory = 0xffffull;
       break;
     case 4:
       end_of_memory = 0xffffffffull;
       break;
-    default:
+    case 8:
       end_of_memory = 0xffffffffffffffffull;
       break;
+    default:
+      lldbassert(false && "Invalid address size.");
+      return LLDB_INVALID_ADDRESS;
     }
 
-    lldbassert(process_sp->GetAddressByteSize() == 4 ||
-               end_of_memory != 0xffffffffull);
-
     MemoryRegionInfo region_info;
     Status err = process_sp->GetMemoryRegionInfo(ret, region_info);
     if (err.Success()) {
@@ -157,16 +156,20 @@
       case 4:
         ret = 0xee000000ull;
         break;
-      default:
+      case 8:
         ret = 0xdead0fff00000000ull;
         break;
+      default:
+        lldbassert(false && "Invalid address size.");
+        return LLDB_INVALID_ADDRESS;
       }
     }
   } else {
     auto back = m_allocations.rbegin();
     lldb::addr_t addr = back->first;
     size_t alloc_size = back->second.m_size;
-    auto align = GetAddressByteSize() == 2 ? 512 : 4096;
+    auto arch = target_sp->GetArchitecture().GetTriple().getArch();
+    auto align = arch == llvm::Triple::msp430 ? 512 : 4096;
     ret = llvm::alignTo(addr + alloc_size, align);
   }
 
Index: lldb/include/lldb/Utility/DataExtractor.h
===================================================================
--- lldb/include/lldb/Utility/DataExtractor.h
+++ lldb/include/lldb/Utility/DataExtractor.h
@@ -843,9 +843,7 @@
   /// \param[in] addr_size
   ///     The size in bytes to use when extracting addresses.
   void SetAddressByteSize(uint32_t addr_size) {
-#ifdef LLDB_CONFIGURATION_DEBUG
     assert(addr_size == 2 || addr_size == 4 || addr_size == 8);
-#endif
     m_addr_size = addr_size;
   }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to