[Lldb-commits] [lldb] r351731 - Fix typos throughout the license files that somehow I and my reviewers

[Chandler Carruth via lldb-commits]

Author: chandlerc
Date: Mon Jan 21 01:52:34 2019
New Revision: 351731

URL: http://llvm.org/viewvc/llvm-project?rev=351731&view=rev
Log:
Fix typos throughout the license files that somehow I and my reviewers
all missed!

Thanks to Alex Bradbury for pointing this out, and the fact that I never
added the intended `legacy` anchor to the developer policy. Add that
anchor too. With hope, this will cause the links to all resolve
successfully.

Modified:
lldb/trunk/LICENSE.TXT

Modified: lldb/trunk/LICENSE.TXT
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/LICENSE.TXT?rev=351731&r1=351730&r2=351731&view=diff
==
--- lldb/trunk/LICENSE.TXT (original)
+++ lldb/trunk/LICENSE.TXT Mon Jan 21 01:52:34 2019
@@ -234,7 +234,7 @@ mechanisms:
file.
 
 ==
-Legacy LLVM License (ttps://llvm.org/docs/DeveloperPolicy.html#legacy):
+Legacy LLVM License (https://llvm.org/docs/DeveloperPolicy.html#legacy):
 ==
 University of Illinois/NCSA
 Open Source License


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: LLDB, jasonmolenda.
Herald added subscribers: lldb-commits, mgorny.

The field `m_decompression_scratch_type` is only used when 
`HAVE_LIBCOMPRESSION` is
defined, which caused a warning which I fixed in rLLDB350675 
 by just marking the variable as always 
used.

This patch fixes this in a better way by only defining the variable (and the 
related `m_decompression_scratch`
variable) when `HAVE_LIBCOMPRESSION` is defined. This also required changing 
the way we handle
`HAVE_LIBCOMPRESSION` works, as this was previously always defined on macOS 
within the source file
but not in the header. Now it's always defined from within our config header 
when CMake defines it or when
we are on macOS.

The field initialization was moved to the header to prevent that we have 
`#ifdef` within our initializer list.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D57011

Files:
  include/lldb/Host/Config.h.cmake
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h


Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -217,8 +217,10 @@
   HostThread m_listen_thread;
   std::string m_listen_url;
 
-  CompressionType m_decompression_scratch_type;
-  void *m_decompression_scratch;
+#if defined(HAVE_LIBCOMPRESSION)
+  CompressionType m_decompression_scratch_type = CompressionType::None;
+  void *m_decompression_scratch = nullptr;
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunication);
 };
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -41,8 +41,7 @@
 #define DEBUGSERVER_BASENAME "lldb-server"
 #endif
 
-#if defined(__APPLE__)
-#define HAVE_LIBCOMPRESSION
+#if defined(HAVE_LIBCOMPRESSION)
 #include 
 #endif
 
@@ -67,10 +66,7 @@
 #endif
   m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512),
   m_send_acks(true), m_compression_type(CompressionType::None),
-  m_listen_url(), m_decompression_scratch_type(CompressionType::None),
-  m_decompression_scratch(nullptr) {
-  // Unused unless HAVE_LIBCOMPRESSION is defined.
-  (void)m_decompression_scratch_type;
+  m_listen_url() {
 }
 
 //--
@@ -81,8 +77,10 @@
 Disconnect();
   }
 
+#if defined(HAVE_LIBCOMPRESSION)
   if (m_decompression_scratch)
 free (m_decompression_scratch);
+#endif
 
   // Stop the communications read thread which is used to parse all incoming
   // packets.  This function will block until the read thread returns.
Index: include/lldb/Host/Config.h.cmake
===
--- include/lldb/Host/Config.h.cmake
+++ include/lldb/Host/Config.h.cmake
@@ -33,4 +33,10 @@
 #cmakedefine HAVE_LIBCOMPRESSION
 #endif
 
+#ifndef HAVE_LIBCOMPRESSION
+#ifdef __APPLE__
+#define HAVE_LIBCOMPRESSION
+#endif
+#endif
+
 #endif // #ifndef LLDB_HOST_CONFIG_H


Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -217,8 +217,10 @@
   HostThread m_listen_thread;
   std::string m_listen_url;
 
-  CompressionType m_decompression_scratch_type;
-  void *m_decompression_scratch;
+#if defined(HAVE_LIBCOMPRESSION)
+  CompressionType m_decompression_scratch_type = CompressionType::None;
+  void *m_decompression_scratch = nullptr;
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunication);
 };
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -41,8 +41,7 @@
 #define DEBUGSERVER_BASENAME "lldb-server"
 #endif
 
-#if defined(__APPLE__)
-#define HAVE_LIBCOMPRESSION
+#if defined(HAVE_LIBCOMPRESSION)
 #include 
 #endif
 
@@ -67,10 +66,7 @@
 #endif
   m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512),
   m_send_acks(true), m_compression_type(CompressionType::None),
-  m_listen_url(), m_decompression_scratch_type(CompressionType::None),
-  m_decompression_scratch(nullptr) {
-  // Unused unless HAVE_LIBCOMPRESSION is defined.
-  (void)m_decompression_scratch_type;
+  m_listen_url() {
 }
 
 //--
@@ -81,8 +77,10 @@
  

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I currently don't have access to a macOS machine and this only affects macOS, 
so I would appreciate if someone could check if this still compiles/works as 
intended. Otherwise I can also just commit it and watch the macOS bots.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57011/new/

https://reviews.llvm.org/D57011



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: beanz, labath.
labath added a reviewer: sgraenitz.
labath added inline comments.



Comment at: include/lldb/Host/Config.h.cmake:32-40
 #ifndef HAVE_LIBCOMPRESSION
 #cmakedefine HAVE_LIBCOMPRESSION
 #endif
 
+#ifndef HAVE_LIBCOMPRESSION
+#ifdef __APPLE__
+#define HAVE_LIBCOMPRESSION

Why is this even necessary? If libcompression is always available on apple 
platforms, then the cmake check should always succeed, and there should be no 
need to fix it up. Maybe the check needs to be fixed/improved?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57011/new/

https://reviews.llvm.org/D57011



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Raphael Isemann via Phabricator via lldb-commits]

teemperor marked an inline comment as done.
teemperor added inline comments.



Comment at: include/lldb/Host/Config.h.cmake:32-40
 #ifndef HAVE_LIBCOMPRESSION
 #cmakedefine HAVE_LIBCOMPRESSION
 #endif
 
+#ifndef HAVE_LIBCOMPRESSION
+#ifdef __APPLE__
+#define HAVE_LIBCOMPRESSION

labath wrote:
> Why is this even necessary? If libcompression is always available on apple 
> platforms, then the cmake check should always succeed, and there should be no 
> need to fix it up. Maybe the check needs to be fixed/improved?
I don't think I found a good answer for that in the git log. Maybe it's a 
workaround for the Xcode project?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57011/new/

https://reviews.llvm.org/D57011



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Host/Config.h.cmake:32-40
 #ifndef HAVE_LIBCOMPRESSION
 #cmakedefine HAVE_LIBCOMPRESSION
 #endif
 
+#ifndef HAVE_LIBCOMPRESSION
+#ifdef __APPLE__
+#define HAVE_LIBCOMPRESSION

teemperor wrote:
> labath wrote:
> > Why is this even necessary? If libcompression is always available on apple 
> > platforms, then the cmake check should always succeed, and there should be 
> > no need to fix it up. Maybe the check needs to be fixed/improved?
> I don't think I found a good answer for that in the git log. Maybe it's a 
> workaround for the Xcode project?
That shouldn't be necessary. XCode has a special version of this file at 
`include/lldb/Host/Config.h`. However, it looks like this macro is not defined 
there. So maybe all that's needed is to hardcode `#define HAVE_LIBCOMPRESSION` 
there?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57011/new/

https://reviews.llvm.org/D57011



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 182797.
teemperor added a comment.

- Moved define to Xcode's Config.h (Thanks Pavel!)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57011/new/

https://reviews.llvm.org/D57011

Files:
  include/lldb/Host/Config.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h


Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -217,8 +217,10 @@
   HostThread m_listen_thread;
   std::string m_listen_url;
 
-  CompressionType m_decompression_scratch_type;
-  void *m_decompression_scratch;
+#if defined(HAVE_LIBCOMPRESSION)
+  CompressionType m_decompression_scratch_type = CompressionType::None;
+  void *m_decompression_scratch = nullptr;
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunication);
 };
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -41,8 +41,7 @@
 #define DEBUGSERVER_BASENAME "lldb-server"
 #endif
 
-#if defined(__APPLE__)
-#define HAVE_LIBCOMPRESSION
+#if defined(HAVE_LIBCOMPRESSION)
 #include 
 #endif
 
@@ -67,10 +66,7 @@
 #endif
   m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512),
   m_send_acks(true), m_compression_type(CompressionType::None),
-  m_listen_url(), m_decompression_scratch_type(CompressionType::None),
-  m_decompression_scratch(nullptr) {
-  // Unused unless HAVE_LIBCOMPRESSION is defined.
-  (void)m_decompression_scratch_type;
+  m_listen_url() {
 }
 
 //--
@@ -81,8 +77,10 @@
 Disconnect();
   }
 
+#if defined(HAVE_LIBCOMPRESSION)
   if (m_decompression_scratch)
 free (m_decompression_scratch);
+#endif
 
   // Stop the communications read thread which is used to parse all incoming
   // packets.  This function will block until the read thread returns.
Index: include/lldb/Host/Config.h
===
--- include/lldb/Host/Config.h
+++ include/lldb/Host/Config.h
@@ -25,6 +25,8 @@
 
 #define HAVE_SIGACTION 1
 
+#define HAVE_LIBCOMPRESSION 1
+
 #else
 
 #error This file is only used by the Xcode build.


Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -217,8 +217,10 @@
   HostThread m_listen_thread;
   std::string m_listen_url;
 
-  CompressionType m_decompression_scratch_type;
-  void *m_decompression_scratch;
+#if defined(HAVE_LIBCOMPRESSION)
+  CompressionType m_decompression_scratch_type = CompressionType::None;
+  void *m_decompression_scratch = nullptr;
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunication);
 };
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -41,8 +41,7 @@
 #define DEBUGSERVER_BASENAME "lldb-server"
 #endif
 
-#if defined(__APPLE__)
-#define HAVE_LIBCOMPRESSION
+#if defined(HAVE_LIBCOMPRESSION)
 #include 
 #endif
 
@@ -67,10 +66,7 @@
 #endif
   m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512),
   m_send_acks(true), m_compression_type(CompressionType::None),
-  m_listen_url(), m_decompression_scratch_type(CompressionType::None),
-  m_decompression_scratch(nullptr) {
-  // Unused unless HAVE_LIBCOMPRESSION is defined.
-  (void)m_decompression_scratch_type;
+  m_listen_url() {
 }
 
 //--
@@ -81,8 +77,10 @@
 Disconnect();
   }
 
+#if defined(HAVE_LIBCOMPRESSION)
   if (m_decompression_scratch)
 free (m_decompression_scratch);
+#endif
 
   // Stop the communications read thread which is used to parse all incoming
   // packets.  This function will block until the read thread returns.
Index: include/lldb/Host/Config.h
===
--- include/lldb/Host/Config.h
+++ include/lldb/Host/Config.h
@@ -25,6 +25,8 @@
 
 #define HAVE_SIGACTION 1
 
+#define HAVE_LIBCOMPRESSION 1
+
 #else
 
 #error This file is only used by the Xcode build.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This looks much better. Thanks.

(I haven't tested on a mac, but it doesn't seem like a particularly dangerous 
change to make).

Maybe, if this sticks, https://reviews.llvm.org/D48977 could be backed out too?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57011/new/

https://reviews.llvm.org/D57011



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

I am sorry for being such a pain, but I don't think this is grammatically 
correct, as `get` and `flatten` are both verbs. I'd call this either 
GetFlatten**ed**WindowsCommandLine or just plain FlattenWindowsCommandLine.



Comment at: source/Utility/Args.cpp:252
+
+  command = llvm::sys::flattenWindowsCommandLine(args);
+  return true;

This will not compile on non-windows systems as `flattenWindowsCommandLine` is 
`#ifdef _WIN32`. We can either try removing the ifdefs from the llvm function, 
or put this code somewhere under `Host/windows` (it could just be a static 
function in ProcessLauncherWindows).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

Why `lldb_private::Flags` is required? `std::bitset` provides the same 
functionality and even more.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55718/new/

https://reviews.llvm.org/D55718



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Hui Huang via Phabricator via lldb-commits]

Hui added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

labath wrote:
> I am sorry for being such a pain, but I don't think this is grammatically 
> correct, as `get` and `flatten` are both verbs. I'd call this either 
> GetFlatten**ed**WindowsCommandLine or just plain FlattenWindowsCommandLine.
There are  two kinds of sources of args in this discussion:

  - from lldb console through stdin which is not raw.
  - from LLDB_SERVER_LOG_FILE environment variables which are raw

We expect to call a GetFlattenedWindowsCommandString for raw input. However it 
seems not the case for now. 

What about adding a TODO in the following in ProcessWindowsLauncher. 

Would like a solution to proceed.

+
+bool GetFlattenedWindowsCommandString(Args args, std::string &command) {
+  if (args.empty())
+return false;
+
+  std::vector args_ref;
+  for (auto &entry : args.entries())
+ args_ref.push_back(entry.ref);
+
+  // TODO: only flatten raw input.
+  // Inputs from lldb console through the stdin are not raw, for example,
+  // A command line like "dir c:\" is attained as "dir c":\\". Trying to 
flatten such
+  // input will result in unexpected errors. In this case, the flattend string 
will be
+  // interpreted as "dir c:\\ which is a wrong usage of `dir` command.
+
+  command = llvm::sys::flattenWindowsCommandLine(args_ref);
+  return true;
 }
+} // namespace



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added inline comments.



Comment at: source/Plugins/Architecture/Arc/ArchitectureArc.cpp:29
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+"ARC-specific algorithms",
+&ArchitectureArc::Create);

clayborg wrote:
> Not a great human readable architecture name here. All other plug-ins use the 
> short architecture name ("arm", "mipc", "ppc64"). Best to just use "arc"?
You are writing about GetPluginNameStatic. The description string is same as 
for other plug-ins (just copy-pasted).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55718/new/

https://reviews.llvm.org/D55718



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56844: Breakpad: Extract parsing code into a separate file

[Pavel Labath via Phabricator via lldb-commits]

labath marked 7 inline comments as done.
labath added a comment.

Thanks for the review. I'll create another review with the changes stemming 
from this.




Comment at: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:74
+  static_assert(sizeof(data) == 20, "");
+  // The textual module id encoding should be between 33 and 40 bytes long,
+  // depending on the size of the age field, which is of variable length.

lemo wrote:
> the comment is great, but I think we should still have symbolic constants for 
> all these magic values
I just had an idea on how to remove the numbers altogether. I'll create a 
separate patch to see what you think.



Comment at: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:34
+
+  ~Record() = default;
+

lemo wrote:
> should this be virtual? (even though the class doesn't have other virtual 
> members, the class hierarchy introduces the possibility for consumers to 
> treat them a polymorphic types - ex. storing as Record* and use the kind type 
> to figure out the concrete type)
Yes, but they won't be able to polymorphically delete these objects through a 
base pointer, as the base destructor is protected.  With a protected 
destructor, the compiler wouldn't warn here even if the class had other virtual 
members.

(BTW, this is one of the reasons I gave up on a generic "parse" function - it'd 
force heap allocations for polymorphic treatment without any big benefit).



Comment at: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:40
+private:
+  Kind TheKind;
+};

lemo wrote:
> Just curious, what is the definitive convention for naming data members? A 
> lot of LLDB code uses the m_camelCase convention.
There isn't one. :/

The reason I used the llvm naming convention here, is because this was a fresh 
class, which didn't have to interact with lldb at all. That's why I could (and 
wanted to) follow llvm coding practices as best as I could. Using lldb names 
for things felt weird here. 



Comment at: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h:84
+
+class PublicRecord : public Record {
+public:

lemo wrote:
> most of these types are just plain data containers, so why not make them 
> structs? (and avoid all the boilerplate associated with public accessors, 
> repetitive constructors, ...)
I thought about that too, but I couldn't really make up my mind. In the end I 
did this, but I can't say I had a good reason for it. I think I can change the 
members to be public, given that you had the same thought.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56844/new/

https://reviews.llvm.org/D56844



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added a reviewer: lemo.

This addresses the issues raised in D56844 . 
It removes the accessors from the
breakpad record structures by making the fields public. Also, I refactor the
UUID parsing code to remove hard-coded constants.


https://reviews.llvm.org/D57037

Files:
  source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -187,19 +187,19 @@
   LLDB_LOG(log, "Failed to parse: {0}. Skipping record.", line);
   continue;
 }
-addr_t file_address = base + record->getAddress();
+addr_t file_address = base + record->Address;
 
 SectionSP section_sp = list.FindSectionContainingFileAddress(file_address);
 if (!section_sp) {
   LLDB_LOG(log,
"Ignoring symbol {0}, whose address ({1}) is outside of the "
"object file. Mismatched symbol file?",
-   record->getName(), file_address);
+   record->Name, file_address);
   continue;
 }
 
 symtab.AddSymbol(Symbol(
-/*symID*/ 0, Mangled(record->getName(), /*is_mangled*/ false),
+/*symID*/ 0, Mangled(record->Name, /*is_mangled*/ false),
 eSymbolTypeCode,
 /*is_global*/ true, /*is_debug*/ false, /*is_trampoline*/ false,
 /*is_artificial*/ false,
Index: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
===
--- source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
+++ source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
@@ -33,13 +33,13 @@
 return llvm::None;
 
   llvm::Triple triple;
-  triple.setArch(Module->getArch());
-  triple.setOS(Module->getOS());
+  triple.setArch(Module->Arch);
+  triple.setOS(Module->OS);
 
   std::tie(line, text) = text.split('\n');
 
   auto Info = InfoRecord::parse(line);
-  UUID uuid = Info && Info->getID() ? Info->getID() : Module->getID();
+  UUID uuid = Info && Info->ID ? Info->ID : Module->ID;
   return Header{ArchSpec(triple), std::move(uuid)};
 }
 
Index: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
===
--- source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
+++ source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
@@ -52,17 +52,14 @@
   ModuleRecord(llvm::Triple::OSType OS, llvm::Triple::ArchType Arch, UUID ID)
   : Record(Module), OS(OS), Arch(Arch), ID(std::move(ID)) {}
 
-  llvm::Triple::OSType getOS() const { return OS; }
-  llvm::Triple::ArchType getArch() const { return Arch; }
-  const UUID &getID() const { return ID; }
-
-private:
   llvm::Triple::OSType OS;
   llvm::Triple::ArchType Arch;
   UUID ID;
 };
 
-bool operator==(const ModuleRecord &L, const ModuleRecord &R);
+bool operator==(const ModuleRecord &L, const ModuleRecord &R) {
+  return L.OS == R.OS && L.Arch == R.Arch && L.ID == R.ID;
+}
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const ModuleRecord &R);
 
 class InfoRecord : public Record {
@@ -70,14 +67,11 @@
   static llvm::Optional parse(llvm::StringRef Line);
   InfoRecord(UUID ID) : Record(Info), ID(std::move(ID)) {}
 
-  const UUID &getID() const { return ID; }
-
-private:
   UUID ID;
 };
 
 inline bool operator==(const InfoRecord &L, const InfoRecord &R) {
-  return L.getID() == R.getID();
+  return L.ID == R.ID;
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const InfoRecord &R);
 
@@ -89,12 +83,6 @@
   : Record(Module), Multiple(Multiple), Address(Address),
 ParamSize(ParamSize), Name(Name) {}
 
-  bool getMultiple() const { return Multiple; }
-  lldb::addr_t getAddress() const { return Address; }
-  lldb::addr_t getParamSize() const { return ParamSize; }
-  llvm::StringRef getName() const { return Name; }
-
-private:
   bool Multiple;
   lldb::addr_t Address;
   lldb::addr_t ParamSize;
Index: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
===
--- source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
+++ source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
@@ -57,17 +57,31 @@
   .Default(Triple::UnknownArch);
 }
 
-static llvm::StringRef consume_front(llvm::StringRef &str, size_t n) {
-  llvm::StringRef result = str.take_front(n);
-  str = str.drop_front(n);
-  return result;
+/// Return the number of hex digits needed to encode an (POD) object of a given
+/// type.
+template  static constexpr size_t hex_digits() {
+  return 2 * sizeof(T);
+}
+
+/// Consume the right number of digits from the input StringRef a

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

Hui wrote:
> labath wrote:
> > I am sorry for being such a pain, but I don't think this is grammatically 
> > correct, as `get` and `flatten` are both verbs. I'd call this either 
> > GetFlatten**ed**WindowsCommandLine or just plain FlattenWindowsCommandLine.
> There are  two kinds of sources of args in this discussion:
> 
>   - from lldb console through stdin which is not raw.
>   - from LLDB_SERVER_LOG_FILE environment variables which are raw
> 
> We expect to call a GetFlattenedWindowsCommandString for raw input. However 
> it seems not the case for now. 
> 
> What about adding a TODO in the following in ProcessWindowsLauncher. 
> 
> Would like a solution to proceed.
> 
> +
> +bool GetFlattenedWindowsCommandString(Args args, std::string &command) {
> +  if (args.empty())
> +return false;
> +
> +  std::vector args_ref;
> +  for (auto &entry : args.entries())
> + args_ref.push_back(entry.ref);
> +
> +  // TODO: only flatten raw input.
> +  // Inputs from lldb console through the stdin are not raw, for example,
> +  // A command line like "dir c:\" is attained as "dir c":\\". Trying to 
> flatten such
> +  // input will result in unexpected errors. In this case, the flattend 
> string will be
> +  // interpreted as "dir c:\\ which is a wrong usage of `dir` command.
> +
> +  command = llvm::sys::flattenWindowsCommandLine(args_ref);
> +  return true;
>  }
> +} // namespace
> 
I am afraid you lost me here. By the time something gets to this function, it 
has already been parsed into individual argv strings. I am not sure which ones 
do you consider raw, or why should it make a difference. (lldb builtin 
interpreter has a concept of "raw" commands, but I don't think that's what you 
mean here)

This function should take those argv strings, no matter what they are, and 
recompose them into a single string. If those strings are wrong, then it's 
output will also be wrong, but that's not a problem of this function -- it's a 
problem of whoever parsed those strings.

I won't have access to a windows machine this week to check this out, but I can 
take a look at that next week. In the mean time, I would be fine with just 
xfailing the single test which fails because of this. I think that's a good 
tradeoff, as this would fix a bunch of other tests as well as unblock you on 
your lldb-server work.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

labath wrote:
> Hui wrote:
> > labath wrote:
> > > I am sorry for being such a pain, but I don't think this is grammatically 
> > > correct, as `get` and `flatten` are both verbs. I'd call this either 
> > > GetFlatten**ed**WindowsCommandLine or just plain 
> > > FlattenWindowsCommandLine.
> > There are  two kinds of sources of args in this discussion:
> > 
> >   - from lldb console through stdin which is not raw.
> >   - from LLDB_SERVER_LOG_FILE environment variables which are raw
> > 
> > We expect to call a GetFlattenedWindowsCommandString for raw input. However 
> > it seems not the case for now. 
> > 
> > What about adding a TODO in the following in ProcessWindowsLauncher. 
> > 
> > Would like a solution to proceed.
> > 
> > +
> > +bool GetFlattenedWindowsCommandString(Args args, std::string &command) {
> > +  if (args.empty())
> > +return false;
> > +
> > +  std::vector args_ref;
> > +  for (auto &entry : args.entries())
> > + args_ref.push_back(entry.ref);
> > +
> > +  // TODO: only flatten raw input.
> > +  // Inputs from lldb console through the stdin are not raw, for example,
> > +  // A command line like "dir c:\" is attained as "dir c":\\". Trying to 
> > flatten such
> > +  // input will result in unexpected errors. In this case, the flattend 
> > string will be
> > +  // interpreted as "dir c:\\ which is a wrong usage of `dir` command.
> > +
> > +  command = llvm::sys::flattenWindowsCommandLine(args_ref);
> > +  return true;
> >  }
> > +} // namespace
> > 
> I am afraid you lost me here. By the time something gets to this function, it 
> has already been parsed into individual argv strings. I am not sure which 
> ones do you consider raw, or why should it make a difference. (lldb builtin 
> interpreter has a concept of "raw" commands, but I don't think that's what 
> you mean here)
> 
> This function should take those argv strings, no matter what they are, and 
> recompose them into a single string. If those strings are wrong, then it's 
> output will also be wrong, but that's not a problem of this function -- it's 
> a problem of whoever parsed those strings.
> 
> I won't have access to a windows machine this week to check this out, but I 
> can take a look at that next week. In the mean time, I would be fine with 
> just xfailing the single test which fails because of this. I think that's a 
> good tradeoff, as this would fix a bunch of other tests as well as unblock 
> you on your lldb-server work.
I can't test this out, but the place I'd expect the problem to be is in 
`ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell`. This function seems 
to be blissfully unaware of the windows quoting rules, and yet it's the one 
that turns `platform shell dir c:\` into `cmd`, `/c` `whatever`. The flatten 
function then gets this argv array and flattens it one more time. I am not sure 
if that's the right thing to do on windows.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Hui Huang via Phabricator via lldb-commits]

Hui added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

labath wrote:
> labath wrote:
> > Hui wrote:
> > > labath wrote:
> > > > I am sorry for being such a pain, but I don't think this is 
> > > > grammatically correct, as `get` and `flatten` are both verbs. I'd call 
> > > > this either GetFlatten**ed**WindowsCommandLine or just plain 
> > > > FlattenWindowsCommandLine.
> > > There are  two kinds of sources of args in this discussion:
> > > 
> > >   - from lldb console through stdin which is not raw.
> > >   - from LLDB_SERVER_LOG_FILE environment variables which are raw
> > > 
> > > We expect to call a GetFlattenedWindowsCommandString for raw input. 
> > > However it seems not the case for now. 
> > > 
> > > What about adding a TODO in the following in ProcessWindowsLauncher. 
> > > 
> > > Would like a solution to proceed.
> > > 
> > > +
> > > +bool GetFlattenedWindowsCommandString(Args args, std::string &command) {
> > > +  if (args.empty())
> > > +return false;
> > > +
> > > +  std::vector args_ref;
> > > +  for (auto &entry : args.entries())
> > > + args_ref.push_back(entry.ref);
> > > +
> > > +  // TODO: only flatten raw input.
> > > +  // Inputs from lldb console through the stdin are not raw, for example,
> > > +  // A command line like "dir c:\" is attained as "dir c":\\". Trying to 
> > > flatten such
> > > +  // input will result in unexpected errors. In this case, the flattend 
> > > string will be
> > > +  // interpreted as "dir c:\\ which is a wrong usage of `dir` command.
> > > +
> > > +  command = llvm::sys::flattenWindowsCommandLine(args_ref);
> > > +  return true;
> > >  }
> > > +} // namespace
> > > 
> > I am afraid you lost me here. By the time something gets to this function, 
> > it has already been parsed into individual argv strings. I am not sure 
> > which ones do you consider raw, or why should it make a difference. (lldb 
> > builtin interpreter has a concept of "raw" commands, but I don't think 
> > that's what you mean here)
> > 
> > This function should take those argv strings, no matter what they are, and 
> > recompose them into a single string. If those strings are wrong, then it's 
> > output will also be wrong, but that's not a problem of this function -- 
> > it's a problem of whoever parsed those strings.
> > 
> > I won't have access to a windows machine this week to check this out, but I 
> > can take a look at that next week. In the mean time, I would be fine with 
> > just xfailing the single test which fails because of this. I think that's a 
> > good tradeoff, as this would fix a bunch of other tests as well as unblock 
> > you on your lldb-server work.
> I can't test this out, but the place I'd expect the problem to be is in 
> `ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell`. This function seems 
> to be blissfully unaware of the windows quoting rules, and yet it's the one 
> that turns `platform shell dir c:\` into `cmd`, `/c` `whatever`. The flatten 
> function then gets this argv array and flattens it one more time. I am not 
> sure if that's the right thing to do on windows.
It is observed when you type "dir c:\" on lldb console, the 
IOHandlerEditline::GetLine will yield "dir c:\\" for you through the standard 
input (I skipped the new line char '\n'). And the 
CommandInterpreter::ResolveCommandImpl won't get the raw command line string, i 
mean "dir c:\", even I force WantsRawCommandString() true to get one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

Hui wrote:
> labath wrote:
> > labath wrote:
> > > Hui wrote:
> > > > labath wrote:
> > > > > I am sorry for being such a pain, but I don't think this is 
> > > > > grammatically correct, as `get` and `flatten` are both verbs. I'd 
> > > > > call this either GetFlatten**ed**WindowsCommandLine or just plain 
> > > > > FlattenWindowsCommandLine.
> > > > There are  two kinds of sources of args in this discussion:
> > > > 
> > > >   - from lldb console through stdin which is not raw.
> > > >   - from LLDB_SERVER_LOG_FILE environment variables which are raw
> > > > 
> > > > We expect to call a GetFlattenedWindowsCommandString for raw input. 
> > > > However it seems not the case for now. 
> > > > 
> > > > What about adding a TODO in the following in ProcessWindowsLauncher. 
> > > > 
> > > > Would like a solution to proceed.
> > > > 
> > > > +
> > > > +bool GetFlattenedWindowsCommandString(Args args, std::string &command) 
> > > > {
> > > > +  if (args.empty())
> > > > +return false;
> > > > +
> > > > +  std::vector args_ref;
> > > > +  for (auto &entry : args.entries())
> > > > + args_ref.push_back(entry.ref);
> > > > +
> > > > +  // TODO: only flatten raw input.
> > > > +  // Inputs from lldb console through the stdin are not raw, for 
> > > > example,
> > > > +  // A command line like "dir c:\" is attained as "dir c":\\". Trying 
> > > > to flatten such
> > > > +  // input will result in unexpected errors. In this case, the 
> > > > flattend string will be
> > > > +  // interpreted as "dir c:\\ which is a wrong usage of `dir` command.
> > > > +
> > > > +  command = llvm::sys::flattenWindowsCommandLine(args_ref);
> > > > +  return true;
> > > >  }
> > > > +} // namespace
> > > > 
> > > I am afraid you lost me here. By the time something gets to this 
> > > function, it has already been parsed into individual argv strings. I am 
> > > not sure which ones do you consider raw, or why should it make a 
> > > difference. (lldb builtin interpreter has a concept of "raw" commands, 
> > > but I don't think that's what you mean here)
> > > 
> > > This function should take those argv strings, no matter what they are, 
> > > and recompose them into a single string. If those strings are wrong, then 
> > > it's output will also be wrong, but that's not a problem of this function 
> > > -- it's a problem of whoever parsed those strings.
> > > 
> > > I won't have access to a windows machine this week to check this out, but 
> > > I can take a look at that next week. In the mean time, I would be fine 
> > > with just xfailing the single test which fails because of this. I think 
> > > that's a good tradeoff, as this would fix a bunch of other tests as well 
> > > as unblock you on your lldb-server work.
> > I can't test this out, but the place I'd expect the problem to be is in 
> > `ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell`. This function 
> > seems to be blissfully unaware of the windows quoting rules, and yet it's 
> > the one that turns `platform shell dir c:\` into `cmd`, `/c` `whatever`. 
> > The flatten function then gets this argv array and flattens it one more 
> > time. I am not sure if that's the right thing to do on windows.
> It is observed when you type "dir c:\" on lldb console, the 
> IOHandlerEditline::GetLine will yield "dir c:\\" for you through the standard 
> input (I skipped the new line char '\n'). And the 
> CommandInterpreter::ResolveCommandImpl won't get the raw command line string, 
> i mean "dir c:\", even I force WantsRawCommandString() true to get one.
How are you observing that? Are you sure that's what happens, and it's not just 
the lldb formatting of c strings (when lldb displays a `const char *` variable, 
it will automatically escape any backslashes within, which means the 
backslashes will appear doubled).

I'd be surprised if extra backslashes appear at this level (they certainly 
don't on linux), as plenty of other things would fail if this didn't work.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r351779 - [Test] Fix up tests affected by the new LLVM header.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Jan 21 19:50:44 2019
New Revision: 351779

URL: http://llvm.org/viewvc/llvm-project?rev=351779&view=rev
Log:
[Test] Fix up tests affected by the new LLVM header.

The new LLVM header is one line shorter than the old one, which lead to
some test failures. Ideally tests should rely on line numbers for
breakpoints or output, but that's a different discussion. Hopefully this
turns the bots green again.

Modified:
lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in
lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py

Modified: lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit?rev=351779&r1=351778&r2=351779&view=diff
==
--- lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit (original)
+++ lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit Mon Jan 21 
19:50:44 2019
@@ -1 +1 @@
-target stop-hook add -f stop-hook.c -l 30 -e 34 -o "expr ptr"
\ No newline at end of file
+target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
\ No newline at end of file

Modified: lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit?rev=351779&r1=351778&r2=351779&view=diff
==
--- lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit (original)
+++ lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit Mon Jan 21 
19:50:44 2019
@@ -1,3 +1,3 @@
-target stop-hook add -f stop-hook.c -l 30 -e 34
+target stop-hook add -f stop-hook.c -l 29 -e 34
 expr ptr
 DONE
\ No newline at end of file

Modified: lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in?rev=351779&r1=351778&r2=351779&view=diff
==
--- lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in (original)
+++ lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in Mon Jan 21 19:50:44 
2019
@@ -1,4 +1,4 @@
-breakpoint set -f simple.c -l 13
+breakpoint set -f simple.c -l 12
 run
 bt
 cont

Modified: lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in?rev=351779&r1=351778&r2=351779&view=diff
==
--- lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in (original)
+++ lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in Mon Jan 21 19:50:44 2019
@@ -1,5 +1,5 @@
 reproducer status
-breakpoint set -f simple.c -l 13
+breakpoint set -f simple.c -l 12
 run
 bt
 cont

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py?rev=351779&r1=351778&r2=351779&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
 Mon Jan 21 19:50:44 2019
@@ -24,12 +24,12 @@ class BreakpointByLineAndColumnTestCase(
 self.build()
 main_c = lldb.SBFileSpec("main.c")
 _, _, _, breakpoint = lldbutil.run_to_line_breakpoint(self,
-  main_c, 20, 50)
+  main_c, 19, 50)
 self.expect("fr v did_call", substrs='1')
 in_then = False
 for i in range(breakpoint.GetNumLocations()):
 b_loc = 
breakpoint.GetLocationAtIndex(i).GetAddress().GetLineEntry()
-self.assertEqual(b_loc.GetLine(), 20)
+self.assertEqual(b_loc.GetLine(), 19)
 in_then |= b_loc.GetColumn() == 50
 self.assertTrue(in_then)
 
@@ -38,11 +38,11 @@ class BreakpointByLineAndColumnTestCase(
 de

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Hui Huang via Phabricator via lldb-commits]

Hui added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

labath wrote:
> Hui wrote:
> > labath wrote:
> > > labath wrote:
> > > > Hui wrote:
> > > > > labath wrote:
> > > > > > I am sorry for being such a pain, but I don't think this is 
> > > > > > grammatically correct, as `get` and `flatten` are both verbs. I'd 
> > > > > > call this either GetFlatten**ed**WindowsCommandLine or just plain 
> > > > > > FlattenWindowsCommandLine.
> > > > > There are  two kinds of sources of args in this discussion:
> > > > > 
> > > > >   - from lldb console through stdin which is not raw.
> > > > >   - from LLDB_SERVER_LOG_FILE environment variables which are raw
> > > > > 
> > > > > We expect to call a GetFlattenedWindowsCommandString for raw input. 
> > > > > However it seems not the case for now. 
> > > > > 
> > > > > What about adding a TODO in the following in ProcessWindowsLauncher. 
> > > > > 
> > > > > Would like a solution to proceed.
> > > > > 
> > > > > +
> > > > > +bool GetFlattenedWindowsCommandString(Args args, std::string 
> > > > > &command) {
> > > > > +  if (args.empty())
> > > > > +return false;
> > > > > +
> > > > > +  std::vector args_ref;
> > > > > +  for (auto &entry : args.entries())
> > > > > + args_ref.push_back(entry.ref);
> > > > > +
> > > > > +  // TODO: only flatten raw input.
> > > > > +  // Inputs from lldb console through the stdin are not raw, for 
> > > > > example,
> > > > > +  // A command line like "dir c:\" is attained as "dir c":\\". 
> > > > > Trying to flatten such
> > > > > +  // input will result in unexpected errors. In this case, the 
> > > > > flattend string will be
> > > > > +  // interpreted as "dir c:\\ which is a wrong usage of `dir` 
> > > > > command.
> > > > > +
> > > > > +  command = llvm::sys::flattenWindowsCommandLine(args_ref);
> > > > > +  return true;
> > > > >  }
> > > > > +} // namespace
> > > > > 
> > > > I am afraid you lost me here. By the time something gets to this 
> > > > function, it has already been parsed into individual argv strings. I am 
> > > > not sure which ones do you consider raw, or why should it make a 
> > > > difference. (lldb builtin interpreter has a concept of "raw" commands, 
> > > > but I don't think that's what you mean here)
> > > > 
> > > > This function should take those argv strings, no matter what they are, 
> > > > and recompose them into a single string. If those strings are wrong, 
> > > > then it's output will also be wrong, but that's not a problem of this 
> > > > function -- it's a problem of whoever parsed those strings.
> > > > 
> > > > I won't have access to a windows machine this week to check this out, 
> > > > but I can take a look at that next week. In the mean time, I would be 
> > > > fine with just xfailing the single test which fails because of this. I 
> > > > think that's a good tradeoff, as this would fix a bunch of other tests 
> > > > as well as unblock you on your lldb-server work.
> > > I can't test this out, but the place I'd expect the problem to be is in 
> > > `ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell`. This function 
> > > seems to be blissfully unaware of the windows quoting rules, and yet it's 
> > > the one that turns `platform shell dir c:\` into `cmd`, `/c` `whatever`. 
> > > The flatten function then gets this argv array and flattens it one more 
> > > time. I am not sure if that's the right thing to do on windows.
> > It is observed when you type "dir c:\" on lldb console, the 
> > IOHandlerEditline::GetLine will yield "dir c:\\" for you through the 
> > standard input (I skipped the new line char '\n'). And the 
> > CommandInterpreter::ResolveCommandImpl won't get the raw command line 
> > string, i mean "dir c:\", even I force WantsRawCommandString() true to get 
> > one.
> How are you observing that? Are you sure that's what happens, and it's not 
> just the lldb formatting of c strings (when lldb displays a `const char *` 
> variable, it will automatically escape any backslashes within, which means 
> the backslashes will appear doubled).
> 
> I'd be surprised if extra backslashes appear at this level (they certainly 
> don't on linux), as plenty of other things would fail if this didn't work.
I set the breakpoint through MSVC, triggered it by typing the command "dir c:\" 
on lldb console and the value I read was "dir c:\\\n". 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

Hui wrote:
> labath wrote:
> > Hui wrote:
> > > labath wrote:
> > > > labath wrote:
> > > > > Hui wrote:
> > > > > > labath wrote:
> > > > > > > I am sorry for being such a pain, but I don't think this is 
> > > > > > > grammatically correct, as `get` and `flatten` are both verbs. I'd 
> > > > > > > call this either GetFlatten**ed**WindowsCommandLine or just plain 
> > > > > > > FlattenWindowsCommandLine.
> > > > > > There are  two kinds of sources of args in this discussion:
> > > > > > 
> > > > > >   - from lldb console through stdin which is not raw.
> > > > > >   - from LLDB_SERVER_LOG_FILE environment variables which are raw
> > > > > > 
> > > > > > We expect to call a GetFlattenedWindowsCommandString for raw input. 
> > > > > > However it seems not the case for now. 
> > > > > > 
> > > > > > What about adding a TODO in the following in 
> > > > > > ProcessWindowsLauncher. 
> > > > > > 
> > > > > > Would like a solution to proceed.
> > > > > > 
> > > > > > +
> > > > > > +bool GetFlattenedWindowsCommandString(Args args, std::string 
> > > > > > &command) {
> > > > > > +  if (args.empty())
> > > > > > +return false;
> > > > > > +
> > > > > > +  std::vector args_ref;
> > > > > > +  for (auto &entry : args.entries())
> > > > > > + args_ref.push_back(entry.ref);
> > > > > > +
> > > > > > +  // TODO: only flatten raw input.
> > > > > > +  // Inputs from lldb console through the stdin are not raw, for 
> > > > > > example,
> > > > > > +  // A command line like "dir c:\" is attained as "dir c":\\". 
> > > > > > Trying to flatten such
> > > > > > +  // input will result in unexpected errors. In this case, the 
> > > > > > flattend string will be
> > > > > > +  // interpreted as "dir c:\\ which is a wrong usage of `dir` 
> > > > > > command.
> > > > > > +
> > > > > > +  command = llvm::sys::flattenWindowsCommandLine(args_ref);
> > > > > > +  return true;
> > > > > >  }
> > > > > > +} // namespace
> > > > > > 
> > > > > I am afraid you lost me here. By the time something gets to this 
> > > > > function, it has already been parsed into individual argv strings. I 
> > > > > am not sure which ones do you consider raw, or why should it make a 
> > > > > difference. (lldb builtin interpreter has a concept of "raw" 
> > > > > commands, but I don't think that's what you mean here)
> > > > > 
> > > > > This function should take those argv strings, no matter what they 
> > > > > are, and recompose them into a single string. If those strings are 
> > > > > wrong, then it's output will also be wrong, but that's not a problem 
> > > > > of this function -- it's a problem of whoever parsed those strings.
> > > > > 
> > > > > I won't have access to a windows machine this week to check this out, 
> > > > > but I can take a look at that next week. In the mean time, I would be 
> > > > > fine with just xfailing the single test which fails because of this. 
> > > > > I think that's a good tradeoff, as this would fix a bunch of other 
> > > > > tests as well as unblock you on your lldb-server work.
> > > > I can't test this out, but the place I'd expect the problem to be is in 
> > > > `ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell`. This function 
> > > > seems to be blissfully unaware of the windows quoting rules, and yet 
> > > > it's the one that turns `platform shell dir c:\` into `cmd`, `/c` 
> > > > `whatever`. The flatten function then gets this argv array and flattens 
> > > > it one more time. I am not sure if that's the right thing to do on 
> > > > windows.
> > > It is observed when you type "dir c:\" on lldb console, the 
> > > IOHandlerEditline::GetLine will yield "dir c:\\" for you through the 
> > > standard input (I skipped the new line char '\n'). And the 
> > > CommandInterpreter::ResolveCommandImpl won't get the raw command line 
> > > string, i mean "dir c:\", even I force WantsRawCommandString() true to 
> > > get one.
> > How are you observing that? Are you sure that's what happens, and it's not 
> > just the lldb formatting of c strings (when lldb displays a `const char *` 
> > variable, it will automatically escape any backslashes within, which means 
> > the backslashes will appear doubled).
> > 
> > I'd be surprised if extra backslashes appear at this level (they certainly 
> > don't on linux), as plenty of other things would fail if this didn't work.
> I set the breakpoint through MSVC, triggered it by typing the command "dir 
> c:\" on lldb console and the value I read was "dir c:\\\n". 
Isn't MSVC doing the same thing here? Otherwise, how would you be able to tell 
a line feed character ("\n") from a literal backslash followed by the letter N?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits 

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Hui Huang via Phabricator via lldb-commits]

Hui added inline comments.



Comment at: include/lldb/Utility/Args.h:121
 
+  bool GetFlattenWindowsCommandString(std::string &command) const;
+

labath wrote:
> Hui wrote:
> > labath wrote:
> > > Hui wrote:
> > > > labath wrote:
> > > > > labath wrote:
> > > > > > Hui wrote:
> > > > > > > labath wrote:
> > > > > > > > I am sorry for being such a pain, but I don't think this is 
> > > > > > > > grammatically correct, as `get` and `flatten` are both verbs. 
> > > > > > > > I'd call this either GetFlatten**ed**WindowsCommandLine or just 
> > > > > > > > plain FlattenWindowsCommandLine.
> > > > > > > There are  two kinds of sources of args in this discussion:
> > > > > > > 
> > > > > > >   - from lldb console through stdin which is not raw.
> > > > > > >   - from LLDB_SERVER_LOG_FILE environment variables which are raw
> > > > > > > 
> > > > > > > We expect to call a GetFlattenedWindowsCommandString for raw 
> > > > > > > input. However it seems not the case for now. 
> > > > > > > 
> > > > > > > What about adding a TODO in the following in 
> > > > > > > ProcessWindowsLauncher. 
> > > > > > > 
> > > > > > > Would like a solution to proceed.
> > > > > > > 
> > > > > > > +
> > > > > > > +bool GetFlattenedWindowsCommandString(Args args, std::string 
> > > > > > > &command) {
> > > > > > > +  if (args.empty())
> > > > > > > +return false;
> > > > > > > +
> > > > > > > +  std::vector args_ref;
> > > > > > > +  for (auto &entry : args.entries())
> > > > > > > + args_ref.push_back(entry.ref);
> > > > > > > +
> > > > > > > +  // TODO: only flatten raw input.
> > > > > > > +  // Inputs from lldb console through the stdin are not raw, for 
> > > > > > > example,
> > > > > > > +  // A command line like "dir c:\" is attained as "dir c":\\". 
> > > > > > > Trying to flatten such
> > > > > > > +  // input will result in unexpected errors. In this case, the 
> > > > > > > flattend string will be
> > > > > > > +  // interpreted as "dir c:\\ which is a wrong usage of `dir` 
> > > > > > > command.
> > > > > > > +
> > > > > > > +  command = llvm::sys::flattenWindowsCommandLine(args_ref);
> > > > > > > +  return true;
> > > > > > >  }
> > > > > > > +} // namespace
> > > > > > > 
> > > > > > I am afraid you lost me here. By the time something gets to this 
> > > > > > function, it has already been parsed into individual argv strings. 
> > > > > > I am not sure which ones do you consider raw, or why should it make 
> > > > > > a difference. (lldb builtin interpreter has a concept of "raw" 
> > > > > > commands, but I don't think that's what you mean here)
> > > > > > 
> > > > > > This function should take those argv strings, no matter what they 
> > > > > > are, and recompose them into a single string. If those strings are 
> > > > > > wrong, then it's output will also be wrong, but that's not a 
> > > > > > problem of this function -- it's a problem of whoever parsed those 
> > > > > > strings.
> > > > > > 
> > > > > > I won't have access to a windows machine this week to check this 
> > > > > > out, but I can take a look at that next week. In the mean time, I 
> > > > > > would be fine with just xfailing the single test which fails 
> > > > > > because of this. I think that's a good tradeoff, as this would fix 
> > > > > > a bunch of other tests as well as unblock you on your lldb-server 
> > > > > > work.
> > > > > I can't test this out, but the place I'd expect the problem to be is 
> > > > > in `ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell`. This 
> > > > > function seems to be blissfully unaware of the windows quoting rules, 
> > > > > and yet it's the one that turns `platform shell dir c:\` into `cmd`, 
> > > > > `/c` `whatever`. The flatten function then gets this argv array and 
> > > > > flattens it one more time. I am not sure if that's the right thing to 
> > > > > do on windows.
> > > > It is observed when you type "dir c:\" on lldb console, the 
> > > > IOHandlerEditline::GetLine will yield "dir c:\\" for you through the 
> > > > standard input (I skipped the new line char '\n'). And the 
> > > > CommandInterpreter::ResolveCommandImpl won't get the raw command line 
> > > > string, i mean "dir c:\", even I force WantsRawCommandString() true to 
> > > > get one.
> > > How are you observing that? Are you sure that's what happens, and it's 
> > > not just the lldb formatting of c strings (when lldb displays a `const 
> > > char *` variable, it will automatically escape any backslashes within, 
> > > which means the backslashes will appear doubled).
> > > 
> > > I'd be surprised if extra backslashes appear at this level (they 
> > > certainly don't on linux), as plenty of other things would fail if this 
> > > didn't work.
> > I set the breakpoint through MSVC, triggered it by typing the command "dir 
> > c:\" on lldb console and the value I read was "dir c:\\\n". 
> Isn't MSVC doing the same thing here? Otherwise, how would you be able to 
> tell a line feed character 

[Lldb-commits] [PATCH] D56590: breakpad: Add FUNC records to the symtab

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351781: breakpad: Add FUNC records to the symtab (authored 
by labath, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56590/new/

https://reviews.llvm.org/D56590

Files:
  lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms
  lldb/trunk/lit/SymbolFile/Breakpad/symtab.test
  lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp

Index: lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
===
--- lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
+++ lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
@@ -50,12 +50,27 @@
   EXPECT_EQ(llvm::None, InfoRecord::parse("INFO CODE_ID"));
 }
 
+TEST(FuncRecord, parse) {
+  EXPECT_EQ(FuncRecord(true, 0x47, 0x7, 0x8, "foo"),
+FuncRecord::parse("FUNC m 47 7 8 foo"));
+  EXPECT_EQ(FuncRecord(false, 0x47, 0x7, 0x8, "foo"),
+FuncRecord::parse("FUNC 47 7 8 foo"));
+
+  EXPECT_EQ(llvm::None, FuncRecord::parse("PUBLIC 47 7 8 foo"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC 47 7 8"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC 47 7"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC 47"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC m"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC"));
+}
+
 TEST(PublicRecord, parse) {
   EXPECT_EQ(PublicRecord(true, 0x47, 0x8, "foo"),
 PublicRecord::parse("PUBLIC m 47 8 foo"));
   EXPECT_EQ(PublicRecord(false, 0x47, 0x8, "foo"),
 PublicRecord::parse("PUBLIC 47 8 foo"));
 
+  EXPECT_EQ(llvm::None, PublicRecord::parse("FUNC 47 8 foo"));
   EXPECT_EQ(llvm::None, PublicRecord::parse("PUBLIC 47 8"));
   EXPECT_EQ(llvm::None, PublicRecord::parse("PUBLIC 47"));
   EXPECT_EQ(llvm::None, PublicRecord::parse("PUBLIC m"));
Index: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
===
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
@@ -80,6 +80,31 @@
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const InfoRecord &R);
 
+class FuncRecord : public Record {
+public:
+  static llvm::Optional parse(llvm::StringRef Line);
+  FuncRecord(bool Multiple, lldb::addr_t Address, lldb::addr_t Size,
+ lldb::addr_t ParamSize, llvm::StringRef Name)
+  : Record(Module), Multiple(Multiple), Address(Address), Size(Size),
+ParamSize(ParamSize), Name(Name) {}
+
+  bool getMultiple() const { return Multiple; }
+  lldb::addr_t getAddress() const { return Address; }
+  lldb::addr_t getSize() const { return Size; }
+  lldb::addr_t getParamSize() const { return ParamSize; }
+  llvm::StringRef getName() const { return Name; }
+
+private:
+  bool Multiple;
+  lldb::addr_t Address;
+  lldb::addr_t Size;
+  lldb::addr_t ParamSize;
+  llvm::StringRef Name;
+};
+
+bool operator==(const FuncRecord &L, const FuncRecord &R);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const FuncRecord &R);
+
 class PublicRecord : public Record {
 public:
   static llvm::Optional parse(llvm::StringRef Line);
Index: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
@@ -191,32 +191,77 @@
   return OS << "INFO CODE_ID " << R.getID().GetAsString();
 }
 
-llvm::Optional PublicRecord::parse(llvm::StringRef Line) {
+static bool parsePublicOrFunc(llvm::StringRef Line, bool &Multiple,
+  lldb::addr_t &Address, lldb::addr_t *Size,
+  lldb::addr_t &ParamSize, llvm::StringRef &Name) {
   // PUBLIC [m] address param_size name
+  // or
+  // FUNC [m] address size param_size name
+
+  Token Tok = Size ? Token::Func : Token::Public;
+
   llvm::StringRef Str;
   std::tie(Str, Line) = getToken(Line);
-  if (toToken(Str) != Token::Public)
-return llvm::None;
+  if (toToken(Str) != Tok)
+return false;
 
   std::tie(Str, Line) = getToken(Line);
-  bool Multiple = Str == "m";
+  Multiple = Str == "m";
 
   if (Multiple)
 std::tie(Str, Line) = getToken(Line);
-  lldb::addr_t Address;
   if (!to_integer(Str, Address, 16))
-return llvm::None;
+return false;
+
+  if (Tok == Token::Func) {
+std::tie(Str, Line) = getToken(Line);
+if (!to_integer(Str, *Size, 16))
+  return false;
+  }
 
   std::tie(Str, Line) = getToken(Line);
-  lldb::addr_t ParamSize;
   if

[Lldb-commits] [lldb] r351781 - breakpad: Add FUNC records to the symtab

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jan 21 20:56:31 2019
New Revision: 351781

URL: http://llvm.org/viewvc/llvm-project?rev=351781&view=rev
Log:
breakpad: Add FUNC records to the symtab

This patch extends SymbolFileBreakpad::AddSymbols to include the symbols
from the FUNC records too. These symbols come from the debug info and
have a size associated with them, so they are given preference in case
there is a PUBLIC record for the same address.

To achieve this, I first pre-process the symbols into a temporary
DenseMap, and then insert the uniqued symbols into the module's symtab.

Reviewers: clayborg, lemo, zturner

Reviewed By: clayborg

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D56590

Modified:
lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms
lldb/trunk/lit/SymbolFile/Breakpad/symtab.test
lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp

Modified: lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms?rev=351781&r1=351780&r2=351781&view=diff
==
--- lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms (original)
+++ lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms Mon Jan 21 20:56:31 
2019
@@ -5,3 +5,5 @@ PUBLIC b0 0 f1
 PUBLIC m c0 0 f2
 PUBLIC d0 0 _start
 PUBLIC ff 0 _out_of_range_ignored
+FUNC b0 c 0 f1_func
+FUNC m a0 d 0 func_only

Modified: lldb/trunk/lit/SymbolFile/Breakpad/symtab.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/symtab.test?rev=351781&r1=351780&r2=351781&view=diff
==
--- lldb/trunk/lit/SymbolFile/Breakpad/symtab.test (original)
+++ lldb/trunk/lit/SymbolFile/Breakpad/symtab.test Mon Jan 21 20:56:31 2019
@@ -3,15 +3,16 @@
 # RUN:   -s %s | FileCheck %s
 
 # CHECK-LABEL: (lldb) image dump symtab symtab.out
-# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 3:
+# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 4:
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address
   Size   Flags  Name
-# CHECK: [0]  0   X Code0x004000b0 
   0x0010 0x f1
-# CHECK: [1]  0   X Code0x004000c0 
   0x0010 0x f2
-# CHECK: [2]  0   X Code0x004000d0 
   0x0022 0x _start
+# CHECK: [0]  0   X Code0x004000c0 
   0x0010 0x f2
+# CHECK: [1]  0   X Code0x004000d0 
   0x0022 0x _start
+# CHECK: [2]  0   X Code0x004000a0 
   0x000d 0x func_only
+# CHECK: [3]  0   X Code0x004000b0 
   0x000c 0x f1_func
 
 # CHECK-LABEL: (lldb) image lookup -a 0x4000b0 -v
 # CHECK: Address: symtab.out[0x004000b0] (symtab.out.PT_LOAD[0]..text2 
+ 0)
-# CHECK: Symbol: id = {0x}, range = 
[0x004000b0-0x004000c0), name="f1"
+# CHECK: Symbol: id = {0x}, range = 
[0x004000b0-0x004000bc), name="f1_func"
 
 # CHECK-LABEL: (lldb) image lookup -n f2 -v
 # CHECK: Address: symtab.out[0x004000c0] (symtab.out.PT_LOAD[0]..text2 
+ 16)

Modified: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp?rev=351781&r1=351780&r2=351781&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp Mon Jan 
21 20:56:31 2019
@@ -191,32 +191,77 @@ llvm::raw_ostream &breakpad::operator<<(
   return OS << "INFO CODE_ID " << R.getID().GetAsString();
 }
 
-llvm::Optional PublicRecord::parse(llvm::StringRef Line) {
+static bool parsePublicOrFunc(llvm::StringRef Line, bool &Multiple,
+  lldb::addr_t &Address, lldb::addr_t *Size,
+  lldb::addr_t &ParamSize, llvm::StringRef &Name) {
   // PUBLIC [m] address param_size name
+  // or
+  // FUNC [m] address size param_size name
+
+  Token Tok = Size ? Token::Func : Token::Public;
+
   llvm::StringRef Str;
   std::tie(Str, Line) = getToken(Line);
-  if (toToken(Str) != Token::Public)
-return llvm::None;
+  if (toToken(Str) != Tok)
+return false;
 
   std::tie(Str, Lin

[Lldb-commits] [PATCH] D57037: BreakpadRecords: Address post-commit feedback

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 182847.
labath added a comment.

rebase


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57037/new/

https://reviews.llvm.org/D57037

Files:
  source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -204,12 +204,12 @@
 
   for (llvm::StringRef line : lines(*m_obj_file, Record::Func)) {
 if (auto record = FuncRecord::parse(line))
-  add_symbol(record->getAddress(), record->getSize(), record->getName());
+  add_symbol(record->Address, record->Size, record->Name);
   }
 
   for (llvm::StringRef line : lines(*m_obj_file, Record::Public)) {
 if (auto record = PublicRecord::parse(line))
-  add_symbol(record->getAddress(), llvm::None, record->getName());
+  add_symbol(record->Address, llvm::None, record->Name);
 else
   LLDB_LOG(log, "Failed to parse: {0}. Skipping record.", line);
   }
Index: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
===
--- source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
+++ source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
@@ -32,13 +32,13 @@
 return llvm::None;
 
   llvm::Triple triple;
-  triple.setArch(Module->getArch());
-  triple.setOS(Module->getOS());
+  triple.setArch(Module->Arch);
+  triple.setOS(Module->OS);
 
   std::tie(line, text) = text.split('\n');
 
   auto Info = InfoRecord::parse(line);
-  UUID uuid = Info && Info->getID() ? Info->getID() : Module->getID();
+  UUID uuid = Info && Info->ID ? Info->ID : Module->ID;
   return Header{ArchSpec(triple), std::move(uuid)};
 }
 
Index: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
===
--- source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
+++ source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
@@ -51,17 +51,14 @@
   ModuleRecord(llvm::Triple::OSType OS, llvm::Triple::ArchType Arch, UUID ID)
   : Record(Module), OS(OS), Arch(Arch), ID(std::move(ID)) {}
 
-  llvm::Triple::OSType getOS() const { return OS; }
-  llvm::Triple::ArchType getArch() const { return Arch; }
-  const UUID &getID() const { return ID; }
-
-private:
   llvm::Triple::OSType OS;
   llvm::Triple::ArchType Arch;
   UUID ID;
 };
 
-bool operator==(const ModuleRecord &L, const ModuleRecord &R);
+bool operator==(const ModuleRecord &L, const ModuleRecord &R) {
+  return L.OS == R.OS && L.Arch == R.Arch && L.ID == R.ID;
+}
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const ModuleRecord &R);
 
 class InfoRecord : public Record {
@@ -69,14 +66,11 @@
   static llvm::Optional parse(llvm::StringRef Line);
   InfoRecord(UUID ID) : Record(Info), ID(std::move(ID)) {}
 
-  const UUID &getID() const { return ID; }
-
-private:
   UUID ID;
 };
 
 inline bool operator==(const InfoRecord &L, const InfoRecord &R) {
-  return L.getID() == R.getID();
+  return L.ID == R.ID;
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const InfoRecord &R);
 
@@ -88,13 +82,6 @@
   : Record(Module), Multiple(Multiple), Address(Address), Size(Size),
 ParamSize(ParamSize), Name(Name) {}
 
-  bool getMultiple() const { return Multiple; }
-  lldb::addr_t getAddress() const { return Address; }
-  lldb::addr_t getSize() const { return Size; }
-  lldb::addr_t getParamSize() const { return ParamSize; }
-  llvm::StringRef getName() const { return Name; }
-
-private:
   bool Multiple;
   lldb::addr_t Address;
   lldb::addr_t Size;
@@ -113,12 +100,6 @@
   : Record(Module), Multiple(Multiple), Address(Address),
 ParamSize(ParamSize), Name(Name) {}
 
-  bool getMultiple() const { return Multiple; }
-  lldb::addr_t getAddress() const { return Address; }
-  lldb::addr_t getParamSize() const { return ParamSize; }
-  llvm::StringRef getName() const { return Name; }
-
-private:
   bool Multiple;
   lldb::addr_t Address;
   lldb::addr_t ParamSize;
Index: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
===
--- source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
+++ source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
@@ -56,17 +56,31 @@
   .Default(Triple::UnknownArch);
 }
 
-static llvm::StringRef consume_front(llvm::StringRef &str, size_t n) {
-  llvm::StringRef result = str.take_front(n);
-  str = str.drop_front(n);
-  return result;
+/// Return the number of hex digits needed to encode an (POD) object of a given
+/// type.
+template  static constexpr size_t hex_digits() {
+  return 2 * sizeof(T);
+}
+
+/// Consume the right 

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Hui Huang via Phabricator via lldb-commits]

Hui added a comment.

In D56230#1361746 , @Hui wrote:

> It could be the llvm::sys::flattenWindowsCommandLine issue to flatten the 
> command “dir c:\" for Windows Command Terminal.
>  However it is not an issue for PS or MingGW.
>
> It is observed the flattened one is
>
>   "\"C:\\WINDOWS\\system32\\cmd.exe\" /C \" dir c:\" "
>
>
> which will be interpreted as the following that is not accepted by 
> CMD.exe.(dir c:\ or dir c:\.\ is fine. There is no '\' directory or file on 
> my Drive c).
>  However it is accepted by PS and MingGW.
>
>   "C:\\WINDOWS\\system32\\cmd.exe" /C " dir c:\\"
>


Hello labath. I am still thinking that it might be 
**llvm::sys::flattenWindowsCommandLine** issue. As you can see, in the 
flattened string "\"C:\\WINDOWS\\system32\\cmd.exe\" /C \" dir c:\",

the '\\' is not doubled for **"C:\\WINDOWS"**, while the '\\' is doubled for 
**"c:\\"** as **"c:"**


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 182848.
tatyana-krasnukha added a comment.

Keep trying to hide the processor's specifics from the ProcessGDBRemote


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55718/new/

https://reviews.llvm.org/D55718

Files:
  include/lldb/Core/Architecture.h
  include/lldb/Utility/ArchSpec.h
  source/Plugins/Architecture/Arc/ArchitectureArc.cpp
  source/Plugins/Architecture/Arc/ArchitectureArc.h
  source/Plugins/Architecture/Arc/CMakeLists.txt
  source/Plugins/Architecture/CMakeLists.txt
  source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  source/Plugins/Process/Utility/DynamicRegisterInfo.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Target/Platform.cpp
  source/Target/Thread.cpp
  source/Utility/ArchSpec.cpp

Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -221,7 +221,8 @@
 {eByteOrderLittle, 4, 1, 1, llvm::Triple::kalimba, ArchSpec::eCore_kalimba4,
  "kalimba4"},
 {eByteOrderLittle, 4, 1, 1, llvm::Triple::kalimba, ArchSpec::eCore_kalimba5,
- "kalimba5"}};
+ "kalimba5"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"}};
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
 // comment out an entry above, you will need to comment out the corresponding
@@ -458,7 +459,9 @@
 {ArchSpec::eCore_kalimba4, llvm::ELF::EM_CSR_KALIMBA,
  llvm::Triple::KalimbaSubArch_v4, 0xu, 0xu}, // KALIMBA
 {ArchSpec::eCore_kalimba5, llvm::ELF::EM_CSR_KALIMBA,
- llvm::Triple::KalimbaSubArch_v5, 0xu, 0xu} // KALIMBA
+ llvm::Triple::KalimbaSubArch_v5, 0xu, 0xu}, // KALIMBA
+{ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu } // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -2061,6 +2061,7 @@
 switch (machine) {
 case llvm::Triple::x86_64:
 case llvm::Triple::x86:
+case llvm::Triple::arc:
 case llvm::Triple::arm:
 case llvm::Triple::aarch64:
 case llvm::Triple::thumb:
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -1869,6 +1869,12 @@
   size_t trap_opcode_size = 0;
 
   switch (arch.GetMachine()) {
+  case llvm::Triple::arc: {
+static const uint8_t g_hex_opcode[] = { 0xff, 0x7f };
+trap_opcode = g_hex_opcode;
+trap_opcode_size = sizeof(g_hex_opcode);
+  } break;
+
   case llvm::Triple::aarch64: {
 static const uint8_t g_aarch64_opcode[] = {0x00, 0x00, 0x20, 0xd4};
 trap_opcode = g_aarch64_opcode;
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1198,6 +1198,44 @@
   }
 }
 
+auto arch_plugin = GetTarget().GetArchitecturePlugin();
+if (nullptr != arch_plugin) {
+  Architecture::ConfigurationRegisterValues reg_values;
+
+  for (const auto ®_name : arch_plugin->ConfigurationRegisterNames()) {
+bool case_sensitive = false;
+auto reg_info = m_register_info.GetRegisterInfo(reg_name, case_sensitive);
+if (!reg_info) {
+  if (log)
+log->Printf("Cannot find register info for \"%s\"",
+reg_name.AsCString());
+  continue;
+}
+
+// Configuration registers are not context-dependent.
+const auto tid = LLDB_INVALID_THREAD_ID;
+// Cannot use GDBRemoteRegisterContext here, it is not created yet.
+DataBufferSP buffer_sp = GetGDBRemote().ReadRegister(tid,
+  reg_info->kinds[eRegisterKindProcessPlugin]);
+if (!buffer_sp || buffer_sp->GetByteSize() < reg_info->byte_size){
+  if (log)
+log->Printf("Failed to read configuration register \"%s\"",
+reg_name.AsCString());
+  continue;
+}
+
+reg_values[reg_name] = RegisterValue(buffer_sp->GetBytes(),
+  buffer_sp->GetByteSize(),
+  GetByteOrder()).GetAsUInt32();
+  }
+
+  if (!arch_plugin->SetFlagsFrom(reg_values) && log)
+log->PutCString("Failed to set architechture's features");
+
+  if (!arch_plugin->MatchFlags(GetTarget().GetArchitecture()) && log)
+log->PutCString("The architecture has incompatible features");
+}
+
 // Find out which Structur