[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
ravitheja updated this revision to Diff 98599.
ravitheja added a comment.
Changes for the feedback recieved in first round of review.
https://reviews.llvm.org/D32585
Files:
docs/lldb-gdb-remote.txt
include/lldb/API/SBTrace.h
include/lldb/Core/TraceOptions.h
include/lldb/Host/common/NativeProcessProtocol.h
include/lldb/Target/Process.h
include/lldb/Utility/StringExtractor.h
source/API/SBProcess.cpp
source/API/SBTrace.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Utility/StringExtractor.cpp
source/Utility/StringExtractorGDBRemote.cpp
source/Utility/StringExtractorGDBRemote.h
Index: source/Utility/StringExtractorGDBRemote.h
===
--- source/Utility/StringExtractorGDBRemote.h
+++ source/Utility/StringExtractorGDBRemote.h
@@ -164,6 +164,12 @@
eServerPacketType__M,
eServerPacketType__m,
eServerPacketType_notify, // '%' notification
+
+eServerPacketType_JTrace_Start,
+eServerPacketType_jTraceBufferRead,
+eServerPacketType_jTraceMetaRead,
+eServerPacketType_JTrace_Stop,
+eServerPacketType_jTraceConfigRead,
};
ServerPacketType GetServerPacketType() const;
Index: source/Utility/StringExtractorGDBRemote.cpp
===
--- source/Utility/StringExtractorGDBRemote.cpp
+++ source/Utility/StringExtractorGDBRemote.cpp
@@ -279,13 +279,26 @@
}
break;
+ case 'J':
+if (PACKET_STARTS_WITH("JTrace:start:"))
+ return eServerPacketType_JTrace_Start;
+if (PACKET_STARTS_WITH("JTrace:stop:"))
+ return eServerPacketType_JTrace_Stop;
+break;
+
case 'j':
if (PACKET_STARTS_WITH("jModulesInfo:"))
return eServerPacketType_jModulesInfo;
if (PACKET_MATCHES("jSignalsInfo"))
return eServerPacketType_jSignalsInfo;
if (PACKET_MATCHES("jThreadsInfo"))
return eServerPacketType_jThreadsInfo;
+if (PACKET_STARTS_WITH("jTrace:buffer:read:"))
+ return eServerPacketType_jTraceBufferRead;
+if (PACKET_STARTS_WITH("jTrace:conf:read:"))
+ return eServerPacketType_jTraceConfigRead;
+if (PACKET_STARTS_WITH("jTrace:meta:read:"))
+ return eServerPacketType_jTraceMetaRead;
break;
case 'v':
Index: source/Utility/StringExtractor.cpp
===
--- source/Utility/StringExtractor.cpp
+++ source/Utility/StringExtractor.cpp
@@ -280,6 +280,15 @@
return result;
}
+bool StringExtractor::Consume_front(const llvm::StringRef &str) {
+ llvm::StringRef S = GetStringRef();
+ if (!S.startswith(str))
+return false;
+ else
+m_index += str.size();
+ return true;
+}
+
size_t StringExtractor::GetHexBytes(llvm::MutableArrayRef dest,
uint8_t fail_fill_value) {
size_t bytes_extracted = 0;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -175,6 +175,21 @@
Error DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
+ lldb::user_id_t StartTrace(const TraceOptions &options,
+ Error &error) override;
+
+ Error StopTrace(lldb::user_id_t uid, lldb::tid_t thread_id) override;
+
+ Error GetData(lldb::user_id_t uid, lldb::tid_t thread_id,
+llvm::MutableArrayRef &buffer,
+size_t offset = 0) override;
+
+ Error GetMetaData(lldb::user_id_t uid, lldb::tid_t thread_id,
+llvm::MutableArrayRef &buffer,
+size_t offset = 0) override;
+
+ Error GetTraceConfig(lldb::user_id_t uid, TraceOptions &options) override;
+
Error GetWatchpointSupportInfo(uint32_t &num) override;
Error GetWatchpointSupportInfo(uint32_t &num, bool &after) override;
@@ -407,6 +422,10 @@
std::map m_thread_id_to_used_usec_map;
uint64_t m_last_signals_version = 0;
+ Error GetTraceData(StreamString &packet, lldb::user_id_t uid,
+ lldb::tid_t thread_id,
+ llvm::MutableArrayRef &buffer, size_t offset);
+
static bool NewThreadNotifyBreakpointHit(void *baton,
StoppointCallbackContext *context,
lldb::user_id_t break_id,
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -65,6 +65,7 @@
#include "lldb/Target/ThreadPlanCallFunction.h"
#include "lldb/Utility/CleanUp.h
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
ravitheja marked 28 inline comments as done.
ravitheja added inline comments.
Comment at: docs/lldb-gdb-remote.txt:212
//--
+// QTrace:1:type:;
+//
clayborg wrote:
> labath wrote:
> > ravitheja wrote:
> > > clayborg wrote:
> > > > Should we make all these new packets JSON based to start with?
> > > > "jTrace"? If we have any need for JSON at all in this or the other new
> > > > packets I would say lets just go with JSON packets. They are currently
> > > > prefixed with "j". If we go this route we should specify the mandatory
> > > > key/value pairs in the header doc. We should also allow a JSON
> > > > dictionary from the trace config up at the SBTrace layer to make it
> > > > into this packet?
> > > I am fine prefixing the packets with "j", I did not understand what you
> > > meant by the last sentence. At the moment I am waiting for Pavel or Tamas
> > > to reply or anyone else and based on the complete feedback, I will upload
> > > a new patch if needed.
> > I think Greg meant you should also change the packet format to take a json
> > argument instead of key-value pairs (The packet should be JTrace if it
> > modifies the state, right ?).
> >
> > If there is a gdb equivalent which matches this functionality sufficiently
> > closely, then I would prefer to stick to that. However, given that we
> > already are attaching a json field to the packet, I think it would make
> > sense to go json all the way.
> I think the entire packet should be:
> ```
> $jTrace:
> ```
> where is a JSON dictionary. Ravitheja had talked before about adding an
> implementation specific JSON dictionary into the JSON that comes in to
> configure the trace at the lldb::SB layer. I was just saying it would be nice
> if this implementation specific JSON was sent down as part of this packet so
> the GDB server can use it. Maybe we just send the entire thing that came in
> at the SB layer?
Yes thats what is currently being done, the JSON Dictionary obtained at SB
layer is being passed down to the server through the remote packets.
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:13
+#include "lldb/Core/TraceOptions.h"
#include "lldb/Host/MainLoop.h"
labath wrote:
> You are missing the traceOptions file from this patch.
The traceOptions are already merged into lldb code, please refer to
->https://reviews.llvm.org/D29581
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:429-430
+ //--
+ virtual void GetTraceConfig(lldb::user_id_t uid, lldb::tid_t threadid,
+ Error &error, TraceOptions &config) {
+error.SetErrorString("Not implemented");
zturner wrote:
> `virtual Error GetTraceConfig(user_id_t uid, tid_ threadid, TraceOptions
> &config)`
The threadid is already inside the TraceOptions, which needs to be set by the
user, I will update the comments here.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1161
+ if (buffersize == std::numeric_limits::max() ||
+ packet.GetBytesLeft() != 0) {
+
zturner wrote:
> Can you use `!packet.empty()`
I think its not possible to use empty function coz it checks the size of
m_packet string inside and not the current m_index.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1391
+
+ if (error_encountered || packet.GetBytesLeft() != 0 ||
+ byte_count == std::numeric_limits::max() ||
zturner wrote:
> `!packet.empty()`
Not possible I think
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h:178
+ virtual lldb::user_id_t StartTrace(lldb::TraceOptionsSP &options,
+ Error &error) override;
labath wrote:
> `const TraceOptions &options` ? Having a reference to a shared pointer seems
> like a huge overkill here.
Yes u are right my mistake.
https://reviews.llvm.org/D32585
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
ravitheja marked 2 inline comments as done. ravitheja added a comment. In https://reviews.llvm.org/D32585#740632, @labath wrote: > I quite like that you have added just the packet plumbing code without an > concrete implementation. However, that is still a significant amount of > parsing code that should be accompanied by a test. The test suite for the > client side of the protocol is ready (TestGdbRemoteCommunicationClient), so > I'd like to see at least that. @labath I was considering writing Unit tests for the remote packets but I thought then I have to write the mock implementation for the trace operations as well, which might end up being a bigger piece of code than the actual packet parsing code. After this patch, I will upload the actual server side code doing the trace stuff for linux, that part of code has unit tests for some core functions. https://reviews.llvm.org/D32585 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
clayborg added a comment.
So my question is: if we go the jTrace packet route, I would like to not use
GDB remote key/value pairs, and just go full JSON. You can make a new JSON dict
and pass all of the GDB remote key/value pairs inside and then you can add a
"jparams" key whose value is the JSON from the SB layer. JSON packets start
with lower case j, not sure why some were upper cased?
Comment at: docs/lldb-gdb-remote.txt:212
//--
+// JTrace:start:
+//
I think JSON packets start with lower case 'j'
Comment at: docs/lldb-gdb-remote.txt:217-218
+// parameters (mandatory and optional) should be appended to the packet
+// although there is no specific order imposed. The parameters need to
+// formatted as a semi-colon seperated list of "Name:Value" pairs.
+// Different tracing types could require different custom parameters.
Need to remove this bit about semicolon separates key/value pairs as everything
is now in the JSON
Comment at: docs/lldb-gdb-remote.txt:239
+//
+// threadidThe id of the thread to start tracingO
+// on.
Maybe state that all threads will be traced if this is not supplied?
Comment at: docs/lldb-gdb-remote.txt:256
+
+send packet: JTrace:start:type:;buffersize:;
+read packet: /E
So I am not sure why we are mixing GDB remote key:value; with JSON here? I was
thinking the packet would be jTrace:X where is the JSON that contains
all key value pairs and it also might have an extra jparams key whose value is
the JSON from the SB interface.
If we need more than 1 jTrace command it could be "jTraceStart:" and
"jTraceStop:"
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:187-188
+ RegisterMemberFunctionHandler(
+ StringExtractorGDBRemote::eServerPacketType_JTrace_Start,
+ &GDBRemoteCommunicationServerLLGS::Handle_JTrace_start);
+ RegisterMemberFunctionHandler(
lower case j
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:196-197
+ RegisterMemberFunctionHandler(
+ StringExtractorGDBRemote::eServerPacketType_JTrace_Stop,
+ &GDBRemoteCommunicationServerLLGS::Handle_JTrace_stop);
+ RegisterMemberFunctionHandler(
lower case j
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:199-200
+ RegisterMemberFunctionHandler(
+ StringExtractorGDBRemote::eServerPacketType_jTraceConfigRead,
+ &GDBRemoteCommunicationServerLLGS::Handle_jTrace_conf_read);
+
lower case j
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1110
+
+ if (!packet.Consume_front("JTrace:start:"))
+return SendIllFormedResponse(packet, "JTrace:start: Ill formed packet ");
lower case j
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:192
+ PacketResult Handle_JTrace_start(StringExtractorGDBRemote &packet);
+
lower case j
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:196
+
+ PacketResult Handle_JTrace_stop(StringExtractorGDBRemote &packet);
+
lower case j
Comment at: source/Utility/StringExtractorGDBRemote.cpp:282
+ case 'J':
+if (PACKET_STARTS_WITH("JTrace:start:"))
lower case j
Comment at: source/Utility/StringExtractorGDBRemote.cpp:283
+ case 'J':
+if (PACKET_STARTS_WITH("JTrace:start:"))
+ return eServerPacketType_JTrace_Start;
lower case j
Comment at: source/Utility/StringExtractorGDBRemote.cpp:285
+ return eServerPacketType_JTrace_Start;
+if (PACKET_STARTS_WITH("JTrace:stop:"))
+ return eServerPacketType_JTrace_Stop;
lower case j
https://reviews.llvm.org/D32585
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
zturner added inline comments.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1125
+uint64_t extracted_value;
+value.getAsInteger(16, extracted_value);
+
Should you be checking the return value of `getAsInteger` here? What if it
didn't parse as an integer? Confusingly, the function returns true if it
failed and false if it suceeded, so watch out.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1200
+uint64_t extracted_value;
+if (!value.getAsInteger(16, extracted_value)) {
+ if (name.equals("threadid"))
Can you invert the conditional here and use an early exit from the loop if
there's an error?
https://reviews.llvm.org/D32585
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
jingham added a comment. If you just have symbols, you can't tell whether private symbols were visible to the current frame or not, but the likelihood is not 0 (I guess it's something like 1/number of CU's). OTOH, your expression could never have seen a private symbol from another module, and 1/nCU > 0. So I would weakly argue for flipping 2 & 3. https://reviews.llvm.org/D33083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
jingham added a comment. But note that the real solution to that problem is implementing some syntax for "symbol from CU", "symbol from Function" etc, as we've discussed in the past. Then what we do by default will be less important, since you have a way to override it. https://reviews.llvm.org/D33083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
spyffe added a comment. I also would flip 2 and 3. I'll give this a shot. https://reviews.llvm.org/D33083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors marked 43 inline comments as done.
jmajors added a comment.
Every comment should either be marked done, with an accompanying code change,
or have a reply comment.
Comment at: unittests/CMakeLists.txt:57
add_subdirectory(Breakpoint)
+add_subdirectory(tools)
add_subdirectory(Core)
takuto.ikuta wrote:
> better to sort alphabetically?
> Using tools instead of Tools intentionally?
I'm following the naming and case of the source directories.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:61-63
+string name;
+thread_info->GetValueForKeyAsString("name", name);
+string reason;
zturner wrote:
> `StringRef name, reason;`
There is no GetValueForKeyAsStringRef(). I need a std::string to populate.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:77-79
+ string key_str;
+ keys->GetItemAtIndexAsString(i, key_str);
+ string value_str;
zturner wrote:
> `StringRef key_str, value_str;`
These need to be strings for the same reason as above.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:83-88
+ if (endian == LITTLE) {
+registers[register_id] = SwitchEndian(value_str);
+ }
+ else {
+registers[register_id] = stoul(value_str, nullptr, 16);
+ }
zturner wrote:
> This code block is unnecessary.
>
> ```
> unsigned long X;
> if (!value_str.getAsInteger(X))
> return some error;
> llvm::support::endian::write(®isters[register_id], X, endian);
> ```
>
> By using llvm endianness functions you can just delete the `SwitchEndian()`
> function entirely, as it's not needed.
These endian functions don't work here. getAsInteger() assumes the number is in
human reading order (i.e. big endian). So it's converting the little endian
string as if it's big, then converting that little endian long to another
little endian long, which does nothing.
I need something that reverses the string first.
What do you think about adding a version of StringRef::getAsInteger that
accepts an endianness parameter?
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:122-123
+ while (true) {
+stringstream ss;
+ss << hex << setw(2) << setfill('0') << register_id;
+string hex_id = ss.str();
zturner wrote:
> Use `llvm::raw_string_ostream` or `raw_svector_ostream` instead of
> `stringstream`.
I don't see anything in either class or their ancestors that will format the
number as a two nibble hex value.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:133
+
+ for (auto i = elements.begin(); i != elements.end(); i++) {
+if (i->first[0] == 'T' && i->first.substr(3, 6) == "thread") {
takuto.ikuta wrote:
> Why not range based for?
That container lacks a necessary constructor.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:82-87
+// Common functions for parsing packet data.
+std::unordered_map SplitPairList(const std::string&
s);
+std::vector SplitList(const std::string& s, char delimeter);
+std::pair SplitPair(const std::string& s);
+std::string HexDecode(const std::string& hex_encoded);
+unsigned long SwitchEndian(const std::string& little_endian);
tberghammer wrote:
> Does these have to be global? Can we make them local to the .cc file
> (anonymous namespace or file static variable)?
I use one of them in the TestClient.cpp file. I could make the other local.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:83
+// Common functions for parsing packet data.
+std::unordered_map SplitPairList(const std::string&
s);
+std::vector SplitList(const std::string& s, char delimeter);
zturner wrote:
> tberghammer wrote:
> > What if the key isn't unique?
> Return an `llvm::StringMap` here. Also the argument should be a
> `StringRef`.
I was working on the assumption that the keys in lldb response packets would be
unique, and that it would map a key to a list of values if it needed to have a
one:many relationship. Are there any that have duplicate keys?
Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:65
+
+ sleep(5); // TODO: Sleep is bad. Can I wait for it to start?
+ SendAck(); // Send this as a handshake.
labath wrote:
> Since you're using reverse-connect (which is good), you should be able to
> detect that the server is ready by the fact it has connected to you.
Are you implying that that connection is a side effect of something I've
called, or that there's another function to call?
When I didn't have this sleep here, I got a very generic error on all
subsequent communication.
Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:70
+void TestClient::StopDebugger() {
+ Host::Kill(server_process_info.GetProce
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors updated this revision to Diff 98673.
jmajors marked an inline comment as done.
jmajors added a comment.
Made changes based on feedback.
https://reviews.llvm.org/D32930
Files:
unittests/CMakeLists.txt
unittests/tools/CMakeLists.txt
unittests/tools/lldb-server/CMakeLists.txt
unittests/tools/lldb-server/inferior/thread_inferior.cpp
unittests/tools/lldb-server/tests/CMakeLists.txt
unittests/tools/lldb-server/tests/MessageObjects.cpp
unittests/tools/lldb-server/tests/MessageObjects.h
unittests/tools/lldb-server/tests/TestClient.cpp
unittests/tools/lldb-server/tests/TestClient.h
unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
Index: unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -0,0 +1,52 @@
+//===-- ThreadsInJstopinfoTest.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+
+#include
+
+#include "gtest/gtest.h"
+#include "TestClient.h"
+
+using namespace CommunicationTests;
+
+TEST(ThreadsInJstopinfoTest, TestStopReplyContainsThreadPcsLlgs) {
+ std::vector inferior_args;
+ // This inferior spawns N threads, then forces a break.
+ inferior_args.push_back(THREAD_INFERIOR);
+ inferior_args.push_back("4");
+
+ auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
+
+ TestClient client(test_info->name(), test_info->test_case_name());
+ client.StartDebugger();
+ client.SetInferior(inferior_args);
+ client.ListThreadsInStopReply();
+ client.Continue();
+ unsigned int pc_reg = client.GetPcRegisterId();
+
+ std::shared_ptr jthreads_info = client.GetJThreadsInfo();
+ ASSERT_TRUE(jthreads_info.get()) << "Error reading JThreadInfo.";
+
+ auto stop_reply = client.GetLatestStopReply();
+ auto stop_reply_pcs = stop_reply->GetThreadPcs();
+ auto thread_infos = jthreads_info->GetThreadInfos();
+ ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
+<< "Thread count mismatch.";
+
+ for (auto stop_reply_pc : stop_reply_pcs) {
+unsigned long tid = stop_reply_pc.first;
+ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
+ << "Thread ID: " << tid << " not in JThreadsInfo.";
+ASSERT_EQ(stop_reply_pcs[tid], thread_infos[tid].ReadRegister(pc_reg))
+ << "Mismatched PC for thread: " << tid;
+ }
+
+ client.StopDebugger();
+}
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -0,0 +1,55 @@
+//===-- TestClient.h *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+#include
+
+#include "lldb/Core/ArchSpec.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
+
+#include "MessageObjects.h"
+
+namespace CommunicationTests {
+// TODO: Make the test client an abstract base class, with different children
+// for different types of connections: llgs v. debugserver
+class TestClient :
+ public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
+public:
+ TestClient(const std::string& test_name, const std::string& test_case_name);
+ virtual ~TestClient();
+ void StartDebugger();
+ void StopDebugger();
+ void SetInferior(llvm::ArrayRef inferior_args);
+ void ListThreadsInStopReply();
+ void SetBreakpoint(unsigned long address);
+ void Continue(unsigned long thread_id = 0);
+ std::shared_ptr GetProcessInfo();
+ std::shared_ptr GetJThreadsInfo();
+ std::shared_ptr GetLatestStopReply();
+ void SendMessage(const std::string& message);
+ void SendMessage(const std::string& message, std::string& response_string);
+ unsigned int GetPcRegisterId();
+
+private:
+ void GenerateConnectionAddress(std::string& address);
+ std::string GenerateLogFileName(const lldb_private::ArchSpec& arch) const;
+ std::string FormatFailedResult(const std::string& message,
+ lldb_private::process_gdb_remote::GDBRemoteCommunication::PacketResult
+ result);
+
+ std::shared_ptr process_info;
+ std::shared_ptr stop_reply;
+ lldb_private::ProcessLaunchInfo server_process_info;
+ std::string test_name;
+ std::string test_case_name;
+ unsigned int pc_register;
+};
+}
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- /dev
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
spyffe updated this revision to Diff 98672.
spyffe added a comment.
Improved symbol lookup per Greg and Jim's suggestion, with flipped steps 2 and
3.
Also added testing for the error case.
https://reviews.llvm.org/D33083
Files:
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
@@ -0,0 +1,11 @@
+#include "One/One.h"
+#include "Two/Two.h"
+
+#include
+
+int main() {
+ one();
+ two();
+ printf("main\n"); // break here
+ return(0);
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
@@ -0,0 +1 @@
+__private_extern__ int conflicting_symbol = 2;
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
@@ -0,0 +1,4 @@
+#ifndef TWO_H
+#define TWO_H
+void two();
+#endif
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
@@ -0,0 +1,6 @@
+#include "Two.h"
+#include
+
+void two() {
+ printf("Two\n"); // break here
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
===
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
@@ -0,0 +1,87 @@
+"""Test that conflicting symbols in different shared libraries work correctly"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRealDefinition(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_frame_var_after_stop_at_implementation(self):
+if self.getArchitecture() == 'i386':
+self.skipTest("requires modern objc runtime")
+self.build()
+self.common_setup()
+
+One_line = line_number('One/One.c', '// break here')
+Two_line = line_number('Two/Two.c', '// break here')
+main_line = line_number('main.c', '// break here')
+lldbutil.run_break_set_by_file_and_line(
+self, 'One.c', One_line, num_expected_locations=1, loc_exact=True)
+lldbutil.run_break_set_by_file_and_line(
+self, 'Two.c', Two_line, num_expected_locations=1, loc_exact=True)
+lldbutil.run_break_set_by_file_and_line(
+self, 'main.c', main_line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+substrs=[' resolved, hit count = 1'])
+
+# This should display correctly.
+self.expect(
+"expr (unsigned long long)conflicting_symbol",
+"Symbol from One should be found",
+substrs=[
+"1"])
+
+self.runCmd("continue", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
clayborg added a comment. The searching code is indeed better and what we now want, but I question of this code should live in ClangExpressionDeclMap::FindGlobalDataSymbol. A better home for this might be in SymbolContext::FindGlobalDataSymbol? Then others can use this functionality. Other clients must want this kind of functionality (other expression parsers). Seems a shame to have it live somewhere where someone would have to copy the code. Thoughts anyone? https://reviews.llvm.org/D33083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
jingham added a comment. I think Greg's right. I can't see anything expression parser specific to how you do this search. I was a little worried that a lot of the other searches will return multiple names (maybe sorting to indicate closeness.) So this one might be confusing. But the API name says you are looking for ONE data symbol, so the behavior when you can't tell amongst the matches actually makes sense. https://reviews.llvm.org/D33083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
zturner added inline comments.
Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:21
+
+ LLVM_BUILTIN_DEBUGTRAP;
+ delay = false;
This will work on MSVC and presumably clang. I'm not sure about gcc. Is that
sufficient for your needs? Do you know if gcc has the `__builtin_debugtrap`
intrinsic?
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:83-88
+ if (endian == LITTLE) {
+registers[register_id] = SwitchEndian(value_str);
+ }
+ else {
+registers[register_id] = stoul(value_str, nullptr, 16);
+ }
jmajors wrote:
> zturner wrote:
> > This code block is unnecessary.
> >
> > ```
> > unsigned long X;
> > if (!value_str.getAsInteger(X))
> > return some error;
> > llvm::support::endian::write(®isters[register_id], X, endian);
> > ```
> >
> > By using llvm endianness functions you can just delete the `SwitchEndian()`
> > function entirely, as it's not needed.
> These endian functions don't work here. getAsInteger() assumes the number is
> in human reading order (i.e. big endian). So it's converting the little
> endian string as if it's big, then converting that little endian long to
> another little endian long, which does nothing.
> I need something that reverses the string first.
>
> What do you think about adding a version of StringRef::getAsInteger that
> accepts an endianness parameter?
Hmm.. What about this?
```
unsigned long X;
if (!value_str.getAsInteger(X))
return some error;
sys::swapByteOrder(X);
```
It shouldn't matter if you swap the bytes in the string before doing the
translation, or swap the bytes in the number after doing the translation should
it?
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:64
+public:
+ static std::shared_ptr Create(const llvm::StringRef response,
+ llvm::support::endianness endian);
Can you constructor argument does not need to be `const`, as `StringRefs` are
immutable by definition.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:69
+protected:
+ explicit StopReply();
+
Don't need `explicit` if there are no arguments. It's mostly useful for single
argument constructors.
https://reviews.llvm.org/D32930
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors marked 2 inline comments as done. jmajors added inline comments. Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:21 + + LLVM_BUILTIN_DEBUGTRAP; + delay = false; zturner wrote: > This will work on MSVC and presumably clang. I'm not sure about gcc. Is > that sufficient for your needs? Do you know if gcc has the > `__builtin_debugtrap` intrinsic? Do we use gcc to build/test lldb? If not, then it shouldn't be an issue. If we ever change our compiler of choice, we can always change this to match. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors updated this revision to Diff 98675.
jmajors added a comment.
Feedback changes.
https://reviews.llvm.org/D32930
Files:
unittests/CMakeLists.txt
unittests/tools/CMakeLists.txt
unittests/tools/lldb-server/CMakeLists.txt
unittests/tools/lldb-server/inferior/thread_inferior.cpp
unittests/tools/lldb-server/tests/CMakeLists.txt
unittests/tools/lldb-server/tests/MessageObjects.cpp
unittests/tools/lldb-server/tests/MessageObjects.h
unittests/tools/lldb-server/tests/TestClient.cpp
unittests/tools/lldb-server/tests/TestClient.h
unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
Index: unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -0,0 +1,52 @@
+//===-- ThreadsInJstopinfoTest.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+
+#include
+
+#include "gtest/gtest.h"
+#include "TestClient.h"
+
+using namespace CommunicationTests;
+
+TEST(ThreadsInJstopinfoTest, TestStopReplyContainsThreadPcsLlgs) {
+ std::vector inferior_args;
+ // This inferior spawns N threads, then forces a break.
+ inferior_args.push_back(THREAD_INFERIOR);
+ inferior_args.push_back("4");
+
+ auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
+
+ TestClient client(test_info->name(), test_info->test_case_name());
+ client.StartDebugger();
+ client.SetInferior(inferior_args);
+ client.ListThreadsInStopReply();
+ client.Continue();
+ unsigned int pc_reg = client.GetPcRegisterId();
+
+ std::shared_ptr jthreads_info = client.GetJThreadsInfo();
+ ASSERT_TRUE(jthreads_info.get()) << "Error reading JThreadInfo.";
+
+ auto stop_reply = client.GetLatestStopReply();
+ auto stop_reply_pcs = stop_reply->GetThreadPcs();
+ auto thread_infos = jthreads_info->GetThreadInfos();
+ ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
+<< "Thread count mismatch.";
+
+ for (auto stop_reply_pc : stop_reply_pcs) {
+unsigned long tid = stop_reply_pc.first;
+ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
+ << "Thread ID: " << tid << " not in JThreadsInfo.";
+ASSERT_EQ(stop_reply_pcs[tid], thread_infos[tid].ReadRegister(pc_reg))
+ << "Mismatched PC for thread: " << tid;
+ }
+
+ client.StopDebugger();
+}
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -0,0 +1,55 @@
+//===-- TestClient.h *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+#include
+
+#include "lldb/Core/ArchSpec.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
+
+#include "MessageObjects.h"
+
+namespace CommunicationTests {
+// TODO: Make the test client an abstract base class, with different children
+// for different types of connections: llgs v. debugserver
+class TestClient :
+ public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
+public:
+ TestClient(const std::string& test_name, const std::string& test_case_name);
+ virtual ~TestClient();
+ void StartDebugger();
+ void StopDebugger();
+ void SetInferior(llvm::ArrayRef inferior_args);
+ void ListThreadsInStopReply();
+ void SetBreakpoint(unsigned long address);
+ void Continue(unsigned long thread_id = 0);
+ std::shared_ptr GetProcessInfo();
+ std::shared_ptr GetJThreadsInfo();
+ std::shared_ptr GetLatestStopReply();
+ void SendMessage(const std::string& message);
+ void SendMessage(const std::string& message, std::string& response_string);
+ unsigned int GetPcRegisterId();
+
+private:
+ void GenerateConnectionAddress(std::string& address);
+ std::string GenerateLogFileName(const lldb_private::ArchSpec& arch) const;
+ std::string FormatFailedResult(const std::string& message,
+ lldb_private::process_gdb_remote::GDBRemoteCommunication::PacketResult
+ result);
+
+ std::shared_ptr process_info;
+ std::shared_ptr stop_reply;
+ lldb_private::ProcessLaunchInfo server_process_info;
+ std::string test_name;
+ std::string test_case_name;
+ unsigned int pc_register;
+};
+}
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/TestClient.c
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
zturner added inline comments. Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:98 + unsigned int register_id; + key_str_ref.getAsInteger(10, register_id); + Do you need to check for an error here? Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:105 +unsigned int register_value; +value_str_ref.getAsInteger(16, register_value); +registers[register_id] = register_value; And here https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added inline comments. Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:200 + + return std::stoul(big_endian, nullptr, 16); +} Throws exceptions. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
clayborg added a comment. Yeah, we need to document exactly what this is doing: looking for the best symbol as an expression parser would want it, preferring symbols from the current module in the SymbolContext, and then looking everywhere. Errors if multiple exist in current module, or in any other modules if not in current, bla bla. https://reviews.llvm.org/D33083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r302833 - [DWARF parser] Produce correct template parameter packs
Author: spyffe
Date: Thu May 11 17:08:05 2017
New Revision: 302833
URL: http://llvm.org/viewvc/llvm-project?rev=302833&view=rev
Log:
[DWARF parser] Produce correct template parameter packs
Templates can end in parameter packs, like this
template struct MyStruct
{ /*...*/ };
LLDB does not currently support these parameter packs;
it does not emit them into the template argument list
at all. This causes problems when you specialize, e.g.:
template <> struct MyStruct
{ /*...*/ };
template <> struct MyStruct : MyStruct
{ /*...*/ };
LLDB generates two template specializations, each with
no template arguments, and then when they are imported
by the ASTImporter into a parser's AST context we get a
single specialization that inherits from itself,
causing Clang's record layout mechanism to smash its
stack.
This patch fixes the problem for classes and adds
tests. The tests for functions fail because Clang's
ASTImporter can't import them at the moment, so I've
xfailed that test.
Differential Revision: https://reviews.llvm.org/D33025
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=302833&r1=302832&r2=302833&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Thu May 11 17:08:05 2017
@@ -275,17 +275,16 @@ public:
bool IsValid() const {
if (args.empty())
return false;
- return args.size() == names.size();
-}
-
-size_t GetSize() const {
- if (IsValid())
-return args.size();
- return 0;
+ return args.size() == names.size() &&
+((bool)pack_name == (bool)packed_args) &&
+(!packed_args || !packed_args->packed_args);
}
llvm::SmallVector names;
llvm::SmallVector args;
+
+const char * pack_name = nullptr;
+std::unique_ptr packed_args;
};
clang::FunctionTemplateDecl *
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile?rev=302833&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
Thu May 11 17:08:05 2017
@@ -0,0 +1,3 @@
+LEVEL = ../../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py?rev=302833&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
Thu May 11 17:08:05 2017
@@ -0,0 +1,7 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(
+__file__, globals(), [
+decorators.expectedFailureAll(
+oslist=["windows"], bugnumber="llvm.org/pr24764")])
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk
[Lldb-commits] [PATCH] D33025: [DWARF parser] Produce correct template parameter packs
This revision was automatically updated to reflect the committed changes.
Closed by commit rL302833: [DWARF parser] Produce correct template parameter
packs (authored by spyffe).
Changed prior to commit:
https://reviews.llvm.org/D33025?vs=98388&id=98687#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33025
Files:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
Index: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h
@@ -275,17 +275,16 @@
bool IsValid() const {
if (args.empty())
return false;
- return args.size() == names.size();
-}
-
-size_t GetSize() const {
- if (IsValid())
-return args.size();
- return 0;
+ return args.size() == names.size() &&
+((bool)pack_name == (bool)packed_args) &&
+(!packed_args || !packed_args->packed_args);
}
llvm::SmallVector names;
llvm::SmallVector args;
+
+const char * pack_name = nullptr;
+std::unique_ptr packed_args;
};
clang::FunctionTemplateDecl *
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/TestFunctionTemplateParameterPack.py
@@ -0,0 +1,6 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(
+__file__, globals(), [
+decorators.expectedFailureAll(bugnumber="rdar://problem/32096064")])
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/main.cpp
@@ -0,0 +1,24 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LIDENSE.TXT for details.
+//
+//===--===//
+
+template int staticSizeof() {
+ return sizeof(T);
+}
+
+template int staticSizeof() {
+ return staticSizeof() + sizeof(T1);
+}
+
+int main (int argc, char const *argv[])
+{
+ int sz = staticSizeof();
+ return staticSizeof() != sz; //% self.expect("expression -- sz == staticSizeof()", "staticSizeof worked", substrs = ["true"])
+ //% self.expect("expression -- sz == staticSizeof() + sizeof(char)", "staticSizeof worked", substrs = ["true"])
+ //% self.expect("expression -- sz == staticSizeof() + sizeof(int) + sizeof(char)", "staticSizeof worked", substrs = ["true"])
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/function-template-parameter-pack/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/Makef
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors marked 5 inline comments as done.
jmajors added inline comments.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:200
+
+ return std::stoul(big_endian, nullptr, 16);
+}
krytarowski wrote:
> Throws exceptions.
I didn't change this, because I'm expecting to remove the whole function.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:83-88
+ if (endian == LITTLE) {
+registers[register_id] = SwitchEndian(value_str);
+ }
+ else {
+registers[register_id] = stoul(value_str, nullptr, 16);
+ }
zturner wrote:
> jmajors wrote:
> > zturner wrote:
> > > This code block is unnecessary.
> > >
> > > ```
> > > unsigned long X;
> > > if (!value_str.getAsInteger(X))
> > > return some error;
> > > llvm::support::endian::write(®isters[register_id], X, endian);
> > > ```
> > >
> > > By using llvm endianness functions you can just delete the
> > > `SwitchEndian()` function entirely, as it's not needed.
> > These endian functions don't work here. getAsInteger() assumes the number
> > is in human reading order (i.e. big endian). So it's converting the little
> > endian string as if it's big, then converting that little endian long to
> > another little endian long, which does nothing.
> > I need something that reverses the string first.
> >
> > What do you think about adding a version of StringRef::getAsInteger that
> > accepts an endianness parameter?
> Hmm.. What about this?
>
> ```
> unsigned long X;
> if (!value_str.getAsInteger(X))
> return some error;
> sys::swapByteOrder(X);
> ```
>
> It shouldn't matter if you swap the bytes in the string before doing the
> translation, or swap the bytes in the number after doing the translation
> should it?
It doesn't matter when I do it. The issue with the other functions was they
were converting target to host, when both were little. For string parsing, it
needs target to big to string, or target to string to big.
Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:76-80
+ for (size_t i = 0; i < inferior_args.size(); i++) {
+if (i > 0) command << ',';
+string hex_encoded = HexEncode(inferior_args[i]);
+command << hex_encoded.size() << ',' << i << ',' << hex_encoded;
+ }
zturner wrote:
> `for (const auto &arg : inferior_args)`
I need an parameter index to pass to the command, so the classic for loop makes
more sense.
https://reviews.llvm.org/D32930
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors updated this revision to Diff 98691.
jmajors marked an inline comment as done.
jmajors added a comment.
More feedback changes.
https://reviews.llvm.org/D32930
Files:
unittests/CMakeLists.txt
unittests/tools/CMakeLists.txt
unittests/tools/lldb-server/CMakeLists.txt
unittests/tools/lldb-server/inferior/thread_inferior.cpp
unittests/tools/lldb-server/tests/CMakeLists.txt
unittests/tools/lldb-server/tests/MessageObjects.cpp
unittests/tools/lldb-server/tests/MessageObjects.h
unittests/tools/lldb-server/tests/TestClient.cpp
unittests/tools/lldb-server/tests/TestClient.h
unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
Index: unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -0,0 +1,52 @@
+//===-- ThreadsInJstopinfoTest.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+
+#include
+
+#include "gtest/gtest.h"
+#include "TestClient.h"
+
+using namespace CommunicationTests;
+
+TEST(ThreadsInJstopinfoTest, TestStopReplyContainsThreadPcsLlgs) {
+ std::vector inferior_args;
+ // This inferior spawns N threads, then forces a break.
+ inferior_args.push_back(THREAD_INFERIOR);
+ inferior_args.push_back("4");
+
+ auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
+
+ TestClient client(test_info->name(), test_info->test_case_name());
+ client.StartDebugger();
+ client.SetInferior(inferior_args);
+ client.ListThreadsInStopReply();
+ client.Continue();
+ unsigned int pc_reg = client.GetPcRegisterId();
+
+ std::shared_ptr jthreads_info = client.GetJThreadsInfo();
+ ASSERT_TRUE(jthreads_info.get()) << "Error reading JThreadInfo.";
+
+ auto stop_reply = client.GetLatestStopReply();
+ auto stop_reply_pcs = stop_reply->GetThreadPcs();
+ auto thread_infos = jthreads_info->GetThreadInfos();
+ ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
+<< "Thread count mismatch.";
+
+ for (auto stop_reply_pc : stop_reply_pcs) {
+unsigned long tid = stop_reply_pc.first;
+ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
+ << "Thread ID: " << tid << " not in JThreadsInfo.";
+ASSERT_EQ(stop_reply_pcs[tid], thread_infos[tid].ReadRegister(pc_reg))
+ << "Mismatched PC for thread: " << tid;
+ }
+
+ client.StopDebugger();
+}
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -0,0 +1,55 @@
+//===-- TestClient.h *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+#include
+
+#include "lldb/Core/ArchSpec.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
+
+#include "MessageObjects.h"
+
+namespace CommunicationTests {
+// TODO: Make the test client an abstract base class, with different children
+// for different types of connections: llgs v. debugserver
+class TestClient :
+ public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
+public:
+ TestClient(const std::string& test_name, const std::string& test_case_name);
+ virtual ~TestClient();
+ void StartDebugger();
+ void StopDebugger();
+ void SetInferior(llvm::ArrayRef inferior_args);
+ void ListThreadsInStopReply();
+ void SetBreakpoint(unsigned long address);
+ void Continue(unsigned long thread_id = 0);
+ std::shared_ptr GetProcessInfo();
+ std::shared_ptr GetJThreadsInfo();
+ std::shared_ptr GetLatestStopReply();
+ void SendMessage(const std::string& message);
+ void SendMessage(const std::string& message, std::string& response_string);
+ unsigned int GetPcRegisterId();
+
+private:
+ void GenerateConnectionAddress(std::string& address);
+ std::string GenerateLogFileName(const lldb_private::ArchSpec& arch) const;
+ std::string FormatFailedResult(const std::string& message,
+ lldb_private::process_gdb_remote::GDBRemoteCommunication::PacketResult
+ result);
+
+ std::shared_ptr process_info;
+ std::shared_ptr stop_reply;
+ lldb_private::ProcessLaunchInfo server_process_info;
+ std::string test_name;
+ std::string test_case_name;
+ unsigned int pc_register;
+};
+}
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- /dev/null
+++
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added a comment. Some headers are UNIX-specific like ``, is it possible to make these tests compatible with Windows? https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added inline comments. Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:1 +#include +#include For the clarity I would put the copyright notice in all files, including tests. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors marked an inline comment as done. jmajors added a comment. I cleaned up the includes. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
jmajors updated this revision to Diff 98702.
jmajors added a comment.
Cleaned up the includes.
https://reviews.llvm.org/D32930
Files:
unittests/CMakeLists.txt
unittests/tools/CMakeLists.txt
unittests/tools/lldb-server/CMakeLists.txt
unittests/tools/lldb-server/inferior/thread_inferior.cpp
unittests/tools/lldb-server/tests/CMakeLists.txt
unittests/tools/lldb-server/tests/MessageObjects.cpp
unittests/tools/lldb-server/tests/MessageObjects.h
unittests/tools/lldb-server/tests/TestClient.cpp
unittests/tools/lldb-server/tests/TestClient.h
unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
Index: unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -0,0 +1,50 @@
+//===-- ThreadsInJstopinfoTest.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+
+#include "gtest/gtest.h"
+#include "TestClient.h"
+
+using namespace CommunicationTests;
+
+TEST(ThreadsInJstopinfoTest, TestStopReplyContainsThreadPcsLlgs) {
+ std::vector inferior_args;
+ // This inferior spawns N threads, then forces a break.
+ inferior_args.push_back(THREAD_INFERIOR);
+ inferior_args.push_back("4");
+
+ auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
+
+ TestClient client(test_info->name(), test_info->test_case_name());
+ client.StartDebugger();
+ client.SetInferior(inferior_args);
+ client.ListThreadsInStopReply();
+ client.Continue();
+ unsigned int pc_reg = client.GetPcRegisterId();
+
+ auto jthreads_info = client.GetJThreadsInfo();
+ ASSERT_TRUE(jthreads_info.get()) << "Error reading JThreadInfo.";
+
+ auto stop_reply = client.GetLatestStopReply();
+ auto stop_reply_pcs = stop_reply->GetThreadPcs();
+ auto thread_infos = jthreads_info->GetThreadInfos();
+ ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
+<< "Thread count mismatch.";
+
+ for (auto stop_reply_pc : stop_reply_pcs) {
+unsigned long tid = stop_reply_pc.first;
+ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
+ << "Thread ID: " << tid << " not in JThreadsInfo.";
+ASSERT_EQ(stop_reply_pcs[tid], thread_infos[tid].ReadRegister(pc_reg))
+ << "Mismatched PC for thread: " << tid;
+ }
+
+ client.StopDebugger();
+}
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -0,0 +1,55 @@
+//===-- TestClient.h *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+#include
+
+#include "lldb/Core/ArchSpec.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
+
+#include "MessageObjects.h"
+
+namespace CommunicationTests {
+// TODO: Make the test client an abstract base class, with different children
+// for different types of connections: llgs v. debugserver
+class TestClient :
+ public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
+public:
+ TestClient(const std::string& test_name, const std::string& test_case_name);
+ virtual ~TestClient();
+ void StartDebugger();
+ void StopDebugger();
+ void SetInferior(llvm::ArrayRef inferior_args);
+ void ListThreadsInStopReply();
+ void SetBreakpoint(unsigned long address);
+ void Continue(unsigned long thread_id = 0);
+ std::shared_ptr GetProcessInfo();
+ std::shared_ptr GetJThreadsInfo();
+ std::shared_ptr GetLatestStopReply();
+ void SendMessage(const std::string& message);
+ void SendMessage(const std::string& message, std::string& response_string);
+ unsigned int GetPcRegisterId();
+
+private:
+ void GenerateConnectionAddress(std::string& address);
+ std::string GenerateLogFileName(const lldb_private::ArchSpec& arch) const;
+ std::string FormatFailedResult(const std::string& message,
+ lldb_private::process_gdb_remote::GDBRemoteCommunication::PacketResult
+ result);
+
+ std::shared_ptr process_info;
+ std::shared_ptr stop_reply;
+ lldb_private::ProcessLaunchInfo server_process_info;
+ std::string test_name;
+ std::string test_case_name;
+ unsigned int pc_register;
+};
+}
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -0,0 +1,228
[Lldb-commits] [lldb] r302850 - xfail TestClassTemplateParameterPack on gcc to mollify lldb-x86_64-ubuntu-14.04-cmake.
Author: spyffe Date: Thu May 11 18:38:21 2017 New Revision: 302850 URL: http://llvm.org/viewvc/llvm-project?rev=302850&view=rev Log: xfail TestClassTemplateParameterPack on gcc to mollify lldb-x86_64-ubuntu-14.04-cmake. Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py?rev=302850&r1=302849&r2=302850&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py Thu May 11 18:38:21 2017 @@ -4,4 +4,6 @@ from lldbsuite.test import decorators lldbinline.MakeInlineTest( __file__, globals(), [ decorators.expectedFailureAll( -oslist=["windows"], bugnumber="llvm.org/pr24764")]) +oslist=["windows"], bugnumber="llvm.org/pr24764"), +decorators.expectedFailureAll( +compiler="gcc")]) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added inline comments.
Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:70
+void TestClient::StopDebugger() {
+ Host::Kill(server_process_info.GetProcessID(), 15);
+}
jmajors wrote:
> labath wrote:
> > This is not portable.
> Is there a portable way of stopping?
`15` on my platform (NetBSD) is `SIGTERM`.
I've implemented similar feature in `NativeProcessNetBSD::Halt()`. Perhaps you
can call `Halt()` as well?
https://reviews.llvm.org/D32930
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added a comment. I can build locally with `make thread_inferior`, how to run it? https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added inline comments. Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:21 + + LLVM_BUILTIN_DEBUGTRAP; + delay = false; jmajors wrote: > zturner wrote: > > This will work on MSVC and presumably clang. I'm not sure about gcc. Is > > that sufficient for your needs? Do you know if gcc has the > > `__builtin_debugtrap` intrinsic? > Do we use gcc to build/test lldb? If not, then it shouldn't be an issue. If > we ever change our compiler of choice, we can always change this to match. Yes, we use and support GCC with libstdc++ to build LLDB including tests. At least on NetBSD. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added a comment. If I'm not mistaken LLVM `__builtin_debugtrap` is defined only for X86. It will also fail on GCC as there is no support for it. The NetBSD buildbot in LLDB runs GCC 5.x with libstdc++. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
krytarowski added a comment. Personally I would rely on LLDB feature to set software/hardware breakpoint in tracee, and never simulate debugtrap with some predefined value. This will ensure portability to more CPUs and operating systems. For example Sun SPARC CPUs (at least on NetBSD) won't pass PC after software breakpoint embedded into a tracee and will loop infinitely whenever we will try to resume execution. https://reviews.llvm.org/D32930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r302874 - Fix Linux Buildbot.
Author: zturner
Date: Fri May 12 00:48:54 2017
New Revision: 302874
URL: http://llvm.org/viewvc/llvm-project?rev=302874&view=rev
Log:
Fix Linux Buildbot.
Modified:
lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp
Modified: lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp?rev=302874&r1=302873&r2=302874&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp Fri May 12
00:48:54 2017
@@ -66,7 +66,7 @@ bool WorkaroundNeeded() {
Log *log = ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD);
::pid_t child_pid = fork();
if (child_pid == -1) {
-LLDB_LOG(log, "failed to fork(): {0}", Error(errno, eErrorTypePOSIX));
+LLDB_LOG(log, "failed to fork(): {0}", Status(errno, eErrorTypePOSIX));
return false;
}
if (child_pid == 0)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r302875 - Update StructuredData::String to return StringRefs.
Author: zturner
Date: Fri May 12 00:49:54 2017
New Revision: 302875
URL: http://llvm.org/viewvc/llvm-project?rev=302875&view=rev
Log:
Update StructuredData::String to return StringRefs.
It was returning const std::string& which was leading to
unnecessary copies all over the place, and preventing people
from doing things like Dict->GetValueForKeyAsString("foo", ref);
Modified:
lldb/trunk/include/lldb/Breakpoint/Breakpoint.h
lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h
lldb/trunk/include/lldb/Core/SearchFilter.h
lldb/trunk/include/lldb/Core/StructuredData.h
lldb/trunk/include/lldb/Target/ThreadSpec.h
lldb/trunk/include/lldb/Utility/UUID.h
lldb/trunk/source/API/SBThread.cpp
lldb/trunk/source/Breakpoint/Breakpoint.cpp
lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
lldb/trunk/source/Breakpoint/BreakpointResolver.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverAddress.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
lldb/trunk/source/Core/FormatEntity.cpp
lldb/trunk/source/Core/SearchFilter.cpp
lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/trunk/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.cpp
lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Target/Thread.cpp
lldb/trunk/source/Target/ThreadSpec.cpp
lldb/trunk/source/Utility/UUID.cpp
Modified: lldb/trunk/include/lldb/Breakpoint/Breakpoint.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/Breakpoint.h?rev=302875&r1=302874&r2=302875&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/Breakpoint.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/Breakpoint.h Fri May 12 00:49:54 2017
@@ -613,7 +613,7 @@ public:
lldb::SearchFilterSP GetSearchFilter() { return m_filter_sp; }
- bool AddName(const char *new_name, Status &error);
+ bool AddName(llvm::StringRef new_name, Status &error);
void RemoveName(const char *name_to_remove) {
if (name_to_remove)
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h?rev=302875&r1=302874&r2=302875&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h Fri May 12 00:49:54
2017
@@ -192,7 +192,7 @@ public:
static const char *ResolverTyToName(enum ResolverTy);
- static ResolverTy NameToResolverTy(const char *name);
+ static ResolverTy NameToResolverTy(llvm::StringRef name);
virtual lldb::BreakpointResolverSP
CopyForBreakpoint(Breakpoint &breakpoint) = 0;
Modified: lldb/trunk/include/lldb/Core/SearchFilter.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/SearchFilter.h?rev=302875&r1=302874&r2=302875&view=diff
==
--- lldb/trunk/include/lldb/Core/SearchFilter.h (original)
+++ lldb/trunk/include/lldb/Core/SearchFilter.h Fri May 12 00:49:54 2017
@@ -285,7 +285,7 @@ public:
static const char *FilterTyToName(enum FilterTy);
- static FilterTy NameToFilterTy(const char *name);
+ static FilterTy NameToFilterTy(llvm::StringRef name);
protected:
// Serialization of SearchFilter options:
Modified: lldb/trunk/include/lldb/Core/StructuredData.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/StructuredData.h?rev=302875&r1=302874&r2=302875&view=diff
==
--- lldb/trunk/include/lldb/Core/StructuredData.h (original)
+++ lldb/trunk/include/lldb/Core/StructuredData.h Fri May 12 00:49:54 2017
@@ -143,15 +143,12 @@ public:
: nullptr);
}
-std::string GetStringValue(const char *fail_value = nullptr) {
+llvm::StringRef GetStringValue(const char *fail_
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
zturner added inline comments.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:61-63
+string name;
+thread_info->GetValueForKeyAsString("name", name);
+string reason;
jmajors wrote:
> zturner wrote:
> > `StringRef name, reason;`
> There is no GetValueForKeyAsStringRef(). I need a std::string to populate.
As of r302875 this is no longer true.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:83-88
+ if (endian == LITTLE) {
+registers[register_id] = SwitchEndian(value_str);
+ }
+ else {
+registers[register_id] = stoul(value_str, nullptr, 16);
+ }
jmajors wrote:
> zturner wrote:
> > jmajors wrote:
> > > zturner wrote:
> > > > This code block is unnecessary.
> > > >
> > > > ```
> > > > unsigned long X;
> > > > if (!value_str.getAsInteger(X))
> > > > return some error;
> > > > llvm::support::endian::write(®isters[register_id], X, endian);
> > > > ```
> > > >
> > > > By using llvm endianness functions you can just delete the
> > > > `SwitchEndian()` function entirely, as it's not needed.
> > > These endian functions don't work here. getAsInteger() assumes the number
> > > is in human reading order (i.e. big endian). So it's converting the
> > > little endian string as if it's big, then converting that little endian
> > > long to another little endian long, which does nothing.
> > > I need something that reverses the string first.
> > >
> > > What do you think about adding a version of StringRef::getAsInteger that
> > > accepts an endianness parameter?
> > Hmm.. What about this?
> >
> > ```
> > unsigned long X;
> > if (!value_str.getAsInteger(X))
> > return some error;
> > sys::swapByteOrder(X);
> > ```
> >
> > It shouldn't matter if you swap the bytes in the string before doing the
> > translation, or swap the bytes in the number after doing the translation
> > should it?
> It doesn't matter when I do it. The issue with the other functions was they
> were converting target to host, when both were little. For string parsing, it
> needs target to big to string, or target to string to big.
Maybe I'm just missing something, but if you've got the string "ABCDEF" which
is a little endian string, then it is supposed to represent the number
0x00EFCDAB or 15,715,755 (assuming a 4 byte number). If you parse it as big
endian, which is what `getAsInteger` does, you're going to end up with the
number 0xABCDEF00 which is 2,882,400,000, which is |AB|CD|EF|00| (on a big
endian machine) or |00|EF|CD|AB| (on a little endian machine). If you then
swap the bytes, you're going to end up with |00|EF|CD|AB| (on a big endian
machine) or |AB|CD|EF|00| on a little endian machine. In both cases, these
represent the number 15,715,755 as desired.
It's possible I'm just getting confused here, but I don't see why the above
code sample doesn't work.
https://reviews.llvm.org/D32930
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
