shafik created this revision.
shafik added reviewers: jingham, aprantl, davide.

lldb::StateType is an enum without an explicit underlying type.

According to [dcl.enum]p8 http://eel.is/c++draft/dcl.enum#8 and 
[expr.static.cast]p10  http://eel.is/c++draft/expr.static.cast#10 we are 
limited to setting StateType to values bound with a value range based on the 
values of the enumerators. Currently we have a test and SWIG interface that 
either explicitly set StateType to a value outside of the allowed range or in 
the case of SWIG does not prevent it.

In both cases using UBSan generates a runtime error when we load a value 
outside of the range allowed by the standard.

This change refactors StateType to keep track of the largest value and range 
checking to the SWIG interface and removes the invalid test case.


https://reviews.llvm.org/D51445

Files:
  include/lldb/lldb-enumerations.h
  scripts/Python/python-typemaps.swig
  source/Interpreter/CommandObject.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  source/Target/Process.cpp
  source/Utility/State.cpp
  unittests/Utility/StateTest.cpp

Index: unittests/Utility/StateTest.cpp
===================================================================
--- unittests/Utility/StateTest.cpp
+++ unittests/Utility/StateTest.cpp
@@ -15,7 +15,18 @@
 using namespace lldb_private;
 
 TEST(StateTest, Formatv) {
-  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", kNumStateType).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", eStateInvalid).str());
+  EXPECT_EQ("unloaded", llvm::formatv("{0}", eStateUnloaded).str());
+  EXPECT_EQ("connected", llvm::formatv("{0}", eStateConnected).str());
+  EXPECT_EQ("attaching", llvm::formatv("{0}", eStateAttaching).str());
+  EXPECT_EQ("launching", llvm::formatv("{0}", eStateLaunching).str());
   EXPECT_EQ("stopped", llvm::formatv("{0}", eStateStopped).str());
-  EXPECT_EQ("unknown", llvm::formatv("{0}", StateType(-1)).str());
+  EXPECT_EQ("running", llvm::formatv("{0}", eStateRunning).str());
+  EXPECT_EQ("stepping", llvm::formatv("{0}", eStateStepping).str());
+  EXPECT_EQ("crashed", llvm::formatv("{0}", eStateCrashed).str());
+  EXPECT_EQ("detached", llvm::formatv("{0}", eStateDetached).str());
+  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("suspended", llvm::formatv("{0}", eStateSuspended).str());
+  
 }
Index: source/Utility/State.cpp
===================================================================
--- source/Utility/State.cpp
+++ source/Utility/State.cpp
@@ -18,6 +18,7 @@
 
 const char *lldb_private::StateAsCString(StateType state) {
   switch (state) {
+  case kNumStateType:
   case eStateInvalid:
     return "invalid";
   case eStateUnloaded:
@@ -78,6 +79,7 @@
   case eStateStepping:
     return true;
 
+  case kNumStateType:
   case eStateConnected:
   case eStateDetached:
   case eStateInvalid:
@@ -93,6 +95,7 @@
 
 bool lldb_private::StateIsStoppedState(StateType state, bool must_exist) {
   switch (state) {
+  case kNumStateType:
   case eStateInvalid:
   case eStateConnected:
   case eStateAttaching:
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -837,6 +837,7 @@
     Destroy(false);
     break;
 
+  case kNumStateType:
   case eStateInvalid:
   case eStateUnloaded:
   case eStateDetached:
@@ -1054,6 +1055,7 @@
     return false;
 
   switch (event_state) {
+  case kNumStateType:
   case eStateInvalid:
   case eStateUnloaded:
   case eStateAttaching:
@@ -1854,6 +1856,7 @@
 
   bool show_error = true;
   switch (GetState()) {
+  case kNumStateType:
   case eStateInvalid:
   case eStateUnloaded:
   case eStateConnected:
@@ -3582,6 +3585,7 @@
     // always report them.
     return_value = true;
     break;
+  case kNumStateType:
   case eStateInvalid:
     // We stopped for no apparent reason, don't report it.
     return_value = false;
Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
===================================================================
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -776,6 +776,7 @@
   DEBUG_PRINTF("DynamicLoaderDarwin::%s(%s)\n", __FUNCTION__,
                StateAsCString(state));
   switch (state) {
+  case kNumStateType:
   case eStateConnected:
   case eStateAttaching:
   case eStateLaunching:
Index: source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===================================================================
--- source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -1475,6 +1475,7 @@
   DEBUG_PRINTF("DynamicLoaderDarwinKernel::%s(%s)\n", __FUNCTION__,
                StateAsCString(state));
   switch (state) {
+  case kNumStateType:
   case eStateConnected:
   case eStateAttaching:
   case eStateLaunching:
Index: source/Interpreter/CommandObject.cpp
===================================================================
--- source/Interpreter/CommandObject.cpp
+++ source/Interpreter/CommandObject.cpp
@@ -221,6 +221,7 @@
     } else {
       StateType state = process->GetState();
       switch (state) {
+      case kNumStateType:
       case eStateInvalid:
       case eStateSuspended:
       case eStateCrashed:
Index: scripts/Python/python-typemaps.swig
===================================================================
--- scripts/Python/python-typemaps.swig
+++ scripts/Python/python-typemaps.swig
@@ -77,6 +77,26 @@
   }
 }
 
+%typemap(in) lldb::StateType {
+  using namespace lldb_private;
+  if (PythonInteger::Check($input))
+  {
+    PythonInteger py_int(PyRefType::Borrowed, $input);
+    int64_t state_type_value = py_int.GetInteger() ;
+
+    if (state_type_value >= lldb::StateType::kNumStateType) {
+      PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
+      return nullptr ;
+    }
+    $1 = static_cast<lldb::StateType>(state_type_value);
+  }
+  else
+  {
+    PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+    return nullptr;
+  }
+}
+
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
Index: include/lldb/lldb-enumerations.h
===================================================================
--- include/lldb/lldb-enumerations.h
+++ include/lldb/lldb-enumerations.h
@@ -54,9 +54,10 @@
   eStateCrashed,   ///< Process or thread has crashed and can be examined.
   eStateDetached,  ///< Process has been detached and can't be examined.
   eStateExited,    ///< Process has exited and can't be examined.
-  eStateSuspended  ///< Process or thread is in a suspended state as far
+  eStateSuspended, ///< Process or thread is in a suspended state as far
                    ///< as the debugger is concerned while other processes
                    ///< or threads get the chance to run.
+  kNumStateType
 };
 
 //----------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to