[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65489#1607801 , @labath wrote:

> Why do we need a separate backend for this? Couldn't this be emitted as a 
> part of the same `#include "XXXProperties.inc"` which defines the property 
> definition? The two logically belong together, and the only reason they were 
> separate variables in the first place was the limitations of constexpr global 
> variables...


I'm not sure what you mean by that last sentence, maybe because I don't know 
the history? Can you elaborate a bit on the "separate variables" part?

To answer your question: the `OptionEnumValueElement`s are used both by 
properties and command options. Technically it is possible to put them in the 
same `.td` files, and emit them in the same `.inc` file. The actual code 
wouldn't change, just the way we invoke: instead of being its own backend, it 
would be called from both existing backends. I don't have a strong preference 
for either approach.


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

https://reviews.llvm.org/D65489



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

Re: [Lldb-commits] [lldb] r367385 - [CompletionRequest] Remove unimplemented members.

[Raphael “Teemperor” Isemann via lldb-commits]

Thanks! Wasn’t sure back then if I can just remove that weird thing

> On Jul 31, 2019, at 5:48 AM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> Author: jdevlieghere
> Date: Tue Jul 30 20:48:29 2019
> New Revision: 367385
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=367385&view=rev
> Log:
> [CompletionRequest] Remove unimplemented members.
> 
> Completion requests have two fields that are essentially unimplemented:
> `m_match_start_point` and `m_max_return_elements`. This would've been
> okay, if it wasn't for the fact that this caused a bunch of useless
> parameters to be passed around. Occasionally there would be a comment or
> assert saying that they are not supported. This patch removes them.
> 
> Modified:
>lldb/trunk/include/lldb/Core/IOHandler.h
>lldb/trunk/include/lldb/Expression/REPL.h
>lldb/trunk/include/lldb/Host/Editline.h
>lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
>lldb/trunk/include/lldb/Utility/CompletionRequest.h
>lldb/trunk/source/API/SBCommandInterpreter.cpp
>lldb/trunk/source/Core/FormatEntity.cpp
>lldb/trunk/source/Core/IOHandler.cpp
>lldb/trunk/source/Expression/REPL.cpp
>lldb/trunk/source/Host/common/Editline.cpp
>lldb/trunk/source/Interpreter/CommandInterpreter.cpp
>lldb/trunk/source/Utility/CompletionRequest.cpp
>lldb/trunk/unittests/Utility/CompletionRequestTest.cpp
> 
> Modified: lldb/trunk/include/lldb/Core/IOHandler.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/IOHandler.h?rev=367385&r1=367384&r2=367385&view=diff
> ==
> --- lldb/trunk/include/lldb/Core/IOHandler.h (original)
> +++ lldb/trunk/include/lldb/Core/IOHandler.h Tue Jul 30 20:48:29 2019
> @@ -200,7 +200,6 @@ public:
> 
>   virtual int IOHandlerComplete(IOHandler &io_handler, const char 
> *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int max_matches,
> StringList &matches, StringList 
> &descriptions);
> 
>   virtual const char *IOHandlerGetFixIndentationCharacters() { return 
> nullptr; }
> @@ -417,7 +416,6 @@ private:
> 
>   static int AutoCompleteCallback(const char *current_line, const char 
> *cursor,
>   const char *last_char,
> -  int skip_first_n_matches, int max_matches,
>   StringList &matches, StringList 
> &descriptions,
>   void *baton);
> #endif
> @@ -452,7 +450,6 @@ public:
> 
>   int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int max_matches,
> StringList &matches, StringList &descriptions) 
> override;
> 
>   void IOHandlerInputComplete(IOHandler &io_handler,
> 
> Modified: lldb/trunk/include/lldb/Expression/REPL.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/REPL.h?rev=367385&r1=367384&r2=367385&view=diff
> ==
> --- lldb/trunk/include/lldb/Expression/REPL.h (original)
> +++ lldb/trunk/include/lldb/Expression/REPL.h Tue Jul 30 20:48:29 2019
> @@ -105,7 +105,6 @@ public:
> 
>   int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int max_matches,
> StringList &matches, StringList &descriptions) 
> override;
> 
> protected:
> 
> Modified: lldb/trunk/include/lldb/Host/Editline.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Editline.h?rev=367385&r1=367384&r2=367385&view=diff
> ==
> --- lldb/trunk/include/lldb/Host/Editline.h (original)
> +++ lldb/trunk/include/lldb/Host/Editline.h Tue Jul 30 20:48:29 2019
> @@ -99,7 +99,6 @@ typedef int (*FixIndentationCallbackType
> 
> typedef int (*CompleteCallbackType)(const char *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int 
> max_matches,
> StringList &matches,
> StringList &descriptions, void *baton);
> 
> 
> Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h?rev=367385&r1=367384&r2=367385&view=diff
> ==
> --- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h (original)
> +++ lldb/trunk/include/lld

[Lldb-commits] [PATCH] D65498: Fix completion for functions in anonymous namespaces

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65498#1607805 , @labath wrote:

> I don't immediately see a better way to fix this, but I'm not terribly 
> familiar with this machinery either. I guess one day we may want to extend 
> the completion machinery to be able to expand "foo" into "(anonymous 
> namespace)::foobar", but that's likely to be a larger undertaking...


Yep, that's how I wanted to fix this before I discovered that was going to be a 
lot harder than I hoped. :-)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65498



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

[Lldb-commits] [PATCH] D65498: Fix completion for functions in anonymous namespaces

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I think it's possible to rewrite the line when doing a completion, but I 
remember that I got annoyed when I tried understand the code responsible for 
that (and decipher the magic return values). Can you file a radar maybe and 
I'll fix this once I got around to refactor the related code? This patch can go 
IMHO, so LGTM.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65498



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

[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D65489#1607807 , @JDevlieghere 
wrote:

> In D65489#1607801 , @labath wrote:
>
> > Why do we need a separate backend for this? Couldn't this be emitted as a 
> > part of the same `#include "XXXProperties.inc"` which defines the property 
> > definition? The two logically belong together, and the only reason they 
> > were separate variables in the first place was the limitations of constexpr 
> > global variables...
>
>
> I'm not sure what you mean by that last sentence, maybe because I don't know 
> the history? Can you elaborate a bit on the "separate variables" part?
>
> To answer your question: the `OptionEnumValueElement`s are used both by 
> properties and command options. Technically it is possible to put them in the 
> same `.td` files, and emit them in the same `.inc` file. The actual code 
> wouldn't change, just the way we invoke: instead of being its own backend, it 
> would be called from both existing backends. I don't have a strong preference 
> for either approach.


Ah, I'm sorry. Somehow I didn't get what the " used by both properties and 
options" part means. I do now, and I agree that in this case, it does not make 
sense to generate these as a part of the properties definition.

However, I'm not sure it then makes sense to generate them at all. I mean, it's 
not like there's any redundancy (like it was for option definitions) in the 
`OptionEnumValueElement` struct, and it's pretty obvious what the individual 
fields mean even without naming them. And the tablegenning definitely increases 
the barrier for understanding the code. Is there any follow-up work or cleanups 
that would not be possible if this just stays a plain C array?


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

https://reviews.llvm.org/D65489



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

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextWindows_i386.cpp:70-103
+static const RegisterInfo *GetRegisterInfoPtr(const ArchSpec &target_arch) {
+  switch (target_arch.GetMachine()) {
+  case llvm::Triple::x86:
+return g_register_infos_i386;
+  default:
+llvm_unreachable("Unhandled target architecture.");
+  }

I think all of this could be bolied down to:
```
RegisterContextWindows_i386::RegisterContextWindows_i386(
const ArchSpec &target_arch)
: lldb_private::RegisterInfoInterface(target_arch),
  m_register_info_p(g_register_infos_i386),
  m_register_info_count(llvm::array_lengthof(g_register_infos_i386)),
  m_user_register_count(llvm::array_lengthof(g_register_infos_i386)) {
  assert(target_arch.GetMachine() == llvm::Triple::x86);
}
```

In fact, I'd probably not even bother with the member variables, but have the 
public RegisterContextWindows_i386 functions return the values directly (just 
like GetGPRSize does).



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:141
+GetRegisterInfo_WoW64(const lldb_private::ArchSpec &arch) {
+  // A WoW64 register info is the same as the i386's.
+  std::vector &g_register_infos =

asmith wrote:
> labath wrote:
> > Hui wrote:
> > > labath wrote:
> > > > Why is all of this complexity necessary? Couldn't you just switch on 
> > > > the target architecture in `CreateRegisterInfoInterface` in 
> > > > NativeRegisterContextWindows_x86_64.cpp and create either 
> > > > `RegisterContextWindows_WoW64` or `RegisterContextWindows_x86_64` ?
> > > > 
> > > > In fact, if RegisterContextWindows_WoW64 is identical to 
> > > > RegisterContextWindows_i386, then why do you even need the _WoW64 
> > > > version of the class in the first place? FWIW, linux also does not have 
> > > > a RegisterContextLinux_32_bit_process_on_64_bit_kernel class...
> > > I think WoW64 is i686 that shall deserve a separate arch specific 
> > > implementation?
> > I am sorry, but I don't follow. Can you repeat the question?
> > 
> > (FWIW, I don't believe that the fact that two things are different from a 
> > certain point of view justifies copy-pasting (at least) 150 LOC. If you 
> > think it's confusing to use something called "i386" in things dealing with 
> > WoW64, you can always typedef the WOW64 context to it, or rename the i386 
> > context to something more generic.)
> I don't want this to block the review and have removed the code.
Thanks. FTR, I'm not opposed to splitting these classes again in the future, if 
we develop a need for them to diverge (though it would be nice to find a way to 
factor the common parts in that case).

However, there's one more thing that bothers me here. It's this business with 
constructing a `RegisterContextWindows_i386` here, copying the entries out of 
it, and re-using them elsewhere while asserting that things were done in the 
right order. It seems very complex, and I'm not sure that complexity is needed. 
It seems to me like all of that could be removed if you just made the decision 
which set of registers to use one level up (i.e., in 
`CreateRegisterInfoInterface` in NativeRegisterContextWindows_x86_64.cpp. You 
could just have that function switch on the process byte size, and return 
RegisterContextWindows_i386 or RegisterContextWindows_x86_64. This is the same 
way things are done on x86 linux (see NativeRegisterContextLinux_x86_64.cpp).


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

https://reviews.llvm.org/D63165



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

[Lldb-commits] [lldb] r367392 - SymbolVendor: Remove the object file member variable

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Jul 31 01:25:25 2019
New Revision: 367392

URL: http://llvm.org/viewvc/llvm-project?rev=367392&view=rev
Log:
SymbolVendor: Remove the object file member variable

Summary:
The last responsibility of the SymbolVendor was to hold an owning
reference to the object file (in case symbols are being read from a
different file than the main module). As SymbolFile classes already hold
a non-owning reference to the object file, we can easily remove this
responsibility of the SymbolVendor by making the SymbolFile reference
owning.

Reviewers: JDevlieghere, clayborg, jingham

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/SymbolVendor.h
lldb/trunk/include/lldb/lldb-private-interfaces.h
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/source/Symbol/SymbolVendor.cpp

Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=367392&r1=367391&r2=367392&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Wed Jul 31 01:25:25 2019
@@ -50,11 +50,12 @@ public:
 kAllAbilities = ((1u << 7) - 1u)
   };
 
-  static SymbolFile *FindPlugin(ObjectFile *obj_file);
+  static SymbolFile *FindPlugin(lldb::ObjectFileSP objfile_sp);
 
   // Constructors and Destructors
-  SymbolFile(ObjectFile *obj_file)
-  : m_obj_file(obj_file), m_abilities(0), m_calculated_abilities(false) {}
+  SymbolFile(lldb::ObjectFileSP objfile_sp)
+  : m_objfile_sp(std::move(objfile_sp)), m_abilities(0),
+m_calculated_abilities(false) {}
 
   ~SymbolFile() override {}
 
@@ -210,8 +211,8 @@ public:
 return CompilerDeclContext();
   }
 
-  ObjectFile *GetObjectFile() { return m_obj_file; }
-  const ObjectFile *GetObjectFile() const { return m_obj_file; }
+  ObjectFile *GetObjectFile() { return m_objfile_sp.get(); }
+  const ObjectFile *GetObjectFile() const { return m_objfile_sp.get(); }
   ObjectFile *GetMainObjectFile();
 
   virtual std::vector ParseCallEdgesInFunction(UserID func_id) {
@@ -246,7 +247,10 @@ protected:
 
   void SetCompileUnitAtIndex(uint32_t idx, const lldb::CompUnitSP &cu_sp);
 
-  ObjectFile *m_obj_file; // The object file that symbols can be extracted 
from.
+  lldb::ObjectFileSP m_objfile_sp; // Keep a reference to the object file in
+   // case it isn't the same as the module
+   // object file (debug symbols in a separate
+   // file)
   llvm::Optional> m_compile_units;
   TypeList m_type_list;
   Symtab *m_symtab = nullptr;

Modified: lldb/trunk/include/lldb/Symbol/SymbolVendor.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolVendor.h?rev=367392&r1=367391&r2=367392&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolVendor.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolVendor.h Wed Jul 31 01:25:25 2019
@@ -131,10 +131,6 @@ public:
   uint32_t GetPluginVersion() override;
 
 protected:
-  lldb::ObjectFileSP m_objfile_sp; // Keep a reference to the object file in
-   // case it isn't the same as the module
-   // object file (debug symbols in a separate
-   // file)
   std::unique_ptr m_sym_file_up; // A single symbol file. 
Subclasses
  // can add more of these if 
needed.
 

Modified: lldb/trunk/include/lldb/lldb-private-interfaces.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-private-interfaces.h?rev=367392&r1=367391&r2=367392&view=diff
=

[Lldb-commits] [PATCH] D65401: SymbolVendor: Remove the object file member variable

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367392: SymbolVendor: Remove the object file member variable 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65401?vs=212175&id=212525#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65401

Files:
  lldb/trunk/include/lldb/Symbol/SymbolFile.h
  lldb/trunk/include/lldb/Symbol/SymbolVendor.h
  lldb/trunk/include/lldb/lldb-private-interfaces.h
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/trunk/source/Symbol/SymbolFile.cpp
  lldb/trunk/source/Symbol/SymbolVendor.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -35,10 +35,10 @@
   static const char *GetPluginDescriptionStatic();
 
   static lldb_private::SymbolFile *
-  CreateInstance(lldb_private::ObjectFile *obj_file);
+  CreateInstance(lldb::ObjectFileSP objfile_sp);
 
   // Constructors and Destructors
-  SymbolFilePDB(lldb_private::ObjectFile *ofile);
+  SymbolFilePDB(lldb::ObjectFileSP objfile_sp);
 
   ~SymbolFilePDB() override;
 
Index: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -119,28 +119,28 @@
 }
 
 lldb_private::SymbolFile *
-SymbolFilePDB::CreateInstance(lldb_private::ObjectFile *obj_file) {
-  return new SymbolFilePDB(obj_file);
+SymbolFilePDB::CreateInstance(ObjectFileSP objfile_sp) {
+  return new SymbolFilePDB(std::move(objfile_sp));
 }
 
-SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file)
-: SymbolFile(object_file), m_session_up(), m_global_scope_up() {}
+SymbolFilePDB::SymbolFilePDB(lldb::ObjectFileSP objfile_sp)
+: SymbolFile(std::move(objfile_sp)), m_session_up(), m_global_scope_up() {}
 
 SymbolFilePDB::~SymbolFilePDB() {}
 
 uint32_t SymbolFilePDB::CalculateAbilities() {
   uint32_t abilities = 0;
-  if (!m_obj_file)
+  if (!m_objfile_sp)
 return 0;
 
   if (!m_session_up) {
 // Lazily load and match the PDB file, but only do this once.
-std::string exePath = m_obj_file->GetFileSpec().GetPath();
+std::string exePath = m_objfile_sp->GetFileSpec().GetPath();
 auto error = loadDataForEXE(PDB_ReaderType::DIA, llvm::StringRef(exePath),
 m_session_up);
 if (error) {
   llvm::consumeError(std::move(error));
-  auto module_sp = m_obj_file->GetModule();
+  auto module_sp = m_objfile_sp->GetModule();
   if (!module_sp)
 return 0;
   // See if any symbol file is specified through `--symfile` option.
@@ -183,7 +183,8 @@
 }
 
 void SymbolFilePDB::InitializeObject() {
-  lldb::addr_t obj_load_address = m_obj_file->GetBaseAddress().GetFileAddress();
+  lldb::addr_t obj_load_address =
+  m_objfile_sp->GetBaseAddress().GetFileAddress();
   lldbassert(obj_load_address && obj_load_address != LLDB_INVALID_ADDRESS);
   m_session_up->setLoadAddress(obj_load_address);
   if (!m_global_scope_up)
@@ -1118,7 +1119,7 @@
   break;
 
 SymbolContext sc;
-sc.module_sp = m_obj_file->GetModule();
+sc.module_sp = m_objfile_sp->GetModule();
 lldbassert(sc.module_sp.get());
 
 if (!name.GetStringRef().equals(
@@ -1164,7 +1165,7 @@
 if (!regex.Execute(var_name))
   continue;
 SymbolContext sc;
-sc.module_sp = m_obj_file->GetModule();
+sc.module_sp = m_objfile_sp->GetModule();
 lldbassert(sc.module_sp.get());
 
 sc.comp_unit = ParseCompileUnitForUID(GetCompilandId(*pdb_data)).get();
@@ -1399,7 +1400,7 @@
   if (!results)
 return;
 
-  auto section_list = m_obj_file->GetSectionList();
+  auto section_list = m_objfile_sp->GetSection

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:141
+GetRegisterInfo_WoW64(const lldb_private::ArchSpec &arch) {
+  // A WoW64 register info is the same as the i386's.
+  std::vector &g_register_infos =

labath wrote:
> asmith wrote:
> > labath wrote:
> > > Hui wrote:
> > > > labath wrote:
> > > > > Why is all of this complexity necessary? Couldn't you just switch on 
> > > > > the target architecture in `CreateRegisterInfoInterface` in 
> > > > > NativeRegisterContextWindows_x86_64.cpp and create either 
> > > > > `RegisterContextWindows_WoW64` or `RegisterContextWindows_x86_64` ?
> > > > > 
> > > > > In fact, if RegisterContextWindows_WoW64 is identical to 
> > > > > RegisterContextWindows_i386, then why do you even need the _WoW64 
> > > > > version of the class in the first place? FWIW, linux also does not 
> > > > > have a RegisterContextLinux_32_bit_process_on_64_bit_kernel class...
> > > > I think WoW64 is i686 that shall deserve a separate arch specific 
> > > > implementation?
> > > I am sorry, but I don't follow. Can you repeat the question?
> > > 
> > > (FWIW, I don't believe that the fact that two things are different from a 
> > > certain point of view justifies copy-pasting (at least) 150 LOC. If you 
> > > think it's confusing to use something called "i386" in things dealing 
> > > with WoW64, you can always typedef the WOW64 context to it, or rename the 
> > > i386 context to something more generic.)
> > I don't want this to block the review and have removed the code.
> Thanks. FTR, I'm not opposed to splitting these classes again in the future, 
> if we develop a need for them to diverge (though it would be nice to find a 
> way to factor the common parts in that case).
> 
> However, there's one more thing that bothers me here. It's this business with 
> constructing a `RegisterContextWindows_i386` here, copying the entries out of 
> it, and re-using them elsewhere while asserting that things were done in the 
> right order. It seems very complex, and I'm not sure that complexity is 
> needed. It seems to me like all of that could be removed if you just made the 
> decision which set of registers to use one level up (i.e., in 
> `CreateRegisterInfoInterface` in NativeRegisterContextWindows_x86_64.cpp. You 
> could just have that function switch on the process byte size, and return 
> RegisterContextWindows_i386 or RegisterContextWindows_x86_64. This is the 
> same way things are done on x86 linux (see 
> NativeRegisterContextLinux_x86_64.cpp).
Unfortunately NativeRegisterContextLinux_x86_64 is not good example here. It 
turns I misread, and that class actually switches on the *host* architecture 
instead of *target*, and its implementation of RegisterContextLinux_x86_64 does 
indeed the i386 copying tricks that are very similar to what you do here. 
However, that class has the "excuse" of needing to fix up the register offsets 
in the i386-on-x86_64 case (for those following along: see 
`UPDATE_REGISTER_INFOS_I386_STRUCT_WITH_X86_64_OFFSETS`). You don't need to do 
anything like that here, so I would hope that we can do something simpler here.

And I'll try to figure out a way to do the linux thing in a saner fashion... 


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

https://reviews.llvm.org/D63165



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

Re: [Lldb-commits] [lldb] r367358 - [SymbolFile] SymbolFileDWARF::ParseLineTable should lock its module

[Pavel Labath via lldb-commits]

On 30/07/2019 23:22, Alex Langford via lldb-commits wrote:

Author: xiaobai
Date: Tue Jul 30 14:22:17 2019
New Revision: 367358

URL: http://llvm.org/viewvc/llvm-project?rev=367358&view=rev
Log:
[SymbolFile] SymbolFileDWARF::ParseLineTable should lock its module

As of svn rL367298, SymbolFileDWARF locks the module in many cases where
it needs to parse some aspect of the DWARF symbol file.
SymbolFileDWARF::ParseLineTable needs to lock the module because
SymbolVendor::ParseLineTable no longer locks it.



Thanks. I'm not sure how this managed to slip through.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r367407 - [lldb][docs] Add CMake version notes for -B flag

[Stefan Granitz via lldb-commits]

Author: stefan.graenitz
Date: Wed Jul 31 03:31:57 2019
New Revision: 367407

URL: http://llvm.org/viewvc/llvm-project?rev=367407&view=rev
Log:
[lldb][docs] Add CMake version notes for -B flag

The original documentation update was reviewed with D65330

Modified:
lldb/trunk/docs/_static/lldb.css
lldb/trunk/docs/resources/build.rst

Modified: lldb/trunk/docs/_static/lldb.css
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/_static/lldb.css?rev=367407&r1=367406&r2=367407&view=diff
==
--- lldb/trunk/docs/_static/lldb.css (original)
+++ lldb/trunk/docs/_static/lldb.css Wed Jul 31 03:31:57 2019
@@ -10,6 +10,14 @@ div.body {
   max-width: 90%;
 }
 
+div.note {
+  padding: 20px 20px 10px 20px;
+}
+
+div.note p.admonition-title {
+  font-size: 130%;
+}
+
 table.mapping {
   width: 100%;
 }

Modified: lldb/trunk/docs/resources/build.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/resources/build.rst?rev=367407&r1=367406&r2=367407&view=diff
==
--- lldb/trunk/docs/resources/build.rst (original)
+++ lldb/trunk/docs/resources/build.rst Wed Jul 31 03:31:57 2019
@@ -125,10 +125,10 @@ checked out above, but now we will have
 * the main build-tree for LLDB in ``/path/to/lldb-build``
 * a provided build-tree for LLVM and Clang in ``/path/to/llvm-build``
 
-Run CMake with ``-B`` pointing to a new directory for the provided build-tree
-and the positional argument pointing to the ``llvm`` directory in the
-source-tree. Note that we leave out LLDB here and only include Clang.
-Then we build the ``ALL`` target with ninja:
+Run CMake with ``-B`` pointing to a new directory for the provided
+build-tree\ :sup:`1` and the positional argument pointing to the ``llvm``
+directory in the source-tree. Note that we leave out LLDB here and only include
+Clang. Then we build the ``ALL`` target with ninja:
 
 ::
 
@@ -151,6 +151,11 @@ case-sensitive!):
   [] /path/to/llvm-project/lldb
   > ninja lldb
 
+.. note::
+
+   #. The ``-B`` argument was undocumented for a while and is only officially
+  supported since `CMake version 3.14
+  `_
 
 .. _CommonCMakeOptions:
 
@@ -321,6 +326,12 @@ Build LLDB standalone for development wi
   > open lldb.xcodeproj
   > cmake --build /path/to/lldb-build --target check-lldb
 
+.. note::
+
+   The ``-B`` argument was undocumented for a while and is only officially
+   supported since `CMake version 3.14
+   `_
+
 
 Building The Documentation
 --


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

[Lldb-commits] [lldb] r367410 - Add missing includes to SymbolFilePDBTests

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Jul 31 04:31:05 2019
New Revision: 367410

URL: http://llvm.org/viewvc/llvm-project?rev=367410&view=rev
Log:
Add missing includes to SymbolFilePDBTests

These became needed after r367368.

Modified:
lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt
lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Modified: lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt?rev=367410&r1=367409&r2=367410&view=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt Wed Jul 31 04:31:05 2019
@@ -9,6 +9,7 @@ add_lldb_unittest(SymbolFilePDBTests
 lldbPluginSymbolFileDWARF
 lldbPluginSymbolFilePDB
 lldbUtilityHelpers
+LLVMTestingSupport
   LINK_COMPONENTS
 Support
 DebugInfoPDB

Modified: lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp?rev=367410&r1=367409&r2=367410&view=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Wed Jul 31 
04:31:05 2019
@@ -13,6 +13,7 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
 
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"


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

[Lldb-commits] [lldb] r367411 - [lldb][NFC] Check in completion crash test in lambda

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Jul 31 04:33:16 2019
New Revision: 367411

URL: http://llvm.org/viewvc/llvm-project?rev=367411&view=rev
Log:
[lldb][NFC] Check in completion crash test in lambda

Added:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/main.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/Makefile?rev=367411&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/Makefile
 Wed Jul 31 04:33:16 2019
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py?rev=367411&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
 Wed Jul 31 04:33:16 2019
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), 
[decorators.skipIf(bugnumber="rdar://53755023")])

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/main.cpp?rev=367411&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/main.cpp
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/main.cpp
 Wed Jul 31 04:33:16 2019
@@ -0,0 +1,6 @@
+int main() {
+  []()
+  { //%self.dbg.GetCommandInterpreter().HandleCompletion("e ", len("e "), 0, 
-1, lldb.SBStringList())
+  }
+  ();
+}


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

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: amccarth, labath.
labath added a comment.

I think @amccarth kind of inherited the PDB stuff from Zach, but if you just 
need to try out a patch on windows, I can do that too. I was hoping I would be 
able to create a core file which you could open up on your end and reproduce 
the problem yourself, but unfortunately it seems that the Native PDB reader 
does not interact very well with core (minidump) files. This is a use case that 
both I and Adrian are interested in, so we will make that work eventually, but 
that might take a while...


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

https://reviews.llvm.org/D65414



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

[Lldb-commits] [lldb] r367413 - Add llvm-style RTTI to ObjectFile hierarchy

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Jul 31 04:57:34 2019
New Revision: 367413

URL: http://llvm.org/viewvc/llvm-project?rev=367413&view=rev
Log:
Add llvm-style RTTI to ObjectFile hierarchy

Summary:
On the heels of D62934, this patch uses the same approach to introduce
llvm RTTI support to the ObjectFile hierarchy. It also replaces the
existing uses of GetPluginName doing run-time type checks with
llvm::dyn_cast and friends.

This formally introduces new dependencies from some other plugins to
ObjectFile plugins. However, I believe this is fine because:
- these dependencies were already kind of there, and the only reason
  we could get away with not modeling them explicitly was because the
  code was relying on magically knowing what will GetPluginName() return
  for a particular kind of object files.
- the dependencies themselves are logical (it makes sense for
  SymbolVendorELF to depend on ObjectFileELF), or at least don't
  actively get in the way (the JitLoaderGDB->MachO thing).
- they don't introduce any new dependency loops as ObjectFile plugins
  don't depend on any other plugins

Reviewers: xiaobai, JDevlieghere, espindola

Subscribers: emaste, mgorny, arichardson, MaskRay, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Symbol/ObjectFile.h
lldb/trunk/source/Plugins/JITLoader/GDB/CMakeLists.txt
lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolVendor/ELF/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
lldb/trunk/source/Plugins/SymbolVendor/MacOSX/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/trunk/source/Symbol/ObjectFile.cpp

Modified: lldb/trunk/include/lldb/Symbol/ObjectFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ObjectFile.h?rev=367413&r1=367412&r2=367413&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ObjectFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/ObjectFile.h Wed Jul 31 04:57:34 2019
@@ -204,6 +204,10 @@ public:
   const char *path_with_object, lldb_private::FileSpec &archive_file,
   lldb_private::ConstString &archive_object, bool must_exist);
 
+  // LLVM RTTI support
+  static char ID;
+  virtual bool isA(const void *ClassID) const { return ClassID == &ID; }
+
   /// Gets the address size in bytes for the current object file.
   ///
   /// \return

Modified: lldb/trunk/source/Plugins/JITLoader/GDB/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/JITLoader/GDB/CMakeLists.txt?rev=367413&r1=367412&r2=367413&view=diff
==
--- lldb/trunk/source/Plugins/JITLoader/GDB/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/JITLoader/GDB/CMakeLists.txt Wed Jul 31 04:57:34 
2019
@@ -16,6 +16,7 @@ add_lldb_library(lldbPluginJITLoaderGDB
 lldbSymbol
 lldbTarget
 lldbUtility
+lldbPluginObjectFileMachO
   LINK_COMPONENTS
 Support
   )

Modified: lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp?rev=367413&r1=367412&r2=367413&view=diff
==
--- lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp (original)
+++ lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp Wed Jul 31 
04:57:34 2019
@@ -6,9 +6,8 @@
 //
 
//===--===//
 
-
-#include "llvm/Support/MathExtras.h"
-
+#include "JITLoaderGDB.h"
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
@@ -26,8 +25,7 @@
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
-
-#include "JITLoaderGDB.h"
+#include "llvm/Support/MathExtras.h"
 
 #include 
 
@@ -326,20 +324,16 @@ bool JITLoaderGDB::ReadJITDescriptorI

[Lldb-commits] [lldb] r367414 - [ProcessWindows] Choose a register context file by preprocessor

[Tatyana Krasnukha via lldb-commits]

Author: tkrasnukha
Date: Wed Jul 31 05:00:30 2019
New Revision: 367414

URL: http://llvm.org/viewvc/llvm-project?rev=367414&view=rev
Log:
[ProcessWindows] Choose a register context file by preprocessor

Replaced Cmake option based check with the preprocessor macro as 
CMAKE_SYSTEM_PROCESSOR doesn't work as expected on Windows.

Fixes llvm.org/pr42724

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

Modified:
lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt

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

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

lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp

lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h

Modified: lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt?rev=367414&r1=367413&r2=367414&view=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt Wed Jul 31 
05:00:30 2019
@@ -7,6 +7,9 @@ add_lldb_library(lldbPluginProcessWindow
   ProcessWindowsLog.cpp
   RegisterContextWindows.cpp
   TargetThreadWindows.cpp
+  x64/RegisterContextWindows_x64.cpp
+  x86/RegisterContextWindows_x86.cpp
+  # TODO add support for ARM (NT) and ARM64
 
   LINK_LIBS
 lldbCore
@@ -20,13 +23,3 @@ add_lldb_library(lldbPluginProcessWindow
   LINK_COMPONENTS
 Support
   )
-
-# TODO add support for ARM (NT) and ARM64
-# TODO build these unconditionally as we cannot do cross-debugging or WoW
-if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|AMD64")
-  target_sources(lldbPluginProcessWindowsCommon PRIVATE
-x64/RegisterContextWindows_x64.cpp)
-elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "i?86|X86")
-  target_sources(lldbPluginProcessWindowsCommon PRIVATE
-x86/RegisterContextWindows_x86.cpp)
-endif()

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=367414&r1=367413&r2=367414&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
 Wed Jul 31 05:00:30 2019
@@ -6,6 +6,8 @@
 //
 
//===--===//
 
+#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)
+
 #include "lldb/Host/windows/HostThreadWindows.h"
 #include "lldb/Host/windows/windows.h"
 #include "lldb/Utility/RegisterValue.h"
@@ -534,3 +536,5 @@ bool RegisterContextWindows_x64::WriteRe
   return ::SetThreadContext(
   wthread.GetHostThread().GetNativeThread().GetSystemHandle(), &m_context);
 }
+
+#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)

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=367414&r1=367413&r2=367414&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
 Wed Jul 31 05:00:30 2019
@@ -9,6 +9,8 @@
 #ifndef liblldb_RegisterContextWindows_x64_H_
 #define liblldb_RegisterContextWindows_x64_H_
 
+#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)
+
 #include "RegisterContextWindows.h"
 #include "lldb/lldb-forward.h"
 
@@ -40,4 +42,6 @@ public:
 };
 }
 
+#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
defined(_M_AMD64)
+
 #endif // #ifndef liblldb_RegisterContextWindows_x64_H_

Modified: 
lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp?rev=367414&r1=367413&r2=367414&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
 Wed Jul 31 05:00:30 2019
@@ -6,6 +6,8 @@
 //
 
//===--===//
 
+#if defined(__i386__) || defined(_M_IX86)

[Lldb-commits] [PATCH] D65450: Add llvm-style RTTI to ObjectFile hierarchy

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367413: Add llvm-style RTTI to ObjectFile hierarchy 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65450?vs=212342&id=212552#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65450

Files:
  lldb/trunk/include/lldb/Symbol/ObjectFile.h
  lldb/trunk/source/Plugins/JITLoader/GDB/CMakeLists.txt
  lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/trunk/source/Plugins/SymbolVendor/ELF/CMakeLists.txt
  lldb/trunk/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
  lldb/trunk/source/Plugins/SymbolVendor/MacOSX/CMakeLists.txt
  lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  lldb/trunk/source/Symbol/ObjectFile.cpp

Index: lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
===
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
@@ -48,6 +48,13 @@
 
   uint32_t GetPluginVersion() override { return 1; }
 
+  // LLVM RTTI support
+  static char ID;
+  bool isA(const void *ClassID) const override {
+return ClassID == &ID || ObjectFile::isA(ClassID);
+  }
+  static bool classof(const ObjectFile *obj) { return obj->isA(&ID); }
+
   // ObjectFile Protocol.
 
   bool ParseHeader() override;
Index: lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
@@ -42,6 +42,8 @@
   return Header{ArchSpec(triple), std::move(uuid)};
 }
 
+char ObjectFileBreakpad::ID;
+
 void ObjectFileBreakpad::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 GetPluginDescriptionStatic(), CreateInstance,
Index: lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
===
--- lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
+++ lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
@@ -46,6 +46,13 @@
 lldb::offset_t length,
 lldb_private::ModuleSpecList &specs);
 
+  // LLVM RTTI support
+  static char ID;
+  bool isA(const void *ClassID) const override {
+return ClassID == &ID || ObjectFile::isA(ClassID);
+  }
+  static bool classof(const ObjectFile *obj) { return obj->isA(&ID); }
+
   // Member Functions
   bool ParseHeader() override;
 
Index: lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
@@ -39,6 +39,8 @@
 using namespace lldb;
 using namespace lldb_private;
 
+char ObjectFileJIT::ID;
+
 void ObjectFileJIT::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 GetPluginDescriptionStatic(), CreateInstance,
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -89,6 +89,13 @@
 
   uint32_t GetPluginVersion() override;
 
+  // LLVM RTTI support
+  static char ID;
+  bool isA(const void *ClassID) const override {
+return ClassID == &ID || ObjectFile::isA(ClassID);
+  }
+  static bool classof(const ObjectFile *obj) { return obj->isA(&ID); }
+
   // ObjectFile Protocol.
   bool ParseHeader() override;
 
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/sou

[Lldb-commits] [PATCH] D65409: [ProcessWindows] Choose a register context file by prepocessor

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367414: [ProcessWindows] Choose a register context file by 
preprocessor (authored by tkrasnukha, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65409?vs=212363&id=212553#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65409

Files:
  lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
  
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
  
lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
  
lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
  
lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h

Index: lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
+++ lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
@@ -7,6 +7,9 @@
   ProcessWindowsLog.cpp
   RegisterContextWindows.cpp
   TargetThreadWindows.cpp
+  x64/RegisterContextWindows_x64.cpp
+  x86/RegisterContextWindows_x86.cpp
+  # TODO add support for ARM (NT) and ARM64
 
   LINK_LIBS
 lldbCore
@@ -20,13 +23,3 @@
   LINK_COMPONENTS
 Support
   )
-
-# TODO add support for ARM (NT) and ARM64
-# TODO build these unconditionally as we cannot do cross-debugging or WoW
-if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|AMD64")
-  target_sources(lldbPluginProcessWindowsCommon PRIVATE
-x64/RegisterContextWindows_x64.cpp)
-elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "i?86|X86")
-  target_sources(lldbPluginProcessWindowsCommon PRIVATE
-x86/RegisterContextWindows_x86.cpp)
-endif()
Index: lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
@@ -6,6 +6,8 @@
 //
 //===--===//
 
+#if defined(__i386__) || defined(_M_IX86)
+
 #include "lldb/Host/windows/HostThreadWindows.h"
 #include "lldb/Host/windows/windows.h"
 #include "lldb/Utility/RegisterValue.h"
@@ -282,3 +284,5 @@
   reg_value.SetUInt32(value);
   return true;
 }
+
+#endif // defined(__i386__) || defined(_M_IX86)
Index: lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h
+++ lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h
@@ -9,6 +9,8 @@
 #ifndef liblldb_RegisterContextWindows_x86_H_
 #define liblldb_RegisterContextWindows_x86_H_
 
+#if defined(__i386__) || defined(_M_IX86)
+
 #include "RegisterContextWindows.h"
 #include "lldb/lldb-forward.h"
 
@@ -44,4 +46,6 @@
 };
 }
 
+#endif // defined(__i386__) || defined(_M_IX86)
+
 #endif // #ifndef liblldb_RegisterContextWindows_x86_H_
Index: lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
+++ lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
@@ -9,6 +9,8 @@
 #ifndef liblldb_RegisterContextWindows_x64_H_
 #define liblldb_RegisterContextWindows_x64_H_
 
+#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || defined(_M_AMD64)
+
 #include "RegisterContextWindows.h"
 #include "lldb/lldb-forward.h"
 
@@ -40,4 +42,6 @@
 };
 }
 
+#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || defined(_M_AMD64)
+
 #endif // #ifndef liblldb_RegisterContextWindows_x64_H_
Index: lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
@@ -6,6 +6,8 @@
 //
 //===--===//
 
+#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || defined(_M_AMD64)
+
 #include "lldb/Host/windows/HostThreadWindows.h"
 #include "lldb/Host/windows/windows.h"
 #include "lldb/Utility/RegisterValue.h"
@@ -534,3 +536,5 @@
   return ::SetThreadContext(
   wthread.GetHostThread().GetNativeThread().GetSystemHandle(), &m_context);
 }
+
+#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || defined(_M_A

[Lldb-commits] [lldb] r367418 - Fix issues with inferior stdout coming out of order

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Jul 31 05:06:50 2019
New Revision: 367418

URL: http://llvm.org/viewvc/llvm-project?rev=367418&view=rev
Log:
Fix issues with inferior stdout coming out of order

Summary:
We've had a bug where two pieces of code, executing on two threads were
attempting to write inferior output simultaneously. The first one was in
Debugger::HandleProcessEvent, which handled the cases where stdout was
coming while the process was running. The second was in
CommandInterpreter::IOHandlerInputComplete, which was ensuring that any
output is printed before the command which caused process to run
terminates.

Both of these things make sense, but the fact they were implemented as
two independent functions without any synchronization meant that race
conditions could occur (e.g. both threads call process->GetSTDOUT, get
two chunks of data, but then end up calling stream->Write in opposite
order). This was most apparent in situations where a process quickly
writes a bunch of output and then exits (as all our register tests do).

This patch adds a mutex to ensure that stdout forwarding happens
atomically. It also refactors a code somewhat in order to reduce code
duplication.

Reviewers: clayborg, jingham

Subscribers: jfb, mgorny, lldb-commits

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

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

Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=367418&r1=367417&r2=367418&view=diff
==
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Wed Jul 31 05:06:50 2019
@@ -345,9 +345,10 @@ protected:
 
   void HandleThreadEvent(const lldb::EventSP &event_sp);
 
-  size_t GetProcessSTDOUT(Process *process, Stream *stream);
-
-  size_t GetProcessSTDERR(Process *process, Stream *stream);
+  // Ensures two threads don't attempt to flush process output in parallel.
+  std::mutex m_output_flush_mutex;
+  void FlushProcessOutput(Process &process, bool flush_stdout,
+  bool flush_stderr);
 
   SourceManager::SourceFileCache &GetSourceFileCache() {
 return m_source_file_cache;

Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h?rev=367418&r1=367417&r2=367418&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h Wed Jul 31 
05:06:50 2019
@@ -518,7 +518,7 @@ protected:
 
   bool IOHandlerInterrupt(IOHandler &io_handler) override;
 
-  size_t GetProcessOutput();
+  void GetProcessOutput();
 
   void SetSynchronous(bool value);
 

Modified: lldb/trunk/source/Core/Debugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=367418&r1=367417&r2=367418&view=diff
==
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Wed Jul 31 05:06:50 2019
@@ -1252,60 +1252,23 @@ void Debugger::HandleBreakpointEvent(con
   //}
 }
 
-size_t Debugger::GetProcessSTDOUT(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
-
-  if (stream) {
-//  The process has stuff waiting for stdout; get it and write it out to 
the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDOUT(stdio_buffer, sizeof(stdio_buffer),
-   error)) > 0) {
-stream->Write(stdio_buffer, len);
-total_bytes += len;
-  }
-}
-stream->Flush();
-  }
-  return total_bytes;
-}
-
-size_t Debugger::GetProcessSTDERR(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
-
-  if (stream) {
-//  The process has stuff waiting for stderr; get it and write it out to 
the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDERR(stdio_buffer, sizeof(stdio_buffer),
-

[Lldb-commits] [lldb] r367416 - [lldb][NFC] Check in another crashing test case

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Jul 31 05:06:22 2019
New Revision: 367416

URL: http://llvm.org/viewvc/llvm-project?rev=367416&view=rev
Log:
[lldb][NFC] Check in another crashing test case

Added:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/TestCompletionCrash2.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/main.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/Makefile?rev=367416&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/Makefile
 Wed Jul 31 05:06:22 2019
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/TestCompletionCrash2.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/TestCompletionCrash2.py?rev=367416&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/TestCompletionCrash2.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/TestCompletionCrash2.py
 Wed Jul 31 05:06:22 2019
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), 
[decorators.skipIf(bugnumber="rdar://53754063")])

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/main.cpp?rev=367416&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/main.cpp
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash2/main.cpp
 Wed Jul 31 05:06:22 2019
@@ -0,0 +1,11 @@
+namespace n {
+template  class a {};
+template  struct shared_ptr {
+  template 
+  static void make_shared() { 
//%self.dbg.GetCommandInterpreter().HandleCompletion("e ", len("e "), 0, -1, 
lldb.SBStringList())
+typedef a c;
+c d;
+  }
+};
+} // namespace n
+int main() { n::shared_ptr::make_shared(); }


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

[Lldb-commits] [PATCH] D65152: Fix issues with inferior stdout coming out of order

[Pavel Labath 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 rL367418: Fix issues with inferior stdout coming out of order 
(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/D65152/new/

https://reviews.llvm.org/D65152

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

Index: lldb/trunk/source/Core/Debugger.cpp
===
--- lldb/trunk/source/Core/Debugger.cpp
+++ lldb/trunk/source/Core/Debugger.cpp
@@ -1252,60 +1252,23 @@
   //}
 }
 
-size_t Debugger::GetProcessSTDOUT(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
-
-  if (stream) {
-//  The process has stuff waiting for stdout; get it and write it out to the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDOUT(stdio_buffer, sizeof(stdio_buffer),
-   error)) > 0) {
-stream->Write(stdio_buffer, len);
-total_bytes += len;
-  }
-}
-stream->Flush();
-  }
-  return total_bytes;
-}
-
-size_t Debugger::GetProcessSTDERR(Process *process, Stream *stream) {
-  size_t total_bytes = 0;
-  if (stream == nullptr)
-stream = GetOutputFile().get();
-
-  if (stream) {
-//  The process has stuff waiting for stderr; get it and write it out to the
-//  appropriate place.
-if (process == nullptr) {
-  TargetSP target_sp = GetTargetList().GetSelectedTarget();
-  if (target_sp)
-process = target_sp->GetProcessSP().get();
-}
-if (process) {
-  Status error;
-  size_t len;
-  char stdio_buffer[1024];
-  while ((len = process->GetSTDERR(stdio_buffer, sizeof(stdio_buffer),
-   error)) > 0) {
-stream->Write(stdio_buffer, len);
-total_bytes += len;
-  }
-}
-stream->Flush();
-  }
-  return total_bytes;
+void Debugger::FlushProcessOutput(Process &process, bool flush_stdout,
+  bool flush_stderr) {
+  const auto &flush = [&](Stream &stream,
+  size_t (Process::*get)(char *, size_t, Status &)) {
+Status error;
+size_t len;
+char buffer[1024];
+while ((len = (process.*get)(buffer, sizeof(buffer), error)) > 0)
+  stream.Write(buffer, len);
+stream.Flush();
+  };
+
+  std::lock_guard guard(m_output_flush_mutex);
+  if (flush_stdout)
+flush(*GetAsyncOutputStream(), &Process::GetSTDOUT);
+  if (flush_stderr)
+flush(*GetAsyncErrorStream(), &Process::GetSTDERR);
 }
 
 // This function handles events that were broadcast by the process.
@@ -1345,15 +1308,9 @@
   pop_process_io_handler);
 }
 
-// Now display and STDOUT
-if (got_stdout || got_state_changed) {
-  GetProcessSTDOUT(process_sp.get(), output_stream_sp.get());
-}
-
-// Now display and STDERR
-if (got_stderr || got_state_changed) {
-  GetProcessSTDERR(process_sp.get(), error_stream_sp.get());
-}
+// Now display STDOUT and STDERR
+FlushProcessOutput(*process_sp, got_stdout || got_state_changed,
+   got_stderr || got_state_changed);
 
 // Give structured data events an opportunity to display.
 if (got_structured_data) {
Index: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp
@@ -2664,32 +2664,14 @@
   }
 }
 
-size_t CommandInterpreter::GetProcessOutput() {
-  //  The process has stuff waiting for stderr; get it and write it out to the
-  //  appropriate place.
-  char stdio_buffer[1024];
-  size_t len;
-  size_t total_bytes = 0;
-  Status error;
+void CommandInterpreter::GetProcessOutput() {
   TargetSP target_sp(m_debugger.GetTargetList().GetSelectedTarget());
-  if (target_sp) {
-ProcessSP process_sp(target_sp->GetProcessSP());
-if (process_sp) {
-  while ((len = process_sp->GetSTDOUT(stdio_buffer, sizeof(stdio_buffer),
-  error)) > 0) {
-size_t bytes_written = len;
-m_debugger.GetOutputFile()->Write(stdio_buffer, bytes_written);
-total_bytes += len;
-  }
-  while ((len = process_s

[Lldb-commits] [lldb] r367420 - [lldb][NFC] Check in completion crash test case

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Jul 31 05:15:21 2019
New Revision: 367420

URL: http://llvm.org/viewvc/llvm-project?rev=367420&view=rev
Log:
[lldb][NFC] Check in completion crash test case

Added:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/main.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/Makefile?rev=367420&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/Makefile
 Wed Jul 31 05:15:21 2019
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py?rev=367420&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/TestCompletionCrashIncompleteRecord.py
 Wed Jul 31 05:15:21 2019
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), 
[decorators.skipIf(bugnumber="rdar://53756116")])

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/main.cpp?rev=367420&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/main.cpp
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/completion-crash-incomplete-record/main.cpp
 Wed Jul 31 05:15:21 2019
@@ -0,0 +1,11 @@
+int i;
+struct F {
+  int &r;
+  F() : r(i) {}
+};
+template  struct unique_ptr {
+  F i;
+  unique_ptr() : i() {//%self.dbg.GetCommandInterpreter().HandleCompletion("e 
", len("e "), 0, -1, lldb.SBStringList())
+}
+};
+int main() {unique_ptr u; }


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

[Lldb-commits] [PATCH] D65509: [lldb][CMake] Avoid 'Autogenerate scheme' dialogs in Xcode projects

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

sgraenitz created this revision.
sgraenitz added a reviewer: jingham.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

Supported in CMake 3.9 and higher: 
https://cmake.org/cmake/help/v3.9/variable/CMAKE_XCODE_GENERATE_SCHEME.html
Older versions will just report it as unused in the end of the configuration 
process.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65509

Files:
  lldb/cmake/caches/Apple-lldb-Xcode.cmake


Index: lldb/cmake/caches/Apple-lldb-Xcode.cmake
===
--- lldb/cmake/caches/Apple-lldb-Xcode.cmake
+++ lldb/cmake/caches/Apple-lldb-Xcode.cmake
@@ -2,3 +2,4 @@
 
 set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
 set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(CMAKE_XCODE_GENERATE_SCHEME ON CACHE BOOL "")


Index: lldb/cmake/caches/Apple-lldb-Xcode.cmake
===
--- lldb/cmake/caches/Apple-lldb-Xcode.cmake
+++ lldb/cmake/caches/Apple-lldb-Xcode.cmake
@@ -2,3 +2,4 @@
 
 set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
 set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(CMAKE_XCODE_GENERATE_SCHEME ON CACHE BOOL "")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65435: SymbolVendor: Introduce Module::GetSymbolFile

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Just add a few null checks and this will be good to go.




Comment at: source/Commands/CommandObjectTarget.cpp:2240
 Module *m = target->GetImages().GetModulePointerAtIndex(image_idx);
-SymbolFile *sf = m->GetSymbolVendor()->GetSymbolFile();
-sf->DumpClangAST(result.GetOutputStream());
+m->GetSymbolFile()->DumpClangAST(result.GetOutputStream());
   }

check m->GetSymbolFile() for NULL



Comment at: source/Commands/CommandObjectTarget.cpp:2265
 Module *m = module_list.GetModulePointerAtIndex(i);
-SymbolFile *sf = m->GetSymbolVendor()->GetSymbolFile();
-sf->DumpClangAST(result.GetOutputStream());
+m->GetSymbolFile()->DumpClangAST(result.GetOutputStream());
   }

check m->GetSymbolFile() for NULL


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

https://reviews.llvm.org/D65435



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

[Lldb-commits] [PATCH] D65435: SymbolVendor: Introduce Module::GetSymbolFile

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Alternatively we can switch to using a reference for:

  SymbolFile &Module::GetSymbolFile(bool can_create Stream *feedback_strm);

As I believe we always fall back to SymbolFileSymtab don't we?


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

https://reviews.llvm.org/D65435



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

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

IMHO: we should keep this command and expand its abilities to help report 
stepping issues, expression issues and more. Why? Getting good bug reports from 
users is quite hard. Allowing them to type "bugreport step --step-in" or 
"bugreport step --step-over" would be really nice. Right now I send people a 
python script that enables logging, dumps the code in and around the source 
line with disassembly, and creates a zip file with the current object file and 
optional debug info file (dSYM or external symbol file).

Happy to hear what others have to say though.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65469



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

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

I will let Jason comment on the unwind specifics since this is his area. I 
caught a few other things that need to be cleaned up.




Comment at: lldb/include/lldb/Target/Unwind.h:40
 uint32_t idx;
+bool behaves_like_zeroth_frame;
 

initialize with value just in case.



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:153-155
+  AddressRange addr_range;
+  m_sym_ctx_valid = TryResolveSymbolContextAndAddressRange(
+  m_current_pc, m_sym_ctx, addr_range);

See comment for RegisterContextLLDB::TryResolveSymbolContextAndAddressRange, 
but if we change as suggested this will become:
```
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, nullptr);
```



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:430-432
+  AddressRange addr_range;
+  m_sym_ctx_valid = TryResolveSymbolContextAndAddressRange(
+  m_current_pc, m_sym_ctx, addr_range);

See comment for RegisterContextLLDB::TryResolveSymbolContextAndAddressRange, 
but if we change as suggested this will become:
```
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, nullptr);
```



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:486-487
 m_sym_ctx.Clear(false);
-m_sym_ctx_valid = false;
-SymbolContextItem resolve_scope =
-eSymbolContextFunction | eSymbolContextSymbol;
-
-ModuleSP temporary_module_sp = temporary_pc.GetModule();
-if (temporary_module_sp &&
-temporary_module_sp->ResolveSymbolContextForAddress(
-temporary_pc, resolve_scope, m_sym_ctx) &
-resolve_scope) {
-  if (m_sym_ctx.GetAddressRange(resolve_scope, 0, false, addr_range))
-m_sym_ctx_valid = true;
-}
+m_sym_ctx_valid = TryResolveSymbolContextAndAddressRange(
+temporary_pc, m_sym_ctx, addr_range);
+

See comment for RegisterContextLLDB::TryResolveSymbolContextAndAddressRange, 
but if we change as suggested this will become:
```
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
```



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:595-624
+bool RegisterContextLLDB::TryResolveSymbolContextAndAddressRange(
+lldb_private::Address pc, lldb_private::SymbolContext &sym_ctx,
+lldb_private::AddressRange &addr_range) {
+  ModuleSP pc_module_sp = pc.GetModule();
+
+  // Can't resolve context without a module.
+  if (!pc_module_sp)

This function doesn't belong in RegisterContextLLDB. It think 
lldb_private::Address would be a better place:

```
/// if "addr_range_ptr" is not NULL, then fill in with the address range of the 
function.
bool lldb_private::Address::ResolveFunctionScope(lldb_private::SymbolContext 
&sym_ctx, lldb_private::AddressRange *addr_range_ptr) {
  constexpr bool resolve_tail_call_address = false;
  constexpr SymbolContextItem resolve_scope = eSymbolContextFunction | 
eSymbolContextSymbol;
  if (!CalculateSymbolContext(&sym_ctx, resolve_scope))
   return false;
  if (!addr_range_ptr)
return true;
  return sym_ctx.GetAddressRange(resolve_scope, 0, false, *addr_range_ptr);  
}
```



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1760
+void RegisterContextLLDB::PropagateTrapHandlerFlag(
+lldb::UnwindPlanSP unwind_plan) {
+  if (unwind_plan->GetUnwindPlanForSignalTrap() != eLazyBoolYes) {

JosephTremoulet wrote:
> I'm a bit ambivalent about adding this part -- the downside is that it's not 
> concretely helping today, because if an eh_frame record has the 'S' flag in 
> its augmentation (which is what `unwind_Plan->GetUnwindPlanForSignalTrap()` 
> reports), we'll have already decremented pc and generated unwind plans based 
> on the prior function when initializing the register context.  But the upside 
> is that this connects the dots between the otherwise-unused bit on the unwind 
> plan and the frame type, and will be in place should we subsequently add code 
> to try the second function's unwind plan as a fallback.
I will let Jason comment on this one.



Comment at: lldb/source/Target/StackFrameList.cpp:453
 lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
+bool behaves_like_zeroth_frame;
 if (idx == 0) {

init with value



Comment at: lldb/source/Target/StackFrameList.cpp:670
 addr_t pc, cfa;
-if (unwinder->GetFrameInfoAtIndex(idx, cfa, pc)) {
+bool behaves_like_zeroth_frame;
+if (unwinder->GetFrameInfoAtIndex(idx, cfa, pc,

init with value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993

[Lldb-commits] [PATCH] D65435: SymbolVendor: Introduce Module::GetSymbolFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D65435#1608276 , @clayborg wrote:

> Alternatively we can switch to using a reference for:
>
>   SymbolFile &Module::GetSymbolFile(bool can_create Stream *feedback_strm);
>
>
> As I believe we always fall back to SymbolFileSymtab don't we?


That is mostly true, but not 100% true in all circumstances. If we manage to 
create an object file for the module, then yes, I believe we should also always 
be able to construct a symbol file. However, we currently can run into 
situations where we fail to create an object file for a module (because the 
constructor does not check that). Instead what people usually do is that after 
constructing a module object, they call GetObjectFile() to check whether the 
module is really valid.

One of my ideas for the future is to change how modules are constructed, so 
that once we get a module object, we can assume that it will always contain a 
valid ObjectFile. At that point we would be able to assume the existence of 
SymbolFile. Until then, I can add the null checks you requested, though I don't 
think they can be really triggered now as those objectfile-less modules should 
not end up in the module list of a target.


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

https://reviews.llvm.org/D65435



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

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I haven't looked at the implementation, but I did run into the problem you are 
fixing in the past, and I am happy that someone is finally implementing this. I 
just wanted to say that for testing, I think you should be able to create some 
hand-written assembly that creates the kind of stack frames and unwind info you 
need to trigger this. You can look at existing tests in `lldb/lit/Unwind` for 
examples.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993



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

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Joseph Tremoulet via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:595-624
+bool RegisterContextLLDB::TryResolveSymbolContextAndAddressRange(
+lldb_private::Address pc, lldb_private::SymbolContext &sym_ctx,
+lldb_private::AddressRange &addr_range) {
+  ModuleSP pc_module_sp = pc.GetModule();
+
+  // Can't resolve context without a module.
+  if (!pc_module_sp)

clayborg wrote:
> This function doesn't belong in RegisterContextLLDB. It think 
> lldb_private::Address would be a better place:
> 
> ```
> /// if "addr_range_ptr" is not NULL, then fill in with the address range of 
> the function.
> bool lldb_private::Address::ResolveFunctionScope(lldb_private::SymbolContext 
> &sym_ctx, lldb_private::AddressRange *addr_range_ptr) {
>   constexpr bool resolve_tail_call_address = false;
>   constexpr SymbolContextItem resolve_scope = eSymbolContextFunction | 
> eSymbolContextSymbol;
>   if (!CalculateSymbolContext(&sym_ctx, resolve_scope))
>return false;
>   if (!addr_range_ptr)
> return true;
>   return sym_ctx.GetAddressRange(resolve_scope, 0, false, *addr_range_ptr);  
> }
> ```
Sure, will update.  Just to double-check a couple points:

 #  I'll actually want to pass `&addr_range` at all three callsites, rather 
than passing `nullptr` at two of them, correct? (at the callsite on 154 to 
preserve the `addr_range` previously defined on 175, and at the callsite on 
431to preserve the `addr_range` previously defined on 470)

 # I see you removed the `resolve_scope & resolved_scope` check.  Am I correct 
that that was intentional and it's redundant with the checks in 
`GetAddressRange` that require `function` or `symbol` to be non-null, or is 
there something more subtle going on here?

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993



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

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:595-624
+bool RegisterContextLLDB::TryResolveSymbolContextAndAddressRange(
+lldb_private::Address pc, lldb_private::SymbolContext &sym_ctx,
+lldb_private::AddressRange &addr_range) {
+  ModuleSP pc_module_sp = pc.GetModule();
+
+  // Can't resolve context without a module.
+  if (!pc_module_sp)

JosephTremoulet wrote:
> clayborg wrote:
> > This function doesn't belong in RegisterContextLLDB. It think 
> > lldb_private::Address would be a better place:
> > 
> > ```
> > /// if "addr_range_ptr" is not NULL, then fill in with the address range of 
> > the function.
> > bool 
> > lldb_private::Address::ResolveFunctionScope(lldb_private::SymbolContext 
> > &sym_ctx, lldb_private::AddressRange *addr_range_ptr) {
> >   constexpr bool resolve_tail_call_address = false;
> >   constexpr SymbolContextItem resolve_scope = eSymbolContextFunction | 
> > eSymbolContextSymbol;
> >   if (!CalculateSymbolContext(&sym_ctx, resolve_scope))
> >return false;
> >   if (!addr_range_ptr)
> > return true;
> >   return sym_ctx.GetAddressRange(resolve_scope, 0, false, *addr_range_ptr); 
> >  
> > }
> > ```
> Sure, will update.  Just to double-check a couple points:
> 
>  #  I'll actually want to pass `&addr_range` at all three callsites, rather 
> than passing `nullptr` at two of them, correct? (at the callsite on 154 to 
> preserve the `addr_range` previously defined on 175, and at the callsite on 
> 431to preserve the `addr_range` previously defined on 470)
> 
>  # I see you removed the `resolve_scope & resolved_scope` check.  Am I 
> correct that that was intentional and it's redundant with the checks in 
> `GetAddressRange` that require `function` or `symbol` to be non-null, or is 
> there something more subtle going on here?
> 
> Thanks.
For 1) above, it seems like you were making a new local variable just so you 
could pass it. If it is actually used somewhere, then yes, do include it. It 
seemed like you were just making a new local for no reason.

For 2) above: You can restore the resolve_scope & resolved_scope if needed. 
Can't remember if we will return eSymbolContextModule if we fail to find 
eSymbolContextFunction or eSymbolContextSymbol. So problably safest to restore 
that so we don't change functionality


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993



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

[Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Wed Jul 31 09:24:55 2019
New Revision: 367441

URL: http://llvm.org/viewvc/llvm-project?rev=367441&view=rev
Log:
Don't crash when pass by value struct has no definition.


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

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=367441&r1=367440&r2=367441&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Wed Jul 
31 09:24:55 2019
@@ -1010,7 +1010,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
 if (attrs.calling_convention == llvm::dwarf::DW_CC_pass_by_value) {
   clang::CXXRecordDecl *record_decl =
   m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-  if (record_decl) {
+  if (record_decl && record_decl->getDefinition()) {
 record_decl->setHasTrivialSpecialMemberForCall();
   }
 }


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

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65469#1608278 , @clayborg wrote:

> IMHO: we should keep this command and expand its abilities to help report 
> stepping issues, expression issues and more. Why? Getting good bug reports 
> from users is quite hard. Allowing them to type "bugreport step --step-in" or 
> "bugreport step --step-over" would be really nice. Right now I send people a 
> python script that enables logging, dumps the code in and around the source 
> line with disassembly, and creates a zip file with the current object file 
> and optional debug info file (dSYM or external symbol file).


What's the benefit of having this instead of having the user generate a 
reproducer?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65469



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

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D65469#1608631 , @JDevlieghere 
wrote:

> In D65469#1608278 , @clayborg wrote:
>
> > IMHO: we should keep this command and expand its abilities to help report 
> > stepping issues, expression issues and more. Why? Getting good bug reports 
> > from users is quite hard. Allowing them to type "bugreport step --step-in" 
> > or "bugreport step --step-over" would be really nice. Right now I send 
> > people a python script that enables logging, dumps the code in and around 
> > the source line with disassembly, and creates a zip file with the current 
> > object file and optional debug info file (dSYM or external symbol file).
>
>
> What's the benefit of having this instead of having the user generate a 
> reproducer?


I am guessing the reproducer wouldn't help us with any of the user files. We 
often need to see the binaries, or enable some logging when the user repros the 
bug. Users don't often attach all of the files that are needed, nor do they 
know what logs to enable and how to attach them. I am not up to date on exactly 
what reproducers do, but I would guess that they don't save off just the 
required files or enable any logging and attach logs?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65469



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

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

I am able to do some testing as well.


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

https://reviews.llvm.org/D65414



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

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 212615.
JosephTremoulet added a comment.

- Move resolution helper from RegisterContextLLDB to lldb_private::Address
- Defensively initialize out arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993

Files:
  lldb/include/lldb/Core/Address.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/Target/Unwind.h
  lldb/source/Core/Address.cpp
  lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
  lldb/source/Plugins/Process/Utility/HistoryUnwind.h
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
  lldb/source/Plugins/Process/Utility/UnwindLLDB.cpp
  lldb/source/Plugins/Process/Utility/UnwindLLDB.h
  lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
  lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/StackFrameList.cpp

Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -394,11 +394,13 @@
 bool cfa_is_valid = false;
 addr_t pc =
 callee->GetAddressRange().GetBaseAddress().GetLoadAddress(&target);
+constexpr bool behaves_like_zeroth_frame = false;
 SymbolContext sc;
 callee->CalculateSymbolContext(&sc);
 auto synth_frame = std::make_shared(
 m_thread.shared_from_this(), frame_idx, concrete_frame_idx, cfa,
-cfa_is_valid, pc, StackFrame::Kind::Artificial, &sc);
+cfa_is_valid, pc, StackFrame::Kind::Artificial,
+behaves_like_zeroth_frame, &sc);
 m_frames.push_back(synth_frame);
 LLDB_LOG(log, "Pushed frame {0}", callee->GetDisplayName());
   }
@@ -448,6 +450,7 @@
 uint32_t idx = m_concrete_frames_fetched++;
 lldb::addr_t pc = LLDB_INVALID_ADDRESS;
 lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
+bool behaves_like_zeroth_frame = (idx == 0);
 if (idx == 0) {
   // We might have already created frame zero, only create it if we need
   // to.
@@ -455,8 +458,9 @@
 RegisterContextSP reg_ctx_sp(m_thread.GetRegisterContext());
 
 if (reg_ctx_sp) {
-  const bool success =
-  unwinder && unwinder->GetFrameInfoAtIndex(idx, cfa, pc);
+  const bool success = unwinder &&
+   unwinder->GetFrameInfoAtIndex(
+   idx, cfa, pc, behaves_like_zeroth_frame);
   // There shouldn't be any way not to get the frame info for frame
   // 0. But if the unwinder can't make one, lets make one by hand
   // with the SP as the CFA and see if that gets any further.
@@ -467,7 +471,7 @@
 
   unwind_frame_sp = std::make_shared(
   m_thread.shared_from_this(), m_frames.size(), idx, reg_ctx_sp,
-  cfa, pc, nullptr);
+  cfa, pc, behaves_like_zeroth_frame, nullptr);
   m_frames.push_back(unwind_frame_sp);
 }
   } else {
@@ -475,8 +479,9 @@
 cfa = unwind_frame_sp->m_id.GetCallFrameAddress();
   }
 } else {
-  const bool success =
-  unwinder && unwinder->GetFrameInfoAtIndex(idx, cfa, pc);
+  const bool success = unwinder &&
+   unwinder->GetFrameInfoAtIndex(
+   idx, cfa, pc, behaves_like_zeroth_frame);
   if (!success) {
 // We've gotten to the end of the stack.
 SetAllFramesFetched();
@@ -485,7 +490,7 @@
   const bool cfa_is_valid = true;
   unwind_frame_sp = std::make_shared(
   m_thread.shared_from_this(), m_frames.size(), idx, cfa, cfa_is_valid,
-  pc, StackFrame::Kind::Regular, nullptr);
+  pc, StackFrame::Kind::Regular, behaves_like_zeroth_frame, nullptr);
 
   // Create synthetic tail call frames between the previous frame and the
   // newly-found frame. The new frame's index may change after this call,
@@ -527,10 +532,11 @@
   while (unwind_sc.GetParentOfInlinedScope(
   curr_frame_address, next_frame_sc, next_frame_address)) {
 next_frame_sc.line_entry.ApplyFileMappings(target_sp);
-StackFrameSP frame_sp(
-new StackFrame(m_thread.shared_from_this(), m_frames.size(), idx,
-   unwind_frame_sp->GetRegisterContextSP(), cfa,
-   next_frame_address, &next_frame_sc));
+behaves_like_zeroth_frame = false;
+StackFrameSP frame_sp(new StackFrame(
+m_thread.shared_from_this(), m_frames.size(), idx,
+unwind_frame_sp->GetRegisterContextSP(), cfa, next_frame_address,
+behaves_like_zeroth_frame, &next_frame_sc));
 
 m_frames.push_back(frame_sp);
 unwind_sc = next_frame_sc;
@@ -661,11 +667,13 @@
   Unwind *unwinder = m_thread.GetU

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Code looks fine now. Now Jason will need to chime in with comments on the new 
unwind functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993



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

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65469#1608653 , @clayborg wrote:

> In D65469#1608631 , @JDevlieghere 
> wrote:
>
> > In D65469#1608278 , @clayborg 
> > wrote:
> >
> > > IMHO: we should keep this command and expand its abilities to help report 
> > > stepping issues, expression issues and more. Why? Getting good bug 
> > > reports from users is quite hard. Allowing them to type "bugreport step 
> > > --step-in" or "bugreport step --step-over" would be really nice. Right 
> > > now I send people a python script that enables logging, dumps the code in 
> > > and around the source line with disassembly, and creates a zip file with 
> > > the current object file and optional debug info file (dSYM or external 
> > > symbol file).
> >
> >
> > What's the benefit of having this instead of having the user generate a 
> > reproducer?
>
>
> I am guessing the reproducer wouldn't help us with any of the user files. We 
> often need to see the binaries, or enable some logging when the user repros 
> the bug. Users don't often attach all of the files that are needed, nor do 
> they know what logs to enable and how to attach them. I am not up to date on 
> exactly what reproducers do, but I would guess that they don't save off just 
> the required files or enable any logging and attach logs?


That's exactly what the reproducers do actually. They capture user interaction, 
the GDB remote packets and any files used by the debugger. They currently don't 
enable logs, because you can debug LLDB during reproducer replay, but that 
would be trivial to include as well.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65469



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

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The thing the "bugreport unwind" adds is that it runs a handful of commands 
that gather data useful in diagnosing unwind problems.  That's useful when the 
information you need might not be output by the session in which the bug 
appeared.

The same thing could be achieved by having "reproducer generate" take arguments 
that the user can specify to automate gathering more data that would be 
relevant to a particular kind of bug.  So "bugreport unwind" -> "reproducer 
generate unwind" which would run the same extra commands that "bugreport 
unwind" does (the results will get captured by the same mechanism that's 
catching all the other session output) and then the reproducer would be 
generated.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65469



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

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

If we don't want to forget that there was specific useful info, we could gate 
this patch on implementing the "reproducer generate " feature.  That 
should be easy to add if we're clear this this is the right design for 
reproducers.  But I don't think that "bugreport unwind" was all that much used, 
so it might be better to design this at leisure.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65469



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

[Lldb-commits] [PATCH] D65498: Fix completion for functions in anonymous namespaces

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65498#1607819 , @teemperor wrote:

> I think it's possible to rewrite the line when doing a completion, but I 
> remember that I got annoyed when I tried understand the code responsible for 
> that (and decipher the magic return values). Can you file a radar maybe and 
> I'll fix this once I got around to refactor the related code? This patch can 
> go IMHO, so LGTM.


I think we could solve this issue by storing more information with the 
candidates, like a bit that says that we should clear (part) of the current 
line. However, currently candidates are stored in StringLists, which do not 
have such capabilities. The part that rewrites the line should be relatively 
straightforward.

I've filed rdar://53769355. I might come back to it myself as I'd like to have 
this non-prefix completion functionality.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65498



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

[Lldb-commits] [lldb] r367455 - Fix completion for functions in anonymous namespaces

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Jul 31 10:58:00 2019
New Revision: 367455

URL: http://llvm.org/viewvc/llvm-project?rev=367455&view=rev
Log:
Fix completion for functions in anonymous namespaces

I was going through some of the old bugs and came across PR21069 which I
was able to reproduce. The issue is that we match the regex `^foo`
against the `DW_AT_name` in the DWARF, which for our anonymous function
is indeed `foo`. However, when we get the function name from the symbol
context, the result is `(anonymous namespace)::foo()`. This throws off
completions, which assumes that it's appending to whatever is already
present on the input, resulting in a bogus
`b fooonymous\ namespace)::foo()`.

Bug report: https://llvm.org/PR21069

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
lldb/trunk/source/Commands/CommandCompletions.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=367455&r1=367454&r2=367455&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
 Wed Jul 31 10:58:00 2019
@@ -297,3 +297,6 @@ class CommandLineCompletionTestCase(Test
 self.complete_from_to('breakpoint set -n Fo',
   'breakpoint set -n Foo::Bar(int,\\ int)',
   turn_off_re_match=True)
+# No completion for Qu because the candidate is
+# (anonymous namespace)::Quux().
+self.complete_from_to('breakpoint set -n Qu', '')

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp?rev=367455&r1=367454&r2=367455&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp 
Wed Jul 31 10:58:00 2019
@@ -7,6 +7,8 @@ public:
 }
 };
 
+namespace { int Quux (void) { return 0; } }
+
 struct Container { int MemberVar; };
 
 int main()
@@ -17,5 +19,6 @@ int main()
 
 Container container;
 Container *ptr_container = &container;
+int q = Quux();
 return container.MemberVar = 3; // Break here
 }

Modified: lldb/trunk/source/Commands/CommandCompletions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandCompletions.cpp?rev=367455&r1=367454&r2=367455&view=diff
==
--- lldb/trunk/source/Commands/CommandCompletions.cpp (original)
+++ lldb/trunk/source/Commands/CommandCompletions.cpp Wed Jul 31 10:58:00 2019
@@ -471,7 +471,11 @@ Searcher::CallbackReturn CommandCompleti
 for (uint32_t i = 0; i < sc_list.GetSize(); i++) {
   if (sc_list.GetContextAtIndex(i, sc)) {
 ConstString func_name = sc.GetFunctionName(Mangled::ePreferDemangled);
-if (!func_name.IsEmpty())
+// Ensure that the function name matches the regex. This is more than a
+// sanity check. It is possible that the demangled function name does
+// not start with the prefix, for example when it's in an anonymous
+// namespace.
+if (!func_name.IsEmpty() && m_regex.Execute(func_name.GetStringRef()))
   m_match_set.insert(func_name);
   }
 }


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

Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

[Raphael Isemann via lldb-commits]

It seems that patch is lacking a test (which doesn't seem too hard to provide).

Am Mi., 31. Juli 2019 um 18:24 Uhr schrieb Greg Clayton via
lldb-commits :
>
> Author: gclayton
> Date: Wed Jul 31 09:24:55 2019
> New Revision: 367441
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367441&view=rev
> Log:
> Don't crash when pass by value struct has no definition.
>
>
> Modified:
> lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>
> Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=367441&r1=367440&r2=367441&view=diff
> ==
> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
> (original)
> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Wed 
> Jul 31 09:24:55 2019
> @@ -1010,7 +1010,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
>  if (attrs.calling_convention == llvm::dwarf::DW_CC_pass_by_value) {
>clang::CXXRecordDecl *record_decl =
>m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
> -  if (record_decl) {
> +  if (record_decl && record_decl->getDefinition()) {
>  record_decl->setHasTrivialSpecialMemberForCall();
>}
>  }
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65498: Fix completion for functions in anonymous namespaces

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367455: Fix completion for functions in anonymous namespaces 
(authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65498?vs=212505&id=212627#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65498

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
  lldb/trunk/source/Commands/CommandCompletions.cpp


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
@@ -7,6 +7,8 @@
 }
 };
 
+namespace { int Quux (void) { return 0; } }
+
 struct Container { int MemberVar; };
 
 int main()
@@ -17,5 +19,6 @@
 
 Container container;
 Container *ptr_container = &container;
+int q = Quux();
 return container.MemberVar = 3; // Break here
 }
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
@@ -297,3 +297,6 @@
 self.complete_from_to('breakpoint set -n Fo',
   'breakpoint set -n Foo::Bar(int,\\ int)',
   turn_off_re_match=True)
+# No completion for Qu because the candidate is
+# (anonymous namespace)::Quux().
+self.complete_from_to('breakpoint set -n Qu', '')
Index: lldb/trunk/source/Commands/CommandCompletions.cpp
===
--- lldb/trunk/source/Commands/CommandCompletions.cpp
+++ lldb/trunk/source/Commands/CommandCompletions.cpp
@@ -471,7 +471,11 @@
 for (uint32_t i = 0; i < sc_list.GetSize(); i++) {
   if (sc_list.GetContextAtIndex(i, sc)) {
 ConstString func_name = sc.GetFunctionName(Mangled::ePreferDemangled);
-if (!func_name.IsEmpty())
+// Ensure that the function name matches the regex. This is more than a
+// sanity check. It is possible that the demangled function name does
+// not start with the prefix, for example when it's in an anonymous
+// namespace.
+if (!func_name.IsEmpty() && m_regex.Execute(func_name.GetStringRef()))
   m_match_set.insert(func_name);
   }
 }


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/main.cpp
@@ -7,6 +7,8 @@
 }
 };
 
+namespace { int Quux (void) { return 0; } }
+
 struct Container { int MemberVar; };
 
 int main()
@@ -17,5 +19,6 @@
 
 Container container;
 Container *ptr_container = &container;
+int q = Quux();
 return container.MemberVar = 3; // Break here
 }
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
@@ -297,3 +297,6 @@
 self.complete_from_to('breakpoint set -n Fo',
   'breakpoint set -n Foo::Bar(int,\\ int)',
   turn_off_re_match=True)
+# No completion for Qu because the candidate is
+# (anonymous namespace)::Quux().
+self.complete_from_to('breakpoint set -n Qu', '')
Index: lldb/trunk/source/Commands/CommandCompletions.cpp
===
--- lldb/trunk/source/Commands/CommandCompletions.cpp
+++ lldb/trunk/source/Commands/CommandCompletions.cpp
@@ -471,7 +471,11 @@
 for (uint32_t i = 0; i < sc_list.GetSize(); i++) {
   if (sc_list.GetContextAtIndex(i, sc)) {
 ConstString func_name = sc.GetFunctionName(Mangled::ePreferDemangled);
-if (!func_name.IsEmpty())
+// Ensure that the function name matches the regex. This is more than a
+// sanity check. It is possible that the demangled function name does
+// not start with the prefix, for example when it's in an anonymous
+// namespace.
+if (!func_name.IsEmpty() && m_regex.Execute

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65469#1608737 , @jingham wrote:

> If we don't want to forget that there was specific useful info, we could gate 
> this patch on implementing the "reproducer generate " feature.  That 
> should be easy to add if we're clear this this is the right design for 
> reproducers.  But I don't think that "bugreport unwind" was all that much 
> used, so it might be better to design this at leisure.


Until the reproducers prove to be insufficient to capture a particular bug, I 
don't plan to invest in this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65469



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

[Lldb-commits] [lldb] r367459 - [CommandCompletions] Remove commented out code.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Jul 31 11:09:26 2019
New Revision: 367459

URL: http://llvm.org/viewvc/llvm-project?rev=367459&view=rev
Log:
[CommandCompletions] Remove commented out code.

We use version control here.

Modified:
lldb/trunk/include/lldb/Interpreter/CommandCompletions.h

Modified: lldb/trunk/include/lldb/Interpreter/CommandCompletions.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandCompletions.h?rev=367459&r1=367458&r2=367459&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/CommandCompletions.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandCompletions.h Wed Jul 31 
11:09:26 2019
@@ -179,14 +179,6 @@ public:
 size_t DoCompletion(SearchFilter *filter) override;
 
   private:
-//struct NameCmp {
-//bool operator() (const ConstString& lhs, const ConstString&
-//rhs) const
-//{
-//return lhs < rhs;
-//}
-//};
-
 RegularExpression m_regex;
 typedef std::set collection;
 collection m_match_set;


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

Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

[Greg Clayton via lldb-commits]

> On Jul 31, 2019, at 10:57 AM, Raphael Isemann  wrote:
> 
> It seems that patch is lacking a test (which doesn't seem too hard to 
> provide).

I am not the original author of this patch that was causing the crash, just 
fixing a crash that was introduced by the patch. 

I am all ears for anyone that can provide me with DWARF to help reproduce this 
scenario where we have a DW_CC_pass_by_value struct with no definition. Not 
sure how you would have a compiler that is passing a struct to a function as a 
parameter and yet does not emit debug info for that struct it is clearly using 
in the debug info.

> 
> Am Mi., 31. Juli 2019 um 18:24 Uhr schrieb Greg Clayton via
> lldb-commits :
>> 
>> Author: gclayton
>> Date: Wed Jul 31 09:24:55 2019
>> New Revision: 367441
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=367441&view=rev
>> Log:
>> Don't crash when pass by value struct has no definition.
>> 
>> 
>> Modified:
>>lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>> 
>> Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=367441&r1=367440&r2=367441&view=diff
>> ==
>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
>> (original)
>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Wed 
>> Jul 31 09:24:55 2019
>> @@ -1010,7 +1010,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
>> if (attrs.calling_convention == llvm::dwarf::DW_CC_pass_by_value) {
>>   clang::CXXRecordDecl *record_decl =
>>   m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
>> -  if (record_decl) {
>> +  if (record_decl && record_decl->getDefinition()) {
>> record_decl->setHasTrivialSpecialMemberForCall();
>>   }
>> }
>> 
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

Re: [Lldb-commits] [lldb] r367385 - [CompletionRequest] Remove unimplemented members.

[Jim Ingham via lldb-commits]

This wasn't a "weird thing".  

When you have a match with potentially many hits, it would be very handy to say 
"give me the first 10", then "give me the next 10" so you only do the work to 
fetch as much as your paginator would show.  That would allow us to avoid 
unnecessary work when somebody gets a huge number of hits because they 
accidentally tried to complete "a" or something...  Right now we stop to gather 
all the matches and then paginate the full list so it can take a long time to 
get back control so you can then say "I don't want to see any more matches, I 
hit  by accident".

Passing in a start index and num_elements is the natural way to express this.

The problem was that none of the searches in lldb that produce long lists of 
matches were designed so the that you can resume the search midway along, and 
so it never had any useful clients.  OTOH, we haven't produced any such 
clients, and it seems like we aren't ever going to be able to use it, so I'm 
fine with removing the feature.

Jim

> On Jul 31, 2019, at 12:01 AM, Raphael “Teemperor” Isemann via lldb-commits 
>  wrote:
> 
> Thanks! Wasn’t sure back then if I can just remove that weird thing
> 
>> On Jul 31, 2019, at 5:48 AM, Jonas Devlieghere via lldb-commits 
>>  wrote:
>> 
>> Author: jdevlieghere
>> Date: Tue Jul 30 20:48:29 2019
>> New Revision: 367385
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=367385&view=rev
>> Log:
>> [CompletionRequest] Remove unimplemented members.
>> 
>> Completion requests have two fields that are essentially unimplemented:
>> `m_match_start_point` and `m_max_return_elements`. This would've been
>> okay, if it wasn't for the fact that this caused a bunch of useless
>> parameters to be passed around. Occasionally there would be a comment or
>> assert saying that they are not supported. This patch removes them.
>> 
>> Modified:
>>   lldb/trunk/include/lldb/Core/IOHandler.h
>>   lldb/trunk/include/lldb/Expression/REPL.h
>>   lldb/trunk/include/lldb/Host/Editline.h
>>   lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
>>   lldb/trunk/include/lldb/Utility/CompletionRequest.h
>>   lldb/trunk/source/API/SBCommandInterpreter.cpp
>>   lldb/trunk/source/Core/FormatEntity.cpp
>>   lldb/trunk/source/Core/IOHandler.cpp
>>   lldb/trunk/source/Expression/REPL.cpp
>>   lldb/trunk/source/Host/common/Editline.cpp
>>   lldb/trunk/source/Interpreter/CommandInterpreter.cpp
>>   lldb/trunk/source/Utility/CompletionRequest.cpp
>>   lldb/trunk/unittests/Utility/CompletionRequestTest.cpp
>> 
>> Modified: lldb/trunk/include/lldb/Core/IOHandler.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/IOHandler.h?rev=367385&r1=367384&r2=367385&view=diff
>> ==
>> --- lldb/trunk/include/lldb/Core/IOHandler.h (original)
>> +++ lldb/trunk/include/lldb/Core/IOHandler.h Tue Jul 30 20:48:29 2019
>> @@ -200,7 +200,6 @@ public:
>> 
>>  virtual int IOHandlerComplete(IOHandler &io_handler, const char 
>> *current_line,
>>const char *cursor, const char *last_char,
>> -int skip_first_n_matches, int max_matches,
>>StringList &matches, StringList 
>> &descriptions);
>> 
>>  virtual const char *IOHandlerGetFixIndentationCharacters() { return 
>> nullptr; }
>> @@ -417,7 +416,6 @@ private:
>> 
>>  static int AutoCompleteCallback(const char *current_line, const char 
>> *cursor,
>>  const char *last_char,
>> -  int skip_first_n_matches, int max_matches,
>>  StringList &matches, StringList 
>> &descriptions,
>>  void *baton);
>> #endif
>> @@ -452,7 +450,6 @@ public:
>> 
>>  int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
>>const char *cursor, const char *last_char,
>> -int skip_first_n_matches, int max_matches,
>>StringList &matches, StringList &descriptions) 
>> override;
>> 
>>  void IOHandlerInputComplete(IOHandler &io_handler,
>> 
>> Modified: lldb/trunk/include/lldb/Expression/REPL.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/REPL.h?rev=367385&r1=367384&r2=367385&view=diff
>> ==
>> --- lldb/trunk/include/lldb/Expression/REPL.h (original)
>> +++ lldb/trunk/include/lldb/Expression/REPL.h Tue Jul 30 20:48:29 2019
>> @@ -105,7 +105,6 @@ public:
>> 
>>  int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
>>const char *cursor, const char *last_char,
>> -int skip_first_n_matches, int max_matches,
>>StringList &matches, StringList &descriptions) 
>> override;
>> 
>> protected:
>> 
>> Modified: lldb/trunk/include/lldb/Host/Edit

Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

[Davide Italiano via lldb-commits]

On Wed, Jul 31, 2019 at 11:29 AM Greg Clayton via lldb-commits
 wrote:
>
>
>
> > On Jul 31, 2019, at 10:57 AM, Raphael Isemann  wrote:
> >
> > It seems that patch is lacking a test (which doesn't seem too hard to 
> > provide).
>
> I am not the original author of this patch that was causing the crash, just 
> fixing a crash that was introduced by the patch.
>

Which patch was causing the crash?

> I am all ears for anyone that can provide me with DWARF to help reproduce 
> this scenario where we have a DW_CC_pass_by_value struct with no definition. 
> Not sure how you would have a compiler that is passing a struct to a function 
> as a parameter and yet does not emit debug info for that struct it is clearly 
> using in the debug info.
>

I assume this reproduced somewhere(TM) and you verified that your
patch fixes the bug? You can probably attach the binary/dSYM/* and we
can work from there?

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

[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65489#1607833 , @labath wrote:

> In D65489#1607807 , @JDevlieghere 
> wrote:
>
> > In D65489#1607801 , @labath wrote:
> >
> > > Why do we need a separate backend for this? Couldn't this be emitted as a 
> > > part of the same `#include "XXXProperties.inc"` which defines the 
> > > property definition? The two logically belong together, and the only 
> > > reason they were separate variables in the first place was the 
> > > limitations of constexpr global variables...
> >
> >
> > I'm not sure what you mean by that last sentence, maybe because I don't 
> > know the history? Can you elaborate a bit on the "separate variables" part?
> >
> > To answer your question: the `OptionEnumValueElement`s are used both by 
> > properties and command options. Technically it is possible to put them in 
> > the same `.td` files, and emit them in the same `.inc` file. The actual 
> > code wouldn't change, just the way we invoke: instead of being its own 
> > backend, it would be called from both existing backends. I don't have a 
> > strong preference for either approach.
>
>
> Ah, I'm sorry. Somehow I didn't get what the " used by both properties and 
> options" part means. I do now, and I agree that in this case, it does not 
> make sense to generate these as a part of the properties definition.
>
> However, I'm not sure it then makes sense to generate them at all. I mean, 
> it's not like there's any redundancy (like it was for option definitions) in 
> the `OptionEnumValueElement` struct, and it's pretty obvious what the 
> individual fields mean even without naming them. And the tablegenning 
> definitely increases the barrier for understanding the code. Is there any 
> follow-up work or cleanups that would not be possible if this just stays a 
> plain C array?


I don't have any cleanups planned for now. My motivation is purely aesthetical: 
I don't like the `// clang-format off` markers and think the C arrays look 
messy with the multiline oddly broken up strings. Anyway, I just mention it for 
context. I don't want to push this through if the consensus is that this is 
overkill.


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

https://reviews.llvm.org/D65489



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

[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D65489#1608904 , @JDevlieghere 
wrote:

> I don't have any cleanups planned for now. My motivation is purely 
> aesthetical: I don't like the `// clang-format off` markers and think the C 
> arrays look messy with the multiline oddly broken up strings. Anyway, I just 
> mention it for context. I don't want to push this through if the consensus is 
> that this is overkill.


Yeah, the C arrays aren't the prettiest sight, but OTOH your tablegen files 
don't respect the column limit, and so if you view them with a line-wrapping 
editor (such as this phabricator page), they don't look particularly nice 
either.

I'm not sure the result is better, but it is possible to control the way 
clang-format lays out arrays like this via trailing commas. If you include a 
trailing comma, it will put each entry on a separate line, which may be better 
for those long strings (though that's highly subjective). E.g.:

  static constexpr OptionEnumValueElement s_stop_show_column_values[] = {
  {
  eStopShowColumnAnsiOrCaret,
  "ansi-or-caret",
  "Highlight the stop column with ANSI terminal codes when color/ANSI "
  "mode is enabled; otherwise, fall back to using a text-only caret (^) 
"
  "as if \"caret-only\" mode was selected.",
  },
  {
  eStopShowColumnAnsi,
  "ansi",
  "Highlight the stop column with ANSI terminal codes when running LLDB 
"
  "with color/ANSI enabled.",
  },
  {
  eStopShowColumnCaret,
  "caret",
  "Highlight the stop column with a caret character (^) underneath the "
  "stop column. This method introduces a new line in source listings "
  "that display thread stop locations.",
  },
  {
  eStopShowColumnNone,
  "none",
  "Do not highlight the stop column.",
  },
  };

instead of:

  static constexpr OptionEnumValueElement s_stop_show_column_values[] = {
  {eStopShowColumnAnsiOrCaret, "ansi-or-caret",
   "Highlight the stop column with ANSI terminal codes when color/ANSI mode 
"
   "is enabled; otherwise, fall back to using a text-only caret (^) as if "
   "\"caret-only\" mode was selected."},
  {eStopShowColumnAnsi, "ansi",
   "Highlight the stop column with ANSI terminal codes when running LLDB "
   "with color/ANSI enabled."},
  {eStopShowColumnCaret, "caret",
   "Highlight the stop column with a caret character (^) underneath the 
stop "
   "column. This method introduces a new line in source listings that "
   "display thread stop locations."},
  {eStopShowColumnNone, "none", "Do not highlight the stop column."}};


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

https://reviews.llvm.org/D65489



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

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 212642.
JosephTremoulet added a comment.

- add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993

Files:
  lldb/include/lldb/Core/Address.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/Target/Unwind.h
  lldb/lit/Unwind/Inputs/trap_frame_sym_ctx.s
  lldb/lit/Unwind/trap_frame_sym_ctx.test
  lldb/source/Core/Address.cpp
  lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
  lldb/source/Plugins/Process/Utility/HistoryUnwind.h
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
  lldb/source/Plugins/Process/Utility/UnwindLLDB.cpp
  lldb/source/Plugins/Process/Utility/UnwindLLDB.h
  lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
  lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/StackFrameList.cpp

Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -394,11 +394,13 @@
 bool cfa_is_valid = false;
 addr_t pc =
 callee->GetAddressRange().GetBaseAddress().GetLoadAddress(&target);
+constexpr bool behaves_like_zeroth_frame = false;
 SymbolContext sc;
 callee->CalculateSymbolContext(&sc);
 auto synth_frame = std::make_shared(
 m_thread.shared_from_this(), frame_idx, concrete_frame_idx, cfa,
-cfa_is_valid, pc, StackFrame::Kind::Artificial, &sc);
+cfa_is_valid, pc, StackFrame::Kind::Artificial,
+behaves_like_zeroth_frame, &sc);
 m_frames.push_back(synth_frame);
 LLDB_LOG(log, "Pushed frame {0}", callee->GetDisplayName());
   }
@@ -448,6 +450,7 @@
 uint32_t idx = m_concrete_frames_fetched++;
 lldb::addr_t pc = LLDB_INVALID_ADDRESS;
 lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
+bool behaves_like_zeroth_frame = (idx == 0);
 if (idx == 0) {
   // We might have already created frame zero, only create it if we need
   // to.
@@ -455,8 +458,9 @@
 RegisterContextSP reg_ctx_sp(m_thread.GetRegisterContext());
 
 if (reg_ctx_sp) {
-  const bool success =
-  unwinder && unwinder->GetFrameInfoAtIndex(idx, cfa, pc);
+  const bool success = unwinder &&
+   unwinder->GetFrameInfoAtIndex(
+   idx, cfa, pc, behaves_like_zeroth_frame);
   // There shouldn't be any way not to get the frame info for frame
   // 0. But if the unwinder can't make one, lets make one by hand
   // with the SP as the CFA and see if that gets any further.
@@ -467,7 +471,7 @@
 
   unwind_frame_sp = std::make_shared(
   m_thread.shared_from_this(), m_frames.size(), idx, reg_ctx_sp,
-  cfa, pc, nullptr);
+  cfa, pc, behaves_like_zeroth_frame, nullptr);
   m_frames.push_back(unwind_frame_sp);
 }
   } else {
@@ -475,8 +479,9 @@
 cfa = unwind_frame_sp->m_id.GetCallFrameAddress();
   }
 } else {
-  const bool success =
-  unwinder && unwinder->GetFrameInfoAtIndex(idx, cfa, pc);
+  const bool success = unwinder &&
+   unwinder->GetFrameInfoAtIndex(
+   idx, cfa, pc, behaves_like_zeroth_frame);
   if (!success) {
 // We've gotten to the end of the stack.
 SetAllFramesFetched();
@@ -485,7 +490,7 @@
   const bool cfa_is_valid = true;
   unwind_frame_sp = std::make_shared(
   m_thread.shared_from_this(), m_frames.size(), idx, cfa, cfa_is_valid,
-  pc, StackFrame::Kind::Regular, nullptr);
+  pc, StackFrame::Kind::Regular, behaves_like_zeroth_frame, nullptr);
 
   // Create synthetic tail call frames between the previous frame and the
   // newly-found frame. The new frame's index may change after this call,
@@ -527,10 +532,11 @@
   while (unwind_sc.GetParentOfInlinedScope(
   curr_frame_address, next_frame_sc, next_frame_address)) {
 next_frame_sc.line_entry.ApplyFileMappings(target_sp);
-StackFrameSP frame_sp(
-new StackFrame(m_thread.shared_from_this(), m_frames.size(), idx,
-   unwind_frame_sp->GetRegisterContextSP(), cfa,
-   next_frame_address, &next_frame_sc));
+behaves_like_zeroth_frame = false;
+StackFrameSP frame_sp(new StackFrame(
+m_thread.shared_from_this(), m_frames.size(), idx,
+unwind_frame_sp->GetRegisterContextSP(), cfa, next_frame_address,
+behaves_like_zeroth_frame, &next_frame_sc));
 
 m_frames.push_back(frame_sp);
 unwind_sc = next_frame_sc;
@@ -661,11 +667,13 @@
   Unwind *unwinder = m_thread.GetUnwinder();

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet added a comment.

In D64993#1608452 , @labath wrote:

> I haven't looked at the implementation, but I did run into the problem you 
> are fixing in the past, and I am happy that someone is finally implementing 
> this. I just wanted to say that for testing, I think you should be able to 
> create some hand-written assembly that creates the kind of stack frames and 
> unwind info you need to trigger this. You can look at existing tests in 
> `lldb/lit/Unwind` for examples.


Thanks for the tip!  I was indeed able to copy one of those and adapt it to 
test this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993



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

Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

[via lldb-commits]

> -Original Message-
> From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf
> Of Greg Clayton via lldb-commits
> Sent: Wednesday, July 31, 2019 2:29 PM
> To: Raphael Isemann
> Cc: lldb-commits
> Subject: Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by
> value struct has no definition.
> 
> 
> 
> > On Jul 31, 2019, at 10:57 AM, Raphael Isemann 
> wrote:
> >
> > It seems that patch is lacking a test (which doesn't seem too hard to
> provide).
> 
> I am not the original author of this patch that was causing the crash,
> just fixing a crash that was introduced by the patch.
> 
> I am all ears for anyone that can provide me with DWARF to help reproduce
> this scenario where we have a DW_CC_pass_by_value struct with no
> definition. Not sure how you would have a compiler that is passing a
> struct to a function as a parameter and yet does not emit debug info for
> that struct it is clearly using in the debug info.

One wonders how you discovered the crash.

I know that Clang will skip the content of a type if it determines that
the definition is certain to be found elsewhere.  For example, if it has 
virtual methods and the key function is not defined in the current CU.  
Whether it will still add a CC attribute, I don't know.
--paulr

> 
> >
> > Am Mi., 31. Juli 2019 um 18:24 Uhr schrieb Greg Clayton via
> > lldb-commits :
> >>
> >> Author: gclayton
> >> Date: Wed Jul 31 09:24:55 2019
> >> New Revision: 367441
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=367441&view=rev
> >> Log:
> >> Don't crash when pass by value struct has no definition.
> >>
> >>
> >> Modified:
> >>lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
> >>
> >> Modified:
> lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
> >> URL: http://llvm.org/viewvc/llvm-
> project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
> ?rev=367441&r1=367440&r2=367441&view=diff
> >>
> ==
> 
> >> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
> (original)
> >> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
> Wed Jul 31 09:24:55 2019
> >> @@ -1010,7 +1010,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
> >> if (attrs.calling_convention == llvm::dwarf::DW_CC_pass_by_value) {
> >>   clang::CXXRecordDecl *record_decl =
> >>   m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
> >> -  if (record_decl) {
> >> +  if (record_decl && record_decl->getDefinition()) {
> >> record_decl->setHasTrivialSpecialMemberForCall();
> >>   }
> >> }
> >>
> >>
> >> ___
> >> lldb-commits mailing list
> >> lldb-commits@lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65489#1608936 , @labath wrote:

> In D65489#1608904 , @JDevlieghere 
> wrote:
>
> > I don't have any cleanups planned for now. My motivation is purely 
> > aesthetical: I don't like the `// clang-format off` markers and think the C 
> > arrays look messy with the multiline oddly broken up strings. Anyway, I 
> > just mention it for context. I don't want to push this through if the 
> > consensus is that this is overkill.
>
>
> Yeah, the C arrays aren't the prettiest sight, but OTOH your tablegen files 
> don't respect the column limit, and so if you view them with a line-wrapping 
> editor (such as this phabricator page), they don't look particularly nice 
> either.


True, but at least you can grep without having to guess where the ling break 
might have ended up :-)

> I'm not sure the result is better, but it is possible to control the way 
> clang-format lays out arrays like this via trailing commas. If you include a 
> trailing comma, it will put each entry on a separate line, which may be 
> better for those long strings (though that's highly subjective). E.g.:
> 
>   static constexpr OptionEnumValueElement s_stop_show_column_values[] = {
>   {
>   eStopShowColumnAnsiOrCaret,
>   "ansi-or-caret",
>   "Highlight the stop column with ANSI terminal codes when color/ANSI 
> "
>   "mode is enabled; otherwise, fall back to using a text-only caret 
> (^) "
>   "as if \"caret-only\" mode was selected.",
>   },
>   {
>   eStopShowColumnAnsi,
>   "ansi",
>   "Highlight the stop column with ANSI terminal codes when running 
> LLDB "
>   "with color/ANSI enabled.",
>   },
>   {
>   eStopShowColumnCaret,
>   "caret",
>   "Highlight the stop column with a caret character (^) underneath 
> the "
>   "stop column. This method introduces a new line in source listings "
>   "that display thread stop locations.",
>   },
>   {
>   eStopShowColumnNone,
>   "none",
>   "Do not highlight the stop column.",
>   },
>   };

This looks much, much better imho. I can live with reformatting them if we 
don't want to go the tablegen route.


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

https://reviews.llvm.org/D65489



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

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Harlan Haskins via Phabricator via lldb-commits]

harlanhaskins created this revision.
harlanhaskins added reviewers: arphaman, bruno.
Herald added subscribers: lldb-commits, cfe-commits, jsji, kadircet, 
dexonsmith, jkorous, MaskRay, kbarton, nemanjai.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.
Herald added projects: clang, LLDB.
harlanhaskins edited the summary of this revision.
Herald added subscribers: wuzish, ormris, rnkovacs.

Currently, clang's FileManager uses NULL as an indicator that a particular file
did not exist, but would not propagate errors like permission issues. Instead,
teach FileManager to use llvm::ErrorOr internally and return rich errors for
failures.

This patch updates the existing API and updates the callers throughout clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65534

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
  clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
  clang-tools-extra/clang-move/Move.cpp
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/modularize/ModularizeUtilities.cpp
  clang/include/clang/Basic/FileManager.h
  clang/lib/ARCMigrate/FileRemapper.cpp
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/Module.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/Rewrite/FrontendActions.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/lib/Serialization/ModuleManager.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Refactoring.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/clang-refactor/TestSupport.cpp
  clang/tools/clang-rename/ClangRename.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/RewriterTestContext.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }
Index: clang/unittests/Tooling/RewriterTestContext.h
===
--- clang/unittests/Tooling/RewriterTestContext.h
+++ clang/unittests/Tooling/RewriterTestContext

[Lldb-commits] [lldb] r367480 - [API] Remove use of ClangASTContext from SBTarget

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Wed Jul 31 13:47:38 2019
New Revision: 367480

URL: http://llvm.org/viewvc/llvm-project?rev=367480&view=rev
Log:
[API] Remove use of ClangASTContext from SBTarget

Summary:
The methods to find types in a Target aren't clang specific and are
pretty generalizable to type systems. Additionally, to support some of
the use cases in SBTarget, I've added a "GetScratchTypeSystems" method
to Target to support getting all type systems for a target we are
debugging.

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

Modified:
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Target/Target.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=367480&r1=367479&r2=367480&view=diff
==
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Wed Jul 31 13:47:38 2019
@@ -1026,6 +1026,8 @@ public:
   GetScratchTypeSystemForLanguage(lldb::LanguageType language,
   bool create_on_demand = true);
 
+  std::vector GetScratchTypeSystems(bool create_on_demand = 
true);
+
   PersistentExpressionState *
   GetPersistentExpressionStateForLanguage(lldb::LanguageType language);
 

Modified: lldb/trunk/source/API/SBTarget.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBTarget.cpp?rev=367480&r1=367479&r2=367480&view=diff
==
--- lldb/trunk/source/API/SBTarget.cpp (original)
+++ lldb/trunk/source/API/SBTarget.cpp Wed Jul 31 13:47:38 2019
@@ -44,11 +44,11 @@
 #include "lldb/Core/ValueObjectList.h"
 #include "lldb/Core/ValueObjectVariable.h"
 #include "lldb/Host/Host.h"
-#include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/DeclVendor.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
+#include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/Language.h"
@@ -1858,11 +1858,11 @@ lldb::SBType SBTarget::FindFirstType(con
 }
 
 // No matches, search for basic typename matches
-ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
-if (clang_ast)
-  return LLDB_RECORD_RESULT(SBType(ClangASTContext::GetBasicType(
-  clang_ast->getASTContext(), const_typename)));
+for (auto *type_system : target_sp->GetScratchTypeSystems())
+  if (auto type = type_system->GetBuiltinTypeByName(const_typename))
+return LLDB_RECORD_RESULT(SBType(type));
   }
+
   return LLDB_RECORD_RESULT(SBType());
 }
 
@@ -1872,10 +1872,9 @@ SBType SBTarget::GetBasicType(lldb::Basi
 
   TargetSP target_sp(GetSP());
   if (target_sp) {
-ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
-if (clang_ast)
-  return LLDB_RECORD_RESULT(SBType(
-  ClangASTContext::GetBasicType(clang_ast->getASTContext(), type)));
+for (auto *type_system : target_sp->GetScratchTypeSystems())
+  if (auto compiler_type = type_system->GetBasicTypeFromAST(type))
+return LLDB_RECORD_RESULT(SBType(compiler_type));
   }
   return LLDB_RECORD_RESULT(SBType());
 }
@@ -1918,10 +1917,10 @@ lldb::SBTypeList SBTarget::FindTypes(con
 
 if (sb_type_list.GetSize() == 0) {
   // No matches, search for basic typename matches
-  ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
-  if (clang_ast)
-sb_type_list.Append(SBType(ClangASTContext::GetBasicType(
-clang_ast->getASTContext(), const_typename)));
+  for (auto *type_system : target_sp->GetScratchTypeSystems())
+if (auto compiler_type =
+type_system->GetBuiltinTypeByName(const_typename))
+  sb_type_list.Append(SBType(compiler_type));
 }
   }
   return LLDB_RECORD_RESULT(sb_type_list);

Modified: lldb/trunk/source/Target/Target.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=367480&r1=367479&r2=367480&view=diff
==
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Wed Jul 31 13:47:38 2019
@@ -2153,6 +2153,34 @@ Target::GetScratchTypeSystemForLanguage(
 create_on_demand);
 }
 
+std::vector Target::GetScratchTypeSystems(bool create_on_demand) 
{
+  if (!m_valid)
+return {};
+
+  std::vector scratch_type_systems;
+
+  std::set languages_for_types;
+  std::set languages_for_expressions;
+
+  Language::GetLanguagesSupportingTypeSystems(languages_for_types,
+  languages_for_expressions);
+
+  for (auto lang : languages_for_expressions) {
+auto type_system_or_err 

[Lldb-commits] [lldb] r367481 - [GDBRemote] Reflow comments and improve docs.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Jul 31 13:47:44 2019
New Revision: 367481

URL: http://llvm.org/viewvc/llvm-project?rev=367481&view=rev
Log:
[GDBRemote] Reflow comments and improve docs.

Improved the GDB client base documentation while I was reading through
it. Looks like it got messed up during the automatic comment reflow a
while ago.

Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h?rev=367481&r1=367480&r2=367481&view=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h 
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h Wed Jul 
31 13:47:44 2019
@@ -24,12 +24,10 @@ public:
 virtual void HandleAsyncMisc(llvm::StringRef data) = 0;
 virtual void HandleStopReply() = 0;
 
-// 
=
 /// Process asynchronously-received structured data.
 ///
 /// \param[in] data
 ///   The complete data packet, expected to start with JSON-async.
-// 
=
 virtual void HandleAsyncStructuredDataPacket(llvm::StringRef data) = 0;
   };
 
@@ -83,42 +81,48 @@ protected:
   virtual void OnRunPacketSent(bool first);
 
 private:
-  // Variables handling synchronization between the Continue thread and any
-  // other threads
-  // wishing to send packets over the connection. Either the continue thread 
has
-  // control over
-  // the connection (m_is_running == true) or the connection is free for an
-  // arbitrary number of
-  // other senders to take which indicate their interest by incrementing
-  // m_async_count.
-  // Semantics of individual states:
-  // - m_continue_packet == false, m_async_count == 0: connection is free
-  // - m_continue_packet == true, m_async_count == 0: only continue thread is
-  // present
-  // - m_continue_packet == true, m_async_count > 0: continue thread has
-  // control, async threads
-  //   should interrupt it and wait for it to set m_continue_packet to false
-  // - m_continue_packet == false, m_async_count > 0: async threads have
-  // control, continue
-  //   thread needs to wait for them to finish (m_async_count goes down to 0).
+  /// Variables handling synchronization between the Continue thread and any
+  /// other threads wishing to send packets over the connection. Either the
+  /// continue thread has control over the connection (m_is_running == true) or
+  /// the connection is free for an arbitrary number of other senders to take
+  /// which indicate their interest by incrementing m_async_count.
+  ///
+  /// Semantics of individual states:
+  ///
+  /// - m_continue_packet == false, m_async_count == 0:
+  ///   connection is free
+  /// - m_continue_packet == true, m_async_count == 0:
+  ///   only continue thread is present
+  /// - m_continue_packet == true, m_async_count > 0:
+  ///   continue thread has control, async threads should interrupt it and wait
+  ///   for it to set m_continue_packet to false
+  /// - m_continue_packet == false, m_async_count > 0:
+  ///   async threads have control, continue thread needs to wait for them to
+  ///   finish (m_async_count goes down to 0).
+  /// @{
   std::mutex m_mutex;
   std::condition_variable m_cv;
-  // Packet with which to resume after an async interrupt. Can be changed by an
-  // async thread
-  // e.g. to inject a signal.
+
+  /// Packet with which to resume after an async interrupt. Can be changed by
+  /// an async thread e.g. to inject a signal.
   std::string m_continue_packet;
-  // When was the interrupt packet sent. Used to make sure we time out if the
-  // stub does not
-  // respond to interrupt requests.
+
+  /// When was the interrupt packet sent. Used to make sure we time out if the
+  /// stub does not respond to interrupt requests.
   std::chrono::time_point m_interrupt_time;
+
+  /// Number of threads interested in sending.
   uint32_t m_async_count;
+
+  /// Whether the continue thread has control.
   bool m_is_running;
-  bool m_should_stop; // Whether we should resume after a stop.
-  // end of continue thread synchronization block
 
-  // This handles the synchronization between individual async threads. For now
-  // they just use a
-  // simple mutex.
+  /// Whether we should resume after a stop.
+  bool m_should_stop;
+  /// @}
+
+  /// This handles the synchronization between individual async threads. For
+  /// now they just use a simple mutex.
   std::recursive_mutex m_async_mutex;
 
   bool ShouldStop(const UnixSignals &signals,


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

[Lldb-commits] [PATCH] D64964: [API] Remove use of ClangASTContext from SBTarget

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367480: [API] Remove use of ClangASTContext from SBTarget 
(authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64964

Files:
  lldb/trunk/include/lldb/Target/Target.h
  lldb/trunk/source/API/SBTarget.cpp
  lldb/trunk/source/Target/Target.cpp

Index: lldb/trunk/source/API/SBTarget.cpp
===
--- lldb/trunk/source/API/SBTarget.cpp
+++ lldb/trunk/source/API/SBTarget.cpp
@@ -44,11 +44,11 @@
 #include "lldb/Core/ValueObjectList.h"
 #include "lldb/Core/ValueObjectVariable.h"
 #include "lldb/Host/Host.h"
-#include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/DeclVendor.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
+#include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/Language.h"
@@ -1858,11 +1858,11 @@
 }
 
 // No matches, search for basic typename matches
-ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
-if (clang_ast)
-  return LLDB_RECORD_RESULT(SBType(ClangASTContext::GetBasicType(
-  clang_ast->getASTContext(), const_typename)));
+for (auto *type_system : target_sp->GetScratchTypeSystems())
+  if (auto type = type_system->GetBuiltinTypeByName(const_typename))
+return LLDB_RECORD_RESULT(SBType(type));
   }
+
   return LLDB_RECORD_RESULT(SBType());
 }
 
@@ -1872,10 +1872,9 @@
 
   TargetSP target_sp(GetSP());
   if (target_sp) {
-ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
-if (clang_ast)
-  return LLDB_RECORD_RESULT(SBType(
-  ClangASTContext::GetBasicType(clang_ast->getASTContext(), type)));
+for (auto *type_system : target_sp->GetScratchTypeSystems())
+  if (auto compiler_type = type_system->GetBasicTypeFromAST(type))
+return LLDB_RECORD_RESULT(SBType(compiler_type));
   }
   return LLDB_RECORD_RESULT(SBType());
 }
@@ -1918,10 +1917,10 @@
 
 if (sb_type_list.GetSize() == 0) {
   // No matches, search for basic typename matches
-  ClangASTContext *clang_ast = target_sp->GetScratchClangASTContext();
-  if (clang_ast)
-sb_type_list.Append(SBType(ClangASTContext::GetBasicType(
-clang_ast->getASTContext(), const_typename)));
+  for (auto *type_system : target_sp->GetScratchTypeSystems())
+if (auto compiler_type =
+type_system->GetBuiltinTypeByName(const_typename))
+  sb_type_list.Append(SBType(compiler_type));
 }
   }
   return LLDB_RECORD_RESULT(sb_type_list);
Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -2153,6 +2153,34 @@
 create_on_demand);
 }
 
+std::vector Target::GetScratchTypeSystems(bool create_on_demand) {
+  if (!m_valid)
+return {};
+
+  std::vector scratch_type_systems;
+
+  std::set languages_for_types;
+  std::set languages_for_expressions;
+
+  Language::GetLanguagesSupportingTypeSystems(languages_for_types,
+  languages_for_expressions);
+
+  for (auto lang : languages_for_expressions) {
+auto type_system_or_err =
+GetScratchTypeSystemForLanguage(lang, create_on_demand);
+if (!type_system_or_err)
+  LLDB_LOG_ERROR(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_TARGET),
+ type_system_or_err.takeError(),
+ "Language '{}' has expression support but no scratch type "
+ "system available",
+ Language::GetNameForLanguageType(lang));
+else
+  scratch_type_systems.emplace_back(&type_system_or_err.get());
+  }
+
+  return scratch_type_systems;
+}
+
 PersistentExpressionState *
 Target::GetPersistentExpressionStateForLanguage(lldb::LanguageType language) {
   auto type_system_or_err = GetScratchTypeSystemForLanguage(language, true);
Index: lldb/trunk/include/lldb/Target/Target.h
===
--- lldb/trunk/include/lldb/Target/Target.h
+++ lldb/trunk/include/lldb/Target/Target.h
@@ -1026,6 +1026,8 @@
   GetScratchTypeSystemForLanguage(lldb::LanguageType language,
   bool create_on_demand = true);
 
+  std::vector GetScratchTypeSystems(bool create_on_demand = true);
+
   PersistentExpressionState *
   GetPersistentExpressionStateForLanguage(lldb::LanguageType language);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llv

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Really on the lldb side, Jonas is the right person to review this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

[and Raphael for the clang vendor bits]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Jan Korous via Phabricator via lldb-commits]

jkorous added inline comments.



Comment at: clang/include/clang/Basic/FileManager.h:143
   ///
-  llvm::StringMap SeenDirEntries;
+  llvm::StringMap, llvm::BumpPtrAllocator>
+  SeenDirEntries;

Maybe we could replace this with some type that has hard-to-use-incorrectly 
interface instead of using `containsValue()`.



Comment at: clang/include/clang/Basic/FileManager.h:217
   ///
-  /// This returns NULL if the file doesn't exist.
+  /// This returns a \c std::error_code if there was an error loading the file.
   ///

Does that mean that it's now safe to assume the value is `!= NULL` in the 
absence of errors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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

Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

[Jim Ingham via lldb-commits]

> On Jul 31, 2019, at 12:50 PM, via lldb-commits  
> wrote:
> 
> 
> 
>> -Original Message-
>> From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf
>> Of Greg Clayton via lldb-commits
>> Sent: Wednesday, July 31, 2019 2:29 PM
>> To: Raphael Isemann
>> Cc: lldb-commits
>> Subject: Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by
>> value struct has no definition.
>> 
>> 
>> 
>>> On Jul 31, 2019, at 10:57 AM, Raphael Isemann 
>> wrote:
>>> 
>>> It seems that patch is lacking a test (which doesn't seem too hard to
>> provide).
>> 
>> I am not the original author of this patch that was causing the crash,
>> just fixing a crash that was introduced by the patch.
>> 
>> I am all ears for anyone that can provide me with DWARF to help reproduce
>> this scenario where we have a DW_CC_pass_by_value struct with no
>> definition. Not sure how you would have a compiler that is passing a
>> struct to a function as a parameter and yet does not emit debug info for
>> that struct it is clearly using in the debug info.
> 
> One wonders how you discovered the crash.

Presumably a crash report of some kind?  This is the sort of crash that 
wouldn't be hard to diagnose from a crash report even if there was no way to 
know how you got there...

That happens pretty frequently over here.  We get lots of crash reports 
delivered automatically if users opt into that feature of macOS - which many 
users do.  But sometimes it's hard to track back to the originator or ofttimes 
the report comes from someone who is unable to give us their sources and can't 
or isn't motivated to make a small reproducer.

So you are left with an obvious crash, and in this case a simple way to avoid 
the crash and straightforwardly get out of the situation.  But you have no idea 
how you could have gotten into that state, and no way to get help figuring it 
out.  At some point you have to cut your losses trying to find a reproducer, 
but it seems a shame to drop an obvious fix because of that...

Jim


> 
> I know that Clang will skip the content of a type if it determines that
> the definition is certain to be found elsewhere.  For example, if it has 
> virtual methods and the key function is not defined in the current CU.  
> Whether it will still add a CC attribute, I don't know.
> --paulr
> 
>> 
>>> 
>>> Am Mi., 31. Juli 2019 um 18:24 Uhr schrieb Greg Clayton via
>>> lldb-commits :
 
 Author: gclayton
 Date: Wed Jul 31 09:24:55 2019
 New Revision: 367441
 
 URL: http://llvm.org/viewvc/llvm-project?rev=367441&view=rev
 Log:
 Don't crash when pass by value struct has no definition.
 
 
 Modified:
   lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
 
 Modified:
>> lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
 URL: http://llvm.org/viewvc/llvm-
>> project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>> ?rev=367441&r1=367440&r2=367441&view=diff
 
>> ==
>> 
 --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>> (original)
 +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>> Wed Jul 31 09:24:55 2019
 @@ -1010,7 +1010,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
if (attrs.calling_convention == llvm::dwarf::DW_CC_pass_by_value) {
  clang::CXXRecordDecl *record_decl =
  m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
 -  if (record_decl) {
 +  if (record_decl && record_decl->getDefinition()) {
record_decl->setHasTrivialSpecialMemberForCall();
  }
}
 
 
 ___
 lldb-commits mailing list
 lldb-commits@lists.llvm.org
 https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Harlan Haskins via Phabricator via lldb-commits]

harlanhaskins marked 2 inline comments as done.
harlanhaskins added inline comments.



Comment at: clang/include/clang/Basic/FileManager.h:143
   ///
-  llvm::StringMap SeenDirEntries;
+  llvm::StringMap, llvm::BumpPtrAllocator>
+  SeenDirEntries;

jkorous wrote:
> Maybe we could replace this with some type that has hard-to-use-incorrectly 
> interface instead of using `containsValue()`.
You're right, these should store `llvm::ErrorOr` and use 
`std::err::no_such_file_or_directory` as a placeholder instead of `nullptr`.



Comment at: clang/include/clang/Basic/FileManager.h:217
   ///
-  /// This returns NULL if the file doesn't exist.
+  /// This returns a \c std::error_code if there was an error loading the file.
   ///

jkorous wrote:
> Does that mean that it's now safe to assume the value is `!= NULL` in the 
> absence of errors?
That's the intent of these changes, yes, but it should also be documented. 👍


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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

[Lldb-commits] [PATCH] D65363: [lldb-vscode] add `launchCommands` to handle launch specific commands

[Wanyi Ye via Phabricator via lldb-commits]

kusmour updated this revision to Diff 212683.
kusmour added a comment.

complete the corresponding test for extra `launchCommands`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65363

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1180,6 +1180,7 @@
   g_vsc.pre_run_commands = GetStrings(arguments, "preRunCommands");
   g_vsc.stop_commands = GetStrings(arguments, "stopCommands");
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
+  auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
 
@@ -1252,11 +1253,19 @@
 
   // Run any pre run LLDB commands the user specified in the launch.json
   g_vsc.RunPreRunCommands();
+  if (launchCommands.empty()) {
+// Disable async events so the launch will be successful when we return from
+// the launch call and the launch will happen synchronously
+g_vsc.debugger.SetAsync(false);
+g_vsc.target.Launch(g_vsc.launch_info, error);
+g_vsc.debugger.SetAsync(true);
+  } else {
+g_vsc.RunLLDBCommands("Running launchCommands:", launchCommands);
+// The custom commands might have created a new target so we should use the
+// selected target after these commands are run.
+g_vsc.target = g_vsc.debugger.GetSelectedTarget();
+  }
 
-  // Disable async events so the launch will be successful when we return from
-  // the launch call and the launch will happen synchronously
-  g_vsc.debugger.SetAsync(false);
-  g_vsc.target.Launch(g_vsc.launch_info, error);
   if (error.Fail()) {
 response["success"] = llvm::json::Value(false);
 EmplaceSafeString(response, "message", std::string(error.GetCString()));
@@ -1266,7 +1275,7 @@
   SendProcessEvent(Launch);
   g_vsc.SendJSON(llvm::json::Value(CreateEventObject("initialized")));
   // Reenable async events and start the event thread to catch async events.
-  g_vsc.debugger.SetAsync(true);
+  // g_vsc.debugger.SetAsync(true);
 }
 
 // "NextRequest": {
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -558,7 +558,7 @@
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None, sourcePath=None,
-   debuggerRoot=None):
+   debuggerRoot=None, launchCommands=None):
 args_dict = {
 'program': program
 }
@@ -591,6 +591,8 @@
 args_dict['sourcePath'] = sourcePath
 if debuggerRoot:
 args_dict['debuggerRoot'] = debuggerRoot
+if launchCommands:
+args_dict['launchCommands'] = launchCommands
 command_dict = {
 'command': 'launch',
 'type': 'request',
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -245,20 +245,17 @@
 self.assertTrue(response['success'],
 'attach failed (%s)' % (response['message']))
 
-def build_and_launch(self, program, args=None, cwd=None, env=None,
- stopOnEntry=False, disableASLR=True,
- disableSTDIO=False, shellExpandArguments=False,
- trace=False, initCommands=None, preRunCommands=None,
- stopCommands=None, exitCommands=None,
- sourcePath=None, debuggerRoot=None):
-'''Build the default Makefile target, create the VSCode debug adaptor,
-   and launch the process.
+def launch(self, program=None, args=None, cwd=None, env=None,
+   stopOnEntry=False, disableASLR=True,
+   disableSTDIO=False, shellExpandArguments=False,
+   trace=False, initCommands=None, preRunCommands=None,
+   stopCommands=None, exitCommands=None,sourcePath= None,
+   debuggerRoot=None, launchCommands=None):
+'''Sending launch request to vsc

[Lldb-commits] [PATCH] D65546: Fix TestThreadSpecificBreakpoint on Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: stella.stamenova.

This test was frequently hanging on Windows, causing a timeout after 10 
minutes.  The short delay (100 microsecond) in the sample program could cause a 
deadlock in the Windows thread pool, as I've explained in the test program's 
comments.

Now that it doesn't hang, it passes reliably, so I've removed the 
Windows-specific XFAIL.

I've tried to clarify the comments in TestThreadSpecificGBreakpoint.py by 
eliminating some redundancy and typos, and I simplified away a couple 
unnecessary assignments.


https://reviews.llvm.org/D65546

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp


Index: 
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
@@ -5,7 +5,14 @@
 thread_function ()
 {
 // Set thread-specific breakpoint here.
-std::this_thread::sleep_for(std::chrono::microseconds(100));
+std::this_thread::sleep_for(std::chrono::milliseconds(20));
+// On Windows, a sleep_for of less than about 16 ms effectively calls
+// Sleep(0).  The MS standard thread implementation uses a system thread
+// pool, which can deadlock on a Sleep(0), hanging not only the secondary
+// thread but the entire test.  I increased the delay to 20 ms to ensure
+// Sleep is called with a delay greater than 0.  The deadlock potential
+// is described here:
+// 
https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleep#remarks
 }
 
 int 
Index: 
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
@@ -27,7 +27,6 @@
 
 @add_test_categories(['pyapi'])
 
-@expectedFailureAll(oslist=["windows"])
 @expectedFailureAll(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], 
archs=['armv7', 'armv7k'], bugnumber='rdar://problem/34563920') # armv7 ios 
problem - breakpoint with tid qualifier isn't working
 @expectedFailureNetBSD
 def test_thread_id(self):
@@ -46,13 +45,6 @@
 (target, process, main_thread, main_breakpoint) = 
lldbutil.run_to_source_breakpoint(self,
 "Set main breakpoint here", main_source_spec)
 
-main_thread_id = main_thread.GetThreadID()
-
-# This test works by setting a breakpoint in a function conditioned to 
stop only on
-# the main thread, and then calling this function on a secondary 
thread, joining,
-# and then calling again on the main thread.  If the thread specific 
breakpoint works
-# then it should not be hit on the secondary thread, only on the main
-# thread.
 thread_breakpoint = target.BreakpointCreateBySourceRegex(
 "Set thread-specific breakpoint here", main_source_spec)
 self.assertGreater(
@@ -60,11 +52,11 @@
 0,
 "thread breakpoint has no locations associated with it.")
 
-# Set the thread-specific breakpoint to only stop on the main thread.  
The run the function
-# on another thread and join on it.  If the thread-specific breakpoint 
works, the next
-# stop should be on the main thread.
-
-main_thread_id = main_thread.GetThreadID()
+# Set the thread-specific breakpoint to stop only on the main thread
+# before the secondary thread has a chance to execute it.  The main
+# thread joins the secondary thread, and then the main thread will
+# execute the code at the breakpoint.  If the thread-specific
+# breakpoint works, the next stop will be on the main thread.
 setter_method(main_thread, thread_breakpoint)
 
 process.Continue()
@@ -81,5 +73,5 @@
 "thread breakpoint stopped at unexpected number of threads")
 self.assertEqual(
 stopped_threads[0].GetThreadID(),
-main_thread_id,
+main_thread.GetThreadID(),
 "thread breakpoint stopped at the wrong thread")


Index: lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
+++ lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_

[Lldb-commits] [PATCH] D65546: Fix TestThreadSpecificBreakpoint on Windows

[Stella Stamenova via Phabricator via lldb-commits]

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

LGTM. Please keep an eye on the bot after you commit though


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

https://reviews.llvm.org/D65546



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

[Lldb-commits] [PATCH] D65547: [Reproducers] Force replay in synchronous mode.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, labath, teemperor.
Herald added a project: LLDB.

Replaying a reproducer in asynchronous mode never makes sense. This patch 
disables asynchronous mode during replay.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D65547

Files:
  lldb/lit/Reproducer/TestSynchronous.test
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -109,7 +109,7 @@
   Properties(OptionValuePropertiesSP(
   new OptionValueProperties(ConstString("interpreter",
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
-  m_debugger(debugger), m_synchronous_execution(synchronous_execution),
+  m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),
   m_command_io_handler_sp(), m_comment_char('#'),
   m_batch_command_mode(false), m_truncation_warning(eNoTruncation),
@@ -118,6 +118,7 @@
   SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
   SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
+  SetSynchronous(synchronous_execution);
   CheckInWithManager();
   m_collection_sp->Initialize(g_interpreter_properties);
 }
@@ -2504,6 +2505,9 @@
 bool CommandInterpreter::GetSynchronous() { return m_synchronous_execution; }
 
 void CommandInterpreter::SetSynchronous(bool value) {
+  // Asynchronous mode is not supported with reproducers.
+  if (repro::Reproducer::Instance().GetLoader())
+return;
   m_synchronous_execution = value;
 }
 
Index: lldb/lit/Reproducer/TestSynchronous.test
===
--- /dev/null
+++ lldb/lit/Reproducer/TestSynchronous.test
@@ -0,0 +1,13 @@
+# Ensure that replay happens in synchronous mode.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'script 
lldb.debugger.SetAsync(True)' -o 'script lldb.debugger.GetAsync()' -o 
'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix REPLAY
+
+# CAPTURE: script lldb.debugger.SetAsync(True)
+# CAPTURE-NEXT: script lldb.debugger.GetAsync()
+# CAPTURE-NEXT: True
+
+# REPLAY: script lldb.debugger.SetAsync(True)
+# REPLAY-NEXT: script lldb.debugger.GetAsync()
+# REPLAY-NEXT: False


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -109,7 +109,7 @@
   Properties(OptionValuePropertiesSP(
   new OptionValueProperties(ConstString("interpreter",
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
-  m_debugger(debugger), m_synchronous_execution(synchronous_execution),
+  m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),
   m_command_io_handler_sp(), m_comment_char('#'),
   m_batch_command_mode(false), m_truncation_warning(eNoTruncation),
@@ -118,6 +118,7 @@
   SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
   SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
+  SetSynchronous(synchronous_execution);
   CheckInWithManager();
   m_collection_sp->Initialize(g_interpreter_properties);
 }
@@ -2504,6 +2505,9 @@
 bool CommandInterpreter::GetSynchronous() { return m_synchronous_execution; }
 
 void CommandInterpreter::SetSynchronous(bool value) {
+  // Asynchronous mode is not supported with reproducers.
+  if (repro::Reproducer::Instance().GetLoader())
+return;
   m_synchronous_execution = value;
 }
 
Index: lldb/lit/Reproducer/TestSynchronous.test
===
--- /dev/null
+++ lldb/lit/Reproducer/TestSynchronous.test
@@ -0,0 +1,13 @@
+# Ensure that replay happens in synchronous mode.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'script lldb.debugger.SetAsync(True)' -o 'script lldb.debugger.GetAsync()' -o 'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix REPLAY
+
+# CAPTURE: script lldb.debugger.SetAsync(True)
+# CAPTURE-NEXT: script lldb.debugger.GetAsync()
+# CAPTURE-NEXT: True
+
+# REPLAY: script lldb.debugger.SetAsync(True)
+# REPLAY-NEXT: script lldb.debugger.GetAsync()
+# REPLAY-NEXT: False
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65547: [Reproducers] Force replay in synchronous mode.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 212695.
JDevlieghere added a comment.

Be more precise in comment.


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

https://reviews.llvm.org/D65547

Files:
  lldb/lit/Reproducer/TestSynchronous.test
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -109,7 +109,7 @@
   Properties(OptionValuePropertiesSP(
   new OptionValueProperties(ConstString("interpreter",
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
-  m_debugger(debugger), m_synchronous_execution(synchronous_execution),
+  m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),
   m_command_io_handler_sp(), m_comment_char('#'),
   m_batch_command_mode(false), m_truncation_warning(eNoTruncation),
@@ -118,6 +118,7 @@
   SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
   SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
+  SetSynchronous(synchronous_execution);
   CheckInWithManager();
   m_collection_sp->Initialize(g_interpreter_properties);
 }
@@ -2504,6 +2505,9 @@
 bool CommandInterpreter::GetSynchronous() { return m_synchronous_execution; }
 
 void CommandInterpreter::SetSynchronous(bool value) {
+  // Asynchronous mode is not supported during reproducer replay.
+  if (repro::Reproducer::Instance().GetLoader())
+return;
   m_synchronous_execution = value;
 }
 
Index: lldb/lit/Reproducer/TestSynchronous.test
===
--- /dev/null
+++ lldb/lit/Reproducer/TestSynchronous.test
@@ -0,0 +1,13 @@
+# Ensure that replay happens in synchronous mode.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'script 
lldb.debugger.SetAsync(True)' -o 'script lldb.debugger.GetAsync()' -o 
'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix REPLAY
+
+# CAPTURE: script lldb.debugger.SetAsync(True)
+# CAPTURE-NEXT: script lldb.debugger.GetAsync()
+# CAPTURE-NEXT: True
+
+# REPLAY: script lldb.debugger.SetAsync(True)
+# REPLAY-NEXT: script lldb.debugger.GetAsync()
+# REPLAY-NEXT: False


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -109,7 +109,7 @@
   Properties(OptionValuePropertiesSP(
   new OptionValueProperties(ConstString("interpreter",
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
-  m_debugger(debugger), m_synchronous_execution(synchronous_execution),
+  m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),
   m_command_io_handler_sp(), m_comment_char('#'),
   m_batch_command_mode(false), m_truncation_warning(eNoTruncation),
@@ -118,6 +118,7 @@
   SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
   SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
+  SetSynchronous(synchronous_execution);
   CheckInWithManager();
   m_collection_sp->Initialize(g_interpreter_properties);
 }
@@ -2504,6 +2505,9 @@
 bool CommandInterpreter::GetSynchronous() { return m_synchronous_execution; }
 
 void CommandInterpreter::SetSynchronous(bool value) {
+  // Asynchronous mode is not supported during reproducer replay.
+  if (repro::Reproducer::Instance().GetLoader())
+return;
   m_synchronous_execution = value;
 }
 
Index: lldb/lit/Reproducer/TestSynchronous.test
===
--- /dev/null
+++ lldb/lit/Reproducer/TestSynchronous.test
@@ -0,0 +1,13 @@
+# Ensure that replay happens in synchronous mode.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'script lldb.debugger.SetAsync(True)' -o 'script lldb.debugger.GetAsync()' -o 'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix REPLAY
+
+# CAPTURE: script lldb.debugger.SetAsync(True)
+# CAPTURE-NEXT: script lldb.debugger.GetAsync()
+# CAPTURE-NEXT: True
+
+# REPLAY: script lldb.debugger.SetAsync(True)
+# REPLAY-NEXT: script lldb.debugger.GetAsync()
+# REPLAY-NEXT: False
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65547: [Reproducers] Force replay in synchronous mode.

[Davide Italiano via Phabricator via lldb-commits]

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

Thanks!


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

https://reviews.llvm.org/D65547



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

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: clang/include/clang/Basic/FileManager.h:217
   ///
-  /// This returns NULL if the file doesn't exist.
+  /// This returns a \c std::error_code if there was an error loading the file.
   ///

harlanhaskins wrote:
> jkorous wrote:
> > Does that mean that it's now safe to assume the value is `!= NULL` in the 
> > absence of errors?
> That's the intent of these changes, yes, but it should also be documented. 👍
In line with the previous comment, should we just pass a reference then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Harlan Haskins via Phabricator via lldb-commits]

harlanhaskins added inline comments.



Comment at: clang/include/clang/Basic/FileManager.h:217
   ///
-  /// This returns NULL if the file doesn't exist.
+  /// This returns a \c std::error_code if there was an error loading the file.
   ///

JDevlieghere wrote:
> harlanhaskins wrote:
> > jkorous wrote:
> > > Does that mean that it's now safe to assume the value is `!= NULL` in the 
> > > absence of errors?
> > That's the intent of these changes, yes, but it should also be documented. 👍
> In line with the previous comment, should we just pass a reference then?
I'm fine with doing that, but it would introduce a significant amount more 
churn into this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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

[Lldb-commits] [lldb] r367494 - [Reproducers] Force replay in synchronous mode.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Jul 31 16:34:45 2019
New Revision: 367494

URL: http://llvm.org/viewvc/llvm-project?rev=367494&view=rev
Log:
[Reproducers] Force replay in synchronous mode.

Replaying a reproducer in asynchronous mode never makes sense. This
patch disables asynchronous mode during replay.

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

Added:
lldb/trunk/lit/Reproducer/TestSynchronous.test
Modified:
lldb/trunk/source/Interpreter/CommandInterpreter.cpp

Added: lldb/trunk/lit/Reproducer/TestSynchronous.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestSynchronous.test?rev=367494&view=auto
==
--- lldb/trunk/lit/Reproducer/TestSynchronous.test (added)
+++ lldb/trunk/lit/Reproducer/TestSynchronous.test Wed Jul 31 16:34:45 2019
@@ -0,0 +1,13 @@
+# Ensure that replay happens in synchronous mode.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'script 
lldb.debugger.SetAsync(True)' -o 'script lldb.debugger.GetAsync()' -o 
'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix REPLAY
+
+# CAPTURE: script lldb.debugger.SetAsync(True)
+# CAPTURE-NEXT: script lldb.debugger.GetAsync()
+# CAPTURE-NEXT: True
+
+# REPLAY: script lldb.debugger.SetAsync(True)
+# REPLAY-NEXT: script lldb.debugger.GetAsync()
+# REPLAY-NEXT: False

Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=367494&r1=367493&r2=367494&view=diff
==
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Wed Jul 31 16:34:45 
2019
@@ -109,7 +109,7 @@ CommandInterpreter::CommandInterpreter(D
   Properties(OptionValuePropertiesSP(
   new OptionValueProperties(ConstString("interpreter",
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
-  m_debugger(debugger), m_synchronous_execution(synchronous_execution),
+  m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),
   m_command_io_handler_sp(), m_comment_char('#'),
   m_batch_command_mode(false), m_truncation_warning(eNoTruncation),
@@ -118,6 +118,7 @@ CommandInterpreter::CommandInterpreter(D
   SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
   SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
+  SetSynchronous(synchronous_execution);
   CheckInWithManager();
   m_collection_sp->Initialize(g_interpreter_properties);
 }
@@ -2504,6 +2505,9 @@ void CommandInterpreter::HandleCommandsF
 bool CommandInterpreter::GetSynchronous() { return m_synchronous_execution; }
 
 void CommandInterpreter::SetSynchronous(bool value) {
+  // Asynchronous mode is not supported during reproducer replay.
+  if (repro::Reproducer::Instance().GetLoader())
+return;
   m_synchronous_execution = value;
 }
 


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

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Harlan Haskins via Phabricator via lldb-commits]

harlanhaskins updated this revision to Diff 212701.
harlanhaskins added a comment.

Store references instead of raw pointers in FileManger's cache


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
  clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
  clang-tools-extra/clang-move/Move.cpp
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/modularize/ModularizeUtilities.cpp
  clang/include/clang/Basic/FileManager.h
  clang/lib/ARCMigrate/FileRemapper.cpp
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/Module.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/Rewrite/FrontendActions.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/lib/Serialization/ModuleManager.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Refactoring.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/clang-refactor/TestSupport.cpp
  clang/tools/clang-rename/ClangRename.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/RewriterTestContext.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }
Index: clang/unittests/Tooling/RewriterTestContext.h
===
--- clang/unittests/Tooling/RewriterTestContext.h
+++ clang/unittests/Tooling/RewriterTestContext.h
@@ -56,9 +56,9 @@
 llvm::MemoryBuffer::getMemBuffer(Content);
 InMemoryFileSystem->addFile(Name, 0, std::move(Source));
 
-const FileEntry *Entry = Files.getFile(Name);
-assert(Entry != nullptr);
-return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
+auto Entry = Files.getFile(Name);
+assert(Entry);
+return Sources.createFileID(*Entry, SourceLocation(), SrcMgr::C_User);
   }
 
   // FIXME: this code is mostly a duplicate of
@@ -73,14 +73,14 @@
 llvm::raw_fd_o

[Lldb-commits] [PATCH] D65547: [Reproducers] Force replay in synchronous mode.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367494: [Reproducers] Force replay in synchronous mode. 
(authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65547?vs=212695&id=212702#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65547

Files:
  lldb/trunk/lit/Reproducer/TestSynchronous.test
  lldb/trunk/source/Interpreter/CommandInterpreter.cpp


Index: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp
@@ -109,7 +109,7 @@
   Properties(OptionValuePropertiesSP(
   new OptionValueProperties(ConstString("interpreter",
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
-  m_debugger(debugger), m_synchronous_execution(synchronous_execution),
+  m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),
   m_command_io_handler_sp(), m_comment_char('#'),
   m_batch_command_mode(false), m_truncation_warning(eNoTruncation),
@@ -118,6 +118,7 @@
   SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
   SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
+  SetSynchronous(synchronous_execution);
   CheckInWithManager();
   m_collection_sp->Initialize(g_interpreter_properties);
 }
@@ -2504,6 +2505,9 @@
 bool CommandInterpreter::GetSynchronous() { return m_synchronous_execution; }
 
 void CommandInterpreter::SetSynchronous(bool value) {
+  // Asynchronous mode is not supported during reproducer replay.
+  if (repro::Reproducer::Instance().GetLoader())
+return;
   m_synchronous_execution = value;
 }
 
Index: lldb/trunk/lit/Reproducer/TestSynchronous.test
===
--- lldb/trunk/lit/Reproducer/TestSynchronous.test
+++ lldb/trunk/lit/Reproducer/TestSynchronous.test
@@ -0,0 +1,13 @@
+# Ensure that replay happens in synchronous mode.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'script 
lldb.debugger.SetAsync(True)' -o 'script lldb.debugger.GetAsync()' -o 
'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix REPLAY
+
+# CAPTURE: script lldb.debugger.SetAsync(True)
+# CAPTURE-NEXT: script lldb.debugger.GetAsync()
+# CAPTURE-NEXT: True
+
+# REPLAY: script lldb.debugger.SetAsync(True)
+# REPLAY-NEXT: script lldb.debugger.GetAsync()
+# REPLAY-NEXT: False


Index: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp
@@ -109,7 +109,7 @@
   Properties(OptionValuePropertiesSP(
   new OptionValueProperties(ConstString("interpreter",
   IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
-  m_debugger(debugger), m_synchronous_execution(synchronous_execution),
+  m_debugger(debugger), m_synchronous_execution(true),
   m_skip_lldbinit_files(false), m_skip_app_init_files(false),
   m_command_io_handler_sp(), m_comment_char('#'),
   m_batch_command_mode(false), m_truncation_warning(eNoTruncation),
@@ -118,6 +118,7 @@
   SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
   SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
+  SetSynchronous(synchronous_execution);
   CheckInWithManager();
   m_collection_sp->Initialize(g_interpreter_properties);
 }
@@ -2504,6 +2505,9 @@
 bool CommandInterpreter::GetSynchronous() { return m_synchronous_execution; }
 
 void CommandInterpreter::SetSynchronous(bool value) {
+  // Asynchronous mode is not supported during reproducer replay.
+  if (repro::Reproducer::Instance().GetLoader())
+return;
   m_synchronous_execution = value;
 }
 
Index: lldb/trunk/lit/Reproducer/TestSynchronous.test
===
--- lldb/trunk/lit/Reproducer/TestSynchronous.test
+++ lldb/trunk/lit/Reproducer/TestSynchronous.test
@@ -0,0 +1,13 @@
+# Ensure that replay happens in synchronous mode.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'script lldb.debugger.SetAsync(True)' -o 'script lldb.debugger.GetAsync()' -o 'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix REPLAY
+
+# CAPTURE: script lldb.debugger.SetAsync(True)
+# CAPTURE-NEXT: script lldb.debugger.GetAsync()
+# CAPTURE-NEXT: True
+
+# R

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

[Jan Korous via Phabricator via lldb-commits]

jkorous added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:240
+auto File = SourceMgr.getFileManager().getFile(FilePath);
+if (!File)
+  return SourceLocation();

Previously we'd hit the assert in `translateFile()` called from 
`getOrCreateFileID()`. This seems like a better way to handle that.



Comment at: clang/include/clang/Basic/FileManager.h:217
   ///
-  /// This returns NULL if the file doesn't exist.
+  /// This returns a \c std::error_code if there was an error loading the file.
   ///

harlanhaskins wrote:
> JDevlieghere wrote:
> > harlanhaskins wrote:
> > > jkorous wrote:
> > > > Does that mean that it's now safe to assume the value is `!= NULL` in 
> > > > the absence of errors?
> > > That's the intent of these changes, yes, but it should also be 
> > > documented. 👍
> > In line with the previous comment, should we just pass a reference then?
> I'm fine with doing that, but it would introduce a significant amount more 
> churn into this patch.
I feel that this patch is already huge. It's a good idea though - maybe a 
separate patch?



Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:156
+  auto newE = FileMgr->getFile(tempPath);
+  if (newE) {
+remap(origFE, *newE);

It seems like we're now avoiding the assert in `remap()` we'd hit previously. 
Is this fine?



Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:229
 const FileEntry *FileRemapper::getOriginalFile(StringRef filePath) {
-  const FileEntry *file = FileMgr->getFile(filePath);
+  const FileEntry *file;
+  if (auto fileOrErr = FileMgr->getFile(filePath))

Shouldn't we initialize this guy to `nullptr`?



Comment at: clang/lib/Basic/FileManager.cpp:73
   if (llvm::sys::path::is_separator(Filename[Filename.size() - 1]))
-return nullptr; // If Filename is a directory.
+return std::errc::is_a_directory; // If Filename is a directory.
 

I think you've made the code self-documenting. Could you please remove the 
comment?



Comment at: clang/lib/Frontend/FrontendAction.cpp:715
   SmallString<128> DirNative;
-  llvm::sys::path::native(PCHDir->getName(), DirNative);
+  if (*PCHDir == nullptr) {
+llvm::dbgs() << "Directory " << PCHInclude << " was null but no error 
thrown";

Could this actually happen? I was expecting the behavior to be consistent with 
`getFile()`.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1448
   if (getHeaderSearchOpts().ModuleMapFileHomeIsCwd)
-Dir = FileMgr.getDirectory(".");
+Dir = *FileMgr.getDirectory(".");
   else {

Seems like we're introducing assert here. Is that fine?



Comment at: clang/unittests/Basic/FileManagerTest.cpp:260
+
+  EXPECT_EQ(f1 ? *f1 : nullptr,
+f2 ? *f2 : nullptr);

Just an idea - should we compare error codes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Michał Górny via Phabricator via lldb-commits]

mgorny planned changes to this revision.
mgorny added a comment.

It seems that the patch is wrong with the current logic, and this is somehow 
blurry because of Linux implementation limitations. FWIU, `eStateSuspended` 
will never happen and instead we should keep threads without explicit action 
suspsended. It's unclear yet whether this is desirable long-term, or if we 
should change the logic to explicitly indicate state for each thread.


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

https://reviews.llvm.org/D64647



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

[Lldb-commits] [PATCH] D65555: [lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski.
Herald added a reviewer: jfb.
mgorny added a parent revision: D64647: [lldb] [Process/NetBSD] Implement 
per-thread execution control.
Herald added a subscriber: dexonsmith.

// NB:
Now, this patch is most likely correct. However, I've tested it only with the 
other patch applied, and many tests started hanging forever. If you have any 
comments, I'd be happy to process them but I won't commit it until all the 
issues have been resolved.


https://reviews.llvm.org/D6

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentBreakpointOneDelayBreakpointThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentCrashWithBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentCrashWithSignal.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelayedCrashWithBreakpointSignal.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentTwoBreakpointThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h

Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
@@ -98,6 +98,7 @@
   bool HasThreadNoLock(lldb::tid_t thread_id);
 
   NativeThreadNetBSD &AddThread(lldb::tid_t thread_id);
+  void RemoveThread(lldb::tid_t thread_id);
 
   void MonitorCallback(lldb::pid_t pid, int signal);
   void MonitorExited(lldb::pid_t pid, WaitStatus status);
Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -9,7 +9,6 @@
 #include "NativeProcessNetBSD.h"
 
 
-
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
@@ -99,6 +98,14 @@
   pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate,
   Info.GetArchitecture(), mainloop));
 
+  // Enable event reporting
+  ptrace_event_t events;
+  // TODO: PTRACE_FORK | PTRACE_VFORK | PTRACE_POSIX_SPAWN?
+  events.pe_set_event = PTRACE_LWP_CREATE | PTRACE_LWP_EXIT;
+  status = PtraceWrapper(PT_SET_EVENT_MASK, pid, &events, sizeof(events));
+  if (status.Fail())
+return status.ToError();
+
   status = process_up->ReinitializeThreads();
   if (status.Fail())
 return status.ToError();
@@ -238,6 +245,31 @@
   static_cast(*thread).SetStoppedByExec();
 SetState(StateType::eStateStopped, true);
   } break;
+  case TRAP_LWP: {
+ptrace_state_t pst;
+Status error = PtraceWrapper(PT_GET_PROCESS_STATE, GetID(), &pst,
+ sizeof(pst));
+if (error.Fail()) {
+  SetState(StateType::eStateInvalid);
+  return;
+}
+
+switch (pst.pe_report_event) {
+  case PTRACE_LWP_CREATE:
+AddThread(pst.pe_lwp);
+break;
+  case PTRACE_LWP_EXIT:
+RemoveThread(pst.pe_lwp);
+break;
+}
+
+error = PtraceWrapper(PT_CONTINUE, GetID(), reinterpret_cast(1), 0);
+if (error.Fail()) {
+  SetState(StateType::eStateInvalid);
+  return;
+}
+SetState(StateType::eStateRunning, true);
+  } break;
   case TRAP_DBREG: {
 // Find the thread.
 NativeThreadNetBSD* thread = nullptr;
@@ -694,6 +726,21 @@
   return static_cast(*m_threads.back());
 }
 
+void NativeProcessNetBSD::RemoveThread(lldb::tid_t thread_id) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+  LLDB_LOG(log, "pid {0} removing thread with tid {1}", GetID(), thread_id);
+
+  assert(HasThreadNoLock(thread_id) &&
+ "attempted to remove a thread that does not exist");
+
+  for (auto it = m_threads.begin(); it != m_threads.end(); ++it) {
+if (*it && ((*it)->GetID() == thread_id)) {
+  m_threads.erase(it);
+  break;
+}
+  }
+}
+
 Status NativeProcessNetBSD::Attach() {
   // Attach to the requested process.
   // An attach will cause the thread to stop with a SIGSTOP.
Index: lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py
@@ -27,7 +27

Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

[Pavel Labath via lldb-commits]

On 01/08/2019 00:14, Jim Ingham via lldb-commits wrote:




On Jul 31, 2019, at 12:50 PM, via lldb-commits  
wrote:




-Original Message-
From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf
Of Greg Clayton via lldb-commits
Sent: Wednesday, July 31, 2019 2:29 PM
To: Raphael Isemann
Cc: lldb-commits
Subject: Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by
value struct has no definition.




On Jul 31, 2019, at 10:57 AM, Raphael Isemann 

wrote:


It seems that patch is lacking a test (which doesn't seem too hard to

provide).

I am not the original author of this patch that was causing the crash,
just fixing a crash that was introduced by the patch.

I am all ears for anyone that can provide me with DWARF to help reproduce
this scenario where we have a DW_CC_pass_by_value struct with no
definition. Not sure how you would have a compiler that is passing a
struct to a function as a parameter and yet does not emit debug info for
that struct it is clearly using in the debug info.


Presumably one could construct a type DIE by hand which contains a 
DW_AT_declaration and a DW_AT_calling_convention. Even if this isn't the 
exact same way how the original bug came to be, it is a still a valid 
thing to test, and so it's better than no test at all.


That said, if the problem here is that the type is not fully defined, I 
am wondering if it wouldn't be better to move the calling convention 
stuff into the if(!is_forward_declaration) block right above it.

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

[Lldb-commits] [PATCH] D65555: [lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP]

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks fine to me. Minor comments inline.




Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:271
+}
+SetState(StateType::eStateRunning, true);
+  } break;

I don't think you need this. Given that you never reported a stop event, as far 
as the rest of the world is concerned the process is still "running" (i.e., 
this "stop to notice a new thread" is just an implementation detail).



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:737
+  for (auto it = m_threads.begin(); it != m_threads.end(); ++it) {
+if (*it && ((*it)->GetID() == thread_id)) {
+  m_threads.erase(it);

It looks like the rest of the code assumes (as I think it should) that the 
thread list does not contain null pointers. See e.g. the loop on line 276.


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

https://reviews.llvm.org/D6



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

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D64647#1609600 , @mgorny wrote:

> It seems that the patch is wrong with the current logic, and this is somehow 
> blurry because of Linux implementation limitations. FWIU, `eStateSuspended` 
> will never happen and instead we should keep threads without explicit action 
> suspsended. It's unclear yet whether this is desirable long-term, or if we 
> should change the logic to explicitly indicate state for each thread.


Yes, eStateSuspended will never happen on linux. It wouldn't be too hard to 
make use of that state -- we could set unresumed threads to "suspened" in 
NPL::Resume, and then when we go to stop all threads (NPL::StopRunningThreads), 
we would just avoid sending signals to suspended threads and just silently set 
them back to "stopped". However, I don't see a reason to do that now, as it 
would just add code for jumping between the states.

However, if it makes your life easier, I think you should be able to use 
eStateSuspended inside NetBSD code for anything you want. Nobody will inspect 
the state of threads while the process is running, so that could just be an 
implementation detail.

In fact I suspect the nobody will inspect the thread state whatsoever because 
once the process is stopped, all threads are assumed to be stopped too.


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

https://reviews.llvm.org/D64647



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