[Lldb-commits] [lldb] 3a28042 - [lldb][NFC] Early exit in DWARFASTParserClang::ParseArrayType

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-11-27T09:28:01+01:00
New Revision: 3a280422b66a31af694782746ec0b5b7552a82a1

URL: 
https://github.com/llvm/llvm-project/commit/3a280422b66a31af694782746ec0b5b7552a82a1
DIFF: 
https://github.com/llvm/llvm-project/commit/3a280422b66a31af694782746ec0b5b7552a82a1.diff

LOG: [lldb][NFC] Early exit in DWARFASTParserClang::ParseArrayType

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index fe6ab3064447..df5c81f2e830 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1257,83 +1257,83 @@ TypeSP DWARFASTParserClang::ParseArrayType(const 
DWARFDIE &die,
   DWARFDIE type_die = attrs.type.Reference();
   Type *element_type = dwarf->ResolveTypeUID(type_die, true);
 
-  if (element_type) {
-auto array_info = ParseChildArrayInfo(die);
-if (array_info) {
-  attrs.byte_stride = array_info->byte_stride;
-  attrs.bit_stride = array_info->bit_stride;
-}
-if (attrs.byte_stride == 0 && attrs.bit_stride == 0)
-  attrs.byte_stride = element_type->GetByteSize().getValueOr(0);
-CompilerType array_element_type = element_type->GetForwardCompilerType();
-
-if (ClangASTContext::IsCXXClassType(array_element_type) &&
-!array_element_type.GetCompleteType()) {
-  ModuleSP module_sp = die.GetModule();
-  if (module_sp) {
-if (die.GetCU()->GetProducer() == eProducerClang)
-  module_sp->ReportError(
-  "DWARF DW_TAG_array_type DIE at 0x%8.8x has a "
-  "class/union/struct element type DIE 0x%8.8x that is a "
-  "forward declaration, not a complete definition.\nTry "
-  "compiling the source file with -fstandalone-debug or "
-  "disable -gmodules",
-  die.GetOffset(), type_die.GetOffset());
-else
-  module_sp->ReportError(
-  "DWARF DW_TAG_array_type DIE at 0x%8.8x has a "
-  "class/union/struct element type DIE 0x%8.8x that is a "
-  "forward declaration, not a complete definition.\nPlease "
-  "file a bug against the compiler and include the "
-  "preprocessed output for %s",
-  die.GetOffset(), type_die.GetOffset(), GetUnitName(die).c_str());
-  }
-
-  // We have no choice other than to pretend that the element class
-  // type is complete. If we don't do this, clang will crash when
-  // trying to layout the class. Since we provide layout
-  // assistance, all ivars in this class and other classes will be
-  // fine, this is the best we can do short of crashing.
-  if (ClangASTContext::StartTagDeclarationDefinition(array_element_type)) {
-ClangASTContext::CompleteTagDeclarationDefinition(array_element_type);
-  } else {
-module_sp->ReportError("DWARF DIE at 0x%8.8x was not able to "
-   "start its definition.\nPlease file a "
-   "bug and attach the file at the start "
-   "of this error message",
-   type_die.GetOffset());
-  }
-}
+  if (!element_type)
+return nullptr;
 
-uint64_t array_element_bit_stride =
-attrs.byte_stride * 8 + attrs.bit_stride;
-CompilerType clang_type;
-if (array_info && array_info->element_orders.size() > 0) {
-  uint64_t num_elements = 0;
-  auto end = array_info->element_orders.rend();
-  for (auto pos = array_info->element_orders.rbegin(); pos != end; ++pos) {
-num_elements = *pos;
-clang_type = m_ast.CreateArrayType(array_element_type, num_elements,
-   attrs.is_vector);
-array_element_type = clang_type;
-array_element_bit_stride = num_elements
-   ? array_element_bit_stride * 
num_elements
-   : array_element_bit_stride;
-  }
+  llvm::Optional array_info = ParseChildArrayInfo(die);
+  if (array_info) {
+attrs.byte_stride = array_info->byte_stride;
+attrs.bit_stride = array_info->bit_stride;
+  }
+  if (attrs.byte_stride == 0 && attrs.bit_stride == 0)
+attrs.byte_stride = element_type->GetByteSize().getValueOr(0);
+  CompilerType array_element_type = element_type->GetForwardCompilerType();
+
+  if (ClangASTContext::IsCXXClassType(array_element_type) &&
+  !array_element_type.GetCompleteType()) {
+ModuleSP module_sp = die.GetModule();
+if (module_sp) {
+  if (die.GetCU()->GetProducer() == eProducerClang)
+module_sp->ReportError(
+"DWARF DW_TAG_array_type DIE at 0x%8.8x has a "
+"cl

[Lldb-commits] [PATCH] D70770: [lldb][NFC] Remove unused CompilerType memory functions

[Raphael Isemann via Phabricator via lldb-commits]

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

All these functions are unused from what I can see. Unless I'm missing 
something here, this code
can go the way of the Dodo.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70770

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -874,173 +874,6 @@
   return false;
 }
 
-bool CompilerType::SetValueFromScalar(const Scalar &value, Stream &strm) {
-  if (!IsValid())
-return false;
-
-  // Aggregate types don't have scalar values
-  if (!IsAggregateType()) {
-strm.GetFlags().Set(Stream::eBinary);
-uint64_t count = 0;
-lldb::Encoding encoding = GetEncoding(count);
-
-if (encoding == lldb::eEncodingInvalid || count != 1)
-  return false;
-
-llvm::Optional bit_width = GetBitSize(nullptr);
-if (!bit_width)
-  return false;
-
-// This function doesn't currently handle non-byte aligned assignments
-if ((*bit_width % 8) != 0)
-  return false;
-
-const uint64_t byte_size = (*bit_width + 7) / 8;
-switch (encoding) {
-case lldb::eEncodingInvalid:
-  break;
-case lldb::eEncodingVector:
-  break;
-case lldb::eEncodingUint:
-  switch (byte_size) {
-  case 1:
-strm.PutHex8(value.UInt());
-return true;
-  case 2:
-strm.PutHex16(value.UInt());
-return true;
-  case 4:
-strm.PutHex32(value.UInt());
-return true;
-  case 8:
-strm.PutHex64(value.ULongLong());
-return true;
-  default:
-break;
-  }
-  break;
-
-case lldb::eEncodingSint:
-  switch (byte_size) {
-  case 1:
-strm.PutHex8(value.SInt());
-return true;
-  case 2:
-strm.PutHex16(value.SInt());
-return true;
-  case 4:
-strm.PutHex32(value.SInt());
-return true;
-  case 8:
-strm.PutHex64(value.SLongLong());
-return true;
-  default:
-break;
-  }
-  break;
-
-case lldb::eEncodingIEEE754:
-  if (byte_size <= sizeof(long double)) {
-if (byte_size == sizeof(float)) {
-  strm.PutFloat(value.Float());
-  return true;
-} else if (byte_size == sizeof(double)) {
-  strm.PutDouble(value.Double());
-  return true;
-} else if (byte_size == sizeof(long double)) {
-  strm.PutDouble(value.LongDouble());
-  return true;
-}
-  }
-  break;
-}
-  }
-  return false;
-}
-
-bool CompilerType::ReadFromMemory(lldb_private::ExecutionContext *exe_ctx,
-  lldb::addr_t addr, AddressType address_type,
-  lldb_private::DataExtractor &data) {
-  if (!IsValid())
-return false;
-
-  // Can't convert a file address to anything valid without more context (which
-  // Module it came from)
-  if (address_type == eAddressTypeFile)
-return false;
-
-  if (!GetCompleteType())
-return false;
-
-  auto byte_size =
-  GetByteSize(exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
-  if (!byte_size)
-return false;
-
-  if (data.GetByteSize() < *byte_size) {
-lldb::DataBufferSP data_sp(new DataBufferHeap(*byte_size, '\0'));
-data.SetData(data_sp);
-  }
-
-  uint8_t *dst = const_cast(data.PeekData(0, *byte_size));
-  if (dst != nullptr) {
-if (address_type == eAddressTypeHost) {
-  if (addr == 0)
-return false;
-  // The address is an address in this process, so just copy it
-  memcpy(dst, reinterpret_cast(addr), *byte_size);
-  return true;
-} else {
-  Process *process = nullptr;
-  if (exe_ctx)
-process = exe_ctx->GetProcessPtr();
-  if (process) {
-Status error;
-return process->ReadMemory(addr, dst, *byte_size, error) == *byte_size;
-  }
-}
-  }
-  return false;
-}
-
-bool CompilerType::WriteToMemory(lldb_private::ExecutionContext *exe_ctx,
- lldb::addr_t addr, AddressType address_type,
- StreamString &new_value) {
-  if (!IsValid())
-return false;
-
-  // Can't convert a file address to anything valid without more context (which
-  // Module it came from)
-  if (address_type == eAddressTypeFile)
-return false;
-
-  if (!GetCompleteType())
-return false;
-
-  auto byte_size =
-  GetByteSize(exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
-  if (!byte_size)
-return false;
-
-  if (*byte_size > 0) {
-if (address_type == eAddressTypeHost) {
-  // The address is an address in this process, so just copy it
-  memcpy((void *)addr, new_value.GetData(), *byte_siz

[Lldb-commits] [PATCH] D70742: [LLDB] [Windows] Avoid using InitializeContext for allocating a CONTEXT. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG344bdeb797b3: [LLDB] Avoid using InitializeContext for 
zero-initializing a CONTEXT. NFC. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70742

Files:
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
@@ -154,15 +154,8 @@
 return true;
 
   TargetThreadWindows &wthread = static_cast(m_thread);
-  uint8_t buffer[2048];
-  memset(buffer, 0, sizeof(buffer));
-  PCONTEXT tmpContext = NULL;
-  DWORD contextLength = (DWORD)sizeof(buffer);
-  if (!::InitializeContext(buffer, kWinContextFlags, &tmpContext,
-   &contextLength)) {
-return false;
-  }
-  memcpy(&m_context, tmpContext, sizeof(m_context));
+  memset(&m_context, 0, sizeof(m_context));
+  m_context.ContextFlags = kWinContextFlags;
   if (::SuspendThread(
   wthread.GetHostThread().GetNativeThread().GetSystemHandle()) ==
   (DWORD)-1) {


Index: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
@@ -154,15 +154,8 @@
 return true;
 
   TargetThreadWindows &wthread = static_cast(m_thread);
-  uint8_t buffer[2048];
-  memset(buffer, 0, sizeof(buffer));
-  PCONTEXT tmpContext = NULL;
-  DWORD contextLength = (DWORD)sizeof(buffer);
-  if (!::InitializeContext(buffer, kWinContextFlags, &tmpContext,
-   &contextLength)) {
-return false;
-  }
-  memcpy(&m_context, tmpContext, sizeof(m_context));
+  memset(&m_context, 0, sizeof(m_context));
+  m_context.ContextFlags = kWinContextFlags;
   if (::SuspendThread(
   wthread.GetHostThread().GetNativeThread().GetSystemHandle()) ==
   (DWORD)-1) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70770: [lldb][NFC] Remove unused CompilerType memory functions

[Pavel Labath via Phabricator via lldb-commits]

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

If they are indeed used, we'll find out soon enough...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D70770



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

[Lldb-commits] [PATCH] D70772: I implemented the features listed in this document: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0616r0.pdf and built libc++ using ninja without any errors/w

[Fady Farag via Phabricator via lldb-commits]

Afadyfarag created this revision.
Afadyfarag added reviewers: clayborg, aprantl, JDevlieghere, EricWF.
Afadyfarag added a project: libc++.
Herald added a reviewer: mclow.lists.

1. Changes:

  libcxx/include/numeric


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70772

Files:
  libcxx/CMakeLists.txt
  libcxx/include/algorithm
  libcxx/include/forward_list
  libcxx/include/numeric

Index: libcxx/include/numeric
===
--- libcxx/include/numeric
+++ libcxx/include/numeric
@@ -143,7 +143,9 @@
 
 #include <__config>
 #include 
+#if _LIBCPP_STD_VER > 17 
 #include  // for numeric_limits
+#endif
 #include 
 #include  // for isnormal
 #include 
@@ -172,8 +174,13 @@
 _Tp
 accumulate(_InputIterator __first, _InputIterator __last, _Tp __init, _BinaryOperation __binary_op)
 {
-for (; __first != __last; ++__first)
+for (; __first != __last; ++__first) {
+#if _LIBCPP_STD_VER > 17
+__init = __binary_op(_VSTD::move(__init), *__first);
+#else
 __init = __binary_op(__init, *__first);
+#endif
+}
 return __init;
 }
 
@@ -222,8 +229,13 @@
 inner_product(_InputIterator1 __first1, _InputIterator1 __last1, _InputIterator2 __first2,
   _Tp __init, _BinaryOperation1 __binary_op1, _BinaryOperation2 __binary_op2)
 {
-for (; __first1 != __last1; ++__first1, (void) ++__first2)
+for (; __first1 != __last1; ++__first1, (void) ++__first2) {
+#if _LIBCPP_STD_VER > 17
+__init = __binary_op1(_VSTD::move(__init), __binary_op2(*__first1, *__first2));
+#else
 __init = __binary_op1(__init, __binary_op2(*__first1, *__first2));
+#endif
+}
 return __init;
 }
 
@@ -292,8 +304,13 @@
 *__result = __t;
 for (++__first, (void) ++__result; __first != __last; ++__first, (void) ++__result)
 {
+#if _LIBCPP_STD_VER > 17
+__t = __binary_op(_VSTD::move(__t), *__first);
+*__result = __t;
+#else
 __t = __binary_op(__t, *__first);
 *__result = __t;
+#endif
 }
 }
 return __result;
@@ -442,7 +459,11 @@
 for (++__first, (void) ++__result; __first != __last; ++__first, (void) ++__result)
 {
 typename iterator_traits<_InputIterator>::value_type __t2(*__first);
+#if _LIBCPP_STD_VER > 17
+*__result = __binary_op(__t2, _VSTD::move(__t1));
+#else
 *__result = __binary_op(__t2, __t1);
+#endif
 __t1 = _VSTD::move(__t2);
 }
 }
Index: libcxx/include/forward_list
===
--- libcxx/include/forward_list
+++ libcxx/include/forward_list
@@ -1502,11 +1502,12 @@
 #endif  // _LIBCPP_CXX03_LANG
 
 template 
-void
+decltype(auto)
 forward_list<_Tp, _Alloc>::remove(const value_type& __v)
 {
 forward_list<_Tp, _Alloc> __deleted_nodes(get_allocator()); // collect the nodes we're removing
 iterator __e = end();
+__VSTD::size_type __rm = 0;
 for (iterator __i = before_begin(); __i.__get_begin()->__next_ != nullptr;)
 {
 if (__i.__get_begin()->__next_->__value_ == __v)
@@ -1515,6 +1516,7 @@
 for (; __j != __e && *__j == __v; ++__j)
 ;
 __deleted_nodes.splice_after(__deleted_nodes.before_begin(), *this, __i, __j);
+++__rm;
 if (__j == __e)
 break;
 __i = __j;
@@ -1522,15 +1524,21 @@
 else
 ++__i;
 }
+#if _LIBCPP_STD_VER > 17
+return __rm;
+#else
+(void) __rm;
+#endif
 }
 
 template 
 template 
-void
+decltype(auto)
 forward_list<_Tp, _Alloc>::remove_if(_Predicate __pred)
 {
 forward_list<_Tp, _Alloc> __deleted_nodes(get_allocator()); // collect the nodes we're removing
 iterator __e = end();
+__VSTD::size_type __rm = 0;
 for (iterator __i = before_begin(); __i.__get_begin()->__next_ != nullptr;)
 {
 if (__pred(__i.__get_begin()->__next_->__value_))
@@ -1539,6 +1547,7 @@
 for (; __j != __e && __pred(*__j); ++__j)
 ;
 __deleted_nodes.splice_after(__deleted_nodes.before_begin(), *this, __i, __j);
+++__rm;
 if (__j == __e)
 break;
 __i = __j;
@@ -1546,6 +1555,11 @@
 else
 ++__i;
 }
+#if _LIBCPP_STD_VER > 17
+return __rm;
+#else
+(void) __rm;
+#endif
 }
 
 template 
Index: libcxx/include/algorithm
===
--- libcxx/include/algorithm
+++ libcxx/include/algorithm
@@ -281,6 +281,10 @@
 template 
 OutputIterator
 rotate_copy(ForwardIterator first, ForwardIterator middle, ForwardIterator last, OutputIterator result);
+ 
+template
+constexpr ForwardIterator shift_left(ForwardIterator first, ForwardIterator last,
+typename iterator_traits::difference_type n);
 
 template 
 void
@@ -3107,6 +3111,41 @@
 }
 #endif
 
+// shift_left
+#if _LIBCPP_STD_VE

[Lldb-commits] [lldb] f1b1173 - [lldb][NFC] Remove unused CompilerType memory functions

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-11-27T09:51:50+01:00
New Revision: f1b117394d7f9ae6decf9730ed9d443ca1b54769

URL: 
https://github.com/llvm/llvm-project/commit/f1b117394d7f9ae6decf9730ed9d443ca1b54769
DIFF: 
https://github.com/llvm/llvm-project/commit/f1b117394d7f9ae6decf9730ed9d443ca1b54769.diff

LOG: [lldb][NFC] Remove unused CompilerType memory functions

Summary:
All these functions are unused from what I can see. Unless I'm missing 
something here, this code
can go the way of the Dodo.

Reviewers: labath

Reviewed By: labath

Subscribers: abidh, JDevlieghere, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Symbol/CompilerType.h
lldb/source/Symbol/CompilerType.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index cedd2523a5a8..91d9c5e48d20 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -357,14 +357,6 @@ class CompilerType {
   bool GetValueAsScalar(const DataExtractor &data, lldb::offset_t data_offset,
 size_t data_byte_size, Scalar &value) const;
 
-  bool SetValueFromScalar(const Scalar &value, Stream &strm);
-
-  bool ReadFromMemory(ExecutionContext *exe_ctx, lldb::addr_t addr,
-  AddressType address_type, DataExtractor &data);
-
-  bool WriteToMemory(ExecutionContext *exe_ctx, lldb::addr_t addr,
- AddressType address_type, StreamString &new_value);
-
   void Clear() {
 m_type = nullptr;
 m_type_system = nullptr;

diff  --git a/lldb/source/Symbol/CompilerType.cpp 
b/lldb/source/Symbol/CompilerType.cpp
index 571a8570a43b..d35213120b4d 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -874,173 +874,6 @@ bool CompilerType::GetValueAsScalar(const 
lldb_private::DataExtractor &data,
   return false;
 }
 
-bool CompilerType::SetValueFromScalar(const Scalar &value, Stream &strm) {
-  if (!IsValid())
-return false;
-
-  // Aggregate types don't have scalar values
-  if (!IsAggregateType()) {
-strm.GetFlags().Set(Stream::eBinary);
-uint64_t count = 0;
-lldb::Encoding encoding = GetEncoding(count);
-
-if (encoding == lldb::eEncodingInvalid || count != 1)
-  return false;
-
-llvm::Optional bit_width = GetBitSize(nullptr);
-if (!bit_width)
-  return false;
-
-// This function doesn't currently handle non-byte aligned assignments
-if ((*bit_width % 8) != 0)
-  return false;
-
-const uint64_t byte_size = (*bit_width + 7) / 8;
-switch (encoding) {
-case lldb::eEncodingInvalid:
-  break;
-case lldb::eEncodingVector:
-  break;
-case lldb::eEncodingUint:
-  switch (byte_size) {
-  case 1:
-strm.PutHex8(value.UInt());
-return true;
-  case 2:
-strm.PutHex16(value.UInt());
-return true;
-  case 4:
-strm.PutHex32(value.UInt());
-return true;
-  case 8:
-strm.PutHex64(value.ULongLong());
-return true;
-  default:
-break;
-  }
-  break;
-
-case lldb::eEncodingSint:
-  switch (byte_size) {
-  case 1:
-strm.PutHex8(value.SInt());
-return true;
-  case 2:
-strm.PutHex16(value.SInt());
-return true;
-  case 4:
-strm.PutHex32(value.SInt());
-return true;
-  case 8:
-strm.PutHex64(value.SLongLong());
-return true;
-  default:
-break;
-  }
-  break;
-
-case lldb::eEncodingIEEE754:
-  if (byte_size <= sizeof(long double)) {
-if (byte_size == sizeof(float)) {
-  strm.PutFloat(value.Float());
-  return true;
-} else if (byte_size == sizeof(double)) {
-  strm.PutDouble(value.Double());
-  return true;
-} else if (byte_size == sizeof(long double)) {
-  strm.PutDouble(value.LongDouble());
-  return true;
-}
-  }
-  break;
-}
-  }
-  return false;
-}
-
-bool CompilerType::ReadFromMemory(lldb_private::ExecutionContext *exe_ctx,
-  lldb::addr_t addr, AddressType address_type,
-  lldb_private::DataExtractor &data) {
-  if (!IsValid())
-return false;
-
-  // Can't convert a file address to anything valid without more context (which
-  // Module it came from)
-  if (address_type == eAddressTypeFile)
-return false;
-
-  if (!GetCompleteType())
-return false;
-
-  auto byte_size =
-  GetByteSize(exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
-  if (!byte_size)
-return false;
-
-  if (data.GetByteSize() < *byte_size) {
-lldb::DataBufferSP data_sp(new DataBufferHeap(*byte_size, '\0'));
-data.SetData(data_sp);
-  }
-
-  uint8_t *dst = const_cast(data.PeekD

[Lldb-commits] [PATCH] D70770: [lldb][NFC] Remove unused CompilerType memory functions

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf1b117394d7f: [lldb][NFC] Remove unused CompilerType memory 
functions (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70770

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -874,173 +874,6 @@
   return false;
 }
 
-bool CompilerType::SetValueFromScalar(const Scalar &value, Stream &strm) {
-  if (!IsValid())
-return false;
-
-  // Aggregate types don't have scalar values
-  if (!IsAggregateType()) {
-strm.GetFlags().Set(Stream::eBinary);
-uint64_t count = 0;
-lldb::Encoding encoding = GetEncoding(count);
-
-if (encoding == lldb::eEncodingInvalid || count != 1)
-  return false;
-
-llvm::Optional bit_width = GetBitSize(nullptr);
-if (!bit_width)
-  return false;
-
-// This function doesn't currently handle non-byte aligned assignments
-if ((*bit_width % 8) != 0)
-  return false;
-
-const uint64_t byte_size = (*bit_width + 7) / 8;
-switch (encoding) {
-case lldb::eEncodingInvalid:
-  break;
-case lldb::eEncodingVector:
-  break;
-case lldb::eEncodingUint:
-  switch (byte_size) {
-  case 1:
-strm.PutHex8(value.UInt());
-return true;
-  case 2:
-strm.PutHex16(value.UInt());
-return true;
-  case 4:
-strm.PutHex32(value.UInt());
-return true;
-  case 8:
-strm.PutHex64(value.ULongLong());
-return true;
-  default:
-break;
-  }
-  break;
-
-case lldb::eEncodingSint:
-  switch (byte_size) {
-  case 1:
-strm.PutHex8(value.SInt());
-return true;
-  case 2:
-strm.PutHex16(value.SInt());
-return true;
-  case 4:
-strm.PutHex32(value.SInt());
-return true;
-  case 8:
-strm.PutHex64(value.SLongLong());
-return true;
-  default:
-break;
-  }
-  break;
-
-case lldb::eEncodingIEEE754:
-  if (byte_size <= sizeof(long double)) {
-if (byte_size == sizeof(float)) {
-  strm.PutFloat(value.Float());
-  return true;
-} else if (byte_size == sizeof(double)) {
-  strm.PutDouble(value.Double());
-  return true;
-} else if (byte_size == sizeof(long double)) {
-  strm.PutDouble(value.LongDouble());
-  return true;
-}
-  }
-  break;
-}
-  }
-  return false;
-}
-
-bool CompilerType::ReadFromMemory(lldb_private::ExecutionContext *exe_ctx,
-  lldb::addr_t addr, AddressType address_type,
-  lldb_private::DataExtractor &data) {
-  if (!IsValid())
-return false;
-
-  // Can't convert a file address to anything valid without more context (which
-  // Module it came from)
-  if (address_type == eAddressTypeFile)
-return false;
-
-  if (!GetCompleteType())
-return false;
-
-  auto byte_size =
-  GetByteSize(exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
-  if (!byte_size)
-return false;
-
-  if (data.GetByteSize() < *byte_size) {
-lldb::DataBufferSP data_sp(new DataBufferHeap(*byte_size, '\0'));
-data.SetData(data_sp);
-  }
-
-  uint8_t *dst = const_cast(data.PeekData(0, *byte_size));
-  if (dst != nullptr) {
-if (address_type == eAddressTypeHost) {
-  if (addr == 0)
-return false;
-  // The address is an address in this process, so just copy it
-  memcpy(dst, reinterpret_cast(addr), *byte_size);
-  return true;
-} else {
-  Process *process = nullptr;
-  if (exe_ctx)
-process = exe_ctx->GetProcessPtr();
-  if (process) {
-Status error;
-return process->ReadMemory(addr, dst, *byte_size, error) == *byte_size;
-  }
-}
-  }
-  return false;
-}
-
-bool CompilerType::WriteToMemory(lldb_private::ExecutionContext *exe_ctx,
- lldb::addr_t addr, AddressType address_type,
- StreamString &new_value) {
-  if (!IsValid())
-return false;
-
-  // Can't convert a file address to anything valid without more context (which
-  // Module it came from)
-  if (address_type == eAddressTypeFile)
-return false;
-
-  if (!GetCompleteType())
-return false;
-
-  auto byte_size =
-  GetByteSize(exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
-  if (!byte_size)
-return false;
-
-  if (*byte_size > 0) {
-if (address_type == eAddressTypeHost) {
-  // The address is an address in this process, so just copy it
-  memcpy((void *)addr, new_value.GetData(), *byte_size);
-  return tr

[Lldb-commits] [PATCH] D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D70745#1760917 , @amccarth wrote:

> Is `.eh_frame` the only one that matters?  Should this be more general and 
> compare `const_sect_name` to the full name and the truncated name for any 
> known section names?


Out of the sections name I see in executables, it's only `.eh_frame` where this 
is relevant. LLD produces another similarly truncated section name, 
`.gcc_except_table` truncated to `.gcc_exc`, but LLDB doesn't look for that one.

As for doing it in general for any known section name; I think that could end 
up ambiguous for some of the `.debug_*` section types, like `.debug_line` and 
`.debug_loc` which both would end up as `.debug_l` in truncated form. Although, 
as those section names wouldn't be truncated in the executables, I don't think 
a generic check for a truncated form would hurt either.

> Although not directly relevant to this patch, it seems like a lot this 
> cascade could be made more readable and maintainable by replacing the simple 
> if-name-set-section_type cases were replaced with a lookup into a table (and 
> that lookup could be the one place to check both the full name and the 
> truncated name).



In D70745#1761259 , @labath wrote:

> I think you should be able to write a test with a yaml2obj + `lldb-test 
> object-file`. That's how the equivalent elf functionality is tested (see 
> `test/Shell/ObjectFile/ELF/section-types.yaml`). It won't check that the 
> section is actually parsed properly, but I don't think that's needed for the 
> kind of fix you're making here.


Ok, thanks! Yes, that's exactly the kind of test I was thinking of.

> As for the long if-else cascade, the equivalent elf code was recently 
> refactored to use llvm::StringSwitch. Doing the same here would be nice.

A StringSwitch sounds nice. Some of the cases come with extra conditions 
(header flags and checking section sizes) though - is it best to keep that as 
is, if the StringSwitch didn't match anything?

As for @amccarth's suggestion on generically checking for truncated forms, I 
guess that's not doable with a StringSwitch, but would require e.g. creating 
more of an ad-hoc table, that we could iterate over, checking for both full and 
truncated matches? Does that sound sensible to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70745



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

[Lldb-commits] [lldb] 92d5ea5 - [lldb][NFC] Move TypeSystem RTTI to static variable to remove swift reference

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-11-27T10:28:02+01:00
New Revision: 92d5ea5d1674c38e03d130c6b04afa118e94ef4a

URL: 
https://github.com/llvm/llvm-project/commit/92d5ea5d1674c38e03d130c6b04afa118e94ef4a
DIFF: 
https://github.com/llvm/llvm-project/commit/92d5ea5d1674c38e03d130c6b04afa118e94ef4a.diff

LOG: [lldb][NFC] Move TypeSystem RTTI to static variable to remove swift 
reference

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTContext.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Symbol/ClangASTContext.cpp
lldb/source/Symbol/CompilerDecl.cpp
lldb/source/Symbol/CompilerDeclContext.cpp
lldb/source/Symbol/TypeSystem.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
b/lldb/include/lldb/Symbol/ClangASTContext.h
index f4428c682182..20421bca305e 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -41,15 +41,17 @@ namespace lldb_private {
 class Declaration;
 
 class ClangASTContext : public TypeSystem {
+  // LLVM RTTI support
+  static char ID;
+
 public:
   typedef void (*CompleteTagDeclCallback)(void *baton, clang::TagDecl *);
   typedef void (*CompleteObjCInterfaceDeclCallback)(void *baton,
 clang::ObjCInterfaceDecl 
*);
 
   // llvm casting support
-  static bool classof(const TypeSystem *ts) {
-return ts->getKind() == TypeSystem::eKindClang;
-  }
+  bool isA(const void *ClassID) const override { return ClassID == &ID; }
+  static bool classof(const TypeSystem *ts) { return ts->isA(&ID); }
 
   // Constructors and Destructors
   explicit ClangASTContext(llvm::StringRef triple = "");

diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index 6283d67baba5..ea860647fdb1 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -52,47 +52,11 @@ struct LanguageSet {
 /// Interface for representing the Type Systems in 
diff erent languages.
 class TypeSystem : public PluginInterface {
 public:
-  // Intrusive type system that allows us to use llvm casting.
-  //
-  // To add a new type system:
-  //
-  // 1 - Add a new enumeration for llvm casting below for your TypeSystem
-  // subclass, here we will use eKindFoo
-  //
-  // 2 - Your TypeSystem subclass will inherit from TypeSystem and needs
-  // to implement a static classof() function that returns your
-  // enumeration:
-  //
-  //class Foo : public lldb_private::TypeSystem
-  //{
-  //static bool classof(const TypeSystem *ts)
-  //{
-  //return ts->getKind() == TypeSystem::eKindFoo;
-  //}
-  //};
-  //
-  // 3 - Contruct your TypeSystem subclass with the enumeration from below
-  //
-  //Foo() :
-  //TypeSystem(TypeSystem::eKindFoo),
-  //...
-  //{
-  //}
-  //
-  // Then you can use the llvm casting on any "TypeSystem *" to get an instance
-  // of your subclass.
-  enum LLVMCastKind {
-eKindClang,
-eKindSwift,
-kNumKinds
-  };
-
   // Constructors and Destructors
-  TypeSystem(LLVMCastKind kind);
-
   ~TypeSystem() override;
 
-  LLVMCastKind getKind() const { return m_kind; }
+  // LLVM RTTI support
+  virtual bool isA(const void *ClassID) const = 0;
 
   static lldb::TypeSystemSP CreateInstance(lldb::LanguageType language,
Module *module);
@@ -493,8 +457,7 @@ class TypeSystem : public PluginInterface {
   virtual bool IsMeaninglessWithoutDynamicResolution(void *type);
 
 protected:
-  const LLVMCastKind m_kind; // Support for llvm casting
-  SymbolFile *m_sym_file;
+  SymbolFile *m_sym_file = nullptr;
 };
 
 class TypeSystemMap {

diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 244ac8ce5ff8..e413029f0300 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -337,6 +337,8 @@ static ClangASTMap &GetASTMap() {
   return *g_map_ptr;
 }
 
+char ClangASTContext::ID;
+
 bool ClangASTContext::IsOperator(llvm::StringRef name,
  clang::OverloadedOperatorKind &op_kind) {
   // All operators have to start with "operator".
@@ -522,8 +524,7 @@ static void ParseLangArgs(LangOptions &Opts, InputKind IK, 
const char *triple) {
   Opts.NoInlineDefine = !Opt;
 }
 
-ClangASTContext::ClangASTContext(llvm::StringRef target_triple)
-: TypeSystem(TypeSystem::eKindClang) {
+ClangASTContext::ClangASTContext(llvm::StringRef target_triple) {
   if (!target_triple.empty())
 SetTargetTriple(target_triple);
   // The caller didn't pass an ASTContext so create a new one for this
@@ -531,16 +532,14 @@ ClangASTContext::ClangASTContext(llvm::StringRef 
target_triple)
   CreateASTContext();
 }
 
-ClangASTContext::ClangASTContext(ArchSpec arch)
-: TypeSystem(TypeSys

[Lldb-commits] [PATCH] D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D70745#1761392 , @mstorsjo wrote:

> > As for the long if-else cascade, the equivalent elf code was recently 
> > refactored to use llvm::StringSwitch. Doing the same here would be nice.
>
> A StringSwitch sounds nice. Some of the cases come with extra conditions 
> (header flags and checking section sizes) though - is it best to keep that as 
> is, if the StringSwitch didn't match anything?


I don't really know what are the conventions in the COFF world (you probably 
know that better), but I would tend to trust the section flags *more* than any 
name-based deductions. So I'd try to factor this such (and this is what I've 
done in the elf case) to do the section flag checks first, and only then fall 
back to the section names. The checks for the size 0 in data vs. bss sections 
looks weird to me, and I don't know if its really needed, but if you think it 
is, then it's probably best to pull that those cases out separately.

> As for @amccarth's suggestion on generically checking for truncated forms, I 
> guess that's not doable with a StringSwitch, but would require e.g. creating 
> more of an ad-hoc table, that we could iterate over, checking for both full 
> and truncated matches? Does that sound sensible to you?

If you think it's good to check the truncated name on all sections, then yes, 
some kind of a table would probably be nice. If it's just one or two cases, 
then you can just spell out both forms in the StringSwitch 
(`.Cases(".eh_frame", ".eh_fram", eSectionTypeEHFrame)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70745



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

[Lldb-commits] [PATCH] D70774: [lldb] NFC: refactor CompileUnit::ResolveSymbolContext

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
kwk planned changes to this revision.

I found the above named method hard to read because it had

a) many nested blocks and
b) one return statement at the end with some logic involved.

I decided to refactor this function by employing an early exit strategy.
In order to capture the logic in the return statement and to not have it
repeated more than once I chose to implement a very small lamda function
that captures all the variables it needs.

This is a non-functional change (NFC).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70774

Files:
  lldb/source/Symbol/CompileUnit.cpp

Index: lldb/source/Symbol/CompileUnit.cpp
===
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -275,80 +275,90 @@
 
   const uint32_t prev_size = sc_list.GetSize();
 
+  auto returnLambda = [&sc_list, prev_size] {
+return sc_list.GetSize() - prev_size;
+  };
+
   SymbolContext sc(GetModule());
   sc.comp_unit = this;
 
-  if (line != 0) {
-LineTable *line_table = sc.comp_unit->GetLineTable();
-
-if (line_table != nullptr) {
-  uint32_t found_line;
-  uint32_t line_idx;
-
-  if (num_file_indexes == 1) {
-// We only have a single support file that matches, so use the line
-// table function that searches for a line entries that match a single
-// support file index
-LineEntry line_entry;
-line_idx = line_table->FindLineEntryIndexByFileIndex(
-0, file_indexes.front(), line, exact, &line_entry);
-
-// If "exact == true", then "found_line" will be the same as "line". If
-// "exact == false", the "found_line" will be the closest line entry
-// with a line number greater than "line" and we will use this for our
-// subsequent line exact matches below.
-found_line = line_entry.line;
-
-while (line_idx != UINT32_MAX) {
-  // If they only asked for the line entry, then we're done, we can
-  // just copy that over. But if they wanted more than just the line
-  // number, fill it in.
-  if (resolve_scope == eSymbolContextLineEntry) {
-sc.line_entry = line_entry;
-  } else {
-line_entry.range.GetBaseAddress().CalculateSymbolContext(
-&sc, resolve_scope);
-  }
-
-  sc_list.Append(sc);
-  line_idx = line_table->FindLineEntryIndexByFileIndex(
-  line_idx + 1, file_indexes.front(), found_line, true,
-  &line_entry);
-}
-  } else {
-// We found multiple support files that match "file_spec" so use the
-// line table function that searches for a line entries that match a
-// multiple support file indexes.
-LineEntry line_entry;
-line_idx = line_table->FindLineEntryIndexByFileIndex(
-0, file_indexes, line, exact, &line_entry);
-
-// If "exact == true", then "found_line" will be the same as "line". If
-// "exact == false", the "found_line" will be the closest line entry
-// with a line number greater than "line" and we will use this for our
-// subsequent line exact matches below.
-found_line = line_entry.line;
-
-while (line_idx != UINT32_MAX) {
-  if (resolve_scope == eSymbolContextLineEntry) {
-sc.line_entry = line_entry;
-  } else {
-line_entry.range.GetBaseAddress().CalculateSymbolContext(
-&sc, resolve_scope);
-  }
-
-  sc_list.Append(sc);
-  line_idx = line_table->FindLineEntryIndexByFileIndex(
-  line_idx + 1, file_indexes, found_line, true, &line_entry);
-}
-  }
-}
+  if (line == 0) {
+return returnLambda();
   } else if (file_spec_matches_cu_file_spec && !check_inlines) {
 // only append the context if we aren't looking for inline call sites by
 // file and line and if the file spec matches that of the compile unit
 sc_list.Append(sc);
+return returnLambda();
+  }
+
+  LineTable *line_table = sc.comp_unit->GetLineTable();
+
+  if (line_table == nullptr)
+return returnLambda();
+
+  uint32_t found_line;
+  uint32_t line_idx;
+  LineEntry line_entry;
+
+  if (num_file_indexes == 1) {
+// We only have a single support file that matches, so use the line
+// table function that searches for a line entries that match a single
+// support file index
+line_idx = line_table->FindLineEntryIndexByFileIndex(
+0, file_indexes.front(), line, exact, &line_entry);
+
+// If "exact == true", then "found_line" will be the same as "line". If
+// "exact == false", the "found_line" will be the closest line entry
+// with a line number greater than "line" and we will use this for our
+// subseq

[Lldb-commits] [PATCH] D70774: [lldb] NFC: refactor CompileUnit::ResolveSymbolContext

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D70774#1761444 , @teemperor wrote:

> Let's just remove that return value. It's anyway only used in one place or so 
> where we can just call GetSize() manually. Then we also don't need the 
> lambda. Otherwise this LGTM, thanks!


Okay, fair enough. Let me do the changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70774



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

[Lldb-commits] [PATCH] D70774: [lldb] NFC: refactor CompileUnit::ResolveSymbolContext

[Raphael Isemann via Phabricator via lldb-commits]

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

Let's just remove that return value. It's anyway only used in one place or so 
where we can just call GetSize() manually. Then we also don't need the lambda. 
Otherwise this LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70774



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

[Lldb-commits] [PATCH] D70774: [lldb] NFC: refactor CompileUnit::ResolveSymbolContext

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 231201.
kwk added a comment.

Small changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70774

Files:
  lldb/source/Symbol/CompileUnit.cpp

Index: lldb/source/Symbol/CompileUnit.cpp
===
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -275,80 +275,72 @@
 
   const uint32_t prev_size = sc_list.GetSize();
 
+  auto returnLambda = [&sc_list, prev_size] {
+return sc_list.GetSize() - prev_size;
+  };
+
   SymbolContext sc(GetModule());
   sc.comp_unit = this;
 
-  if (line != 0) {
-LineTable *line_table = sc.comp_unit->GetLineTable();
-
-if (line_table != nullptr) {
-  uint32_t found_line;
-  uint32_t line_idx;
-
-  if (num_file_indexes == 1) {
-// We only have a single support file that matches, so use the line
-// table function that searches for a line entries that match a single
-// support file index
-LineEntry line_entry;
-line_idx = line_table->FindLineEntryIndexByFileIndex(
-0, file_indexes.front(), line, exact, &line_entry);
-
-// If "exact == true", then "found_line" will be the same as "line". If
-// "exact == false", the "found_line" will be the closest line entry
-// with a line number greater than "line" and we will use this for our
-// subsequent line exact matches below.
-found_line = line_entry.line;
-
-while (line_idx != UINT32_MAX) {
-  // If they only asked for the line entry, then we're done, we can
-  // just copy that over. But if they wanted more than just the line
-  // number, fill it in.
-  if (resolve_scope == eSymbolContextLineEntry) {
-sc.line_entry = line_entry;
-  } else {
-line_entry.range.GetBaseAddress().CalculateSymbolContext(
-&sc, resolve_scope);
-  }
-
-  sc_list.Append(sc);
-  line_idx = line_table->FindLineEntryIndexByFileIndex(
-  line_idx + 1, file_indexes.front(), found_line, true,
-  &line_entry);
-}
-  } else {
-// We found multiple support files that match "file_spec" so use the
-// line table function that searches for a line entries that match a
-// multiple support file indexes.
-LineEntry line_entry;
-line_idx = line_table->FindLineEntryIndexByFileIndex(
-0, file_indexes, line, exact, &line_entry);
-
-// If "exact == true", then "found_line" will be the same as "line". If
-// "exact == false", the "found_line" will be the closest line entry
-// with a line number greater than "line" and we will use this for our
-// subsequent line exact matches below.
-found_line = line_entry.line;
-
-while (line_idx != UINT32_MAX) {
-  if (resolve_scope == eSymbolContextLineEntry) {
-sc.line_entry = line_entry;
-  } else {
-line_entry.range.GetBaseAddress().CalculateSymbolContext(
-&sc, resolve_scope);
-  }
-
-  sc_list.Append(sc);
-  line_idx = line_table->FindLineEntryIndexByFileIndex(
-  line_idx + 1, file_indexes, found_line, true, &line_entry);
-}
-  }
-}
-  } else if (file_spec_matches_cu_file_spec && !check_inlines) {
+  if (line == 0)
+return returnLambda();
+  
+  if (file_spec_matches_cu_file_spec && !check_inlines) {
 // only append the context if we aren't looking for inline call sites by
 // file and line and if the file spec matches that of the compile unit
 sc_list.Append(sc);
+return returnLambda();
+  }
+
+  LineTable *line_table = sc.comp_unit->GetLineTable();
+
+  if (line_table == nullptr)
+return returnLambda();
+
+  uint32_t line_idx;
+  LineEntry line_entry;
+
+  if (num_file_indexes == 1) {
+// We only have a single support file that matches, so use the line
+// table function that searches for a line entries that match a single
+// support file index
+line_idx = line_table->FindLineEntryIndexByFileIndex(
+0, file_indexes.front(), line, exact, &line_entry);
+  } else {
+// We found multiple support files that match "file_spec" so use the
+// line table function that searches for a line entries that match a
+// multiple support file indexes.
+line_idx = line_table->FindLineEntryIndexByFileIndex(0, file_indexes, line,
+ exact, &line_entry);
+  }
+  
+  // If "exact == true", then "found_line" will be the same as "line". If
+  // "exact == false", the "found_line" will be the closest line entry
+  // with a line number greater than "line" and we will use this for our
+  // subsequent line exact matches below.
+  uint32_t found_line = line_entry.line;
+  
+  whi

[Lldb-commits] [PATCH] D70774: [lldb] NFC: refactor CompileUnit::ResolveSymbolContext

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

What we've started doing these days is to just remove the "number of things 
appended" return value from these kinds of functions, as they make it very easy 
to introduce bugs. (And most callers don't care about those, and those that do, 
can implement this easily on their own.)

It doesn't look like this function has that many callers, so it should be easy 
to do the same here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70774



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

[Lldb-commits] [PATCH] D70775: [lldb] Add MockTypeSystem and some basic unit test for CompilerType

[Raphael Isemann via Phabricator via lldb-commits]

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

There are a lot of classes using TypeSystem in some way (mostly indirectly
through CompilerType) and we can't really unittest this code at the moment 
without
having some kind of mock TypeSystem.

This adds a MockTypeSystem to the TestingSupport that provides a stub 
implementation
for all the deleted functions. Also adds a very basic CompilerTest that just 
uses the MockTypeSystem
as a simple TypeSystem placeholder.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70775

Files:
  lldb/unittests/Symbol/CMakeLists.txt
  lldb/unittests/Symbol/CompilerTypeTest.cpp
  lldb/unittests/TestingSupport/CMakeLists.txt
  lldb/unittests/TestingSupport/MockTypeSystem.cpp
  lldb/unittests/TestingSupport/MockTypeSystem.h

Index: lldb/unittests/TestingSupport/MockTypeSystem.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/MockTypeSystem.h
@@ -0,0 +1,407 @@
+//===-- MockTypeSystem.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
+//
+//===--===//
+
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeSystem.h"
+
+namespace lldb_private {
+/// A TypeSystem implementation that throws an error if any of its methods are
+/// called. Should be used as a base class for new TypeSystem subclasses in
+/// tests.
+struct MockTypeSystem : public TypeSystem {
+  // llvm casting support
+  static char ID;
+  bool isA(const void *ClassID) const override { return ClassID == &ID; }
+  static bool classof(const TypeSystem *ts) { return ts->isA(&ID); }
+
+  ConstString GetPluginName() override { llvm_unreachable("not implemented"); }
+
+  uint32_t GetPluginVersion() override { llvm_unreachable("not implemented"); }
+
+  ConstString DeclGetName(void *opaque_decl) override {
+llvm_unreachable("not implemented");
+  }
+
+  CompilerType GetTypeForDecl(void *opaque_decl) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool DeclContextIsStructUnionOrClass(void *opaque_decl_ctx) override {
+llvm_unreachable("not implemented");
+  }
+
+  ConstString DeclContextGetName(void *opaque_decl_ctx) override {
+llvm_unreachable("not implemented");
+  }
+
+  ConstString DeclContextGetScopeQualifiedName(void *opaque_decl_ctx) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool
+  DeclContextIsClassMethod(void *opaque_decl_ctx,
+   lldb::LanguageType *language_ptr,
+   bool *is_instance_method_ptr,
+   ConstString *language_object_name_ptr) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool DeclContextIsContainedInLookup(void *opaque_decl_ctx,
+  void *other_opaque_decl_ctx) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsArrayType(lldb::opaque_compiler_type_t type,
+   CompilerType *element_type, uint64_t *size,
+   bool *is_incomplete) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsAggregateType(lldb::opaque_compiler_type_t type) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsCharType(lldb::opaque_compiler_type_t type) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsCompleteType(lldb::opaque_compiler_type_t type) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsDefined(lldb::opaque_compiler_type_t type) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsFloatingPointType(lldb::opaque_compiler_type_t type, uint32_t &count,
+   bool &is_complex) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsFunctionType(lldb::opaque_compiler_type_t type,
+  bool *is_variadic_ptr) override {
+llvm_unreachable("not implemented");
+  }
+
+  size_t
+  GetNumberOfFunctionArguments(lldb::opaque_compiler_type_t type) override {
+llvm_unreachable("not implemented");
+  }
+
+  CompilerType GetFunctionArgumentAtIndex(lldb::opaque_compiler_type_t type,
+  const size_t index) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsFunctionPointerType(lldb::opaque_compiler_type_t type) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsBlockPointerType(lldb::opaque_compiler_type_t type,
+  CompilerType *function_pointer_type_ptr) override {
+llvm_unreachable("not implemented");
+  }
+
+  bool IsIntegerType(lldb::opaque_compiler_type_t type,
+ bool &is_signed

[Lldb-commits] [PATCH] D70775: [lldb] Add MockTypeSystem and some basic unit test for CompilerType

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this should go into `TestingSupport/Symbol` (new folder) similar to how 
NativeProcessUtils.h is in `TestingSupport/Host` (to avoid the basic testing 
support functionality depending on the whole world).

It's not obvious how you intend to use this class, but instead of defining all 
methods as llvm_unreachable you could consider mocking them via gmock. This way 
you wouldn't need to create a separate mock class for each use case, and could 
instead "program" the mocks via something like `EXPECT_CALL(my_mock, 
GetNumVirtualBaseClasses(42)).Return(47);`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D70775



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

[Lldb-commits] [PATCH] D70778: [LLDB] [PECOFF] Factorize mapping section names to types using StringSwitch. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth.
Herald added a project: LLDB.

Keep the existing special cases based on combinations of section name, flags 
and sizes/offsets.

Some of those special cases have been added intentionally, with test cases, 
rather recently, so I don't want to try to generalize them at the moment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70778

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -810,34 +810,36 @@
 const uint32_t nsects = m_sect_headers.size();
 ModuleSP module_sp(GetModule());
 for (uint32_t idx = 0; idx < nsects; ++idx) {
-  ConstString const_sect_name(GetSectionName(m_sect_headers[idx]));
+  llvm::StringRef sect_name = GetSectionName(m_sect_headers[idx]);
+  ConstString const_sect_name(sect_name);
   static ConstString g_code_sect_name(".code");
   static ConstString g_CODE_sect_name("CODE");
   static ConstString g_data_sect_name(".data");
   static ConstString g_DATA_sect_name("DATA");
   static ConstString g_bss_sect_name(".bss");
   static ConstString g_BSS_sect_name("BSS");
-  static ConstString g_debug_sect_name(".debug");
-  static ConstString g_reloc_sect_name(".reloc");
-  static ConstString g_stab_sect_name(".stab");
-  static ConstString g_stabstr_sect_name(".stabstr");
-  static ConstString g_sect_name_dwarf_debug_abbrev(".debug_abbrev");
-  static ConstString g_sect_name_dwarf_debug_aranges(".debug_aranges");
-  static ConstString g_sect_name_dwarf_debug_frame(".debug_frame");
-  static ConstString g_sect_name_dwarf_debug_info(".debug_info");
-  static ConstString g_sect_name_dwarf_debug_line(".debug_line");
-  static ConstString g_sect_name_dwarf_debug_loc(".debug_loc");
-  static ConstString g_sect_name_dwarf_debug_loclists(".debug_loclists");
-  static ConstString g_sect_name_dwarf_debug_macinfo(".debug_macinfo");
-  static ConstString g_sect_name_dwarf_debug_names(".debug_names");
-  static ConstString g_sect_name_dwarf_debug_pubnames(".debug_pubnames");
-  static ConstString g_sect_name_dwarf_debug_pubtypes(".debug_pubtypes");
-  static ConstString g_sect_name_dwarf_debug_ranges(".debug_ranges");
-  static ConstString g_sect_name_dwarf_debug_str(".debug_str");
-  static ConstString g_sect_name_dwarf_debug_types(".debug_types");
-  static ConstString g_sect_name_eh_frame(".eh_frame");
-  static ConstString g_sect_name_go_symtab(".gosymtab");
-  SectionType section_type = eSectionTypeOther;
+  SectionType section_type =
+  llvm::StringSwitch(sect_name)
+  .Case(".debug", eSectionTypeDebug)
+  .Case(".stabstr", eSectionTypeDataCString)
+  .Case(".reloc", eSectionTypeOther)
+  .Case(".debug_abbrev", eSectionTypeDWARFDebugAbbrev)
+  .Case(".debug_aranges", eSectionTypeDWARFDebugAranges)
+  .Case(".debug_frame", eSectionTypeDWARFDebugFrame)
+  .Case(".debug_info", eSectionTypeDWARFDebugInfo)
+  .Case(".debug_line", eSectionTypeDWARFDebugLine)
+  .Case(".debug_loc", eSectionTypeDWARFDebugLoc)
+  .Case(".debug_loclists", eSectionTypeDWARFDebugLocLists)
+  .Case(".debug_macinfo", eSectionTypeDWARFDebugMacInfo)
+  .Case(".debug_names", eSectionTypeDWARFDebugNames)
+  .Case(".debug_pubnames", eSectionTypeDWARFDebugPubNames)
+  .Case(".debug_pubtypes", eSectionTypeDWARFDebugPubTypes)
+  .Case(".debug_ranges", eSectionTypeDWARFDebugRanges)
+  .Case(".debug_str", eSectionTypeDWARFDebugStr)
+  .Case(".debug_types", eSectionTypeDWARFDebugTypes)
+  .Case(".eh_frame", eSectionTypeEHFrame)
+  .Case(".gosymtab", eSectionTypeGoSymtab)
+  .Default(eSectionTypeInvalid);
   if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_CNT_CODE &&
   ((const_sect_name == g_code_sect_name) ||
(const_sect_name == g_CODE_sect_name))) {
@@ -858,45 +860,11 @@
   section_type = eSectionTypeZeroFill;
 else
   section_type = eSectionTypeData;
-  } else if (const_sect_name == g_debug_sect_name) {
-section_type = eSectionTypeDebug;
-  } else if (const_sect_name == g_stabstr_sect_name) {
-section_type = eSectionTypeDataCString;
-  } else if (const_sect_name == g_reloc_sect_name) {
-section_type = eSectionTypeOther;
-  } else if (const_sect_name == g_sect_name_dwarf_debug_abbrev)
-section_type = eSectionTypeDWARFDebugAbbrev;
-  else if (const_sect_name == g_sect_name_dwarf_debug_ara

[Lldb-commits] [PATCH] D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D70745#1761421 , @labath wrote:

> In D70745#1761392 , @mstorsjo wrote:
>
> > > As for the long if-else cascade, the equivalent elf code was recently 
> > > refactored to use llvm::StringSwitch. Doing the same here would be nice.
> >
> > A StringSwitch sounds nice. Some of the cases come with extra conditions 
> > (header flags and checking section sizes) though - is it best to keep that 
> > as is, if the StringSwitch didn't match anything?
>
>
> I don't really know what are the conventions in the COFF world (you probably 
> know that better), but I would tend to trust the section flags *more* than 
> any name-based deductions. So I'd try to factor this such (and this is what 
> I've done in the elf case) to do the section flag checks first, and only then 
> fall back to the section names. The checks for the size 0 in data vs. bss 
> sections looks weird to me, and I don't know if its really needed, but if you 
> think it is, then it's probably best to pull that those cases out separately.


Some of those special cases were added rather recently (1 year ago), so I'd 
rather not touch them. Checking the flags alone doesn't say much for e.g. 
"data" sections (that can be anything between actual data and debug info), so I 
didn't find much I dared to touch here, but it can at least be made some amount 
more readable, in D70778 .

>> As for @amccarth's suggestion on generically checking for truncated forms, I 
>> guess that's not doable with a StringSwitch, but would require e.g. creating 
>> more of an ad-hoc table, that we could iterate over, checking for both full 
>> and truncated matches? Does that sound sensible to you?
> 
> If you think it's good to check the truncated name on all sections, then yes, 
> some kind of a table would probably be nice. If it's just one or two cases, 
> then you can just spell out both forms in the StringSwitch 
> (`.Cases(".eh_frame", ".eh_fram", eSectionTypeEHFrame)`

Ok, going with just a single Cases here. A table and generic logic for checking 
truncated forms might be nice, but I don't see a direct need right now.


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

https://reviews.llvm.org/D70745



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

[Lldb-commits] [PATCH] D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 231212.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Updated on top of D70778 , added a testcase.


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

https://reviews.llvm.org/D70745

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/test/Shell/ObjectFile/PECOFF/section-types.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/section-types.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/section-types.yaml
@@ -0,0 +1,92 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK-LABEL: Name: .text
+# CHECK-NEXT: Type: code
+
+# CHECK-LABEL: Name: .eh_fram
+# CHECK-NEXT: Type: eh-frame
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4096
+  ImageBase:   4194304
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_NO_SEH, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ImportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:
+RelativeVirtualAddress: 0
+Size:0
+  CertificateTable:
+RelativeVirtualAddress: 0
+Size:0
+  BaseRelocationTable:
+RelativeVirtualAddress: 12288
+Size:12
+  Debug:
+RelativeVirtualAddress: 0
+Size:0
+  Architecture:
+RelativeVirtualAddress: 0
+Size:0
+  GlobalPtr:
+RelativeVirtualAddress: 0
+Size:0
+  TlsTable:
+RelativeVirtualAddress: 0
+Size:0
+  LoadConfigTable:
+RelativeVirtualAddress: 0
+Size:0
+  BoundImport:
+RelativeVirtualAddress: 0
+Size:0
+  IAT:
+RelativeVirtualAddress: 0
+Size:0
+  DelayImportDescriptor:
+RelativeVirtualAddress: 0
+Size:0
+  ClrRuntimeHeader:
+RelativeVirtualAddress: 0
+Size:0
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 5
+SectionData: 5589E55DC3
+  - Name:.eh_fram
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 52
+SectionData: 1400017A5200017C0801000C0404880118001C1045410E088502420D0500
+  - Name:.reloc
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  12288
+VirtualSize: 12
+SectionData: 00200C002030
+symbols:
+...
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -837,7 +837,8 @@
   .Case(".debug_ranges", eSectionTypeDWARFDebugRanges)
   .Case(".debug_str", eSectionTypeDWARFDebugStr)
   .Case(".debug_types", eSectionTypeDWARFDebugTypes)
-  .Case(".eh_frame", eSectionTypeEHFrame)
+  // .eh_frame can be truncated to 8 chars.
+  .Cases(".eh_frame", ".eh_fram", eSectionTypeEHFrame)
   .Case(".gosymtab", eSectionTypeGoSymtab)
   .Default(eSectionTypeInvalid);
   if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_CNT_CODE &&
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70775: [lldb] Add MockTypeSystem and some basic unit test for CompilerType

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 231226.
teemperor added a comment.

I tried moving this to gmock, but after an hour of not being able to even 
declare a simple virtual method due to issues like this 
 I don't think this approach 
is really practical. gmock is a nightmare to debug with all these completely 
undocumented macros. If gmock was actually documented and would work, then I 
think we should use it. But at the current state I can't even get it to compile 
code it seems without having to diagnostic-disable hacks, so that's a no-go

Also we anyway also want to model things like fake type hierarchies that one 
can traverse via CompilerType/TypeSystem (e.g. when doing completions like in 
the Variable completion code), so simple mocking is anyway not really the right 
fit.

Changes:

- Renamed MockTypeSystem -> FakeTypeSystem to reflect that this is not just 
mocking.
- Moved TestingSupport library to its own subdirectory and library that links 
against Symbol.
- Replaced `llvm_unreachable` in the unimplemented functions with macro to make 
migration easier if someone has a better approach to handling them.


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

https://reviews.llvm.org/D70775

Files:
  lldb/unittests/Symbol/CMakeLists.txt
  lldb/unittests/Symbol/CompilerTypeTest.cpp
  lldb/unittests/TestingSupport/CMakeLists.txt
  lldb/unittests/TestingSupport/Symbol/CMakeLists.txt
  lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.cpp
  lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.h

Index: lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.h
@@ -0,0 +1,408 @@
+//===-- FakeTypeSystem.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
+//
+//===--===//
+
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeSystem.h"
+
+namespace lldb_private {
+/// A TypeSystem implementation that throws an error if any of its methods are
+/// called. Should be used as a base class for new TypeSystem subclasses in
+/// tests.
+struct FakeTypeSystem : public TypeSystem {
+  // llvm casting support
+  static char ID;
+  bool isA(const void *ClassID) const override { return ClassID == &ID; }
+  static bool classof(const TypeSystem *ts) { return ts->isA(&ID); }
+
+#define UNIMPLEMENTED_METHOD llvm_unreachable("not implemented")
+
+  ConstString GetPluginName() override { UNIMPLEMENTED_METHOD; }
+
+  uint32_t GetPluginVersion() override { UNIMPLEMENTED_METHOD; }
+
+  ConstString DeclGetName(void *opaque_decl) override { UNIMPLEMENTED_METHOD; }
+
+  CompilerType GetTypeForDecl(void *opaque_decl) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool DeclContextIsStructUnionOrClass(void *opaque_decl_ctx) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  ConstString DeclContextGetName(void *opaque_decl_ctx) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  ConstString DeclContextGetScopeQualifiedName(void *opaque_decl_ctx) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool
+  DeclContextIsClassMethod(void *opaque_decl_ctx,
+   lldb::LanguageType *language_ptr,
+   bool *is_instance_method_ptr,
+   ConstString *language_object_name_ptr) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool DeclContextIsContainedInLookup(void *opaque_decl_ctx,
+  void *other_opaque_decl_ctx) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsArrayType(lldb::opaque_compiler_type_t type,
+   CompilerType *element_type, uint64_t *size,
+   bool *is_incomplete) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsAggregateType(lldb::opaque_compiler_type_t type) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsCharType(lldb::opaque_compiler_type_t type) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsCompleteType(lldb::opaque_compiler_type_t type) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsDefined(lldb::opaque_compiler_type_t type) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsFloatingPointType(lldb::opaque_compiler_type_t type, uint32_t &count,
+   bool &is_complex) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsFunctionType(lldb::opaque_compiler_type_t type,
+  bool *is_variadic_ptr) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  size_t
+  GetNumberOfFunctionArguments(lldb::opaque_compiler_type_t type) override {
+UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetFunctionArgumentAtIndex(lldb::opaque_compiler_type_t type,
+  

[Lldb-commits] [PATCH] D70778: [LLDB] [PECOFF] Factorize mapping section names to types using StringSwitch. NFCI.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:864-866
+// If the StringSwitch above picked any type, including
+// eSectionTypeOther, accept that instead of the generic mappings
+// based on flags below.

This makes pretty weird control flow. I think it would be way clearer if all of 
this code were moved into a function like `GetSectionType` (there's a function 
like that in ObjectFileELF). Then you can use return statements to shortcut 
control flow, like so:
```
if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_CNT_CODE &&
  ((const_sect_name == g_code_sect_name) ||
   (const_sect_name == g_CODE_sect_name)))
  return eSectionTypeCode;
if (...)
  return eSectionTypeZeroFill;
SectionType type = StringSwitch(name)...;
if (type != Invalid)
  return type;
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70778



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

[Lldb-commits] [PATCH] D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name

[Adrian McCarthy via Phabricator via lldb-commits]

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

Nice.  LGTM.


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

https://reviews.llvm.org/D70745



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

[Lldb-commits] [PATCH] D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

In D70745#1761544 , @mstorsjo wrote:

> In D70745#1761421 , @labath wrote:
>
> > In D70745#1761392 , @mstorsjo 
> > wrote:
> >
> > > > As for the long if-else cascade, the equivalent elf code was recently 
> > > > refactored to use llvm::StringSwitch. Doing the same here would be nice.
> > >
> > > A StringSwitch sounds nice. Some of the cases come with extra conditions 
> > > (header flags and checking section sizes) though - is it best to keep 
> > > that as is, if the StringSwitch didn't match anything?
> >
> >
> > I don't really know what are the conventions in the COFF world (you 
> > probably know that better), but I would tend to trust the section flags 
> > *more* than any name-based deductions. So I'd try to factor this such (and 
> > this is what I've done in the elf case) to do the section flag checks 
> > first, and only then fall back to the section names. The checks for the 
> > size 0 in data vs. bss sections looks weird to me, and I don't know if its 
> > really needed, but if you think it is, then it's probably best to pull that 
> > those cases out separately.
>
>
> Some of those special cases were added rather recently (1 year ago), so I'd 
> rather not touch them. Checking the flags alone doesn't say much for e.g. 
> "data" sections (that can be anything between actual data and debug info), so 
> I didn't find much I dared to touch here, but it can at least be made some 
> amount more readable, in D70778 .
>
> >> As for @amccarth's suggestion on generically checking for truncated forms, 
> >> I guess that's not doable with a StringSwitch, but would require e.g. 
> >> creating more of an ad-hoc table, that we could iterate over, checking for 
> >> both full and truncated matches? Does that sound sensible to you?
> > 
> > If you think it's good to check the truncated name on all sections, then 
> > yes, some kind of a table would probably be nice. If it's just one or two 
> > cases, then you can just spell out both forms in the StringSwitch 
> > (`.Cases(".eh_frame", ".eh_fram", eSectionTypeEHFrame)`
>
> Ok, going with just a single Cases here. A table and generic logic for 
> checking truncated forms might be nice, but I don't see a direct need right 
> now.


SGTM.


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

https://reviews.llvm.org/D70745



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

[Lldb-commits] [PATCH] D70775: [lldb] Add MockTypeSystem and some basic unit test for CompilerType

[Pavel Labath via Phabricator via lldb-commits]

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

In D70775#1761597 , @teemperor wrote:

> I tried moving this to gmock, but after an hour of not being able to even 
> declare a simple virtual method due to issues like this 
>  I don't think this approach 
> is really practical. gmock is a nightmare to debug with all these completely 
> undocumented macros. If gmock was actually documented and would work, then I 
> think we should use it. But at the current state I can't even get it to 
> compile code it seems without having to diagnostic-disable hacks, so that's a 
> no-go


That's easy to work around by just not using any "override" keywords in the 
class which defines the mock methods. This is unfortunate, but I don't think 
it's a showstopper. If anything, it's a reason for updating to a newer gmock 
version, since that's solved there.

I'm not sure what documentation you were looking at, but overall, I've found 
googlemock to be documented 

 pretty well (definitely better than llvm or lldb).

So, I don't think either of these things are a reason to *not use* it. However, 
they are not reasons for *using* it either. Gmock is good for some cases, and 
not so good for others. Right now it's not clear to me what kind of uses you 
have in mind, but in the simple tests you have in this patch, it is true that 
gmock would not provide any value. We can revisit that decision later, or add a 
new gmock-based type system class (there's nothing stopping us from having more 
than one).

> 
> 
>   Also we anyway also want to model things like fake type hierarchies that 
> one can traverse via CompilerType/TypeSystem (e.g. when doing completions 
> like in the Variable completion code), so simple mocking is anyway not really 
> the right fit.

Yes, that is something where gmock would not be particularly useful..

> Changes:
> 
> - Renamed MockTypeSystem -> FakeTypeSystem to reflect that this is not just 
> mocking.
> - Moved TestingSupport library to its own subdirectory and library that links 
> against Symbol.

Thanks.

> - Replaced `llvm_unreachable` in the unimplemented functions with macro to 
> make migration easier if someone has a better approach to handling them.

I don't think this brings any value (the "verboseness" of 
`llvm_unreachable("unimplemented")` was not my main issue with this approach). 
Let's just revert that.




Comment at: lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.h:22
+
+#define UNIMPLEMENTED_METHOD llvm_unreachable("not implemented")
+

I don't think this is worth a macro. If you really want to save a couple of 
characters you can just declare a `unimplemented` static member function...


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

https://reviews.llvm.org/D70775



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

[Lldb-commits] [PATCH] D70778: [LLDB] [PECOFF] Factorize mapping section names to types using StringSwitch. NFCI.

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:864-866
+// If the StringSwitch above picked any type, including
+// eSectionTypeOther, accept that instead of the generic mappings
+// based on flags below.

labath wrote:
> This makes pretty weird control flow. I think it would be way clearer if all 
> of this code were moved into a function like `GetSectionType` (there's a 
> function like that in ObjectFileELF). Then you can use return statements to 
> shortcut control flow, like so:
> ```
> if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_CNT_CODE &&
>   ((const_sect_name == g_code_sect_name) ||
>(const_sect_name == g_CODE_sect_name)))
>   return eSectionTypeCode;
> if (...)
>   return eSectionTypeZeroFill;
> SectionType type = StringSwitch(name)...;
> if (type != Invalid)
>   return type;
> ...
> ```
+1.  Factoring the decision tree into a separate function is part of what I was 
trying to ask for in the patch.  That it simplifies things with early returns 
is an added bonus.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70778



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

[Lldb-commits] [PATCH] D70772: I implemented the features listed in this document: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0616r0.pdf and built libc++ using ninja without any errors/w

[Marshall Clow via Phabricator via lldb-commits]

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

This is a mess. 
The changes in `libcxx/CMakeLists.txt` are unrelated to P0616.
The changes in `algorithm` are also unrelated to P0616.
The changes in `forward_list` are also unrelated to P0616, **and** are based on 
an outdated version of that file.
You proposed the changes in `numeric` before (in D69286 
) and then never came back to them.
And you have no tests.

Please don't keep proposing the same things over and over.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70772



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

[Lldb-commits] [PATCH] D70778: [LLDB] [PECOFF] Factorize mapping section names to types using StringSwitch. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 231318.
mstorsjo added a comment.

Split the code into a separate function as suggested.


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

https://reviews.llvm.org/D70778

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -283,6 +283,8 @@
   void DumpDependentModules(lldb_private::Stream *s);
 
   llvm::StringRef GetSectionName(const section_header_t §);
+  static lldb::SectionType GetSectionType(llvm::StringRef sect_name,
+  const section_header_t §);
 
   typedef std::vector SectionHeaderColl;
   typedef SectionHeaderColl::iterator SectionHeaderCollIter;
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -787,6 +787,75 @@
   return false;
 }
 
+SectionType ObjectFilePECOFF::GetSectionType(llvm::StringRef sect_name,
+ const section_header_t §) {
+  ConstString const_sect_name(sect_name);
+  static ConstString g_code_sect_name(".code");
+  static ConstString g_CODE_sect_name("CODE");
+  static ConstString g_data_sect_name(".data");
+  static ConstString g_DATA_sect_name("DATA");
+  static ConstString g_bss_sect_name(".bss");
+  static ConstString g_BSS_sect_name("BSS");
+
+  if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_CODE &&
+  ((const_sect_name == g_code_sect_name) ||
+   (const_sect_name == g_CODE_sect_name))) {
+return eSectionTypeCode;
+  } else if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA &&
+ ((const_sect_name == g_data_sect_name) ||
+  (const_sect_name == g_DATA_sect_name))) {
+if (sect.size == 0 && sect.offset == 0)
+  return eSectionTypeZeroFill;
+else
+  return eSectionTypeData;
+  } else if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA &&
+ ((const_sect_name == g_bss_sect_name) ||
+  (const_sect_name == g_BSS_sect_name))) {
+if (sect.size == 0)
+  return eSectionTypeZeroFill;
+else
+  return eSectionTypeData;
+  }
+
+  SectionType section_type =
+  llvm::StringSwitch(sect_name)
+  .Case(".debug", eSectionTypeDebug)
+  .Case(".stabstr", eSectionTypeDataCString)
+  .Case(".reloc", eSectionTypeOther)
+  .Case(".debug_abbrev", eSectionTypeDWARFDebugAbbrev)
+  .Case(".debug_aranges", eSectionTypeDWARFDebugAranges)
+  .Case(".debug_frame", eSectionTypeDWARFDebugFrame)
+  .Case(".debug_info", eSectionTypeDWARFDebugInfo)
+  .Case(".debug_line", eSectionTypeDWARFDebugLine)
+  .Case(".debug_loc", eSectionTypeDWARFDebugLoc)
+  .Case(".debug_loclists", eSectionTypeDWARFDebugLocLists)
+  .Case(".debug_macinfo", eSectionTypeDWARFDebugMacInfo)
+  .Case(".debug_names", eSectionTypeDWARFDebugNames)
+  .Case(".debug_pubnames", eSectionTypeDWARFDebugPubNames)
+  .Case(".debug_pubtypes", eSectionTypeDWARFDebugPubTypes)
+  .Case(".debug_ranges", eSectionTypeDWARFDebugRanges)
+  .Case(".debug_str", eSectionTypeDWARFDebugStr)
+  .Case(".debug_types", eSectionTypeDWARFDebugTypes)
+  .Case(".eh_frame", eSectionTypeEHFrame)
+  .Case(".gosymtab", eSectionTypeGoSymtab)
+  .Default(eSectionTypeInvalid);
+  if (section_type != eSectionTypeInvalid)
+return section_type;
+
+  if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_CODE) {
+return eSectionTypeCode;
+  } else if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA) {
+return eSectionTypeData;
+  } else if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA) {
+if (sect.size == 0)
+  return eSectionTypeZeroFill;
+else
+  return eSectionTypeData;
+  } else {
+return eSectionTypeOther;
+  }
+}
+
 void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
   if (m_sections_up)
 return;
@@ -810,104 +879,9 @@
 const uint32_t nsects = m_sect_headers.size();
 ModuleSP module_sp(GetModule());
 for (uint32_t idx = 0; idx < nsects; ++idx) {
-  ConstString const_sect_name(GetSectionName(m_sect_headers[idx]));
-  static ConstString g_code_sect_name(".code");
-  static ConstString g_CODE_sect_name("CODE");
-  static ConstString g_data_sect_name(".data");
-  static ConstString g_DATA_sect_name("DATA");
-  static ConstString g_bss_sect_name(".bss");
-  static ConstString g_BSS_sect_name("BSS");
-  static ConstString g_debug_sect_nam

[Lldb-commits] [PATCH] D70796: [LLDB] Always interpret arm instructions as thumb on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth.
Herald added a subscriber: kristof.beyls.
Herald added a project: LLDB.

Windows on arm always uses thumb mode, and doesn't have most of the mechanisms 
that are used in e.g. ELF for distinguishing between arm and thumb.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70796

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/disassemble-thumb.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/disassemble-thumb.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/disassemble-thumb.yaml
@@ -0,0 +1,92 @@
+# RUN: yaml2obj %s > %t.exe
+# RUN: %lldb %t.exe -o "disassemble -b -n entry" -b | FileCheck %s
+
+# CHECK: {{.*}}.exe[0x401000] <+0>: 0x0040 lsls   r0, r0, #0x1
+# CHECK: {{.*}}.exe[0x401002] <+2>: 0x4770 bx lr
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4097
+  ImageBase:   4194304
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ImportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:
+RelativeVirtualAddress: 0
+Size:0
+  CertificateTable:
+RelativeVirtualAddress: 0
+Size:0
+  BaseRelocationTable:
+RelativeVirtualAddress: 0
+Size:0
+  Debug:
+RelativeVirtualAddress: 0
+Size:0
+  Architecture:
+RelativeVirtualAddress: 0
+Size:0
+  GlobalPtr:
+RelativeVirtualAddress: 0
+Size:0
+  TlsTable:
+RelativeVirtualAddress: 0
+Size:0
+  LoadConfigTable:
+RelativeVirtualAddress: 0
+Size:0
+  BoundImport:
+RelativeVirtualAddress: 0
+Size:0
+  IAT:
+RelativeVirtualAddress: 0
+Size:0
+  DelayImportDescriptor:
+RelativeVirtualAddress: 0
+Size:0
+  ClrRuntimeHeader:
+RelativeVirtualAddress: 0
+Size:0
+header:
+  Machine: IMAGE_FILE_MACHINE_ARMNT
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 4
+SectionData: '40007047'
+symbols:
+  - Name:.text
+Value:   0
+SectionNumber:   1
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_STATIC
+  - Name:entry
+Value:   0
+SectionNumber:   1
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+StorageClass:IMAGE_SYM_CLASS_EXTERNAL
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -1443,6 +1443,9 @@
 GetCore() == ArchSpec::Core::eCore_thumbv6m) {
   return true;
 }
+// Windows on ARM is always thumb.
+if (GetTriple().isOSWindows())
+  return true;
   }
   return false;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70797: [LLDB] Use r11 as frame pointer on Windows on ARM

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth.
Herald added a subscriber: kristof.beyls.
Herald added a project: LLDB.

The change in itself is straightforward based on reading the code, but I don't 
have a real world case where I know this made a difference, so I don't really 
know what to use to distill a test case out of though.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70797

Files:
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp


Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -850,6 +850,7 @@
 
   /* On Apple iOS et al, the frame pointer register is always r7.
* Typically on other ARM systems, thumb code uses r7; arm code uses r11.
+   * Windows on ARM, which is in thumb mode, uses r11 though.
*/
 
   uint32_t fp_regnum = 11;
@@ -857,7 +858,7 @@
   if (is_apple)
 fp_regnum = 7;
 
-  if (m_opcode_mode == eModeThumb)
+  if (m_opcode_mode == eModeThumb && !m_arch.GetTriple().isOSWindows())
 fp_regnum = 7;
 
   return fp_regnum;
@@ -879,6 +880,7 @@
 
   /* On Apple iOS et al, the frame pointer register is always r7.
* Typically on other ARM systems, thumb code uses r7; arm code uses r11.
+   * Windows on ARM, which is in thumb mode, uses r11 though.
*/
 
   uint32_t fp_regnum = dwarf_r11;
@@ -886,7 +888,7 @@
   if (is_apple)
 fp_regnum = dwarf_r7;
 
-  if (m_opcode_mode == eModeThumb)
+  if (m_opcode_mode == eModeThumb && !m_arch.GetTriple().isOSWindows())
 fp_regnum = dwarf_r7;
 
   return fp_regnum;


Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -850,6 +850,7 @@
 
   /* On Apple iOS et al, the frame pointer register is always r7.
* Typically on other ARM systems, thumb code uses r7; arm code uses r11.
+   * Windows on ARM, which is in thumb mode, uses r11 though.
*/
 
   uint32_t fp_regnum = 11;
@@ -857,7 +858,7 @@
   if (is_apple)
 fp_regnum = 7;
 
-  if (m_opcode_mode == eModeThumb)
+  if (m_opcode_mode == eModeThumb && !m_arch.GetTriple().isOSWindows())
 fp_regnum = 7;
 
   return fp_regnum;
@@ -879,6 +880,7 @@
 
   /* On Apple iOS et al, the frame pointer register is always r7.
* Typically on other ARM systems, thumb code uses r7; arm code uses r11.
+   * Windows on ARM, which is in thumb mode, uses r11 though.
*/
 
   uint32_t fp_regnum = dwarf_r11;
@@ -886,7 +888,7 @@
   if (is_apple)
 fp_regnum = dwarf_r7;
 
-  if (m_opcode_mode == eModeThumb)
+  if (m_opcode_mode == eModeThumb && !m_arch.GetTriple().isOSWindows())
 fp_regnum = dwarf_r7;
 
   return fp_regnum;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

In D49466#1761156 , @MaskRay wrote:

> The ugly path separator pattern `{{(/|)}}` appears in 60+ tests. Can we 
> teach clang and other tools to
>
> 1. accept both `/` and `\` input


In general they do, AFAIK, although it's not feasible in cases where `/` is the 
character that introduces an option, which is common on standard Windows 
utilities.

> 2. but only output `/`
> 
>   on Windows?

This is often actually incorrect for Windows.

> We can probably remove llvm::sys::path::Style::{windows,posix,native} from 
> include/llvm/Support/Path.h and only keep the posix form.

It's highly unlikely that will be correct for all cases, and certainly will not 
match users' expectations.  Debug info, for example, will want normal Windows 
syntax file paths with `\`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

@labath I think you are misunderstanding the patch.  This is not autoselecting 
the dependencies.  It is simply doing that based on an existing option that we 
have - `LLVM_ENABLE_ZLIB`.  We could always search for zlib and override the 
results with `LLVM_ENABLE_ZLIB` as well.  The current build will continue to 
just work - zlib is used only for the compressed debug sections (which requires 
the user to opt-in).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70764



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

[Lldb-commits] [PATCH] D70778: [LLDB] [PECOFF] Factorize mapping section names to types using StringSwitch. NFCI.

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for taking the time to do this. Just get rid of the `else`s and this is 
good to go.




Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:803-804
+   (const_sect_name == g_CODE_sect_name))) {
+return eSectionTypeCode;
+  } else if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA &&
+ ((const_sect_name == g_data_sect_name) ||

Now that this is a `return`, you don't need the `else` as per 
.


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

https://reviews.llvm.org/D70778



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

[Lldb-commits] [PATCH] D70796: [LLDB] Always interpret arm instructions as thumb on windows

[Pavel Labath via Phabricator via lldb-commits]

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

Seems reasonable. Just out of curiosity, is this restriction enforced in some 
way? E.g., if I manually do a `bx` to an arm address, will the system kill me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70796



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

[Lldb-commits] [PATCH] D70797: [LLDB] Use r11 as frame pointer on Windows on ARM

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jasonmolenda.
labath added a comment.

Presumably, this should change how unwind plans are computed. The unwind 
machinery will treat a `mov %sp, %fp`(whatever is the arm equivalent) as a 
signal that it should switch to an frame-pointer based unwind. This logic 
should not kick in if lldb does not think this register is the frame pointer 
register. You should be able to check this via `image show-unwind`. If that 
shows some difference then it should be possible to concoct a function where 
the unwind goes wrong because of this, but that's not necessary -- it should be 
enough to just check that a reasonable unwind plan is generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70797



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

[Lldb-commits] [PATCH] D70796: [LLDB] Always interpret arm instructions as thumb on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D70796#1762491 , @labath wrote:

> Seems reasonable. Just out of curiosity, is this restriction enforced in some 
> way? E.g., if I manually do a `bx` to an arm address, will the system kill me?


No, such simple cases do work (or at least they used to), but the linker 
doesn't really have any means to e.g. set the x bit in bl/blx instructions 
properly. And allegedly, in earlier versions, the kernel didn't switch modes 
correctly when entering the kernel, so entering the kernel from arm code, would 
try to execute the kernel's code in arm mode, effectively crashing the system 
(https://reviews.llvm.org/D43005#1018582 for reference). I think that aspect is 
fixed these days, but it's pretty much unsupported on most levels where the 
interworking requires some support from the tools or frameworks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70796



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