[Lldb-commits] [PATCH] D27459: Add a more succinct logging syntax
labath added inline comments. Comment at: source/Core/Log.cpp:78 + char *text; + vasprintf(&text, format, args); + message << text; zturner wrote: > dancol wrote: > > I usually implement printf-into-std::string by using `vsnprintf` to figure > > out how many characters we generate, using `std::string::resize` to create > > a buffer of that many characters (unfortunately, zero-filling them), then > > `vsnprintf` directly into that buffer. This way, you only need one > > allocation. > > > > The currant approach involves at least three allocations: first, the string > > generated by `vasprintf`. Second, the internal `stringstream` buffer. > > Third, the copy of the buffer that `std::stringstream::str` generates. > > > > It's more expensive that it needs to be. > To be fair, we should really be deleting these methods long term, and using > `formatv`. This way you often end up with 0 allocations (for short > messages), and on top of that, only one underlying format call (as opposed to > wasting time calling `vasprintf` twice here). > The currant approach involves at least three allocations: first, the string > generated by vasprintf. Second, the internal stringstream buffer. Third, the > copy of the buffer that std::stringstream::str generates. `str()` returns a reference to the underlying buffer (this is an llvm stream, not std::stringstream), so there is no copy there. Since this is not really performance critical, I'd prefer to keep the code simpler (plus, I think the extra vsnprintf call balances the extra malloc, roughly speaking). And, same as Zachary, I hope this code will go away eventually. https://reviews.llvm.org/D27459 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r292360 - Add a more succinct logging syntax
Author: labath
Date: Wed Jan 18 05:00:26 2017
New Revision: 292360
URL: http://llvm.org/viewvc/llvm-project?rev=292360&view=rev
Log:
Add a more succinct logging syntax
This adds the LLDB_LOG macro, which enables one to write more succinct log
statements.
if (log)
log->Printf("log something: %d", var);
becomes
LLDB_LOG(log, "log something: {0}, var);
The macro still internally does the "if(log)" dance, so the arguments are only
evaluated if logging is enabled, meaning it has the same overhead as the
previous syntax.
Additionally, the log statements will be automatically prefixed with the file
and function generating the log (if the corresponding new argument to the "log
enable" command is enabled), so one does not need to manually specify this in
the log statement.
It also uses the new llvm formatv syntax, which means we don't have to worry
about PRIx64 macros and similar, and we can log complex object (llvm::StringRef,
lldb_private::Error, ...) more easily.
Differential Revision: https://reviews.llvm.org/D27459
Added:
lldb/trunk/unittests/Core/LogTest.cpp
Modified:
lldb/trunk/include/lldb/Core/Log.h
lldb/trunk/source/Commands/CommandObjectLog.cpp
lldb/trunk/source/Core/Log.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
lldb/trunk/unittests/Core/CMakeLists.txt
Modified: lldb/trunk/include/lldb/Core/Log.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Log.h?rev=292360&r1=292359&r2=292360&view=diff
==
--- lldb/trunk/include/lldb/Core/Log.h (original)
+++ lldb/trunk/include/lldb/Core/Log.h Wed Jan 18 05:00:26 2017
@@ -10,14 +10,6 @@
#ifndef liblldb_Log_h_
#define liblldb_Log_h_
-// C Includes
-#include
-#include
-#include
-#include
-
-// C++ Includes
-// Other libraries and framework includes
// Project includes
#include "lldb/Core/ConstString.h"
#include "lldb/Core/Flags.h"
@@ -25,7 +17,12 @@
#include "lldb/Core/PluginInterface.h"
#include "lldb/lldb-private.h"
+// Other libraries and framework includes
#include "llvm/Support/FormatVariadic.h"
+// C++ Includes
+#include
+#include
+// C Includes
//--
// Logging Options
@@ -39,6 +36,7 @@
#define LLDB_LOG_OPTION_PREPEND_THREAD_NAME (1U << 6)
#define LLDB_LOG_OPTION_BACKTRACE (1U << 7)
#define LLDB_LOG_OPTION_APPEND (1U << 8)
+#define LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION (1U << 9)
//--
// Logging Functions
@@ -109,8 +107,10 @@ public:
void PutCString(const char *cstr);
void PutString(llvm::StringRef str);
- template void Format(const char *fmt, Args &&... args) {
-PutString(llvm::formatv(fmt, std::forward(args)...).str());
+ template
+ void Format(llvm::StringRef file, llvm::StringRef function,
+ const char *format, Args &&... args) {
+Format(file, function, llvm::formatv(format, std::forward(args)...));
}
// CLEANUP: Add llvm::raw_ostream &Stream() function.
@@ -155,6 +155,13 @@ protected:
private:
DISALLOW_COPY_AND_ASSIGN(Log);
+
+ void WriteHeader(llvm::raw_ostream &OS, llvm::StringRef file,
+ llvm::StringRef function);
+ void WriteMessage(const std::string &message);
+
+ void Format(llvm::StringRef file, llvm::StringRef function,
+ const llvm::formatv_object_base &payload);
};
class LogChannel : public PluginInterface {
@@ -186,4 +193,11 @@ private:
} // namespace lldb_private
+#define LLDB_LOG(log, ...)
\
+ do {
\
+::lldb_private::Log *log_private = (log);
\
+if (log_private)
\
+ log_private->Format(__FILE__, __FUNCTION__, __VA_ARGS__);
\
+ } while (0)
+
#endif // liblldb_Log_h_
Modified: lldb/trunk/source/Commands/CommandObjectLog.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectLog.cpp?rev=292360&r1=292359&r2=292360&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectLog.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectLog.cpp Wed Jan 18 05:00:26 2017
@@ -48,6 +48,7 @@ static OptionDefinition g_log_options[]
{ LLDB_OPT_SET_1, false, "thread-name",'n', OptionParser::eNoArgument,
nullptr, nullptr, 0, eArgTypeNone, "Prepend all log lines with the thread
name for the thread that generates the log line." },
{ LLDB_OPT_SET_1, false, "stack", 'S', OptionParser::eNoArgument,
nullptr, nullptr, 0, eArgTypeNone, "Append a stack backtrace to each log
line." },
{ LLDB_OPT_SET_1, false, "append", 'a'
[Lldb-commits] [PATCH] D27459: Add a more succinct logging syntax
This revision was automatically updated to reflect the committed changes.
Closed by commit rL292360: Add a more succinct logging syntax (authored by
labath).
Changed prior to commit:
https://reviews.llvm.org/D27459?vs=84690&id=84814#toc
Repository:
rL LLVM
https://reviews.llvm.org/D27459
Files:
lldb/trunk/include/lldb/Core/Log.h
lldb/trunk/source/Commands/CommandObjectLog.cpp
lldb/trunk/source/Core/Log.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
lldb/trunk/unittests/Core/CMakeLists.txt
lldb/trunk/unittests/Core/LogTest.cpp
Index: lldb/trunk/include/lldb/Core/Log.h
===
--- lldb/trunk/include/lldb/Core/Log.h
+++ lldb/trunk/include/lldb/Core/Log.h
@@ -10,22 +10,19 @@
#ifndef liblldb_Log_h_
#define liblldb_Log_h_
-// C Includes
-#include
-#include
-#include
-#include
-
-// C++ Includes
-// Other libraries and framework includes
// Project includes
#include "lldb/Core/ConstString.h"
#include "lldb/Core/Flags.h"
#include "lldb/Core/Logging.h"
#include "lldb/Core/PluginInterface.h"
#include "lldb/lldb-private.h"
+// Other libraries and framework includes
#include "llvm/Support/FormatVariadic.h"
+// C++ Includes
+#include
+#include
+// C Includes
//--
// Logging Options
@@ -39,6 +36,7 @@
#define LLDB_LOG_OPTION_PREPEND_THREAD_NAME (1U << 6)
#define LLDB_LOG_OPTION_BACKTRACE (1U << 7)
#define LLDB_LOG_OPTION_APPEND (1U << 8)
+#define LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION (1U << 9)
//--
// Logging Functions
@@ -109,8 +107,10 @@
void PutCString(const char *cstr);
void PutString(llvm::StringRef str);
- template void Format(const char *fmt, Args &&... args) {
-PutString(llvm::formatv(fmt, std::forward(args)...).str());
+ template
+ void Format(llvm::StringRef file, llvm::StringRef function,
+ const char *format, Args &&... args) {
+Format(file, function, llvm::formatv(format, std::forward(args)...));
}
// CLEANUP: Add llvm::raw_ostream &Stream() function.
@@ -155,6 +155,13 @@
private:
DISALLOW_COPY_AND_ASSIGN(Log);
+
+ void WriteHeader(llvm::raw_ostream &OS, llvm::StringRef file,
+ llvm::StringRef function);
+ void WriteMessage(const std::string &message);
+
+ void Format(llvm::StringRef file, llvm::StringRef function,
+ const llvm::formatv_object_base &payload);
};
class LogChannel : public PluginInterface {
@@ -186,4 +193,11 @@
} // namespace lldb_private
+#define LLDB_LOG(log, ...) \
+ do { \
+::lldb_private::Log *log_private = (log); \
+if (log_private) \
+ log_private->Format(__FILE__, __FUNCTION__, __VA_ARGS__);\
+ } while (0)
+
#endif // liblldb_Log_h_
Index: lldb/trunk/unittests/Core/LogTest.cpp
===
--- lldb/trunk/unittests/Core/LogTest.cpp
+++ lldb/trunk/unittests/Core/LogTest.cpp
@@ -0,0 +1,56 @@
+//===-- LogTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Core/Log.h"
+#include "lldb/Core/StreamString.h"
+#include "lldb/Host/Host.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static std::string GetLogString(uint32_t log_options, const char *format,
+int arg) {
+ std::shared_ptr stream_sp(new StreamString());
+ Log log_(stream_sp);
+ log_.GetOptions().Reset(log_options);
+ Log *log = &log_;
+ LLDB_LOG(log, format, arg);
+ return stream_sp->GetString();
+}
+
+TEST(LogTest, LLDB_LOG_nullptr) {
+ Log *log = nullptr;
+ LLDB_LOG(log, "{0}", 0); // Shouldn't crash
+}
+
+TEST(LogTest, log_options) {
+ EXPECT_EQ("Hello World 47\n", GetLogString(0, "Hello World {0}", 47));
+ EXPECT_EQ("Hello World 47\n",
+GetLogString(LLDB_LOG_OPTION_THREADSAFE, "Hello World {0}", 47));
+
+ {
+std::string msg =
+GetLogString(LLDB_LOG_OPTION_PREPEND_SEQUENCE, "Hello World {0}", 47);
+int seq_no;
+EXPECT_EQ(1, sscanf(msg.c_str(), "%d Hello World 47", &seq_no));
+ }
+
+ EXPECT_EQ(
+ "LogTest.cpp:GetLogString Hello "
+ "World 47\n",
+ GetLogString(LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION, "Hello World {0}", 47));
+
+ EXPECT_EQ(llvm::formatv("[{0}/{1}] Hello Wor
[Lldb-commits] [lldb] r292364 - Fix windows build for previous commit
Author: labath Date: Wed Jan 18 06:29:51 2017 New Revision: 292364 URL: http://llvm.org/viewvc/llvm-project?rev=292364&view=rev Log: Fix windows build for previous commit We get an error about a redefinition of getcwd(). This seems to fix it. Modified: lldb/trunk/unittests/Core/LogTest.cpp Modified: lldb/trunk/unittests/Core/LogTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/LogTest.cpp?rev=292364&r1=292363&r2=292364&view=diff == --- lldb/trunk/unittests/Core/LogTest.cpp (original) +++ lldb/trunk/unittests/Core/LogTest.cpp Wed Jan 18 06:29:51 2017 @@ -7,10 +7,11 @@ // //===--===// +#include "gtest/gtest.h" + #include "lldb/Core/Log.h" #include "lldb/Core/StreamString.h" #include "lldb/Host/Host.h" -#include "gtest/gtest.h" using namespace lldb; using namespace lldb_private; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28808: Fix a bug where lldb does not respect the packet size.
abidh updated this revision to Diff 84817.
abidh added a comment.
Added the check to avoid integer underflow.
https://reviews.llvm.org/D28808
Files:
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2729,16 +2729,19 @@
size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size =
+ binary_memory_read ? m_max_memory_size : m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
char packet[64];
int packet_len;
- bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
packet_len = ::snprintf(packet, sizeof(packet), "%c%" PRIx64 ",%" PRIx64,
binary_memory_read ? 'x' : 'm', (uint64_t)addr,
(uint64_t)size);
@@ -2785,11 +2788,13 @@
size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf,
size_t size, Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size = m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
StreamString packet;
@@ -3988,6 +3993,20 @@
stub_max_size = reasonable_largeish_default;
}
+ // Memory packet have other overheads too like Maddr,size:#NN
+ // Instead of calculating the bytes taken by size and addr every
+ // time, we take a maximum guess here.
+ if (stub_max_size > 70)
+stub_max_size -= 32 + 32 + 6;
+ else {
+// In unlikely scenario that max packet size is less then 70, we will
+// hope that data being written is small enough to fit.
+Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+if (log)
+ log->Warning("Packet size is too small."
+ "LLDB may face problems while writing memory");
+ }
+
m_max_memory_size = stub_max_size;
} else {
m_max_memory_size = conservative_default;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2729,16 +2729,19 @@
size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size =
+ binary_memory_read ? m_max_memory_size : m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
char packet[64];
int packet_len;
- bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
packet_len = ::snprintf(packet, sizeof(packet), "%c%" PRIx64 ",%" PRIx64,
binary_memory_read ? 'x' : 'm', (uint64_t)addr,
(uint64_t)size);
@@ -2785,11 +2788,13 @@
size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf,
size_t size, Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size = m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
StreamString packet;
@@ -3988,6 +3993,20 @@
stub_max_size = reasonable_largeish_default;
}
+ // Memory packet have o
[Lldb-commits] [PATCH] D28808: Fix a bug where lldb does not respect the packet size.
abidh added inline comments. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3999 + // time, we take a maximum guess here. + stub_max_size -= 32 + 32 + 6; m_max_memory_size = stub_max_size; clayborg wrote: > You need to check "sub_max_size" here. What if it says 64? We will then use a > very large unsigned number UINT64_MAX - 6. Thanks for review. I had a check in my development branch then removed it as the scenario seemed unlikely and there was not a good way to return error from this function. I was inclined to put this check in DoMemoryWrite/read and check the exact size every time but it looks overkill. The check added in this revision probably should be enough. https://reviews.llvm.org/D28808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28858: Replace getcwd with the llvm equivalent
labath created this revision.
getcwd() is not available (well.. um.. deprecated?) on windows, and the way
PosixApi.h is providing it causes strange compile errors when it's included in
the wrong order. The best way to avoid that is to just not use chdir.
This replaces all uses of getcwd in generic code. There are still a couple of
more uses, but these are in platform-specific code.
chdir() is causing a similar problem, but for that there is no llvm equivalent
for that (yet).
https://reviews.llvm.org/D28858
Files:
include/lldb/Host/windows/PosixApi.h
source/Host/windows/Windows.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
source/Target/Platform.cpp
source/Target/ProcessLaunchInfo.cpp
source/Target/TargetList.cpp
Index: source/Target/TargetList.cpp
===
--- source/Target/TargetList.cpp
+++ source/Target/TargetList.cpp
@@ -7,11 +7,6 @@
//
//===--===//
-// C Includes
-// C++ Includes
-// Other libraries and framework includes
-#include "llvm/ADT/SmallString.h"
-
// Project includes
#include "lldb/Core/Broadcaster.h"
#include "lldb/Core/Debugger.h"
@@ -29,6 +24,10 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/TargetList.h"
+// Other libraries and framework includes
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+
using namespace lldb;
using namespace lldb_private;
@@ -369,12 +368,11 @@
if (file.IsRelative() && !user_exe_path.empty()) {
// Ignore paths that start with "./" and "../"
if (!user_exe_path.startswith("./") && !user_exe_path.startswith("../")) {
-char cwd[PATH_MAX];
-if (getcwd(cwd, sizeof(cwd))) {
- std::string cwd_user_exe_path(cwd);
- cwd_user_exe_path += '/';
- cwd_user_exe_path += user_exe_path;
- FileSpec cwd_file(cwd_user_exe_path, false);
+llvm::SmallString<64> cwd;
+if (! llvm::sys::fs::current_path(cwd)) {
+ cwd += '/';
+ cwd += user_exe_path;
+ FileSpec cwd_file(cwd, false);
if (cwd_file.Exists())
file = cwd_file;
}
Index: source/Target/ProcessLaunchInfo.cpp
===
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -23,6 +23,7 @@
#include "lldb/Target/Target.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/FileSystem.h"
#if !defined(_WIN32)
#include
@@ -368,10 +369,8 @@
if (working_dir) {
new_path += working_dir.GetPath();
} else {
-char current_working_dir[PATH_MAX];
-const char *cwd =
-getcwd(current_working_dir, sizeof(current_working_dir));
-if (cwd && cwd[0])
+llvm::SmallString<64> cwd;
+if (! llvm::sys::fs::current_path(cwd))
new_path += cwd;
}
std::string curr_path;
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -523,11 +523,11 @@
FileSpec Platform::GetWorkingDirectory() {
if (IsHost()) {
-char cwd[PATH_MAX];
-if (getcwd(cwd, sizeof(cwd)))
- return FileSpec{cwd, true};
-else
+llvm::SmallString<64> cwd;
+if (llvm::sys::fs::current_path(cwd))
return FileSpec{};
+else
+ return FileSpec(cwd, true);
} else {
if (!m_working_dir)
m_working_dir = GetRemoteWorkingDirectory();
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -356,12 +356,12 @@
// If this packet is sent to a platform, then change the current working
// directory
- char cwd[PATH_MAX];
- if (getcwd(cwd, sizeof(cwd)) == NULL)
-return SendErrorResponse(errno);
+ llvm::SmallString<64> cwd;
+ if (std::error_code ec = llvm::sys::fs::current_path(cwd))
+return SendErrorResponse(ec.value());
StreamString response;
- response.PutBytesAsRawHex8(cwd, strlen(cwd));
+ response.PutBytesAsRawHex8(cwd.data(), cwd.size());
return SendPacketNoLock(response.GetString());
}
Index: source/Host/windows/Windows.cpp
===
--- source/Host/windows/Windows.cpp
+++ source/Host/windows/Windows.cpp
@@ -23,10 +23,9 @@
#include
#include
-// These prototypes are defined in , but it also defines chdir() and
-// getcwd(), giving multiply defined errors
+// These prototypes are defined in , but it also defines chdir(),
+// giving multiply defined errors
extern "C" {
-char *_getcwd(char *buffe
[Lldb-commits] [PATCH] D28808: Fix a bug where lldb does not respect the packet size.
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
A few cleanups on the logging. See inlined comments.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4004
+// hope that data being written is small enough to fit.
+Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+if (log)
We should use a GDB remote log channel instead of the LLDB channel. Since this
is communication based I suggest:
```
Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_COMM|
GDBR_LOG_MEMORY));
```
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4006-4007
+if (log)
+ log->Warning("Packet size is too small."
+ "LLDB may face problems while writing memory");
+ }
Missing space between "small." and "LLDB". We should also warn once, not every
time.
https://reviews.llvm.org/D28808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r292414 - Fix new Log unit test
Author: labath
Date: Wed Jan 18 11:31:55 2017
New Revision: 292414
URL: http://llvm.org/viewvc/llvm-project?rev=292414&view=rev
Log:
Fix new Log unit test
the test was flaky because I specified the format string for the process id
incorrectly. This should fix it.
Modified:
lldb/trunk/unittests/Core/LogTest.cpp
Modified: lldb/trunk/unittests/Core/LogTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/LogTest.cpp?rev=292414&r1=292413&r2=292414&view=diff
==
--- lldb/trunk/unittests/Core/LogTest.cpp (original)
+++ lldb/trunk/unittests/Core/LogTest.cpp Wed Jan 18 11:31:55 2017
@@ -48,7 +48,7 @@ TEST(LogTest, log_options) {
"World 47\n",
GetLogString(LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION, "Hello World {0}",
47));
- EXPECT_EQ(llvm::formatv("[{0}/{1}] Hello World 47\n",
+ EXPECT_EQ(llvm::formatv("[{0,0+4}/{1,0+4}] Hello World 47\n",
Host::GetCurrentProcessID(),
Host::GetCurrentThreadID())
.str(),
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28808: Fix a bug where lldb does not respect the packet size.
abidh updated this revision to Diff 84857.
abidh added a comment.
Updated log calls as advised.
https://reviews.llvm.org/D28808
Files:
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2729,16 +2729,19 @@
size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size =
+ binary_memory_read ? m_max_memory_size : m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
char packet[64];
int packet_len;
- bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
packet_len = ::snprintf(packet, sizeof(packet), "%c%" PRIx64 ",%" PRIx64,
binary_memory_read ? 'x' : 'm', (uint64_t)addr,
(uint64_t)size);
@@ -2785,11 +2788,13 @@
size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf,
size_t size, Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size = m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
StreamString packet;
@@ -3988,6 +3993,21 @@
stub_max_size = reasonable_largeish_default;
}
+ // Memory packet have other overheads too like Maddr,size:#NN
+ // Instead of calculating the bytes taken by size and addr every
+ // time, we take a maximum guess here.
+ if (stub_max_size > 70)
+stub_max_size -= 32 + 32 + 6;
+ else {
+// In unlikely scenario that max packet size is less then 70, we will
+// hope that data being written is small enough to fit.
+Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(
+GDBR_LOG_COMM | GDBR_LOG_MEMORY));
+if (log)
+ log->Warning("Packet size is too small. "
+ "LLDB may face problems while writing memory");
+ }
+
m_max_memory_size = stub_max_size;
} else {
m_max_memory_size = conservative_default;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2729,16 +2729,19 @@
size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size =
+ binary_memory_read ? m_max_memory_size : m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
char packet[64];
int packet_len;
- bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
packet_len = ::snprintf(packet, sizeof(packet), "%c%" PRIx64 ",%" PRIx64,
binary_memory_read ? 'x' : 'm', (uint64_t)addr,
(uint64_t)size);
@@ -2785,11 +2788,13 @@
size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf,
size_t size, Error &error) {
GetMaxMemorySize();
- if (size > m_max_memory_size) {
+ // M and m packets take 2 bytes for 1 byte of memory
+ size_t max_memory_size = m_max_memory_size / 2;
+ if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
// lldb_private::Process so it is ok to do this.
-size = m_max_memory_size;
+size = max_memory_size;
}
StreamString packet;
@@ -3988,6 +3993,21 @@
stub_max_size = reasonable_largeish_default;
}
+ //
[Lldb-commits] [PATCH] D28808: Fix a bug where lldb does not respect the packet size.
abidh marked 2 inline comments as done.
abidh added inline comments.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4006-4007
+if (log)
+ log->Warning("Packet size is too small."
+ "LLDB may face problems while writing memory");
+ }
clayborg wrote:
> Missing space between "small." and "LLDB". We should also warn once, not
> every time.
This function has a check at the top so the packet size is checked only once.
https://reviews.llvm.org/D28808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r292454 - Fix a problem with the new dyld interface code -- when a new process
Author: jmolenda
Date: Wed Jan 18 18:20:29 2017
New Revision: 292454
URL: http://llvm.org/viewvc/llvm-project?rev=292454&view=rev
Log:
Fix a problem with the new dyld interface code -- when a new process
starts up, we need to clear the target's image list and only add
the binaries into the target that are actually present in this
process run.
Modified:
lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
Modified:
lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp?rev=292454&r1=292453&r2=292454&view=diff
==
--- lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
(original)
+++ lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
Wed Jan 18 18:20:29 2017
@@ -151,6 +151,11 @@ void DynamicLoaderMacOS::ClearNotificati
void DynamicLoaderMacOS::DoInitialImageFetch() {
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+ // Remove any binaries we pre-loaded in the Target before
launching/attaching.
+ // If the same binaries are present in the process, we'll get them from the
+ // shared module cache, we won't need to re-load them from disk.
+ UnloadAllImages();
+
StructuredData::ObjectSP all_image_info_json_sp(
m_process->GetLoadedDynamicLibrariesInfos());
ImageInfo::collection image_infos;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
