<div dir="ltr">> <span style="font-family:arial,sans-serif;font-size:13px">llgs: fix thread names broken by recent native thread changes.</span><br style="font-family:arial,sans-serif;font-size:13px"><div><br></div><div>That's not quite accurate. The usage of a stack variable pointer after it went out of scope was there before - the issue just didn't surface until the code was shuffled.</div><div><br></div><div>A better title would have been:</div><div><br></div><div>llgs: fix Linux thread name bug surfaced by recent native thread changes</div><div><br></div><div>-Todd</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 12, 2014 at 3:51 PM, Todd Fiala <span dir="ltr"><<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: tfiala<br>
Date: Fri Sep 12 17:51:49 2014<br>
New Revision: 217717<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=217717&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=217717&view=rev</a><br>
Log:<br>
llgs: fix thread names broken by recent native thread changes.<br>
<br>
* Fixes the local stack variable return pointer usage in NativeThreadLinux::GetName().<br>
* Changes NativeThreadProtocol::GetName() to return a std::string.<br>
* Adds a unit test to verify thread names don't regress in the future. Currently only run on Linux since I know default thread names there.<br>
<br>
Modified:<br>
lldb/trunk/source/Host/common/NativeThreadProtocol.h<br>
lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp<br>
lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h<br>
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp<br>
lldb/trunk/test/tools/lldb-gdbserver/TestGdbRemote_qThreadStopInfo.py<br>
<br>
Modified: lldb/trunk/source/Host/common/NativeThreadProtocol.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeThreadProtocol.h?rev=217717&r1=217716&r2=217717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeThreadProtocol.h?rev=217717&r1=217716&r2=217717&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Host/common/NativeThreadProtocol.h (original)<br>
+++ lldb/trunk/source/Host/common/NativeThreadProtocol.h Fri Sep 12 17:51:49 2014<br>
@@ -31,7 +31,7 @@ namespace lldb_private<br>
{<br>
}<br>
<br>
- virtual const char *<br>
+ virtual std::string<br>
GetName() = 0;<br>
<br>
virtual lldb::StateType<br>
<br>
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp?rev=217717&r1=217716&r2=217717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp?rev=217717&r1=217716&r2=217717&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp (original)<br>
+++ lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp Fri Sep 12 17:51:49 2014<br>
@@ -61,7 +61,7 @@ NativeThreadLinux::NativeThreadLinux (Na<br>
{<br>
}<br>
<br>
-const char *<br>
+std::string<br>
NativeThreadLinux::GetName()<br>
{<br>
NativeProcessProtocolSP process_sp = m_process_wp.lock ();<br>
<br>
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h?rev=217717&r1=217716&r2=217717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h?rev=217717&r1=217716&r2=217717&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h (original)<br>
+++ lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h Fri Sep 12 17:51:49 2014<br>
@@ -27,7 +27,7 @@ namespace lldb_private<br>
// ---------------------------------------------------------------------<br>
// NativeThreadProtocol Interface<br>
// ---------------------------------------------------------------------<br>
- const char *<br>
+ std::string<br>
GetName() override;<br>
<br>
lldb::StateType<br>
<br>
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp?rev=217717&r1=217716&r2=217717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp?rev=217717&r1=217716&r2=217717&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp (original)<br>
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp Fri Sep 12 17:51:49 2014<br>
@@ -871,21 +871,21 @@ GDBRemoteCommunicationServer::SendStopRe<br>
response.Printf ("thread:%" PRIx64 ";", tid);<br>
<br>
// Include the thread name if there is one.<br>
- const char *thread_name = thread_sp->GetName ();<br>
- if (thread_name && thread_name[0])<br>
+ const std::string thread_name = thread_sp->GetName ();<br>
+ if (!thread_name.empty ())<br>
{<br>
- size_t thread_name_len = strlen(thread_name);<br>
+ size_t thread_name_len = thread_name.length ();<br>
<br>
- if (::strcspn (thread_name, "$#+-;:") == thread_name_len)<br>
+ if (::strcspn (thread_name.c_str (), "$#+-;:") == thread_name_len)<br>
{<br>
response.PutCString ("name:");<br>
- response.PutCString (thread_name);<br>
+ response.PutCString (thread_name.c_str ());<br>
}<br>
else<br>
{<br>
// The thread name contains special chars, send as hex bytes.<br>
response.PutCString ("hexname:");<br>
- response.PutCStringAsRawHex8 (thread_name);<br>
+ response.PutCStringAsRawHex8 (thread_name.c_str ());<br>
}<br>
response.PutChar (';');<br>
}<br>
<br>
Modified: lldb/trunk/test/tools/lldb-gdbserver/TestGdbRemote_qThreadStopInfo.py<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-gdbserver/TestGdbRemote_qThreadStopInfo.py?rev=217717&r1=217716&r2=217717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-gdbserver/TestGdbRemote_qThreadStopInfo.py?rev=217717&r1=217716&r2=217717&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/test/tools/lldb-gdbserver/TestGdbRemote_qThreadStopInfo.py (original)<br>
+++ lldb/trunk/test/tools/lldb-gdbserver/TestGdbRemote_qThreadStopInfo.py Fri Sep 12 17:51:49 2014<br>
@@ -1,3 +1,4 @@<br>
+import sys<br>
import unittest2<br>
<br>
import gdbremote_testcase<br>
@@ -40,6 +41,7 @@ class TestGdbRemote_qThreadStopInfo(gdbr<br>
<br>
# Grab stop reply for each thread via qThreadStopInfo{tid:hex}.<br>
stop_replies = {}<br>
+ thread_dicts = {}<br>
for thread in threads:<br>
# Run the qThreadStopInfo command.<br>
self.reset_test_sequence()<br>
@@ -67,10 +69,13 @@ class TestGdbRemote_qThreadStopInfo(gdbr<br>
self.assertIsNotNone(stop_result_text)<br>
stop_replies[kv_thread_id] = int(stop_result_text, 16)<br>
<br>
- return stop_replies<br>
+ # Hang on to the key-val dictionary for the thread.<br>
+ thread_dicts[kv_thread_id] = kv_dict<br>
+<br>
+ return (stop_replies, thread_dicts)<br>
<br>
def qThreadStopInfo_works_for_multiple_threads(self, thread_count):<br>
- stop_replies = self.gather_stop_replies_via_qThreadStopInfo(thread_count)<br>
+ (stop_replies, _) = self.gather_stop_replies_via_qThreadStopInfo(thread_count)<br>
self.assertEquals(len(stop_replies), thread_count)<br>
<br>
@debugserver_test<br>
@@ -90,7 +95,7 @@ class TestGdbRemote_qThreadStopInfo(gdbr<br>
self.qThreadStopInfo_works_for_multiple_threads(self.THREAD_COUNT)<br>
<br>
def qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt(self, thread_count):<br>
- stop_replies = self.gather_stop_replies_via_qThreadStopInfo(thread_count)<br>
+ (stop_replies, _) = self.gather_stop_replies_via_qThreadStopInfo(thread_count)<br>
self.assertIsNotNone(stop_replies)<br>
<br>
no_stop_reason_count = sum(1 for stop_reason in stop_replies.values() if stop_reason == 0)<br>
@@ -118,6 +123,33 @@ class TestGdbRemote_qThreadStopInfo(gdbr<br>
self.set_inferior_startup_launch()<br>
self.qThreadStopInfo_only_reports_one_thread_stop_reason_during_interrupt(self.THREAD_COUNT)<br>
<br>
+ def qThreadStopInfo_has_valid_thread_names(self, thread_count, expected_thread_name):<br>
+ (_, thread_dicts) = self.gather_stop_replies_via_qThreadStopInfo(thread_count)<br>
+ self.assertIsNotNone(thread_dicts)<br>
+<br>
+ for thread_dict in thread_dicts.values():<br>
+ name = thread_dict.get("name")<br>
+ self.assertIsNotNone(name)<br>
+ self.assertEquals(name, expected_thread_name)<br>
+<br>
+ @unittest2.skip("MacOSX doesn't have a default thread name")<br>
+ @debugserver_test<br>
+ @dsym_test<br>
+ def test_qThreadStopInfo_has_valid_thread_names_debugserver_dsym(self):<br>
+ self.init_debugserver_test()<br>
+ self.buildDsym()<br>
+ self.set_inferior_startup_launch()<br>
+ self.qThreadStopInfo_has_valid_thread_names(self.THREAD_COUNT, "a.out")<br>
+<br>
+ @unittest2.skipUnless(sys.platform.startswith("linux"), "test requires OS with set, equal thread names by default")<br>
+ @llgs_test<br>
+ @dwarf_test<br>
+ def test_qThreadStopInfo_has_valid_thread_names_llgs_dwarf(self):<br>
+ self.init_llgs_test()<br>
+ self.buildDwarf()<br>
+ self.set_inferior_startup_launch()<br>
+ self.qThreadStopInfo_has_valid_thread_names(self.THREAD_COUNT, "a.out")<br>
+<br>
<br>
if __name__ == '__main__':<br>
unittest2.main()<br>
<br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34);background-repeat:initial initial">tfiala@google.com</span></a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table><br></div>
</div>