[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Alexandre Ganea via Phabricator via lldb-commits 
aganea added inline comments.



Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:29
+  llvm::codeview::CPUType cpu_type;
+  if (arch_type == llvm::Triple::ArchType::aarch64) {
+cpu_type = llvm::codeview::CPUType::ARM64;

Shouldn’t ArchType::aarch64_be and ArchType::aarch64_32 enums be handled here 
as well?



Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:32
+  } else {
+cpu_type = llvm::codeview::CPUType::X64;
+  }

Intel80386 for consistency?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62772



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

[Lldb-commits] [PATCH] D62771: [LLDBRegisterNum] Update function call llvm::codeview::getRegisterNames(CPUType) in lldb

2019-06-02 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

As I said on the other patch, I don't believe this belongs in LLDB. Otherwise, 
you require every user of codeview to maintain and update an accurate mapping 
of targets currently supported by llvm::codeview. Instead, this should be part 
of codeview API.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62771



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

[Lldb-commits] [PATCH] D62771: [LLDBRegisterNum] Update function call llvm::codeview::getRegisterNames(CPUType) in lldb

2019-06-02 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd accepted this revision.
compnerd added a comment.

Generally, `clang-format` the changes, it will catch the formatting things.




Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:28
+  llvm::codeview::CPUType cpu;
+  switch(arch_type) {
+case llvm::Triple::ArchType::aarch64:

Space after the `switch`.



Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:35
+default:
+  cpu = llvm::codeview::CPUType::X64;
+  }

Please add a `break` after this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62771



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

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.



Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:29
+  llvm::codeview::CPUType cpu_type;
+  if (arch_type == llvm::Triple::ArchType::aarch64) {
+cpu_type = llvm::codeview::CPUType::ARM64;

aganea wrote:
> Shouldn’t ArchType::aarch64_be and ArchType::aarch64_32 enums be handled here 
> as well?
I think that we should use a `switch` to cover the targets.  `/Oy` will allow 
FPO on x86 as well.  There is also WoA (ARM32).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62772



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

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Tom Tan via Phabricator via lldb-commits 
TomTan marked an inline comment as done.
TomTan added inline comments.



Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:29
+  llvm::codeview::CPUType cpu_type;
+  if (arch_type == llvm::Triple::ArchType::aarch64) {
+cpu_type = llvm::codeview::CPUType::ARM64;

compnerd wrote:
> aganea wrote:
> > Shouldn’t ArchType::aarch64_be and ArchType::aarch64_32 enums be handled 
> > here as well?
> I think that we should use a `switch` to cover the targets.  `/Oy` will allow 
> FPO on x86 as well.  There is also WoA (ARM32).
Seems no, aarch64_be or aarch64_32 is not supported by CodeView or Windows.



Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:32
+  } else {
+cpu_type = llvm::codeview::CPUType::X64;
+  }

aganea wrote:
> Intel80386 for consistency?
x86 and x64 are handled as the same `CPUType` by CodeView. LLVM pdb dump tool 
also sets X64 as default.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62772



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

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Tom Tan via Phabricator via lldb-commits 
TomTan updated this revision to Diff 202614.
TomTan added a comment.

Change to switch/case based on comment.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62772

Files:
  source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp


Index: source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
===
--- source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -25,8 +25,19 @@
 
 static uint32_t ResolveLLDBRegisterNum(llvm::StringRef reg_name, 
llvm::Triple::ArchType arch_type) {
   // lookup register name to get lldb register number
+  llvm::codeview::CPUType cpu_type;
+  switch (arch_type) {
+case llvm::Triple::ArchType::aarch64:
+  cpu_type = llvm::codeview::CPUType::ARM64;
+  break;
+
+default:
+  cpu_type = llvm::codeview::CPUType::X64;
+  break;
+  }
+
   llvm::ArrayRef> register_names =
-  llvm::codeview::getRegisterNames();
+  llvm::codeview::getRegisterNames(cpu_type);
   auto it = llvm::find_if(
   register_names,
   [®_name](const llvm::EnumEntry ®ister_entry) {


Index: source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
===
--- source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -25,8 +25,19 @@
 
 static uint32_t ResolveLLDBRegisterNum(llvm::StringRef reg_name, llvm::Triple::ArchType arch_type) {
   // lookup register name to get lldb register number
+  llvm::codeview::CPUType cpu_type;
+  switch (arch_type) {
+case llvm::Triple::ArchType::aarch64:
+  cpu_type = llvm::codeview::CPUType::ARM64;
+  break;
+
+default:
+  cpu_type = llvm::codeview::CPUType::X64;
+  break;
+  }
+
   llvm::ArrayRef> register_names =
-  llvm::codeview::getRegisterNames();
+  llvm::codeview::getRegisterNames(cpu_type);
   auto it = llvm::find_if(
   register_names,
   [®_name](const llvm::EnumEntry ®ister_entry) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Tom Tan via Phabricator via lldb-commits 
TomTan marked an inline comment as done.
TomTan added inline comments.



Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:29
+  llvm::codeview::CPUType cpu_type;
+  if (arch_type == llvm::Triple::ArchType::aarch64) {
+cpu_type = llvm::codeview::CPUType::ARM64;

TomTan wrote:
> compnerd wrote:
> > aganea wrote:
> > > Shouldn’t ArchType::aarch64_be and ArchType::aarch64_32 enums be handled 
> > > here as well?
> > I think that we should use a `switch` to cover the targets.  `/Oy` will 
> > allow FPO on x86 as well.  There is also WoA (ARM32).
> Seems no, aarch64_be or aarch64_32 is not supported by CodeView or Windows.
Ok. It makes sense to use switch/case. CodeView doesn't support WoA (ARM32) so 
no need to add case for it here.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62772



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

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

2019-06-02 Thread Alex Langford via Phabricator via lldb-commits 
xiaobai created this revision.
xiaobai added reviewers: compnerd, labath, JDevlieghere, davide.

LookupRuntimeSymbol seems like a general LanguageRuntime method.
Although no other language runtime currently implements this, there's no
reason another language runtime couldn't use this.

Additionally, this breaks IRExecutionUnit's dependency on
ObjCLanguageRuntime.


https://reviews.llvm.org/D62795

Files:
  include/lldb/Target/LanguageRuntime.h
  include/lldb/Target/ObjCLanguageRuntime.h
  source/Expression/IRExecutionUnit.cpp


Index: source/Expression/IRExecutionUnit.cpp
===
--- source/Expression/IRExecutionUnit.cpp
+++ source/Expression/IRExecutionUnit.cpp
@@ -24,7 +24,7 @@
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/ExecutionContext.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
+#include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
@@ -902,9 +902,8 @@
 return LLDB_INVALID_ADDRESS;
   }
 
-  ObjCLanguageRuntime *runtime = process_sp->GetObjCLanguageRuntime();
-
-  if (runtime) {
+  if (LanguageRuntime *runtime =
+  process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC)) {
 for (const SearchSpec &spec : specs) {
   lldb::addr_t symbol_load_addr = runtime->LookupRuntimeSymbol(spec.name);
 
Index: include/lldb/Target/ObjCLanguageRuntime.h
===
--- include/lldb/Target/ObjCLanguageRuntime.h
+++ include/lldb/Target/ObjCLanguageRuntime.h
@@ -264,13 +264,6 @@
   virtual size_t GetByteOffsetForIvar(CompilerType &parent_qual_type,
   const char *ivar_name);
 
-  // Given the name of an Objective-C runtime symbol (e.g., ivar offset
-  // symbol), try to determine from the runtime what the value of that symbol
-  // would be. Useful when the underlying binary is stripped.
-  virtual lldb::addr_t LookupRuntimeSymbol(ConstString name) {
-return LLDB_INVALID_ADDRESS;
-  }
-
   bool HasNewLiteralsAndIndexing() {
 if (m_has_new_literals_and_indexing == eLazyBoolCalculate) {
   if (CalculateHasNewLiteralsAndIndexing())
Index: include/lldb/Target/LanguageRuntime.h
===
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -166,6 +166,13 @@
 return false;
   }
 
+  // Given the name of a runtime symbol (e.g. in Objective-C, an ivar offset
+  // symbol), try to determine from the runtime what the value of that symbol
+  // would be. Useful when the underlying binary is stripped.
+  virtual lldb::addr_t LookupRuntimeSymbol(ConstString name) {
+return LLDB_INVALID_ADDRESS;
+  }
+
 protected:
   // Classes that inherit from LanguageRuntime can see and modify these
 


Index: source/Expression/IRExecutionUnit.cpp
===
--- source/Expression/IRExecutionUnit.cpp
+++ source/Expression/IRExecutionUnit.cpp
@@ -24,7 +24,7 @@
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/ExecutionContext.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
+#include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
@@ -902,9 +902,8 @@
 return LLDB_INVALID_ADDRESS;
   }
 
-  ObjCLanguageRuntime *runtime = process_sp->GetObjCLanguageRuntime();
-
-  if (runtime) {
+  if (LanguageRuntime *runtime =
+  process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC)) {
 for (const SearchSpec &spec : specs) {
   lldb::addr_t symbol_load_addr = runtime->LookupRuntimeSymbol(spec.name);
 
Index: include/lldb/Target/ObjCLanguageRuntime.h
===
--- include/lldb/Target/ObjCLanguageRuntime.h
+++ include/lldb/Target/ObjCLanguageRuntime.h
@@ -264,13 +264,6 @@
   virtual size_t GetByteOffsetForIvar(CompilerType &parent_qual_type,
   const char *ivar_name);
 
-  // Given the name of an Objective-C runtime symbol (e.g., ivar offset
-  // symbol), try to determine from the runtime what the value of that symbol
-  // would be. Useful when the underlying binary is stripped.
-  virtual lldb::addr_t LookupRuntimeSymbol(ConstString name) {
-return LLDB_INVALID_ADDRESS;
-  }
-
   bool HasNewLiteralsAndIndexing() {
 if (m_has_new_literals_and_indexing == eLazyBoolCalculate) {
   if (CalculateHasNewLiteralsAndIndexing())
Index: include/lldb/Target/LanguageRuntime.h
===
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -166,6 +166,13 @@
 return false;
   }
 
+  // Given the name of a runtime symbol (e

[Lldb-commits] [PATCH] D62785: Fix compilation following r362280

2019-06-02 Thread Alexandre Ganea via Phabricator via lldb-commits 
aganea abandoned this revision.
aganea added a comment.

Will be fixed by D62772 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62785



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

[Lldb-commits] [PATCH] D62796: [Target] Generalize some behavior in Target::SymbolsDidLoad

2019-06-02 Thread Alex Langford via Phabricator via lldb-commits 
xiaobai created this revision.
xiaobai added reviewers: compnerd, JDevlieghere, davide, labath.

SymbolsDidLoad is currently only implemented for ObjCLanguageRuntime,
but that doesn't mean that it couldn't be useful for other Langauges. Although
this change seems like it's generalizing for the sake of purity, this removes
Target's dependency on ObjCLanguageRuntime.


https://reviews.llvm.org/D62796

Files:
  include/lldb/Target/LanguageRuntime.h
  include/lldb/Target/ObjCLanguageRuntime.h
  source/Target/Target.cpp


Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -43,7 +43,6 @@
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/LanguageRuntime.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
@@ -1668,12 +1667,9 @@
 void Target::SymbolsDidLoad(ModuleList &module_list) {
   if (m_valid && module_list.GetSize()) {
 if (m_process_sp) {
-  LanguageRuntime *runtime =
-  m_process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC);
-  if (runtime) {
-ObjCLanguageRuntime *objc_runtime = (ObjCLanguageRuntime *)runtime;
+  if (LanguageRuntime *objc_runtime =
+  m_process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC))
 objc_runtime->SymbolsDidLoad(module_list);
-  }
 }
 
 m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
Index: include/lldb/Target/ObjCLanguageRuntime.h
===
--- include/lldb/Target/ObjCLanguageRuntime.h
+++ include/lldb/Target/ObjCLanguageRuntime.h
@@ -282,7 +282,7 @@
 return (m_has_new_literals_and_indexing == eLazyBoolYes);
   }
 
-  virtual void SymbolsDidLoad(const ModuleList &module_list) {
+  void SymbolsDidLoad(const ModuleList &module_list) override {
 m_negative_complete_class_cache.clear();
   }
 
Index: include/lldb/Target/LanguageRuntime.h
===
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -143,6 +143,8 @@
 return false;
   }
 
+  virtual void SymbolsDidLoad(const ModuleList &module_list) { return; }
+
   virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
   bool stop_others) = 
0;
 


Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -43,7 +43,6 @@
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/LanguageRuntime.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
@@ -1668,12 +1667,9 @@
 void Target::SymbolsDidLoad(ModuleList &module_list) {
   if (m_valid && module_list.GetSize()) {
 if (m_process_sp) {
-  LanguageRuntime *runtime =
-  m_process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC);
-  if (runtime) {
-ObjCLanguageRuntime *objc_runtime = (ObjCLanguageRuntime *)runtime;
+  if (LanguageRuntime *objc_runtime =
+  m_process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC))
 objc_runtime->SymbolsDidLoad(module_list);
-  }
 }
 
 m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
Index: include/lldb/Target/ObjCLanguageRuntime.h
===
--- include/lldb/Target/ObjCLanguageRuntime.h
+++ include/lldb/Target/ObjCLanguageRuntime.h
@@ -282,7 +282,7 @@
 return (m_has_new_literals_and_indexing == eLazyBoolYes);
   }
 
-  virtual void SymbolsDidLoad(const ModuleList &module_list) {
+  void SymbolsDidLoad(const ModuleList &module_list) override {
 m_negative_complete_class_cache.clear();
   }
 
Index: include/lldb/Target/LanguageRuntime.h
===
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -143,6 +143,8 @@
 return false;
   }
 
+  virtual void SymbolsDidLoad(const ModuleList &module_list) { return; }
+
   virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
   bool stop_others) = 0;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r362339 - [Commands] Remove unused header

2019-06-02 Thread Alex Langford via lldb-commits 
Author: xiaobai
Date: Sun Jun  2 14:11:21 2019
New Revision: 362339

URL: http://llvm.org/viewvc/llvm-project?rev=362339&view=rev
Log:
[Commands] Remove unused header

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

Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=362339&r1=362338&r2=362339&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Sun Jun  2 14:11:21 
2019
@@ -10,7 +10,6 @@
 #include "llvm/ADT/StringRef.h"
 
 #include "CommandObjectExpression.h"
-#include "Plugins/ExpressionParser/Clang/ClangExpressionVariable.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Core/ValueObjectVariable.h"


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

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::SetCompilerTypeFromPersistentDecl

2019-06-02 Thread Alex Langford via Phabricator via lldb-commits 
xiaobai created this revision.
xiaobai added reviewers: compnerd, davide, labath, JDevlieghere, aprantl, 
jingham.
Herald added a subscriber: mgorny.

PersistentStateExpressions (e.g. ClangPersistentVariables) have the
ability to define types using expressions that persist throughout the
debugging session. SetCompilerTypeFromPersistentDecl is a useful
operation to have if you need to use any of those persistently declared types,
like in CommandObjectMemory.

This decouples clang from CommandObjectMemory and decouples Plugins from
Commands in general.


https://reviews.llvm.org/D62797

Files:
  include/lldb/Expression/ExpressionVariable.h
  source/Commands/CMakeLists.txt
  source/Commands/CommandObjectMemory.cpp
  source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
  source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h

Index: source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
===
--- source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
+++ source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
@@ -50,6 +50,9 @@
 return "$";
   }
 
+  bool SetCompilerTypeFromPersistentDecl(ConstString type_name,
+ CompilerType &compiler_type) override;
+
   void RegisterPersistentDecl(ConstString name, clang::NamedDecl *decl);
 
   clang::NamedDecl *GetPersistentDecl(ConstString name);
Index: source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
@@ -9,6 +9,7 @@
 #include "ClangPersistentVariables.h"
 
 #include "lldb/Core/Value.h"
+#include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Log.h"
@@ -52,6 +53,19 @@
 m_next_persistent_variable_id--;
 }
 
+bool ClangPersistentVariables::SetCompilerTypeFromPersistentDecl(
+ConstString type_name, CompilerType &compiler_type) {
+  if (clang::TypeDecl *tdecl = llvm::dyn_cast_or_null(
+  GetPersistentDecl(type_name))) {
+compiler_type.SetCompilerType(
+ClangASTContext::GetASTContext(&tdecl->getASTContext()),
+reinterpret_cast(
+const_cast(tdecl->getTypeForDecl(;
+return true;
+  }
+  return false;
+}
+
 void ClangPersistentVariables::RegisterPersistentDecl(ConstString name,
   clang::NamedDecl *decl) {
   m_persistent_decls.insert(
Index: source/Commands/CommandObjectMemory.cpp
===
--- source/Commands/CommandObjectMemory.cpp
+++ source/Commands/CommandObjectMemory.cpp
@@ -6,16 +6,14 @@
 //
 //===--===//
 
-#include "clang/AST/Decl.h"
-
 #include "CommandObjectMemory.h"
-#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/DumpDataExtractor.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/ValueObjectMemory.h"
 #include "lldb/DataFormatters/ValueObjectPrinter.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -25,13 +23,14 @@
 #include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
 #include "lldb/Interpreter/OptionValueString.h"
 #include "lldb/Interpreter/Options.h"
-#include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/MemoryHistory.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -472,21 +471,13 @@
 exact_match, 1, searched_symbol_files,
 type_list);
 
-  if (type_list.GetSize() == 0 && lookup_type_name.GetCString() &&
-  *lookup_type_name.GetCString() == '$') {
-if (ClangPersistentVariables *persistent_vars =
-llvm::dyn_cast_or_null(
-target->GetPersistentExpressionStateForLanguage(
-lldb::eLanguageTypeC))) {
-  clang::TypeDecl *tdecl = llvm::dyn_cast_or_null(
-  persistent_vars->GetPersistentDecl(
-  ConstString(lookup_type_name)));
-
-  if (tdecl) {
-clang_ast_type.SetCompilerType(
-ClangASTContext::GetASTContext(&tdecl->getASTContext()),
-reinterpret_cast(
-

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::SetCompilerTypeFromPersistentDecl

2019-06-02 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd added inline comments.



Comment at: source/Commands/CommandObjectMemory.cpp:479
+if (persistent_vars->SetCompilerTypeFromPersistentDecl(
+lookup_type_name, clang_ast_type))
+  break;

Why is the parameter `clang_ast_type` and not based on 
`persistent_vars->getKind()`?



Comment at: 
source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp:58
+ConstString type_name, CompilerType &compiler_type) {
+  if (clang::TypeDecl *tdecl = llvm::dyn_cast_or_null(
+  GetPersistentDecl(type_name))) {

NIT: `decl` would be a nicer name


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

https://reviews.llvm.org/D62797



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

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::SetCompilerTypeFromPersistentDecl

2019-06-02 Thread Alex Langford via Phabricator via lldb-commits 
xiaobai marked an inline comment as done.
xiaobai added inline comments.



Comment at: source/Commands/CommandObjectMemory.cpp:479
+if (persistent_vars->SetCompilerTypeFromPersistentDecl(
+lookup_type_name, clang_ast_type))
+  break;

compnerd wrote:
> Why is the parameter `clang_ast_type` and not based on 
> `persistent_vars->getKind()`?
`clang_ast_type` is of type CompilerType. I was going to change the name in a 
follow up but I can do it in this change too.


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

https://reviews.llvm.org/D62797



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

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

This should get the build working again, so lets get this fixed, we can improve 
it later




Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:29
+  llvm::codeview::CPUType cpu_type;
+  if (arch_type == llvm::Triple::ArchType::aarch64) {
+cpu_type = llvm::codeview::CPUType::ARM64;

TomTan wrote:
> TomTan wrote:
> > compnerd wrote:
> > > aganea wrote:
> > > > Shouldn’t ArchType::aarch64_be and ArchType::aarch64_32 enums be 
> > > > handled here as well?
> > > I think that we should use a `switch` to cover the targets.  `/Oy` will 
> > > allow FPO on x86 as well.  There is also WoA (ARM32).
> > Seems no, aarch64_be or aarch64_32 is not supported by CodeView or Windows.
> Ok. It makes sense to use switch/case. CodeView doesn't support WoA (ARM32) 
> so no need to add case for it here.
Huh?  How does `cl` generate that then?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62772



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

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Tom Tan via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362349: [COFF, ARM64] Fix CodeView API change for 
getRegisterNames (authored by TomTan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62772?vs=202614&id=202628#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62772

Files:
  
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp


Index: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
===
--- 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -25,8 +25,19 @@
 
 static uint32_t ResolveLLDBRegisterNum(llvm::StringRef reg_name, 
llvm::Triple::ArchType arch_type) {
   // lookup register name to get lldb register number
+  llvm::codeview::CPUType cpu_type;
+  switch (arch_type) {
+case llvm::Triple::ArchType::aarch64:
+  cpu_type = llvm::codeview::CPUType::ARM64;
+  break;
+
+default:
+  cpu_type = llvm::codeview::CPUType::X64;
+  break;
+  }
+
   llvm::ArrayRef> register_names =
-  llvm::codeview::getRegisterNames();
+  llvm::codeview::getRegisterNames(cpu_type);
   auto it = llvm::find_if(
   register_names,
   [®_name](const llvm::EnumEntry ®ister_entry) {


Index: lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -25,8 +25,19 @@
 
 static uint32_t ResolveLLDBRegisterNum(llvm::StringRef reg_name, llvm::Triple::ArchType arch_type) {
   // lookup register name to get lldb register number
+  llvm::codeview::CPUType cpu_type;
+  switch (arch_type) {
+case llvm::Triple::ArchType::aarch64:
+  cpu_type = llvm::codeview::CPUType::ARM64;
+  break;
+
+default:
+  cpu_type = llvm::codeview::CPUType::X64;
+  break;
+  }
+
   llvm::ArrayRef> register_names =
-  llvm::codeview::getRegisterNames();
+  llvm::codeview::getRegisterNames(cpu_type);
   auto it = llvm::find_if(
   register_names,
   [®_name](const llvm::EnumEntry ®ister_entry) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

2019-06-02 Thread Tom Tan via Phabricator via lldb-commits 
TomTan added a comment.

In D62772#1526824 , @compnerd wrote:

> This should get the build working again, so lets get this fixed, we can 
> improve it later


Thanks, committed the fix to unblock build. For WoA(ARM32) which is not 
supported, I meant LLVM CodeView, not CodeView specification. 
`llvm::codeview::getRegisterNames(cpu_type)` will return incorrect register 
name even the right `CPUType` is passed in for WoA(ARM32), because it has not 
been implemented in LLVM CodeView yet.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62772



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

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

2019-06-02 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 202640.
aadsm marked an inline comment as done.
aadsm added a comment.
Herald added a subscriber: mgorny.

Address comments and add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62499

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/tools/lldb-server/tests/CMakeLists.txt
  lldb/unittests/tools/lldb-server/tests/CommunicationServerTest.cpp

Index: lldb/unittests/tools/lldb-server/tests/CommunicationServerTest.cpp
===
--- /dev/null
+++ lldb/unittests/tools/lldb-server/tests/CommunicationServerTest.cpp
@@ -0,0 +1,136 @@
+//===-- CommunicationServerTest.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Utility/Connection.h"
+
+namespace lldb_private {
+namespace process_gdb_remote {
+
+typedef std::shared_ptr> PacketsSP;
+// NB: This class doesn't use the override keyword to avoid
+// -Winconsistent-missing-override warnings from the compiler. The
+// inconsistency comes from the overriding definitions in the MOCK_*** macros.
+struct MockConnection : public lldb_private::Connection {
+  MockConnection(PacketsSP packets_sp) : m_packets_sp(packets_sp){};
+
+  MOCK_METHOD2(Connect,
+   lldb::ConnectionStatus(llvm::StringRef url, Status *error_ptr));
+  MOCK_METHOD5(Read, size_t(void *dst, size_t dst_len,
+const Timeout &timeout,
+lldb::ConnectionStatus &status, Status *error_ptr));
+  MOCK_METHOD0(GetURI, std::string());
+  MOCK_METHOD0(InterruptRead, bool());
+
+  lldb::ConnectionStatus Disconnect(Status *error_ptr) {
+return lldb::eConnectionStatusSuccess;
+  };
+
+  bool IsConnected() const { return true; };
+  size_t Write(const void *dst, size_t dst_len, lldb::ConnectionStatus &status,
+   Status *error_ptr) {
+std::string packet;
+for (size_t i = 0; i < dst_len; i++) {
+  packet.append(1, *(static_cast(dst) + i));
+}
+m_packets_sp->push_back(packet);
+return dst_len;
+  };
+
+  PacketsSP m_packets_sp;
+};
+
+struct MockServer : public GDBRemoteCommunicationServer {
+  MockServer()
+  : GDBRemoteCommunicationServer("mock-server", "mock-server.listener") {
+m_send_acks = false;
+m_send_error_strings = true;
+m_packets_sp = llvm::make_unique>();
+SetConnection(new testing::NiceMock(m_packets_sp));
+  }
+
+  PacketResult SendErrorResponse(uint8_t error) {
+return GDBRemoteCommunicationServer::SendErrorResponse(error);
+  }
+
+  PacketResult SendErrorResponse(const Status &error) {
+return GDBRemoteCommunicationServer::SendErrorResponse(error);
+  }
+
+  PacketResult SendErrorResponse(llvm::Error error) {
+return GDBRemoteCommunicationServer::SendErrorResponse(std::move(error));
+  }
+
+  const PacketsSP GetPackets() { return m_packets_sp; }
+
+  PacketsSP m_packets_sp;
+};
+
+TEST(CommunicationServerTest, SendErrorResponse_ErrorNumber) {
+  MockServer server;
+
+  server.SendErrorResponse(0x42);
+
+  EXPECT_EQ(server.GetPackets()->size(), 1UL);
+  EXPECT_EQ(server.GetPackets()->at(0), std::string("$E42#ab"));
+}
+
+TEST(CommunicationServerTest, SendErrorResponse_Status) {
+  MockServer server;
+  Status status;
+
+  status.SetError(0x42, lldb::eErrorTypeGeneric);
+  status.SetErrorString("Test error message");
+
+  server.SendErrorResponse(status);
+
+  EXPECT_EQ(server.GetPackets()->size(), 1UL);
+  EXPECT_EQ(server.GetPackets()->at(0),
+std::string("$E42;54657374206572726f72206d657373616765#ad"));
+}
+
+TEST(CommunicationServerTest, SendErrorResponse_UnimplementedError) {
+  MockServer server;
+
+  auto error =
+  llvm::make_error("Test unimplemented error");
+  server.SendErrorResponse(std::move(error));
+
+  EXPECT_EQ(server.GetPackets()->size(), 1UL);
+  EXPECT_EQ(server.GetPackets()->at(0), std::string("$#00"));
+}
+
+TEST(CommunicationServerTest, SendErrorResponse_StringError) {
+  MockServer server;
+
+  auto error = llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "String error test");
+
+  server.SendErrorResponse

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

2019-06-02 Thread António Afonso via Phabricator via lldb-commits 
aadsm marked an inline comment as done.
aadsm added inline comments.



Comment at: 
lldb/unittests/tools/lldb-server/tests/CommunicationServerTest.cpp:40-42
+for (size_t i = 0; i < dst_len; i++) {
+  packet.append(1, *(static_cast(dst) + i));
+}

Is there a better way to do this? I found the copy function but it accepts a 
`char *`, not a `const char *`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62499



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

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-02 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 202642.
aadsm added a comment.

Address commentds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62500

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Utility/AuxVector.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt

Index: lldb/source/Plugins/Process/Utility/CMakeLists.txt
===
--- lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ lldb/source/Plugins/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_library(lldbPluginProcessUtility PLUGIN
+  AuxVector.cpp
   DynamicRegisterInfo.cpp
   FreeBSDSignals.cpp
   GDBRemoteSignals.cpp
Index: lldb/source/Plugins/Process/Utility/AuxVector.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/AuxVector.h
@@ -0,0 +1,73 @@
+//===-- AuxVector.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_AuxVector_H_
+#define liblldb_AuxVector_H_
+
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Log.h"
+#include 
+
+class AuxVector {
+
+public:
+  AuxVector(lldb_private::DataExtractor &data);
+
+  /// Constants describing the type of entry.
+  /// On Linux, running "LD_SHOW_AUXV=1 ./executable" will spew AUX
+  /// information. Added AUXV prefix to avoid potential conflicts with system-
+  /// defined macros
+  enum EntryType {
+AUXV_AT_NULL = 0,  ///< End of auxv.
+AUXV_AT_IGNORE = 1,///< Ignore entry.
+AUXV_AT_EXECFD = 2,///< File descriptor of program.
+AUXV_AT_PHDR = 3,  ///< Program headers.
+AUXV_AT_PHENT = 4, ///< Size of program header.
+AUXV_AT_PHNUM = 5, ///< Number of program headers.
+AUXV_AT_PAGESZ = 6,///< Page size.
+AUXV_AT_BASE = 7,  ///< Interpreter base address.
+AUXV_AT_FLAGS = 8, ///< Flags.
+AUXV_AT_ENTRY = 9, ///< Program entry point.
+AUXV_AT_NOTELF = 10,   ///< Set if program is not an ELF.
+AUXV_AT_UID = 11,  ///< UID.
+AUXV_AT_EUID = 12, ///< Effective UID.
+AUXV_AT_GID = 13,  ///< GID.
+AUXV_AT_EGID = 14, ///< Effective GID.
+AUXV_AT_CLKTCK = 17,   ///< Clock frequency (e.g. times(2)).
+AUXV_AT_PLATFORM = 15, ///< String identifying platform.
+AUXV_AT_HWCAP =
+16, ///< Machine dependent hints about processor capabilities.
+AUXV_AT_FPUCW = 18, ///< Used FPU control word.
+AUXV_AT_DCACHEBSIZE = 19,   ///< Data cache block size.
+AUXV_AT_ICACHEBSIZE = 20,   ///< Instruction cache block size.
+AUXV_AT_UCACHEBSIZE = 21,   ///< Unified cache block size.
+AUXV_AT_IGNOREPPC = 22, ///< Entry should be ignored.
+AUXV_AT_SECURE = 23,///< Boolean, was exec setuid-like?
+AUXV_AT_BASE_PLATFORM = 24, ///< String identifying real platforms.
+AUXV_AT_RANDOM = 25,///< Address of 16 random bytes.
+AUXV_AT_EXECFN = 31,///< Filename of executable.
+AUXV_AT_SYSINFO = 32, ///< Pointer to the global system page used for system
+  /// calls and other nice things.
+AUXV_AT_SYSINFO_EHDR = 33,
+AUXV_AT_L1I_CACHESHAPE = 34, ///< Shapes of the caches.
+AUXV_AT_L1D_CACHESHAPE = 35,
+AUXV_AT_L2_CACHESHAPE = 36,
+AUXV_AT_L3_CACHESHAPE = 37,
+  };
+
+  llvm::Optional GetAuxValue(enum EntryType entry_type) const;
+  void DumpToLog(lldb_private::Log *log) const;
+  const char *GetEntryName(EntryType type) const;
+
+private:
+  void ParseAuxv(lldb_private::DataExtractor &data);
+
+  std::unordered_map m_auxv_entries;
+};
+
+#endif
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 
+#include "Plugins/Process/Utility/AuxVector.h"
 #include "lldb/Host/Debug.h"
 #include "lldb/Host/HostThread.h"
 #include "lldb/Host/linux/Support.h"
@@ -102,6 +103,8 @@
 return getProcFile(GetID(), "auxv");
   }
 
+  llvm::Optional GetA

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-02 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 202649.
aadsm added a comment.

Fix patch snafu


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62500

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Utility/AuxVector.cpp
  lldb/source/Plugins/Process/Utility/AuxVector.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt

Index: lldb/source/Plugins/Process/Utility/CMakeLists.txt
===
--- lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ lldb/source/Plugins/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_library(lldbPluginProcessUtility PLUGIN
+  AuxVector.cpp
   DynamicRegisterInfo.cpp
   FreeBSDSignals.cpp
   GDBRemoteSignals.cpp
Index: lldb/source/Plugins/Process/Utility/AuxVector.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/AuxVector.h
@@ -0,0 +1,73 @@
+//===-- AuxVector.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_AuxVector_H_
+#define liblldb_AuxVector_H_
+
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Log.h"
+#include 
+
+class AuxVector {
+
+public:
+  AuxVector(lldb_private::DataExtractor &data);
+
+  /// Constants describing the type of entry.
+  /// On Linux, running "LD_SHOW_AUXV=1 ./executable" will spew AUX
+  /// information. Added AUXV prefix to avoid potential conflicts with system-
+  /// defined macros
+  enum EntryType {
+AUXV_AT_NULL = 0,  ///< End of auxv.
+AUXV_AT_IGNORE = 1,///< Ignore entry.
+AUXV_AT_EXECFD = 2,///< File descriptor of program.
+AUXV_AT_PHDR = 3,  ///< Program headers.
+AUXV_AT_PHENT = 4, ///< Size of program header.
+AUXV_AT_PHNUM = 5, ///< Number of program headers.
+AUXV_AT_PAGESZ = 6,///< Page size.
+AUXV_AT_BASE = 7,  ///< Interpreter base address.
+AUXV_AT_FLAGS = 8, ///< Flags.
+AUXV_AT_ENTRY = 9, ///< Program entry point.
+AUXV_AT_NOTELF = 10,   ///< Set if program is not an ELF.
+AUXV_AT_UID = 11,  ///< UID.
+AUXV_AT_EUID = 12, ///< Effective UID.
+AUXV_AT_GID = 13,  ///< GID.
+AUXV_AT_EGID = 14, ///< Effective GID.
+AUXV_AT_CLKTCK = 17,   ///< Clock frequency (e.g. times(2)).
+AUXV_AT_PLATFORM = 15, ///< String identifying platform.
+AUXV_AT_HWCAP =
+16, ///< Machine dependent hints about processor capabilities.
+AUXV_AT_FPUCW = 18, ///< Used FPU control word.
+AUXV_AT_DCACHEBSIZE = 19,   ///< Data cache block size.
+AUXV_AT_ICACHEBSIZE = 20,   ///< Instruction cache block size.
+AUXV_AT_UCACHEBSIZE = 21,   ///< Unified cache block size.
+AUXV_AT_IGNOREPPC = 22, ///< Entry should be ignored.
+AUXV_AT_SECURE = 23,///< Boolean, was exec setuid-like?
+AUXV_AT_BASE_PLATFORM = 24, ///< String identifying real platforms.
+AUXV_AT_RANDOM = 25,///< Address of 16 random bytes.
+AUXV_AT_EXECFN = 31,///< Filename of executable.
+AUXV_AT_SYSINFO = 32, ///< Pointer to the global system page used for system
+  /// calls and other nice things.
+AUXV_AT_SYSINFO_EHDR = 33,
+AUXV_AT_L1I_CACHESHAPE = 34, ///< Shapes of the caches.
+AUXV_AT_L1D_CACHESHAPE = 35,
+AUXV_AT_L2_CACHESHAPE = 36,
+AUXV_AT_L3_CACHESHAPE = 37,
+  };
+
+  llvm::Optional GetAuxValue(enum EntryType entry_type) const;
+  void DumpToLog(lldb_private::Log *log) const;
+  const char *GetEntryName(EntryType type) const;
+
+private:
+  void ParseAuxv(lldb_private::DataExtractor &data);
+
+  std::unordered_map m_auxv_entries;
+};
+
+#endif
Index: lldb/source/Plugins/Process/Utility/AuxVector.cpp
===
--- lldb/source/Plugins/Process/Utility/AuxVector.cpp
+++ lldb/source/Plugins/Process/Utility/AuxVector.cpp
@@ -7,98 +7,59 @@
 //===--===//
 
 #include "AuxVector.h"
-#include "lldb/Target/Process.h"
-#include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/DataExtractor.h"
-#incl

[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

2019-06-02 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 202651.
aadsm added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62501

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h

Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -131,11 +131,14 @@
 protected:
   llvm::Expected>
   GetSoftwareBreakpointTrapOpcode(size_t size_hint) override;
+  template 
+  lldb::addr_t GetELFImageInfoAddress();
 
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
   std::unique_ptr m_aux_vector;
+  lldb::addr_t m_elf_image_info_addr = LLDB_INVALID_ADDRESS;
 
   LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector> m_mem_region_cache;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -38,6 +38,7 @@
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringExtractor.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
@@ -1390,8 +1391,12 @@
 }
 
 lldb::addr_t NativeProcessLinux::GetSharedLibraryInfoAddress() {
-  // punt on this for now
-  return LLDB_INVALID_ADDRESS;
+  if (GetAddressByteSize() == 8)
+return GetELFImageInfoAddress();
+  else
+return GetELFImageInfoAddress();
 }
 
 size_t NativeProcessLinux::UpdateThreads() {
@@ -2097,3 +2102,77 @@
 
   return m_aux_vector->GetAuxValue(type);
 }
+
+template 
+lldb::addr_t NativeProcessLinux::GetELFImageInfoAddress() {
+  if (m_elf_image_info_addr != LLDB_INVALID_ADDRESS)
+return m_elf_image_info_addr;
+
+  llvm::Optional maybe_phdr_addr =
+  GetAuxValue(AuxVector::AUXV_AT_PHDR);
+  llvm::Optional maybe_phdr_entry_size =
+  GetAuxValue(AuxVector::AUXV_AT_PHENT);
+  llvm::Optional maybe_phdr_num_entries =
+  GetAuxValue(AuxVector::AUXV_AT_PHNUM);
+  if (!maybe_phdr_addr || !maybe_phdr_entry_size || !maybe_phdr_num_entries)
+return LLDB_INVALID_ADDRESS;
+  lldb::addr_t phdr_addr = *maybe_phdr_addr;
+  size_t phdr_entry_size = *maybe_phdr_entry_size;
+  size_t phdr_num_entries = *maybe_phdr_num_entries;
+  lldb::addr_t load_base = phdr_addr - sizeof(ELF_EHDR);
+
+  // Find the PT_DYNAMIC segment (.dynamic section) in the program header and
+  // what the ELF believes to be the load base.
+  lldb::addr_t elf_load_base = 0;
+  bool found_elf_load_base = false;
+  lldb::addr_t dynamic_section_addr = 0;
+  uint64_t dynamic_section_size = 0;
+  bool found_dynamic_section = false;
+  ELF_PHDR phdr_entry;
+  for (size_t i = 0; i < phdr_num_entries; i++) {
+size_t bytes_read;
+auto error = ReadMemory(phdr_addr + i * phdr_entry_size, &phdr_entry,
+sizeof(phdr_entry), bytes_read);
+if (!error.Success())
+  return LLDB_INVALID_ADDRESS;
+
+if ((!found_elf_load_base || phdr_entry.p_vaddr < elf_load_base) &&
+phdr_entry.p_type == llvm::ELF::PT_LOAD) {
+  elf_load_base = phdr_entry.p_vaddr;
+  found_elf_load_base = true;
+}
+
+if (phdr_entry.p_type == llvm::ELF::PT_DYNAMIC) {
+  dynamic_section_addr = phdr_entry.p_vaddr;
+  dynamic_section_size = phdr_entry.p_memsz;
+  found_dynamic_section = true;
+}
+  }
+
+  if (!found_elf_load_base || !found_dynamic_section)
+return LLDB_INVALID_ADDRESS;
+
+  // Find the DT_DEBUG entry in the .dynamic section
+  // The load base we got from the auxiliary vector is the source of truth. If
+  // we got a different load base from the ELF then we need to correct the
+  // .dynamic section address with the difference we found.
+  dynamic_section_addr += (load_base - elf_load_base);
+  ELF_DYN dynamic_entry;
+  size_t dynamic_num_entries = dynamic_section_size / sizeof(dynamic_entry);
+  for (size_t i = 0; i < dynamic_num_entries; i++) {
+size_t bytes_read;
+auto error = ReadMemory(dynamic_section_addr + i * sizeof(dynamic_entry),
+&dynamic_entry, sizeof(dynamic_entry), bytes_read);
+if (!error.Success())
+  return LLDB_INVALID_ADDRESS;
+// Return the &DT_DEBUG->d_ptr which points to r_debug which contains the
+// link_map.
+if (dynamic_entry.d_tag == llvm::ELF::DT_DEBUG) {
+  m_elf_image_info_addr = dynamic_section_addr + i * sizeof(dynamic_entry) +
+  sizeof(dynamic_entry.d_tag);
+  return m_elf_image_info_addr;
+}
+  }
+
+  return LLDB_INVALID_ADDRESS;
+}
Index: lldb

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-02 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 202652.
aadsm added a comment.

Address comments and add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteLibrariesSvr4Support.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2765,6 +2765,27 @@
 return std::move(*buffer_or_error);
   }
 
+#if defined(__linux__)
+  if (object == "libraries-svr4") {
+std::vector library_list;
+auto status = m_debugged_process_up->GetLoadedSharedLibraries(library_list);
+if (!status.Success())
+  return status.ToError();
+
+StreamString response;
+response.Printf("");
+for (auto const &library : library_list) {
+  response.Printf("", library.ld_addr);
+}
+response.Printf("");
+return std::move(
+MemoryBuffer::getMemBufferCopy(response.GetString(), __FUNCTION__));
+  }
+#endif
+
   return llvm::make_error(
   "Xfer object not supported");
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -826,6 +826,9 @@
   response.PutCString(";QPassSignals+");
   response.PutCString(";qXfer:auxv:read+");
 #endif
+#if defined(__linux__)
+  response.PutCString(";qXfer:libraries-svr4:read+");
+#endif
 
   return SendPacketNoLock(response.GetString());
 }
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -79,6 +79,9 @@
 
   lldb::addr_t GetSharedLibraryInfoAddress() override;
 
+  Status GetLoadedSharedLibraries(
+  std::vector &library_list) override;
+
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }
@@ -134,6 +137,17 @@
   template 
   lldb::addr_t GetELFImageInfoAddress();
 
+  template  struct ELFLinkMap {
+T l_addr;
+T l_name;
+T l_ld;
+T l_next;
+T l_prev;
+  };
+  template 
+  Status ReadSharedLibraryInfo(lldb::addr_t link_map_addr,
+   SharedLibraryInfo &info);
+
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2176,3 +2176,76 @@
 
   return LLDB_INVALID_ADDRESS;
 }
+
+template 
+Status NativeProcessLinux::ReadSharedLibraryInfo(lldb::addr_t link_map_addr,
+ SharedLibraryInfo &info) {
+  ELFLinkMap link_map;
+  size_t bytes_read;
+  auto error =
+  ReadMemory(link_map_addr, &link_map, sizeof(link_map), bytes_read);
+  if (!error.Success())
+return error;
+
+  char name_buffer[PATH_MAX];
+  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
+ bytes_read);
+  if (!error.Success())
+return error;
+
+  info.name = std::string(name_buffer);
+  info.link_map = link_map_addr;
+  info.base_addr = link_map.l_addr;
+  info.ld_addr = link_map.l_ld;
+  info.next = link_map.l_next;
+#if defined(__linux__) && !defined(__ANDROID__)
+  // On non-android linux systems, main executable has an empty path.
+  info.main = info.name.empty();
+#elif defined(__linux__) && defined(__ANDROID__)
+  // On android, the main executable has a load address of 0.
+  info.main = info.ld_addr == 0;
+#else
+  info.main = false;
+#endif
+
+  return Status();
+}
+
+Status NativeProcessLinux::GetLoadedSharedLibraries(
+std::vector &library_list) {
+  // Address of DT_DEBUG.d_ptr which points to r_debug
+  lldb::addr_t info_address = GetSharedLibraryInfoAddress();
+  if (info_address == LLDB_INVALID_ADDRESS)
+return Status("Invalid shared library info address");
+  // Address of r_debug
+  lldb::addr_t address = 0;
+  size_t bytes_read;

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-02 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 202654.
aadsm added a comment.

Missing clang-format-diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62500

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Utility/AuxVector.cpp
  lldb/source/Plugins/Process/Utility/AuxVector.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt

Index: lldb/source/Plugins/Process/Utility/CMakeLists.txt
===
--- lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ lldb/source/Plugins/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_library(lldbPluginProcessUtility PLUGIN
+  AuxVector.cpp
   DynamicRegisterInfo.cpp
   FreeBSDSignals.cpp
   GDBRemoteSignals.cpp
Index: lldb/source/Plugins/Process/Utility/AuxVector.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/AuxVector.h
@@ -0,0 +1,73 @@
+//===-- AuxVector.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_AuxVector_H_
+#define liblldb_AuxVector_H_
+
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Log.h"
+#include 
+
+class AuxVector {
+
+public:
+  AuxVector(lldb_private::DataExtractor &data);
+
+  /// Constants describing the type of entry.
+  /// On Linux, running "LD_SHOW_AUXV=1 ./executable" will spew AUX
+  /// information. Added AUXV prefix to avoid potential conflicts with system-
+  /// defined macros
+  enum EntryType {
+AUXV_AT_NULL = 0,  ///< End of auxv.
+AUXV_AT_IGNORE = 1,///< Ignore entry.
+AUXV_AT_EXECFD = 2,///< File descriptor of program.
+AUXV_AT_PHDR = 3,  ///< Program headers.
+AUXV_AT_PHENT = 4, ///< Size of program header.
+AUXV_AT_PHNUM = 5, ///< Number of program headers.
+AUXV_AT_PAGESZ = 6,///< Page size.
+AUXV_AT_BASE = 7,  ///< Interpreter base address.
+AUXV_AT_FLAGS = 8, ///< Flags.
+AUXV_AT_ENTRY = 9, ///< Program entry point.
+AUXV_AT_NOTELF = 10,   ///< Set if program is not an ELF.
+AUXV_AT_UID = 11,  ///< UID.
+AUXV_AT_EUID = 12, ///< Effective UID.
+AUXV_AT_GID = 13,  ///< GID.
+AUXV_AT_EGID = 14, ///< Effective GID.
+AUXV_AT_CLKTCK = 17,   ///< Clock frequency (e.g. times(2)).
+AUXV_AT_PLATFORM = 15, ///< String identifying platform.
+AUXV_AT_HWCAP =
+16, ///< Machine dependent hints about processor capabilities.
+AUXV_AT_FPUCW = 18, ///< Used FPU control word.
+AUXV_AT_DCACHEBSIZE = 19,   ///< Data cache block size.
+AUXV_AT_ICACHEBSIZE = 20,   ///< Instruction cache block size.
+AUXV_AT_UCACHEBSIZE = 21,   ///< Unified cache block size.
+AUXV_AT_IGNOREPPC = 22, ///< Entry should be ignored.
+AUXV_AT_SECURE = 23,///< Boolean, was exec setuid-like?
+AUXV_AT_BASE_PLATFORM = 24, ///< String identifying real platforms.
+AUXV_AT_RANDOM = 25,///< Address of 16 random bytes.
+AUXV_AT_EXECFN = 31,///< Filename of executable.
+AUXV_AT_SYSINFO = 32, ///< Pointer to the global system page used for system
+  /// calls and other nice things.
+AUXV_AT_SYSINFO_EHDR = 33,
+AUXV_AT_L1I_CACHESHAPE = 34, ///< Shapes of the caches.
+AUXV_AT_L1D_CACHESHAPE = 35,
+AUXV_AT_L2_CACHESHAPE = 36,
+AUXV_AT_L3_CACHESHAPE = 37,
+  };
+
+  llvm::Optional GetAuxValue(enum EntryType entry_type) const;
+  void DumpToLog(lldb_private::Log *log) const;
+  const char *GetEntryName(EntryType type) const;
+
+private:
+  void ParseAuxv(lldb_private::DataExtractor &data);
+
+  std::unordered_map m_auxv_entries;
+};
+
+#endif
Index: lldb/source/Plugins/Process/Utility/AuxVector.cpp
===
--- lldb/source/Plugins/Process/Utility/AuxVector.cpp
+++ lldb/source/Plugins/Process/Utility/AuxVector.cpp
@@ -7,98 +7,59 @@
 //===--===//
 
 #include "AuxVector.h"
-#include "lldb/Target/Process.h"
-#include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/DataExtractor