date:20230510

[Lldb-commits] [lldb] 25495c9 - [lldb][NFCI] Remove n^2 loops and simplify iterator usage

2023-05-10 Thread Felipe de Azevedo Piovezan via lldb-commits


Author: Felipe de Azevedo Piovezan
Date: 2023-05-10T06:18:27-04:00
New Revision: 25495c9b4c05cb52bacdbc91ba7ee7da7b9a857c

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

LOG: [lldb][NFCI] Remove n^2 loops and simplify iterator usage

The code inside Broadcaster makes usage of iterators using olden C++ coding
style. Hidden in this old style is a couple of N^2 loops: we iterate over a map
(sequentially), removing the first element that matches some predicate. The
search is _always_ done from the start of the map, which implies that, if the
map has N elements and if all matches happen on the second half of the map, then
we visit the first N/2 elements exactly N/2 * N/2 times.

Ideally some of the code here would benefit from `std::map`s own "erase_if", but
this is only available with C++20:
https://en.cppreference.com/w/cpp/container/map/erase_if

We spent quite some time trying to make these loops more elegant, but it is
surprisingly tricky to do so.

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

Added: 


Modified: 
lldb/source/Utility/Broadcaster.cpp

Removed: 




diff  --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index 66c78978571fa..58e393863b5a5 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -377,13 +377,10 @@ bool BroadcasterManager::UnregisterListenerForEvents(
 
   // Go through the map and delete the exact matches, and build a list of
   // matches that weren't exact to re-add:
-  while (true) {
-collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter,
-   listener_matches_and_shared_bits);
-if (iter == end_iter) {
+  for (auto iter = m_event_map.begin(), end = m_event_map.end();;) {
+iter = find_if(iter, end, listener_matches_and_shared_bits);
+if (iter == end)
   break;
-}
 uint32_t iter_event_bits = (*iter).first.GetEventBits();
 removed_some = true;
 
@@ -392,12 +389,12 @@ bool BroadcasterManager::UnregisterListenerForEvents(
   to_be_readded.emplace_back(event_spec.GetBroadcasterClass(),
  new_event_bits);
 }
-m_event_map.erase(iter);
+iter = m_event_map.erase(iter);
   }
 
   // Okay now add back the bits that weren't completely removed:
-  for (size_t i = 0; i < to_be_readded.size(); i++) {
-m_event_map.insert(event_listener_key(to_be_readded[i], listener_sp));
+  for (const auto &event : to_be_readded) {
+m_event_map.insert(event_listener_key(event, listener_sp));
   }
 
   return removed_some;
@@ -412,9 +409,8 @@ ListenerSP BroadcasterManager::GetListenerForEventSpec(
 return input.first.IsContainedIn(event_spec);
   };
 
-  collection::const_iterator iter, end_iter = m_event_map.end();
-  iter = find_if(m_event_map.begin(), end_iter, event_spec_matches);
-  if (iter != end_iter)
+  auto iter = llvm::find_if(m_event_map, event_spec_matches);
+  if (iter != m_event_map.end())
 return (*iter).second;
 
   return nullptr;
@@ -427,24 +423,21 @@ void BroadcasterManager::RemoveListener(Listener 
*listener) {
 return input.get() == listener;
   };
 
-  listener_collection::iterator iter = m_listeners.begin(),
-end_iter = m_listeners.end();
-
-  iter = std::find_if(iter, end_iter, listeners_predicate);
-  if (iter != end_iter)
+  if (auto iter = llvm::find_if(m_listeners, listeners_predicate);
+  iter != m_listeners.end())
 m_listeners.erase(iter);
 
-  while (true) {
-auto events_predicate =
-[&listener](const event_listener_key &input) -> bool {
-  return input.second.get() == listener;
-};
-collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter, events_predicate);
-if (iter == end_iter)
+  auto events_predicate = [listener](const event_listener_key &input) -> bool {
+return input.second.get() == listener;
+  };
+
+  // TODO: use 'std::map::erase_if' when moving to c++20.
+  for (auto iter = m_event_map.begin(), end = m_event_map.end();;) {
+iter = find_if(iter, end, events_predicate);
+if (iter == end)
   break;
 
-m_event_map.erase(iter);
+iter = m_event_map.erase(iter);
   }
 }
 
@@ -459,13 +452,13 @@ void BroadcasterManager::RemoveListener(const 
lldb::ListenerSP &listener_sp) {
   if (m_listeners.erase(listener_sp) == 0)
 return;
 
-  while (true) {
-collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter, listener_matches);
+  // TODO: use 'std::map::erase_if' when moving to c++20.
+  for (auto iter = m_event_map.begin(), end_iter = m_event_map.end();;) {
+iter = find_if(iter, end_iter, listener_ma

[Lldb-commits] [PATCH] D150219: [lldb][NFCI] Remove n^2 loops and simplify iterator usage

2023-05-10 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG25495c9b4c05: [lldb][NFCI] Remove n^2 loops and simplify 
iterator usage (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150219

Files:
  lldb/source/Utility/Broadcaster.cpp

Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -377,13 +377,10 @@
 
   // Go through the map and delete the exact matches, and build a list of
   // matches that weren't exact to re-add:
-  while (true) {
-collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter,
-   listener_matches_and_shared_bits);
-if (iter == end_iter) {
+  for (auto iter = m_event_map.begin(), end = m_event_map.end();;) {
+iter = find_if(iter, end, listener_matches_and_shared_bits);
+if (iter == end)
   break;
-}
 uint32_t iter_event_bits = (*iter).first.GetEventBits();
 removed_some = true;
 
@@ -392,12 +389,12 @@
   to_be_readded.emplace_back(event_spec.GetBroadcasterClass(),
  new_event_bits);
 }
-m_event_map.erase(iter);
+iter = m_event_map.erase(iter);
   }
 
   // Okay now add back the bits that weren't completely removed:
-  for (size_t i = 0; i < to_be_readded.size(); i++) {
-m_event_map.insert(event_listener_key(to_be_readded[i], listener_sp));
+  for (const auto &event : to_be_readded) {
+m_event_map.insert(event_listener_key(event, listener_sp));
   }
 
   return removed_some;
@@ -412,9 +409,8 @@
 return input.first.IsContainedIn(event_spec);
   };
 
-  collection::const_iterator iter, end_iter = m_event_map.end();
-  iter = find_if(m_event_map.begin(), end_iter, event_spec_matches);
-  if (iter != end_iter)
+  auto iter = llvm::find_if(m_event_map, event_spec_matches);
+  if (iter != m_event_map.end())
 return (*iter).second;
 
   return nullptr;
@@ -427,24 +423,21 @@
 return input.get() == listener;
   };
 
-  listener_collection::iterator iter = m_listeners.begin(),
-end_iter = m_listeners.end();
-
-  iter = std::find_if(iter, end_iter, listeners_predicate);
-  if (iter != end_iter)
+  if (auto iter = llvm::find_if(m_listeners, listeners_predicate);
+  iter != m_listeners.end())
 m_listeners.erase(iter);
 
-  while (true) {
-auto events_predicate =
-[&listener](const event_listener_key &input) -> bool {
-  return input.second.get() == listener;
-};
-collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter, events_predicate);
-if (iter == end_iter)
+  auto events_predicate = [listener](const event_listener_key &input) -> bool {
+return input.second.get() == listener;
+  };
+
+  // TODO: use 'std::map::erase_if' when moving to c++20.
+  for (auto iter = m_event_map.begin(), end = m_event_map.end();;) {
+iter = find_if(iter, end, events_predicate);
+if (iter == end)
   break;
 
-m_event_map.erase(iter);
+iter = m_event_map.erase(iter);
   }
 }
 
@@ -459,13 +452,13 @@
   if (m_listeners.erase(listener_sp) == 0)
 return;
 
-  while (true) {
-collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter, listener_matches);
+  // TODO: use 'std::map::erase_if' when moving to c++20.
+  for (auto iter = m_event_map.begin(), end_iter = m_event_map.end();;) {
+iter = find_if(iter, end_iter, listener_matches);
 if (iter == end_iter)
   break;
 
-m_event_map.erase(iter);
+iter = m_event_map.erase(iter);
   }
 }
 
@@ -490,11 +483,9 @@
 
 void BroadcasterManager::Clear() {
   std::lock_guard guard(m_manager_mutex);
-  listener_collection::iterator end_iter = m_listeners.end();
 
-  for (listener_collection::iterator iter = m_listeners.begin();
-   iter != end_iter; iter++)
-(*iter)->BroadcasterManagerWillDestruct(this->shared_from_this());
+  for (auto &listener : m_listeners)
+listener->BroadcasterManagerWillDestruct(this->shared_from_this());
   m_listeners.clear();
   m_event_map.clear();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

2023-05-10 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve accepted this revision.
fdeazeve added a comment.

LGTM! I like how the static_cast now is explicit about the fact that some 
truncation is going on from the ULEB reading.

Do you have plans to do something similar for the attribute typedef?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150228

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

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

2023-05-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

In D150228#4331943 , @fdeazeve wrote:

> Do you have plans to do something similar for the attribute typedef?

Yes I would like to do that too! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150228

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

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

2023-05-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord updated this revision to Diff 521047.
bulbazord added a comment.

Fix a warning in HashedNameToDIE


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150228

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -158,6 +158,7 @@
   atoms.push_back({type, form});
   atom_mask |= 1u << type;
   switch (form) {
+  default:
   case DW_FORM_indirect:
   case DW_FORM_exprloc:
   case DW_FORM_flag_present:
@@ -227,7 +228,7 @@
   } else {
 for (uint32_t i = 0; i < atom_count; ++i) {
   AtomType type = (AtomType)data.GetU16(&offset);
-  dw_form_t form = (dw_form_t)data.GetU16(&offset);
+  auto form = static_cast(data.GetU16(&offset));
   AppendAtom(type, form);
 }
   }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -45,7 +45,7 @@
   const DWARFUnit *GetUnit() const { return m_unit; }
   void SetUnit(const DWARFUnit *unit) { m_unit = unit; }
   dw_form_t Form() const { return m_form; }
-  dw_form_t& FormRef() { return m_form; }
+  dw_form_t &FormRef() { return m_form; }
   void SetForm(dw_form_t form) { m_form = form; }
   const ValueType &Value() const { return m_value; }
   ValueType &ValueRef() { return m_value; }
@@ -83,7 +83,7 @@
   // Compile unit where m_value was located.
   // It may be different from compile unit where m_value refers to.
   const DWARFUnit *m_unit = nullptr; // Unit for this form
-  dw_form_t m_form = 0;  // Form for this value
+  dw_form_t m_form = dw_form_t(0);   // Form for this value
   ValueType m_value;// Contains all data for the form
 };
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -25,7 +25,7 @@
 
 void DWARFFormValue::Clear() {
   m_unit = nullptr;
-  m_form = 0;
+  m_form = dw_form_t(0);
   m_value = ValueTypeTag();
 }
 
@@ -127,7 +127,7 @@
   m_value.value.uval = data.GetMaxU64(offset_ptr, ref_addr_size);
   break;
 case DW_FORM_indirect:
-  m_form = data.GetULEB128(offset_ptr);
+  m_form = static_cast(data.GetULEB128(offset_ptr));
   indirect = true;
   break;
 case DW_FORM_flag_present:
@@ -321,9 +321,10 @@
   return true;
 
   case DW_FORM_indirect: {
-dw_form_t indirect_form = debug_info_data.GetULEB128(offset_ptr);
-return DWARFFormValue::SkipValue(indirect_form, debug_info_data, offset_ptr,
- unit);
+  auto indirect_form =
+  static_cast(debug_info_data.GetULEB128(offset_ptr));
+  return DWARFFormValue::SkipValue(indirect_form, debug_info_data,
+   offset_ptr, unit);
   }
 
   default:
@@ -573,8 +574,10 @@
   case DW_FORM_block2:
   case DW_FORM_block4:
 return true;
+  default:
+return false;
   }
-  return false;
+  llvm_unreachable("All cases handled above!");
 }
 
 bool DWARFFormValue::IsDataForm(const dw_form_t form) {
@@ -586,8 +589,10 @@
   case DW_FORM_data4:
   case DW_FORM_data8:
 return true;
+  default:
+return false;
   }
-  return false;
+  llvm_unreachable("All cases handled above!");
 }
 
 bool DWARFFormValue::FormIsSupported(dw_form_t form) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -177,7 +177,7 @@
 
 case DW_FORM_indirect:
   form_is_indirect = true;
-  form = data.GetULEB128(&offset);
+  form = static_cast(data.GetULEB128(&offset));
   break;
 
 case DW_FORM_strp:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -55,7 +55,7 @@
   dw_a

[Lldb-commits] [lldb] 2ec334d - [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

2023-05-10 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-05-10T11:17:30-07:00
New Revision: 2ec334dc7b7329c6b71faa037a0d926e8e130c20

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

LOG: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

LLDB currently defines `dw_form_t` as a `uint16_t` which makes sense.
However, LLVM also defines a similar type `llvm::dwarf::Form` which is
an enum backed by a `uint16_t`. Switching to the llvm implementation
means that we can more easily interoperate with the LLVM DWARF code.

Additionally, we get some type checking out of this: I found that
DWARFAttribute had a method called `FormAtIndex` that returned a
`dw_attr_t`. Although `dw_attr_t` is also a `uint16_t` under the hood,
the type checking benefits here are undeniable: If this had returned a
something of different signedness/width, we could have had some bad
bugs.

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

Added: 


Modified: 
lldb/include/lldb/Core/dwarf.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/dwarf.h b/lldb/include/lldb/Core/dwarf.h
index e930200393c27..6fcf2f8cc0c78 100644
--- a/lldb/include/lldb/Core/dwarf.h
+++ b/lldb/include/lldb/Core/dwarf.h
@@ -22,7 +22,7 @@ namespace dwarf {
 }
 
 typedef uint16_t dw_attr_t;
-typedef uint16_t dw_form_t;
+typedef llvm::dwarf::Form dw_form_t;
 typedef llvm::dwarf::Tag dw_tag_t;
 typedef uint64_t dw_addr_t; // Dwarf address define that must be big enough for
 // any addresses in the compile units that get

diff  --git 
a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
index 62d75c69afa86..5bd3b23dc95fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -41,7 +41,7 @@ DWARFAbbreviationDeclaration::extract(const 
DWARFDataExtractor &data,
 
   while (data.ValidOffset(*offset_ptr)) {
 dw_attr_t attr = data.GetULEB128(offset_ptr);
-dw_form_t form = data.GetULEB128(offset_ptr);
+auto form = static_cast(data.GetULEB128(offset_ptr));
 
 // This is the last attribute for this abbrev decl, but there may still be
 // more abbrev decls, so return MoreItems to indicate to the caller that

diff  --git 
a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
index 7922a14b33f18..1c69894f82eca 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
@@ -28,7 +28,8 @@ class DWARFAbbreviationDeclaration {
   bool HasChildren() const { return m_has_children; }
   size_t NumAttributes() const { return m_attributes.size(); }
   dw_form_t GetFormByIndex(uint32_t idx) const {
-return m_attributes.size() > idx ? m_attributes[idx].get_form() : 0;
+return m_attributes.size() > idx ? m_attributes[idx].get_form()
+ : dw_form_t(0);
   }
 
   // idx is assumed to be valid when calling GetAttrAndFormByIndex()

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
index a31ee861179c2..faba5f5e9f6e1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -55,7 +55,7 @@ class DWARFAttributes {
   dw_attr_t AttributeAtIndex(uint32_t i) const {
 return m_infos[i].attr.get_attr();
   }
-  dw_attr_t FormAtIndex(uint32_t i) const { return m_infos[i].attr.get_form(); 
}
+  dw_form_t FormAtIndex(uint32_t i) const { return m_infos[i].attr.get_form(); 
}
   DWARFFormValue::ValueType ValueAtIndex(uint32_t i) const {
 return m_infos[i].attr.get_value();
   }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index bd2bb3d839c6a..1db71c0eccdf5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -177,7 +177,7 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
 
 case DW_FORM_indirect:
   form_is_indirect = true;

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

2023-05-10 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2ec334dc7b73: [lldb][NFCI] Replace dw_form_t with 
llvm::dwarf::Form (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150228

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -158,6 +158,7 @@
   atoms.push_back({type, form});
   atom_mask |= 1u << type;
   switch (form) {
+  default:
   case DW_FORM_indirect:
   case DW_FORM_exprloc:
   case DW_FORM_flag_present:
@@ -227,7 +228,7 @@
   } else {
 for (uint32_t i = 0; i < atom_count; ++i) {
   AtomType type = (AtomType)data.GetU16(&offset);
-  dw_form_t form = (dw_form_t)data.GetU16(&offset);
+  auto form = static_cast(data.GetU16(&offset));
   AppendAtom(type, form);
 }
   }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -45,7 +45,7 @@
   const DWARFUnit *GetUnit() const { return m_unit; }
   void SetUnit(const DWARFUnit *unit) { m_unit = unit; }
   dw_form_t Form() const { return m_form; }
-  dw_form_t& FormRef() { return m_form; }
+  dw_form_t &FormRef() { return m_form; }
   void SetForm(dw_form_t form) { m_form = form; }
   const ValueType &Value() const { return m_value; }
   ValueType &ValueRef() { return m_value; }
@@ -83,7 +83,7 @@
   // Compile unit where m_value was located.
   // It may be different from compile unit where m_value refers to.
   const DWARFUnit *m_unit = nullptr; // Unit for this form
-  dw_form_t m_form = 0;  // Form for this value
+  dw_form_t m_form = dw_form_t(0);   // Form for this value
   ValueType m_value;// Contains all data for the form
 };
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -25,7 +25,7 @@
 
 void DWARFFormValue::Clear() {
   m_unit = nullptr;
-  m_form = 0;
+  m_form = dw_form_t(0);
   m_value = ValueTypeTag();
 }
 
@@ -127,7 +127,7 @@
   m_value.value.uval = data.GetMaxU64(offset_ptr, ref_addr_size);
   break;
 case DW_FORM_indirect:
-  m_form = data.GetULEB128(offset_ptr);
+  m_form = static_cast(data.GetULEB128(offset_ptr));
   indirect = true;
   break;
 case DW_FORM_flag_present:
@@ -321,9 +321,10 @@
   return true;
 
   case DW_FORM_indirect: {
-dw_form_t indirect_form = debug_info_data.GetULEB128(offset_ptr);
-return DWARFFormValue::SkipValue(indirect_form, debug_info_data, offset_ptr,
- unit);
+  auto indirect_form =
+  static_cast(debug_info_data.GetULEB128(offset_ptr));
+  return DWARFFormValue::SkipValue(indirect_form, debug_info_data,
+   offset_ptr, unit);
   }
 
   default:
@@ -573,8 +574,10 @@
   case DW_FORM_block2:
   case DW_FORM_block4:
 return true;
+  default:
+return false;
   }
-  return false;
+  llvm_unreachable("All cases handled above!");
 }
 
 bool DWARFFormValue::IsDataForm(const dw_form_t form) {
@@ -586,8 +589,10 @@
   case DW_FORM_data4:
   case DW_FORM_data8:
 return true;
+  default:
+return false;
   }
-  return false;
+  llvm_unreachable("All cases handled above!");
 }
 
 bool DWARFFormValue::FormIsSupported(dw_form_t form) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -177,7 +177,7 @@
 
 case DW_FORM_indirect:
   form_is_indirect = true;
-  form = data.GetULEB128(&offset);
+  form = static_cast(data.GetULEB128(&offset));
   break;
 
 case DW_FORM_strp:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ ll

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

2023-05-10 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

I don't understand why the succeeding return value for `GetFramesUpTo` is 
`false`. It looks counter-intuitive to me. What's the motivation behind that ?




Comment at: lldb/include/lldb/Target/StackFrameList.h:103-104
 
-  void GetFramesUpTo(uint32_t end_idx);
+  /// Gets frames up to end_idx (which can be greater than the actual number of
+  /// frames.)  Returns true if the function was interrupted, false otherwise.
+  bool GetFramesUpTo(uint32_t end_idx, bool allow_interrupt = true);

bulbazord wrote:
> Is the range inclusive of `end_idx`? as in, is it [0, end_idx) or [0, 
> end_idx]?
@bulbazord I believe this should be inclusive.

@jingham This comment sounds like we only return `true` if the function was 
interrupted, which is not the expected behavior, right ?



Comment at: lldb/source/Target/StackFrameList.cpp:448
   if (m_frames.size() > end_idx || GetAllFramesFetched())
-return;
+return false;
 

I find it confusing the fail here given that we've succeeded at fetching the 
frames



Comment at: lldb/source/Target/StackFrameList.cpp:633
+return was_interrupted;
+  return false;
 }

Again, I believe this should return `true` since we succeeded at retrieving all 
the requested frames.



Comment at: lldb/source/Target/StackFrameList.cpp:683-684
   // there are that many.  If there weren't then you asked for too many frames.
-  GetFramesUpTo(idx);
+  bool interrupted = GetFramesUpTo(idx);
+  if (interrupted) {
+Log *log = GetLog(LLDBLog::Thread);

bulbazord wrote:
> nit: Since `interrupted` isn't used anywhere else, you could do something 
> like:
> ```
> if (bool interrupted = GetFramesUpTo(idx)) {
>  // Code here
> }
> ```
> You could also just do `if (GetFramesUpTo(idx))` but I think the name of the 
> function isn't descriptive enough to do that and stay readable.
+1: reading `if (GetFramesUpTo(idx))`, I'd never think that the succeeding 
result would be `false`.



Comment at: 
lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py:46
+
+self.dbg.CancelInterruptRequest()
+

Is this necessary if you already have it in the `cleanup` method ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150236

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

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for `__bf16` to `BF16Ty`

2023-05-10 Thread M. Zeeshan Siddiqui via Phabricator via lldb-commits

codemzs created this revision.
codemzs added reviewers: tahonermann, erichkeane, stuij.
Herald added subscribers: mattd, gchakrabarti, asavonic, ctetreau, kerbowa, 
arphaman, kristof.beyls, jvesely.
Herald added a project: All.
codemzs requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, jholewinski.
Herald added projects: clang, LLDB.

  This change updates internal type identifiers for `__bf16` from
  `BFloat16Ty` to `BF16Ty` and renames any associated variables and
  function names accordingly. The rationale for this change comes from
  the review feedback on https://reviews.llvm.org/D149573, which pointed
  out the confusing naming issues between `__bf16` and the upcoming
  `std::bfloat16_t`. This modification only affects LLVM/Clang specific
  code and does not interfere with target-specific code, such as
  NeonEmitters, SVE Type, AArch64, etc. The existing names are being
  updated to avoid potential mistakes and enhance clarity in the codebase.
  The change is made in a separate patch, as suggested in the review, to
  ensure a smooth integration of std::bfloat16_t support.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150291

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/arm_neon_incl.td
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/tools/libclang/CXType.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4909,7 +4909,7 @@
 case clang::BuiltinType::Float128:
 case clang::BuiltinType::Double:
 case clang::BuiltinType::LongDouble:
-case clang::BuiltinType::BFloat16:
+case clang::BuiltinType::BF16:
 case clang::BuiltinType::Ibm128:
   return lldb::eEncodingIEEE754;
 
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -618,7 +618,7 @@
 TKIND(Pipe);
 TKIND(Attributed);
 TKIND(BTFTagAttributed);
-TKIND(BFloat16);
+TKIND(BF16);
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) TKIND(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -7026,8 +7026,8 @@
 case PREDEF_TYPE_INT128_ID:
   T = Context.Int128Ty;
   break;
-case PREDEF_TYPE_BFLOAT16_ID:
-  T = Context.BFloat16Ty;
+case PREDEF_TYPE_BF16_ID:
+  T = Context.BF16Ty;
   break;
 case PREDEF_TYPE_HALF_ID:
   T = Context.HalfTy;
Index: clang/lib/Serialization/ASTCommon.cpp
===
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -270,8 +270,8 @@
   case BuiltinType::OMPIterator:
 ID = PREDEF_TYPE_OMP_ITERATOR;
 break;
-  case BuiltinType::BFloat16:
-ID = PREDEF_TYPE_BFLOAT16_ID;
+  case BuiltinType::BF16:
+ID = PREDEF_TYPE_BF16_ID;
 break;
   }
 
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaTy

[Lldb-commits] [PATCH] D150299: [lldb][NFCI] Redefine dw_attr_t typedef with llvm::dwarf::Attribute

2023-05-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord created this revision.
bulbazord added reviewers: aprantl, jingham, clayborg, rastogishubham, fdeazeve.
Herald added a subscriber: arphaman.
Herald added a reviewer: shafik.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Similar to dw_form_t, dw_attr_t is typedef'd to be a uint16_t. LLVM
defines their type `llvm::dwarf::Attribute` as an enum backed by a
uint16_t. Switching to the LLVM type buys us type checking and the
requirement of explicit casts.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150299

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -239,6 +239,8 @@
 dw_attr_t attr = attributes.AttributeAtIndex(i);
 DWARFFormValue form_value;
 switch (attr) {
+default:
+  break;
 case DW_AT_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
 name = form_value.AsCString();
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -391,6 +391,8 @@
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_loclists_base:
   SetLoclistsBase(form_value.Unsigned());
   break;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -40,7 +40,7 @@
   m_has_children = data.GetU8(offset_ptr);
 
   while (data.ValidOffset(*offset_ptr)) {
-dw_attr_t attr = data.GetULEB128(offset_ptr);
+auto attr = static_cast(data.GetULEB128(offset_ptr));
 auto form = static_cast(data.GetULEB128(offset_ptr));
 
 // This is the last attribute for this abbrev decl, but there may still be
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -274,6 +274,8 @@
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_abstract_origin:
   abstract_origin = form_value;
   break;
Index: lldb/include/lldb/Core/dwarf.h
===
--- lldb/include/lldb/Core/dwarf.h
+++ lldb/include/lldb/Core/dwarf.h
@@ -21,7 +21,7 @@
 }
 }
 
-typedef uint16_t dw_attr_t;
+typedef llvm::dwarf::Attribute dw_attr_t;
 typedef llvm::dwarf::Form dw_form_t;
 typedef llvm::dwarf::Tag dw_tag_t;
 typedef uint64_t dw_addr_t; // Dwarf address define that must be big enough for


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -239,6 +239,8 @@
 dw_attr_t attr = attributes.AttributeAtIndex(i);
 DWARFFormValue form_value;
 switch (attr) {
+default:
+  break;
 case DW_AT_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
 name = form_value.AsCString();
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -391,6 +391,8 @@
 if (!attributes.ExtractFormValueAtIndex(i, form_value))
   continue;
 switch (attr) {
+default:
+  break;
 case DW_AT_loclists_base:
   SetLoclistsBase(form_value.Unsigned());
   break;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -40,7 +40,7 @@
   m_has_children = data.GetU8(offset_ptr);
 
   while (data.ValidOffset(*offset_ptr)) {
-d

[Lldb-commits] [PATCH] D150303: [LLDB] Add some declarations related to REPL support for mojo

2023-05-10 Thread walter erquinigo via Phabricator via lldb-commits

wallace created this revision.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This simple diff declares some enum values needed to create a REPL for the mojo 
language.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150303

Files:
  lldb/include/lldb/Expression/ExpressionTypeSystemHelper.h
  lldb/include/lldb/Expression/ExpressionVariable.h
  lldb/include/lldb/Expression/REPL.h


Index: lldb/include/lldb/Expression/REPL.h
===
--- lldb/include/lldb/Expression/REPL.h
+++ lldb/include/lldb/Expression/REPL.h
@@ -21,7 +21,7 @@
 class REPL : public IOHandlerDelegate {
 public:
   // See TypeSystem.h for how to add subclasses to this.
-  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, kNumKinds };
+  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, eKindMojo, kNumKinds };
 
   LLVMCastKind getKind() const { return m_kind; }
 
Index: lldb/include/lldb/Expression/ExpressionVariable.h
===
--- lldb/include/lldb/Expression/ExpressionVariable.h
+++ lldb/include/lldb/Expression/ExpressionVariable.h
@@ -25,7 +25,7 @@
 : public std::enable_shared_from_this {
 public:
   // See TypeSystem.h for how to add subclasses to this.
-  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, kNumKinds };
+  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, eKindMojo, kNumKinds };
 
   LLVMCastKind getKind() const { return m_kind; }
 
@@ -203,7 +203,7 @@
 class PersistentExpressionState : public ExpressionVariableList {
 public:
   // See TypeSystem.h for how to add subclasses to this.
-  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, kNumKinds };
+  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, eKindMojo, kNumKinds };
 
   LLVMCastKind getKind() const { return m_kind; }
 
Index: lldb/include/lldb/Expression/ExpressionTypeSystemHelper.h
===
--- lldb/include/lldb/Expression/ExpressionTypeSystemHelper.h
+++ lldb/include/lldb/Expression/ExpressionTypeSystemHelper.h
@@ -29,6 +29,7 @@
 eKindClangHelper,
 eKindSwiftHelper,
 eKindGoHelper,
+eKindMojoHelper,
 kNumKinds
   };
 


Index: lldb/include/lldb/Expression/REPL.h
===
--- lldb/include/lldb/Expression/REPL.h
+++ lldb/include/lldb/Expression/REPL.h
@@ -21,7 +21,7 @@
 class REPL : public IOHandlerDelegate {
 public:
   // See TypeSystem.h for how to add subclasses to this.
-  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, kNumKinds };
+  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, eKindMojo, kNumKinds };
 
   LLVMCastKind getKind() const { return m_kind; }
 
Index: lldb/include/lldb/Expression/ExpressionVariable.h
===
--- lldb/include/lldb/Expression/ExpressionVariable.h
+++ lldb/include/lldb/Expression/ExpressionVariable.h
@@ -25,7 +25,7 @@
 : public std::enable_shared_from_this {
 public:
   // See TypeSystem.h for how to add subclasses to this.
-  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, kNumKinds };
+  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, eKindMojo, kNumKinds };
 
   LLVMCastKind getKind() const { return m_kind; }
 
@@ -203,7 +203,7 @@
 class PersistentExpressionState : public ExpressionVariableList {
 public:
   // See TypeSystem.h for how to add subclasses to this.
-  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, kNumKinds };
+  enum LLVMCastKind { eKindClang, eKindSwift, eKindGo, eKindMojo, kNumKinds };
 
   LLVMCastKind getKind() const { return m_kind; }
 
Index: lldb/include/lldb/Expression/ExpressionTypeSystemHelper.h
===
--- lldb/include/lldb/Expression/ExpressionTypeSystemHelper.h
+++ lldb/include/lldb/Expression/ExpressionTypeSystemHelper.h
@@ -29,6 +29,7 @@
 eKindClangHelper,
 eKindSwiftHelper,
 eKindGoHelper,
+eKindMojoHelper,
 kNumKinds
   };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150303: [LLDB] Add some declarations related to REPL support for mojo

2023-05-10 Thread River Riddle via Phabricator via lldb-commits

rriddle added a comment.

Could we switch the RTTI to use the llvm RTTI extension mechanism, instead of 
enums? Other classes have started doing this, and it'd be really nice if users 
can write REPLS without needing to touch upstream LLDB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150303

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

[Lldb-commits] [PATCH] D150303: [LLDB] Add some declarations related to REPL support for mojo

2023-05-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

In D150303#4333215 , @rriddle wrote:

> Could we switch the RTTI to use the llvm RTTI extension mechanism, instead of 
> enums? Other classes have started doing this, and it'd be really nice if 
> users can write REPLS without needing to touch upstream LLDB.

+1, LLVM's RTTI mechanism would make it easier to add new things without 
needing to change base classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150303

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

[Lldb-commits] [PATCH] D150313: Fix libstdc++ data formatter for reference/pointer to std::string

2023-05-10 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere, 
kusmour, GeorgeHuyubo.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch fixes libstdc++ data formatter for reference/pointer to std::string.
The failure testcases are added which succeed with the patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150313

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -10,6 +10,8 @@
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
 std::basic_string uchar(5, 'a');
+auto &rq = q, &rQ = Q;
+std::string *pq = &q, *pQ = &Q;
 S.assign(L"!"); // Set break point at this line.
 return 0;
 }
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -57,6 +57,11 @@
 var_empty = self.frame().FindVariable('empty')
 var_q = self.frame().FindVariable('q')
 var_Q = self.frame().FindVariable('Q')
+var_rq = self.frame().FindVariable('rq')
+var_rQ = self.frame().FindVariable('rQ')
+var_pq = self.frame().FindVariable('pq')
+var_pQ = self.frame().FindVariable('pQ')
+
 var_uchar = self.frame().FindVariable('uchar')
 
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary 
wrong")
@@ -75,6 +80,18 @@
 var_Q.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
 "Q summary wrong")
 self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
+self.assertEqual(
+var_rq.GetSummary(), '"hello world"',
+"rq summary wrong")
+self.assertEqual(
+var_rQ.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
+"rQ summary wrong")
+self.assertEqual(
+var_pq.GetSummary(), '"hello world"',
+"pq summary wrong")
+self.assertEqual(
+var_pQ.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
+"pQ summary wrong")
 
 self.runCmd("next")
 
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -233,8 +233,15 @@
 ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   const bool scalar_is_load_addr = true;
   AddressType addr_type;
-  lldb::addr_t addr_of_string =
-  valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
+  lldb::addr_t addr_of_string = LLDB_INVALID_ADDRESS;
+  if (valobj.IsPointerOrReferenceType()) {
+Status error;
+ValueObjectSP pointee_sp = valobj.Dereference(error);
+if (pointee_sp && error.Success())
+  addr_of_string = pointee_sp->GetAddressOf(scalar_is_load_addr, 
&addr_type);
+  } else
+addr_of_string =
+valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
   if (addr_of_string != LLDB_INVALID_ADDRESS) {
 switch (addr_type) {
 case eAddressTypeLoad: {


Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -10,6 +10,8 @@
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
 std::basic_string uchar(5, 'a');
+auto &rq = q, &rQ = Q;
+std::string *pq = &q, *pQ = &Q;
 S.assign(L"!"); // Set break point at this line.
 return 0;
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py

[Lldb-commits] [PATCH] D150239: ObjectFileMachO: Prioritize the TEXT segment as the mach header segment, regardless of the order the segments appear in the file

2023-05-10 Thread Jim Ingham via Phabricator via lldb-commits

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

LGTM for support of something that really should hurt a little more than you 
are making it...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150239

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

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

2023-05-10 Thread Jim Ingham via Phabricator via lldb-commits

jingham created this revision.
jingham added reviewers: mib, JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

  When the Debugger runs HandleProcessEvent it should allow
  selecting the "Most relevant" frame.
  
  If you don't do that, then the correct frame gets selected, but it
  happens AFTER the frame info gets printed in the stop message, so
  you don't see the selected frame.
  
  The test for this hid the issue because it ran `frame info` and
  checked the result of that.  That happens after the recognizer selects
  the frame, and so it was right.  But if the recognizer is working
  correctly it will have already done the same printing in the stop
  message, and this way we also verify that the stop message was right.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150315

Files:
  lldb/source/Core/Debugger.cpp
  lldb/test/Shell/Recognizer/assert.test


Index: lldb/test/Shell/Recognizer/assert.test
===
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -9,7 +9,6 @@
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
 # CHECK: thread #{{.*}}stop reason = hit program assert
-frame info
 # CHECK: frame #{{.*}}`main at assert.c
 frame recognizer info 0
 # CHECK: frame 0 is recognized by Assert StackFrame Recognizer
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1699,8 +1699,10 @@
 
 // Display running state changes first before any STDIO
 if (got_state_changed && !state_is_stopped) {
+  // This is a public stop which we are going to announce to the user, so 
+  // we should force the most relevant frame selection here.
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 
@@ -1740,7 +1742,7 @@
 // Now display any stopped state changes after any STDIO
 if (got_state_changed && state_is_stopped) {
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 


Index: lldb/test/Shell/Recognizer/assert.test
===
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -9,7 +9,6 @@
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
 # CHECK: thread #{{.*}}stop reason = hit program assert
-frame info
 # CHECK: frame #{{.*}}`main at assert.c
 frame recognizer info 0
 # CHECK: frame 0 is recognized by Assert StackFrame Recognizer
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1699,8 +1699,10 @@
 
 // Display running state changes first before any STDIO
 if (got_state_changed && !state_is_stopped) {
+  // This is a public stop which we are going to announce to the user, so 
+  // we should force the most relevant frame selection here.
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 
@@ -1740,7 +1742,7 @@
 // Now display any stopped state changes after any STDIO
 if (got_state_changed && state_is_stopped) {
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

2023-05-10 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150315

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

[Lldb-commits] [lldb] 7b5dc63 - When the Debugger runs HandleProcessEvent it should allow

2023-05-10 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2023-05-10T15:40:40-07:00
New Revision: 7b5dc63fc4464e777e4210a68120b36cb283a9fd

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

LOG: When the Debugger runs HandleProcessEvent it should allow
selecting the "Most relevant" frame.

If you don't do that, then the correct frame gets selected, but it
happens AFTER the frame info gets printed in the stop message, so
you don't see the selected frame.

The test for this hid the issue because it ran `frame info` and
checked the result of that.  That happens after the recognizer selects
the frame, and so it was right.  But if the recognizer is working
correctly it will have already done the same printing in the stop
message, and this way we also verify that the stop message was right.

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

Added: 


Modified: 
lldb/source/Core/Debugger.cpp
lldb/test/Shell/Recognizer/assert.test

Removed: 




diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index fbfee28183dd0..e39879953820f 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1699,8 +1699,10 @@ void Debugger::HandleProcessEvent(const EventSP 
&event_sp) {
 
 // Display running state changes first before any STDIO
 if (got_state_changed && !state_is_stopped) {
+  // This is a public stop which we are going to announce to the user, so 
+  // we should force the most relevant frame selection here.
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 
@@ -1740,7 +1742,7 @@ void Debugger::HandleProcessEvent(const EventSP 
&event_sp) {
 // Now display any stopped state changes after any STDIO
 if (got_state_changed && state_is_stopped) {
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 

diff  --git a/lldb/test/Shell/Recognizer/assert.test 
b/lldb/test/Shell/Recognizer/assert.test
index 3e302ee403382..201e834cf357e 100644
--- a/lldb/test/Shell/Recognizer/assert.test
+++ b/lldb/test/Shell/Recognizer/assert.test
@@ -9,7 +9,6 @@
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
 # CHECK: thread #{{.*}}stop reason = hit program assert
-frame info
 # CHECK: frame #{{.*}}`main at assert.c
 frame recognizer info 0
 # CHECK: frame 0 is recognized by Assert StackFrame Recognizer



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

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

2023-05-10 Thread Jim Ingham via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b5dc63fc446: When the Debugger runs HandleProcessEvent it 
should allow (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150315

Files:
  lldb/source/Core/Debugger.cpp
  lldb/test/Shell/Recognizer/assert.test


Index: lldb/test/Shell/Recognizer/assert.test
===
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -9,7 +9,6 @@
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
 # CHECK: thread #{{.*}}stop reason = hit program assert
-frame info
 # CHECK: frame #{{.*}}`main at assert.c
 frame recognizer info 0
 # CHECK: frame 0 is recognized by Assert StackFrame Recognizer
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1699,8 +1699,10 @@
 
 // Display running state changes first before any STDIO
 if (got_state_changed && !state_is_stopped) {
+  // This is a public stop which we are going to announce to the user, so 
+  // we should force the most relevant frame selection here.
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 
@@ -1740,7 +1742,7 @@
 // Now display any stopped state changes after any STDIO
 if (got_state_changed && state_is_stopped) {
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 


Index: lldb/test/Shell/Recognizer/assert.test
===
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -9,7 +9,6 @@
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
 # CHECK: thread #{{.*}}stop reason = hit program assert
-frame info
 # CHECK: frame #{{.*}}`main at assert.c
 frame recognizer info 0
 # CHECK: frame 0 is recognized by Assert StackFrame Recognizer
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1699,8 +1699,10 @@
 
 // Display running state changes first before any STDIO
 if (got_state_changed && !state_is_stopped) {
+  // This is a public stop which we are going to announce to the user, so 
+  // we should force the most relevant frame selection here.
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 
@@ -1740,7 +1742,7 @@
 // Now display any stopped state changes after any STDIO
 if (got_state_changed && state_is_stopped) {
   Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-  DoNoSelectMostRelevantFrame,
+  SelectMostRelevantFrame,
   pop_process_io_handler);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for `__bf16` to `BF16Ty`

2023-05-10 Thread Tom Honermann via Phabricator via lldb-commits

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

This looks great, thank you for doing this!

Requested changes are just to undo some of the style changes.




Comment at: clang/include/clang/Basic/Specifiers.h:82-89
+TST_class, // C++ class type
+TST_interface, // C++ (Microsoft-specific) __interface type
+TST_typename,  // Typedef, C++ class-name or enum name, etc.
 TST_typeofType,// C2x (and GNU extension) typeof(type-name)
 TST_typeofExpr,// C2x (and GNU extension) typeof(expression)
 TST_typeof_unqualType, // C2x typeof_unqual(type-name)
 TST_typeof_unqualExpr, // C2x typeof_unqual(expression)

Whitespace changes to code that is otherwise not being changed are discouraged 
due to the impact on tools like `git blame`. If you want to make such a change, 
please do so as a separate commit and add the commit to 
`.git-blame-ignore-revs`.



Comment at: clang/lib/AST/ASTContext.cpp:7075-7076
   case BuiltinType::Float128:   return Float128Rank;
-  case BuiltinType::BFloat16:   return BFloat16Rank;
+  case BuiltinType::BF16:
+return BF16Rank;
   case BuiltinType::Ibm128: return Ibm128Rank;

Please keep the same style here (I know `clang-format` might want to do 
differently; just ignore it in this case).



Comment at: clang/lib/AST/ItaniumMangle.cpp:3617-3619
+case BuiltinType::BF16:
+  EltName = "bfloat16_t";
+  break;

Likewise, please maintain consistent style here.

Thank you for *not* changing the literal! :)



Comment at: clang/lib/Sema/DeclSpec.cpp:590-591
   case DeclSpec::TST_atomic: return "_Atomic";
-  case DeclSpec::TST_BFloat16: return "__bf16";
+  case DeclSpec::TST_BF16:
+return "__bf16";
 #define GENERIC_IMAGE_TYPE(ImgType, Id) \

Please maintain consistent style here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150291

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

[Lldb-commits] [lldb] f9759d0 - Prioritize using a segment with the name TEXT instead off fileoff 0

2023-05-10 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2023-05-10T16:42:38-07:00
New Revision: f9759d0cb6dc19aa67dbe1c3d56404041f8b4c7e

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

LOG: Prioritize using a segment with the name TEXT instead off fileoff 0

lldb needs to find the virtual address of the mach header of a
binary.  It first scans for a segment which starts at file offset
0, and uses the vmaddr of that segment.  If no segment starts at
fileoff 0, it looks for a segment named __TEXT.

This patch changes the order of those, to first search for the TEXT
segment.  We have a situation where binaries exist that have the
DATA segment first, which does not have the vmaddr of the mach header,
it merely happens to come first in the binary file.  It's an unusual
arrangement, but not breaking any rules of Mach-O.  So lldb needs
to handle this.

Differential Revision: https://reviews.llvm.org/D150239
rdar://109128418

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index e941850d29b78..150dde8304cb8 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6059,6 +6059,16 @@ Section *ObjectFileMachO::GetMachHeaderSection() {
   SectionList *section_list = GetSectionList();
   if (!section_list)
 return nullptr;
+
+  // Some binaries can have a TEXT segment with a non-zero file offset.
+  // Binaries in the shared cache are one example.  Some hand-generated
+  // binaries may not be laid out in the normal TEXT,DATA,LC_SYMTAB order
+  // in the file, even though they're laid out correctly in vmaddr terms.
+  SectionSP text_segment_sp =
+  section_list->FindSectionByName(GetSegmentNameTEXT());
+  if (text_segment_sp.get() && SectionIsLoadable(text_segment_sp.get()))
+return text_segment_sp.get();
+
   const size_t num_sections = section_list->GetSize();
   for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx) {
 Section *section = section_list->GetSectionAtIndex(sect_idx).get();
@@ -6066,14 +6076,6 @@ Section *ObjectFileMachO::GetMachHeaderSection() {
   return section;
   }
 
-  // We may have a binary in the shared cache that has a non-zero
-  // file address for its first segment, traditionally the __TEXT segment.
-  // Search for it by name and return it as our next best guess.
-  SectionSP text_segment_sp =
-  GetSectionList()->FindSectionByName(GetSegmentNameTEXT());
-  if (text_segment_sp.get() && SectionIsLoadable(text_segment_sp.get()))
-return text_segment_sp.get();
-
   return nullptr;
 }
 



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

[Lldb-commits] [PATCH] D150239: ObjectFileMachO: Prioritize the TEXT segment as the mach header segment, regardless of the order the segments appear in the file

2023-05-10 Thread Jason Molenda via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9759d0cb6dc: Prioritize using a segment with the name TEXT 
instead off fileoff 0 (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150239

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp


Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6059,6 +6059,16 @@
   SectionList *section_list = GetSectionList();
   if (!section_list)
 return nullptr;
+
+  // Some binaries can have a TEXT segment with a non-zero file offset.
+  // Binaries in the shared cache are one example.  Some hand-generated
+  // binaries may not be laid out in the normal TEXT,DATA,LC_SYMTAB order
+  // in the file, even though they're laid out correctly in vmaddr terms.
+  SectionSP text_segment_sp =
+  section_list->FindSectionByName(GetSegmentNameTEXT());
+  if (text_segment_sp.get() && SectionIsLoadable(text_segment_sp.get()))
+return text_segment_sp.get();
+
   const size_t num_sections = section_list->GetSize();
   for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx) {
 Section *section = section_list->GetSectionAtIndex(sect_idx).get();
@@ -6066,14 +6076,6 @@
   return section;
   }
 
-  // We may have a binary in the shared cache that has a non-zero
-  // file address for its first segment, traditionally the __TEXT segment.
-  // Search for it by name and return it as our next best guess.
-  SectionSP text_segment_sp =
-  GetSectionList()->FindSectionByName(GetSegmentNameTEXT());
-  if (text_segment_sp.get() && SectionIsLoadable(text_segment_sp.get()))
-return text_segment_sp.get();
-
   return nullptr;
 }
 


Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6059,6 +6059,16 @@
   SectionList *section_list = GetSectionList();
   if (!section_list)
 return nullptr;
+
+  // Some binaries can have a TEXT segment with a non-zero file offset.
+  // Binaries in the shared cache are one example.  Some hand-generated
+  // binaries may not be laid out in the normal TEXT,DATA,LC_SYMTAB order
+  // in the file, even though they're laid out correctly in vmaddr terms.
+  SectionSP text_segment_sp =
+  section_list->FindSectionByName(GetSegmentNameTEXT());
+  if (text_segment_sp.get() && SectionIsLoadable(text_segment_sp.get()))
+return text_segment_sp.get();
+
   const size_t num_sections = section_list->GetSize();
   for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx) {
 Section *section = section_list->GetSectionAtIndex(sect_idx).get();
@@ -6066,14 +6076,6 @@
   return section;
   }
 
-  // We may have a binary in the shared cache that has a non-zero
-  // file address for its first segment, traditionally the __TEXT segment.
-  // Search for it by name and return it as our next best guess.
-  SectionSP text_segment_sp =
-  GetSectionList()->FindSectionByName(GetSegmentNameTEXT());
-  if (text_segment_sp.get() && SectionIsLoadable(text_segment_sp.get()))
-return text_segment_sp.get();
-
   return nullptr;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

2023-05-10 Thread M. Zeeshan Siddiqui via Phabricator via lldb-commits

codemzs updated this revision to Diff 521163.
codemzs marked 4 inline comments as done.
codemzs retitled this revision from "[Clang] Rename internal type identifier(s) 
for `__bf16` to `BF16Ty`" to "[Clang] Rename internal type identifier(s) for 
__bf16 to BF16Ty".
codemzs added a comment.

PR feedback: Revert style changes.


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

https://reviews.llvm.org/D150291

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/arm_neon_incl.td
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/tools/libclang/CXType.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4909,7 +4909,7 @@
 case clang::BuiltinType::Float128:
 case clang::BuiltinType::Double:
 case clang::BuiltinType::LongDouble:
-case clang::BuiltinType::BFloat16:
+case clang::BuiltinType::BF16:
 case clang::BuiltinType::Ibm128:
   return lldb::eEncodingIEEE754;
 
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -618,7 +618,7 @@
 TKIND(Pipe);
 TKIND(Attributed);
 TKIND(BTFTagAttributed);
-TKIND(BFloat16);
+TKIND(BF16);
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) TKIND(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -7024,8 +7024,8 @@
 case PREDEF_TYPE_INT128_ID:
   T = Context.Int128Ty;
   break;
-case PREDEF_TYPE_BFLOAT16_ID:
-  T = Context.BFloat16Ty;
+case PREDEF_TYPE_BF16_ID:
+  T = Context.BF16Ty;
   break;
 case PREDEF_TYPE_HALF_ID:
   T = Context.HalfTy;
Index: clang/lib/Serialization/ASTCommon.cpp
===
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -270,8 +270,8 @@
   case BuiltinType::OMPIterator:
 ID = PREDEF_TYPE_OMP_ITERATOR;
 break;
-  case BuiltinType::BFloat16:
-ID = PREDEF_TYPE_BFLOAT16_ID;
+  case BuiltinType::BF16:
+ID = PREDEF_TYPE_BF16_ID;
 break;
   }
 
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,12 +1518,12 @@
 Result = Context.Float16Ty;
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
-  case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type() &&
+  case DeclSpec::TST_BF16:
+if (!S.Context.getTargetInfo().hasBF16Type() &&
 !(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice) &&
 !S.getLangOpts().SYCLIsDevice)
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << "__bf16";
-Result = Context.BFloat16Ty;
+Result = Context.BF16Ty;
 break;
   case DeclSpec::TST_float:   Result = Context.FloatTy; break;
   case DeclSpec::TST_double:
@@ -8133,7 +8133,7 @@
  BTy->getKind() == BuiltinType::ULongLong |

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

2023-05-10 Thread M. Zeeshan Siddiqui via Phabricator via lldb-commits

codemzs added a comment.

Thank you for pointing that out and for reviewing my code. I appreciate your 
guidance. I was following the LLVM contribution guidelines to use git 
clang-format, but I understand the importance of maintaining existing code 
styles that may be altered by git-clang format. I will be more mindful of this 
in the future. I have reverted the style changes and updated the patch as per 
your feedback.


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

https://reviews.llvm.org/D150291

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

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

2023-05-10 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



Comment at: lldb/include/lldb/Target/StackFrameList.h:103-104
 
-  void GetFramesUpTo(uint32_t end_idx);
+  /// Gets frames up to end_idx (which can be greater than the actual number of
+  /// frames.)  Returns true if the function was interrupted, false otherwise.
+  bool GetFramesUpTo(uint32_t end_idx, bool allow_interrupt = true);

mib wrote:
> bulbazord wrote:
> > Is the range inclusive of `end_idx`? as in, is it [0, end_idx) or [0, 
> > end_idx]?
> @bulbazord I believe this should be inclusive.
> 
> @jingham This comment sounds like we only return `true` if the function was 
> interrupted, which is not the expected behavior, right ?
The function used to return void, so it had no expected behavior before.  I am 
defining the "bool" return here, and I define it to be "was_interrupted" as I 
say in the doc string.  That's also why the return values you are questioning 
below are actually correct.  This is NOT returning "success" it is returning 
"interrupted".



Comment at: lldb/source/Target/StackFrameList.cpp:448
   if (m_frames.size() > end_idx || GetAllFramesFetched())
-return;
+return false;
 

mib wrote:
> I find it confusing the fail here given that we've succeeded at fetching the 
> frames
GetFramesUpTo returns true if interrupted, false if not.  This was not 
interrupted, so it should return false.  I could switch this to "true if not 
interrupted" so that I could return true here, but that would be weird.  This 
isn't a "success or fail" result, it's an "interrupted or not interrupted" 
result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150236

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

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

2023-05-10 Thread Jim Ingham via Phabricator via lldb-commits

jingham updated this revision to Diff 521183.
jingham added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150236

Files:
  lldb/include/lldb/Target/StackFrameList.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/test/API/functionalities/bt-interrupt/Makefile
  lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py
  lldb/test/API/functionalities/bt-interrupt/main.c

Index: lldb/test/API/functionalities/bt-interrupt/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/bt-interrupt/main.c
@@ -0,0 +1,23 @@
+#include 
+
+// This example is meant to recurse infinitely.
+// The extra struct is just to make the frame dump
+// more complicated.
+
+struct Foo {
+  int a;
+  int b;
+  char *c;
+};
+
+int
+forgot_termination(int input, struct Foo my_foo) {
+  return forgot_termination(++input, my_foo);
+}
+
+int
+main()
+{
+  struct Foo myFoo = {100, 300, "A string you will print a lot"}; // Set a breakpoint here
+  return forgot_termination(1, myFoo);
+}
Index: lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py
===
--- /dev/null
+++ lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py
@@ -0,0 +1,49 @@
+"""
+Ensure that when the interrupt is raised we still make frame 0.
+and make sure "GetNumFrames" isn't interrupted.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestInterruptingBacktrace(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_backtrace_interrupt(self):
+"""Use RequestInterrupt followed by stack operations
+   to ensure correct interrupt behavior for stacks."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.bt_interrupt_test()
+
+def bt_interrupt_test(self):
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+   "Set a breakpoint here", self.main_source_file)
+
+# Now continue, and when we stop we will have crashed.
+process.Continue()
+self.dbg.RequestInterrupt()
+
+# Be sure to turn this off again:
+def cleanup ():
+if self.dbg.InterruptRequested():
+self.dbg.CancelInterruptRequest()
+self.addTearDownHook(cleanup)
+
+frame_0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame_0.IsValid(), "Got a good 0th frame")
+# The interrupt flag is up already, so any attempt to backtrace
+# should be cut short:
+frame_1 = thread.GetFrameAtIndex(1)
+self.assertFalse(frame_1.IsValid(), "Prevented from getting more frames")
+# Since GetNumFrames is a contract, we don't interrupt it:
+num_frames = thread.GetNumFrames()
+print(f"Number of frames: {num_frames}")
+self.assertGreater(num_frames, 1, "Got many frames")
+
+self.dbg.CancelInterruptRequest()
+
+
Index: lldb/test/API/functionalities/bt-interrupt/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/bt-interrupt/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -85,8 +85,8 @@
 return;
 
   std::lock_guard guard(m_mutex);
-
-  GetFramesUpTo(0);
+  
+  GetFramesUpTo(0, false /* allow_interrupt */);
   if (m_frames.empty())
 return;
   if (!m_frames[0]->IsInlined()) {
@@ -436,21 +436,22 @@
 next_frame.SetFrameIndex(m_frames.size());
 }
 
-void StackFrameList::GetFramesUpTo(uint32_t end_idx) {
+bool StackFrameList::GetFramesUpTo(uint32_t end_idx, bool allow_interrupt) {
   // Do not fetch frames for an invalid thread.
+  bool was_interrupted = false;
   if (!m_thread.IsValid())
-return;
+return false;
 
   // We've already gotten more frames than asked for, or we've already finished
   // unwinding, return.
   if (m_frames.size() > end_idx || GetAllFramesFetched())
-return;
+return false;
 
   Unwind &unwinder = m_thread.GetUnwinder();
 
   if (!m_show_inlined_frames) {
 GetOnlyConcreteFramesUpTo(end_idx, unwinder);
-return;
+return false;
   }
 
 #if defined(DEBUG_STACK_FRAMES)
@@ -474,13 +475,6 @@
   StackFrameSP unwind_frame_sp;
   Debugger &dbg = m_thread.GetProcess()->GetTarget().GetDebugger();
   do {
-// Check for interruption here when building the frames - this is the
-// expensive part, Dump late

[Lldb-commits] [lldb] 25495c9 - [lldb][NFCI] Remove n^2 loops and simplify iterator usage

[Lldb-commits] [PATCH] D150219: [lldb][NFCI] Remove n^2 loops and simplify iterator usage

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

[Lldb-commits] [lldb] 2ec334d - [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for `__bf16` to `BF16Ty`

[Lldb-commits] [PATCH] D150299: [lldb][NFCI] Redefine dw_attr_t typedef with llvm::dwarf::Attribute

[Lldb-commits] [PATCH] D150303: [LLDB] Add some declarations related to REPL support for mojo

[Lldb-commits] [PATCH] D150303: [LLDB] Add some declarations related to REPL support for mojo

[Lldb-commits] [PATCH] D150303: [LLDB] Add some declarations related to REPL support for mojo

[Lldb-commits] [PATCH] D150313: Fix libstdc++ data formatter for reference/pointer to std::string

[Lldb-commits] [PATCH] D150239: ObjectFileMachO: Prioritize the TEXT segment as the mach header segment, regardless of the order the segments appear in the file

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

[Lldb-commits] [lldb] 7b5dc63 - When the Debugger runs HandleProcessEvent it should allow

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for `__bf16` to `BF16Ty`

[Lldb-commits] [lldb] f9759d0 - Prioritize using a segment with the name TEXT instead off fileoff 0

[Lldb-commits] [PATCH] D150239: ObjectFileMachO: Prioritize the TEXT segment as the mach header segment, regardless of the order the segments appear in the file

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Lldb-commits] [PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

26 matches

Site Navigation

Mail list logo

Footer information