implement gdb-set output-radix

[ki.stfu]

Summary

Patch from [e-mail]

[abidh]

Looks ok apart from one change that seems like a different issue. So please remove that part. If will be good to add a test case too.

tools/lldb-mi/MICmdCmdVar.cpp

1064 ↗

(On Diff #22232)

This change does not seem related to the rest of the patch. It will be better to submit it separately.

[ki.stfu]

tools/lldb-mi/MICmdCmdVar.cpp

1060 ↗

(On Diff #22232)

} newline
else newline
{

1064 ↗

(On Diff #22232)

agree. please create separate review for it.

[ChuckR]

removed extraneous fix. Can somebody please check this in on my behalf?

[ki.stfu]

I agree with @abidh, that we should add tests for this. Could you do it?

[ChuckR]

In D8430#143454, @ki.stfu wrote:

I agree with @abidh, that we should add tests for this. Could you do it?

Is there any documentation/guidance on adding tests? Or lldb/lldb-mi's testing infrastructure in general?

[abidh]

In D8430#143459, @ChuckR wrote:

In D8430#143454, @ki.stfu wrote:

I agree with @abidh, that we should add tests for this. Could you do it?

Is there any documentation/guidance on adding tests? Or lldb/lldb-mi's testing infrastructure in general?

Please look in the test/tools/lldb-mi and you will see already present tests.

[ki.stfu]

In D8430#143459, @ChuckR wrote:

In D8430#143454, @ki.stfu wrote:

I agree with @abidh, that we should add tests for this. Could you do it?

Is there any documentation/guidance on adding tests? Or lldb/lldb-mi's testing infrastructure in general?

I'll add tests for -gdb-set/-gdb-show soon (it still on review D8566). So please add your tests into MiGdbSetShowTestCase like I did.

[ChuckR]

added test

[ChuckR]

All lld-mi tests passing.

[ki.stfu]

Looks good apart the following: move your test to MiGdbSetShowTestCase, please.

test/tools/lldb-mi/variable/TestMiVar.py

192 ↗

(On Diff #23194)

Move this test to test/tools/lldb-mi/TestMiGdbSetShow.py

[ChuckR]

moved test

[ChuckR]

Can somebody check this in for me now that it has been approved?

[ki.stfu]

In D8430#152217, @ChuckR wrote:

Can somebody check this in for me now that it has been approved?

Yes, I can do that.

[ki.stfu]

Rebase this against ToT

lldb/trunk/test/tools/lldb-mi/TestMiGdbSetShow.py

def test_lldbmi_gdb_show_unknown(self):
         """Test that 'lldb-mi --interpreter' fails when showing an unknown property."""
 
         self.spawnLldbMi(args = None)
 
         # Test that -gdb-show fails if property is unknown
         self.runCmd("-gdb-show unknown")
         self.expect("\^error")
 
+
+    @lldbmi_test
+    @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
+    @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    @skipIfLinux # llvm.org/pr22841: lldb-mi tests fail on all Linux buildbots
+    def test_lldbmi_gdb_set_ouptut_radix(self):
+        """Test that 'lldb-mi --interpreter' works for -gdb-set output-radix."""
+
+        self.spawnLldbMi(args = None)
+
+        # Load executable
+        self.runCmd("-file-exec-and-symbols %s" % self.myexe)
+        self.expect("\^done")
+
+        # Run to BP_printf
+        line = line_number('main.cpp', '// BP_printf')
+        self.runCmd("-break-insert main.cpp:%d" % line)
+        self.expect("\^done,bkpt={number=\"1\"")
+        self.runCmd("-exec-run")
+        self.expect("\^running");
+        self.expect("\*stopped,reason=\"breakpoint-hit\"")
+
+        # Setup variable
+        self.runCmd("-var-create var_a * a");
+        self.expect("\^done,name=\"var_a\",numchild=\"0\",value=\"10\",type=\"int\",thread-id=\"1\",has_more=\"0\"")
+
+        # Test default output
+        self.runCmd("-var-evaluate-expression var_a");
+        self.expect("\^done,value=\"10\"");
+
+        # Test hex output
+        self.runCmd("-gdb-set output-radix 16");
+        self.expect("\^done");
+        self.runCmd("-var-evaluate-expression var_a");
+        self.expect("\^done,value=\"0xa\"");
+
+        # Test octal output
+        self.runCmd("-gdb-set output-radix 8");
+        self.expect("\^done");
+        self.runCmd("-var-evaluate-expression var_a");
+        self.expect("\^done,value=\"012\"");
+
+        # Test decimal output
+        self.runCmd("-gdb-set output-radix 10");
+        self.expect("\^done");
+        self.runCmd("-var-evaluate-expression var_a");
+        self.expect("\^done,value=\"10\"");
+
 if __name__ == '__main__':
     unittest2.main()

lldb/trunk/test/tools/lldb-mi/main.cpp

 //===-- main.cpp ------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include <cstdio>
 
 int
 main(int argc, char const *argv[])
 {
-    printf("argc=%d\n", argc);
+    int a  = 10;
+
+    printf("argc=%d\n", argc); // BP_printf
     return 0;
 }

lldb/trunk/tools/lldb-mi/MICmdCmdGdbSet.h

private:
     typedef bool (CMICmdCmdGdbSet::*FnGdbOptionPtr)(const CMIUtilString::VecString_t &vrWords);
     typedef std::map<CMIUtilString, FnGdbOptionPtr> MapGdbOptionNameToFnGdbOptionPtr_t;
 
     // Methods:
   private:
     bool GetOptionFn(const CMIUtilString &vrGdbOptionName, FnGdbOptionPtr &vrwpFn) const;
     bool OptionFnTargetAsync(const CMIUtilString::VecString_t &vrWords);
     bool OptionFnSolibSearchPath(const CMIUtilString::VecString_t &vrWords);
+    bool OptionFnOutputRadix(const CMIUtilString::VecString_t &vrWords);
     bool OptionFnFallback(const CMIUtilString::VecString_t &vrWords);
 
     // Attributes:
   private:
     const static MapGdbOptionNameToFnGdbOptionPtr_t ms_mapGdbOptionNameToFnGdbOptionPtr;
     //
     const CMIUtilString m_constStrArgNamedThreadGrp;
     const CMIUtilString m_constStrArgNamedGdbOption;
     bool m_bGdbOptionRecognised;   // True = This command has a function with a name that matches the Print argument, false = not found
     bool m_bGdbOptionFnSuccessful; // True = The print function completed its task ok, false = function failed for some reason
     bool m_bGbbOptionFnHasError;   // True = The option function has an error condition (not the command!), false = option function ok.
     CMIUtilString m_strGdbOptionName;
     CMIUtilString m_strGdbOptionFnError;
 };

lldb/trunk/tools/lldb-mi/MICmdCmdGdbSet.cpp

 #include "MICmdArgValListOfN.h"
 #include "MICmdArgValOptionLong.h"
 #include "MICmnLLDBDebugSessionInfo.h"
 
 // Instantiations:
 const CMICmdCmdGdbSet::MapGdbOptionNameToFnGdbOptionPtr_t CMICmdCmdGdbSet::ms_mapGdbOptionNameToFnGdbOptionPtr = {
     {"target-async", &CMICmdCmdGdbSet::OptionFnTargetAsync},
     // { "auto-solib-add", &CMICmdCmdGdbSet::OptionFnAutoSolibAdd },    // Example code if need to implement GDB set other options
+    {"output-radix", &CMICmdCmdGdbSet::OptionFnOutputRadix},
     {"solib-search-path", &CMICmdCmdGdbSet::OptionFnSolibSearchPath},
     {"fallback", &CMICmdCmdGdbSet::OptionFnFallback}};
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMICmdCmdGdbSet constructor.
 // Type:    Method.
 // Args:    None.
 // Return:  None.
if (!m_rLLDBDebugSessionInfo.SharedDataAdd<CMIUtilString>(rStrKeySolibPath, rStrValSolibPath))
         SetError(CMIUtilString::Format(MIRSRC(IDS_DBGSESSION_ERR_SHARED_DATA_ADD), m_cmdData.strMiCmd.c_str(), rStrKeySolibPath.c_str()));
         return MIstatus::failure;
     }
 
     return MIstatus::success;
 }
 
 //++ ------------------------------------------------------------------------------------
+// Details: Carry out work to complete the GDB set option 'output-radix' to prepare
+//          and send back information asked for.
+// Type:    Method.
+// Args:    vrWords - (R) List of additional parameters used by this option.
+// Return:  MIstatus::success - Functional succeeded.
+//          MIstatus::failure - Functional failed.
+// Throws:  None.
+//--
+bool
+CMICmdCmdGdbSet::OptionFnOutputRadix(const CMIUtilString::VecString_t &vrWords)
+{
+    // Check we have at least one argument
+    if (vrWords.size() < 1)
+    {
+        m_bGbbOptionFnHasError = true;
+        m_strGdbOptionFnError = MIRSRC(IDS_CMD_ERR_GDBSET_OPT_SOLIBSEARCHPATH);
+        return MIstatus::failure;
+    }
+    const CMIUtilString &rStrValOutputRadix(vrWords[0]);
+    
+    CMICmnLLDBDebugSessionInfoVarObj::varFormat_e  format = CMICmnLLDBDebugSessionInfoVarObj::eVarFormat_Invalid;
+    MIint64 radix;
+    if (rStrValOutputRadix.ExtractNumber(radix))
+    {
+        switch (radix)
+        {
+        case 8:
+            format = CMICmnLLDBDebugSessionInfoVarObj::eVarFormat_Octal;
+            break;
+        case 10:
+            format = CMICmnLLDBDebugSessionInfoVarObj::eVarFormat_Natural;
+            break;
+        case 16:
+            format = CMICmnLLDBDebugSessionInfoVarObj::eVarFormat_Hex;
+            break;
+        default:
+            format = CMICmnLLDBDebugSessionInfoVarObj::eVarFormat_Invalid;
+            break;
+        }
+    }
+    if (format == CMICmnLLDBDebugSessionInfoVarObj::eVarFormat_Invalid)
+    {
+        m_bGbbOptionFnHasError = false;
+        SetError(CMIUtilString::Format(MIRSRC(IDS_DBGSESSION_ERR_SHARED_DATA_ADD), m_cmdData.strMiCmd.c_str(), "Output Radix"));
+        return MIstatus::failure;
+    }
+    CMICmnLLDBDebugSessionInfoVarObj::VarObjSetFormat(format);
+    
+    return MIstatus::success;
+}
+
+//++ ------------------------------------------------------------------------------------
 // Details: Carry out work to complete the GDB set option to prepare and send back the
 //          requested information.
 // Type:    Method.
 // Args:    None.
 // Return:  MIstatus::success - Functional succeeded.
 //          MIstatus::failure - Functional failed.
 // Throws:  None.
 //--

lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfoVarObj.h

public:
     static void VarObjAdd(const CMICmnLLDBDebugSessionInfoVarObj &vrVarObj);
     static void VarObjDelete(const CMIUtilString &vrVarName);
     static bool VarObjGet(const CMIUtilString &vrVarName, CMICmnLLDBDebugSessionInfoVarObj &vrwVarObj);
     static void VarObjUpdate(const CMICmnLLDBDebugSessionInfoVarObj &vrVarObj);
     static void VarObjIdInc(void);
     static MIuint VarObjIdGet(void);
     static void VarObjIdResetToZero(void);
     static void VarObjClear(void);
+    static void VarObjSetFormat(varFormat_e eDefaultFormat);
 
     // Methods:
   public:
     /* ctor */ CMICmnLLDBDebugSessionInfoVarObj(void);
     /* ctor */ CMICmnLLDBDebugSessionInfoVarObj(const CMIUtilString &vrStrNameReal, const CMIUtilString &vrStrName,
                                                 const lldb::SBValue &vrValue);
     /* ctor */ CMICmnLLDBDebugSessionInfoVarObj(const CMIUtilString &vrStrNameReal, const CMIUtilString &vrStrName,
                                                 const lldb::SBValue &vrValue, const CMIUtilString &vrStrVarObjParentName);
private:
     bool MoveOther(CMICmnLLDBDebugSessionInfoVarObj &vrwOther);
 
     // Attributes:
   private:
     static const MIchar *ms_aVarFormatStrings[];
     static const MIchar *ms_aVarFormatChars[];
     static MapKeyToVarObj_t ms_mapVarIdToVarObj;
     static MIuint ms_nVarUniqueId;
+    static varFormat_e ms_eDefaultFormat;    // overrides "natural" format
     //
     // *** Upate the copy move constructors and assignment operator ***
     varFormat_e m_eVarFormat;
     varType_e m_eVarType;
     CMIUtilString m_strName;
     lldb::SBValue m_SBValue;
     CMIUtilString m_strNameReal;
     CMIUtilString m_strFormattedValue;
     CMIUtilString m_strVarObjParentName;
     // *** Upate the copy move constructors and assignment operator ***
 };

lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfoVarObj.cpp

const MIchar *CMICmnLLDBDebugSessionInfoVarObj::ms_aVarFormatStrings[] = {
     // *** Order is import here.
     "<Invalid var format>", "binary", "octal", "decimal", "hexadecimal", "natural"};
 const MIchar *CMICmnLLDBDebugSessionInfoVarObj::ms_aVarFormatChars[] = {
     // CODETAG_SESSIONINFO_VARFORMAT_ENUM
     // *** Order is import here.
     "<Invalid var format>", "t", "o", "d", "x", "N"};
 CMICmnLLDBDebugSessionInfoVarObj::MapKeyToVarObj_t CMICmnLLDBDebugSessionInfoVarObj::ms_mapVarIdToVarObj;
 MIuint CMICmnLLDBDebugSessionInfoVarObj::ms_nVarUniqueId = 0; // Index from 0
+CMICmnLLDBDebugSessionInfoVarObj::varFormat_e CMICmnLLDBDebugSessionInfoVarObj::ms_eDefaultFormat = eVarFormat_Natural;
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMICmnLLDBDebugSessionInfoVarObj constructor.
 // Type:    Method.
 // Args:    None.
 // Return:  None.
 // Throws:  None.
 //--

 // Returns: CMIUtilString       - Numerical formatted string.
 // Throws:  None.
 //--
 CMIUtilString
 CMICmnLLDBDebugSessionInfoVarObj::GetStringFormatted(const MIuint64 vnValue, const MIchar *vpStrValueNatural,
                                                      const CMICmnLLDBDebugSessionInfoVarObj::varFormat_e veVarFormat)
 {
     CMIUtilString strFormattedValue;
+    CMICmnLLDBDebugSessionInfoVarObj::varFormat_e veFormat = veVarFormat;
+    if (ms_eDefaultFormat != eVarFormat_Invalid && veVarFormat == eVarFormat_Natural)
+    {
+        veFormat = ms_eDefaultFormat;
+    }
 
-    switch (veVarFormat)
+    switch (veFormat)
     {
         case eVarFormat_Binary:
             strFormattedValue = CMIUtilString::FormatBinary(vnValue);
             break;
         case eVarFormat_Octal:
             strFormattedValue = CMIUtilString::Format("0%llo", vnValue);
             break;
         case eVarFormat_Decimal:

 //--
 void
 CMICmnLLDBDebugSessionInfoVarObj::VarObjIdResetToZero(void)
 {
     ms_nVarUniqueId = 0;
 }
 
 //++ ------------------------------------------------------------------------------------
+// Details: Default format is globally used as the data format when "natural" is in effect, that is, this overrides the default
+// Type:    Static method.
+// Args:    None.
+// Returns: None.
+// Throws:  None.
+//--
+void
+CMICmnLLDBDebugSessionInfoVarObj::VarObjSetFormat(varFormat_e eDefaultFormat)
+{
+    ms_eDefaultFormat = eDefaultFormat;
+}
+
+
+//++ ------------------------------------------------------------------------------------
 // Details: A count is kept of the number of var value objects created. This is count is
 //          used to ID the var value object. Increment the count by 1.
 // Type:    Static method.
 // Args:    None.
 // Returns: None.
 // Throws:  None.
 //--
 void