This is an archive of the discontinued LLVM Phabricator instance.

Differential D18880

-thread-info in lldbmi does not conform to protocol. Should end with current thread id
ClosedPublic

Authored by jacdavis on Apr 7 2016, 4:37 PM.

Details

Reviewers
lldb-commits
abidh
zturner
Summary

-thread-info in lldbmi does not conform to protocol. Should end with current thread id as described here: https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Thread-Commands.html#GDB_002fMI-Thread-Commands

When printing all threads, the current thread id should be printed afterwards.

Example:
-thread-info

^done,threads=[
{id="2",target-id="Thread 0xb7e14b90 (LWP 21257)",
   frame={level="0",addr="0xffffe410",func="__kernel_vsyscall",
           args=[]},state="running"},
{id="1",target-id="Thread 0xb7e156b0 (LWP 21254)",
   frame={level="0",addr="0x0804891f",func="foo",
           args=[{name="i",value="10"}],
           file="/tmp/a.c",fullname="/tmp/a.c",line="158"},
           state="running"}],
current-thread-id="1"
(gdb)

Diff Detail

Event Timeline

jacdavis updated this revision to Diff 52977.Apr 7 2016, 4:37 PM
jacdavis retitled this revision from to -thread-info in lldbmi does not conform to protocol. Should end with current thread id.
jacdavis updated this object.
jacdavis added reviewers: lldb-commits, abidh.
zturner added a subscriber: zturner.Apr 7 2016, 4:45 PM

Hi Jackson,

Two suggestions:

  1. Please clang-format the patch. Let me know if you need help getting this working.
  2. use C++11 threads instead of pthreads. This makes the tests portable to Windows. Even if the test still fails on Windows for other reasons, at least it's closer.
jacdavis updated this revision to Diff 52991.Apr 7 2016, 6:15 PM

Updated to use C++ 11 threads. Clang-format tried to format the diff in the llvm style. Since this is all lldb code, is that actually what is expected? None of the braces matched etc....

zturner added a comment.Apr 7 2016, 6:21 PM

We have an lldb style file in llvm/tools/lldb, it should be picked up
automatically if you run it from the lldb directory. I'm not sure though if
it's based on the folder you're in when you run clang-format or by
searching up the tree from the target file until it finds a rule file

jacdavis updated this revision to Diff 53028.Apr 8 2016, 8:39 AM

Ran formatting tool with lldb format. pthread linker option is still required for C++11 threads so left it in. New test still disabled for windows.

jacdavis updated this revision to Diff 53031.Apr 8 2016, 8:54 AM

last diff missed lldb-mi changes

zturner accepted this revision.Apr 8 2016, 10:03 AM
zturner added a reviewer: zturner.

Looks fine to me. TBH I haven't seen any of the lldb-mi owners around in months, so I'm not sure if they're still active

This revision is now accepted and ready to land.Apr 8 2016, 10:03 AM
ChuckR closed this revision.Apr 8 2016, 3:25 PM
ChuckR added a subscriber: ChuckR.

closed with http://reviews.llvm.org/rL265858

abidh edited edge metadata.Apr 13 2016, 4:41 AM
In D18880#395530, @zturner wrote:

Looks fine to me. TBH I haven't seen any of the lldb-mi owners around in months, so I'm not sure if they're still active

Sorry for late reply. I have been busy lately but keep an eye on lldb-mi patches.

Revision Contents

PathSize
packages/
Python/
lldbsuite/
test/
tools/
lldb-mi/
threadinfo/
7 lines
39 lines
21 lines
tools/
lldb-mi/
4 lines
23 lines

Diff 53031

packages/Python/lldbsuite/test/tools/lldb-mi/threadinfo/Makefile

LEVEL = ../../../make
LDFLAGS=-pthread
CXX_SOURCES := test_threadinfo.cpp
include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/tools/lldb-mi/threadinfo/TestMiThreadInfo.py

"""
Test lldb-mi -thread-info command.
"""
from __future__ import print_function
import lldbmi_testcase
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
class MiThreadInfoTestCase(lldbmi_testcase.MiTestCaseBase):
mydir = TestBase.compute_mydir(__file__)
@skipIfWindows # pthreads not supported on Windows
@skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
def test_lldbmi_thread_info(self):
"""Test that -thread-info prints thread info and the current-thread-id"""
self.spawnLldbMi(args = None)
# Load executable
self.runCmd("-file-exec-and-symbols %s" % self.myexe)
self.expect("\^done")
self.runCmd("-break-insert ThreadProc")
self.expect("\^done")
# Run to the breakpoint
self.runCmd("-exec-run")
self.expect("\^running")
self.expect("\*stopped,reason=\"breakpoint-hit\"")
self.runCmd("-thread-info")
self.expect("\^done,threads=\[\{id=\"1\",(.*)\},\{id=\"2\",(.*)\],current-thread-id=\"2\"")
self.runCmd("-gdb-quit")

packages/Python/lldbsuite/test/tools/lldb-mi/threadinfo/test_threadinfo.cpp

#include <cstdlib>
#include <iostream>
#include <thread>
using namespace std;
void
ThreadProc()
{
int i = 0;
i++;
}
int
main()
{
thread t(ThreadProc);
t.join();
return 0;
}

tools/lldb-mi/MICmdCmdThread.h

Context not available.
bool m_bThreadInvalid; // True = invalid, false = ok bool m_bThreadInvalid; // True = invalid, false = ok
VecMIValueTuple_t m_vecMIValueTuple; VecMIValueTuple_t m_vecMIValueTuple;
const CMIUtilString m_constStrArgNamedThreadId; const CMIUtilString m_constStrArgNamedThreadId;
// mi value of current-thread-id if multiple threads are requested
bool m_bHasCurrentThread;
CMICmnMIValue m_miValueCurrThreadId;
}; };
Context not available.

tools/lldb-mi/MICmdCmdThread.cpp

Context not available.
// Throws: None. // Throws: None.
//-- //--
CMICmdCmdThreadInfo::CMICmdCmdThreadInfo() CMICmdCmdThreadInfo::CMICmdCmdThreadInfo()
: m_bSingleThread(false) : m_bSingleThread(false),
, m_bThreadInvalid(true) m_bThreadInvalid(true),
, m_constStrArgNamedThreadId("thread-id") m_constStrArgNamedThreadId("thread-id"),
m_bHasCurrentThread(false)
{ {
// Command factory matches this name with that received from the stdin stream // Command factory matches this name with that received from the stdin stream
m_strMiCmd = "thread-info"; m_strMiCmd = "thread-info";
Context not available.
} }
} }
// -thread-info with multiple threads ends with the current thread id if any
if (thread.IsValid())
{
const CMIUtilString strId(CMIUtilString::Format("%d", thread.GetIndexID()));
CMICmnMIValueConst miValueCurrThreadId(strId);
m_miValueCurrThreadId = miValueCurrThreadId;
m_bHasCurrentThread = true;
}
return MIstatus::success; return MIstatus::success;
} }
Context not available.
++it; ++it;
} }
const CMICmnMIValueResult miValueResult("threads", miValueList); CMICmnMIValueResult miValueResult("threads", miValueList);
if (m_bHasCurrentThread)
{
CMIUtilString strCurrThreadId = "current-thread-id";
miValueResult.Add(strCurrThreadId, m_miValueCurrThreadId);
}
const CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Done, miValueResult); const CMICmnMIResultRecord miRecordResult(m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Done, miValueResult);
m_miResultRecord = miRecordResult; m_miResultRecord = miRecordResult;
Context not available.