Re: [Lldb-commits] [PATCH] D56315: Add a verbose mode to "image dump line-table" and use it to write a .debug_line test

[Pavel Labath via lldb-commits]

On 04/01/2019 17:53, Zachary Turner wrote:
It seems like this test could be made to work with non dwarf debug info 
by compiling a real source file. WDYT?


Well.. I could write a test by compiling a real source, but it wouldn't 
be *this* test. The idea here was to validate the parsing of the debug 
info into lldb internal data structures (including all the 
is_start_of_statement, is_end_of_prologue bits, etc.), and for that I 
need to be able to specify the input with great precision.


Without that, the only thing I could assert is that there is "some" line 
entry for a given line number, which isn't going to be much useful 
because that is already be extensively tested when setting line 
breakpoints.


So, the main use I see for this is testing the workings of a specific 
parser. Then, if that parser works correctly, anything which consumes 
the data from it (e.g., the breakpoint machinery) should work fine too.

___
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 requested changes to this revision.
labath added inline comments.
This revision now requires changes to proceed.



Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1138
+  // Double quote the string.
+  ::snprintf(arg_cstr, sizeof(arg_cstr), "--log-channels=\"%s\"",
+ env_debugserver_log_channels.c_str());

zturner wrote:
> Hui wrote:
> > This change is essential others seem not that necessary but nice to have in 
> > order to support multiple systems.
> I would prefer to avoid using C-style formatting if possible.  LLVM has some 
> nice formatting routines that are safer and easier to understand.
> 
> While we're here, perhaps we could change `char arg_cstr[PATH_MAX];` up at 
> the top of this function to `std::string arg_str;`
> 
> Regardless, we can re-write this in a number of ways, such as:
> ```
> arg_str = llvm::formatv("--log-channels = \"{0}\", 
> env_debugserver_log_channels).str();
> arg_str = (Twine("--log-channels = \"") + env_debugserver_log_channels + 
> "\"").str();
> arg_str = llvm::format("--log-channels = \"%s\"", 
> env_debugserver_log_channels);
> ```
> 
> to get rid of this snprintf call.
Actually, this shouldn't be done here at all. The assumption here is that the 
contents of the `Args` vector will get passed verbatim to the argc+argv of the 
called program. On unix systems we will do just that, so if you add the quotes 
here they will be passed the lldb-server, and then the parse there will fail 
because we will get extra quotes we didn't expect.

Since Windows needs special logic in order to ensure this, that logic should be 
inside windows-specific code, probably ProcessLauncherWindows. That will make 
all other instances of launching processes with spaces work, not just this 
particular case. It seems ProcessLauncherWindows uses 
Args::GetQuotedCommandString to do this job, but this function seems extremely 
under-equipped for this job. I'd suggest implementing a windows-specific 
version of this in ProcessLauncherWindows, which will do whatever fixups are 
necessary to make this happen.


Repository:
  rLLDB LLDB

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] D56293: Use the minidump exception record if present

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Looks fine to me (but please wait for an ack from Zachary).




Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:313
 
-  for (lldb::tid_t tid = 0; tid < num_threads; ++tid) {
+  for (size_t t_index = 0; t_index < num_threads; ++t_index) {
 llvm::ArrayRef context;

Since you're touching this, you might as well change this to a range-based for 
loop.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:316
+const auto& thread = m_thread_list[t_index];
+auto context_location = thread.thread_context;
+

LLVM tries to avoid `auto` overuse. For example, here I had to look up 
thread_context to see what type would this be. Could you put the actual type 
here?


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

https://reviews.llvm.org/D56293



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

Re: [Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

[Pavel Labath via lldb-commits]

On 04/01/2019 17:48, Zachary Turner wrote:
For those kinds of cases, we could use obj2yaml and check in yaml right? 
Fwiw I tried to round-trip an exe through obj->yaml->obj recently and 
the resulting exe was incorrect but it was close, so I think there’s 
only some small fixes needed.




I agree yaml2obj should be used whenever possible, but here I'm talking 
specifically about things that cannot be repesented in yaml. For 
example, you can't get yaml2obj to produce a binary that's truncated in 
the middle of the section list. Or one whose section contents points 
back to the section list... And yet it makes sense to test that lldb 
doesn't crash or do other weird stuff when opening these files. It also 
probably doesn't make sense to write a special tool for generating 
these, as there aren't going to be many of these cases, and each of them 
will be special in some way.

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

[Lldb-commits] [lldb] r350510 - ProcessLaunchInfo: remove Debugger reference

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jan  7 02:59:57 2019
New Revision: 350510

URL: http://llvm.org/viewvc/llvm-project?rev=350510&view=rev
Log:
ProcessLaunchInfo: remove Debugger reference

Summary:
The Debuffer object was being used in "GetListenerForProcess" to provide
a default listener object if one was not specified in the launch_info
object.

Since all the callers of this function immediately passed the result to
Target::CreateProcess, it was easy to move this logic there instead.

This brings us one step closer towards being able to move the LaunchInfo
classes to the Host layer (which is there the launching code that
consumes them lives).

Reviewers: zturner, jingham, teemperor

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/trunk/source/Plugins/Platform/Windows/PlatformWindows.cpp
lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/trunk/source/Target/ProcessLaunchInfo.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h?rev=350510&r1=350509&r2=350510&view=diff
==
--- lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h (original)
+++ lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h Mon Jan  7 02:59:57 2019
@@ -131,8 +131,6 @@ public:
 m_listener_sp = listener_sp;
   }
 
-  lldb::ListenerSP GetListenerForProcess(Debugger &debugger);
-
   lldb::ListenerSP GetHijackListener() const { return m_hijack_listener_sp; }
 
   void SetHijackListener(const lldb::ListenerSP &listener_sp) {

Modified: lldb/trunk/include/lldb/Target/Target.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=350510&r1=350509&r2=350510&view=diff
==
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Mon Jan  7 02:59:57 2019
@@ -533,7 +533,9 @@ public:
   //--
   void Dump(Stream *s, lldb::DescriptionLevel description_level);
 
-  const lldb::ProcessSP &CreateProcess(lldb::ListenerSP listener,
+  // If listener_sp is null, the listener of the owning Debugger object will be
+  // used.
+  const lldb::ProcessSP &CreateProcess(lldb::ListenerSP listener_sp,
llvm::StringRef plugin_name,
const FileSpec *crash_file);
 

Modified: lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp?rev=350510&r1=350509&r2=350510&view=diff
==
--- lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp Mon Jan  7 
02:59:57 2019
@@ -318,8 +318,8 @@ PlatformLinux::DebugProcess(ProcessLaunc
 
   // Now create the gdb-remote process.
   LLDB_LOG(log, "having target create process with gdb-remote plugin");
-  process_sp = target->CreateProcess(
-  launch_info.GetListenerForProcess(debugger), "gdb-remote", nullptr);
+  process_sp =
+  target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
 
   if (!process_sp) {
 error.SetErrorString("CreateProcess() failed for gdb-remote process");

Modified: lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp?rev=350510&r1=350509&r2=350510&view=diff
==
--- lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp Mon Jan  7 
02:59:57 2019
@@ -287,8 +287,8 @@ PlatformNetBSD::DebugProcess(ProcessLaun
 
   // Now create the gdb-remote process.
   LLDB_LOG(log, "having target create process with gdb-remote plugin");
-  process_sp = target->CreateProcess(
-  launch_info.GetListenerForProcess(debugger), "gdb-remote", nullptr);
+  process_sp =
+  target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
 
   if (!process_sp) {
 error.SetErrorString("CreateProcess() failed for gdb-remote process");

Modified: lldb/trunk/source/Plugins/Platform/Windows/PlatformWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Windows/PlatformWindows.cpp?rev=350510&r1=350509&r2=350510&view=diff
=

[Lldb-commits] [PATCH] D56174: ProcessLaunchInfo: remove Debugger reference

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350510: ProcessLaunchInfo: remove Debugger reference 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56174

Files:
  lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
  lldb/trunk/include/lldb/Target/Target.h
  lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/trunk/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/trunk/source/Target/ProcessLaunchInfo.cpp
  lldb/trunk/source/Target/Target.cpp

Index: lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -496,8 +496,8 @@
 
   // The darwin always currently uses the GDB remote debugger plug-in
   // so even when debugging locally we are debugging remotely!
-  process_sp = target->CreateProcess(
-  launch_info.GetListenerForProcess(debugger), "gdb-remote", NULL);
+  process_sp = target->CreateProcess(launch_info.GetListener(),
+ "gdb-remote", NULL);
 
   if (process_sp) {
 error = process_sp->ConnectRemote(nullptr, connect_url.c_str());
Index: lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
===
--- lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
+++ lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
@@ -287,8 +287,8 @@
 
   // Now create the gdb-remote process.
   LLDB_LOG(log, "having target create process with gdb-remote plugin");
-  process_sp = target->CreateProcess(
-  launch_info.GetListenerForProcess(debugger), "gdb-remote", nullptr);
+  process_sp =
+  target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
 
   if (!process_sp) {
 error.SetErrorString("CreateProcess() failed for gdb-remote process");
Index: lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
===
--- lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
+++ lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
@@ -318,8 +318,8 @@
 
   // Now create the gdb-remote process.
   LLDB_LOG(log, "having target create process with gdb-remote plugin");
-  process_sp = target->CreateProcess(
-  launch_info.GetListenerForProcess(debugger), "gdb-remote", nullptr);
+  process_sp =
+  target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
 
   if (!process_sp) {
 error.SetErrorString("CreateProcess() failed for gdb-remote process");
Index: lldb/trunk/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/trunk/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/trunk/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -437,9 +437,8 @@
 ProcessAttachInfo attach_info(launch_info);
 return Attach(attach_info, debugger, target, error);
   } else {
-ProcessSP process_sp =
-target->CreateProcess(launch_info.GetListenerForProcess(debugger),
-  launch_info.GetProcessPluginName(), nullptr);
+ProcessSP process_sp = target->CreateProcess(
+launch_info.GetListener(), launch_info.GetProcessPluginName(), nullptr);
 
 // We need to launch and attach to the process.
 launch_info.GetFlags().Set(eLaunchFlagDebug);
Index: lldb/trunk/source/Target/ProcessLaunchInfo.cpp
===
--- lldb/trunk/source/Target/ProcessLaunchInfo.cpp
+++ lldb/trunk/source/Target/ProcessLaunchInfo.cpp
@@ -9,7 +9,6 @@
 
 #include 
 
-#include "lldb/Core/Debugger.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
@@ -433,10 +432,3 @@
   }
   return false;
 }
-
-ListenerSP ProcessLaunchInfo::GetListenerForProcess(Debugger &debugger) {
-  if (m_listener_sp)
-return m_listener_sp;
-  else
-return debugger.GetListener();
-}
Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -194,6 +194,8 @@
 const lldb::ProcessSP &Target::CreateProcess(ListenerSP listener_sp,
  llvm::StringRef plugin_name,
  const FileSpec *crash_file) {
+  if (!listener_sp)
+listener_sp = GetDebugger(

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

[Pavel Labath via Phabricator via lldb-commits]

labath removed a reviewer: labath.
labath added a comment.

Removing the self-accept (oops).


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

https://reviews.llvm.org/D55434



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

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Thank you for the review.


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

https://reviews.llvm.org/D55434



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

[Lldb-commits] [lldb] r350511 - ObjectFileBreakpad: Implement sections

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jan  7 03:14:08 2019
New Revision: 350511

URL: http://llvm.org/viewvc/llvm-project?rev=350511&view=rev
Log:
ObjectFileBreakpad: Implement sections

Summary:
This patch allows ObjectFileBreakpad to parse the contents of Breakpad
files into sections. This sounds slightly odd at first, but in essence
its not too different from how other object files handle things. For
example in elf files, the symtab section consists of a number of
"records", where each record represents a single symbol. The same is
true for breakpad's PUBLIC section, except in this case, the records will be
textual instead of binary.

To keep sections contiguous, I create a new section every time record
type changes. Normally, the breakpad processor will group all records of
the same type in one block, but the format allows them to be intermixed,
so in general, the "object file" may contain multiple sections with the
same record type.

Reviewers: clayborg, zturner, lemo, markmentovai, amccarth

Subscribers: lldb-commits

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

Added:
lldb/trunk/lit/Modules/Breakpad/Inputs/discontiguous-sections.syms
lldb/trunk/lit/Modules/Breakpad/Inputs/sections-trailing-func.syms
lldb/trunk/lit/Modules/Breakpad/Inputs/sections.syms
lldb/trunk/lit/Modules/Breakpad/discontiguous-sections.test
lldb/trunk/lit/Modules/Breakpad/sections-trailing-func.test
lldb/trunk/lit/Modules/Breakpad/sections.test
Modified:
lldb/trunk/include/lldb/Utility/DataExtractor.h
lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp

Modified: lldb/trunk/include/lldb/Utility/DataExtractor.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/DataExtractor.h?rev=350511&r1=350510&r2=350511&view=diff
==
--- lldb/trunk/include/lldb/Utility/DataExtractor.h (original)
+++ lldb/trunk/include/lldb/Utility/DataExtractor.h Mon Jan  7 03:14:08 2019
@@ -14,6 +14,7 @@
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
+#include "llvm/ADT/ArrayRef.h"
 
 #include 
 #include 
@@ -1094,6 +1095,10 @@ public:
 
   void Checksum(llvm::SmallVectorImpl &dest, uint64_t max_data = 0);
 
+  llvm::ArrayRef GetData() const {
+return {GetDataStart(), GetByteSize()};
+  }
+
 protected:
   //--
   // Member variables

Added: lldb/trunk/lit/Modules/Breakpad/Inputs/discontiguous-sections.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/Breakpad/Inputs/discontiguous-sections.syms?rev=350511&view=auto
==
--- lldb/trunk/lit/Modules/Breakpad/Inputs/discontiguous-sections.syms (added)
+++ lldb/trunk/lit/Modules/Breakpad/Inputs/discontiguous-sections.syms Mon Jan  
7 03:14:08 2019
@@ -0,0 +1,5 @@
+MODULE Linux x86_64 24B5D199F0F766FF5DC30 linux.out
+INFO CODE_ID B52499D1F0F766FF5DC3
+FILE 0 /tmp/a.c
+PUBLIC 1010 0 _start
+FILE 1 /tmp/b.c

Added: lldb/trunk/lit/Modules/Breakpad/Inputs/sections-trailing-func.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/Breakpad/Inputs/sections-trailing-func.syms?rev=350511&view=auto
==
--- lldb/trunk/lit/Modules/Breakpad/Inputs/sections-trailing-func.syms (added)
+++ lldb/trunk/lit/Modules/Breakpad/Inputs/sections-trailing-func.syms Mon Jan  
7 03:14:08 2019
@@ -0,0 +1,8 @@
+MODULE Linux x86_64 24B5D199F0F766FF5DC30 linux.out
+INFO CODE_ID B52499D1F0F766FF5DC3
+FILE 0 /tmp/a.c
+FUNC 1010 10 0 _start
+1010 4 4 0
+1014 5 5 0
+1019 5 6 0
+101e 2 7 0

Added: lldb/trunk/lit/Modules/Breakpad/Inputs/sections.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/Breakpad/Inputs/sections.syms?rev=350511&view=auto
==
--- lldb/trunk/lit/Modules/Breakpad/Inputs/sections.syms (added)
+++ lldb/trunk/lit/Modules/Breakpad/Inputs/sections.syms Mon Jan  7 03:14:08 
2019
@@ -0,0 +1,12 @@
+MODULE Linux x86_64 24B5D199F0F766FF5DC30 linux.out
+INFO CODE_ID B52499D1F0F766FF5DC3
+FILE 0 /tmp/a.c
+FUNC 1010 10 0 _start
+1010 4 4 0
+1014 5 5 0
+1019 5 6 0
+101e 2 7 0
+PUBLIC 1010 0 _start
+STACK CFI INIT 1010 10 .cfa: $rsp 8 + .ra: .cfa -8 + ^
+STACK CFI 1011 $rbp: .cfa -16 + ^ .cfa: $rsp 16 +
+STACK CFI 1014 .cfa: $rbp 16 +

Added: lldb/trunk/lit/Modules/Breakpad/discontiguous-sections.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/Breakpad/discontiguous-sections.test?rev=350511&view=auto
==
--- lldb/trunk/lit/Modules/Breakpad/discontiguous-sections.test (added)
+++ lldb/trunk/lit/Modules/Breakpad/discontiguous-sections.test Mon Jan

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350511: ObjectFileBreakpad: Implement sections (authored by 
labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55434?vs=177209&id=180450#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55434

Files:
  lldb/trunk/include/lldb/Utility/DataExtractor.h
  lldb/trunk/lit/Modules/Breakpad/Inputs/discontiguous-sections.syms
  lldb/trunk/lit/Modules/Breakpad/Inputs/sections-trailing-func.syms
  lldb/trunk/lit/Modules/Breakpad/Inputs/sections.syms
  lldb/trunk/lit/Modules/Breakpad/discontiguous-sections.test
  lldb/trunk/lit/Modules/Breakpad/sections-trailing-func.test
  lldb/trunk/lit/Modules/Breakpad/sections.test
  lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp

Index: lldb/trunk/include/lldb/Utility/DataExtractor.h
===
--- lldb/trunk/include/lldb/Utility/DataExtractor.h
+++ lldb/trunk/include/lldb/Utility/DataExtractor.h
@@ -14,6 +14,7 @@
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
+#include "llvm/ADT/ArrayRef.h"
 
 #include 
 #include 
@@ -1094,6 +1095,10 @@
 
   void Checksum(llvm::SmallVectorImpl &dest, uint64_t max_data = 0);
 
+  llvm::ArrayRef GetData() const {
+return {GetDataStart(), GetByteSize()};
+  }
+
 protected:
   //--
   // Member variables
Index: lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
@@ -10,6 +10,7 @@
 #include "Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "llvm/ADT/StringExtras.h"
 
@@ -23,8 +24,41 @@
   UUID uuid;
   static llvm::Optional parse(llvm::StringRef text);
 };
+
+enum class Token { Unknown, Module, Info, File, Func, Public, Stack };
 } // namespace
 
+static Token toToken(llvm::StringRef str) {
+  return llvm::StringSwitch(str)
+  .Case("MODULE", Token::Module)
+  .Case("INFO", Token::Info)
+  .Case("FILE", Token::File)
+  .Case("FUNC", Token::Func)
+  .Case("PUBLIC", Token::Public)
+  .Case("STACK", Token::Stack)
+  .Default(Token::Unknown);
+}
+
+static llvm::StringRef toString(Token t) {
+  switch (t) {
+  case Token::Unknown:
+return "";
+  case Token::Module:
+return "MODULE";
+  case Token::Info:
+return "INFO";
+  case Token::File:
+return "FILE";
+  case Token::Func:
+return "FUNC";
+  case Token::Public:
+return "PUBLIC";
+  case Token::Stack:
+return "STACK";
+  }
+  llvm_unreachable("Unknown token!");
+}
+
 static llvm::Triple::OSType toOS(llvm::StringRef str) {
   using llvm::Triple;
   return llvm::StringSwitch(str)
@@ -103,7 +137,7 @@
   llvm::StringRef token, line;
   std::tie(line, text) = text.split('\n');
   std::tie(token, line) = getToken(line);
-  if (token != "MODULE")
+  if (toToken(token) != Token::Module)
 return llvm::None;
 
   std::tie(token, line) = getToken(line);
@@ -236,5 +270,46 @@
 }
 
 void ObjectFileBreakpad::CreateSections(SectionList &unified_section_list) {
-  // TODO
+  if (m_sections_ap)
+return;
+  m_sections_ap = llvm::make_unique();
+
+  Token current_section = Token::Unknown;
+  offset_t section_start;
+  llvm::StringRef text = toStringRef(m_data.GetData());
+  uint32_t next_section_id = 1;
+  auto maybe_add_section = [&](const uint8_t *end_ptr) {
+if (current_section == Token::Unknown)
+  return; // We have been called before parsing the first line.
+
+offset_t end_offset = end_ptr - m_data.GetDataStart();
+auto section_sp = std::make_shared(
+GetModule(), this, next_section_id++,
+ConstString(toString(current_section)), eSectionTypeOther,
+/*file_vm_addr*/ 0, /*vm_size*/ 0, section_start,
+end_offset - section_start, /*log2align*/ 0, /*flags*/ 0);
+m_sections_ap->AddSection(section_sp);
+unified_section_list.AddSection(section_sp);
+  };
+  while (!text.empty()) {
+llvm::StringRef line;
+std::tie(line, text) = text.split('\n');
+
+Token token = toToken(getToken(line).first);
+if (token == Token::Unknown) {
+  // We assume this is a line record, which logically belongs to the Func
+  // section. Errors will be handled when parsing the Func section.
+  token = Token::Func;
+}
+if (token == current_section)
+  continue;
+
+// Changing sections, finish off the previous one, if there was any.
+maybe_add_section(line.bytes_begin());
+// And sta

[Lldb-commits] [PATCH] D56389: [CMake] Fix standalone builds: make dependency to LLVM's `count` conditional

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz created this revision.
sgraenitz added reviewers: davide, aprantl, JDevlieghere.
Herald added a subscriber: mgorny.
Herald added a reviewer: alexshap.

In standalone builds of LLDB we currently have no target `count` that tests can 
depend on. This may be fixed in the future by exporting the target from LLVM 
similar to targets llvm-config, dsymutil and others.

In-tree build with this patch:

  $ ninja -t query tools/lldb/lit/check-lldb-lit
  tools/lldb/lit/check-lldb-lit:
  input: phony
  tools/lldb/lit/CMakeFiles/check-lldb-lit
  bin/FileCheck
  bin/clang
  bin/count
  bin/darwin-debug
  bin/debugserver
  bin/dsymutil
  bin/llc
  bin/lldb
  bin/lldb-mi
  bin/lldb-server
  bin/lldb-test
  bin/llvm-config
  bin/llvm-mc
  bin/llvm-objcopy
  bin/not
  bin/yaml2obj
  lib/liblldb.dylib
  projects/libcxx/lib/cxx
  tools/lldb/unittests/LLDBUnitTests
  outputs:
  tools/lldb/test/check-lldb
  check-lldb-lit

Standalone build with this patch:

  $ ninja -t query lit/check-lldb-lit
  lit/check-lldb-lit:
  input: phony
  lit/CMakeFiles/check-lldb-lit
  bin/darwin-debug
  bin/debugserver
  bin/lldb
  bin/lldb-mi
  bin/lldb-server
  bin/lldb-test
  lib/liblldb.dylib
  unittests/LLDBUnitTests
  outputs:
  test/check-lldb
  check-lldb-lit


https://reviews.llvm.org/D56389

Files:
  lit/CMakeLists.txt


Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -26,7 +26,6 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
-  count
   LLDBUnitTests
   dsymutil
   llc
@@ -73,6 +72,7 @@
 if(NOT LLDB_BUILT_STANDALONE)
   list(APPEND LLDB_TEST_DEPS
 FileCheck
+count
 not
 )
 endif()


Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -26,7 +26,6 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
-  count
   LLDBUnitTests
   dsymutil
   llc
@@ -73,6 +72,7 @@
 if(NOT LLDB_BUILT_STANDALONE)
   list(APPEND LLDB_TEST_DEPS
 FileCheck
+count
 not
 )
 endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56389: [CMake] Fix standalone builds: make dependency to LLVM's `count` conditional

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D56389



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

[Lldb-commits] [lldb] r350537 - Simplify testcase by using lldbutil.run_to_source_breakpoint()

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Jan  7 08:27:52 2019
New Revision: 350537

URL: http://llvm.org/viewvc/llvm-project?rev=350537&view=rev
Log:
Simplify testcase by using lldbutil.run_to_source_breakpoint()

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py?rev=350537&r1=350536&r2=350537&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
 Mon Jan  7 08:27:52 2019
@@ -23,8 +23,6 @@ class ModulesInlineFunctionsTestCase(Tes
 def setUp(self):
 # Call super's setUp().
 TestBase.setUp(self)
-# Find the line number to break inside main().
-self.line = line_number('main.m', '// Set breakpoint here.')
 
 @skipUnlessDarwin
 @skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
@@ -34,19 +32,8 @@ class ModulesInlineFunctionsTestCase(Tes
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
 # Break inside the foo function which takes a bar_ptr argument.
-lldbutil.run_break_set_by_file_and_line(
-self, "main.m", self.line, num_expected_locations=1, 
loc_exact=True)
-
-self.runCmd("run", RUN_SUCCEEDED)
-
-# The stop reason of the thread should be breakpoint.
-self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-substrs=['stopped',
- 'stop reason = breakpoint'])
-
-# The breakpoint should have a hit count of 1.
-self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-substrs=[' resolved, hit count = 1'])
+lldbutil.run_to_source_breakpoint(
+self, '// Set breakpoint here.', lldb.SBFileSpec('main.m'))
 
 self.runCmd(
 "settings set target.clang-module-search-paths \"" +


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

[Lldb-commits] [lldb] r350538 - [CMake] Fix standalone builds: make dependency to LLVM's `count` conditional

[Stefan Granitz via lldb-commits]

Author: stefan.graenitz
Date: Mon Jan  7 08:43:04 2019
New Revision: 350538

URL: http://llvm.org/viewvc/llvm-project?rev=350538&view=rev
Log:
[CMake] Fix standalone builds: make dependency to LLVM's `count` conditional

Summary:
In standalone builds of LLDB we currently have no target `count` that tests can 
depend on. This may be fixed in the future by exporting the target from LLVM 
similar to targets llvm-config, dsymutil and others.

In-tree build with this patch:
```
$ ninja -t query tools/lldb/lit/check-lldb-lit
tools/lldb/lit/check-lldb-lit:
input: phony
tools/lldb/lit/CMakeFiles/check-lldb-lit
bin/FileCheck
bin/clang
bin/count
bin/darwin-debug
bin/debugserver
bin/dsymutil
bin/llc
bin/lldb
bin/lldb-mi
bin/lldb-server
bin/lldb-test
bin/llvm-config
bin/llvm-mc
bin/llvm-objcopy
bin/not
bin/yaml2obj
lib/liblldb.dylib
projects/libcxx/lib/cxx
tools/lldb/unittests/LLDBUnitTests
outputs:
tools/lldb/test/check-lldb
check-lldb-lit
```

Standalone build with this patch:
```
$ ninja -t query lit/check-lldb-lit
lit/check-lldb-lit:
input: phony
lit/CMakeFiles/check-lldb-lit
bin/darwin-debug
bin/debugserver
bin/lldb
bin/lldb-mi
bin/lldb-server
bin/lldb-test
lib/liblldb.dylib
unittests/LLDBUnitTests
outputs:
test/check-lldb
check-lldb-lit
```

Reviewers: davide, aprantl, JDevlieghere, alexshap

Reviewed By: davide

Subscribers: mgorny, lldb-commits, #lldb

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

Modified:
lldb/trunk/lit/CMakeLists.txt

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=350538&r1=350537&r2=350538&view=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Mon Jan  7 08:43:04 2019
@@ -26,7 +26,6 @@ string(REPLACE ${CMAKE_CFG_INTDIR} ${LLV
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
-  count
   LLDBUnitTests
   dsymutil
   llc
@@ -73,6 +72,7 @@ configure_lit_site_cfg(
 if(NOT LLDB_BUILT_STANDALONE)
   list(APPEND LLDB_TEST_DEPS
 FileCheck
+count
 not
 )
 endif()


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

[Lldb-commits] [PATCH] D56389: [CMake] Fix standalone builds: make dependency to LLVM's `count` conditional

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB350538: [CMake] Fix standalone builds: make dependency to 
LLVM's `count` conditional (authored by stefan.graenitz, committed by ).
Herald added a subscriber: abidh.

Changed prior to commit:
  https://reviews.llvm.org/D56389?vs=180493&id=180507#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56389

Files:
  lit/CMakeLists.txt


Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -26,7 +26,6 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
-  count
   LLDBUnitTests
   dsymutil
   llc
@@ -73,6 +72,7 @@
 if(NOT LLDB_BUILT_STANDALONE)
   list(APPEND LLDB_TEST_DEPS
 FileCheck
+count
 not
 )
 endif()


Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -26,7 +26,6 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
-  count
   LLDBUnitTests
   dsymutil
   llc
@@ -73,6 +72,7 @@
 if(NOT LLDB_BUILT_STANDALONE)
   list(APPEND LLDB_TEST_DEPS
 FileCheck
+count
 not
 )
 endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r350543 - Fine-tune and document the barrier in TestQueues.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Jan  7 09:18:39 2019
New Revision: 350543

URL: http://llvm.org/viewvc/llvm-project?rev=350543&view=rev
Log:
Fine-tune and document the barrier in TestQueues.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c?rev=350543&r1=350542&r2=350543&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c Mon Jan  7 
09:18:39 2019
@@ -10,6 +10,8 @@ atomic_int thread_count = 0;
 void
 doing_the_work_1(void *in)
 {
+// This is only counted once because the first job in the queue
+// starves all the others.
 atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (1);
@@ -29,13 +31,15 @@ submit_work_1b(void *in)
 dispatch_queue_t *work_performer_1 = (dispatch_queue_t*) in;
 dispatch_async_f (*work_performer_1, NULL, doing_the_work_1);
 dispatch_async_f (*work_performer_1, NULL, doing_the_work_1);
+atomic_fetch_add(&thread_count, 1);
 while (1)
-  sleep (1);
+sleep (1);
 }
 
 void
 doing_the_work_2(void *in)
 {
+atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (1);
 }
@@ -45,7 +49,7 @@ submit_work_2(void *in)
 {
 dispatch_queue_t *work_performer_2 = (dispatch_queue_t*) in;
 int i = 0;
-while (i++ <  5000)
+while (i++ < 5000)
 {
 dispatch_async_f (*work_performer_2, NULL, doing_the_work_2);
 dispatch_async_f (*work_performer_2, NULL, doing_the_work_2);
@@ -57,8 +61,10 @@ submit_work_2(void *in)
 void
 doing_the_work_3(void *in)
 {
+// This counts four times, since the queue is marked as CONCURRENT.
+atomic_fetch_add(&thread_count, 1);
 while (1)
-sleep(1);
+sleep (1);
 }
 
 void
@@ -105,40 +111,40 @@ int main (int argc, const char **argv)
 atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (10);
-});
+  });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), 
^{
 pthread_setname_np ("user interactive QoS");
 atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (10);
-});
+  });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{
 pthread_setname_np ("default QoS");
 atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (10);
-});
+  });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_UTILITY, 0), ^{
 pthread_setname_np ("utility QoS");
 atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (10);
-});
+  });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_BACKGROUND, 0), ^{
 pthread_setname_np ("background QoS");
 atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (10);
-});
+  });
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_UNSPECIFIED, 0), ^{
 pthread_setname_np ("unspecified QoS");
 atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (10);
-});
+  });
 
 // Unfortunately there is no pthread_barrier on darwin.
-while (atomic_load(&thread_count) < 7)
+while (atomic_load(&thread_count) < 13)
 sleep(1);
 
 while (finished_enqueueing_work == 0)


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

[Lldb-commits] [PATCH] D56234: [lldb-server] Add unnamed pipe support to PipeWindows

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Host/windows/PipeWindows.cpp:34-38
   m_read = INVALID_HANDLE_VALUE;
   m_write = INVALID_HANDLE_VALUE;
 
-  m_read_fd = -1;
-  m_write_fd = -1;
+  m_read_fd = PipeWindows::kInvalidDescriptor;
+  m_write_fd = PipeWindows::kInvalidDescriptor;

Perhaps we should use inline member initialization for these fields in the 
class definition.



Comment at: source/Host/windows/PipeWindows.cpp:51-56
+  m_read_fd = (read_handle == INVALID_HANDLE_VALUE)
+  ? PipeWindows::kInvalidDescriptor
+  : _open_osfhandle((intptr_t)read_handle, _O_RDONLY);
+  m_write_fd = (write_handle == INVALID_HANDLE_VALUE)
+   ? PipeWindows::kInvalidDescriptor
+   : _open_osfhandle((intptr_t)write_handle, _O_WRONLY);

With inline member initialization, this becomes:

```
PipeWindows::PipeWindows(HANDLE read, HANDLE write)
  : m_read(read), m_write(write) {
  if (read != INVALID_HANDLE_VALUE)
m_read_fd = _open_osfhandle((intptr_t)read, _O_RDONLY);
  if (write != INVALID_HANDLE_VALUE)
m_write_fd = _open_osfhandle((intptr_t)write, _O_RDONLY);
}
```



Comment at: source/Host/windows/PipeWindows.cpp:58-59
+
   ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
   ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
 }

Is it valid for a creator of a `PipeWindows` to pass in invalid handles when 
using this constructor?  If not, we should assert.  Certainly it doesn't make 
sense if *both* of the handles are invalid, so we can definitely at least 
assert that case.



Comment at: source/Host/windows/PipeWindows.cpp:69
+  sa.lpSecurityDescriptor = 0;
+  sa.bInheritHandle = child_process_inherit ? TRUE : FALSE;
+  BOOL result = ::CreatePipe(&m_read, &m_write, &sa, 1024);

Is the ternary necessary here?  can't we just assign it?



Comment at: source/Host/windows/PipeWindows.cpp:120
 
-  // Open the write end of the pipe.
+  // FIXME: For server end pipe, it's not applicable to use CreateFile to get
+  // read or write end of the pipe for the reason that closing of the file will

labath wrote:
> I find it strange reading about "servers" in a low-level utility class that 
> seemingly has nothing to do with serving. I think this will confuse future 
> readers as they will not look at this in the context of this patch. Could you 
> rephrase this?
I actually think the previous comment was fine?



Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1074
+}
+auto write_handle = socket_pipe.GetWriteNativeHandle();
+debugserver_args.AppendArgument(llvm::StringRef("--pipe"));

labath wrote:
> What do you think about adding `GetWriteNativeHandle` to the generic Pipe 
> interface? In posix the file descriptor _is_ the native handle, so PipePosix 
> could just return the fd, but it would allow you to write this bit of code 
> generically without ifdefs (well, maybe except the `AppendCloseFileAction` 
> below). Similarly, we could have a generic Pipe factory function(*) which 
> takes the handles as input, which would allow you to implement the 
> lldb-server side of things without ifdefs.
> 
> (*) I think a factory function (`Pipe::FromNativeHandles`?) would be better 
> than a constructor to avoid ambiguities between descriptors and handles.
That's one possibility, but at the very least we should at least move the 
`CreateNew` and logging out of the ifdef.



Comment at: tools/lldb-server/lldb-gdbserver.cpp:223-226
 #if defined(_WIN32)
-  return Status("Unnamed pipes are not supported on Windows.");
+  Pipe port_pipe{Pipe::kInvalidHandle, (HANDLE)unnamed_pipe_fd_or_handle};
 #else
+  Pipe port_pipe{Pipe::kInvalidDescriptor, (int)unnamed_pipe_fd_or_handle};

One way to get rid of all these ifdefs is to to define a type called 
`lldb::pipe_t` and change `Pipe::kInvalidDescriptor` to be `lldb::pipe_t 
kInvalidPipe` (for example, see definition of `lldb::thread_t` in 
`lldb-types.h`).  Then you could change the constructor to be 
`Pipe::Pipe(lldb::pipe_t read, lldb::pipe_t write)` and this function could be 
written as:

```
Status writeSocketIdToPipe(lldb::pipe_t pipe) {
  Pipe port_pipe(Pipe::kInvalidPipe, pipe);
  return writeSocketIdToPipe(port_pipe, socket_id);
}
```

At that point though, maybe we don't even need a separate function for this 
anymore.



Comment at: tools/lldb-server/lldb-gdbserver.cpp:335
 // now.
-else if (unnamed_pipe_fd >= 0) {
-  error = writeSocketIdToPipe(unnamed_pipe_fd, socket_id);
+else if (unnamed_pipe_fd_or_handle >= 0) {
+  error = writeSocketIdToPipe(unnamed_pipe_fd_or_handle, socket_id);

This needs to change to `unnamed_pipe_fd_or_handle != Pipe::kInva

[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

[Leonard Mosescu via Phabricator via lldb-commits]

lemo updated this revision to Diff 180515.
lemo added a comment.

Incorporating Pavel's suggestions (range-based loop, explicit type instead of 
auto)


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

https://reviews.llvm.org/D56293

Files:
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb
  lit/Minidump/Windows/Sigsegv/sigsegv.test
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/Threading.h"
 
 #include "Plugins/Process/Utility/StopInfoMachException.h"
+
 // C includes
 // C++ includes
 
@@ -80,7 +81,7 @@
 section_sp, module->base_of_image);
   }
 
-  ObjectFile *GetObjectFile() override { return nullptr; }
+ObjectFile *GetObjectFile() override { return nullptr; }
 
   SectionList *GetSectionList() override {
 return Module::GetUnifiedSectionList();
@@ -305,19 +306,22 @@
 
 bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list,
ThreadList &new_thread_list) {
-  uint32_t num_threads = 0;
-  if (m_thread_list.size() > 0)
-num_threads = m_thread_list.size();
+  for (const MinidumpThread& thread : m_thread_list) {
+MinidumpLocationDescriptor context_location = thread.thread_context;
+
+// If the minidump contains an exception context, use it
+if (m_active_exception != nullptr &&
+m_active_exception->thread_id == thread.thread_id) {
+  context_location = m_active_exception->thread_context;
+}
 
-  for (lldb::tid_t tid = 0; tid < num_threads; ++tid) {
 llvm::ArrayRef context;
 if (!m_is_wow64)
-  context = m_minidump_parser.GetThreadContext(m_thread_list[tid]);
+  context = m_minidump_parser.GetThreadContext(context_location);
 else
-  context = m_minidump_parser.GetThreadContextWow64(m_thread_list[tid]);
+  context = m_minidump_parser.GetThreadContextWow64(thread);
 
-lldb::ThreadSP thread_sp(
-new ThreadMinidump(*this, m_thread_list[tid], context));
+lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
 new_thread_list.AddThread(thread_sp);
   }
   return new_thread_list.GetSize(false) > 0;
@@ -549,9 +553,9 @@
 APPEND_OPT(m_dump_linux_all);
 m_option_group.Finalize();
   }
-  
+
   ~CommandObjectProcessMinidumpDump() {}
-  
+
   Options *GetOptions() override { return &m_option_group; }
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
@@ -563,7 +567,7 @@
   return false;
 }
 SetDefaultOptionsIfNoneAreSet();
-
+
 ProcessMinidump *process = static_cast(
 m_interpreter.GetExecutionContext().GetProcessPtr());
 result.SetStatus(eReturnStatusSuccessFinishResult);
@@ -635,7 +639,7 @@
 LoadSubCommand("dump",
 CommandObjectSP(new CommandObjectProcessMinidumpDump(interpreter)));
   }
-  
+
   ~CommandObjectMultiwordProcessMinidump() {}
 };
 
Index: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -58,6 +58,9 @@
 
   llvm::ArrayRef GetThreads();
 
+  llvm::ArrayRef
+  GetThreadContext(const MinidumpLocationDescriptor &location);
+
   llvm::ArrayRef GetThreadContext(const MinidumpThread &td);
 
   llvm::ArrayRef GetThreadContextWow64(const MinidumpThread &td);
Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -107,11 +107,15 @@
 }
 
 llvm::ArrayRef
-MinidumpParser::GetThreadContext(const MinidumpThread &td) {
-  if (td.thread_context.rva + td.thread_context.data_size > GetData().size())
+MinidumpParser::GetThreadContext(const MinidumpLocationDescriptor &location) {
+  if (location.rva + location.data_size > GetData().size())
 return {};
+  return GetData().slice(location.rva, location.data_size);
+}
 
-  return GetData().slice(td.thread_context.rva, td.thread_context.data_size);
+llvm::ArrayRef
+MinidumpParser::GetThreadContext(const MinidumpThread &td) {
+  return GetThreadContext(td.thread_context);
 }
 
 llvm::ArrayRef
Index: lit/Minidump/Windows/Sigsegv/sigsegv.test
===
--- lit/Minidump/Windows/Sigsegv/sigsegv.test
+++ lit/Minidump/Windows/Sigsegv/sigsegv.test
@@ -0,0 +1,13 @@
+// RUN: cd %p/Inputs
+//

[Lldb-commits] [lldb] r350546 - Use the minidump exception record if present

[Leonard Mosescu via lldb-commits]

Author: lemo
Date: Mon Jan  7 09:55:42 2019
New Revision: 350546

URL: http://llvm.org/viewvc/llvm-project?rev=350546&view=rev
Log:
Use the minidump exception record if present

If the minidump contains a saved exception record use it automatically.

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


Added:
lldb/trunk/lit/Minidump/Windows/
lldb/trunk/lit/Minidump/Windows/Sigsegv/
lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/
lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp
lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp   (with props)
lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit
lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb   (with props)
lldb/trunk/lit/Minidump/Windows/Sigsegv/sigsegv.test
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Added: lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp?rev=350546&view=auto
==
--- lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp (added)
+++ lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp Mon Jan  7 
09:55:42 2019
@@ -0,0 +1,40 @@
+
+// nodefaultlib build: cl -Zi sigsegv.cpp /link /nodefaultlib
+
+#ifdef USE_CRT
+#include 
+#else
+int main();
+extern "C"
+{
+int _fltused;
+void mainCRTStartup() { main(); }
+void printf(const char*, ...) {}
+}
+#endif
+
+void crash(bool crash_self)
+{
+printf("Before...\n");
+if(crash_self)
+{
+printf("Crashing in 3, 2, 1 ...\n");
+*(volatile int*)nullptr = 0;
+}
+printf("After...\n");
+}
+
+int foo(int x, float y, const char* msg)
+{
+bool flag = x > y;
+if(flag)
+printf("x = %d, y = %f, msg = %s\n", x, y, msg);
+crash(flag);
+return x << 1;
+}
+
+int main()
+{
+foo(10, 3.14, "testing");
+}
+

Added: lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp?rev=350546&view=auto
==
Binary file - no diff available.

Propchange: lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp
--
svn:mime-type = application/octet-stream

Added: lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit?rev=350546&view=auto
==
--- lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit (added)
+++ lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit Mon Jan  7 
09:55:42 2019
@@ -0,0 +1,2 @@
+bt all
+dis

Added: lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb?rev=350546&view=auto
==
Binary file - no diff available.

Propchange: lldb/trunk/lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb
--
svn:mime-type = application/octet-stream

Added: lldb/trunk/lit/Minidump/Windows/Sigsegv/sigsegv.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Windows/Sigsegv/sigsegv.test?rev=350546&view=auto
==
--- lldb/trunk/lit/Minidump/Windows/Sigsegv/sigsegv.test (added)
+++ lldb/trunk/lit/Minidump/Windows/Sigsegv/sigsegv.test Mon Jan  7 09:55:42 
2019
@@ -0,0 +1,13 @@
+// RUN: cd %p/Inputs
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 \
+// RUN:   %lldb -c sigsegv.dmp -s sigsegv.lldbinit | FileCheck %s
+
+CHECK: * thread #1, stop reason = Exception 0xc005 encountered at address 
0x7ff7a13110d9
+CHECK:   * frame #0: 0x7ff7a13110d9 sigsegv.exe
+
+CHECK: ->  0x7ff7a13110d9: movl   $0x0, 0x0
+CHECK: 0x7ff7a13110e4: leaq   0x1f45(%rip), %rcx
+CHECK: 0x7ff7a13110eb: callq  0x7ff7a1311019
+CHECK: 0x7ff7a13110f0: addq   $0x28, %rsp
+CHECK: 0x7ff7a13110f4: retq
+CHECK: 0x7ff7a13110f5: int3

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=350546&r1=350545&r2=350546&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpPar

[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

[Leonard Mosescu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB350546: Use the minidump exception record if present 
(authored by lemo, committed by ).

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56293

Files:
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb
  lit/Minidump/Windows/Sigsegv/sigsegv.test
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -58,6 +58,9 @@
 
   llvm::ArrayRef GetThreads();
 
+  llvm::ArrayRef
+  GetThreadContext(const MinidumpLocationDescriptor &location);
+
   llvm::ArrayRef GetThreadContext(const MinidumpThread &td);
 
   llvm::ArrayRef GetThreadContextWow64(const MinidumpThread &td);
Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -107,11 +107,15 @@
 }
 
 llvm::ArrayRef
-MinidumpParser::GetThreadContext(const MinidumpThread &td) {
-  if (td.thread_context.rva + td.thread_context.data_size > GetData().size())
+MinidumpParser::GetThreadContext(const MinidumpLocationDescriptor &location) {
+  if (location.rva + location.data_size > GetData().size())
 return {};
+  return GetData().slice(location.rva, location.data_size);
+}
 
-  return GetData().slice(td.thread_context.rva, td.thread_context.data_size);
+llvm::ArrayRef
+MinidumpParser::GetThreadContext(const MinidumpThread &td) {
+  return GetThreadContext(td.thread_context);
 }
 
 llvm::ArrayRef
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/Threading.h"
 
 #include "Plugins/Process/Utility/StopInfoMachException.h"
+
 // C includes
 // C++ includes
 
@@ -80,7 +81,7 @@
 section_sp, module->base_of_image);
   }
 
-  ObjectFile *GetObjectFile() override { return nullptr; }
+ObjectFile *GetObjectFile() override { return nullptr; }
 
   SectionList *GetSectionList() override {
 return Module::GetUnifiedSectionList();
@@ -305,19 +306,22 @@
 
 bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list,
ThreadList &new_thread_list) {
-  uint32_t num_threads = 0;
-  if (m_thread_list.size() > 0)
-num_threads = m_thread_list.size();
+  for (const MinidumpThread& thread : m_thread_list) {
+MinidumpLocationDescriptor context_location = thread.thread_context;
+
+// If the minidump contains an exception context, use it
+if (m_active_exception != nullptr &&
+m_active_exception->thread_id == thread.thread_id) {
+  context_location = m_active_exception->thread_context;
+}
 
-  for (lldb::tid_t tid = 0; tid < num_threads; ++tid) {
 llvm::ArrayRef context;
 if (!m_is_wow64)
-  context = m_minidump_parser.GetThreadContext(m_thread_list[tid]);
+  context = m_minidump_parser.GetThreadContext(context_location);
 else
-  context = m_minidump_parser.GetThreadContextWow64(m_thread_list[tid]);
+  context = m_minidump_parser.GetThreadContextWow64(thread);
 
-lldb::ThreadSP thread_sp(
-new ThreadMinidump(*this, m_thread_list[tid], context));
+lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
 new_thread_list.AddThread(thread_sp);
   }
   return new_thread_list.GetSize(false) > 0;
@@ -549,9 +553,9 @@
 APPEND_OPT(m_dump_linux_all);
 m_option_group.Finalize();
   }
-  
+
   ~CommandObjectProcessMinidumpDump() {}
-  
+
   Options *GetOptions() override { return &m_option_group; }
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
@@ -563,7 +567,7 @@
   return false;
 }
 SetDefaultOptionsIfNoneAreSet();
-
+
 ProcessMinidump *process = static_cast(
 m_interpreter.GetExecutionContext().GetProcessPtr());
 result.SetStatus(eReturnStatusSuccessFinishResult);
@@ -635,7 +639,7 @@
 LoadSubCommand("dump",
 CommandObjectSP(new CommandObjectProcessMinidumpDump(interpreter)));
   }
-  
+
   ~CommandObjectMultiwordProcessMinidump() {}
 };
 
Index: lit/Minidump/Windows/Sigsegv/sigsegv.test
===
--- lit/Minidump/Windows/Sigsegv/sigsegv.test
+++ lit/Minidump/Windows/Sigs

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: include/lldb/Host/windows/PosixApi.h:78-79
 
+#define WNOHANG 1
+#define WUNTRACED 2
+

krytarowski wrote:
> I think that these symbols should not be leaked here in the first place.
In general we should avoid mocking posix APIs for Windows.  If something is 
currently implemented in terms of a Posix API that doesn't exist on Windows, we 
should provide an implementation of that function with an entirely new name and 
platform-agnostic set of parameters whose interface need not resemble the posix 
interface in any way..  

For example, I see this is used in `lldb-platform.cpp` when we call 
`waitpid()`.  Instead, we could add something like `void 
WaitForAllChildProcessesToExit();` and then just implement that function on 
posix and Windows completely separately.  This is more flexible and portable 
than trying to fit Windows semantics into a posix api, which doesn't always 
work.



Comment at: include/lldb/Host/windows/PosixApi.h:108-111
+inline pid_t waitpid(pid_t pid, int *status, int options) {
+  // To be implemented.
+  return pid_t(-1);
+}

labath wrote:
> What are your plans for this?
> 
> I assume you needed this for the fork-server in lldb-platform.cpp. I think 
> the best way to address that would be to change the code from spawning a 
> separate process for each connection to having a new thread for each 
> connection. That should be much more portable and will avoid the need to mock 
> posix apis on windows. I can help you with that if you run into any problems 
> there.
Are you ok with the behavioral change?  What if one connection crashes?

Assuming we wanted to implement this behavior on Windows, the way to do it 
would be to create all child processes in a single "job" (aka process group), 
as that is the best way to get notifications for when all childs have 
terminated.



Comment at: source/Host/common/SocketAddress.cpp:263-267
+#ifdef _WIN32
+error.SetError(err, lldb::eErrorTypeWin32);
+#else
+error.SetError(err, lldb::eErrorTypePOSIX);
+#endif

Perhaps we could define something like `lldb::eErrorTypeHostNative` to avoid 
this ifdef.



Comment at: tools/lldb-server/SystemInitializerLLGS.cpp:37-52
+#if defined(_WIN32)
+  if (g_ref_count++ == 0) {
+// Require Windows Sockets version 2.2.
+auto wVersion = MAKEWORD(2, 2);
+WSADATA wsaData;
+auto err = WSAStartup(wVersion, &wsaData);
+if (err == 0) {

labath wrote:
> I think a better option here would be to move this code to something like 
> `Socket::Initialize`. Then we could call this from 
> `SystemInitializerCommon::Initialize`, and it would be available to everyone. 
> This would allow us to remove the copy of this code in PlatformWindows, and 
> replace the `WSAStartup` calls in various unittests with 
> `Socket::Initialize()`
Also, why do we need the `g_ref_count`?  Can't we just print an error message 
and call exit if the version is not what we expect?



Comment at: tools/lldb-server/SystemInitializerLLGS.cpp:58-61
+#if defined(_WIN32)
+  if (g_ref_count && --g_ref_count == 0)
+WSACleanup();
+#endif

I don't think Initialize and Terminate need to be re-entrant.  So we can 
probably delete the ref count code here



Comment at: tools/lldb-server/lldb-server.cpp:40
 
-static void initialize() {
+namespace llsvr {
+void initialize() {

labath wrote:
> In other places we use `llgs` (for `LL`db `G`db `S`erver), so I suggest 
> sticking to that.
The `static` from before indicates that the function should not have external 
linkage, so I think we should keep that.  Whether we put it in a namespace or 
not is orthogonal, but for global functions in cpp functions, we generally 
prefer to mark them all static.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56233



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

[Lldb-commits] [PATCH] D56232: [lldb-server] Add remote platform capabilities for Windows

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D56232#1344652 , @labath wrote:

> All of these functions seem identical to their PlatformPOSIX counterparts. Is 
> that right? And I seem to remember already seeing a lot of code duplication 
> between PlatformPOSIX and PlatformWindows.
>
> It sounds to me like we should create a new common base class for 
> PlatformWindows and PlatformPOSIX (`RemoteAwarePlatform`?), and put the 
> common code there.


Is that even necessary?  If a platform is not remote aware, `IsHost()` will 
always just return `true` by definition.  So we could put all of this in the 
existing `Platform` base class.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56232



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

[Lldb-commits] [PATCH] D56232: [lldb-server] Add remote platform capabilities for Windows

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:189-195
+  else {
+if (m_remote_platform_sp)
+  return m_remote_platform_sp->RunShellCommand(
+  command, working_dir, status_ptr, signo_ptr, command_output, 
timeout);
+else
+  return Status("unable to run a remote command without a platform");
+  }

Remove the `else` here since the if branch already has a return.



Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:193-194
+  command, working_dir, status_ptr, signo_ptr, command_output, 
timeout);
+else
+  return Status("unable to run a remote command without a platform");
+  }

And this else too.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56232



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

[Lldb-commits] [PATCH] D56231: [lldb-server] Improve support on Windows

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Host/common/File.cpp:606-607
 #else
 long cur = ::lseek(m_descriptor, 0, SEEK_CUR);
+SeekFromStart(offset);
 error = Write(buf, num_bytes);

Be careful here.  `pwrite` on posix is atomic, which means that if multiple 
threads both use `pwrite` at different offsets, there is no race condition.
The only way to do a similar atomic write-at-offset on Windows is with 
overlapped i/o., and it's a completely different API / interface.

I mention this because in a previous review Pavel suggested to change 
`lldb-server` to spawn 1 thread / connection instead of 1 process / connection, 
so this can be a real problem in such a scenario if they are racing for the 
same file.  Probably they won't be, since each connection will use different 
output files, but we should at least be aware of (and perhaps document) the 
different in atomicity semantics here.

Alternatively, we could just err on the side of safety and put this in a mutex.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56231



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

[Lldb-commits] [PATCH] D56399: [CMake] Fix standalone builds: workaround the cxx target not getting imported yet (unlike clang target)

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz created this revision.
sgraenitz added reviewers: aprantl, JDevlieghere.
Herald added a subscriber: mgorny.

Handle standalone builds separately and print a warning if we have no libcxx.


https://reviews.llvm.org/D56399

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -101,10 +101,22 @@
 list(APPEND LLDB_TEST_DEPS liblldb)
   endif()
 
+  # Add dependencies if we test with the in-tree clang.
+  # This works with standalone builds as they import the clang target.
   if(TARGET clang)
 list(APPEND LLDB_TEST_DEPS clang)
 if(APPLE)
-  list(APPEND LLDB_TEST_DEPS cxx)
+  # If we build clang, we should build libcxx.
+  # FIXME: Standalone builds should import the cxx target as well.
+  if(LLDB_BUILT_STANDALONE)
+# For now check that the include directory exists.
+set(cxx_dir "${LLVM_DIR}/../../../include/c++")
+if(NOT EXISTS ${cxx_dir})
+  message(WARNING "LLDB test suite requires libc++ in 
llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+endif()
+  else()
+list(APPEND LLDB_TEST_DEPS cxx)
+  endif()
 endif()
   endif()
 


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -101,10 +101,22 @@
 list(APPEND LLDB_TEST_DEPS liblldb)
   endif()
 
+  # Add dependencies if we test with the in-tree clang.
+  # This works with standalone builds as they import the clang target.
   if(TARGET clang)
 list(APPEND LLDB_TEST_DEPS clang)
 if(APPLE)
-  list(APPEND LLDB_TEST_DEPS cxx)
+  # If we build clang, we should build libcxx.
+  # FIXME: Standalone builds should import the cxx target as well.
+  if(LLDB_BUILT_STANDALONE)
+# For now check that the include directory exists.
+set(cxx_dir "${LLVM_DIR}/../../../include/c++")
+if(NOT EXISTS ${cxx_dir})
+  message(WARNING "LLDB test suite requires libc++ in llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+endif()
+  else()
+list(APPEND LLDB_TEST_DEPS cxx)
+  endif()
 endif()
   endif()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz created this revision.
sgraenitz added reviewers: aprantl, JDevlieghere, labath, vsk.
Herald added a subscriber: mgorny.

Fix some issues in the code around D56399 :

- remove outdated comments
- streamline empty-string conditions
- remove 2 uses of LLDB_TEST_USE_CUSTOM_C(XX)_COMPILER -- there are two more in 
`lit/CMakeLists.txt` that should follow
- turn FATAL_ERROR no test compilers into WARNING (plus add quotes around uses 
in `lit/CMakeLists.txt`)


https://reviews.llvm.org/D56400

Files:
  CMakeLists.txt
  lit/CMakeLists.txt


Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -12,11 +12,11 @@
 endif()
 
 if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER 
${LLDB_TEST_C_COMPILER})
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER 
"${LLDB_TEST_C_COMPILER}")
 endif ()
 
 if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER 
${LLDB_TEST_CXX_COMPILER})
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER 
"${LLDB_TEST_CXX_COMPILER}")
 endif ()
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -35,43 +35,36 @@
 add_subdirectory(source)
 add_subdirectory(tools)
 
-option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests."
-  ${LLVM_INCLUDE_TESTS})
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
 option(LLDB_TEST_USE_CUSTOM_C_COMPILER "Use the C compiler provided via 
LLDB_TEST_C_COMPILER for building test inferiors (instead of the just-built 
compiler). Defaults to OFF." OFF)
 option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via 
LLDB_TEST_CXX_COMPILER for building test inferiors (instead of the just-built 
compiler). Defaults to OFF." OFF)
+
 if(LLDB_INCLUDE_TESTS)
+  # FIXME: LLDB_TEST_CXX_COMPILER is unused.
+  # FIXME: In standalone builds LLVM_BINARY_DIR points to LLDB's build 
directory and not to LLVM's!
+  # FIXME: LLDB_TEST_USE_CUSTOM_C/CXX_COMPILER options don't work as intended!
+  # FIXME: With generator expressions we could simply query the output paths 
for each target,
+  # but for this LLDB_DOTEST_ARGS must always have a generator step.
 
-  # Set the path to the default lldb test executable. Make the path relative to
-  # LLVM_RUNTIME_OUTPUT_INTDIR: this will be correct even when LLVM and LLDB
-  # have separate binary directories.
+  # Set the path to the default lldb test executable.
   set(LLDB_DEFAULT_TEST_EXECUTABLE 
"${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
 
-  # Set the paths to default llvm tools. Make these paths relative to the LLVM
-  # binary directory.
+  # Set the paths to default llvm tools.
   set(LLDB_DEFAULT_TEST_DSYMUTIL 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
   set(LLDB_DEFAULT_TEST_FILECHECK 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/FileCheck${CMAKE_EXECUTABLE_SUFFIX}")
 
-  if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER AND TARGET clang)
-set(LLDB_DEFAULT_TEST_C_COMPILER 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
-  else()
-set(LLDB_DEFAULT_TEST_C_COMPILER "")
-  endif()
-
-  if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER AND TARGET clang)
-set(LLDB_DEFAULT_TEST_CXX_COMPILER 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang++${CMAKE_EXECUTABLE_SUFFIX}")
-  else()
-set(LLDB_DEFAULT_TEST_CXX_COMPILER "")
+  if(TARGET clang)
+set(LLDB_DEFAULT_TEST_COMPILER 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
   endif()
 
   set(LLDB_TEST_EXECUTABLE "${LLDB_DEFAULT_TEST_EXECUTABLE}" CACHE PATH "lldb 
executable used for testing")
-  set(LLDB_TEST_C_COMPILER "${LLDB_DEFAULT_TEST_C_COMPILER}" CACHE PATH "C 
Compiler to use for building LLDB test inferiors")
-  set(LLDB_TEST_CXX_COMPILER "${LLDB_DEFAULT_TEST_CXX_COMPILER}" CACHE PATH 
"C++ Compiler to use for building LLDB test inferiors")
+  set(LLDB_TEST_C_COMPILER "${LLDB_DEFAULT_TEST_COMPILER}" CACHE PATH "C 
Compiler to use for building LLDB test inferiors")
+  set(LLDB_TEST_CXX_COMPILER "${LLDB_DEFAULT_TEST_COMPILER}" CACHE PATH "C++ 
Compiler to use for building LLDB test inferiors")
   set(LLDB_TEST_DSYMUTIL "${LLDB_DEFAULT_TEST_DSYMUTIL}" CACHE PATH "dsymutil 
used for generating dSYM bundles")
   set(LLDB_TEST_FILECHECK "${LLDB_DEFAULT_TEST_FILECHECK}" CACHE PATH 
"FileCheck used for testing purposes")
 
-  if (("${LLDB_TEST_C_COMPILER}" STREQUAL "") OR
-  ("${LLDB_TEST_CXX_COMPILER}" STREQUAL ""))
-message(FATAL_ERROR "LLDB test compilers not specified.  Tests will not 
run")
+  if((NOT LLDB_TEST_C_CO

[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

[Stefan Gränitz via Phabricator via lldb-commits]

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



Comment at: CMakeLists.txt:47
+  # FIXME: With generator expressions we could simply query the output paths 
for each target,
+  # but for this LLDB_DOTEST_ARGS must always have a generator step.
 

I think these would be all good to fix, but I am not sure I can do it now. 
Should we keep remaining ones as FIXME?
Currently investigating the `LLVM_BINARY_DIR` issue.


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

https://reviews.llvm.org/D56400



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

[Lldb-commits] [lldb] r350557 - Refactor test, no changes expected.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Jan  7 11:19:34 2019
New Revision: 350557

URL: http://llvm.org/viewvc/llvm-project?rev=350557&view=rev
Log:
Refactor test, no changes expected.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c?rev=350557&r1=350556&r2=350557&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c Mon Jan  7 
11:19:34 2019
@@ -4,7 +4,7 @@
 #include 
 #include 
 
-int finished_enqueueing_work = 0;
+atomic_int finished_enqueueing_work = 0;
 atomic_int thread_count = 0;
 
 void
@@ -54,7 +54,7 @@ submit_work_2(void *in)
 dispatch_async_f (*work_performer_2, NULL, doing_the_work_2);
 dispatch_async_f (*work_performer_2, NULL, doing_the_work_2);
 }
-finished_enqueueing_work = 1;
+atomic_fetch_add(&finished_enqueueing_work, 1);
 }
 
 
@@ -144,11 +144,8 @@ int main (int argc, const char **argv)
   });
 
 // Unfortunately there is no pthread_barrier on darwin.
-while (atomic_load(&thread_count) < 13)
-sleep(1);
-
-while (finished_enqueueing_work == 0)
+while ((atomic_load(&thread_count) < 13) || (finished_enqueueing_work == 
0))
 sleep (1);
-stopper ();
 
+stopper ();
 }


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

[Lldb-commits] [lldb] r350559 - Split two sub-tests into separate top-level methods.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Jan  7 11:24:04 2019
New Revision: 350559

URL: http://llvm.org/viewvc/llvm-project?rev=350559&view=rev
Log:
Split two sub-tests into separate top-level methods.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py?rev=350559&r1=350558&r2=350559&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py Mon 
Jan  7 11:24:04 2019
@@ -18,10 +18,16 @@ class TestQueues(TestBase):
 
 @skipUnlessDarwin
 @add_test_categories(['pyapi'])
-def test_with_python_api(self):
+def test_with_python_api_queues(self):
 """Test queues inspection SB APIs."""
 self.build()
 self.queues()
+
+@skipUnlessDarwin
+@add_test_categories(['pyapi'])
+def test_with_python_api_queues_with_backtrace(self):
+"""Test queues inspection SB APIs."""
+self.build()
 self.queues_with_libBacktraceRecording()
 
 def setUp(self):


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

[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

[Stefan Gränitz via Phabricator via lldb-commits]

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



Comment at: CMakeLists.txt:44
+  # FIXME: LLDB_TEST_CXX_COMPILER is unused.
+  # FIXME: In standalone builds LLVM_BINARY_DIR points to LLDB's build 
directory and not to LLVM's!
+  # FIXME: LLDB_TEST_USE_CUSTOM_C/CXX_COMPILER options don't work as intended!

This was here already when the file was created:
https://github.com/llvm-mirror/lldb/blob/98c187f2613fd8e2f5cdf5f8f35503a102a54f11/cmake/modules/LLDBStandalone.cmake#L67

Apparently the below default paths for dsymutil, FileCheck and clang never 
worked in standalone builds..


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

https://reviews.llvm.org/D56400



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

[Lldb-commits] [lldb] r350570 - [lldb] Fix -Wstring-plus-int warning in POSIX-DYLD/AuxVector.cpp

[Jorge Gorbe Moya via lldb-commits]

Author: jgorbe
Date: Mon Jan  7 13:04:12 2019
New Revision: 350570

URL: http://llvm.org/viewvc/llvm-project?rev=350570&view=rev
Log:
[lldb] Fix -Wstring-plus-int warning in POSIX-DYLD/AuxVector.cpp

Modified:
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp

Modified: lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp?rev=350570&r1=350569&r2=350570&view=diff
==
--- lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp (original)
+++ lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp Mon Jan  7 
13:04:12 2019
@@ -100,7 +100,7 @@ const char *AuxVector::GetEntryName(Entr
 
 #define ENTRY_NAME(_type) \
   _type:  \
-  name = #_type + 5
+  name = &#_type[5]
   switch (type) {
 case ENTRY_NAME(AUXV_AT_NULL);   break;
 case ENTRY_NAME(AUXV_AT_IGNORE); break;


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

[Lldb-commits] [PATCH] D56315: Add a verbose mode to "image dump line-table" and use it to write a .debug_line test

[Zachary Turner via Phabricator via lldb-commits]

zturner accepted this revision.
zturner added a comment.

Ok, that makes sense.  BTW, it would be nice if someone ever decided to make 
`llvm-mc` recognize more symbolic constants so we didn't have to use magic 
numbers everywhere.


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

https://reviews.llvm.org/D56315



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

[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: CMakeLists.txt:43
 if(LLDB_INCLUDE_TESTS)
+  # FIXME: LLDB_TEST_CXX_COMPILER is unused.
+  # FIXME: In standalone builds LLVM_BINARY_DIR points to LLDB's build 
directory and not to LLVM's!

Why is this unused? Are we only using `LLDB_TEST_C_COMPILER` and if so, why?



Comment at: CMakeLists.txt:44
+  # FIXME: LLDB_TEST_CXX_COMPILER is unused.
+  # FIXME: In standalone builds LLVM_BINARY_DIR points to LLDB's build 
directory and not to LLVM's!
+  # FIXME: LLDB_TEST_USE_CUSTOM_C/CXX_COMPILER options don't work as intended!

sgraenitz wrote:
> This was here already when the file was created:
> https://github.com/llvm-mirror/lldb/blob/98c187f2613fd8e2f5cdf5f8f35503a102a54f11/cmake/modules/LLDBStandalone.cmake#L67
> 
> Apparently the below default paths for dsymutil, FileCheck and clang never 
> worked in standalone builds..
Instead of this FIXME, should we explicitly unset this variable to prevent 
mistakes in the future? Or is this fixable?


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

https://reviews.llvm.org/D56400



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

[Lldb-commits] [lldb] r350575 - Rename DWARFDIE::GetDWOContext() -> GetDeclContext() (NFC)

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Jan  7 14:47:17 2019
New Revision: 350575

URL: http://llvm.org/viewvc/llvm-project?rev=350575&view=rev
Log:
Rename DWARFDIE::GetDWOContext() -> GetDeclContext() (NFC)

Despite the name, this function has nothing to do with the DWO format.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=350575&r1=350574&r2=350575&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Mon Jan  
7 14:47:17 2019
@@ -144,7 +144,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
 
   // This type comes from an external DWO module.
   std::vector dwo_context;
-  die.GetDWOContext(dwo_context);
+  die.GetDeclContext(dwo_context);
   TypeMap dwo_types;
 
   if (!dwo_module_sp->GetSymbolVendor()->FindTypes(dwo_context, true,

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp?rev=350575&r1=350574&r2=350575&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp Mon Jan  7 14:47:17 
2019
@@ -166,13 +166,13 @@ void DWARFDIE::GetDWARFDeclContext(DWARF
   }
 }
 
-void DWARFDIE::GetDWOContext(std::vector &context) const {
+void DWARFDIE::GetDeclContext(std::vector &context) const {
   const dw_tag_t tag = Tag();
   if (tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit)
 return;
   DWARFDIE parent = GetParent();
   if (parent)
-parent.GetDWOContext(context);
+parent.GetDeclContext(context);
   switch (tag) {
   case DW_TAG_module:
 context.push_back(

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.h?rev=350575&r1=350574&r2=350575&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.h Mon Jan  7 14:47:17 
2019
@@ -90,7 +90,10 @@ public:
 
   void GetDWARFDeclContext(DWARFDeclContext &dwarf_decl_ctx) const;
 
-  void GetDWOContext(std::vector &context) 
const;
+  /// Return this DIE's decl context as it is needed to look up types
+  /// in Clang's -gmodules debug info format.
+  void
+  GetDeclContext(std::vector &context) const;
 
   //--
   // Getting attribute values from the DIE.

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=350575&r1=350574&r2=350575&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Mon Jan  7 
14:47:17 2019
@@ -2573,7 +2573,7 @@ size_t SymbolFileDWARF::FindTypes(const
 
   if (die) {
 std::vector die_context;
-die.GetDWOContext(die_context);
+die.GetDeclContext(die_context);
 if (die_context != context)
   continue;
 


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

[Lldb-commits] [lldb] r350576 - Clarify comment and variable names. (NFC)

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Jan  7 15:02:28 2019
New Revision: 350576

URL: http://llvm.org/viewvc/llvm-project?rev=350576&view=rev
Log:
Clarify comment and variable names. (NFC)

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=350576&r1=350575&r2=350576&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Mon Jan  
7 15:02:28 2019
@@ -142,12 +142,14 @@ TypeSP DWARFASTParserClang::ParseTypeFro
   if (!dwo_module_sp)
 return TypeSP();
 
-  // This type comes from an external DWO module.
-  std::vector dwo_context;
-  die.GetDeclContext(dwo_context);
+  // If this type comes from a Clang module, look in the DWARF section
+  // of the pcm file in the module cache. Clang generates DWO skeleton
+  // units as breadcrumbs to find them.
+  std::vector decl_context;
+  die.GetDeclContext(decl_context);
   TypeMap dwo_types;
 
-  if (!dwo_module_sp->GetSymbolVendor()->FindTypes(dwo_context, true,
+  if (!dwo_module_sp->GetSymbolVendor()->FindTypes(decl_context, true,
dwo_types)) {
 if (!isClangModuleFwdDecl(die))
   return TypeSP();
@@ -159,7 +161,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
   if (!NameModule.second)
 continue;
   SymbolVendor *SymVendor = NameModule.second->GetSymbolVendor();
-  if (SymVendor->FindTypes(dwo_context, true, dwo_types))
+  if (SymVendor->FindTypes(decl_context, true, dwo_types))
 break;
 }
   }


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

[Lldb-commits] [lldb] r350577 - Simplify code.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Jan  7 15:08:16 2019
New Revision: 350577

URL: http://llvm.org/viewvc/llvm-project?rev=350577&view=rev
Log:
Simplify code.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=350577&r1=350576&r2=350577&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Mon Jan  
7 15:08:16 2019
@@ -166,8 +166,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
 }
   }
 
-  const size_t num_dwo_types = dwo_types.GetSize();
-  if (num_dwo_types != 1)
+  if (dwo_types.GetSize() != 1)
 return TypeSP();
 
   // We found a real definition for this type in the Clang module, so lets use


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

[Lldb-commits] [PATCH] D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added reviewers: labath, clayborg, aleksandr.urakov, asmith, amccarth.
Herald added subscribers: JDevlieghere, aprantl.

`ParseDeclsForContext` was originally created to serve the very specific case 
where the context is a function block.  It was never intended to be used for 
arbitrary DeclContexts, however due to the generic name, the DWARF and PDB 
plugins implemented it in this way "just in case".  Then, `lldb-test` came 
along and decided to use it in that way.

Related to this, there are a set of functions in the `SymbolFile` class 
interface whose requirements and expectations are not documented.  For example, 
if you call `ParseCompileUnitFunctions`, there's an inherent requirement that 
you create entries in the underlying clang AST for these functions as well as 
their signature types, because in order to create an `lldb_private::Function` 
object, you have to pass it a `CompilerType` for the parameter representing the 
signature.

On the other hand, there is no similar requirement (either inherent or 
documented) if one were to call `ParseDeclsForContext`.  Specifically, if one 
calls `ParseDeclsForContext`, and some variable declarations, types, and other 
things are added to the clang AST, is it necessary to create `lldb::Variable`, 
`lldb::Type, etc objects representing them?  Nobody knows.  There is, however, 
an //accidental// requirement, because since all of the plugins implemented 
this just in case, `lldb-test` came along and used `ParsedDeclsForContext`, and 
then wrote check lines that depended on this.

When I went to try and implemented the NativePDB reader, I did not adhere to 
this (in fact, from a layering perspective I went out of my way to avoid it), 
and as a result the existing DIA PDB tests don't work when the native PDB 
reader is enabled, because they expect that calling ParseDeclsForContext will 
modify the *module's* view of symbols, and not just the internal AST.

All of this confusion, however, can be avoided if we simply stick to using 
`ParseDeclsForContext` for its original intended use case (blocks), and use a 
different function (`ParseAllDebugSymbols`) for its intended use case which is, 
unsuprisingly, to parse all the debug symbols (which is all `lldb-test` really 
wanted to do anyway).

In the future, I would like to change `ParseDeclsForContext` to 
`ParseDeclsForFunctionBlock`, then delete all of the dead code inside that 
handles other types of DeclContexts (and probably even assert if the 
DeclContext is anything other than a block).

A few PDB tests needed to be fixed up as a result of this, and this also 
exposed a couple of bugs in the DIA PDB reader (doesn't matter much since it 
should be going away soon, but worth mentioning) where the appropriate AST 
entries weren't being created always.


https://reviews.llvm.org/D56418

Files:
  lldb/lit/SymbolFile/PDB/enums-layout.test
  lldb/lit/SymbolFile/PDB/type-quals.test
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -518,6 +518,7 @@
 
 Error opts::symbols::dumpAST(lldb_private::Module &Module) {
   SymbolVendor &plugin = *Module.GetSymbolVendor();
+   Module.ParseAllDebugSymbols();
 
   auto symfile = plugin.GetSymbolFile();
   if (!symfile)
@@ -536,9 +537,6 @@
   if (!tu)
 return make_string_error("Can't retrieve translation unit declaration.");
 
-  symfile->ParseDeclsForContext(CompilerDeclContext(
-  clang_ast_ctx, static_cast(tu)));
-
   tu->print(outs());
 
   return Error::success();
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -20,6 +20,8 @@
 #include "llvm/DebugInfo/PDB/PDB.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 
+class PDBASTParser;
+
 class SymbolFilePDB : public lldb_private::SymbolFile {
 public:
   //--
@@ -224,6 +226,8 @@
   void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland &pdb_compiland,
uint32_t &index);
 
+  PDBASTParser *GetPDBAstParser();
+
   std::unique_ptr
   GetPDBCompilandByUID(uint32_t uid);
 
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -314,6 +314,16 @@
  func_type_uid, mangled, func_type, func_range);
 
   sc.comp_unit->AddFunction(func_sp);
+
+  TypeSystem *type_system = GetTypeSystemForLanguage(lldb::eLanguageType

[Lldb-commits] [lldb] r350599 - [SymbolContext] Remove dead code

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Jan  7 17:35:00 2019
New Revision: 350599

URL: http://llvm.org/viewvc/llvm-project?rev=350599&view=rev
Log:
[SymbolContext] Remove dead code

Removes two methods from SymbolContextList that aren't referenced.

Modified:
lldb/trunk/include/lldb/Symbol/SymbolContext.h
lldb/trunk/source/Symbol/SymbolContext.cpp

Modified: lldb/trunk/include/lldb/Symbol/SymbolContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolContext.h?rev=350599&r1=350598&r2=350599&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolContext.h Mon Jan  7 17:35:00 2019
@@ -228,7 +228,7 @@ public:
 
   bool GetAddressRangeFromHereToEndLine(uint32_t end_line, AddressRange &range,
 Status &error);
-  
+
   //--
   /// Find the best global data symbol visible from this context.
   ///
@@ -465,10 +465,6 @@ public:
 
   bool AppendIfUnique(const SymbolContext &sc, bool 
merge_symbol_into_function);
 
-  bool MergeSymbolContextIntoFunctionContext(const SymbolContext &symbol_sc,
- uint32_t start_idx = 0,
- uint32_t stop_idx = UINT32_MAX);
-
   uint32_t AppendIfUnique(const SymbolContextList &sc_list,
   bool merge_symbol_into_function);
 
@@ -527,18 +523,6 @@ public:
 return m_symbol_contexts[idx];
   }
 
-  //--
-  /// Get accessor for the last symbol context in the list.
-  ///
-  /// @param[out] sc
-  /// A reference to the symbol context to fill in.
-  ///
-  /// @return
-  /// Returns \b true if \a sc was filled in, \b false if the
-  /// list is empty.
-  //--
-  bool GetLastContext(SymbolContext &sc) const;
-
   bool RemoveContextAtIndex(size_t idx);
 
   //--

Modified: lldb/trunk/source/Symbol/SymbolContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/SymbolContext.cpp?rev=350599&r1=350598&r2=350599&view=diff
==
--- lldb/trunk/source/Symbol/SymbolContext.cpp (original)
+++ lldb/trunk/source/Symbol/SymbolContext.cpp Mon Jan  7 17:35:00 2019
@@ -796,14 +796,14 @@ bool SymbolContext::GetAddressRangeFromH
 const Symbol *
 SymbolContext::FindBestGlobalDataSymbol(const ConstString &name, Status 
&error) {
   error.Clear();
-  
+
   if (!target_sp) {
 return nullptr;
   }
-  
+
   Target &target = *target_sp;
   Module *module = module_sp.get();
-  
+
   auto ProcessMatches = [this, &name, &target, module]
   (SymbolContextList &sc_list, Status &error) -> const Symbol* {
 llvm::SmallVector external_symbols;
@@ -815,7 +815,7 @@ SymbolContext::FindBestGlobalDataSymbol(
   if (sym_ctx.symbol) {
 const Symbol *symbol = sym_ctx.symbol;
 const Address sym_address = symbol->GetAddress();
-
+
 if (sym_address.IsValid()) {
   switch (symbol->GetType()) {
 case eSymbolTypeData:
@@ -860,12 +860,12 @@ SymbolContext::FindBestGlobalDataSymbol(
 if (name == symbol->GetReExportedSymbolName() &&
 module == reexport_module_sp.get())
   return nullptr;
-
+
 return FindBestGlobalDataSymbol(
 symbol->GetReExportedSymbolName(), error);
   }
 } break;
-  
+
 case eSymbolTypeCode: // We already lookup functions elsewhere
 case eSymbolTypeVariable:
 case eSymbolTypeLocal:
@@ -893,7 +893,7 @@ SymbolContext::FindBestGlobalDataSymbol(
 }
   }
 }
-
+
 if (external_symbols.size() > 1) {
   StreamString ss;
   ss.Printf("Multiple external symbols found for '%s'\n", 
name.AsCString());
@@ -920,32 +920,32 @@ SymbolContext::FindBestGlobalDataSymbol(
   return nullptr;
 }
   };
-  
+
   if (module) {
 SymbolContextList sc_list;
 module->FindSymbolsWithNameAndType(name, eSymbolTypeAny, sc_list);
 const Symbol *const module_symbol = ProcessMatches(sc_list, error);
-
+
 if (!error.Success()) {
   return nullptr;
 } else if (module_symbol) {
   return module_symbol;
 }
   }
-  
+
   {
 SymbolContextList sc_list;
 target.GetImages().FindSymbolsWithNameAndType(name, eSymbolTypeAny,
   sc_list);
 const Symbol *const target_symbol = ProcessMatches(sc_list, error);
-
+
 if (!error.Success()) {
   return nullptr;
 } else if (target_symbol) {
   return target_symbol;
 }
   

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: davide.

I was looking at the code in BreakpointList.cpp and found it deserved a quick 
cleanup.

- Extract duplicate code
- Use range-based for loop
- Use early return in loops


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D56425

Files:
  source/Breakpoint/BreakpointList.cpp

Index: source/Breakpoint/BreakpointList.cpp
===
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -14,6 +14,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static void NotifyChange(BreakpointSP bp, BreakpointEventType event) {
+  if (bp->GetTarget().EventTypeHasListeners(
+  Target::eBroadcastBitBreakpointChanged))
+bp->GetTarget().BroadcastEvent(
+Target::eBroadcastBitBreakpointChanged,
+new Breakpoint::BreakpointEventData(event, bp));
+}
+
 BreakpointList::BreakpointList(bool is_internal)
 : m_mutex(), m_breakpoints(), m_next_break_id(0),
   m_is_internal(is_internal) {}
@@ -26,33 +34,23 @@
   bp_sp->SetID(m_is_internal ? --m_next_break_id : ++m_next_break_id);
 
   m_breakpoints.push_back(bp_sp);
-  if (notify) {
-if (bp_sp->GetTarget().EventTypeHasListeners(
-Target::eBroadcastBitBreakpointChanged))
-  bp_sp->GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged,
-new Breakpoint::BreakpointEventData(
-eBreakpointEventTypeAdded, bp_sp));
-  }
+  if (notify)
+NotifyChange(bp_sp, eBreakpointEventTypeAdded);
   return bp_sp->GetID();
 }
 
 bool BreakpointList::Remove(break_id_t break_id, bool notify) {
   std::lock_guard guard(m_mutex);
+
   bp_collection::iterator pos = GetBreakpointIDIterator(break_id); // Predicate
-  if (pos != m_breakpoints.end()) {
-BreakpointSP bp_sp(*pos);
-m_breakpoints.erase(pos);
-if (notify) {
-  if (bp_sp->GetTarget().EventTypeHasListeners(
-  Target::eBroadcastBitBreakpointChanged))
-bp_sp->GetTarget().BroadcastEvent(
-Target::eBroadcastBitBreakpointChanged,
-new Breakpoint::BreakpointEventData(eBreakpointEventTypeRemoved,
-bp_sp));
-}
-return true;
-  }
-  return false;
+  if (pos == m_breakpoints.end())
+return false;
+
+  BreakpointSP bp_sp(*pos);
+  m_breakpoints.erase(pos);
+  if (notify)
+NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
+  return true;
 }
 
 void BreakpointList::RemoveInvalidLocations(const ArchSpec &arch) {
@@ -79,18 +77,11 @@
   ClearAllBreakpointSites();
 
   if (notify) {
-bp_collection::iterator pos, end = m_breakpoints.end();
-for (pos = m_breakpoints.begin(); pos != end; ++pos) {
-  if ((*pos)->GetTarget().EventTypeHasListeners(
-  Target::eBroadcastBitBreakpointChanged)) {
-(*pos)->GetTarget().BroadcastEvent(
-Target::eBroadcastBitBreakpointChanged,
-new Breakpoint::BreakpointEventData(eBreakpointEventTypeRemoved,
-*pos));
-  }
-}
+for (const auto &bp_sp : m_breakpoints)
+  NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
   }
-  m_breakpoints.erase(m_breakpoints.begin(), m_breakpoints.end());
+
+  m_breakpoints.clear();
 }
 
 void BreakpointList::RemoveAllowed(bool notify) {
@@ -98,18 +89,12 @@
 
   bp_collection::iterator pos, end = m_breakpoints.end();
   if (notify) {
-for (pos = m_breakpoints.begin(); pos != end; ++pos) {
-  if(!(*pos)->AllowDelete())
-continue;
-  if ((*pos)->GetTarget().EventTypeHasListeners(
-  Target::eBroadcastBitBreakpointChanged)) {
-(*pos)->GetTarget().BroadcastEvent(
-Target::eBroadcastBitBreakpointChanged,
-new Breakpoint::BreakpointEventData(eBreakpointEventTypeRemoved,
-*pos));
-  }
+for (const auto &bp_sp : m_breakpoints) {
+  if (bp_sp->AllowDelete())
+NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
 }
   }
+
   pos = m_breakpoints.begin();
   while ( pos != end) {
 auto bp = *pos;
@@ -199,28 +184,25 @@
 
 BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) {
   std::lock_guard guard(m_mutex);
-  BreakpointSP stop_sp;
-  bp_collection::iterator end = m_breakpoints.end();
-  bp_collection::iterator pos;
   size_t curr_i = 0;
-  for (pos = m_breakpoints.begin(), curr_i = 0; pos != end; ++pos, ++curr_i) {
+  for (const auto &bp_sp : m_breakpoints) {
 if (curr_i == i)
-  stop_sp = *pos;
+  return bp_sp;
+curr_i++;
   }
-  return stop_sp;
+  return {};
 }
 
 const BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) const {
   std::lock_guard guard(m_mutex);
   BreakpointSP stop_sp;
-  bp_collection::const_iterator end = m_breakpoints.end();
-  bp_collection::const_iterator pos;
   size_t curr

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I wonder if we should use a vector instead of a list here too. For small data 
sets a std::vector usually outperforms a std::list. I *think* the data-locality 
would outweigh the cost of copying the shared pointers.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56425



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

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

Looks like a nice/reasonable cleanup, thanks!

Based on the coverage report 
(https://teemperor.de/lldb-coverage/coverage/Users/vsk/src/llvm.org-lldbsan/llvm/tools/lldb/source/Breakpoint/BreakpointList.cpp.html#L217)
 and benchmarks (https://teemperor.de/lldb-bench/static.html) 
GetBreakpointAtIndex doesn't looks that hot, so I'm not sure it's worth 
changing / accidentally regressing.




Comment at: source/Breakpoint/BreakpointList.cpp:196
 
 const BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) const {
   std::lock_guard guard(m_mutex);

Just call the non-const overload and const_cast?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56425



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