[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov updated this revision to Diff 456197.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

Files:
  clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/sugar-common-types.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaObjC/format-strings-objc.m
  compiler-rt/test/ubsan/TestCases/Integer/add-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/no-recover.cpp
  compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
  libcxx/DELETE.ME
  lldb/test/API/commands/expression/rdar42038760/main.c
  lldb/test/API/commands/expression/rdar44436068/main.c

Index: lldb/test/API/commands/expression/rdar44436068/main.c
===
--- lldb/test/API/commands/expression/rdar44436068/main.c
+++ lldb/test/API/commands/expression/rdar44436068/main.c
@@ -3,6 +3,6 @@
 __int128_t n = 1;
 n = n + n;
 return n; //%self.expect("p n", substrs=['(__int128_t) $0 = 2'])
-  //%self.expect("p n + 6", substrs=['(__int128) $1 = 8'])
-  //%self.expect("p n + n", substrs=['(__int128) $2 = 4'])
+  //%self.expect("p n + 6", substrs=['(__int128_t) $1 = 8'])
+  //%self.expect("p n + n", substrs=['(__int128_t) $2 = 4'])
 }
Index: lldb/test/API/commands/expression/rdar42038760/main.c
===
--- lldb/test/API/commands/expression/rdar42038760/main.c
+++ lldb/test/API/commands/expression/rdar42038760/main.c
@@ -10,7 +10,7 @@
   struct S0 l_19;
   l_19.f2 = 419;
   uint32_t l_4037 = 4294967295UL;
-  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(unsigned int) $0 = 358717883'])
+  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(uint32_t) $0 = 358717883'])
 }
 int main()
 {
Index: libcxx/DELETE.ME
===
--- libcxx/DELETE.ME
+++ libcxx/DELETE.ME
@@ -1 +1,2 @@
 D111283
+D111509
Index: compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(uint32_t(1) - uint32_t(2));
-  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type 'unsigned int'
+  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 #endif
 
 #ifdef SUB_I64
   (void)(uint64_t(800ll) - uint64_t(900ll));
-  // CHECK-SUB_I64: 800 - 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-SUB_I64: 800 - 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef SUB_I128
@@ -26,6 +26,6 @@
 # else
   puts("__int128 not supported\n");
 # endif
-  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
@@ -13,7 +13,7 @@
   (void)(uint16_t(0x) * uint16_t(0x8001));
 
   (void)(uint32_t(0x) * uint32_t(0x2));
-  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type 'unsigned int'
+  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 
   return 0;
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
=

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

[Adrian Vogelsgesang via Phabricator via lldb-commits]

avogelsgesang created this revision.
avogelsgesang added reviewers: labath, dblaikie, aprantl, ChuanqiXu, mib.
Herald added a project: All.
avogelsgesang requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

So far, the pretty printer for `std::coroutine_handle` internally
dereferenced the contained frame pointer displayed the `promise`
as a sub-value. As noticed in https://reviews.llvm.org/D132624
by @labath, this can lead to an endless loop in lldb during printing
if the coroutine frame pointers form a cycle.

This commit breaks the cycle by exposing the `promise` as a pointer
type instead of a value type. The depth to which the `frame variable`
and the `expression` commands dereference those pointers can be
controlled using the `--ptr-depth` argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132815

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -68,7 +68,7 @@
 result_children=[
 ValueCheck(name="resume", summary = test_generator_func_ptr_re),
 ValueCheck(name="destroy", summary = test_generator_func_ptr_re),
-ValueCheck(name="promise", value="-1")
+ValueCheck(name="promise", dereference=ValueCheck(value="-1"))
 ])
 
 # Run until after the `co_yield`
Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
@@ -44,7 +44,9 @@
   size_t GetIndexOfChildWithName(ConstString name) override;
 
 private:
-  lldb::ValueObjectSP m_frame_ptr_sp;
+  lldb::ValueObjectSP m_resume_ptr_sp;
+  lldb::ValueObjectSP m_destroy_ptr_sp;
+  lldb::ValueObjectSP m_promise_ptr_sp;
 };
 
 SyntheticChildrenFrontEnd *
Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -16,34 +16,35 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
-static ValueObjectSP GetCoroFramePtrFromHandle(ValueObject &valobj) {
-  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+static uint64_t GetCoroFramePtrFromHandle(ValueObjectSP valobj_sp) {
   if (!valobj_sp)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
 
   // We expect a single pointer in the `coroutine_handle` class.
   // We don't care about its name.
   if (valobj_sp->GetNumChildren() != 1)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
   ValueObjectSP ptr_sp(valobj_sp->GetChildAtIndex(0, true));
   if (!ptr_sp)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
   if (!ptr_sp->GetCompilerType().IsPointerType())
-return nullptr;
-
-  return ptr_sp;
-}
-
-static Function *ExtractFunction(ValueObjectSP &frame_ptr_sp, int offset) {
-  lldb::TargetSP target_sp = frame_ptr_sp->GetTargetSP();
-  lldb::ProcessSP process_sp = frame_ptr_sp->GetProcessSP();
-  auto ptr_size = process_sp->GetAddressByteSize();
+return LLDB_INVALID_ADDRESS;
 
   AddressType addr_type;
-  lldb::addr_t frame_ptr_addr = frame_ptr_sp->GetPointerValue(&addr_type);
+  lldb::addr_t frame_ptr_addr = ptr_sp->GetPointerValue(&addr_type);
   if (!frame_ptr_addr || frame_ptr_addr == LLDB_INVALID_ADDRESS)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
   lldbassert(addr_type == AddressType::eAddressTypeLoad);
+  if (addr_type != AddressType::eAddressTypeLoad)
+return LLDB_INVALID_ADDRESS;
+
+  return frame_ptr_addr;
+}
+
+static Function *ExtractFunction(lldb::TargetSP target_sp,
+ uint64_t frame_ptr_addr, int offset) {
+  lldb::ProcessSP process_sp = target_sp->GetProcessSP();
+  auto ptr_size = process_sp->GetAddressByteSize();
 
   Status error;
   auto func_ptr_addr = frame_ptr_addr + offset * ptr_size;
@@ -59,12 +60,14 @@
   return func_address.CalculateSymbolContextFunction();
 }
 
-static Function *ExtractResumeFunction(ValueObjectSP &frame_ptr_sp) {
-  return ExtractFunction(frame_ptr_sp, 0);
+static Function *ExtractResumeFunction(lldb::TargetSP target_sp,
+   uint64_t frame

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

[Adrian Vogelsgesang via Phabricator via lldb-commits]

avogelsgesang added a comment.

@labath D132815  addresses your point: With 
that change, the `promise` member now is typed as a pointer.

I would prefer to land the patches in the order D132624 
 (this review), D132735 
, D132815 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132624

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

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

In D132624#3754299 , @avogelsgesang 
wrote:

> @labath D132815  addresses your point: With 
> that change, the `promise` member now is typed as a pointer.
>
> I would prefer to land the patches in the order D132624 
>  (this review), D132735 
> , D132815 
> .

You could sort these revisions by 'Edit Related Revisions' automatically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132624

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

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

The summary looks good to me.




Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:70
+
+static bool IsNoopResumeDestroy(Function *f) {
+  if (!f)

Maybe `IsNoopCoro` or something else  looks better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132735

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

[Lldb-commits] [PATCH] D132283: [lldb] [Core] Reimplement Communication::ReadThread using MainLoop

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

mgorny added inline comments.



Comment at: lldb/source/Core/Communication.cpp:427
   // Notify the read thread.
-  m_connection_sp->InterruptRead();
 

labath wrote:
> Have you considered putting this code (some version of it) inside 
> `InterruptRead`? Basically replacing the `select` call inside BytesAvailable 
> with something MainLoop-based ?
To be honest, I've been considering removing `InterruptRead()` entirely, now 
that the read loop is triggered by available input rather than 
read-with-timeout. However, it's still used by editline support.

That said, I'm not sure what's your idea, given that `Connection` does not have 
awareness of `Communication` that's using it. I suppose I could pass the 
`MainLoop` instance as a parameter but we'd still have to maintain a separate 
version for editline support, and we only have a single caller here.


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

https://reviews.llvm.org/D132283

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