[Lldb-commits] [lldb] r369880 - [lldb][NFC] Add ProcessInfo::GetNameAsStringRef to simplify some code

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Mon Aug 26 01:22:52 2019
New Revision: 369880

URL: http://llvm.org/viewvc/llvm-project?rev=369880&view=rev
Log:
[lldb][NFC] Add ProcessInfo::GetNameAsStringRef to simplify some code

Modified:
lldb/trunk/include/lldb/Utility/ProcessInfo.h
lldb/trunk/source/Commands/CommandObjectPlatform.cpp
lldb/trunk/source/Commands/CommandObjectProcess.cpp
lldb/trunk/source/Utility/ProcessInfo.cpp

Modified: lldb/trunk/include/lldb/Utility/ProcessInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ProcessInfo.h?rev=369880&r1=369879&r2=369880&view=diff
==
--- lldb/trunk/include/lldb/Utility/ProcessInfo.h (original)
+++ lldb/trunk/include/lldb/Utility/ProcessInfo.h Mon Aug 26 01:22:52 2019
@@ -38,7 +38,7 @@ public:
 
   const char *GetName() const;
 
-  size_t GetNameLength() const;
+  llvm::StringRef GetNameAsStringRef() const;
 
   FileSpec &GetExecutableFile() { return m_executable; }
 
@@ -165,12 +165,8 @@ public:
 
   void Append(const ProcessInstanceInfo &info) { m_infos.push_back(info); }
 
-  const char *GetProcessNameAtIndex(size_t idx) {
-return ((idx < m_infos.size()) ? m_infos[idx].GetName() : nullptr);
-  }
-
-  size_t GetProcessNameLengthAtIndex(size_t idx) {
-return ((idx < m_infos.size()) ? m_infos[idx].GetNameLength() : 0);
+  llvm::StringRef GetProcessNameAtIndex(size_t idx) {
+return ((idx < m_infos.size()) ? m_infos[idx].GetNameAsStringRef() : "");
   }
 
   lldb::pid_t GetProcessIDAtIndex(size_t idx) {

Modified: lldb/trunk/source/Commands/CommandObjectPlatform.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectPlatform.cpp?rev=369880&r1=369879&r2=369880&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectPlatform.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectPlatform.cpp Mon Aug 26 01:22:52 
2019
@@ -1467,9 +1467,7 @@ public:
 return;
 
   for (uint32_t i = 0; i < num_matches; ++i) {
-request.AddCompletion(
-llvm::StringRef(process_infos.GetProcessNameAtIndex(i),
-process_infos.GetProcessNameLengthAtIndex(i)));
+request.AddCompletion(process_infos.GetProcessNameAtIndex(i));
   }
   return;
 }

Modified: lldb/trunk/source/Commands/CommandObjectProcess.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectProcess.cpp?rev=369880&r1=369879&r2=369880&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectProcess.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectProcess.cpp Mon Aug 26 01:22:52 2019
@@ -353,9 +353,7 @@ public:
   if (num_matches == 0)
 return;
   for (size_t i = 0; i < num_matches; ++i) {
-request.AddCompletion(
-llvm::StringRef(process_infos.GetProcessNameAtIndex(i),
-process_infos.GetProcessNameLengthAtIndex(i)));
+request.AddCompletion(process_infos.GetProcessNameAtIndex(i));
   }
 }
 

Modified: lldb/trunk/source/Utility/ProcessInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ProcessInfo.cpp?rev=369880&r1=369879&r2=369880&view=diff
==
--- lldb/trunk/source/Utility/ProcessInfo.cpp (original)
+++ lldb/trunk/source/Utility/ProcessInfo.cpp Mon Aug 26 01:22:52 2019
@@ -42,8 +42,8 @@ const char *ProcessInfo::GetName() const
   return m_executable.GetFilename().GetCString();
 }
 
-size_t ProcessInfo::GetNameLength() const {
-  return m_executable.GetFilename().GetLength();
+llvm::StringRef ProcessInfo::GetNameAsStringRef() const {
+  return m_executable.GetFilename().GetStringRef();
 }
 
 void ProcessInfo::Dump(Stream &s, Platform *platform) const {


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

[Lldb-commits] [lldb] r369885 - [lldb] Construct the dummy target when the first Debugger object is constructed

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Mon Aug 26 02:20:59 2019
New Revision: 369885

URL: http://llvm.org/viewvc/llvm-project?rev=369885&view=rev
Log:
[lldb] Construct the dummy target when the first Debugger object is constructed

Summary:
We should always have a dummy target, so we might as well construct it directly 
when we create a Debugger object.

The idea is that if this patch doesn't cause any problems that we can get rid 
of all the logic
that handles situations where we don't have a dummy target (as all that code is 
currently
untested as there seems to be no way to have no dummy target in LLDB).

Reviewers: labath, jingham

Reviewed By: labath, jingham

Subscribers: jingham, abidh, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/Core/Debugger.h
lldb/trunk/source/Core/Debugger.cpp

Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=369885&r1=369884&r2=369885&view=diff
==
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Mon Aug 26 02:20:59 2019
@@ -311,7 +311,7 @@ public:
   // selected target, or if no target is present you want to prime the dummy
   // target with entities that will be copied over to new targets.
   Target *GetSelectedOrDummyTarget(bool prefer_dummy = false);
-  Target *GetDummyTarget();
+  Target *GetDummyTarget() { return m_dummy_target_sp.get(); }
 
   lldb::BroadcasterManagerSP GetBroadcasterManager() {
 return m_broadcaster_manager_sp;
@@ -400,6 +400,7 @@ protected:
   Broadcaster m_sync_broadcaster;
   lldb::ListenerSP m_forward_listener_sp;
   llvm::once_flag m_clear_once;
+  lldb::TargetSP m_dummy_target_sp;
 
   // Events for m_sync_broadcaster
   enum {

Modified: lldb/trunk/source/Core/Debugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=369885&r1=369884&r2=369885&view=diff
==
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Mon Aug 26 02:20:59 2019
@@ -721,6 +721,9 @@ Debugger::Debugger(lldb::LogOutputCallba
   assert(default_platform_sp);
   m_platform_list.Append(default_platform_sp, true);
 
+  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
+
   m_collection_sp->Initialize(g_debugger_properties);
   m_collection_sp->AppendProperty(
   ConstString("target"),
@@ -1603,10 +1606,6 @@ void Debugger::JoinIOHandlerThread() {
   }
 }
 
-Target *Debugger::GetDummyTarget() {
-  return m_target_list.GetDummyTarget(*this).get();
-}
-
 Target *Debugger::GetSelectedOrDummyTarget(bool prefer_dummy) {
   Target *target = nullptr;
   if (!prefer_dummy) {


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

[Lldb-commits] [PATCH] D66581: [lldb] Construct the dummy target when the first Dummy object is constructed

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369885: [lldb] Construct the dummy target when the first 
Debugger object is constructed (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66581?vs=216581&id=217095#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66581

Files:
  lldb/trunk/include/lldb/Core/Debugger.h
  lldb/trunk/source/Core/Debugger.cpp


Index: lldb/trunk/source/Core/Debugger.cpp
===
--- lldb/trunk/source/Core/Debugger.cpp
+++ lldb/trunk/source/Core/Debugger.cpp
@@ -721,6 +721,9 @@
   assert(default_platform_sp);
   m_platform_list.Append(default_platform_sp, true);
 
+  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
+
   m_collection_sp->Initialize(g_debugger_properties);
   m_collection_sp->AppendProperty(
   ConstString("target"),
@@ -1603,10 +1606,6 @@
   }
 }
 
-Target *Debugger::GetDummyTarget() {
-  return m_target_list.GetDummyTarget(*this).get();
-}
-
 Target *Debugger::GetSelectedOrDummyTarget(bool prefer_dummy) {
   Target *target = nullptr;
   if (!prefer_dummy) {
Index: lldb/trunk/include/lldb/Core/Debugger.h
===
--- lldb/trunk/include/lldb/Core/Debugger.h
+++ lldb/trunk/include/lldb/Core/Debugger.h
@@ -311,7 +311,7 @@
   // selected target, or if no target is present you want to prime the dummy
   // target with entities that will be copied over to new targets.
   Target *GetSelectedOrDummyTarget(bool prefer_dummy = false);
-  Target *GetDummyTarget();
+  Target *GetDummyTarget() { return m_dummy_target_sp.get(); }
 
   lldb::BroadcasterManagerSP GetBroadcasterManager() {
 return m_broadcaster_manager_sp;
@@ -400,6 +400,7 @@
   Broadcaster m_sync_broadcaster;
   lldb::ListenerSP m_forward_listener_sp;
   llvm::once_flag m_clear_once;
+  lldb::TargetSP m_dummy_target_sp;
 
   // Events for m_sync_broadcaster
   enum {


Index: lldb/trunk/source/Core/Debugger.cpp
===
--- lldb/trunk/source/Core/Debugger.cpp
+++ lldb/trunk/source/Core/Debugger.cpp
@@ -721,6 +721,9 @@
   assert(default_platform_sp);
   m_platform_list.Append(default_platform_sp, true);
 
+  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
+
   m_collection_sp->Initialize(g_debugger_properties);
   m_collection_sp->AppendProperty(
   ConstString("target"),
@@ -1603,10 +1606,6 @@
   }
 }
 
-Target *Debugger::GetDummyTarget() {
-  return m_target_list.GetDummyTarget(*this).get();
-}
-
 Target *Debugger::GetSelectedOrDummyTarget(bool prefer_dummy) {
   Target *target = nullptr;
   if (!prefer_dummy) {
Index: lldb/trunk/include/lldb/Core/Debugger.h
===
--- lldb/trunk/include/lldb/Core/Debugger.h
+++ lldb/trunk/include/lldb/Core/Debugger.h
@@ -311,7 +311,7 @@
   // selected target, or if no target is present you want to prime the dummy
   // target with entities that will be copied over to new targets.
   Target *GetSelectedOrDummyTarget(bool prefer_dummy = false);
-  Target *GetDummyTarget();
+  Target *GetDummyTarget() { return m_dummy_target_sp.get(); }
 
   lldb::BroadcasterManagerSP GetBroadcasterManager() {
 return m_broadcaster_manager_sp;
@@ -400,6 +400,7 @@
   Broadcaster m_sync_broadcaster;
   lldb::ListenerSP m_forward_listener_sp;
   llvm::once_flag m_clear_once;
+  lldb::TargetSP m_dummy_target_sp;
 
   // Events for m_sync_broadcaster
   enum {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66249: [JIT][Breakpoint] Add "BreakpointInjectedSite" and FCB Trampoline

[Pavel Labath via Phabricator via lldb-commits]

labath added a subscriber: xiaobai.
labath added a comment.

In D66249#1643594 , @mib wrote:

> In D66249#1642316 , @labath wrote:
>
> > A bunch more comments from me. So far, I've only tried to highlight the 
> > most obvious problems or style issues.
> >
> >   It's not clear to me whether this is still prototype code or not... If it 
> > isn't, I'll have a bunch more.. :)
>
>
> This is prototype code that I'm trying to upstream to build upon.
>
> From the comments I already gathered, I started thinking of a better 
> implementation of this feature (maybe v2).
>  There are still some parts that need do be done, such as resolving the 
> variables on optimized code or patching dynamically the copied instructions.
>
> But, I'd like to have a baseline that is upstream to start working on these 
> parts.
>
> **All feedback is welcome !** :)


Ok, I see where you're going. I think we have a different interpretation of the 
word "prototype" . When I say "prototype code", I mean: the changes that I've 
made while experimenting, to try out whether a particular approach is feasible, 
and which I'm sharing with other people to get early feedback about the 
direction I am going in. These changes may include various shortcuts or hacks, 
which I've made to speed up the development of this prototype, but I understand 
that these will have to be removed before the final code is submitted (in fact, 
the final patch may often be a near-complete reimplementation of the prototype) 
. In this light, the phrase "prototype which I am trying to upstream" is a bit 
of an oxymoron since by the time it is upstreamed, the code should no longer be 
a prototype. It does not mean that the code should support every possible use 
case right from the start, but it should follow best coding, design and testing 
practices, regardless of any "experimental", "prototype" or other label.

With the eventual upstreaming goal in mind, I've tried to make a more thorough 
pass over this patch. The comments range from minute style issues like using 
`(void)` in function declarations, which can be trivially fixed, to layering 
concerns that may require more serious engineering work. Overall, my biggest 
concern is that there is no proper encapsulation in this patch. Despite being 
advertised as "fully extensible" this patch bakes in a lot of knowledge in very 
generic pieces of code. For instance, the `BreakpointInjectedSite` class is 
riddled with OS (`mach_vm_allocate`), architecture (`rbp`) and language 
(`Builtin.int_trap()`) specific knowledge -- it shouldn't need to know about 
none of those. I think this is the biggest issue that needs to be resolved 
before this patchset is ready for upstreaming. I've also made a number of other 
suggestions (here and in other threads) about possible alternative courses 
implementations. These are somewhat subjective and so I am not expecting you to 
implement all of them. However, I would:
a) expect some sort of a reply saying why you chose to do one thing or the other
b) strongly encourage you to try them out because I believe they will make 
fixing some of the other issues easier (for instance, switching to allocating 
the memory on the stack would mean that we can completely avoid the "how to 
allocate memory on different OSs" discussion, and just deal with architecture 
specifics, since subtracting from the stack pointer works the same way on every 
OS).

Also, this talk of a `v2` makes me worried that this code will become 
deprecated/unmaintained the moment it hits the repository. Lldb has some bad 
experience with fire-and-forget features, and the prospect of this makes me 
want to be even stricter about any bad patterns I notice in the code. IIUC, you 
are now working on a different take on this feature, which kind of means that 
you yourself are not sure about whether this is the best approach here. I 
understand the need for comparison, but why does one of these things need to be 
in the main repo for you to be able to compare them?




Comment at: lldb/include/lldb/Breakpoint/BreakpointInjectedSite.h:175
+  /// The source code needed to copy the variable in the argument 
structure.
+  std::string ParseDWARFExpression(size_t index, Status &error);
+

Return `Expected` instead of a string+error combo.



Comment at: lldb/include/lldb/Breakpoint/BreakpointSite.h:54-56
+   static bool classof(const BreakpointSite *bp_site) {
+ return bp_site->getKind() == eKindBreakpointSite;
+   }

mib wrote:
> labath wrote:
> > mib wrote:
> > > labath wrote:
> > > > This is weird. So, in OO terminology, `BreakpointInjectedSite` "is-a" 
> > > > `BreakpointSite`, but if you ask 
> > > > `llvm::isa(injected_site)`, it will return false?
> > > No, it returns **true**.
> > Then does `llvm::isa(injected_site)` return false? 
> > They can't both return true, given the curr

[Lldb-commits] [PATCH] D66655: [lldb] Fix x86 compilation

[Leonid Mashinskiy via Phabricator via lldb-commits]

leonid.mashinskiy updated this revision to Diff 217101.
leonid.mashinskiy added a comment.

Updated used guarding macros to cut-off ARM and ARM64 architectures as 
@tatyana-krasnukha mentioned


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66655

Files:
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h

Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN64)
+#if defined(__amd64__) || defined(_M_AMD64)
 #ifndef liblldb_NativeRegisterContextWindows_x86_64_h_
 #define liblldb_NativeRegisterContextWindows_x86_64_h_
 
@@ -79,4 +79,4 @@
 } // namespace lldb_private
 
 #endif // liblldb_NativeRegisterContextWindows_x86_64_h_
-#endif // defined(_WIN64)
+#endif // defined(__amd64__) || defined(_M_AMD64)
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN64)
+#if defined(__amd64__) || defined(_M_AMD64)
 
 #include "NativeRegisterContextWindows_x86_64.h"
 #include "NativeRegisterContextWindows_WoW64.h"
@@ -576,4 +576,4 @@
   return 0;
 }
 
-#endif // defined(_WIN64)
+#endif // defined(__amd64__) || defined(_M_AMD64)
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN64)
+#if defined(__i386__) || defined(_M_IX86)
 #ifndef liblldb_NativeRegisterContextWindows_i386_h_
 #define liblldb_NativeRegisterContextWindows_i386_h_
 
@@ -71,4 +71,4 @@
 } // namespace lldb_private
 
 #endif // liblldb_NativeRegisterContextWindows_i386_h_
-#endif // defined(_WIN64)
+#endif // defined(__i386__) || defined(_M_IX86)
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN32) && !defined(_WIN64)
+#if defined(__i386__) || defined(_M_IX86)
 
 #include "NativeRegisterContextWindows_i386.h"
 
@@ -242,7 +242,6 @@
 NativeRegisterContextWindows_i386::ReadRegister(const RegisterInfo *reg_info,
 RegisterValue ®_value) {
   Status error;
-  Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_REGISTERS);
 
   if (!reg_info) {
 error.SetErrorString("reg_info NULL");
@@ -267,7 +266,6 @@
 
 Status NativeRegisterContextWindows_i386::WriteRegister(
 const RegisterInfo *reg_info, const RegisterValue ®_value) {
-  Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_REGISTERS);
   Status error;
 
   if (!reg_info) {
@@ -286,12 +284,12 @@
   }
 
   if (IsGPR(reg))
-return GPRRead(reg, reg_value);
+return GPRWrite(reg, reg_value);
 
   return Status("unimplemented");
 }
 
-Status NativeRegisterContextWindows_x86_64::ReadAllRegisterValues(
+Status NativeRegisterContextWindows_i386::ReadAllRegisterValues(
 lldb::DataBufferSP &data_sp) {
   const size_t data_size = REG_CONTEXT_SIZE;
   data_sp = std::make_shared(data_size, 0);
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
@@ -6,7 +6,7 @@
 //
 //===-

[Lldb-commits] [PATCH] D66654: Prevent D66398 `TestDataFormatterStdList`-like regressions in the future

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jingham.
labath added a subscriber: jingham.
labath added a comment.

+ @jingham for value object stuff

Though I agree with the general principle of this change, the new api which 
implements it seems extremely confusing. Unfortunately, it's not really clear 
to me how to improve that. Maybe it would be better if the `Get` methods 
returned Expected? Then an "error" would mean ambiguous 
match, valid pointer would be "ok", and a null pointer would be "no match"? Or 
maybe both "no match" and "ambiguous match" should be errors? Or maybe, since 
these objects claim to be containers, they shouldn't even treat multiple 
matches as an error, but instead provide an interface to get *all* matches, and 
then this can be converted to an error higher up?




Comment at: lldb/include/lldb/Core/ValueObject.h:460
+  // Store error from Expected<> parameter unless some error is already stored.
+  template  void TakeError(llvm::Expected &&e) {
+if (!e && !GetError().Fail())

I didn't mention this in the previous patches, but I think you're overusing the 
rvalue references here. It's generally better to use regular value objects in 
cases like these because this enforces proper contracts between functions.

Using rvalue references leaves open the possibility for a bug (which you've 
actually made here) that the `TakeError` function will do nothing to the 
passed-in object (on some path of execution), which means that the object will 
continue to exist in the "untaken" state in the caller, and then probably 
assert some distance away from where it was meant to be used.

Passing the argument as a plain object means that this function *must* do 
something with it (really *take* it) or *it* will assert. The cost of that is a 
single object move, but that should generally be trivial for objects that 
implement moving properly.



Comment at: lldb/include/lldb/DataFormatters/FormatClasses.h:176
+  lldb::RegularExpressionSP m_regex1, m_regex2;
+  ConstString m_str;
+};

std::string, please. :) ConstString is severely overused in lldb..


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66654



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

[Lldb-commits] [lldb] r369892 - Breakpad: Add support for parsing STACK WIN records

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 04:25:28 2019
New Revision: 369892

URL: http://llvm.org/viewvc/llvm-project?rev=369892&view=rev
Log:
Breakpad: Add support for parsing STACK WIN records

Summary: The fields that aren't useful for us right now are simply ignored.

Reviewers: amccarth, markmentovai

Subscribers: rnk, lldb-commits

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

Modified:
lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp?rev=369892&r1=369891&r2=369892&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp Mon Aug 
26 04:25:28 2019
@@ -16,7 +16,19 @@ using namespace lldb_private;
 using namespace lldb_private::breakpad;
 
 namespace {
-enum class Token { Unknown, Module, Info, CodeID, File, Func, Public, Stack, 
CFI, Init };
+enum class Token {
+  Unknown,
+  Module,
+  Info,
+  CodeID,
+  File,
+  Func,
+  Public,
+  Stack,
+  CFI,
+  Init,
+  Win,
+};
 }
 
 template
@@ -33,6 +45,7 @@ template <> Token stringTo(llvm::
   .Case("STACK", Token::Stack)
   .Case("CFI", Token::CFI)
   .Case("INIT", Token::Init)
+  .Case("WIN", Token::Win)
   .Default(Token::Unknown);
 }
 
@@ -127,6 +140,8 @@ llvm::Optional Record::cla
 switch (Tok) {
 case Token::CFI:
   return Record::StackCFI;
+case Token::Win:
+  return Record::StackWin;
 default:
   return llvm::None;
 }
@@ -134,13 +149,13 @@ llvm::Optional Record::cla
   case Token::Unknown:
 // Optimistically assume that any unrecognised token means this is a line
 // record, those don't have a special keyword and start directly with a
-// hex number. CODE_ID should never be at the start of a line, but if it
-// is, it can be treated the same way as a garbled line record.
+// hex number.
 return Record::Line;
 
   case Token::CodeID:
   case Token::CFI:
   case Token::Init:
+  case Token::Win:
 // These should never appear at the start of a valid record.
 return llvm::None;
   }
@@ -390,6 +405,81 @@ llvm::raw_ostream &breakpad::operator<<(
   return OS << " " << R.UnwindRules;
 }
 
+llvm::Optional StackWinRecord::parse(llvm::StringRef Line) {
+  // STACK WIN type rva code_size prologue_size epilogue_size parameter_size
+  // saved_register_size local_size max_stack_size has_program_string
+  // program_string_OR_allocates_base_pointer
+
+  if (consume(Line) != Token::Stack)
+return llvm::None;
+  if (consume(Line) != Token::Win)
+return llvm::None;
+
+  llvm::StringRef Str;
+  uint8_t Type;
+  std::tie(Str, Line) = getToken(Line);
+  // Right now we only support the "FrameData" frame type.
+  if (!to_integer(Str, Type) || FrameType(Type) != FrameType::FrameData)
+return llvm::None;
+
+  lldb::addr_t RVA;
+  std::tie(Str, Line) = getToken(Line);
+  if (!to_integer(Str, RVA, 16))
+return llvm::None;
+
+  lldb::addr_t CodeSize;
+  std::tie(Str, Line) = getToken(Line);
+  if (!to_integer(Str, CodeSize, 16))
+return llvm::None;
+
+  // Skip fields which we aren't using right now.
+  std::tie(Str, Line) = getToken(Line); // prologue_size
+  std::tie(Str, Line) = getToken(Line); // epilogue_size
+
+  lldb::addr_t ParameterSize;
+  std::tie(Str, Line) = getToken(Line);
+  if (!to_integer(Str, ParameterSize, 16))
+return llvm::None;
+
+  lldb::addr_t SavedRegisterSize;
+  std::tie(Str, Line) = getToken(Line);
+  if (!to_integer(Str, SavedRegisterSize, 16))
+return llvm::None;
+
+  lldb::addr_t LocalSize;
+  std::tie(Str, Line) = getToken(Line);
+  if (!to_integer(Str, LocalSize, 16))
+return llvm::None;
+
+  std::tie(Str, Line) = getToken(Line); // max_stack_size
+
+  uint8_t HasProgramString;
+  std::tie(Str, Line) = getToken(Line);
+  if (!to_integer(Str, HasProgramString))
+return llvm::None;
+  // FrameData records should always have a program string.
+  if (!HasProgramString)
+return llvm::None;
+
+  return StackWinRecord(RVA, CodeSize, ParameterSize, SavedRegisterSize,
+LocalSize, Line.trim());
+}
+
+bool breakpad::operator==(const StackWinRecord &L, const StackWinRecord &R) {
+  return L.RVA == R.RVA && L.CodeSize == R.CodeSize &&
+ L.ParameterSize == R.ParameterSize &&
+ L.SavedRegisterSize == R.SavedRegisterSize &&
+ L.LocalSize == R.LocalSize && L.ProgramString == R.ProgramString;
+}
+
+llvm::raw_ostream &breakpad::operator<<(llvm::raw_ostream &OS,
+const StackWinRecord &R) {
+  return OS << llvm::formatv(
+ 

[Lldb-commits] [PATCH] D66633: Breakpad: Add support for parsing STACK WIN records

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369892: Breakpad: Add support for parsing STACK WIN records 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66633

Files:
  lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
  lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp

Index: lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
===
--- lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
+++ lldb/trunk/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
@@ -20,6 +20,7 @@
   EXPECT_EQ(Record::Func, Record::classify("FUNC"));
   EXPECT_EQ(Record::Public, Record::classify("PUBLIC"));
   EXPECT_EQ(Record::StackCFI, Record::classify("STACK CFI"));
+  EXPECT_EQ(Record::StackWin, Record::classify("STACK WIN"));
 
   // Any obviously incorrect lines will be classified as such.
   EXPECT_EQ(llvm::None, Record::classify("STACK"));
@@ -119,3 +120,34 @@
   EXPECT_EQ(llvm::None, StackCFIRecord::parse("FILE 47 foo"));
   EXPECT_EQ(llvm::None, StackCFIRecord::parse("42 47"));
 }
+
+TEST(StackWinRecord, parse) {
+  EXPECT_EQ(
+  StackWinRecord(0x47, 0x8, 3, 4, 5, llvm::StringRef("$eip $esp ^ =")),
+  StackWinRecord::parse("STACK WIN 4 47 8 1 2 3 4 5 6 1 $eip $esp ^ ="));
+
+  EXPECT_EQ(llvm::None, StackWinRecord::parse(
+"STACK WIN 0 47 8 1 0 0 0 0 0 1 $eip $esp ^ ="));
+  EXPECT_EQ(llvm::None,
+StackWinRecord::parse("STACK WIN 4 47 8 1 0 0 0 0 0 0 1"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse(
+"STACK WIN 3 47 8 1 0 0 0 0 0 1 $eip $esp ^ ="));
+  EXPECT_EQ(llvm::None,
+StackWinRecord::parse("STACK WIN 3 47 8 1 0 0 0 0 0 0 1"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse(
+"STACK WIN 4 47 8 1 0 0 0 0 1 $eip $esp ^ ="));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47 8 1 0 0 0 0 0"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47 8 1 0 0 0 0"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47 8 1 0 0 0"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47 8 1 0 0"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47 8 1 0"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47 8 1"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47 8"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4 47"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN 4"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK WIN"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse("STACK"));
+  EXPECT_EQ(llvm::None, StackWinRecord::parse(""));
+  EXPECT_EQ(llvm::None, StackCFIRecord::parse("FILE 47 foo"));
+  EXPECT_EQ(llvm::None, StackCFIRecord::parse("42 47"));
+}
Index: lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
@@ -16,7 +16,19 @@
 using namespace lldb_private::breakpad;
 
 namespace {
-enum class Token { Unknown, Module, Info, CodeID, File, Func, Public, Stack, CFI, Init };
+enum class Token {
+  Unknown,
+  Module,
+  Info,
+  CodeID,
+  File,
+  Func,
+  Public,
+  Stack,
+  CFI,
+  Init,
+  Win,
+};
 }
 
 template
@@ -33,6 +45,7 @@
   .Case("STACK", Token::Stack)
   .Case("CFI", Token::CFI)
   .Case("INIT", Token::Init)
+  .Case("WIN", Token::Win)
   .Default(Token::Unknown);
 }
 
@@ -127,6 +140,8 @@
 switch (Tok) {
 case Token::CFI:
   return Record::StackCFI;
+case Token::Win:
+  return Record::StackWin;
 default:
   return llvm::None;
 }
@@ -134,13 +149,13 @@
   case Token::Unknown:
 // Optimistically assume that any unrecognised token means this is a line
 // record, those don't have a special keyword and start directly with a
-// hex number. CODE_ID should never be at the start of a line, but if it
-// is, it can be treated the same way as a garbled line record.
+// hex number.
 return Record::Line;
 
   case Token::CodeID:
   case Token::CFI:
   case Token::Init:
+  case Token::Win:
 // These should never appear at the start of a valid record.
 return llvm::None;
   }
@@ -390,6 +405,81 @@
   return OS << " " << R.UnwindRules;
 }
 
+llvm::Optional StackWinRecord::parse(llvm::StringRef Line) {
+  // STACK WIN type rva code_size prologue_size epilogue_size parameter_size
+  // saved_register_size local_size max_stack_size has_program_string
+  // program_string_OR_allocates_b

[Lldb-commits] [lldb] r369894 - Postfix: move more code out of the PDB plugin

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 04:44:14 2019
New Revision: 369894

URL: http://llvm.org/viewvc/llvm-project?rev=369894&view=rev
Log:
Postfix: move more code out of the PDB plugin

Summary:
Previously we moved the code which parses a single expression out of the PDB
plugin, because that was useful for DWARF expressions in breakpad. However, FPO
programs are used in breakpad files too (when unwinding on windows), so this
completes the job, and moves the rest of the FPO parser too.

Reviewers: amccarth, aleksandr.urakov

Subscribers: aprantl, markmentovai, rnk, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Symbol/PostfixExpression.h
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
lldb/trunk/source/Symbol/PostfixExpression.cpp
lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp

Modified: lldb/trunk/include/lldb/Symbol/PostfixExpression.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/PostfixExpression.h?rev=369894&r1=369893&r2=369894&view=diff
==
--- lldb/trunk/include/lldb/Symbol/PostfixExpression.h (original)
+++ lldb/trunk/include/lldb/Symbol/PostfixExpression.h Mon Aug 26 04:44:14 2019
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include 
 
 namespace lldb_private {
 
@@ -211,7 +212,10 @@ inline T *MakeNode(llvm::BumpPtrAllocato
 
 /// Parse the given postfix expression. The parsed nodes are placed into the
 /// provided allocator.
-Node *Parse(llvm::StringRef expr, llvm::BumpPtrAllocator &alloc);
+Node *ParseOneExpression(llvm::StringRef expr, llvm::BumpPtrAllocator &alloc);
+
+std::vector>
+ParseFPOProgram(llvm::StringRef prog, llvm::BumpPtrAllocator &alloc);
 
 /// Serialize the given expression tree as DWARF. The result is written into 
the
 /// given stream. The AST should not contain any SymbolNodes. If the expression

Modified: lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp?rev=369894&r1=369893&r2=369894&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp Mon 
Aug 26 04:44:14 2019
@@ -430,7 +430,7 @@ bool SymbolFileBreakpad::ParseUnwindRow(
   while (auto rule = GetRule(unwind_rules)) {
 node_alloc.Reset();
 llvm::StringRef lhs = rule->first;
-postfix::Node *rhs = postfix::Parse(rule->second, node_alloc);
+postfix::Node *rhs = postfix::ParseOneExpression(rule->second, node_alloc);
 if (!rhs) {
   LLDB_LOG(log, "Could not parse `{0}` as unwind rhs.", rule->second);
   return false;

Modified: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp?rev=369894&r1=369893&r2=369894&view=diff
==
--- 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
 Mon Aug 26 04:44:14 2019
@@ -51,54 +51,23 @@ static uint32_t ResolveLLDBRegisterNum(l
   return npdb::GetLLDBRegisterNumber(arch_type, reg_id);
 }
 
-static bool ParseFPOSingleAssignmentProgram(llvm::StringRef program,
-llvm::BumpPtrAllocator &alloc,
-llvm::StringRef ®ister_name,
-Node *&ast) {
-  // lvalue of assignment is always first token
-  // rvalue program goes next
-  std::tie(register_name, program) = getToken(program);
-  if (register_name.empty())
-return false;
-
-  ast = Parse(program, alloc);
-  return ast != nullptr;
-}
-
-static Node *ParseFPOProgram(llvm::StringRef program,
+static Node *ResolveFPOProgram(llvm::StringRef program,
  llvm::StringRef register_name,
  llvm::Triple::ArchType arch_type,
  llvm::BumpPtrAllocator &alloc) {
-  llvm::DenseMap dependent_programs;
-
-  size_t cur = 0;
-  while (true) {
-size_t assign_index = program.find('=', cur);
-if (assign_index == llvm::StringRef::npos) {
-  llvm::StringRef tail = program.slice(cur, llvm::StringRef::npos);
-  if (!tail.trim().empty()) {
-// missing assign operator
-return nullptr;
-  }
-  break;
-}
-llvm::StringRef assignment_program = program.slice(cur, assi

[Lldb-commits] [PATCH] D66634: Postfix: move more code out of the PDB plugin

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rL369894: Postfix: move more code out of the PDB plugin 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66634

Files:
  lldb/trunk/include/lldb/Symbol/PostfixExpression.h
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
  lldb/trunk/source/Symbol/PostfixExpression.cpp
  lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp

Index: lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp
===
--- lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp
+++ lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
@@ -71,40 +72,68 @@
   }
 };
 
-static std::string ParseAndStringify(llvm::StringRef expr) {
+static std::string ParseOneAndStringify(llvm::StringRef expr) {
   llvm::BumpPtrAllocator alloc;
-  return ASTPrinter::Print(Parse(expr, alloc));
+  return ASTPrinter::Print(ParseOneExpression(expr, alloc));
 }
 
-TEST(PostfixExpression, Parse) {
-  EXPECT_EQ("int(47)", ParseAndStringify("47"));
-  EXPECT_EQ("$foo", ParseAndStringify("$foo"));
-  EXPECT_EQ("+(int(1), int(2))", ParseAndStringify("1 2 +"));
-  EXPECT_EQ("-(int(1), int(2))", ParseAndStringify("1 2 -"));
-  EXPECT_EQ("@(int(1), int(2))", ParseAndStringify("1 2 @"));
-  EXPECT_EQ("+(int(1), +(int(2), int(3)))", ParseAndStringify("1 2 3 + +"));
-  EXPECT_EQ("+(+(int(1), int(2)), int(3))", ParseAndStringify("1 2 + 3 +"));
-  EXPECT_EQ("^(int(1))", ParseAndStringify("1 ^"));
-  EXPECT_EQ("^(^(int(1)))", ParseAndStringify("1 ^ ^"));
-  EXPECT_EQ("^(+(int(1), ^(int(2", ParseAndStringify("1 2 ^ + ^"));
-  EXPECT_EQ("-($foo, int(47))", ParseAndStringify("$foo 47 -"));
-  EXPECT_EQ("+(int(47), int(-42))", ParseAndStringify("47 -42 +"));
-
-  EXPECT_EQ("nullptr", ParseAndStringify("+"));
-  EXPECT_EQ("nullptr", ParseAndStringify("^"));
-  EXPECT_EQ("nullptr", ParseAndStringify("1 +"));
-  EXPECT_EQ("nullptr", ParseAndStringify("1 2 ^"));
-  EXPECT_EQ("nullptr", ParseAndStringify("1 2 3 +"));
-  EXPECT_EQ("nullptr", ParseAndStringify("^ 1"));
-  EXPECT_EQ("nullptr", ParseAndStringify("+ 1 2"));
-  EXPECT_EQ("nullptr", ParseAndStringify("1 + 2"));
-  EXPECT_EQ("nullptr", ParseAndStringify("1 2"));
-  EXPECT_EQ("nullptr", ParseAndStringify(""));
+TEST(PostfixExpression, ParseOneExpression) {
+  EXPECT_EQ("int(47)", ParseOneAndStringify("47"));
+  EXPECT_EQ("$foo", ParseOneAndStringify("$foo"));
+  EXPECT_EQ("+(int(1), int(2))", ParseOneAndStringify("1 2 +"));
+  EXPECT_EQ("-(int(1), int(2))", ParseOneAndStringify("1 2 -"));
+  EXPECT_EQ("@(int(1), int(2))", ParseOneAndStringify("1 2 @"));
+  EXPECT_EQ("+(int(1), +(int(2), int(3)))", ParseOneAndStringify("1 2 3 + +"));
+  EXPECT_EQ("+(+(int(1), int(2)), int(3))", ParseOneAndStringify("1 2 + 3 +"));
+  EXPECT_EQ("^(int(1))", ParseOneAndStringify("1 ^"));
+  EXPECT_EQ("^(^(int(1)))", ParseOneAndStringify("1 ^ ^"));
+  EXPECT_EQ("^(+(int(1), ^(int(2", ParseOneAndStringify("1 2 ^ + ^"));
+  EXPECT_EQ("-($foo, int(47))", ParseOneAndStringify("$foo 47 -"));
+  EXPECT_EQ("+(int(47), int(-42))", ParseOneAndStringify("47 -42 +"));
+
+  EXPECT_EQ("nullptr", ParseOneAndStringify("+"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("^"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("1 +"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("1 2 ^"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("1 2 3 +"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("^ 1"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("+ 1 2"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("1 + 2"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify("1 2"));
+  EXPECT_EQ("nullptr", ParseOneAndStringify(""));
+}
+
+static std::vector>
+ParseFPOAndStringify(llvm::StringRef prog) {
+  llvm::BumpPtrAllocator alloc;
+  std::vector> parsed =
+  ParseFPOProgram(prog, alloc);
+  auto range = llvm::map_range(
+  parsed, [](const std::pair &pair) {
+return std::make_pair(pair.first, ASTPrinter::Print(pair.second));
+  });
+  return std::vector>(range.begin(),
+  range.end());
+}
+
+TEST(PostfixExpression, ParseFPOProgram) {
+  EXPECT_THAT(ParseFPOAndStringify("a 1 ="),
+  testing::ElementsAre(std::make_pair("a", "int(1)")));
+  EXPECT_THAT(ParseFPOAndStringify("a 1 = b 2 3 + ="),
+  testing::ElementsAre(std::make_pair("a", "int(1)"),
+   std::make_pair

[Lldb-commits] [lldb] r369905 - Fix windows build after r369894

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 05:42:32 2019
New Revision: 369905

URL: http://llvm.org/viewvc/llvm-project?rev=369905&view=rev
Log:
Fix windows build after r369894

Constructing a std::vector from a llvm::map_range fails on windows,
apparently because std::vector expects the input iterator to have a
const operator* (map_range iterator has a non-const one).

This avoids the cleverness and unrolls the map-loop manually (which is
also slightly shorter).

Modified:
lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp

Modified: lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp?rev=369905&r1=369904&r2=369905&view=diff
==
--- lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp (original)
+++ lldb/trunk/unittests/Symbol/PostfixExpressionTest.cpp Mon Aug 26 05:42:32 
2019
@@ -108,12 +108,10 @@ ParseFPOAndStringify(llvm::StringRef pro
   llvm::BumpPtrAllocator alloc;
   std::vector> parsed =
   ParseFPOProgram(prog, alloc);
-  auto range = llvm::map_range(
-  parsed, [](const std::pair &pair) {
-return std::make_pair(pair.first, ASTPrinter::Print(pair.second));
-  });
-  return std::vector>(range.begin(),
-  range.end());
+  std::vector> result;
+  for (const auto &p : parsed)
+result.emplace_back(p.first, ASTPrinter::Print(p.second));
+  return result;
 }
 
 TEST(PostfixExpression, ParseFPOProgram) {


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

[Lldb-commits] [lldb] r369904 - Fix a type mismatch error in GDBRemoteCommunicationServerCommon

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 05:42:28 2019
New Revision: 369904

URL: http://llvm.org/viewvc/llvm-project?rev=369904&view=rev
Log:
Fix a type mismatch error in GDBRemoteCommunicationServerCommon

GetU64 returns a uint64_t. Don't store it in size_t as that is only
32-bit on 32-bit platforms.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=369904&r1=369903&r2=369904&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
 Mon Aug 26 05:42:28 2019
@@ -550,7 +550,7 @@ GDBRemoteCommunicationServerCommon::Hand
   packet.SetFilePos(::strlen("vFile:pread:"));
   int fd = packet.GetS32(-1);
   if (packet.GetChar() == ',') {
-size_t count = packet.GetU64(UINT64_MAX);
+uint64_t count = packet.GetU64(UINT64_MAX);
 if (packet.GetChar() == ',') {
   off_t offset = packet.GetU64(UINT32_MAX);
   if (count == UINT64_MAX) {


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

[Lldb-commits] [lldb] r369906 - ProcessInstanceInfo: Fix dumping of invalid user ids

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 06:03:21 2019
New Revision: 369906

URL: http://llvm.org/viewvc/llvm-project?rev=369906&view=rev
Log:
ProcessInstanceInfo: Fix dumping of invalid user ids

Don't attempt to print invalid user ids. Previously, these would come
out as UINT32_MAX, or as an assertion failure.

Modified:
lldb/trunk/source/Utility/ProcessInfo.cpp
lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp

Modified: lldb/trunk/source/Utility/ProcessInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ProcessInfo.cpp?rev=369906&r1=369905&r2=369906&view=diff
==
--- lldb/trunk/source/Utility/ProcessInfo.cpp (original)
+++ lldb/trunk/source/Utility/ProcessInfo.cpp Mon Aug 26 06:03:21 2019
@@ -189,24 +189,39 @@ void ProcessInstanceInfo::DumpAsTableRow
 if (m_arch.IsValid())
   m_arch.DumpTriple(arch_strm);
 
-auto print = [&](UserIDResolver::id_t id,
- llvm::Optional (UserIDResolver::*get)(
+auto print = [&](bool (ProcessInstanceInfo::*isValid)() const,
+ uint32_t (ProcessInstanceInfo::*getID)() const,
+ llvm::Optional 
(UserIDResolver::*getName)(
  UserIDResolver::id_t id)) {
-  if (auto name = (resolver.*get)(id))
-s.Format("{0,-10} ", *name);
+  const char *format = "{0,-10} ";
+  if (!(this->*isValid)()) {
+s.Format(format, "");
+return;
+  }
+  uint32_t id = (this->*getID)();
+  if (auto name = (resolver.*getName)(id))
+s.Format(format, *name);
   else
-s.Format("{0,-10} ", id);
+s.Format(format, id);
 };
 if (verbose) {
-  print(m_uid, &UserIDResolver::GetUserName);
-  print(m_gid, &UserIDResolver::GetGroupName);
-  print(m_euid, &UserIDResolver::GetUserName);
-  print(m_egid, &UserIDResolver::GetGroupName);
+  print(&ProcessInstanceInfo::UserIDIsValid,
+&ProcessInstanceInfo::GetUserID, &UserIDResolver::GetUserName);
+  print(&ProcessInstanceInfo::GroupIDIsValid,
+&ProcessInstanceInfo::GetGroupID, &UserIDResolver::GetGroupName);
+  print(&ProcessInstanceInfo::EffectiveUserIDIsValid,
+&ProcessInstanceInfo::GetEffectiveUserID,
+&UserIDResolver::GetUserName);
+  print(&ProcessInstanceInfo::EffectiveGroupIDIsValid,
+&ProcessInstanceInfo::GetEffectiveGroupID,
+&UserIDResolver::GetGroupName);
 
   s.Printf("%-24s ", arch_strm.GetData());
 } else {
-  print(m_euid, &UserIDResolver::GetUserName);
-  s.Printf(" %-24s ", arch_strm.GetData());
+  print(&ProcessInstanceInfo::EffectiveUserIDIsValid,
+&ProcessInstanceInfo::GetEffectiveUserID,
+&UserIDResolver::GetUserName);
+  s.Printf("%-24s ", arch_strm.GetData());
 }
 
 if (verbose || show_args) {

Modified: lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp?rev=369906&r1=369905&r2=369906&view=diff
==
--- lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp (original)
+++ lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp Mon Aug 26 06:03:21 
2019
@@ -73,3 +73,21 @@ TEST(ProcessInstanceInfo, DumpTable) {
 )",
   s.GetData());
 }
+
+TEST(ProcessInstanceInfo, DumpTable_invalidUID) {
+  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
+
+  DummyUserIDResolver resolver;
+  StreamString s;
+
+  const bool show_args = false;
+  const bool verbose = false;
+  ProcessInstanceInfo::DumpTableHeader(s, show_args, verbose);
+  info.DumpAsTableRow(s, resolver, show_args, verbose);
+  EXPECT_STREQ(
+  R"(PIDPARENT USER   TRIPLE   NAME
+== == ==  
+47 0 x86_64-pc-linux  a.out
+)",
+  s.GetData());
+}


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

[Lldb-commits] [lldb] r369907 - Move ProcessInstanceInfoTest to Utility

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 06:07:02 2019
New Revision: 369907

URL: http://llvm.org/viewvc/llvm-project?rev=369907&view=rev
Log:
Move ProcessInstanceInfoTest to Utility

The class under test was moved in r355342. This moves the test code too.

Added:
lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
  - copied, changed from r369906, 
lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp
Removed:
lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp
Modified:
lldb/trunk/unittests/Target/CMakeLists.txt
lldb/trunk/unittests/Utility/CMakeLists.txt

Modified: lldb/trunk/unittests/Target/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Target/CMakeLists.txt?rev=369907&r1=369906&r2=369907&view=diff
==
--- lldb/trunk/unittests/Target/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Target/CMakeLists.txt Mon Aug 26 06:07:02 2019
@@ -3,7 +3,6 @@ add_lldb_unittest(TargetTests
   MemoryRegionInfoTest.cpp
   ModuleCacheTest.cpp
   PathMappingListTest.cpp
-  ProcessInstanceInfoTest.cpp
 
   LINK_LIBS
   lldbCore

Removed: lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp?rev=369906&view=auto
==
--- lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp (original)
+++ lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp (removed)
@@ -1,93 +0,0 @@
-//===-- ProcessInstanceInfoTest.cpp -*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Target/Process.h"
-#include "gtest/gtest.h"
-
-using namespace lldb_private;
-
-namespace {
-/// A very simple resolver which fails for even ids and returns a simple string
-/// for odd ones.
-class DummyUserIDResolver : public UserIDResolver {
-protected:
-  llvm::Optional DoGetUserName(id_t uid) override {
-if (uid % 2)
-  return ("user" + llvm::Twine(uid)).str();
-return llvm::None;
-  }
-
-  llvm::Optional DoGetGroupName(id_t gid) override {
-if (gid % 2)
-  return ("group" + llvm::Twine(gid)).str();
-return llvm::None;
-  }
-};
-} // namespace
-
-TEST(ProcessInstanceInfo, Dump) {
-  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
-  info.SetUserID(1);
-  info.SetEffectiveUserID(2);
-  info.SetGroupID(3);
-  info.SetEffectiveGroupID(4);
-
-  DummyUserIDResolver resolver;
-  StreamString s;
-  info.Dump(s, resolver);
-  EXPECT_STREQ(R"(pid = 47
-   name = a.out
-   file = a.out
-   arch = x86_64-pc-linux
-uid = 1 (user1)
-gid = 3 (group3)
-   euid = 2 ()
-   egid = 4 ()
-)",
-   s.GetData());
-}
-
-TEST(ProcessInstanceInfo, DumpTable) {
-  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
-  info.SetUserID(1);
-  info.SetEffectiveUserID(2);
-  info.SetGroupID(3);
-  info.SetEffectiveGroupID(4);
-
-  DummyUserIDResolver resolver;
-  StreamString s;
-
-  const bool show_args = false;
-  const bool verbose = true;
-  ProcessInstanceInfo::DumpTableHeader(s, show_args, verbose);
-  info.DumpAsTableRow(s, resolver, show_args, verbose);
-  EXPECT_STREQ(
-  R"(PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE  
 ARGUMENTS
-== == == == == == 
 
-47 0  user1  group3 2  4  x86_64-pc-linux  

-)",
-  s.GetData());
-}
-
-TEST(ProcessInstanceInfo, DumpTable_invalidUID) {
-  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
-
-  DummyUserIDResolver resolver;
-  StreamString s;
-
-  const bool show_args = false;
-  const bool verbose = false;
-  ProcessInstanceInfo::DumpTableHeader(s, show_args, verbose);
-  info.DumpAsTableRow(s, resolver, show_args, verbose);
-  EXPECT_STREQ(
-  R"(PIDPARENT USER   TRIPLE   NAME
-== == ==  
-47 0 x86_64-pc-linux  a.out
-)",
-  s.GetData());
-}

Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=369907&r1=369906&r2=369907&view=diff
==
--- lldb/trunk/unittests/Utility/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Utility/CMakeLists.txt Mon Aug 26 06:07:02 2019
@@ -18,6 +18,7 @@ add_lldb_unittest(UtilityTests
   NameMatchesTest.cpp
   PredicateTest.cpp
   ProcessIn

[Lldb-commits] [PATCH] D66655: [lldb] Fix x86 compilation

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

Macros `__x86_64__` and `_M_X64` are more common than AMD-branded, though there 
is no functional difference (unless using old versions of the Intel compiler).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66655



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

[Lldb-commits] [PATCH] D66737: [lldb][NFC] Remove dead code that handles situations where LLDB has no dummy target

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: labath.
Herald added subscribers: lldb-commits, abidh.
Herald added a project: LLDB.

We always have a dummy target, so any error handling regarding a missing dummy 
target is dead code now.
Also makes the CommandObject methods that return Target& to express this fact 
in the API.

This patch just for the CommandObject part of LLDB. I'll migrate the rest of 
LLDB in a follow-up patch that's WIP.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D66737

Files:
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectStats.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -787,15 +787,10 @@
 
 // Now check if we have a running process.  If so, we should instruct the
 // process monitor to enable/disable DarwinLog support now.
-Target *target = GetSelectedOrDummyTarget();
-if (!target) {
-  // No target, so there is nothing more to do right now.
-  result.SetStatus(eReturnStatusSuccessFinishNoResult);
-  return true;
-}
+Target &target = GetSelectedOrDummyTarget();
 
 // Grab the active process.
-auto process_sp = target->GetProcessSP();
+auto process_sp = target.GetProcessSP();
 if (!process_sp) {
   // No active process, so there is nothing more to do right now.
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
@@ -877,9 +872,9 @@
 
 // Figure out if we've got a process.  If so, we can tell if DarwinLog is
 // available for that process.
-Target *target = GetSelectedOrDummyTarget();
-auto process_sp = target ? target->GetProcessSP() : ProcessSP();
-if (!target || !process_sp) {
+Target &target = GetSelectedOrDummyTarget();
+auto process_sp = target.GetProcessSP();
+if (!process_sp) {
   stream.PutCString("Availability: unknown (requires process)\n");
   stream.PutCString("Enabled: not applicable "
 "(requires process)\n");
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -917,12 +917,12 @@
   return g_arguments_data[arg_type].help_text;
 }
 
-Target *CommandObject::GetDummyTarget() {
-  return m_interpreter.GetDebugger().GetDummyTarget();
+Target &CommandObject::GetDummyTarget() {
+  return *m_interpreter.GetDebugger().GetDummyTarget();
 }
 
-Target *CommandObject::GetSelectedOrDummyTarget(bool prefer_dummy) {
-  return m_interpreter.GetDebugger().GetSelectedOrDummyTarget(prefer_dummy);
+Target &CommandObject::GetSelectedOrDummyTarget(bool prefer_dummy) {
+  return *m_interpreter.GetDebugger().GetSelectedOrDummyTarget(prefer_dummy);
 }
 
 Thread *CommandObject::GetDefaultThread() {
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -4672,51 +4672,49 @@
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 m_stop_hook_sp.reset();
 
-Target *target = GetSelectedOrDummyTarget();
-if (target) {
-  Target::StopHookSP new_hook_sp = target->CreateStopHook();
-
-  //  First step, make the specifier.
-  std::unique_ptr specifier_up;
-  if (m_options.m_sym_ctx_specified) {
-specifier_up.reset(
-new SymbolContextSpecifier(GetDebugger().GetSelectedTarget()));
-
-if (!m_options.m_module_name.empty()) {
-  specifier_up->AddSpecification(
-  m_options.m_module_name.c_str(),
-  SymbolContextSpecifier::eModuleSpecified);
-}
+Target &target = GetSelectedOrDummyTarget();
+Target::StopHookSP new_hook_sp = target.CreateStopHook();
+
+//  First step, make the specifier.
+std::unique_ptr specifier_up;
+if (m_options.m_sym_ctx_specified) {
+  specifier_up.reset(
+  new SymbolContextSpecifier(GetDebugger().GetSelectedTarget()));
+
+  if (!m_options.m_module_name.empty()) {
+specifier_up->AddSpecification(
+m_options.m_module_name.c_str(),
+SymbolContextSpecifier::eModuleSpecified);
+  }
 
-if (!m_options.m_class_name.empty()) {
-  specifier_up->AddSpecification(
-

[Lldb-commits] [PATCH] D65646: [lldb] Print better diagnostics for user expressions.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a reviewer: LLDB.
teemperor added a comment.

ping :)


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

https://reviews.llvm.org/D65646



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

[Lldb-commits] [PATCH] D66737: [lldb][NFC] Remove dead code that handles situations where LLDB has no dummy target

[Pavel Labath via Phabricator via lldb-commits]

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

This is awesome. Thanks for doing this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66737



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

[Lldb-commits] [lldb] r369908 - [ProcessWindows] Remove equivalent macros

[Tatyana Krasnukha via lldb-commits]

Author: tkrasnukha
Date: Mon Aug 26 06:35:59 2019
New Revision: 369908

URL: http://llvm.org/viewvc/llvm-project?rev=369908&view=rev
Log:
[ProcessWindows] Remove equivalent macros

Modified:

lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp

lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h

Modified: 
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp?rev=369908&r1=369907&r2=369908&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
 Mon Aug 26 06:35:59 2019
@@ -6,7 +6,7 @@
 //
 
//===--===//
 
-#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_X64)
 
 #include "lldb/Host/windows/HostThreadWindows.h"
 #include "lldb/Host/windows/windows.h"
@@ -537,4 +537,4 @@ bool RegisterContextWindows_x64::WriteRe
   wthread.GetHostThread().GetNativeThread().GetSystemHandle(), &m_context);
 }
 
-#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)
+#endif // defined(__x86_64__) || defined(_M_X64)

Modified: 
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h?rev=369908&r1=369907&r2=369908&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
 (original)
+++ 
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
 Mon Aug 26 06:35:59 2019
@@ -9,7 +9,7 @@
 #ifndef liblldb_RegisterContextWindows_x64_H_
 #define liblldb_RegisterContextWindows_x64_H_
 
-#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_X64)
 
 #include "RegisterContextWindows.h"
 #include "lldb/lldb-forward.h"
@@ -42,6 +42,6 @@ public:
 };
 }
 
-#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)
+#endif // defined(__x86_64__) || defined(_M_X64)
 
 #endif // #ifndef liblldb_RegisterContextWindows_x64_H_


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

[Lldb-commits] [PATCH] D66739: [lldb] Add -w flag for expressions to print compiler warnings even if expression succeeds.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added subscribers: lldb-commits, abidh.
Herald added a project: LLDB.
teemperor edited the summary of this revision.

Currently if you evaluate for example "expression int" the user just sees 
nothing and has no way of knowing what
exactly could have gone wrong. This patch adds a `-w` flag to the expression 
command that causes compiler warnings
to be emitted even if the command succeeds. This was the agreed workaround to 
solve rdar://12788008.

The checkerror flag that was added to lldbtest was necessary as we usually 
don't check the error output of commands
when they succeed (but we have to do now).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D66739

Files:
  lldb/include/lldb/Expression/UserExpression.h
  lldb/include/lldb/Target/Target.h
  lldb/packages/Python/lldbsuite/test/expression_command/warnings/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/warnings/TestExprWarnings.py
  lldb/packages/Python/lldbsuite/test/expression_command/warnings/main.cpp
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2335,7 +2335,7 @@
 llvm::StringRef expr, ExecutionContextScope *exe_scope,
 lldb::ValueObjectSP &result_valobj_sp,
 const EvaluateExpressionOptions &options, std::string *fixed_expression,
-ValueObject *ctx_obj) {
+ValueObject *ctx_obj, std::string *warnings) {
   result_valobj_sp.reset();
 
   ExpressionResults execution_results = eExpressionSetupError;
@@ -2385,7 +2385,7 @@
 UserExpression::Evaluate(exe_ctx, options, expr, prefix,
  result_valobj_sp, error, fixed_expression,
  nullptr, // Module
- ctx_obj);
+ ctx_obj, warnings);
   }
 
   return execution_results;
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -143,7 +143,7 @@
 llvm::StringRef expr, llvm::StringRef prefix,
 lldb::ValueObjectSP &result_valobj_sp, Status &error,
 std::string *fixed_expression, lldb::ModuleSP *jit_module_sp_ptr,
-ValueObject *ctx_obj) {
+ValueObject *ctx_obj, std::string *warnings) {
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS |
   LIBLLDB_LOG_STEP));
 
@@ -273,6 +273,7 @@
   parse_success = fixed_expression_sp->Parse(
   fixed_diagnostic_manager, exe_ctx, execution_policy,
   keep_expression_in_memory, generate_debug_info);
+
   if (parse_success) {
 diagnostic_manager.Clear();
 user_expression_sp = fixed_expression_sp;
@@ -301,6 +302,9 @@
   }
 
   if (parse_success) {
+if (warnings)
+  *warnings = diagnostic_manager.GetString();
+
 // If a pointer to a lldb::ModuleSP was passed in, return the JIT'ed module
 // if one was created
 if (jit_module_sp_ptr)
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -353,6 +353,9 @@
 Desc<"When specified, debug the JIT code by setting a breakpoint on the "
 "first instruction and forcing breakpoints to not be ignored (-i0) and no "
 "unwinding to happen on error (-u0).">;
+  def expression_options_warning : Option<"warning", "w">, Groups<[1,2]>,
+Desc<"Enables compiler warnings when parsing the expression, even when the "
+"expression was successfully parsed.">;
   def expression_options_language : Option<"language", "l">, Groups<[1,2]>,
 Arg<"Language">, Desc<"Specifies the Language to use when parsing the "
 "expression.  If not set the target.language setting is used.">;
Index: lldb/source/Commands/CommandObjectExpression.h
===
--- lldb/source/Commands/CommandObjectExpression.h
+++ lldb/source/Commands/CommandObjectExpression.h
@@ -41,6 +41,7 @@
 bool show_types;
 bool show_summary;
 bool debug;
+bool warning;
 uint32_t timeout;
 bool try_all_threads;
 lldb::LanguageType language;
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -143,6 +143,10 @@
   option_arg.str().c_str());
 break;
 
+  c

[Lldb-commits] [PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via lldb-commits]

martong updated this revision to Diff 217135.
martong added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Use resulting Name from HandleNameConflict if set
- Add ODRHandling strategies
- Refactor tests, to avoid some code repetition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/unittests/AST/ASTImporterFixtures.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/include/lldb/Symbol/ClangASTImporter.h

Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -252,7 +252,9 @@
 : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
  master.m_file_manager, true /*minimal*/),
   m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
-  m_master(master), m_source_ctx(source_ctx) {}
+  m_master(master), m_source_ctx(source_ctx) {
+setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+  }
 
 /// Scope guard that attaches a CxxModuleHandler to an ASTImporterDelegate
 /// and deattaches it at the end of the scope. Supports being used multiple
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -1879,7 +1879,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2432,6 +2432,62 @@
   EXPECT_EQ(ToD1, ToD2);
 }
 
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5127,15 +5183,6 @@
   }
 }
 
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest,
-DefaultTestValuesForRunOptions, );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
-::testing::Values(ArgVector()), );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
-::testing::Values(ArgVector()), );
-
 TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
   Decl *FromTU = getTuDecl(
   R"(
@@ -5233,8 +5280,8 @@
   // prototype (inside 'friend') for it comes first in the AST and is not
   // linked to the definition.
   EXPECT_EQ(ImportedDef, ToClassDef);
-}  
-  
+}
+
 struct LLDBLookupTest : ASTImporterOptionSpecificTestBase {
   LLDBLookupTest() {
 Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
@@ -5362,10 +5409,388 @@
   EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
 }
 
+// FIXME Put ODR handling strategy related tests into their own test file. And
+// create type parameterized tests for them like we do in
+// ASTImporterGenericRedeclTest.cpp
+struct ConflictingDeclsWithConservativeStrategy
+: ASTImporterOptionSpecificTestBase {};
+
+struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase {
+  ConflictingDeclsWithLiberalStrateg

[Lldb-commits] [PATCH] D66739: [lldb] Add -w flag for expressions to print compiler warnings even if expression succeeds.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

You could avoid the business with adding a flag to `self.expect` (which has 
enough flags already) by just making this a lit test. :)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66739



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

[Lldb-commits] [PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

@shafik I have updated the patch with ODR handling strategies as per our 
discusson.

For the record I copy our discussion here:

On Sat, Aug 24, 2019 at 12:50 AM Shafik Yaghmour  
wrote:

> Apologies, I missed this earlier!
> 
> On Wed, Aug 21, 2019 at 2:44 AM Gábor Márton  wrote:
>  >
>  > Hi Shafik,
>  >
>  > > Right now you will end up w/ an arbitrary one of them but we do want to 
> support
>  > a way to choose between them eventually.
>  >
>  > Actually, right now (if the patch is not merged) we end up having both of 
> them in the AST. Some parts of the AST reference the existing definition, 
> while some other parts reference the new definition. Also, the regular lookup 
> will find both definitions.
>  >
>  > If the patch is merged then only the first (the existing) definition is 
> kept and an error is reported.
>  >
>  > > AFAICT this would prevent such a solution. At least that is how the
>  > new test case for RecordDecl make it appear
>  >
>  > Yes, you will never be able to remove an existing node from the AST, so I 
> don't think an either-or choosing mechanism is feasible. But you may be able 
> to decide whether you want to add the new and conflicting node. And you may 
> be able to use a different name for the new conflicting node. This may help 
> clients to see which node they are using.
>  > I want to create an additional patch which builds on this patch. Here is 
> the essence of what I'd like to have:
>  >   if (!ConflictingDecls.empty()) {
>  > Expected Resolution = Importer.HandleNameConflict(
>  > Name, DC, Decl::IDNS_Member, ConflictingDecls.data(),
>  > ConflictingDecls.size());
>  > if (Resolution)
>  >   Name = Resolution.get();
>  > else
>  >   return Resolution.takeError();
>  >   }
>  > Consider the "else" branch. I'd like to have such an "else" branch 
> everywhere. The point is to use the result of HandleNameConflict (if it is 
> set to something). This way it is possible to create any kind of ODR handling 
> by overwriting HandleNameConflict in the descendant classes.
>  >
>  > We identified 3 possible strategies so far:
>  >
>  > Conservative. In case of name conflict propagate the error. This should be 
> the default behavior.
>  > Liberal. In case of name conflict create a new node with the same name and 
> do not propagate the error.
>  > Rename. In case of name conflict create a new node with a different name 
> (e.g. with a prefix of the TU's name). Do not propagate the error.
>  >
>  >
>  > If we add a new conflicting node beside the exiting one, then some clients 
> of the AST which rely on lookup will be confused. The CTU client does not 
> rely on lookup so that is not really a problem there, but I don't know what 
> this could cause with LLDB. Perhaps the renaming strategy could work there 
> too.
>  > The Conservative and Liberal strategies are very easy to implement, and I 
> am going to create patches for that if we have consensus.
> 
> We know currently we do have cases where we have ODR violations w/
>  RecordDecl due to use of an opaque struct in the API headers and a
>  concrete instance internally e.g.:
> 
> //opaque
>  struct A {
> 
>   char buf[16];
> 
> };
> 
> //concrete
>  struct A {
> 
>   double d;
>   int64_t x;
> 
> };
> 
> and we don't want this to become an error.
> 
> I think we would at least one the choice of Conservative or Liberal to
>  be configurable and maybe LLDB would default to Liberal. This would
>  enable us to keep the status quo and not break existing cases any
>  worse than they already are.
> 
> I would prefer that would be done in this PR since I don't want to be
>  in a situation where we branch internally and we have this change but
>  not the follow-up change.
> 
>> >  I don't see how like you comment says this does not effect CXXRecordDecl
>  >
>  > In my comment I do not say that CXXRecordDecls are exceptions from the 
> general way of handling ODR violations.
>  > The comment is about that we shall not report ODR errors if we are not 
> 100% sure that we face one.
>  > So we should report an ODR error only if the found Decl and the newly 
> imported Decl have the same kind.
>  > I.e. both are CXXRecordDecls.
>  > For example, let's assume we import a CXXRecordDecl and we find an 
> existing ClassTemplateDecl with the very same Name.
>  > Then we should not report an ODR violation.
> 
> Thank you for the clarification, I misunderstood the comment, now it
>  makes more sense.
> 
>> Thanks,
>  > Gabor
>  >
>  >
>  > On Mon, Aug 19, 2019 at 5:46 PM Shafik Yaghmour 
>  wrote:
>  >>
>  >> Have a nice vacation :-)
>  >>
>  >> On Mon, Aug 19, 2019 at 8:40 AM Gábor Márton  
> wrote:
>  >> >
>  >> > Hi Shafik,
>  >> >
>  >> > I'll have an answer for you on Wednesday, I'm on vacation until then.
>  >> >
>  >> > Thanks,
>  >> > Gábor
>  >> >
>  >> > On Sat, 17 Aug 2019, 04:30 Shafik Yaghmour,  
> wrote:
>  >> >>
>  >> >> Hello Gábor,
>  >> >>
>  >> >> I was looking at

[Lldb-commits] [lldb] r369910 - Really fix the type mismatch error in GDBRemoteCommunicationServerCommon

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 06:56:33 2019
New Revision: 369910

URL: http://llvm.org/viewvc/llvm-project?rev=369910&view=rev
Log:
Really fix the type mismatch error in GDBRemoteCommunicationServerCommon

My previous attempt in attempt in r369904 actually broke the 32bit build
because File::Read expects to take a reference to size_t. Fix the
warning by using SIZE_MAX to denote failure instead.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=369910&r1=369909&r2=369910&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
 Mon Aug 26 06:56:33 2019
@@ -550,10 +550,10 @@ GDBRemoteCommunicationServerCommon::Hand
   packet.SetFilePos(::strlen("vFile:pread:"));
   int fd = packet.GetS32(-1);
   if (packet.GetChar() == ',') {
-uint64_t count = packet.GetU64(UINT64_MAX);
+size_t count = packet.GetU64(SIZE_MAX);
 if (packet.GetChar() == ',') {
   off_t offset = packet.GetU64(UINT32_MAX);
-  if (count == UINT64_MAX) {
+  if (count == SIZE_MAX) {
 response.Printf("F-1:%i", EINVAL);
 return SendPacketNoLock(response.GetString());
   }


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

[Lldb-commits] [PATCH] D66655: [lldb] Fix x86 compilation

[Leonid Mashinskiy via Phabricator via lldb-commits]

leonid.mashinskiy updated this revision to Diff 217140.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66655

Files:
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
  source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h

Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN64)
+#if defined(__x86_64__) || defined(_M_X64)
 #ifndef liblldb_NativeRegisterContextWindows_x86_64_h_
 #define liblldb_NativeRegisterContextWindows_x86_64_h_
 
@@ -79,4 +79,4 @@
 } // namespace lldb_private
 
 #endif // liblldb_NativeRegisterContextWindows_x86_64_h_
-#endif // defined(_WIN64)
+#endif // defined(__x86_64__) || defined(_M_X64)
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN64)
+#if defined(__x86_64__) || defined(_M_X64)
 
 #include "NativeRegisterContextWindows_x86_64.h"
 #include "NativeRegisterContextWindows_WoW64.h"
@@ -576,4 +576,4 @@
   return 0;
 }
 
-#endif // defined(_WIN64)
+#endif // defined(__x86_64__) || defined(_M_X64)
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN64)
+#if defined(__i386__) || defined(_M_IX86)
 #ifndef liblldb_NativeRegisterContextWindows_i386_h_
 #define liblldb_NativeRegisterContextWindows_i386_h_
 
@@ -71,4 +71,4 @@
 } // namespace lldb_private
 
 #endif // liblldb_NativeRegisterContextWindows_i386_h_
-#endif // defined(_WIN64)
+#endif // defined(__i386__) || defined(_M_IX86)
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN32) && !defined(_WIN64)
+#if defined(__i386__) || defined(_M_IX86)
 
 #include "NativeRegisterContextWindows_i386.h"
 
@@ -242,7 +242,6 @@
 NativeRegisterContextWindows_i386::ReadRegister(const RegisterInfo *reg_info,
 RegisterValue ®_value) {
   Status error;
-  Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_REGISTERS);
 
   if (!reg_info) {
 error.SetErrorString("reg_info NULL");
@@ -267,7 +266,6 @@
 
 Status NativeRegisterContextWindows_i386::WriteRegister(
 const RegisterInfo *reg_info, const RegisterValue ®_value) {
-  Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_REGISTERS);
   Status error;
 
   if (!reg_info) {
@@ -286,12 +284,12 @@
   }
 
   if (IsGPR(reg))
-return GPRRead(reg, reg_value);
+return GPRWrite(reg, reg_value);
 
   return Status("unimplemented");
 }
 
-Status NativeRegisterContextWindows_x86_64::ReadAllRegisterValues(
+Status NativeRegisterContextWindows_i386::ReadAllRegisterValues(
 lldb::DataBufferSP &data_sp) {
   const size_t data_size = REG_CONTEXT_SIZE;
   data_sp = std::make_shared(data_size, 0);
Index: source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
===
--- source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
+++ source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#if defined(_WIN64)
+#if defined(__x86_64__) || defined(_M_X64)
 #ifndef liblldb_NativeRegisterContextWindows_WoW64_h_
 #define 

[Lldb-commits] [PATCH] D66742: Obliterate LLDB_CONFIGURATION_BUILDANDINTEGRATION

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jingham, sgraenitz.
Herald added a subscriber: emaste.

With the XCode project gone, there doesn't seem to be anything setting
this macro anymore -- and the macro wasn't doing much anyway.


https://reviews.llvm.org/D66742

Files:
  source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
  source/Plugins/Process/POSIX/CrashReason.cpp
  source/Plugins/Process/POSIX/ProcessMessage.cpp


Index: source/Plugins/Process/POSIX/ProcessMessage.cpp
===
--- source/Plugins/Process/POSIX/ProcessMessage.cpp
+++ source/Plugins/Process/POSIX/ProcessMessage.cpp
@@ -15,11 +15,6 @@
 }
 
 const char *ProcessMessage::PrintKind(Kind kind) {
-#ifdef LLDB_CONFIGURATION_BUILDANDINTEGRATION
-  // Just return the code in ascii for integration builds.
-  chcar str[8];
-  sprintf(str, "%d", reason);
-#else
   const char *str = nullptr;
 
   switch (kind) {
@@ -60,8 +55,6 @@
 str = "eExecMessage";
 break;
   }
-#endif
-
   return str;
 }
 
Index: source/Plugins/Process/POSIX/CrashReason.cpp
===
--- source/Plugins/Process/POSIX/CrashReason.cpp
+++ source/Plugins/Process/POSIX/CrashReason.cpp
@@ -229,11 +229,6 @@
 }
 
 const char *CrashReasonAsString(CrashReason reason) {
-#ifdef LLDB_CONFIGURATION_BUILDANDINTEGRATION
-  // Just return the code in ascii for integration builds.
-  chcar str[8];
-  sprintf(str, "%d", reason);
-#else
   const char *str = nullptr;
 
   switch (reason) {
@@ -315,8 +310,6 @@
 str = "eFloatSubscriptRange";
 break;
   }
-#endif
-
   return str;
 }
 
Index: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===
--- source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -37,9 +37,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-// We disable the tracing of ptrace calls for integration builds to avoid the
-// additional indirection and checks.
-#ifndef LLDB_CONFIGURATION_BUILDANDINTEGRATION
 // Wrapper for ptrace to catch errors and log calls.
 
 const char *Get_PT_IO_OP(int op) {
@@ -137,9 +134,6 @@
 
 #define PTRACE(req, pid, addr, data)   
\
   PtraceWrapper((req), (pid), (addr), (data), #req, __FILE__, __LINE__)
-#else
-PtraceWrapper((req), (pid), (addr), (data))
-#endif
 
 // Static implementations of ProcessMonitor::ReadMemory and
 // ProcessMonitor::WriteMemory.  This enables mutual recursion between these


Index: source/Plugins/Process/POSIX/ProcessMessage.cpp
===
--- source/Plugins/Process/POSIX/ProcessMessage.cpp
+++ source/Plugins/Process/POSIX/ProcessMessage.cpp
@@ -15,11 +15,6 @@
 }
 
 const char *ProcessMessage::PrintKind(Kind kind) {
-#ifdef LLDB_CONFIGURATION_BUILDANDINTEGRATION
-  // Just return the code in ascii for integration builds.
-  chcar str[8];
-  sprintf(str, "%d", reason);
-#else
   const char *str = nullptr;
 
   switch (kind) {
@@ -60,8 +55,6 @@
 str = "eExecMessage";
 break;
   }
-#endif
-
   return str;
 }
 
Index: source/Plugins/Process/POSIX/CrashReason.cpp
===
--- source/Plugins/Process/POSIX/CrashReason.cpp
+++ source/Plugins/Process/POSIX/CrashReason.cpp
@@ -229,11 +229,6 @@
 }
 
 const char *CrashReasonAsString(CrashReason reason) {
-#ifdef LLDB_CONFIGURATION_BUILDANDINTEGRATION
-  // Just return the code in ascii for integration builds.
-  chcar str[8];
-  sprintf(str, "%d", reason);
-#else
   const char *str = nullptr;
 
   switch (reason) {
@@ -315,8 +310,6 @@
 str = "eFloatSubscriptRange";
 break;
   }
-#endif
-
   return str;
 }
 
Index: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===
--- source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -37,9 +37,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-// We disable the tracing of ptrace calls for integration builds to avoid the
-// additional indirection and checks.
-#ifndef LLDB_CONFIGURATION_BUILDANDINTEGRATION
 // Wrapper for ptrace to catch errors and log calls.
 
 const char *Get_PT_IO_OP(int op) {
@@ -137,9 +134,6 @@
 
 #define PTRACE(req, pid, addr, data)   \
   PtraceWrapper((req), (pid), (addr), (data), #req, __FILE__, __LINE__)
-#else
-PtraceWrapper((req), (pid), (addr), (data))
-#endif
 
 // Static implementations of ProcessMonitor::ReadMemory and
 // ProcessMonitor::WriteMemory.  This enables mutual recursion between these
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66744: NativeProcessLinux: Remove some register context boilerplate

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: mgorny, jankratochvil, omjavaid, alexandreyy, uweigand.
Herald added subscribers: kbarton, javed.absar, nemanjai.

This patch follows the spirit of D63594 , and 
removes some null checks
for things which should be operating invariants. Specifically
{Read,Write}[GF]PR now no longer check whether the supplied buffers are
null, because they never are. After this, the Do*** versions of these
function no longer serve any purpose and are inlined into their callers.

Other cleanups are possible here too, but I am taking this one step at a
time because this involves a lot of architecture-specific code, which I
don't have the hardware to test on (I did do a build-test though).


https://reviews.llvm.org/D66744

Files:
  source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
  source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h
  source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h

Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -14,6 +14,7 @@
 #include "Plugins/Process/Linux/NativeRegisterContextLinux.h"
 #include "Plugins/Process/Utility/RegisterContext_s390x.h"
 #include "Plugins/Process/Utility/lldb-s390x-register-enums.h"
+#include 
 
 namespace lldb_private {
 namespace process_linux {
@@ -66,13 +67,18 @@
   Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
   const RegisterValue &value) override;
 
-  Status DoReadGPR(void *buf, size_t buf_size) override;
+  Status ReadGPR() override;
 
-  Status DoWriteGPR(void *buf, size_t buf_size) override;
+  Status WriteGPR() override;
 
-  Status DoReadFPR(void *buf, size_t buf_size) override;
+  Status ReadFPR() override;
 
-  Status DoWriteFPR(void *buf, size_t buf_size) override;
+  Status WriteFPR() override;
+
+  void *GetGPRBuffer() override { return &m_regs; }
+  size_t GetGPRSize() override { return sizeof(m_regs); }
+  void *GetFPRBuffer() override { return &m_fp_regs; }
+  size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 
 private:
   // Info about register ranges.
@@ -90,6 +96,9 @@
   RegInfo m_reg_info;
   lldb::addr_t m_watchpoint_addr;
 
+  s390_regs m_regs;
+  s390_fp_regs m_fp_regs;
+
   // Private member methods.
   bool IsRegisterSetAvailable(uint32_t set_index) const;
 
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
@@ -18,7 +18,6 @@
 
 #include "Plugins/Process/Utility/RegisterContextLinux_s390x.h"
 
-#include 
 #include 
 #include 
 
@@ -196,13 +195,12 @@
   reg_info->name);
 
   if (IsGPR(reg)) {
-s390_regs regs;
-Status error = DoReadGPR(®s, sizeof(regs));
+Status error = ReadGPR();
 if (error.Fail())
   return error;
 
-uint8_t *src = (uint8_t *)®s + reg_info->byte_offset;
-assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(regs));
+uint8_t *src = (uint8_t *)&m_regs + reg_info->byte_offset;
+assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(m_regs));
 switch (reg_info->byte_size) {
 case 4:
   reg_value.SetUInt32(*(uint32_t *)src);
@@ -218,14 +216,13 @@
   }
 
   if (IsFPR(reg)) {
-s390_fp_regs fp_regs;
-Status error = DoReadFPR(&fp_regs, sizeof(fp_regs));
+Status error = ReadFPR();
 if (error.Fail())
   return error;
 
 // byte_offset is just the offset within FPR, not the whole user area.
-uint8_t *src = (uint8_t *)&fp_regs + reg_info->byte_offset;
-assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(fp_regs));
+uint8_t *src = (uint8_t *)&m_fp_regs + reg_info->byte_offset;
+assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(m_fp_regs));
 switch (reg_info->byte_size) {
 case 4:
   reg_value.SetUInt32(*(uint32_t *)src);
@@ -275,13 +272,12 @@
   reg_info->name);
 
   if (IsGPR(reg)) {
-s390_regs regs;
-Status error = DoReadGPR(®s, sizeof(regs));
+Status error = ReadGPR();
 if (error.Fail())
   return error;
 
-uint8_t *dst = (uint8_t *)®s + reg_info->byte_offset;
-assert(

[Lldb-commits] [PATCH] D66739: [lldb] Add -w flag for expressions to print compiler warnings even if expression succeeds.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Target/Target.h:1106
+  std::string *fixed_expression = nullptr, ValueObject *ctx_obj = nullptr,
+  std::string *warnings = nullptr);
 

Is there anyway we could store this and things like the fixed expression in the 
`ExpressionResults` and then specify in the `EvaluateExpressionOptions` whether 
we want this to be set or not?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66739



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

[Lldb-commits] [PATCH] D66745: DWARFExpression: Simplify class interface

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, clayborg.
Herald added a subscriber: aprantl.

The DWARFExpression methods have a lot of arguments. This removes two of
them by removing the ability to slice the expression via two offset+size
parameters. This is a functionality that it is not always needed, and
when it is, we already have a different handy way of slicing a data
extractor which we can use instead.


https://reviews.llvm.org/D66745

Files:
  include/lldb/Expression/DWARFExpression.h
  source/Expression/DWARFExpression.cpp
  source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  source/Target/RegisterContext.cpp
  unittests/Expression/DWARFExpressionTest.cpp

Index: unittests/Expression/DWARFExpressionTest.cpp
===
--- unittests/Expression/DWARFExpressionTest.cpp
+++ unittests/Expression/DWARFExpressionTest.cpp
@@ -24,8 +24,8 @@
   Status status;
   if (!DWARFExpression::Evaluate(
   /*exe_ctx*/ nullptr, /*reg_ctx*/ nullptr, /*opcode_ctx*/ nullptr,
-  extractor, /*dwarf_cu*/ nullptr, /*offset*/ 0, expr.size(),
-  lldb::eRegisterKindLLDB, /*initial_value_ptr*/ nullptr,
+  extractor, /*dwarf_cu*/ nullptr, lldb::eRegisterKindLLDB,
+  /*initial_value_ptr*/ nullptr,
   /*object_address_ptr*/ nullptr, result, &status))
 return status.ToError();
 
Index: source/Target/RegisterContext.cpp
===
--- source/Target/RegisterContext.cpp
+++ source/Target/RegisterContext.cpp
@@ -82,14 +82,12 @@
   DataExtractor dwarf_data(dwarf_opcode_ptr, dwarf_opcode_len,
arch.GetByteOrder(), addr_size);
   ModuleSP opcode_ctx;
-  DWARFExpression dwarf_expr(opcode_ctx, dwarf_data, nullptr, 0,
- dwarf_opcode_len);
+  DWARFExpression dwarf_expr(opcode_ctx, dwarf_data, nullptr);
   Value result;
   Status error;
-  const lldb::offset_t offset = 0;
   if (dwarf_expr.Evaluate(&exe_ctx, this, opcode_ctx, dwarf_data, nullptr,
-  offset, dwarf_opcode_len, eRegisterKindDWARF, nullptr,
-  nullptr, result, &error)) {
+  eRegisterKindDWARF, nullptr, nullptr, result,
+  &error)) {
 expr_result = result.GetScalar().SInt(-1);
 switch (expr_result) {
 case 0:
Index: source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -174,7 +174,7 @@
   DataBufferSP buffer =
   std::make_shared(stream.GetData(), stream.GetSize());
   DataExtractor extractor(buffer, byte_order, address_size, byte_size);
-  DWARFExpression result(module, extractor, nullptr, 0, buffer->GetByteSize());
+  DWARFExpression result(module, extractor, nullptr);
   result.SetRegisterKind(register_kind);
 
   return result;
Index: source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
===
--- source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
+++ source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
@@ -122,7 +122,7 @@
   DataBufferSP buffer =
   std::make_shared(stream.GetData(), stream.GetSize());
   DataExtractor extractor(buffer, byte_order, address_size, byte_size);
-  DWARFExpression result(module, extractor, nullptr, 0, buffer->GetByteSize());
+  DWARFExpression result(module, extractor, nullptr);
   result.SetRegisterKind(register_kind);
 
   return result;
@@ -247,6 +247,6 @@
   .take_front(size);
   buffer->CopyData(bytes.data(), size);
   DataExtractor extractor(buffer, lldb::eByteOrderLittle, address_size);
-  DWARFExpression result(nullptr, extractor, nullptr, 0, size);
+  DWARFExpression result(nullptr, extractor, nullptr);
   return result;
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3243,15 +3243,18 @@
 uint32_t block_offset =
 form_value.BlockData() - debug_info_data.GetDataStart();
 uint32_t block_length = form_value.Unsigned();
-location = DWARFExpression(module, debug_info_data, die.GetCU(),
-   block_offset, block_length);
+location = DWARFExpression(
+ 

[Lldb-commits] [PATCH] D66634: Postfix: move more code out of the PDB plugin

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

I have a couple inline questions.  After that, it looks fine.




Comment at: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:61
 
+  for (auto it = parsed.begin(), end = parsed.end(); it != end; ++it) {
 // Emplace valid dependent subtrees to make target assignment independent

I would recommend making `parsed` a `const` vector.  The lambda captures the 
iterator, and while the code currently looks fine, I'd hate for something to 
change in the future that could invalidate the captured iterator.



Comment at: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:69
+  return pair.second;
+  }
+

This looks O(n^2).  Will that matter here or will the expressions always be 
short?



Comment at: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:84
   // found target assignment program - no need to parse further
-  return rvalue_ast;
+  return it->second;
 }

It's a shame the `pair` loses the more descriptive field names, but I don't see 
a good way to make it better, so this is a useless comment.



Comment at: lldb/trunk/source/Symbol/PostfixExpression.cpp:101
+if (!rhs)
+  return {};
+result.emplace_back(lhs, rhs);

Is it correct to return an empty vector here rather than `result`?  If there's 
one bad expression, you'll consider the entire FPO program empty.  That's 
sounds plausible, but I thought I'd ask.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66634



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

[Lldb-commits] [PATCH] D66655: [lldb] Fix x86 compilation

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

A few typos remained after copy-pasting




Comment at: 
source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp:279
 // This is likely an internal register for lldb use only and should not be
 // directly queried.
 error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "

written?



Comment at: 
source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp:281
 error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "
"register, cannot read directly",
reg_info->name);

write


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66655



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

[Lldb-commits] [lldb] r369922 - [dotest] Print invocation when encountering an error.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Aug 26 09:08:53 2019
New Revision: 369922

URL: http://llvm.org/viewvc/llvm-project?rev=369922&view=rev
Log:
[dotest] Print invocation when encountering an error.

With this patch dotest.py will print the full invocation whenever it
fails to parse its arguments. The dotest invocation is usually build up
with different inputs, potentially coming from CMake, lldb-dotest, lit
or passed directly. This can make debugging hard, especially on CI,
where there might be another layer of indirection. This aims to make
that a bit easier.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/dotest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=369922&r1=369921&r2=369922&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Mon Aug 26 09:08:53 2019
@@ -225,8 +225,12 @@ def parseOptionsAndInitTestdirs():
 platform_system = platform.system()
 platform_machine = platform.machine()
 
-parser = dotest_args.create_parser()
-args = dotest_args.parse_args(parser, sys.argv[1:])
+try:
+parser = dotest_args.create_parser()
+args = dotest_args.parse_args(parser, sys.argv[1:])
+except:
+print(' '.join(sys.argv))
+raise
 
 if args.unset_env_varnames:
 for env_var in args.unset_env_varnames:


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

[Lldb-commits] [lldb] r369930 - TestFunctionStarts.py: add synchronization

[Frederic Riss via lldb-commits]

Author: friss
Date: Mon Aug 26 10:14:05 2019
New Revision: 369930

URL: http://llvm.org/viewvc/llvm-project?rev=369930&view=rev
Log:
TestFunctionStarts.py: add synchronization

We have started to see the no_binary version of this test
fail. The reason is that the binary was being removed
before the spawn actually launched the inferior. Add a
simple filesystem based synchronization to avoid this race.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/TestFunctionStarts.py
lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/main.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/TestFunctionStarts.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/TestFunctionStarts.py?rev=369930&r1=369929&r2=369930&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/TestFunctionStarts.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/TestFunctionStarts.py
 Mon Aug 26 10:14:05 2019
@@ -43,8 +43,20 @@ class FunctionStartsTestCase(TestBase):
 except CalledProcessError as cmd_error:
 self.fail("Strip failed: %d"%(cmd_error.returncode))
 
-popen = self.spawnSubprocess(exe)
+# Use a file as a synchronization point between test and inferior.
+pid_file_path = lldbutil.append_to_process_working_directory(self,
+"token_pid_%d" % (int(os.getpid(
+self.addTearDownHook(
+lambda: self.run_platform_command(
+"rm %s" %
+(pid_file_path)))
+
+popen = self.spawnSubprocess(exe, [pid_file_path])
 self.addTearDownHook(self.cleanupSubprocesses)
+
+# Wait until process has fully started up.
+pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
 if in_memory:
   remove_file(exe)
 
@@ -68,7 +80,8 @@ class FunctionStartsTestCase(TestBase):
 thread = threads[0]
 self.assertTrue(thread.num_frames > 1, "Couldn't backtrace.")
 name = thread.frame[1].GetFunctionName()
-self.assertEqual("___lldb_unnamed_symbol1$$StripMe", name, "Frame name 
not synthetic")
+self.assertTrue(name.startswith("___lldb_unnamed_symbol"))
+self.assertTrue(name.endswith("$$StripMe"))
 
 
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/main.cpp?rev=369930&r1=369929&r2=369930&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/main.cpp 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/function-starts/main.cpp 
Mon Aug 26 10:14:05 2019
@@ -2,6 +2,7 @@
 #include 
 
 #include 
+#include 
 #include 
 
 extern void dont_strip_me()
@@ -21,6 +22,11 @@ static void *a_function()
 
 int main(int argc, char const *argv[])
 {
+{
+// Create file to signal that this process has started up.
+std::ofstream f;
+f.open(argv[1]);
+}
 a_function();
 return 0;
 }


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

[Lldb-commits] [lldb] r369939 - [lldb][NFC] Remove dead code that handles situations where LLDB has no dummy target

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Mon Aug 26 11:12:44 2019
New Revision: 369939

URL: http://llvm.org/viewvc/llvm-project?rev=369939&view=rev
Log:
[lldb][NFC] Remove dead code that handles situations where LLDB has no dummy 
target

Summary:
We always have a dummy target, so any error handling regarding a missing dummy 
target is dead code now.
Also makes the CommandObject methods that return Target& to express this fact 
in the API.

This patch just for the CommandObject part of LLDB. I'll migrate the rest of 
LLDB in a follow-up patch that's WIP.

Reviewers: labath

Reviewed By: labath

Subscribers: abidh, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/Interpreter/CommandObject.h
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/trunk/source/Commands/CommandObjectExpression.cpp
lldb/trunk/source/Commands/CommandObjectFrame.cpp
lldb/trunk/source/Commands/CommandObjectStats.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Interpreter/CommandObject.cpp

lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Modified: lldb/trunk/include/lldb/Interpreter/CommandObject.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandObject.h?rev=369939&r1=369938&r2=369939&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/CommandObject.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandObject.h Mon Aug 26 11:12:44 2019
@@ -340,8 +340,8 @@ protected:
   // This is for use in the command interpreter, when you either want the
   // selected target, or if no target is present you want to prime the dummy
   // target with entities that will be copied over to new targets.
-  Target *GetSelectedOrDummyTarget(bool prefer_dummy = false);
-  Target *GetDummyTarget();
+  Target &GetSelectedOrDummyTarget(bool prefer_dummy = false);
+  Target &GetDummyTarget();
 
   // If a command needs to use the "current" thread, use this call. Command
   // objects will have an ExecutionContext to use, and that may or may not have

Modified: lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp?rev=369939&r1=369938&r2=369939&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp Mon Aug 26 11:12:44 
2019
@@ -550,14 +550,7 @@ public:
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
-Target *target = GetSelectedOrDummyTarget(m_dummy_options.m_use_dummy);
-
-if (target == nullptr) {
-  result.AppendError("Invalid target.  Must set target before setting "
- "breakpoints (see 'target create' command).");
-  result.SetStatus(eReturnStatusFailed);
-  return false;
-}
+Target &target = GetSelectedOrDummyTarget(m_dummy_options.m_use_dummy);
 
 // The following are the various types of breakpoints that could be set:
 //   1).  -f -l -p  [-s -g]   (setting breakpoint by source location)
@@ -619,16 +612,11 @@ protected:
   // Only check for inline functions if
   LazyBool check_inlines = eLazyBoolCalculate;
 
-  bp_sp = target->CreateBreakpoint(&(m_options.m_modules), 
-   file,
-   m_options.m_line_num,
-   m_options.m_column,
-   m_options.m_offset_addr,
-   check_inlines, 
-   m_options.m_skip_prologue,
-   internal, 
-   m_options.m_hardware,
-   m_options.m_move_to_nearest_code);
+  bp_sp = target.CreateBreakpoint(
+  &(m_options.m_modules), file, m_options.m_line_num,
+  m_options.m_column, m_options.m_offset_addr, check_inlines,
+  m_options.m_skip_prologue, internal, m_options.m_hardware,
+  m_options.m_move_to_nearest_code);
 } break;
 
 case eSetTypeAddress: // Breakpoint by address
@@ -640,12 +628,11 @@ protected:
   if (num_modules_specified == 1) {
 const FileSpec *file_spec =
 m_options.m_modules.GetFileSpecPointerAtIndex(0);
-bp_sp = target->CreateAddressInModuleBreakpoint(m_options.m_load_addr,
-internal, file_spec,
-m_options.m_hardware);
+bp_sp = target.CreateAddressInModuleBreakpoint(
+m_options.m_load_addr, internal, file_

[Lldb-commits] [PATCH] D66737: [lldb][NFC] Remove dead code that handles situations where LLDB has no dummy target

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb2380c9fa4b: [lldb][NFC] Remove dead code that handles 
situations where LLDB has no dummy… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66737

Files:
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectStats.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -787,15 +787,10 @@
 
 // Now check if we have a running process.  If so, we should instruct the
 // process monitor to enable/disable DarwinLog support now.
-Target *target = GetSelectedOrDummyTarget();
-if (!target) {
-  // No target, so there is nothing more to do right now.
-  result.SetStatus(eReturnStatusSuccessFinishNoResult);
-  return true;
-}
+Target &target = GetSelectedOrDummyTarget();
 
 // Grab the active process.
-auto process_sp = target->GetProcessSP();
+auto process_sp = target.GetProcessSP();
 if (!process_sp) {
   // No active process, so there is nothing more to do right now.
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
@@ -877,9 +872,9 @@
 
 // Figure out if we've got a process.  If so, we can tell if DarwinLog is
 // available for that process.
-Target *target = GetSelectedOrDummyTarget();
-auto process_sp = target ? target->GetProcessSP() : ProcessSP();
-if (!target || !process_sp) {
+Target &target = GetSelectedOrDummyTarget();
+auto process_sp = target.GetProcessSP();
+if (!process_sp) {
   stream.PutCString("Availability: unknown (requires process)\n");
   stream.PutCString("Enabled: not applicable "
 "(requires process)\n");
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -917,12 +917,12 @@
   return g_arguments_data[arg_type].help_text;
 }
 
-Target *CommandObject::GetDummyTarget() {
-  return m_interpreter.GetDebugger().GetDummyTarget();
+Target &CommandObject::GetDummyTarget() {
+  return *m_interpreter.GetDebugger().GetDummyTarget();
 }
 
-Target *CommandObject::GetSelectedOrDummyTarget(bool prefer_dummy) {
-  return m_interpreter.GetDebugger().GetSelectedOrDummyTarget(prefer_dummy);
+Target &CommandObject::GetSelectedOrDummyTarget(bool prefer_dummy) {
+  return *m_interpreter.GetDebugger().GetSelectedOrDummyTarget(prefer_dummy);
 }
 
 Thread *CommandObject::GetDefaultThread() {
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -4672,51 +4672,49 @@
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 m_stop_hook_sp.reset();
 
-Target *target = GetSelectedOrDummyTarget();
-if (target) {
-  Target::StopHookSP new_hook_sp = target->CreateStopHook();
-
-  //  First step, make the specifier.
-  std::unique_ptr specifier_up;
-  if (m_options.m_sym_ctx_specified) {
-specifier_up.reset(
-new SymbolContextSpecifier(GetDebugger().GetSelectedTarget()));
-
-if (!m_options.m_module_name.empty()) {
-  specifier_up->AddSpecification(
-  m_options.m_module_name.c_str(),
-  SymbolContextSpecifier::eModuleSpecified);
-}
+Target &target = GetSelectedOrDummyTarget();
+Target::StopHookSP new_hook_sp = target.CreateStopHook();
+
+//  First step, make the specifier.
+std::unique_ptr specifier_up;
+if (m_options.m_sym_ctx_specified) {
+  specifier_up.reset(
+  new SymbolContextSpecifier(GetDebugger().GetSelectedTarget()));
+
+  if (!m_options.m_module_name.empty()) {
+specifier_up->AddSpecification(
+m_options.m_module_name.c_str(),
+SymbolContextSpecifier::eModuleSpecified);
+  }
 
-if (!m_options.m_class_name.empty()) {
-  specifier_up->AddSpecification(
-  m_options.m_class_name.c_str(),
-  SymbolContextSpecifier::eClassOrNamespaceSpecified);
-}
+  if (!m_options.m_class_name.empty()) {
+   

[Lldb-commits] [PATCH] D66654: Prevent D66398 `TestDataFormatterStdList`-like regressions in the future

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

What about having the first one that matched being the one that is used and 
avoid errors all together? We do this with the regex alias stuff already. With 
regular expressions people are sure to make ones that conflict with each other.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66654



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

[Lldb-commits] [PATCH] D66249: [JIT][Breakpoint] Add "BreakpointInjectedSite" and FCB Trampoline

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.



Comment at: lldb/source/Breakpoint/BreakpointInjectedSite.cpp:341
+  "\n"
+  "   typedef unsigned intuint32_t;\n"
+  "   typedef unsigned long long  uint64_t ;\n"

labath wrote:
> aprantl wrote:
> > This seems awfully specific. Shouldn't this be target-dependent, and is 
> > there no pre-existing table in LLDB that we could derive this from?
> This entire function is target specific. I would struggle to find a single 
> line in this expression that could be reused on linux for instance (and let's 
> not even talk about windows..).
> I still believe that the simplest approach here would be to allocate memory 
> for this structure on the stack, side-stepping the whole "how to allocate 
> memory in a target-independent manner" discussion completely.
As others have state, this is entirely target-specific. I'd really rather avoid 
behavior specific to one platform in something a general as lldbBreakpoint.



Comment at: lldb/source/Breakpoint/BreakpointInjectedSite.cpp:12
+#include "Plugins/ExpressionParser/Clang/ClangExpressionVariable.h"
+#include "Plugins/ExpressionParser/Clang/ClangUserExpression.h"
+#include "lldb/Expression/ExpressionVariable.h"

Please don't introduce dependencies on Plugins in non-plugin libraries. I'm 
working on removing them.

Is BreakpointInjectedSite inherently tied to clang? If so, why not move it to 
the ClangExpressionParser plugin? If not, there should be some kind of 
generalization to ExpressionParser and PersistentExpressions.



Comment at: lldb/source/Breakpoint/BreakpointInjectedSite.cpp:246
+  ClangUserExpression *clang_expr =
+  llvm::dyn_cast(m_condition_expression_sp.get());
+

labath wrote:
> This also looks like an improper core code -> plugin dependency.
Indeed it is. Let's find a way to avoid that.



Comment at: lldb/source/Breakpoint/BreakpointInjectedSite.cpp:264
+  for (uint32_t i = 0; i < num_elements; ++i) {
+const clang::NamedDecl *decl = nullptr;
+llvm::Value *value = nullptr;

Could you use CompilerDecl here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66249



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

[Lldb-commits] [PATCH] D66654: Prevent D66398 `TestDataFormatterStdList`-like regressions in the future

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D66654#1645658 , @clayborg wrote:

> What about having the first one that matched being the one that is used and 
> avoid errors all together?


This is how it worked so far before the fix D66398 
 (which made the regexes unambiguous - always 
matching at most 1 regex for any string).  The previous version of the patch 
 just sorted the regexes 
alphabetically because original LLDB did not sort them at all printing random 
results.

@labath did not like the alphabetical sorting as (IIUC) adding formatters from 
a new library can affect formatting for a different library.

> We do this with the regex alias stuff already.

I haven't found the work `alias` in either `grep -ri alias $(find -name 
DataFormatters)` or in  Variable Formatting doc 
.

> With regular expressions people are sure to make ones that conflict with each 
> other.

So which order to choose? It should not have different output during different 
runs of LLDB.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66654



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

[Lldb-commits] [PATCH] D66249: [JIT][Breakpoint] Add "BreakpointInjectedSite" and FCB Trampoline

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked 4 inline comments as done.
mib added a comment.

In D66249#1644791 , @labath wrote:

> In D66249#1643594 , @mib wrote:
>
> > In D66249#1642316 , @labath wrote:
> >
> > > A bunch more comments from me. So far, I've only tried to highlight the 
> > > most obvious problems or style issues.
> > >
> > >   It's not clear to me whether this is still prototype code or not... If 
> > > it isn't, I'll have a bunch more.. :)
> >
> >
> > This is prototype code that I'm trying to upstream to build upon.
> >
> > From the comments I already gathered, I started thinking of a better 
> > implementation of this feature (maybe v2).
> >  There are still some parts that need do be done, such as resolving the 
> > variables on optimized code or patching dynamically the copied instructions.
> >
> > But, I'd like to have a baseline that is upstream to start working on these 
> > parts.
> >
> > **All feedback is welcome !** :)
>
>
> Ok, I see where you're going. I think we have a different interpretation of 
> the word "prototype" . When I say "prototype code", I mean: the changes that 
> I've made while experimenting, to try out whether a particular approach is 
> feasible, and which I'm sharing with other people to get early feedback about 
> the direction I am going in. These changes may include various shortcuts or 
> hacks, which I've made to speed up the development of this prototype, but I 
> understand that these will have to be removed before the final code is 
> submitted (in fact, the final patch may often be a near-complete 
> reimplementation of the prototype) . In this light, the phrase "prototype 
> which I am trying to upstream" is a bit of an oxymoron since by the time it 
> is upstreamed, the code should no longer be a prototype. It does not mean 
> that the code should support every possible use case right from the start, 
> but it should follow best coding, design and testing practices, regardless of 
> any "experimental", "prototype" or other label.
>
> With the eventual upstreaming goal in mind, I've tried to make a more 
> thorough pass over this patch. The comments range from minute style issues 
> like using `(void)` in function declarations, which can be trivially fixed, 
> to layering concerns that may require more serious engineering work. Overall, 
> my biggest concern is that there is no proper encapsulation in this patch. 
> Despite being advertised as "fully extensible" this patch bakes in a lot of 
> knowledge in very generic pieces of code. For instance, the 
> `BreakpointInjectedSite` class is riddled with OS (`mach_vm_allocate`), 
> architecture (`rbp`) and language (`Builtin.int_trap()`) specific knowledge 
> -- it shouldn't need to know about none of those. I think this is the biggest 
> issue that needs to be resolved before this patchset is ready for 
> upstreaming. I've also made a number of other suggestions (here and in other 
> threads) about possible alternative courses implementations. These are 
> somewhat subjective and so I am not expecting you to implement all of them. 
> However, I would:
>  a) expect some sort of a reply saying why you chose to do one thing or the 
> other
>  b) strongly encourage you to try them out because I believe they will make 
> fixing some of the other issues easier (for instance, switching to allocating 
> the memory on the stack would mean that we can completely avoid the "how to 
> allocate memory on different OSs" discussion, and just deal with architecture 
> specifics, since subtracting from the stack pointer works the same way on 
> every OS).
>
> Also, this talk of a `v2` makes me worried that this code will become 
> deprecated/unmaintained the moment it hits the repository. Lldb has some bad 
> experience with fire-and-forget features, and the prospect of this makes me 
> want to be even stricter about any bad patterns I notice in the code. IIUC, 
> you are now working on a different take on this feature, which kind of means 
> that you yourself are not sure about whether this is the best approach here. 
> I understand the need for comparison, but why does one of these things need 
> to be in the main repo for you to be able to compare them?


Thanks for taking the time to review this patch **thoroughly**! I really 
appreciate it :) !

I totally get you're meaning of //prototype//, but I used this word to reflect 
what I was able to come up with during my internship.

I'm already working on a new implementation that would make all the 
architecture/OS/language specific bits more generic, by doing a stack 
allocation for the `$__lldb_arg` and having the trap in the trampoline directly 
(avoiding using the languages builtins).
The updated patch should arrive by the end of the week.

I'm also planning to generate the trampoline using inline assembly (`asm`)  
with `.cfi` instructions as you suggested, but this will c

[Lldb-commits] [lldb] r369970 - [Core] GetAPInt should return an Optional

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Mon Aug 26 14:09:57 2019
New Revision: 369970

URL: http://llvm.org/viewvc/llvm-project?rev=369970&view=rev
Log:
[Core] GetAPInt should return an Optional

The current implementation returns a bool for indicating success and
whether or not the APInt passed by reference was populated. Instead of
doing that, I think it makes more sense to return an Optional.

Modified:
lldb/trunk/source/Core/DumpDataExtractor.cpp

Modified: lldb/trunk/source/Core/DumpDataExtractor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/DumpDataExtractor.cpp?rev=369970&r1=369969&r2=369970&view=diff
==
--- lldb/trunk/source/Core/DumpDataExtractor.cpp (original)
+++ lldb/trunk/source/Core/DumpDataExtractor.cpp Mon Aug 26 14:09:57 2019
@@ -20,6 +20,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "clang/AST/ASTContext.h"
@@ -28,6 +29,7 @@
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 
 #include 
@@ -64,8 +66,9 @@ static float half2float(uint16_t half) {
   return u.f * ldexpf(1, -112);
 }
 
-static bool GetAPInt(const DataExtractor &data, lldb::offset_t *offset_ptr,
- lldb::offset_t byte_size, llvm::APInt &result) {
+static llvm::Optional GetAPInt(const DataExtractor &data,
+lldb::offset_t *offset_ptr,
+lldb::offset_t byte_size) {
   llvm::SmallVector uint64_array;
   lldb::offset_t bytes_left = byte_size;
   uint64_t u64;
@@ -81,8 +84,7 @@ static bool GetAPInt(const DataExtractor
   }
   uint64_array.push_back(u64);
 }
-result = llvm::APInt(byte_size * 8, 
llvm::ArrayRef(uint64_array));
-return true;
+return llvm::APInt(byte_size * 8, llvm::ArrayRef(uint64_array));
   } else if (byte_order == lldb::eByteOrderBig) {
 lldb::offset_t be_offset = *offset_ptr + byte_size;
 lldb::offset_t temp_offset;
@@ -101,18 +103,17 @@ static bool GetAPInt(const DataExtractor
   uint64_array.push_back(u64);
 }
 *offset_ptr += byte_size;
-result = llvm::APInt(byte_size * 8, 
llvm::ArrayRef(uint64_array));
-return true;
+return llvm::APInt(byte_size * 8, llvm::ArrayRef(uint64_array));
   }
-  return false;
+  return llvm::None;
 }
 
 static lldb::offset_t DumpAPInt(Stream *s, const DataExtractor &data,
 lldb::offset_t offset, lldb::offset_t 
byte_size,
 bool is_signed, unsigned radix) {
-  llvm::APInt apint;
-  if (GetAPInt(data, &offset, byte_size, apint)) {
-std::string apint_str(apint.toString(radix, is_signed));
+  llvm::Optional apint = GetAPInt(data, &offset, byte_size);
+  if (apint.hasValue()) {
+std::string apint_str(apint.getValue().toString(radix, is_signed));
 switch (radix) {
 case 2:
   s->Write("0b", 2);
@@ -572,10 +573,11 @@ lldb::offset_t lldb_private::DumpDataExt
 apint);
   apfloat.toString(sv, format_precision, format_max_padding);
 } else if (item_bit_size == ast->getTypeSize(ast->DoubleTy)) {
-  llvm::APInt apint;
-  if (GetAPInt(DE, &offset, item_byte_size, apint)) {
+  llvm::Optional apint =
+  GetAPInt(DE, &offset, item_byte_size);
+  if (apint.hasValue()) {
 llvm::APFloat 
apfloat(ast->getFloatTypeSemantics(ast->DoubleTy),
-  apint);
+  apint.getValue());
 apfloat.toString(sv, format_precision, format_max_padding);
   }
 } else if (item_bit_size == ast->getTypeSize(ast->LongDoubleTy)) {
@@ -586,9 +588,10 @@ lldb::offset_t lldb_private::DumpDataExt
   if (&semantics == &llvm::APFloatBase::x87DoubleExtended())
 byte_size = (llvm::APFloat::getSizeInBits(semantics) + 7) / 8;
 
-  llvm::APInt apint;
-  if (GetAPInt(DE, &offset, byte_size, apint)) {
-llvm::APFloat apfloat(semantics, apint);
+  llvm::Optional apint =
+  GetAPInt(DE, &offset, byte_size);
+  if (apint.hasValue()) {
+llvm::APFloat apfloat(semantics, apint.getValue());
 apfloat.toString(sv, format_precision, format_max_padding);
   }
 } else if (item_bit_size == ast->getTypeSize(ast->HalfTy)) {


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

[Lldb-commits] [PATCH] D66739: [lldb] Add -w flag for expressions to print compiler warnings even if expression succeeds.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Expression/UserExpression.cpp:276
   keep_expression_in_memory, generate_debug_info);
+
   if (parse_success) {

Is this just white space?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66739



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

[Lldb-commits] [PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added a comment.

Other than my two comment this LGTM




Comment at: clang/lib/AST/ASTImporter.cpp:3452
 << FoundField->getType();
-
-  return make_error(ImportError::NameConflict);
+  ConflictingDecls.push_back(FoundField);
 }

I am a little concerned about this change. I am wondering if this was 
previously handled differently as a band-aid for some other issues that only 
showed up in rare cases when they iterated over all the results but not when 
they errored out on the first. 

Could we make the changes to `VisitFieldDecl` a separate PR so it is easier to 
revert later on if we do find this causes a regression we are not currently 
covering?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5615
+
+TEST_P(ConflictingDeclsWithLiberalStrategy, Typedef) {
+  Decl *ToTU = getToTuDecl(

Since the "Liberal" strategy should be the current status quo, I think it might 
make sense to have a first PR that just has the `*LiberalStrategy` tests to 
verify that indeed this is the current behavior as we expect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692



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

[Lldb-commits] [lldb] r369987 - [build_exception] Decode build failure messages

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Mon Aug 26 16:24:45 2019
New Revision: 369987

URL: http://llvm.org/viewvc/llvm-project?rev=369987&view=rev
Log:
[build_exception] Decode build failure messages

This is so that the test harness pretty-prints build error messages in
trace mode, instead of dumping a raw python bytes object.

Modified:
lldb/trunk/packages/Python/lldbsuite/test_event/build_exception.py

Modified: lldb/trunk/packages/Python/lldbsuite/test_event/build_exception.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test_event/build_exception.py?rev=369987&r1=369986&r2=369987&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test_event/build_exception.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test_event/build_exception.py Mon Aug 
26 16:24:45 2019
@@ -13,4 +13,4 @@ class BuildError(Exception):
 @staticmethod
 def format_build_error(command, command_output):
 return "Error when building test subject.\n\nBuild 
Command:\n{}\n\nBuild Command Output:\n{}".format(
-command, command_output)
+command, command_output.decode("utf-8"))


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

[Lldb-commits] [PATCH] D66774: [dotest] Remove long running test "decorator" and re-enable tests.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a reviewer: jfb.
Herald added a project: LLDB.

Today I discovered the `skipLongRunningTest` decorator and to my surprise all 
the tests were passing without the decorator. They don't seem to be that 
expensive either, they take a few seconds but we have tests that take much 
longer than that. As such I propose to remove the decorator and enable them by 
default.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D66774

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyCrash.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManySignals.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
  
lldb/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
  lldb/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py
  
lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestFoundationDisassembly.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py

Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -560,18 +560,6 @@
 print("Restore dir to:", cls.oldcwd, file=sys.stderr)
 os.chdir(cls.oldcwd)
 
-@classmethod
-def skipLongRunningTest(cls):
-"""
-By default, we skip long running test case.
-This can be overridden by passing '-l' to the test driver (dotest.py).
-"""
-if "LLDB_SKIP_LONG_RUNNING_TEST" in os.environ and "NO" == os.environ[
-"LLDB_SKIP_LONG_RUNNING_TEST"]:
-return False
-else:
-return True
-
 def enableLogChannelsForCurrentTest(self):
 if len(lldbtest_config.channels) == 0:
 return
Index: lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestFoundationDisassembly.py
===
--- lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestFoundationDisassembly.py
+++ lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestFoundationDisassembly.py
@@ -18,11 +18,6 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-# rdar://problem/8504895
-# Crash while doing 'disassemble -n "-[NSNumber descriptionWithLocale:]"
-@unittest2.skipIf(
-TestBase.skipLongRunningTest(),
-"Skip this long running test")
 def test_foundation_disasm(self):
 """Do 'disassemble -n func' on each and every 'Code' symbol entry from the Foundation.framework."""
 self.build()
Index: lldb/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py
===
--- lldb/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py
@@ -22,11 +22,6 @@
 # Find the line number to break inside main().
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-# rdar://problem/8504895
-# Crash while doing 'disassemble -n "-[NSNumber descriptionWithLocale:]"
-@unittest2.skipIf(
-TestBase.skipLongRunningTest(),
-"Skip this long running test")
 def test_stdcxx_disasm(self):
 """Do 'disassemble' on each and every 'Code' symbol entry from the std c++ lib."""
 self.build()
Index: lldb/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
@@ -145,9 +145,6 @@
 for f in test_source_files:
 if f.endswith(".cpp") or f.endswith(".c"):
 @add_test_categories(["dwarf"])
-@unittest2.skipIf(
-TestBase.skipLongRunningTest(),
-"Skip this long running test")
 def test_function_dwarf(self, f=f):
 if f.endswith(".cpp"):
 d = {'CXX_SOURCES': f}
Index: lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
+++ lldb/packages/Python/lldbsuite/test/functionalitie

[Lldb-commits] [PATCH] D66774: [dotest] Remove long running test "decorator" and re-enable tests.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.
Herald added a subscriber: dexonsmith.

LGTM


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66774



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

[Lldb-commits] [lldb] r369990 - Send error message on failed attach from debugerserver.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Mon Aug 26 17:08:27 2019
New Revision: 369990

URL: http://llvm.org/viewvc/llvm-project?rev=369990&view=rev
Log:
Send error message on failed attach from debugerserver.

Instead of using a magic return error code from debugserver to
indicate that an attach failed because of SIP being enabled in
RNBRemote::HandlePacket_v, use the extended error reporting that
Pavel added to lldb/lldb-server in https://reviews.llvm.org/D45573


 

Modified:
lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=369990&r1=369989&r2=369990&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Mon Aug 26 17:08:27 2019
@@ -2667,6 +2667,17 @@ void append_hex_value(std::ostream &ostr
   }
 }
 
+std::string cstring_to_asciihex_string(const char *str) {
+  std::string hex_str;
+  hex_str.reserve (strlen (str) * 2);
+  while (str && *str) {
+char hexbuf[5];
+snprintf (hexbuf, sizeof(hexbuf), "%02x", *str++);
+hex_str += hexbuf;
+  }
+  return hex_str;
+}
+
 void append_hexified_string(std::ostream &ostrm, const std::string &string) {
   size_t string_size = string.size();
   const char *string_buf = string.c_str();
@@ -3818,8 +3829,13 @@ rnb_err_t RNBRemote::HandlePacket_v(cons
 }
   }
   if (attach_failed_due_to_sip) {
-SendPacket("E87"); // E87 is the magic value which says that we are
-   // not allowed to attach
+std::string return_message = "E96;";
+return_message += cstring_to_asciihex_string(
+"Process attach denied, possibly because "
+"System Integrity Protection is enabled and "
+"process does not allow attaching.");
+
+SendPacket(return_message.c_str());
 DNBLogError("Attach failed because process does not allow "
 "attaching: \"%s\".",
 err_str);


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

[Lldb-commits] [PATCH] D66774: [dotest] Remove long running test "decorator" and re-enable tests.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL369995: [dotest] Remove long running test 
"decorator" and re-enable tests. (authored by JDevlieghere, committed 
by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66774?vs=217266&id=217278#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66774

Files:
  lldb/trunk/packages/Python/lldbsuite/test/configuration.py
  lldb/trunk/packages/Python/lldbsuite/test/dotest.py
  lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyCrash.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManySignals.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestFoundationDisassembly.py
  lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Index: lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py
@@ -84,7 +84,6 @@
 metavar='filterspec',
 action='append',
 help='Specify a filter, which consists of the test class name, a dot, followed by the test method, to only admit such test into the test suite')  # FIXME: Example?
-X('-l', "Don't skip long running tests")
 group.add_argument(
 '-p',
 metavar='pattern',
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -560,18 +560,6 @@
 print("Restore dir to:", cls.oldcwd, file=sys.stderr)
 os.chdir(cls.oldcwd)
 
-@classmethod
-def skipLongRunningTest(cls):
-"""
-By default, we skip long running test case.
-This can be overridden by passing '-l' to the test driver (dotest.py).
-"""
-if "LLDB_SKIP_LONG_RUNNING_TEST" in os.environ and "NO" == os.environ[
-"LLDB_SKIP_LONG_RUNNING_TEST"]:
-return False
-else:
-return True
-
 def enableLogChannelsForCurrentTest(self):
 if len(lldbtest_config.channels) == 0:
 return
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
@@ -12,9 +12,6 @@
 
 mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
-@unittest2.skipIf(
-TestBase.skipLongRunningTest(),
-"Skip this long running test")
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
 @add_test_categories(["watchpoint"])
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyCrash.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyCrash.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyCrash.py
@@ -12,9 +12,6 @@
 
 mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
-@unittest2.skipIf(
-TestBase.skipLongRunningTest(),
-"Skip this long running test")
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
 def test(self):
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManySignals.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManySignals.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManySignals.py
@@ -12,9 +12,6 @@
 
 mydir = ConcurrentEventsBase.co

[Lldb-commits] [lldb] r369995 - [dotest] Remove long running test "decorator" and re-enable tests.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Aug 26 17:18:22 2019
New Revision: 369995

URL: http://llvm.org/viewvc/llvm-project?rev=369995&view=rev
Log:
[dotest] Remove long running test "decorator" and re-enable tests.

Today I discovered the skipLongRunningTest decorator and to my surprise
all the tests were passing without the decorator. They don't seem to be
that expensive either, they take a few seconds but we have tests that
take much longer than that. As such I propose to remove the decorator
and enable them by default.

Differential revision: https://reviews.llvm.org/D66774

Modified:
lldb/trunk/packages/Python/lldbsuite/test/configuration.py
lldb/trunk/packages/Python/lldbsuite/test/dotest.py
lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyCrash.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManySignals.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestFoundationDisassembly.py
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/configuration.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/configuration.py?rev=369995&r1=369994&r2=369995&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/configuration.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/configuration.py Mon Aug 26 
17:18:22 2019
@@ -57,9 +57,6 @@ cflags_extras = ''
 # The filters (testclass.testmethod) used to admit tests into our test suite.
 filters = []
 
-# By default, we skip long running test case.  Use '-l' option to override.
-skip_long_running_test = True
-
 # Parsable mode silences headers, and any other output this script might 
generate, and instead
 # prints machine-readable output similar to what clang tests produce.
 parsable = False

Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=369995&r1=369994&r2=369995&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Mon Aug 26 17:18:22 2019
@@ -367,9 +367,6 @@ def parseOptionsAndInitTestdirs():
 usage(parser)
 configuration.filters.extend(args.f)
 
-if args.l:
-configuration.skip_long_running_test = False
-
 if args.framework:
 configuration.lldbFrameworkPath = args.framework
 
@@ -1143,10 +1140,6 @@ def run_suite():
 
 setupSysPath()
 
-#
-# If '-l' is specified, do not skip the long running tests.
-if not configuration.skip_long_running_test:
-os.environ["LLDB_SKIP_LONG_RUNNING_TEST"] = "NO"
 
 # For the time being, let's bracket the test runner within the
 # lldb.SBDebugger.Initialize()/Terminate() pair.

Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py?rev=369995&r1=369994&r2=369995&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py Mon Aug 26 
17:18:22 2019
@@ -84,7 +84,6 @@ def create_parser():
 metavar='filterspec',
 action='append',
 help='Specify a filter, which consists of the test class name, a dot, 
followed by the test method, to only admit such test into the test suite')  # 
FIXME: Example?
-X('-l', "Don't skip long running tests")
 group.add_argument(
 '-p',
 metavar='pattern',

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py?rev=369995&r1=369994&r2=369995&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_event

[Lldb-commits] [lldb] r370002 - [ConnectionFileDescriptor] Add shutdown check in ::Write.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Aug 26 18:34:16 2019
New Revision: 370002

URL: http://llvm.org/viewvc/llvm-project?rev=370002&view=rev
Log:
[ConnectionFileDescriptor] Add shutdown check in ::Write.

The disconnect method sets the shutdown flag to true. This currently
only prevents any reads from happening, but not writes, which is
incorrect. Presumably this was just an oversight when adding
synchronization to the class. This adds the same shutdown check to the
Write method.

Over-the-shoulder reviewed by Jim!

Modified:
lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Modified: lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp?rev=370002&r1=370001&r2=370002&view=diff
==
--- lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp (original)
+++ lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp Mon Aug 26 
18:34:16 2019
@@ -312,9 +312,6 @@ ConnectionStatus ConnectionFileDescripto
   // descriptor.  If that's the case, then send the "q" char to the command
   // file channel so the read will wake up and the connection will then know to
   // shut down.
-
-  m_shutting_down = true;
-
   std::unique_lock locker(m_mutex, std::defer_lock);
   if (!locker.try_lock()) {
 if (m_pipe.CanWrite()) {
@@ -334,6 +331,9 @@ ConnectionStatus ConnectionFileDescripto
 locker.lock();
   }
 
+  // Prevents reads and writes during shutdown.
+  m_shutting_down = true;
+
   Status error = m_read_sp->Close();
   Status error2 = m_write_sp->Close();
   if (error.Fail() || error2.Fail())
@@ -369,6 +369,8 @@ size_t ConnectionFileDescriptor::Read(vo
   }
 
   if (m_shutting_down) {
+if (error_ptr)
+  error_ptr->SetErrorString("shutting down");
 status = eConnectionStatusError;
 return 0;
   }
@@ -473,6 +475,13 @@ size_t ConnectionFileDescriptor::Write(c
 return 0;
   }
 
+  if (m_shutting_down) {
+if (error_ptr)
+  error_ptr->SetErrorString("shutting down");
+status = eConnectionStatusError;
+return 0;
+  }
+
   Status error;
 
   size_t bytes_sent = src_len;


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

[Lldb-commits] [lldb] r370003 - [test] Disable two of the recently (re)enabled tests on Windows.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Aug 26 18:34:19 2019
New Revision: 370003

URL: http://llvm.org/viewvc/llvm-project?rev=370003&view=rev
Log:
[test] Disable two of the recently (re)enabled tests on Windows.

This disables two tests on Windows that I re-enabled in r369995.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py?rev=370003&r1=370002&r2=370003&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py
 Mon Aug 26 18:34:19 2019
@@ -144,6 +144,7 @@ for d in test_source_dirs:
 # Generate test cases based on the collected source files
 for f in test_source_files:
 if f.endswith(".cpp") or f.endswith(".c"):
+@skipIfWindows
 @add_test_categories(["dwarf"])
 def test_function_dwarf(self, f=f):
 if f.endswith(".cpp"):

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py?rev=370003&r1=370002&r2=370003&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py 
Mon Aug 26 18:34:19 2019
@@ -22,6 +22,7 @@ class StdCXXDisassembleTestCase(TestBase
 # Find the line number to break inside main().
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
+@skipIfWindows
 def test_stdcxx_disasm(self):
 """Do 'disassemble' on each and every 'Code' symbol entry from the std 
c++ lib."""
 self.build()


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

[Lldb-commits] [lldb] r370019 - CommandObjectExpression: Fix a misleading-indentation warning

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 23:42:42 2019
New Revision: 370019

URL: http://llvm.org/viewvc/llvm-project?rev=370019&view=rev
Log:
CommandObjectExpression: Fix a misleading-indentation warning

Modified:
lldb/trunk/source/Commands/CommandObjectExpression.cpp

Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=370019&r1=370018&r2=370019&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Mon Aug 26 23:42:42 
2019
@@ -379,117 +379,115 @@ bool CommandObjectExpression::EvaluateEx
   if (!target)
 target = &GetDummyTarget();
 
-lldb::ValueObjectSP result_valobj_sp;
-bool keep_in_memory = true;
-StackFrame *frame = exe_ctx.GetFramePtr();
-
-EvaluateExpressionOptions options;
-options.SetCoerceToId(m_varobj_options.use_objc);
-options.SetUnwindOnError(m_command_options.unwind_on_error);
-options.SetIgnoreBreakpoints(m_command_options.ignore_breakpoints);
-options.SetKeepInMemory(keep_in_memory);
-options.SetUseDynamic(m_varobj_options.use_dynamic);
-options.SetTryAllThreads(m_command_options.try_all_threads);
-options.SetDebug(m_command_options.debug);
-options.SetLanguage(m_command_options.language);
-options.SetExecutionPolicy(
-m_command_options.allow_jit
-? EvaluateExpressionOptions::default_execution_policy
-: lldb_private::eExecutionPolicyNever);
-
-bool auto_apply_fixits;
-if (m_command_options.auto_apply_fixits == eLazyBoolCalculate)
-  auto_apply_fixits = target->GetEnableAutoApplyFixIts();
-else
-  auto_apply_fixits = m_command_options.auto_apply_fixits == eLazyBoolYes;
-
-options.SetAutoApplyFixIts(auto_apply_fixits);
-
-if (m_command_options.top_level)
-  options.SetExecutionPolicy(eExecutionPolicyTopLevel);
-
-// If there is any chance we are going to stop and want to see what went
-// wrong with our expression, we should generate debug info
-if (!m_command_options.ignore_breakpoints ||
-!m_command_options.unwind_on_error)
-  options.SetGenerateDebugInfo(true);
-
-if (m_command_options.timeout > 0)
-  options.SetTimeout(std::chrono::microseconds(m_command_options.timeout));
-else
-  options.SetTimeout(llvm::None);
-
-ExpressionResults success = target->EvaluateExpression(
-expr, frame, result_valobj_sp, options, &m_fixed_expression);
-
-// We only tell you about the FixIt if we applied it.  The compiler errors
-// will suggest the FixIt if it parsed.
-if (error_stream && !m_fixed_expression.empty() &&
-target->GetEnableNotifyAboutFixIts()) {
-  if (success == eExpressionCompleted)
-error_stream->Printf(
-"  Fix-it applied, fixed expression was: \n%s\n",
-m_fixed_expression.c_str());
-}
-
-if (result_valobj_sp) {
-  Format format = m_format_options.GetFormat();
-
-  if (result_valobj_sp->GetError().Success()) {
-if (format != eFormatVoid) {
-  if (format != eFormatDefault)
-result_valobj_sp->SetFormat(format);
-
-  if (m_varobj_options.elem_count > 0) {
-Status error(CanBeUsedForElementCountPrinting(*result_valobj_sp));
-if (error.Fail()) {
-  result->AppendErrorWithFormat(
-  "expression cannot be used with --element-count %s\n",
-  error.AsCString(""));
-  result->SetStatus(eReturnStatusFailed);
-  return false;
-}
+  lldb::ValueObjectSP result_valobj_sp;
+  bool keep_in_memory = true;
+  StackFrame *frame = exe_ctx.GetFramePtr();
+
+  EvaluateExpressionOptions options;
+  options.SetCoerceToId(m_varobj_options.use_objc);
+  options.SetUnwindOnError(m_command_options.unwind_on_error);
+  options.SetIgnoreBreakpoints(m_command_options.ignore_breakpoints);
+  options.SetKeepInMemory(keep_in_memory);
+  options.SetUseDynamic(m_varobj_options.use_dynamic);
+  options.SetTryAllThreads(m_command_options.try_all_threads);
+  options.SetDebug(m_command_options.debug);
+  options.SetLanguage(m_command_options.language);
+  options.SetExecutionPolicy(
+  m_command_options.allow_jit
+  ? EvaluateExpressionOptions::default_execution_policy
+  : lldb_private::eExecutionPolicyNever);
+
+  bool auto_apply_fixits;
+  if (m_command_options.auto_apply_fixits == eLazyBoolCalculate)
+auto_apply_fixits = target->GetEnableAutoApplyFixIts();
+  else
+auto_apply_fixits = m_command_options.auto_apply_fixits == eLazyBoolYes;
+
+  options.SetAutoApplyFixIts(auto_apply_fixits);
+
+  if (m_command_options.top_level)
+options.SetExecutionPolicy(eExecutionPolicyTopLevel);
+
+  // If there is any chance we are going to stop and want to 

[Lldb-commits] [lldb] r370020 - Fix TestStdCXXDisassembly.py

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Aug 26 23:43:45 2019
New Revision: 370020

URL: http://llvm.org/viewvc/llvm-project?rev=370020&view=rev
Log:
Fix TestStdCXXDisassembly.py

missing decorator import.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py?rev=370020&r1=370019&r2=370020&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/stl/TestStdCXXDisassembly.py 
Mon Aug 26 23:43:45 2019
@@ -10,7 +10,7 @@ import os
 import lldb
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
-
+from lldbsuite.test.decorators import *
 
 class StdCXXDisassembleTestCase(TestBase):
 


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