Skip to content
This repository was archived by the owner on May 21, 2019. It is now read-only.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c71899e

Browse files
author
Greg Clayton
committedJan 18, 2011
Thread safety changes in debugserver and also in the process GDB remote plugin.
I added support for asking if the GDB remote server supports thread suffixes for packets that should be thread specific (register read/write packets) because the way the GDB remote protocol does it right now is to have a notion of a current thread for register and memory reads/writes (set via the "$Hg%x" packet) and a current thread for running ("$Hc%x"). Now we ask the remote GDB server if it supports adding the thread ID to the register packets and we enable that feature in LLDB if supported. This stops us from having to send a bunch of packets that update the current thread ID to some value which is prone to error, or extra packets. git-svn-id: https://llvm.org/svn/llvm-project/llvdb/trunk@123762 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 6c9662e commit c71899e

17 files changed

+301
-172
lines changed
 

‎source/Expression/ClangUserExpression.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include "lldb/Core/ConstString.h"
2222
#include "lldb/Core/Log.h"
23+
#include "lldb/Core/StreamFile.h"
2324
#include "lldb/Core/StreamString.h"
2425
#include "lldb/Core/ValueObjectConstResult.h"
2526
#include "lldb/Expression/ASTResultSynthesizer.h"
@@ -365,6 +366,12 @@ ClangUserExpression::PrepareToExecuteJITExpression (Stream &error_stream,
365366
error_stream.Printf("Couldn't materialize struct: %s\n", materialize_error.AsCString());
366367
return false;
367368
}
369+
370+
#if 0
371+
// jingham: look here
372+
StreamFile logfile ("/tmp/exprs.txt", "a");
373+
logfile.Printf("0x%16.16llx: thread = 0x%4.4x, expr = '%s'\n", m_jit_addr, exe_ctx.thread ? exe_ctx.thread->GetID() : -1, m_expr_text.c_str());
374+
#endif
368375

369376
if (log)
370377
{

‎source/Expression/ClangUtilityFunction.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "lldb/Core/ConstString.h"
1919
#include "lldb/Core/Stream.h"
20+
#include "lldb/Core/StreamFile.h"
2021
#include "lldb/Expression/ClangExpressionDeclMap.h"
2122
#include "lldb/Expression/ClangExpressionParser.h"
2223
#include "lldb/Expression/ClangUtilityFunction.h"
@@ -124,6 +125,15 @@ ClangUtilityFunction::Install (Stream &error_stream,
124125

125126
Error jit_error = parser.MakeJIT (m_jit_begin, m_jit_end, exe_ctx);
126127

128+
#if 0
129+
// jingham: look here
130+
StreamFile logfile ("/tmp/exprs.txt", "a");
131+
logfile.Printf ("0x%16.16llx: func = %s, source =\n%s\n",
132+
m_jit_begin,
133+
m_function_name.c_str(),
134+
m_function_text.c_str());
135+
#endif
136+
127137
m_expr_decl_map->DidParse();
128138

129139
m_expr_decl_map.reset();

‎source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ using namespace lldb_private;
3434
GDBRemoteCommunication::GDBRemoteCommunication() :
3535
Communication("gdb-remote.packets"),
3636
m_send_acks (true),
37+
m_thread_suffix_supported (false),
3738
m_rx_packet_listener ("gdbremote.rx_packet"),
3839
m_sequence_mutex (Mutex::eMutexTypeRecursive),
3940
m_is_running (false),

‎source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,18 @@ class GDBRemoteCommunication :
9999
m_send_acks = enabled;
100100
}
101101

102+
bool
103+
GetThreadSuffixSupported () const
104+
{
105+
return m_thread_suffix_supported;
106+
}
107+
108+
void
109+
SetThreadSuffixSupported (bool enabled)
110+
{
111+
m_thread_suffix_supported = enabled;
112+
}
113+
102114
bool
103115
SendAsyncSignal (int signo);
104116

@@ -244,7 +256,8 @@ class GDBRemoteCommunication :
244256
//------------------------------------------------------------------
245257
// Classes that inherit from GDBRemoteCommunication can see and modify these
246258
//------------------------------------------------------------------
247-
bool m_send_acks;
259+
bool m_send_acks:1,
260+
m_thread_suffix_supported:1;
248261
lldb_private::Listener m_rx_packet_listener;
249262
lldb_private::Mutex m_sequence_mutex; // Restrict access to sending/receiving packets to a single thread at a time
250263
lldb_private::Predicate<bool> m_is_running;

‎source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -217,15 +217,19 @@ GDBRemoteRegisterContext::ReadRegisterBytes (uint32_t reg, DataExtractor &data)
217217
Mutex::Locker locker;
218218
if (gdb_comm.GetSequenceMutex (locker))
219219
{
220-
if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
220+
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
221+
if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
221222
{
222-
char packet[32];
223+
char packet[64];
223224
StringExtractorGDBRemote response;
224-
int packet_len;
225+
int packet_len = 0;
225226
if (m_read_all_at_once)
226227
{
227228
// Get all registers in one packet
228-
packet_len = ::snprintf (packet, sizeof(packet), "g");
229+
if (thread_suffix_supported)
230+
packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4x;", m_thread.GetID());
231+
else
232+
packet_len = ::snprintf (packet, sizeof(packet), "g");
229233
assert (packet_len < (sizeof(packet) - 1));
230234
if (gdb_comm.SendPacketAndWaitForResponse(packet, response, 1, false))
231235
{
@@ -237,7 +241,10 @@ GDBRemoteRegisterContext::ReadRegisterBytes (uint32_t reg, DataExtractor &data)
237241
else
238242
{
239243
// Get each register individually
240-
packet_len = ::snprintf (packet, sizeof(packet), "p%x", reg);
244+
if (thread_suffix_supported)
245+
packet_len = ::snprintf (packet, sizeof(packet), "p%x;thread:%4.4x;", reg, m_thread.GetID());
246+
else
247+
packet_len = ::snprintf (packet, sizeof(packet), "p%x", reg);
241248
assert (packet_len < (sizeof(packet) - 1));
242249
if (gdb_comm.SendPacketAndWaitForResponse(packet, response, 1, false))
243250
PrivateSetRegisterValue (reg, response);
@@ -319,7 +326,8 @@ GDBRemoteRegisterContext::WriteRegisterBytes (uint32_t reg, DataExtractor &data,
319326
Mutex::Locker locker;
320327
if (gdb_comm.GetSequenceMutex (locker))
321328
{
322-
if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
329+
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
330+
if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
323331
{
324332
uint32_t offset, end_offset;
325333
StreamString packet;
@@ -336,6 +344,9 @@ GDBRemoteRegisterContext::WriteRegisterBytes (uint32_t reg, DataExtractor &data,
336344
eByteOrderHost,
337345
eByteOrderHost);
338346

347+
if (thread_suffix_supported)
348+
packet.Printf (";thread:%4.4x;", m_thread.GetID());
349+
339350
// Invalidate all register values
340351
InvalidateIfNeeded (true);
341352

@@ -361,6 +372,9 @@ GDBRemoteRegisterContext::WriteRegisterBytes (uint32_t reg, DataExtractor &data,
361372
eByteOrderHost,
362373
eByteOrderHost);
363374

375+
if (thread_suffix_supported)
376+
packet.Printf (";thread:%4.4x;", m_thread.GetID());
377+
364378
// Invalidate just this register
365379
m_reg_valid[reg] = false;
366380
if (gdb_comm.SendPacketAndWaitForResponse(packet.GetString().c_str(),
@@ -391,16 +405,31 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
391405
Mutex::Locker locker;
392406
if (gdb_comm.GetSequenceMutex (locker))
393407
{
394-
if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
408+
char packet[32];
409+
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
410+
if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
395411
{
396-
if (gdb_comm.SendPacketAndWaitForResponse("g", response, 1, false))
412+
int packet_len = 0;
413+
if (thread_suffix_supported)
414+
packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4x", m_thread.GetID());
415+
else
416+
packet_len = ::snprintf (packet, sizeof(packet), "g");
417+
assert (packet_len < (sizeof(packet) - 1));
418+
419+
if (gdb_comm.SendPacketAndWaitForResponse(packet, packet_len, response, 1, false))
397420
{
398421
if (response.IsErrorPacket())
399422
return false;
400-
423+
401424
response.GetStringRef().insert(0, 1, 'G');
402-
data_sp.reset (new DataBufferHeap(response.GetStringRef().c_str(),
403-
response.GetStringRef().size()));
425+
if (thread_suffix_supported)
426+
{
427+
char thread_id_cstr[64];
428+
::snprintf (thread_id_cstr, sizeof(thread_id_cstr), ";thread:%4.4x;", m_thread.GetID());
429+
response.GetStringRef().append (thread_id_cstr);
430+
}
431+
data_sp.reset (new DataBufferHeap (response.GetStringRef().c_str(),
432+
response.GetStringRef().size()));
404433
return true;
405434
}
406435
}
@@ -419,7 +448,8 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data
419448
Mutex::Locker locker;
420449
if (gdb_comm.GetSequenceMutex (locker))
421450
{
422-
if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
451+
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
452+
if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
423453
{
424454
if (gdb_comm.SendPacketAndWaitForResponse((const char *)data_sp->GetBytes(),
425455
data_sp->GetByteSize(),

‎source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,13 @@ ProcessGDBRemote::ConnectToDebugserver (const char *host_port)
554554
if (response.IsOKPacket())
555555
m_gdb_comm.SetAckMode (false);
556556
}
557+
558+
if (m_gdb_comm.SendPacketAndWaitForResponse("QThreadSuffixSupported", response, 1, false))
559+
{
560+
if (response.IsOKPacket())
561+
m_gdb_comm.SetThreadSuffixSupported (true);
562+
}
563+
557564
}
558565
return error;
559566
}
@@ -2021,7 +2028,7 @@ ProcessGDBRemote::SetCurrentGDBRemoteThreadForRun (int tid)
20212028
return true;
20222029

20232030
char packet[32];
2024-
const int packet_len = ::snprintf (packet, sizeof(packet), "Hg%x", tid);
2031+
const int packet_len = ::snprintf (packet, sizeof(packet), "Hc%x", tid);
20252032
assert (packet_len + 1 < sizeof(packet));
20262033
StringExtractorGDBRemote response;
20272034
if (m_gdb_comm.SendPacketAndWaitForResponse(packet, packet_len, response, 2, false))

‎source/Target/ThreadList.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ ThreadList::ShouldReportRun (Event *event_ptr)
340340
void
341341
ThreadList::Clear()
342342
{
343+
Mutex::Locker locker(m_threads_mutex);
343344
m_stop_id = 0;
344345
m_threads.clear();
345346
m_selected_tid = LLDB_INVALID_THREAD_ID;
@@ -504,6 +505,7 @@ ThreadList::WillResume ()
504505
void
505506
ThreadList::DidResume ()
506507
{
508+
Mutex::Locker locker(m_threads_mutex);
507509
collection::iterator pos, end = m_threads.end();
508510
for (pos = m_threads.begin(); pos != end; ++pos)
509511
{

‎source/Utility/StringExtractor.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,14 @@ class StringExtractor
113113
size_t
114114
GetHexByteString (std::string &str);
115115

116+
const char *
117+
Peek ()
118+
{
119+
if (m_index < m_packet.size())
120+
return m_packet.c_str() + m_index;
121+
return NULL;
122+
}
123+
116124
protected:
117125
//------------------------------------------------------------------
118126
// For StringExtractor only

‎tools/debugserver/source/DNBThreadResumeActions.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ class DNBThreadResumeActions
8585
{
8686
return m_actions.size();
8787
}
88+
89+
void
90+
Clear()
91+
{
92+
m_actions.clear();
93+
m_signal_handled.clear();
94+
}
8895

8996
protected:
9097
std::vector<DNBThreadResumeAction> m_actions;

‎tools/debugserver/source/MacOSX/MachProcess.cpp

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ MachProcess::MachProcess() :
101101
m_stdio_thread (0),
102102
m_stdio_mutex (PTHREAD_MUTEX_RECURSIVE),
103103
m_stdout_data (),
104+
m_thread_actions (),
104105
m_thread_list (),
105106
m_exception_messages (),
106107
m_exception_messages_mutex (PTHREAD_MUTEX_RECURSIVE),
@@ -314,7 +315,8 @@ MachProcess::Resume (const DNBThreadResumeActions& thread_actions)
314315

315316
if (CanResume(state))
316317
{
317-
PrivateResume(thread_actions);
318+
m_thread_actions = thread_actions;
319+
PrivateResume();
318320
return true;
319321
}
320322
else if (state == eStateRunning)
@@ -337,7 +339,8 @@ MachProcess::Kill (const struct timespec *timeout_abstime)
337339
DNBError err;
338340
err.SetErrorToErrno();
339341
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Kill() DoSIGSTOP() ::ptrace (PT_KILL, pid=%u, 0, 0) => 0x%8.8x (%s)", err.Error(), err.AsString());
340-
PrivateResume (DNBThreadResumeActions (eStateRunning, 0));
342+
m_thread_actions = DNBThreadResumeActions (eStateRunning, 0);
343+
PrivateResume ();
341344
return true;
342345
}
343346

@@ -392,7 +395,8 @@ MachProcess::DoSIGSTOP (bool clear_bps_and_wps, uint32_t *thread_idx_ptr)
392395
// No threads were stopped with a SIGSTOP, we need to run and halt the
393396
// process with a signal
394397
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::DoSIGSTOP() state = %s -- resuming process", DNBStateAsString (state));
395-
PrivateResume (DNBThreadResumeActions (eStateRunning, 0));
398+
m_thread_actions = DNBThreadResumeActions (eStateRunning, 0);
399+
PrivateResume ();
396400

397401
// Reset the event that says we were indeed running
398402
m_events.ResetEvents(eEventProcessRunningStateChanged);
@@ -428,20 +432,19 @@ MachProcess::Detach()
428432
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Detach() DoSIGSTOP() returned %s", DNBStateAsString(state));
429433

430434
{
431-
DNBThreadResumeActions thread_actions;
435+
m_thread_actions.Clear();
432436
DNBThreadResumeAction thread_action;
433437
thread_action.tid = m_thread_list.ThreadIDAtIndex (thread_idx);
434438
thread_action.state = eStateRunning;
435439
thread_action.signal = -1;
436440
thread_action.addr = INVALID_NUB_ADDRESS;
437441

438-
thread_actions.Append (thread_action);
439-
440-
thread_actions.SetDefaultThreadActionIfNeeded (eStateRunning, 0);
442+
m_thread_actions.Append (thread_action);
443+
m_thread_actions.SetDefaultThreadActionIfNeeded (eStateRunning, 0);
441444

442445
PTHREAD_MUTEX_LOCKER (locker, m_exception_messages_mutex);
443446

444-
ReplyToAllExceptions (thread_actions);
447+
ReplyToAllExceptions ();
445448

446449
}
447450

@@ -597,7 +600,7 @@ MachProcess::WriteMemory (nub_addr_t addr, nub_size_t size, const void *buf)
597600

598601

599602
void
600-
MachProcess::ReplyToAllExceptions (const DNBThreadResumeActions& thread_actions)
603+
MachProcess::ReplyToAllExceptions ()
601604
{
602605
PTHREAD_MUTEX_LOCKER(locker, m_exception_messages_mutex);
603606
if (m_exception_messages.empty() == false)
@@ -610,13 +613,13 @@ MachProcess::ReplyToAllExceptions (const DNBThreadResumeActions& thread_actions)
610613
DNBLogThreadedIf(LOG_EXCEPTIONS, "Replying to exception %d...", std::distance(begin, pos));
611614
int thread_reply_signal = 0;
612615

613-
const DNBThreadResumeAction *action = thread_actions.GetActionForThread (pos->state.thread_port, false);
616+
const DNBThreadResumeAction *action = m_thread_actions.GetActionForThread (pos->state.thread_port, false);
614617

615618
if (action)
616619
{
617620
thread_reply_signal = action->signal;
618621
if (thread_reply_signal)
619-
thread_actions.SetSignalHandledForThread (pos->state.thread_port);
622+
m_thread_actions.SetSignalHandledForThread (pos->state.thread_port);
620623
}
621624

622625
DNBError err (pos->Reply(this, thread_reply_signal));
@@ -630,20 +633,20 @@ MachProcess::ReplyToAllExceptions (const DNBThreadResumeActions& thread_actions)
630633
}
631634
}
632635
void
633-
MachProcess::PrivateResume (const DNBThreadResumeActions& thread_actions)
636+
MachProcess::PrivateResume ()
634637
{
635638
PTHREAD_MUTEX_LOCKER (locker, m_exception_messages_mutex);
636639

637-
ReplyToAllExceptions (thread_actions);
640+
ReplyToAllExceptions ();
638641
// bool stepOverBreakInstruction = step;
639642

640643
// Let the thread prepare to resume and see if any threads want us to
641644
// step over a breakpoint instruction (ProcessWillResume will modify
642645
// the value of stepOverBreakInstruction).
643-
m_thread_list.ProcessWillResume (this, thread_actions);
646+
m_thread_list.ProcessWillResume (this, m_thread_actions);
644647

645648
// Set our state accordingly
646-
if (thread_actions.NumActionsWithState(eStateStepping))
649+
if (m_thread_actions.NumActionsWithState(eStateStepping))
647650
SetState (eStateStepping);
648651
else
649652
SetState (eStateRunning);
@@ -1100,7 +1103,7 @@ MachProcess::ExceptionMessageBundleComplete()
11001103
else
11011104
{
11021105
// Resume without checking our current state.
1103-
PrivateResume (DNBThreadResumeActions (eStateRunning, 0));
1106+
PrivateResume ();
11041107
}
11051108
}
11061109
else

0 commit comments

Comments
 (0)
This repository has been archived.