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