date:20180926

[Lldb-commits] [lldb] r343076 - Fix a memory read bug in lldb-server

2018-09-26 Thread Pavel Labath via lldb-commits

Author: labath
Date: Wed Sep 26 00:31:41 2018
New Revision: 343076

URL: http://llvm.org/viewvc/llvm-project?rev=343076&view=rev
Log:
Fix a memory read bug in lldb-server

NativeProcessProtocol::ReadMemoryWithoutTrap had a bug, where it failed
to properly remove inserted breakpoint opcodes if the memory read
partially overlapped the trap opcode. This could not happen on x86
because it has a one-byte breakpoint instruction, but it could happen on
arm, which has a 4-byte breakpoint instruction (in arm mode).

Since triggerring this condition would only be possible on an arm
machine (and even then it would be a bit tricky). I test this using a
NativeProcessProtocol unit test.

Modified:
lldb/trunk/source/Host/common/NativeBreakpointList.cpp
lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp

Modified: lldb/trunk/source/Host/common/NativeBreakpointList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeBreakpointList.cpp?rev=343076&r1=343075&r2=343076&view=diff
==
--- lldb/trunk/source/Host/common/NativeBreakpointList.cpp (original)
+++ lldb/trunk/source/Host/common/NativeBreakpointList.cpp Wed Sep 26 00:31:41 
2018
@@ -210,20 +210,31 @@ Status NativeBreakpointList::GetBreakpoi
 
 Status NativeBreakpointList::RemoveTrapsFromBuffer(lldb::addr_t addr, void 
*buf,
size_t size) const {
+  auto data = llvm::makeMutableArrayRef(static_cast(buf), size);
   for (const auto &map : m_breakpoints) {
-lldb::addr_t bp_addr = map.first;
-// Breapoint not in range, ignore
-if (bp_addr < addr || addr + size <= bp_addr)
-  continue;
 const auto &bp_sp = map.second;
 // Not software breakpoint, ignore
 if (!bp_sp->IsSoftwareBreakpoint())
   continue;
 auto software_bp_sp = std::static_pointer_cast(bp_sp);
-auto opcode_addr = static_cast(buf) + bp_addr - addr;
-auto saved_opcodes = software_bp_sp->m_saved_opcodes;
+
+lldb::addr_t bp_addr = map.first;
 auto opcode_size = software_bp_sp->m_opcode_size;
-::memcpy(opcode_addr, saved_opcodes, opcode_size);
+
+// Breapoint not in range, ignore
+if (bp_addr + opcode_size < addr || addr + size <= bp_addr)
+  continue;
+
+auto saved_opcodes =
+llvm::makeArrayRef(software_bp_sp->m_saved_opcodes, opcode_size);
+if (bp_addr < addr) {
+  saved_opcodes = saved_opcodes.drop_front(addr - bp_addr);
+  bp_addr = addr;
+}
+auto bp_data = data.drop_front(bp_addr - addr);
+std::copy_n(saved_opcodes.begin(),
+std::min(saved_opcodes.size(), bp_data.size()),
+bp_data.begin());
   }
   return Status();
 }

Modified: lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp?rev=343076&r1=343075&r2=343076&view=diff
==
--- lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp (original)
+++ lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp Wed Sep 26 00:31:41 
2018
@@ -70,10 +70,22 @@ public:
   llvm::ArrayRef Data));
 
   using NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode;
+  llvm::Expected> ReadMemoryWithoutTrap(addr_t Addr,
+ size_t Size);
 
 private:
   ArchSpec Arch;
 };
+
+class FakeMemory {
+public:
+  FakeMemory(llvm::ArrayRef Data) : Data(Data) {}
+  llvm::Expected> Read(addr_t Addr, size_t Size);
+  llvm::Expected Write(addr_t Addr, llvm::ArrayRef Chunk);
+
+private:
+  std::vector Data;
+};
 } // namespace
 
 Status MockProcess::ReadMemory(addr_t Addr, void *Buf, size_t Size,
@@ -101,6 +113,37 @@ Status MockProcess::WriteMemory(addr_t A
   return Status();
 }
 
+llvm::Expected>
+MockProcess::ReadMemoryWithoutTrap(addr_t Addr, size_t Size) {
+  std::vector Data(Size, 0);
+  size_t BytesRead;
+  Status ST = NativeProcessProtocol::ReadMemoryWithoutTrap(
+  Addr, Data.data(), Data.size(), BytesRead);
+  if (ST.Fail())
+return ST.ToError();
+  Data.resize(BytesRead);
+  return std::move(Data);
+}
+
+llvm::Expected> FakeMemory::Read(addr_t Addr,
+  size_t Size) {
+  if (Addr >= Data.size())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Address out of range.");
+  Size = std::min(Size, Data.size() - Addr);
+  return std::vector(&Data[Addr], &Data[Addr + Size]);
+}
+
+llvm::Expected FakeMemory::Write(addr_t Addr,
+ llvm::ArrayRef Chunk) {
+  if (Addr >= Data.size())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Address out of range.");
+  size_t Size = std::min(Chunk.size(), Data.size() - Addr);
+  std::cop

[Lldb-commits] [PATCH] D52532: Pull GetSoftwareBreakpointPCOffset into base class

2018-09-26 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: krytarowski, zturner.

This function encodes the knowledge of whether the PC points to the
breakpoint instruction of the one following it after the breakpoint is
"hit". This behavior mainly(*) depends on the architecture and not on the
OS, so it makes sense for it to be implemented in the base class, where
it can be shared between different implementations (Linux and NetBSD
atm).

(*) It is possible for an OS to expose a different API, perhaps by doing
some fixups in the kernel. In this case, the implementation can override
this function to implement custom behavior.


https://reviews.llvm.org/D52532

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

Index: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -322,35 +322,16 @@
   return error;
 }
 
-Status NativeProcessNetBSD::GetSoftwareBreakpointPCOffset(
-uint32_t &actual_opcode_size) {
-  // FIXME put this behind a breakpoint protocol class that can be
-  // set per architecture.  Need ARM, MIPS support here.
-  static const uint8_t g_i386_opcode[] = {0xCC};
-  switch (m_arch.GetMachine()) {
-  case llvm::Triple::x86_64:
-actual_opcode_size = static_cast(sizeof(g_i386_opcode));
-return Status();
-  default:
-assert(false && "CPU type not supported!");
-return Status("CPU type not supported");
-  }
-}
-
 Status
 NativeProcessNetBSD::FixupBreakpointPCAsNeeded(NativeThreadNetBSD &thread) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_BREAKPOINTS));
   Status error;
   // Find out the size of a breakpoint (might depend on where we are in the
   // code).
   NativeRegisterContext& context = thread.GetRegisterContext();
-  uint32_t breakpoint_size = 0;
-  error = GetSoftwareBreakpointPCOffset(breakpoint_size);
-  if (error.Fail()) {
-LLDB_LOG(log, "GetBreakpointSize() failed: {0}", error);
-return error;
-  } else
-LLDB_LOG(log, "breakpoint size: {0}", breakpoint_size);
+  uint32_t breakpoint_size = GetSoftwareBreakpointPCOffset();
+  LLDB_LOG(log, "breakpoint size: {0}", breakpoint_size);
+
   // First try probing for a breakpoint at a software breakpoint location: PC -
   // breakpoint size.
   const lldb::addr_t initial_pc_addr =
Index: source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- source/Plugins/Process/Linux/NativeProcessLinux.h
+++ source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -182,8 +182,6 @@
 
   NativeThreadLinux &AddThread(lldb::tid_t thread_id);
 
-  Status GetSoftwareBreakpointPCOffset(uint32_t &actual_opcode_size);
-
   Status FixupBreakpointPCAsNeeded(NativeThreadLinux &thread);
 
   /// Writes a siginfo_t structure corresponding to the given thread ID to the
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1502,40 +1502,6 @@
   return m_threads.size();
 }
 
-Status NativeProcessLinux::GetSoftwareBreakpointPCOffset(
-uint32_t &actual_opcode_size) {
-  // FIXME put this behind a breakpoint protocol class that can be
-  // set per architecture.  Need ARM, MIPS support here.
-  static const uint8_t g_i386_opcode[] = {0xCC};
-  static const uint8_t g_s390x_opcode[] = {0x00, 0x01};
-
-  switch (m_arch.GetMachine()) {
-  case llvm::Triple::x86:
-  case llvm::Triple::x86_64:
-actual_opcode_size = static_cast(sizeof(g_i386_opcode));
-return Status();
-
-  case llvm::Triple::systemz:
-actual_opcode_size = static_cast(sizeof(g_s390x_opcode));
-return Status();
-
-  case llvm::Triple::arm:
-  case llvm::Triple::aarch64:
-  case llvm::Triple::mips64:
-  case llvm::Triple::mips64el:
-  case llvm::Triple::mips:
-  case llvm::Triple::mipsel:
-  case llvm::Triple::ppc64le:
-// On these architectures the PC don't get updated for breakpoint hits
-actual_opcode_size = 0;
-return Status();
-
-  default:
-assert(false && "CPU type not supported!");
-return Status("CPU type not supported");
-  }
-}
-
 Status NativeProcessLinux::SetBreakpoint(lldb::addr_t addr, uint32_t size,
  bool hardware) {
   if (hardware)
@@ -1763,13 +1729,8 @@
   // code).
   NativeRegisterContext &context = thread.GetRegisterContext();
 
-  uint32_t breakpoint_size = 0;
-  error = GetSoftwareBreakpointPCOffset(breakpoint_size);
-  if (error.Fail()) {
-LLDB_LOG(log, "GetBreakpointSize() failed: {0}", error);
-return error;
-  } else

[Lldb-commits] [PATCH] D52532: Pull GetSoftwareBreakpointPCOffset into base class

2018-09-26 Thread Kamil Rytarowski via Phabricator via lldb-commits

krytarowski added a comment.

I was wondering whether we want to normalize this inside the kernel and always 
advance the Program Counter.. but it's easier to manage it in userland.


https://reviews.llvm.org/D52532



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

[Lldb-commits] [PATCH] D52501: [PDB] Restore the calling convention from PDB

2018-09-26 Thread Aleksandr Urakov via Phabricator via lldb-commits

aleksandr.urakov updated this revision to Diff 167081.
aleksandr.urakov added a comment.

Thanks!

I just have extracted the test cases in a separate test and have specified 
`-m32` key, so the test can run also on 64-bit machines.


https://reviews.llvm.org/D52501

Files:
  include/lldb/Symbol/ClangASTContext.h
  lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
  lit/SymbolFile/PDB/calling-conventions.test
  lit/SymbolFile/PDB/pointers.test
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -2058,7 +2058,8 @@
 
 CompilerType ClangASTContext::CreateFunctionType(
 ASTContext *ast, const CompilerType &result_type, const CompilerType *args,
-unsigned num_args, bool is_variadic, unsigned type_quals) {
+unsigned num_args, bool is_variadic, unsigned type_quals,
+clang::CallingConv cc) {
   if (ast == nullptr)
 return CompilerType(); // invalid AST
 
@@ -2086,6 +2087,7 @@
 
   // TODO: Detect calling convention in DWARF?
   FunctionProtoType::ExtProtoInfo proto_info;
+  proto_info.ExtInfo = cc;
   proto_info.Variadic = is_variadic;
   proto_info.ExceptionSpec = EST_None;
   proto_info.TypeQuals = type_quals;
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -331,6 +331,26 @@
   return name == "`anonymous namespace'" || name == "`anonymous-namespace'";
 }
 
+static clang::CallingConv TranslateCallingConvention(PDB_CallingConv pdb_cc) {
+  switch (pdb_cc) {
+  case llvm::codeview::CallingConvention::NearC:
+return clang::CC_C;
+  case llvm::codeview::CallingConvention::NearStdCall:
+return clang::CC_X86StdCall;
+  case llvm::codeview::CallingConvention::NearFast:
+return clang::CC_X86FastCall;
+  case llvm::codeview::CallingConvention::ThisCall:
+return clang::CC_X86ThisCall;
+  case llvm::codeview::CallingConvention::NearVector:
+return clang::CC_X86VectorCall;
+  case llvm::codeview::CallingConvention::NearPascal:
+return clang::CC_X86Pascal;
+  default:
+assert(false && "Unknown calling convention");
+return clang::CC_C;
+  }
+}
+
 PDBASTParser::PDBASTParser(lldb_private::ClangASTContext &ast) : m_ast(ast) {}
 
 PDBASTParser::~PDBASTParser() {}
@@ -603,9 +623,10 @@
   type_quals |= clang::Qualifiers::Const;
 if (func_sig->isVolatileType())
   type_quals |= clang::Qualifiers::Volatile;
+auto cc = TranslateCallingConvention(func_sig->getCallingConvention());
 CompilerType func_sig_ast_type =
 m_ast.CreateFunctionType(return_ast_type, arg_list.data(),
- arg_list.size(), is_variadic, type_quals);
+ arg_list.size(), is_variadic, type_quals, cc);
 
 GetDeclarationForSymbol(type, decl);
 return std::make_shared(
Index: lit/SymbolFile/PDB/pointers.test
===
--- lit/SymbolFile/PDB/pointers.test
+++ lit/SymbolFile/PDB/pointers.test
@@ -28,7 +28,7 @@
 MAIN: Variable{{.*}}, name = "p_member_field"
 MAIN-SAME:(int ST::*), scope = local
 MAIN: Variable{{.*}}, name = "p_member_method"
-MAIN-SAME:(int (ST::*)(int)), scope = local
+MAIN-SAME:(int (ST::*)(int) __attribute__((thiscall))), scope = local
 
 F:   Function{[[FID2:.*]]}, demangled = {{.*}}f(int)
 F-NEXT:  Block{[[FID2]]}
Index: lit/SymbolFile/PDB/calling-conventions.test
===
--- /dev/null
+++ lit/SymbolFile/PDB/calling-conventions.test
@@ -0,0 +1,11 @@
+REQUIRES: windows, lld
+RUN: clang-cl -m32 /Zi /GS- /c %S/Inputs/CallingConventionsTest.cpp /o %t.obj
+RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe
+RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s
+
+CHECK: Module: {{.*}}
+CHECK-DAG: int (*FuncCCallPtr)();
+CHECK-DAG: int (*FuncStdCallPtr)() __attribute__((stdcall));
+CHECK-DAG: int (*FuncFastCallPtr)() __attribute__((fastcall));
+CHECK-DAG: int (*FuncVectorCallPtr)() __attribute__((vectorcall));
+CHECK-DAG: int (S::*FuncThisCallPtr)() __attribute__((thiscall));
Index: lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
===
--- /dev/null
+++ lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
@@ -0,0 +1,20 @@
+int FuncCCall() { return 0; }
+auto FuncCCallPtr = &FuncCCall;
+
+int __stdcall FuncStdCall() { return 0; }
+auto FuncStdCallPtr = &FuncStdCall;
+
+int __fastcall FuncFastCall() { return 0; }
+auto FuncFastCallPtr = &FuncFastCall;
+
+int __vectorcall FuncVectorCall() { return 0; }
+auto FuncVectorCallPtr = &FuncVectorCall;
+
+struct S {
+  int FuncThisCall() { return 0; }
+};
+

[Lldb-commits] [lldb] r343084 - [PDB] Restore the calling convention from PDB

2018-09-26 Thread Aleksandr Urakov via lldb-commits

Author: aleksandr.urakov
Date: Wed Sep 26 02:03:34 2018
New Revision: 343084

URL: http://llvm.org/viewvc/llvm-project?rev=343084&view=rev
Log:
[PDB] Restore the calling convention from PDB

Summary:
This patch implements restoring of the calling convention from PDB.
It is necessary for expressions evaluation, if we want to call a function
of the debuggee process with a calling convention other than ccall.

Reviewers: clayborg, zturner, labath, asmith

Reviewed By: clayborg

Subscribers: teemperor, lldb-commits, stella.stamenova

Tags: #lldb

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

Added:
lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test
Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/lit/SymbolFile/PDB/pointers.test
lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=343084&r1=343083&r2=343084&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Wed Sep 26 02:03:34 2018
@@ -377,7 +377,17 @@ public:
  const CompilerType &result_type,
  const CompilerType *args,
  unsigned num_args, bool is_variadic,
- unsigned type_quals);
+ unsigned type_quals,
+ clang::CallingConv cc);
+
+  static CompilerType CreateFunctionType(clang::ASTContext *ast,
+ const CompilerType &result_type,
+ const CompilerType *args,
+ unsigned num_args, bool is_variadic,
+ unsigned type_quals) {
+return ClangASTContext::CreateFunctionType(
+ast, result_type, args, num_args, is_variadic, type_quals, 
clang::CC_C);
+  }
 
   CompilerType CreateFunctionType(const CompilerType &result_type,
   const CompilerType *args, unsigned num_args,
@@ -386,6 +396,15 @@ public:
 getASTContext(), result_type, args, num_args, is_variadic, type_quals);
   }
 
+  CompilerType CreateFunctionType(const CompilerType &result_type,
+  const CompilerType *args, unsigned num_args,
+  bool is_variadic, unsigned type_quals,
+  clang::CallingConv cc) {
+return ClangASTContext::CreateFunctionType(getASTContext(), result_type,
+   args, num_args, is_variadic,
+   type_quals, cc);
+  }
+
   clang::ParmVarDecl *CreateParameterDeclaration(const char *name,
  const CompilerType 
¶m_type,
  int storage);

Added: lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp?rev=343084&view=auto
==
--- lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp (added)
+++ lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp Wed Sep 26 
02:03:34 2018
@@ -0,0 +1,20 @@
+int FuncCCall() { return 0; }
+auto FuncCCallPtr = &FuncCCall;
+
+int __stdcall FuncStdCall() { return 0; }
+auto FuncStdCallPtr = &FuncStdCall;
+
+int __fastcall FuncFastCall() { return 0; }
+auto FuncFastCallPtr = &FuncFastCall;
+
+int __vectorcall FuncVectorCall() { return 0; }
+auto FuncVectorCallPtr = &FuncVectorCall;
+
+struct S {
+  int FuncThisCall() { return 0; }
+};
+auto FuncThisCallPtr = &S::FuncThisCall;
+
+int main() {
+  return 0;
+}

Added: lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test?rev=343084&view=auto
==
--- lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test (added)
+++ lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test Wed Sep 26 02:03:34 
2018
@@ -0,0 +1,11 @@
+REQUIRES: windows, lld
+RUN: clang-cl -m32 /Zi /GS- /c %S/Inputs/CallingConventionsTest.cpp /o %t.obj
+RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe
+RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s
+
+CHECK: Module: {{.*}}
+CHECK-DAG: int (*FuncCCallPtr)();
+CHECK-DAG: int (*FuncStdCallPtr)() __attribute__((stdcall));
+

[Lldb-commits] [PATCH] D52501: [PDB] Restore the calling convention from PDB

2018-09-26 Thread Aleksandr Urakov via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343084: [PDB] Restore the calling convention from PDB 
(authored by aleksandr.urakov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52501?vs=167081&id=167084#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52501

Files:
  lldb/trunk/include/lldb/Symbol/ClangASTContext.h
  lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
  lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test
  lldb/trunk/lit/SymbolFile/PDB/pointers.test
  lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp

Index: lldb/trunk/source/Symbol/ClangASTContext.cpp
===
--- lldb/trunk/source/Symbol/ClangASTContext.cpp
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp
@@ -2058,7 +2058,8 @@
 
 CompilerType ClangASTContext::CreateFunctionType(
 ASTContext *ast, const CompilerType &result_type, const CompilerType *args,
-unsigned num_args, bool is_variadic, unsigned type_quals) {
+unsigned num_args, bool is_variadic, unsigned type_quals,
+clang::CallingConv cc) {
   if (ast == nullptr)
 return CompilerType(); // invalid AST
 
@@ -2086,6 +2087,7 @@
 
   // TODO: Detect calling convention in DWARF?
   FunctionProtoType::ExtProtoInfo proto_info;
+  proto_info.ExtInfo = cc;
   proto_info.Variadic = is_variadic;
   proto_info.ExceptionSpec = EST_None;
   proto_info.TypeQuals = type_quals;
Index: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -331,6 +331,26 @@
   return name == "`anonymous namespace'" || name == "`anonymous-namespace'";
 }
 
+static clang::CallingConv TranslateCallingConvention(PDB_CallingConv pdb_cc) {
+  switch (pdb_cc) {
+  case llvm::codeview::CallingConvention::NearC:
+return clang::CC_C;
+  case llvm::codeview::CallingConvention::NearStdCall:
+return clang::CC_X86StdCall;
+  case llvm::codeview::CallingConvention::NearFast:
+return clang::CC_X86FastCall;
+  case llvm::codeview::CallingConvention::ThisCall:
+return clang::CC_X86ThisCall;
+  case llvm::codeview::CallingConvention::NearVector:
+return clang::CC_X86VectorCall;
+  case llvm::codeview::CallingConvention::NearPascal:
+return clang::CC_X86Pascal;
+  default:
+assert(false && "Unknown calling convention");
+return clang::CC_C;
+  }
+}
+
 PDBASTParser::PDBASTParser(lldb_private::ClangASTContext &ast) : m_ast(ast) {}
 
 PDBASTParser::~PDBASTParser() {}
@@ -603,9 +623,10 @@
   type_quals |= clang::Qualifiers::Const;
 if (func_sig->isVolatileType())
   type_quals |= clang::Qualifiers::Volatile;
+auto cc = TranslateCallingConvention(func_sig->getCallingConvention());
 CompilerType func_sig_ast_type =
 m_ast.CreateFunctionType(return_ast_type, arg_list.data(),
- arg_list.size(), is_variadic, type_quals);
+ arg_list.size(), is_variadic, type_quals, cc);
 
 GetDeclarationForSymbol(type, decl);
 return std::make_shared(
Index: lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test
===
--- lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test
+++ lldb/trunk/lit/SymbolFile/PDB/calling-conventions.test
@@ -0,0 +1,11 @@
+REQUIRES: windows, lld
+RUN: clang-cl -m32 /Zi /GS- /c %S/Inputs/CallingConventionsTest.cpp /o %t.obj
+RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe
+RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s
+
+CHECK: Module: {{.*}}
+CHECK-DAG: int (*FuncCCallPtr)();
+CHECK-DAG: int (*FuncStdCallPtr)() __attribute__((stdcall));
+CHECK-DAG: int (*FuncFastCallPtr)() __attribute__((fastcall));
+CHECK-DAG: int (*FuncVectorCallPtr)() __attribute__((vectorcall));
+CHECK-DAG: int (S::*FuncThisCallPtr)() __attribute__((thiscall));
Index: lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
===
--- lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
+++ lldb/trunk/lit/SymbolFile/PDB/Inputs/CallingConventionsTest.cpp
@@ -0,0 +1,20 @@
+int FuncCCall() { return 0; }
+auto FuncCCallPtr = &FuncCCall;
+
+int __stdcall FuncStdCall() { return 0; }
+auto FuncStdCallPtr = &FuncStdCall;
+
+int __fastcall FuncFastCall() { return 0; }
+auto FuncFastCallPtr = &FuncFastCall;
+
+int __vectorcall FuncVectorCall() { return 0; }
+auto FuncVectorCallPtr = &FuncVectorCall;
+
+struct S {
+  int FuncThisCall() { return 0; }
+};
+auto FuncThisCallPtr = &S::FuncThisCall;
+
+int main() {
+  return 0;
+}
Index: lldb/trunk/lit/SymbolFile/PDB/pointers.test
==

[Lldb-commits] [lldb] r343087 - [unittest] Fix NativeProcessProtocolTest.cpp (NFC)

2018-09-26 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Wed Sep 26 03:09:44 2018
New Revision: 343087

URL: http://llvm.org/viewvc/llvm-project?rev=343087&view=rev
Log:
[unittest] Fix NativeProcessProtocolTest.cpp (NFC)

Cast std::min's second argument to size_t to prevent conflicting types
for parameter deduction.

Modified:
lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp

Modified: lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp?rev=343087&r1=343086&r2=343087&view=diff
==
--- lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp (original)
+++ lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp Wed Sep 26 03:09:44 
2018
@@ -130,7 +130,7 @@ llvm::Expected> Fak
   if (Addr >= Data.size())
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Address out of range.");
-  Size = std::min(Size, Data.size() - Addr);
+  Size = std::min(Size, Data.size() - (size_t)Addr);
   return std::vector(&Data[Addr], &Data[Addr + Size]);
 }
 
@@ -139,7 +139,7 @@ llvm::Expected FakeMemory::Write
   if (Addr >= Data.size())
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Address out of range.");
-  size_t Size = std::min(Chunk.size(), Data.size() - Addr);
+  size_t Size = std::min(Chunk.size(), Data.size() - (size_t)Addr);
   std::copy_n(Chunk.begin(), Size, &Data[Addr]);
   return Size;
 }


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

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Adds the module lock to all virtual methods from SymbolFile

2018-09-26 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

This patch adds locking to all the virtual entry-points in DWARFSymbolFile to 
prevent corruption of its data structures as a result of concurrent access.

Please refer to https://reviews.llvm.org/D48393 for more information.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52543

Files:
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -263,6 +263,8 @@
 }
 
 TypeList *SymbolFileDWARF::GetTypeList() {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
   if (debug_map_symfile)
 return debug_map_symfile->GetTypeList();
@@ -272,6 +274,8 @@
 void SymbolFileDWARF::GetTypes(const DWARFDIE &die, dw_offset_t min_die_offset,
dw_offset_t max_die_offset, uint32_t type_mask,
TypeSet &type_set) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   if (die) {
 const dw_offset_t die_offset = die.GetOffset();
 
@@ -435,6 +439,8 @@
 }
 
 TypeSystem *SymbolFileDWARF::GetTypeSystemForLanguage(LanguageType language) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
   TypeSystem *type_system;
   if (debug_map_symfile) {
@@ -848,13 +854,17 @@
 }
 
 uint32_t SymbolFileDWARF::GetNumCompileUnits() {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   DWARFDebugInfo *info = DebugInfo();
   if (info)
 return info->GetNumCompileUnits();
   return 0;
 }
 
 CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   CompUnitSP cu_sp;
   DWARFDebugInfo *info = DebugInfo();
   if (info) {
@@ -867,6 +877,8 @@
 
 Function *SymbolFileDWARF::ParseCompileUnitFunction(const SymbolContext &sc,
 const DWARFDIE &die) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   if (die.IsValid()) {
 TypeSystem *type_system =
 GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
@@ -890,6 +902,8 @@
 }
 lldb::LanguageType
 SymbolFileDWARF::ParseCompileUnitLanguage(const SymbolContext &sc) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu)
@@ -921,6 +935,8 @@
 
 bool SymbolFileDWARF::ParseCompileUnitSupportFiles(
 const SymbolContext &sc, FileSpecList &support_files) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
@@ -946,6 +962,8 @@
 
 bool SymbolFileDWARF::ParseCompileUnitIsOptimized(
 const lldb_private::SymbolContext &sc) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu)
 return dwarf_cu->GetIsOptimized();
@@ -955,6 +973,8 @@
 bool SymbolFileDWARF::ParseImportedModules(
 const lldb_private::SymbolContext &sc,
 std::vector &imported_modules) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
@@ -1032,6 +1052,8 @@
 }
 
 bool SymbolFileDWARF::ParseCompileUnitLineTable(const SymbolContext &sc) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   assert(sc.comp_unit);
   if (sc.comp_unit->GetLineTable() != NULL)
 return true;
@@ -1117,6 +1139,8 @@
 }
 
 bool SymbolFileDWARF::ParseCompileUnitDebugMacros(const SymbolContext &sc) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   assert(sc.comp_unit);
 
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -1145,6 +1169,8 @@
 const DWARFDIE &orig_die,
 addr_t subprogram_low_pc,
 uint32_t depth) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   size_t blocks_added = 0;
   DWARFDIE die = orig_die;
   while (die) {
@@ -1284,6 +1310,8 @@
 }
 
 void SymbolFileDWARF::ParseDeclsForContext(CompilerDeclContext decl_ctx) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   TypeSystem *type_system = decl_ctx.GetTypeSystem();
   DWARFASTParser *ast

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

2018-09-26 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz updated this revision to Diff 167109.
sgraenitz marked an inline comment as done.
sgraenitz added a comment.

Skip AddOSOARanges() if the OSO debug map entry has multiple compile units.


https://reviews.llvm.org/D52375

Files:
  include/lldb/Symbol/Symtab.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Symbol/Symtab.cpp

Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -509,6 +509,16 @@
   return indexes.size() - prev_size;
 }
 
+bool Symtab::HasSymbolWithTypeAndFlags(lldb::SymbolType symbol_type,
+   uint32_t flags_value) const {
+  std::lock_guard guard(m_mutex);
+  for (const Symbol &symbol : m_symbols)
+if (symbol.GetType() == symbol_type && symbol.GetFlags() == flags_value)
+  return true;
+
+  return false;
+}
+
 uint32_t Symtab::AppendSymbolIndexesWithType(SymbolType symbol_type,
  Debug symbol_debug_type,
  Visibility symbol_visibility,
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -131,7 +131,11 @@
   uint32_t GetPluginVersion() override;
 
 protected:
-  enum { kHaveInitializedOSOs = (1 << 0), kNumFlags };
+  enum {
+kHaveInitializedOSOs = (1 << 0),
+kHaveCheckedCUInfos = (1 << 1),
+kNumFlags
+  };
 
   friend class DebugMapModule;
   friend struct DIERef;
@@ -172,13 +176,17 @@
   first_symbol_id(UINT32_MAX), last_symbol_id(UINT32_MAX),
   file_range_map(), file_range_map_valid(false) {}
 
+bool Init(lldb_private::Symbol *so, lldb_private::Symbol *oso);
 const FileRangeMap &GetFileRangeMap(SymbolFileDWARFDebugMap *exe_symfile);
   };
 
   //--
   // Protected Member Functions
   //--
   void InitOSO();
+  bool HasCompileUnits();
+  void ReportErrorInitOSO(lldb_private::Symbol *so_symbol,
+  lldb_private::Symbol *oso_symbol, uint32_t oso_idx);
 
   static uint32_t GetOSOIndexFromUserID(lldb::user_id_t uid) {
 return (uint32_t)((uid >> 32ull) - 1ull);
@@ -247,6 +255,11 @@
 
   lldb::CompUnitSP GetCompileUnit(SymbolFileDWARF *oso_dwarf);
 
+  lldb::CompUnitSP GetCompileUnitByIndex(SymbolFileDWARF *oso_dwarf,
+ uint32_t cu_idx);
+  lldb::CompUnitSP GetCompileUnitByOffset(SymbolFileDWARF *oso_dwarf,
+  dw_offset_t cu_offset);
+
   CompileUnitInfo *GetCompileUnitInfo(SymbolFileDWARF *oso_dwarf);
 
   lldb::TypeSP
@@ -297,6 +310,8 @@
   // Member Variables
   //--
   std::bitset m_flags;
+  bool m_has_compile_unit_infos;
+  std::vector m_oso_compile_unit_offset;
   std::vector m_compile_unit_infos;
   std::vector m_func_indexes; // Sorted by address
   std::vector m_glob_indexes;
@@ -373,7 +388,7 @@
   LinkOSOLineTable(SymbolFileDWARF *oso_symfile,
lldb_private::LineTable *line_table);
 
-  size_t AddOSOARanges(SymbolFileDWARF *dwarf2Data,
+  size_t AddOSOARanges(SymbolFileDWARF *dwarf2Data, dw_offset_t cu_offset,
DWARFDebugAranges *debug_aranges);
 };
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -37,6 +37,7 @@
 #include "lldb/Symbol/VariableList.h"
 #include "llvm/Support/ScopedPrinter.h"
 
+#include "DWARFCompileUnit.h"
 #include "LogChannelDWARF.h"
 #include "SymbolFileDWARF.h"
 
@@ -169,6 +170,23 @@
   return file_range_map;
 }
 
+bool SymbolFileDWARFDebugMap::CompileUnitInfo::Init(Symbol *so, Symbol *oso) {
+  if (!so || so->GetType() != eSymbolTypeSourceFile)
+return false;
+
+  if (!oso || oso->GetType() != eSymbolTypeObjectFile)
+return false;
+
+  if (so->GetSiblingIndex() == UINT32_MAX)
+return false;
+
+  llvm::StringRef so_file_name = so->GetName().GetStringRef();
+  so_file.SetFile(so_file_name, false, FileSpec::Style::native);
+  oso_mod_time = llvm::sys::toTimePoint(oso->GetIntegerValue(0));
+  oso_path = oso->GetName();
+  return true;
+}
+
 class DebugMapModule : public Module {
 public:
   DebugMapModule(const ModuleSP &exe_module_sp, uint32_t cu_idx,
@@ -251,61 +269,63 @@
 }
 
 SymbolFil

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

2018-09-26 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added inline comments.

Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:439
+die->GetAttributeAddressRanges(dwarf, this, ranges, check_hi_lo_pc);
+
 if (num_ranges > 0) {

clayborg wrote:
> Empty CUs won't have any children. Easy to skip by getting the CU die and 
> doing:
> ```
> if (!die->HasChildren())
> ```
> 
Sorry if that was too unspecific. With "no code left" I meant it has no 
instructions. The DIE does have child DIEs for declarations (e.g. 
TAG_namespace, TAG_enumeration_type, TAG_imported_declaration), but nothing 
that would form a range.

I could traverse the DWARF and filter for DIEs with instructions (e.g. 
TAG_subprogram), but it seems pretty much what `die->BuildAddressRangeTable()` 
does already.

Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:488-489
 }
-  } else
-debug_map_sym_file->AddOSOARanges(dwarf, debug_aranges);
+  } //else
+//debug_map_sym_file->AddOSOARanges(dwarf, debug_aranges);
 }

clayborg wrote:
> revert if the "if (!die->hasChildren())" trick works.
> revert if the "if (!die->hasChildren())" trick works.
As noted above, unfortunately, it doesn't. The exploration below got quite 
verbose. 

It looks like a conceptual clash to me: For OSO debug map entries with multiple 
CUs, we can't map between symtab entries and CUs. Thus, each CU's FileRangeMap 
will be populated from the entire symtab range of that OSO debug map entry and 
the call to `AddOSOARanges()` here will add all of that to `debug_aranges` for 
each CU again.

**We should have one FileRangeMap per OSO debug map entry and not one per CU.**

Even if I fixed `AddOSOARanges()` to write the current CU offset instead of 
`dwarf2Data->GetID()` and FileRangeMap existed per OSO debug map entry (or the 
huge number of duplicates was acceptable), it would still add entries from 
other CUs of that OSO debug map entry. This cannot work with the current 
approach in `SymbolFileDWARF::ResolveSymbolContext()` because we will have #CUs 
candidates here, but we only look for a single result:
```
const dw_offset_t cu_offset =
debug_info->GetCompileUnitAranges().FindAddress(file_vm_addr);
```

In order to keep the current `AddOSOARanges()` approach and support multiple 
CUs per OSO debug map entry, `SymbolFileDWARF::ResolveSymbolContext()` must be 
changed to test all candidate CUs. While these changes may be important and the 
code may really benefit from a cleanup, it goes far beyond the scope of this 
bug fix.

**The only reasonable alternative I see is to skip `AddOSOARanges()` for OSO 
debug map entries with multiple CUs.**

Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1531
+  if (m_compile_unit_infos.size() > 1)
+return 0;
+

Skipping AddOSOARanges() here.

https://reviews.llvm.org/D52375

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

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

2018-09-26 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/source/Target/StackFrameList.cpp:331
+dfs(next_callee);
+if (ambiguous)
+  return;

On what path can this happen? Aren't all paths that set `ambiguous=true` 
returning immediately?



Comment at: lldb/source/Target/StackFrameList.cpp:404
+callee->CalculateSymbolContext(&sc);
+StackFrameSP synth_frame{new StackFrame(
+m_thread.shared_from_this(), frame_idx, concrete_frame_idx, cfa,

`auto synth_frame = llvm::make_unique(..)` ?


https://reviews.llvm.org/D50478



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

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

2018-09-26 Thread Vedant Kumar via Phabricator via lldb-commits

vsk updated this revision to Diff 167148.
vsk marked an inline comment as done.
vsk added a comment.

- Use std::make_shared(...), instead of StackFrameSP(new ...).


https://reviews.llvm.org/D50478

Files:
  lldb/include/lldb/API/SBFrame.h
  lldb/include/lldb/Core/FormatEntity.h
  lldb/include/lldb/Symbol/Block.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/Target/StackFrameList.h
  lldb/include/lldb/Target/ThreadPlanStepOut.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
  
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
  lldb/scripts/interface/SBFrame.i
  lldb/source/API/SBFrame.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Symbol/Block.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/ThreadPlanStepOut.cpp

Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -48,26 +48,44 @@
   m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
   m_immediate_step_from_function(nullptr),
   m_calculate_return_value(gather_return_value) {
+  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

2018-09-26 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added inline comments.



Comment at: lldb/source/Target/StackFrameList.cpp:331
+dfs(next_callee);
+if (ambiguous)
+  return;

aprantl wrote:
> On what path can this happen? Aren't all paths that set `ambiguous=true` 
> returning immediately?
If a recursive call to dfs() sets `ambiguous = true` and returns early, we also 
want its caller to return early instead of visiting any additional edges. This 
is a small optimization: the code is correct without the early return.


https://reviews.llvm.org/D50478



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

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

2018-09-26 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:431
 const size_t num_ranges =
-die->GetAttributeAddressRanges(dwarf, this, ranges, false);
+die->GetAttributeAddressRanges(dwarf, this, ranges, check_hi_lo_pc);
 if (num_ranges > 0) {

sgraenitz wrote:
> clayborg wrote:
> > sgraenitz wrote:
> > > @JDevlieghere Thanks for your feedback!
> > > 
> > > @everyone:
> > > This parameter is, actually, one of the key questions in this review: If 
> > > there is no DW_AT_range attribute for the compile unit, can we safely 
> > > fall back to DW_AT_low/high_pc?
> > I have seen DWARF where the DW_AT_ranges is incorrect and I have seen cases 
> > where the low pc is just set to zero and the high pc is set to the max 
> > address. So you might have many overlapping ranges between different CUs in 
> > the latter case. We might need to be more vigilant with high/low pc values 
> > though and that was the reason that we previously ignored high/low pc 
> > values. If the stuff is missing, it should index the DWARF and generate the 
> > address ranges table manually right? What was the reasoning behind this 
> > change?
> I had a closer look at the individual fallbacks and you are right. After 
> fixing `SymbolFileDWARF::ParseCompileUnit()` we have the correct 
> `CompileUnit` pointers in `DWARFUnit::m_user_data` for all CUs. Thus, the 
> below `die->BuildAddressRangeTable(dwarf, this, debug_aranges);` now 
> correctly constructs the ranges! Great, with this we can keep 
> `check_hi_lo_pc=false`.
> 
> I didn't notice this while testing, because of the following issue that kept 
> my tests failing: 2 of my LTO object's CUs have no code remaining after 
> optimization, so we step into the next fallback assuming 'a line tables only 
> situation' and eventually call `AddOSOARanges()`, which adds to 
> `debug_aranges` all entries of the CU's FileRangeMap with a zero offset.
> 
> Thus, my `debug_aranges` had 28000 entries too much and most requests would 
> continue to return `0` as before. For now I commented out the call below.
> 
> **What do you think is a good way to avoid this for empty CUs?
> IIUC they should have no DW_AT_low/high_pc right? :-D**
+1 to @clayborg's suggestion here that we either use AT_ranges, or build up the 
ranges by parsing the low/high pc's in subprograms. ISTM that it'd be 
acceptable to implement the fallback path as a follow-up, provided that doing 
so isn't a regression.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:489
+  } //else
+//debug_map_sym_file->AddOSOARanges(dwarf, debug_aranges);
 }

Could you leave an in-source comment explaining why this is commented out?


https://reviews.llvm.org/D52375



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

[Lldb-commits] [PATCH] D52561: Refactor ClangUserExpression::GetLanguageForExpr

2018-09-26 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: xbolva00.
Herald added a subscriber: lldb-commits.

The `ClangUserExpression::GetLanguageForExpr` method is currently a big
source of sadness, as it's name implies that it's an accessor method, but it 
actually
is also initializing some variables that we need for parsing. This caused that 
we
currently call this getter just for it's side effects while ignoring it's 
return value,
which is confusing for the reader.

This patch renames it to `UpdateLanguageForExpr` and merges all calls to the
method into a single call in `ClangUserExpression::PrepareForParsing` (as 
calling
this method is anyway mandatory for parsing to succeed)

While looking at the code, I also found that we actually have two language
variables in this class hierarchy. The normal `Language` from the UserExpression
class and the `LanguageForExpr` that we implemented in this subclass. Both
don't seem to actually contain the same value, so we probably should look at 
this
next.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52561

Files:
  source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

Index: source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
===
--- source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -177,8 +177,8 @@
 lldb::addr_t struct_address,
 DiagnosticManager &diagnostic_manager) override;
 
-  llvm::Optional GetLanguageForExpr(
-  DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx);
+  void UpdateLanguageForExpr(DiagnosticManager &diagnostic_manager,
+ ExecutionContext &exe_ctx);
   bool SetupPersistentState(DiagnosticManager &diagnostic_manager,
ExecutionContext &exe_ctx);
   bool PrepareForParsing(DiagnosticManager &diagnostic_manager,
@@ -201,6 +201,9 @@
 lldb::TargetSP m_target_sp;
   };
 
+  /// The language type of the current expression.
+  lldb::LanguageType m_expr_lang = lldb::eLanguageTypeUnknown;
+
   /// The absolute character position in the transformed source code where the
   /// user code (as typed by the user) starts. If the variable is empty, then we
   /// were not able to calculate this position.
Index: source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -376,9 +376,9 @@
   }
 }
 
-llvm::Optional ClangUserExpression::GetLanguageForExpr(
+void ClangUserExpression::UpdateLanguageForExpr(
 DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx) {
-  lldb::LanguageType lang_type = lldb::LanguageType::eLanguageTypeUnknown;
+  m_expr_lang = lldb::LanguageType::eLanguageTypeUnknown;
 
   std::string prefix = m_expr_prefix;
 
@@ -390,30 +390,29 @@
 m_expr_text.c_str()));
 
 if (m_in_cplusplus_method)
-  lang_type = lldb::eLanguageTypeC_plus_plus;
+  m_expr_lang = lldb::eLanguageTypeC_plus_plus;
 else if (m_in_objectivec_method)
-  lang_type = lldb::eLanguageTypeObjC;
+  m_expr_lang = lldb::eLanguageTypeObjC;
 else
-  lang_type = lldb::eLanguageTypeC;
+  m_expr_lang = lldb::eLanguageTypeC;
 
-if (!source_code->GetText(m_transformed_text, lang_type, m_in_static_method,
-  exe_ctx)) {
+if (!source_code->GetText(m_transformed_text, m_expr_lang,
+  m_in_static_method, exe_ctx)) {
   diagnostic_manager.PutString(eDiagnosticSeverityError,
"couldn't construct expression body");
-  return llvm::Optional();
+  return;
 }
 
 // Find and store the start position of the original code inside the
 // transformed code. We need this later for the code completion.
 std::size_t original_start;
 std::size_t original_end;
 bool found_bounds = source_code->GetOriginalBodyBounds(
-m_transformed_text, lang_type, original_start, original_end);
+m_transformed_text, m_expr_lang, original_start, original_end);
 if (found_bounds) {
   m_user_expression_start_pos = original_start;
 }
   }
-  return lang_type;
 }
 
 bool ClangUserExpression::PrepareForParsing(
@@ -437,6 +436,8 @@
   ApplyObjcCastHack(m_expr_text);
 
   SetupDeclVendor(exe_ctx, m_target);
+
+  UpdateLanguageForExpr(diagnostic_manager, exe_ctx);
   return true;
 }
 
@@ -450,11 +451,6 @@
   if (!PrepareForParsing(diagnostic_manager, exe_ctx))
 return false;
 
-  lldb::LanguageType lang_type = lldb::LanguageType::eLanguageTypeUnknown;
-  if (auto new_lang = GetLanguageForExpr(diagnostic_manager, exe_ctx)) {
-

Re: [Lldb-commits] [lldb] r343087 - [unittest] Fix NativeProcessProtocolTest.cpp (NFC)

2018-09-26 Thread Pavel Labath via lldb-commits

Thanks.

On 26/09/18 12:09, Jonas Devlieghere via lldb-commits wrote:
> Author: jdevlieghere
> Date: Wed Sep 26 03:09:44 2018
> New Revision: 343087
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=343087&view=rev
> Log:
> [unittest] Fix NativeProcessProtocolTest.cpp (NFC)
> 
> Cast std::min's second argument to size_t to prevent conflicting types
> for parameter deduction.
> 
> Modified:
> lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp
> 
> Modified: lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp?rev=343087&r1=343086&r2=343087&view=diff
> ==
> --- lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp (original)
> +++ lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp Wed Sep 26 
> 03:09:44 2018
> @@ -130,7 +130,7 @@ llvm::Expected> Fak
>if (Addr >= Data.size())
>  return llvm::createStringError(llvm::inconvertibleErrorCode(),
> "Address out of range.");
> -  Size = std::min(Size, Data.size() - Addr);
> +  Size = std::min(Size, Data.size() - (size_t)Addr);
>return std::vector(&Data[Addr], &Data[Addr + Size]);
>  }
>  
> @@ -139,7 +139,7 @@ llvm::Expected FakeMemory::Write
>if (Addr >= Data.size())
>  return llvm::createStringError(llvm::inconvertibleErrorCode(),
> "Address out of range.");
> -  size_t Size = std::min(Chunk.size(), Data.size() - Addr);
> +  size_t Size = std::min(Chunk.size(), Data.size() - (size_t)Addr);
>std::copy_n(Chunk.begin(), Size, &Data[Addr]);
>return Size;
>  }
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

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

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Adds the module lock to all virtual methods from SymbolFile

2018-09-26 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

It is the SymbolVendor's job to do the locking. And in many cases it already 
is. I stopped with inlined comments after a few comments as it would apply to 
this entire patch.

It would be great if we can replace all of the locations where you added lock 
guards with lldbasserts to assert that the lock is already taken. Then we might 
be able to catch places where the locks are not being respected. But in general 
the symbol vendor should take the lock, use N number of SymbolFile instances to 
complete the request, and then release the lock. Makes things easier on the 
lldb_private::SymbolFile subclasses this way.




Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:266-267
 TypeList *SymbolFileDWARF::GetTypeList() {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();

This shouldn't require locking as it is just handing out a type list ivar from 
one place or another.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:277-278
TypeSet &type_set) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   if (die) {

This is not a SymbolFile override. It shouldn't require the locking.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:442-443
 TypeSystem *SymbolFileDWARF::GetTypeSystemForLanguage(LanguageType language) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();

This shouldn't require locking as it is just handing out a type system from one 
place or another.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:857-858
 uint32_t SymbolFileDWARF::GetNumCompileUnits() {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   DWARFDebugInfo *info = DebugInfo();

SymbolVendor::GetNumCompileUnits() already takes the module lock for us. All 
other internal calls to this should be protected as well



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:866-867
 CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   CompUnitSP cu_sp;

SymbolVendor already takes the module lock for us



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:880-881
 const DWARFDIE &die) {
+  std::lock_guard guard(
+  GetObjectFile()->GetModule()->GetMutex());
   if (die.IsValid()) {

SymbolVendor requests that lead to this call should take the lock for us...


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52543



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

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

2018-09-26 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added inline comments.

Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1531
+  if (m_compile_unit_infos.size() > 1)
+return 0;
+

sgraenitz wrote:
> Skipping AddOSOARanges() here.
> Could you leave an in-source comment explaining why this is commented out?

@vsk AddOSOARanges() was commented out only temporarily. I reverted it in the 
latest patch and added these lines with a comment. Is that what you were asking 
for?

https://reviews.llvm.org/D52375

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

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Adds the module lock to all virtual methods from SymbolFile

2018-09-26 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In https://reviews.llvm.org/D52543#1246877, @clayborg wrote:

> It is the SymbolVendor's job to do the locking. And in many cases it already 
> is. I stopped with inlined comments after a few comments as it would apply to 
> this entire patch.


Thanks Greg. Honestly I don't know the code well enough to be sure where we 
*need* a lock and where we can get away without. Since it's a recursive mutex I 
figured there's no harm in playing it safe. Not everything goes through the 
symbol vendor though, for example the backtrace below is the result of a 
corruption and it looks like we're calling directly into SymbolFileDWARF.

  >  1 com.apple.LLDB.framework   0x02f8dcf5 bool 
llvm::DenseMapBase, 
llvm::detail::DenseMapPair >, void*, DIERef, 
llvm::DenseMapInfo, llvm::detail::DenseMapPair 
>::LookupBucketFor(void* const&, llvm::detail::DenseMapPair const*&) const + 45
 2 com.apple.LLDB.framework   0x02f8e91e 
llvm::DenseMapBase, 
llvm::detail::DenseMapPair >, 
DWARFDebugInfoEntry const*, clang::Decl*, 
llvm::DenseMapInfo, 
llvm::detail::DenseMapPair 
>::FindAndConstruct(DWARFDebugInfoEntry const*&&) + 28
 3 com.apple.LLDB.framework   0x02f81f94 
DWARFASTParserClang::ParseTypeFromDWARF(lldb_private::SymbolContext const&, 
DWARFDIE const&, lldb_private::Log*, bool*) + 6144
 4 com.apple.LLDB.framework   0x03150f75 
SymbolFileDWARF::ParseType(lldb_private::SymbolContext const&, DWARFDIE const&, 
bool*) + 209
 5 com.apple.LLDB.framework   0x0314a70e 
SymbolFileDWARF::GetTypeForDIE(DWARFDIE const&, bool) + 486
 6 com.apple.LLDB.framework   0x03149ee2 
SymbolFileDWARF::ResolveType(DWARFDIE const&, bool, bool) + 68
 7 com.apple.LLDB.framework   0x02f88728 
DWARFASTParserClang::GetClangDeclContextForDIE(DWARFDIE const&) + 176
 8 com.apple.LLDB.framework   0x02f8c0ae 
DWARFASTParserClang::GetDeclContextForUIDFromDWARF(DWARFDIE const&) + 24
 9 com.apple.LLDB.framework   0x031636fe DWARFDIE::GetDeclContext() 
const + 60
10 com.apple.LLDB.framework   0x03149dd5 
SymbolFileDWARF::GetDeclContextForUID(unsigned long long) + 53
11 com.apple.LLDB.framework   0x0315c6ed 
SymbolFileDWARFDebugMap::GetDeclContextForUID(u



> It would be great if we can replace all of the locations where you added lock 
> guards with lldbasserts to assert that the lock is already taken. Then we 
> might be able to catch places where the locks are not being respected. But in 
> general the symbol vendor should take the lock, use N number of SymbolFile 
> instances to complete the request, and then release the lock. Makes things 
> easier on the lldb_private::SymbolFile subclasses this way.

That sounds like a really good idea, I'll definitely do that.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52543



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

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

2018-09-26 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1531
+  if (m_compile_unit_infos.size() > 1)
+return 0;
+

sgraenitz wrote:
> sgraenitz wrote:
> > Skipping AddOSOARanges() here.
> > Could you leave an in-source comment explaining why this is commented out?
> 
> @vsk AddOSOARanges() was commented out only temporarily. I reverted it in the 
> latest patch and added these lines with a comment. Is that what you were 
> asking for?
Ah sure sure, sorry, I missed the broader context of the change.


https://reviews.llvm.org/D52375



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

[Lldb-commits] [lldb] r343134 - Fix OSX build after r343130

2018-09-26 Thread Tatyana Krasnukha via lldb-commits

Author: tkrasnukha
Date: Wed Sep 26 12:41:57 2018
New Revision: 343134

URL: http://llvm.org/viewvc/llvm-project?rev=343134&view=rev
Log:
Fix OSX build after r343130

Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp?rev=343134&r1=343133&r2=343134&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp Wed Sep 
26 12:41:57 2018
@@ -188,12 +188,12 @@ const char *PlatformDarwinKernel::GetDes
 
 static PropertyDefinition g_properties[] = {
 {"search-locally-for-kexts", OptionValue::eTypeBoolean, true, true, NULL,
- NULL, "Automatically search for kexts on the local system when doing "
+ {}, "Automatically search for kexts on the local system when doing "
"kernel debugging."},
-{"kext-directories", OptionValue::eTypeFileSpecList, false, 0, NULL, NULL,
+{"kext-directories", OptionValue::eTypeFileSpecList, false, 0, NULL, {},
  "Directories/KDKs to search for kexts in when starting a kernel debug "
  "session."},
-{NULL, OptionValue::eTypeInvalid, false, 0, NULL, NULL, NULL}};
+{NULL, OptionValue::eTypeInvalid, false, 0, NULL, {}, NULL}};
 
 enum { ePropertySearchForKexts = 0, ePropertyKextDirectories };
 


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

[Lldb-commits] [lldb] r343141 - Fix ProcessKDP after r343130

2018-09-26 Thread Tatyana Krasnukha via lldb-commits

Author: tkrasnukha
Date: Wed Sep 26 13:31:39 2018
New Revision: 343141

URL: http://llvm.org/viewvc/llvm-project?rev=343141&view=rev
Log:
Fix ProcessKDP after r343130

Modified:
lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp

Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp?rev=343141&r1=343140&r2=343141&view=diff
==
--- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (original)
+++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Wed Sep 26 
13:31:39 2018
@@ -56,9 +56,9 @@ using namespace lldb_private;
 namespace {
 
 static PropertyDefinition g_properties[] = {
-{"packet-timeout", OptionValue::eTypeUInt64, true, 5, NULL, NULL,
+{"packet-timeout", OptionValue::eTypeUInt64, true, 5, NULL, {},
  "Specify the default packet timeout in seconds."},
-{NULL, OptionValue::eTypeInvalid, false, 0, NULL, NULL, NULL}};
+{NULL, OptionValue::eTypeInvalid, false, 0, NULL, {}, NULL}};
 
 enum { ePropertyPacketTimeout };
 


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

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Tatyana Krasnukha via Phabricator via lldb-commits

tatyana-krasnukha created this revision.
tatyana-krasnukha added reviewers: clayborg, zturner, jingham.
tatyana-krasnukha added a project: LLDB.
Herald added subscribers: lldb-commits, teemperor, JDevlieghere.

As a follow-up to https://reviews.llvm.org/D49017.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52572

Files:
  include/lldb/Interpreter/OptionValueProperties.h
  include/lldb/Interpreter/Property.h
  source/Core/Debugger.cpp
  source/Core/ModuleList.cpp
  source/Interpreter/CommandInterpreter.cpp
  source/Interpreter/OptionValueProperties.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Target/Platform.cpp
  source/Target/Process.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp

Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -80,8 +80,7 @@
 {"trace-thread", OptionValue::eTypeBoolean, false, false, nullptr, {},
  "If true, this thread will single-step and log execution."},
 {"max-backtrace-depth", OptionValue::eTypeUInt64, false, 30, nullptr,
- {}, "Maximum number of frames to backtrace."},
-{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, {}, nullptr}};
+ {}, "Maximum number of frames to backtrace."}};
 
 enum {
   ePropertyStepInAvoidsNoDebug,
Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -3352,8 +3352,7 @@
  nullptr, {}, "If true, LLDB will show variables that are meant to "
   "support the operation of a language's runtime support."},
 {"non-stop-mode", OptionValue::eTypeBoolean, false, 0, nullptr, {},
- "Disable lock-step debugging, instead control threads independently."},
-{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, {}, nullptr}};
+ "Disable lock-step debugging, instead control threads independently."}};
 
 enum {
   ePropertyDefaultArch,
@@ -3483,8 +3482,7 @@
  "ivars and local variables.  "
  "But it can make expressions run much more slowly."},
 {"use-modern-type-lookup", OptionValue::eTypeBoolean, true, false, nullptr,
- {}, "If true, use Clang's modern type lookup infrastructure."},
-{nullptr, OptionValue::eTypeInvalid, true, 0, nullptr, {}, nullptr}};
+ {}, "If true, use Clang's modern type lookup infrastructure."}};
 
 enum { ePropertyInjectLocalVars = 0, ePropertyUseModernTypeLookup };
 
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -147,8 +147,7 @@
  "stepping and variable availability may not behave as expected."},
 {"stop-on-exec", OptionValue::eTypeBoolean, true, true,
  nullptr, {},
- "If true, stop when a shared library is loaded or unloaded."},
-{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, {}, nullptr}};
+ "If true, stop when a shared library is loaded or unloaded."}};
 
 enum {
   ePropertyDisableMemCache,
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -71,8 +71,7 @@
 {"use-module-cache", OptionValue::eTypeBoolean, true, true, nullptr,
  {}, "Use module cache."},
 {"module-cache-directory", OptionValue::eTypeFileSpec, true, 0, nullptr,
- {}, "Root directory for cached modules."},
-{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, {}, nullptr}};
+ {}, "Root directory for cached modules."}};
 
 enum { ePropertyUseModuleCache, ePropertyModuleCacheDirectory };
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -118,9 +118,7 @@
  "links will be resolved at DWARF parse time."},
 {"ignore-file-indexes", OptionValue::eTypeBoolean, true, 0, nullptr, {},
  "Ignore indexes present in the object files and always index DWARF "
- "manually."},
-{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, {}, nullptr},
-};
+ "manually."}};
 
 enum {
   ePropertySymLinkPaths,
Index: source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ source/Plugins/S

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Greg Clayton via Phabricator via lldb-commits

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

Good stuff!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52572



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

Re: [Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Zachary Turner via lldb-commits

It would be nice if You could replace the logic that iterates these arrays.
We no longer need to terminate on a sentinel nullptr entry and can now use
range based for loop
On Wed, Sep 26, 2018 at 2:06 PM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg accepted this revision.
> clayborg added a comment.
> This revision is now accepted and ready to land.
>
> Good stuff!
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D52572
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a subscriber: tatyana-krasnukha.
zturner added a comment.

It would be nice if You could replace the logic that iterates these arrays.
We no longer need to terminate on a sentinel nullptr entry and can now use
range based for loop


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52572



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

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

2018-09-26 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added inline comments.

Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:468
 // situation. Check the line tables and build the arange table from this.
 SymbolContext sc;
 sc.comp_unit = dwarf->GetCompUnitForDWARFCompUnit(this);

@clayborg Any idea how to reproduce this code path? I expected test_lt as below 
would do, but debug_aranges is read from DIE even there. Is it (only) for 
backwards compatibility?
```
clang -c -gline-tables-only -o main_lt.o main.c
clang -c -gline-tables-only -o cu1_lt.o cu1.c
clang -gline-tables-only main_lt.o cu1_lt.o -o test_lt
```

Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1531
+  if (m_compile_unit_infos.size() > 1)
+return 0;
+

vsk wrote:
> sgraenitz wrote:
> > sgraenitz wrote:
> > > Skipping AddOSOARanges() here.
> > > Could you leave an in-source comment explaining why this is commented out?
> > 
> > @vsk AddOSOARanges() was commented out only temporarily. I reverted it in 
> > the latest patch and added these lines with a comment. Is that what you 
> > were asking for?
> Ah sure sure, sorry, I missed the broader context of the change.
No worries, thanks for having a look.

https://reviews.llvm.org/D52375

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

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Tatyana Krasnukha via Phabricator via lldb-commits

tatyana-krasnukha added a comment.

Hmm... There is no such logic left, isn't it?
If you mean iterating OptionValueProperties::m_properties vector, I can replace 
cycles with range based for loop, but this doesn't seem to be related to these 
changes.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52572



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

Re: [Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Zachary Turner via lldb-commits

You might be right, I’m on mobile so hard for me to review. I saw a bunch
of nullptr so assumed it was still using sentinel values for iterating. If
not, ignore my suggestion
On Wed, Sep 26, 2018 at 4:26 PM Tatyana Krasnukha via Phabricator <
revi...@reviews.llvm.org> wrote:

> tatyana-krasnukha added a comment.
>
> Hmm... There is no such logic left, isn't it?
> If you mean iterating OptionValueProperties::m_properties vector, I can
> replace cycles with range based for loop, but this doesn't seem to be
> related to these changes.
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D52572
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

You might be right, I’m on mobile so hard for me to review. I saw a bunch
of nullptr so assumed it was still using sentinel values for iterating. If
not, ignore my suggestion


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52572



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

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Tatyana Krasnukha via Phabricator via lldb-commits

tatyana-krasnukha added a comment.

It seems you are speaking about OptionDefinition, not PropertyDefinition. I was 
going to create a separate revision for it. Should I add OptionDefinition 
related changes to here?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52572



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

Re: [Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Zachary Turner via lldb-commits

No, separate revision is fine. Thanks!
On Wed, Sep 26, 2018 at 4:40 PM Tatyana Krasnukha via Phabricator <
revi...@reviews.llvm.org> wrote:

> tatyana-krasnukha added a comment.
>
> It seems you are speaking about OptionDefinition, not PropertyDefinition.
> I was going to create a separate revision for it. Should I add
> OptionDefinition related changes to here?
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D52572
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

2018-09-26 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

No, separate revision is fine. Thanks!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52572



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

[Lldb-commits] [lldb] r343164 - llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)

2018-09-26 Thread Fangrui Song via lldb-commits

Author: maskray
Date: Wed Sep 26 20:35:05 2018
New Revision: 343164

URL: http://llvm.org/viewvc/llvm-project?rev=343164&view=rev
Log:
llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)

The convenience wrapper in STLExtras is available since rL342102.

Modified:
lldb/trunk/source/Breakpoint/Breakpoint.cpp

Modified: lldb/trunk/source/Breakpoint/Breakpoint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Breakpoint.cpp?rev=343164&r1=343163&r2=343164&view=diff
==
--- lldb/trunk/source/Breakpoint/Breakpoint.cpp (original)
+++ lldb/trunk/source/Breakpoint/Breakpoint.cpp Wed Sep 26 20:35:05 2018
@@ -785,8 +785,8 @@ void Breakpoint::ModuleReplaced(ModuleSP
   // we go.
 
   if (old_id_vec.size() == new_id_vec.size()) {
-llvm::sort(old_id_vec.begin(), old_id_vec.end());
-llvm::sort(new_id_vec.begin(), new_id_vec.end());
+llvm::sort(old_id_vec);
+llvm::sort(new_id_vec);
 size_t num_elements = old_id_vec.size();
 for (size_t idx = 0; idx < num_elements; idx++) {
   BreakpointLocationSP old_loc_sp =


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

[Lldb-commits] [lldb] r343076 - Fix a memory read bug in lldb-server

[Lldb-commits] [PATCH] D52532: Pull GetSoftwareBreakpointPCOffset into base class

[Lldb-commits] [PATCH] D52532: Pull GetSoftwareBreakpointPCOffset into base class

[Lldb-commits] [PATCH] D52501: [PDB] Restore the calling convention from PDB

[Lldb-commits] [lldb] r343084 - [PDB] Restore the calling convention from PDB

[Lldb-commits] [PATCH] D52501: [PDB] Restore the calling convention from PDB

[Lldb-commits] [lldb] r343087 - [unittest] Fix NativeProcessProtocolTest.cpp (NFC)

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Adds the module lock to all virtual methods from SymbolFile

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Lldb-commits] [PATCH] D52561: Refactor ClangUserExpression::GetLanguageForExpr

Re: [Lldb-commits] [lldb] r343087 - [unittest] Fix NativeProcessProtocolTest.cpp (NFC)

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Adds the module lock to all virtual methods from SymbolFile

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Adds the module lock to all virtual methods from SymbolFile

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Lldb-commits] [lldb] r343134 - Fix OSX build after r343130

[Lldb-commits] [lldb] r343141 - Fix ProcessKDP after r343130

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

Re: [Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

Re: [Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

Re: [Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

[Lldb-commits] [lldb] r343164 - llvm::sort(C.begin(), C.end(), ...) -> llvm::sort(C, ...)

34 matches

Site Navigation

Mail list logo

Footer information