[Lldb-commits] [PATCH] D133944: [lldb][tests][gmodules] Test for expression evaluator crash for types with template base class

[Gabor Marton via Phabricator via lldb-commits]

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

Looks good to me!




Comment at: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h:7
+struct ClassInMod1 {
+  ClassInMod3 VecInMod1;
+};

I was wondering whether you would have the crash if you used the directly the 
base class? I.e. if you had the `ClassInMod3Base VecInMod1` here and 
`ClassInMod3Base VecInMod2` in the other module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133944

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

[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

[Gabor Marton via Phabricator via lldb-commits]

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

LGTM, Thanks!




Comment at: clang/lib/AST/DeclBase.cpp:1781
   if (Name && !hasLazyLocalLexicalLookups() &&
   !hasLazyExternalLexicalLookups()) {
 if (StoredDeclsMap *Map = LookupPtr) {

Michael137 wrote:
> Michael137 wrote:
> > Could merge the two if-blocks now I suppose
> Nevermind, not true
Yeah, we set HasLazyExternalLexicalLookups to true in 
https://reviews.llvm.org/D61333 exactly for the reason to continue the lookup 
into decl chain.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4927-4929
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
+  EXPECT_EQ(FoundDecls.size(), 1u);

Very good, now the behavior is the same that we have in case of the 
`ASTImporterLookupTable`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

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

[Lldb-commits] [lldb] aff1f63 - [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via lldb-commits]

Author: Matheus Izvekov
Date: 2022-09-16T11:55:40+02:00
New Revision: aff1f6310e5f4cea92c4504853d5fd824754a74f

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

LOG: [clang] use getCommonSugar in an assortment of places

For this patch, a simple search was performed for patterns where there are
two types (usually an LHS and an RHS) which are structurally the same, and there
is some result type which is resolved as either one of them (typically LHS for
consistency).

We change those cases to resolve as the common sugared type between those two,
utilizing the new infrastructure created for this purpose.

Signed-off-by: Matheus Izvekov 

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

Added: 
clang/test/Sema/sugar-common-types.c
clang/test/SemaCXX/sugar-common-types.cpp

Modified: 
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/complex-int.c
clang/test/Sema/matrix-type-operators.c
clang/test/Sema/nullability.c
clang/test/SemaCXX/complex-conversion.cpp
clang/test/SemaCXX/matrix-type-operators.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
lldb/test/API/commands/expression/rdar42038760/main.c
lldb/test/API/commands/expression/rdar44436068/main.c

Removed: 




diff  --git 
a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp 
b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
index 70ecc202d0b28..6e38ea77ad68d 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
@@ -330,7 +330,7 @@ TEST_F(ExtractVariableTest, Test) {
  void bar() {
int (*placeholder)(int) = foo('c'); (void)placeholder;
  })cpp"},
-  // Arithmetic on typedef types yields plain integer types
+  // Arithmetic on typedef types preserves typedef types
   {R"cpp(typedef long NSInteger;
  void varDecl() {
 NSInteger a = 2 * 5;
@@ -339,7 +339,7 @@ TEST_F(ExtractVariableTest, Test) {
R"cpp(typedef long NSInteger;
  void varDecl() {
 NSInteger a = 2 * 5;
-long placeholder = a * 7; NSInteger b = placeholder + 3;
+NSInteger placeholder = a * 7; NSInteger b = placeholder + 3;
  })cpp"},
   };
   for (const auto &IO : InputOutputs) {

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
index 2fc5621c2fb8f..34da420fd0cc5 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -42,7 +42,7 @@ void narrowing_size_method() {
   // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
 
   i = j + v.size();
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'global_size_t' (aka 'long long') to signed type 'int' is 
implementation-defined [cppcoreguidelines-narrowing-conversions]
   // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
 }
 
@@ -51,7 +51,7 @@ void narrowing_size_method_binary_expr() {
   int j;
   vector v;
   i = j + v.size();
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'global_size_t' (aka 'long long') to signed type 'int' is 
implementation-defined [cppcoreguidelines-narrowing-conversion

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

[Matheus Izvekov via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaff1f6310e5f: [clang] use getCommonSugar in an assortment of 
places (authored by mizvekov).

Changed prior to commit:
  https://reviews.llvm.org/D111509?vs=460587&id=460691#toc

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/complex-int.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/complex-conversion.cpp
  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
  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: 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: runtim

[Lldb-commits] [PATCH] D133944: [lldb][tests][gmodules] Test for expression evaluator crash for types with template base class

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h:7
+struct ClassInMod1 {
+  ClassInMod3 VecInMod1;
+};

martong wrote:
> I was wondering whether you would have the crash if you used the directly the 
> base class? I.e. if you had the `ClassInMod3Base VecInMod1` here and 
> `ClassInMod3Base VecInMod2` in the other module.
Very true! Will reduce it down further, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133944

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

[Lldb-commits] [PATCH] D134029: [LLDB] Properly return errors from "memory region --all"

[David Spickett via Phabricator via lldb-commits]

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

When I wrote the initial version I forgot that a region being
unmapped is not an error. There are real errors that we don't
want to hide, such as the remote not supporting the
qMemoryRegionInfo packet (gdbserver does not).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134029

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -8,6 +8,8 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+from lldbsuite.test.gdbclientutils import MockGDBServerResponder
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
 
 
 class MemoryCommandRegion(TestBase):
@@ -126,3 +128,36 @@
 
 previous_base = region_base
 previous_end = region_end
+
+class MemoryCommandRegionAll(GDBRemoteTestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_all_error(self):
+# The --all option should keep looping until the end of the memory 
range.
+# If there is an error it should be reported as if you were just asking
+# for one region. In this case the error is the remote not supporting
+# qMemoryRegionInfo.
+# (a region being unmapped is not an error, we just get a result
+# describing an unmapped range)
+class MyResponder(MockGDBServerResponder):
+def qMemoryRegionInfo(self, addr):
+# Empty string means unsupported.
+return ""
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+self.runCmd("log enable gdb-remote packets")
+self.addTearDownHook(
+  lambda: self.runCmd("log disable gdb-remote packets"))
+
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateStopped])
+
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+interp.HandleCommand("memory region --all ", result)
+self.assertFalse(result.Succeeded())
+self.assertEqual(result.GetError(),
+"error: qMemoryRegionInfo is not supported\n")
\ No newline at end of file
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1830,9 +1830,6 @@
   addr = region_info.GetRange().GetRangeEnd();
 }
   }
-
-  // Even if we read nothing, don't error for --all
-  error.Clear();
 } else {
   lldb_private::MemoryRegionInfo region_info;
   error = process_sp->GetMemoryRegionInfo(load_addr, region_info);


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -8,6 +8,8 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+from lldbsuite.test.gdbclientutils import MockGDBServerResponder
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
 
 
 class MemoryCommandRegion(TestBase):
@@ -126,3 +128,36 @@
 
 previous_base = region_base
 previous_end = region_end
+
+class MemoryCommandRegionAll(GDBRemoteTestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_all_error(self):
+# The --all option should keep looping until the end of the memory range.
+# If there is an error it should be reported as if you were just asking
+# for one region. In this case the error is the remote not supporting
+# qMemoryRegionInfo.
+# (a region being unmapped is not an error, we just get a result
+# describing an unmapped range)
+class MyResponder(MockGDBServerResponder):
+def qMemoryRegionInfo(self, addr):
+# Empty string means unsupported.
+return ""
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+self.runCmd("log enable gdb-remote packets")
+self.addTearDownHook(
+  lambda: self.runCmd("log disable gdb-remote packets"))
+
+

[Lldb-commits] [PATCH] D134030: [LLDB] Fix "memory region --all" when there is no ABI plugin

[David Spickett via Phabricator via lldb-commits]

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

There are two conditions for the loop exit. Either we hit LLDB_INVALID_ADDRESS
or the ABI tells us we are beyond mappable memory.

I made a mistake in that second part that meant if you had no ABI plugin
--all would stop on the first loop and return nothing.

If there's no ABI plugin we should only check for LLDB_INVALID_ADDRESS.

Depends on D134029 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134030

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -160,4 +160,38 @@
 interp.HandleCommand("memory region --all ", result)
 self.assertFalse(result.Succeeded())
 self.assertEqual(result.GetError(),
-"error: qMemoryRegionInfo is not supported\n")
\ No newline at end of file
+"error: qMemoryRegionInfo is not supported\n")
+
+@skipIfAsan
+def test_all_no_abi_plugin(self):
+# There are two conditions for breaking the all loop. Either we get to
+# LLDB_INVALID_ADDRESS, or the ABI plugin tells us we have got beyond
+# the mappable range. If we don't have an ABI plugin, the option 
should still
+# work and only check the first condition.
+
+class MyResponder(MockGDBServerResponder):
+def qMemoryRegionInfo(self, addr):
+if addr == 0:
+return "start:0;size:1;"
+# Goes until the end of memory.
+if addr == 0x1:
+return "start:1;size:fffe;"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateStopped])
+
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+interp.HandleCommand("memory region --all ", result)
+self.assertTrue(result.Succeeded())
+self.assertEqual(result.GetOutput(),
+"[0x-0x0001) ---\n"
+"[0x0001-0x) ---\n")
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1821,7 +1821,7 @@
  // When there are non-address bits the last range will not extend
  // to LLDB_INVALID_ADDRESS but to the max virtual address.
  // This prevents us looping forever if that is the case.
- (abi && (abi->FixAnyAddress(addr) == addr))) {
+ (!abi || (abi->FixAnyAddress(addr) == addr))) {
 lldb_private::MemoryRegionInfo region_info;
 error = process_sp->GetMemoryRegionInfo(addr, region_info);
 


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -160,4 +160,38 @@
 interp.HandleCommand("memory region --all ", result)
 self.assertFalse(result.Succeeded())
 self.assertEqual(result.GetError(),
-"error: qMemoryRegionInfo is not supported\n")
\ No newline at end of file
+"error: qMemoryRegionInfo is not supported\n")
+
+@skipIfAsan
+def test_all_no_abi_plugin(self):
+# There are two conditions for breaking the all loop. Either we get to
+# LLDB_INVALID_ADDRESS, or the ABI plugin tells us we have got beyond
+# the mappable range. If we don't have an ABI plugin, the option should still
+# work and only check the first condition.
+
+class MyResponder(MockGDBServerResponder):
+def qMemoryRegionInfo(self, addr):
+if addr == 0:
+return "start:0;size:1;"
+# Goes until the end of memory.
+if addr == 0x1:
+return "start:1;size:fffe;"
+
+self.server.respon

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting with reading/writing memory in a Scripted Process (WIP)

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This patch should improve error reporting by passing the status object from the 
Scripted Process plugin down to the python interface, by converting it to a 
SBError (wrapped into a PyObject).

This way, the Scripted Process python implementation can report back errors 
when reading or writing memory to lldb.

This will also be used when initializing the ScriptedProcess python object.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134033

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/API/SBError.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/SBError.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  
lldb/test/API/functionalities/scripted_process_passthru/scripted_process_passthru.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -193,6 +193,52 @@
   return python::PythonObject();
 }
 
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::ValueObjectSP value_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::TargetSP target_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::ProcessSP process_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::ThreadPlanSP thread_plan_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::BreakpointSP breakpoint_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::StatusSP status_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(const StructuredDataImpl &data_impl) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::ThreadSP thread_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::StackFrameSP frame_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::DebuggerSP debugger_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::WatchpointSP watchpoint_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::BreakpointLocationSP bp_loc_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(lldb::ExecutionContextRefSP ctx_sp) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(const TypeSummaryOptions &summary_options) {
+  return python::PythonObject();
+}
+python::PythonObject lldb_private::ToSWIGWrapper(const SymbolContext &sym_ctx) {
+  return python::PythonObject();
+}
+
 python::PythonObject lldb_private::LLDBSwigPythonCreateScriptedProcess(
 const char *python_class_name, const char *session_dictionary_name,
 const lldb::TargetSP &target_sp, const StructuredDataImpl &args_impl,
Index: lldb/test/API/functionalities/scripted_process_passthru/scripted_process_passthru.py
===
--- lldb/test/API/functionalities/scripted_process_passthru/scripted_process_passthru.py
+++ lldb/test/API/functionalities/scripted_process_passthru/scripted_process_passthru.py
@@ -46,11 +46,20 @@
 
 return data
 
+def write_memory_at_address(self, addr: int, buf : bytes, size: int) -> lldb.SBData:
+error = lldb.SBError()
+bytes_written = self.parent_process.WriteMemory(addr, size, error)
+
+if error.Fail():
+return 0
+
+return bytes_written
+
 def get_loaded_images(self):
 return self.loaded_images
 
 def get_process_id(self) -> int:
-return 42
+return self.parent_process.GetProcessID()
 
 def should_stop(self) -> bool:
 return True
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
===
--- lldb/source/Plugins/S

[Lldb-commits] [PATCH] D133944: [lldb][tests][gmodules] Test for expression evaluator crash for types with template base class

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 460717.
Michael137 added a comment.

- Reduce test further
- Separate check on decl structure into new function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133944

Files:
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/Makefile
  
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.h
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/main.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module.modulemap
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.h
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.h

Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.h
@@ -0,0 +1,10 @@
+#ifndef MOD2_H_IN
+#define MOD2_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod2 {
+  ClassInMod3 VecInMod2;
+};
+
+#endif
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.cpp
@@ -0,0 +1,3 @@
+#include "module2.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.h
@@ -0,0 +1,10 @@
+#ifndef MOD1_H_IN
+#define MOD1_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod1 {
+  ClassInMod3 VecInMod1;
+};
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.cpp
@@ -0,0 +1,3 @@
+#include "module1.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module.modulemap
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module.modulemap
@@ -0,0 +1,14 @@
+module Module1 {
+  header "module1.h"
+  export *
+}
+
+module Module2 {
+  header "module2.h"
+  export *
+}
+
+module BaseModule {
+  header "base_module.h"
+  export *
+}
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/main.cpp
@@ -0,0 +1,15 @@
+#include "module1.h"
+#include "module2.h"
+
+#include 
+
+int main() {
+  ClassInMod1 FromMod1;
+  ClassInMod2 FromMod2;
+
+  FromMod1.VecInMod1.Member = 137;
+  FromMod2.VecInMod2.Member = 42;
+
+  std::puts("Break here");
+  return 0;
+}
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.h
@@ -0,0 +1,6 @@
+#ifndef MOD3_H_IN
+#define MOD3_H_IN
+
+template  struct ClassInMod3 { int Member = 0; };
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.cpp
@@ -0,0 +1,3 @@
+#include "base_module.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -0,0 +1,72 @@
+"""
+Tests the scenario where we evaluate expressions
+of two types in different modules that reference
+a class template instantiated with the same
+template argument.
+
+Note that,
+1. Since the decls originate from modules, LLDB
+   marks them as such and Clang doesn't create
+   a LookupPtr map on the corresponding DeclContext.
+   This prevents regular DeclContext::lookup from
+   succeeding.
+2. Because we reference the same class template
+   from two different modules we get a redeclaration
+   chain for the class's ClassTemplateSpecializationDecl.
+   The importer will import all FieldDecls into the
+   same DeclContext on the redeclaration chain. If
+   we don't do the bookkeeping correctly we end up
+   with duplicate decls on the same DeclContext

[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 460718.
Michael137 added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -34,7 +34,6 @@
 self.main_source_file = lldb.SBFileSpec("main.cpp")
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_template_arg(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
 
@@ -51,7 +50,6 @@
 ])
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_duplicate_decls(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.


Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -34,7 +34,6 @@
 self.main_source_file = lldb.SBFileSpec("main.cpp")
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_template_arg(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", self.main_source_file)
 
@@ -51,7 +50,6 @@
 ])
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_duplicate_decls(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", self.main_source_file)
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting with reading/writing memory in a Scripted Process (WIP)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:186
+  StatusSP status_sp = std::make_shared(error);
+  PythonObject* sb_error = new PythonObject(ToSWIGWrapper(status_sp));
+  

@labath In order to pass down the `Status&` to python, I create a `StatusSP` 
(to avoid leaking the `lldb_private::Status` type to python), and turn that 
into a `python::PyObject` by calling `ToSWIGWrapper`. This is why I moved its 
declaration to `SWIGPythonBridge.h`.

Finally, the `python::PyObject` is wrapped into another `PythonObject*` so it 
can be passed to `GetPythonValueFormatString` and `PyObject_CallMethod` in 
`ScriptedPythonInterface::Dispatch`

I tried to follow the logic explained in 7f09ab0, however, when `ToSWIGWrapper` 
is called at runtime, it crashes in Python:

```
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS 
(code=1, address=0x10)
frame #0: 0x0001056f3c3c Python`PyTuple_New + 212
frame #1: 0x000135524720 
liblldb.16.0.0git.dylib`SWIG_Python_NewShadowInstance(data=0x611d6340, 
swig_this=0x000105ec5d70) at LLDBWrapPython.cpp:2284:28
  * frame #2: 0x000135515a00 
liblldb.16.0.0git.dylib`SWIG_Python_NewPointerObj(self=0x, 
ptr=0x61dc4040, type=0x00014448c140, flags=1) at 
LLDBWrapPython.cpp:2395:22
frame #3: 0x0001355157f4 
liblldb.16.0.0git.dylib`lldb_private::python::ToSWIGHelper(obj=0x61dc4040,
 info=0x00014448c140) at python-swigsafecast.swig:5:29
frame #4: 0x000135515e14 
liblldb.16.0.0git.dylib`lldb_private::python::ToSWIGWrapper(status_sp=std::__1::shared_ptr::element_type
 @ 0x60ac9a18 strong=3 weak=1) at python-swigsafecast.swig:37:10
frame #5: 0x0001367383c4 
liblldb.16.0.0git.dylib`lldb_private::ScriptedProcessPythonInterface::ReadMemoryAtAddress(this=0x611cd2c0,
 address=8034160640, size=512, error=0x00016b35c650) at 
ScriptedProcessPythonInterface.cpp:161:45
```

Am I doing something wrong or maybe I'm missing something ?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134033

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

[Lldb-commits] [PATCH] D134035: [LLDB] Format lldb-server's target XML

[David Spickett via Phabricator via lldb-commits]

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

So that the XML isn't one giant line. Which wasn't
a problem for lldb but was for me trying to troubleshoot
it using the logs.

It now looks like:



  aarch64
  
<...>

  




Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134035

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestPtyServer.py
  
lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py


Index: 
lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
===
--- 
lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
+++ 
lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
@@ -25,7 +25,7 @@
 LENGTH),
 {   
 "direction": "send", 
-"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$"), 
+"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$", 
flags=re.DOTALL),
 "capture": {1: "target_xml"}
 }],
 True)
Index: lldb/test/API/tools/lldb-server/TestPtyServer.py
===
--- lldb/test/API/tools/lldb-server/TestPtyServer.py
+++ lldb/test/API/tools/lldb-server/TestPtyServer.py
@@ -63,7 +63,7 @@
 "read packet: $qXfer:features:read:target.xml:0,20#00",
 {
 "direction": "send",
-"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$"),
+"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$", 
flags=re.DOTALL),
 "capture": {1: "target_xml"},
 }],
 True)
Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3066,19 +3066,24 @@
 
   StreamString response;
 
-  response.Printf("");
-  response.Printf("");
+  response.Printf("\n");
+  response.Printf("\n");
+  response.IndentMore();
 
-  response.Printf("%s",
+  response.Indent();
+  response.Printf("%s\n",
   m_current_process->GetArchitecture()
   .GetTriple()
   .getArchName()
   .str()
   .c_str());
 
-  response.Printf("");
+  response.Indent("\n");
 
   const int registers_count = reg_context.GetUserRegisterCount();
+  if (registers_count)
+response.IndentMore();
+
   for (int reg_index = 0; reg_index < registers_count; reg_index++) {
 const RegisterInfo *reg_info =
 reg_context.GetRegisterInfoAtIndex(reg_index);
@@ -3090,7 +3095,9 @@
   continue;
 }
 
-response.Printf("name, reg_info->byte_size * 8, reg_index);
 
 if (!reg_context.RegisterOffsetIsDynamic())
@@ -3139,11 +3146,15 @@
   response.Printf("\" ");
 }
 
-response.Printf("/>");
+response.Printf("/>\n");
   }
 
-  response.Printf("");
-  response.Printf("");
+  if (registers_count)
+response.IndentLess();
+
+  response.Indent("\n");
+  response.IndentLess();
+  response.Indent("\n");
   return MemoryBuffer::getMemBufferCopy(response.GetString(), "target.xml");
 }
 


Index: lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
===
--- lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
+++ lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
@@ -25,7 +25,7 @@
 LENGTH),
 {   
 "direction": "send", 
-"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$"), 
+"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$", flags=re.DOTALL),
 "capture": {1: "target_xml"}
 }],
 True)
Index: lldb/test/API/tools/lldb-server/TestPtyServer.py
===
--- lldb/test/API/tools/lldb-server/TestPtyServer.py
+++ lldb/test/API/tools/lldb-server/TestPtyServer.py
@@ -63,7 +63,7 @@
 "read packet: $qXfer:features:read:target.xml:0,20#00",
 {
 "direction": "send",
-"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$"),
+"regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$", flags=re.DOTALL),
 "capture": {1: "target_xml"},
 }],
 True)
Index

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, DavidSpickett.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

If a process has multiple threads, the thread with the stop
info might not be the first one in the thread list.

On Windows, under certain circumstances, processes seem to have
one or more extra threads that haven't been launched by the
executable itself, waiting in NtWaitForWorkViaWorkerFactory. If the
main (stopped) thread isn't the first one in the list (the order
seems nondeterministic), DidProcessStopAbnormally() would return
false prematurely, instead of inspecting later threads.

The main observable effect of DidProcessStopAbnormally() erroneously
returning false, is when running lldb with multiple "-o" parameters
to specify multiple commands to execute on the command line.

After an abnormal stop, lldb would stop executing "-o" parameters
and execute "-k" parameters instead - but due to this issue, it
would instead keep executing "-o" parameters as if there was no
abnormal stop. (If multiple parameters are specified via a script
file via the "-s" option, all of the commands in that file are
executed regardless of whether there's an abnormal stop inbetween.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134037

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2441,7 +2441,7 @@
   for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
 StopInfoSP stop_info = thread_sp->GetStopInfo();
 if (!stop_info)
-  return false;
+  continue;
 
 const StopReason reason = stop_info->GetStopReason();
 if (reason == eStopReasonException ||


Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2441,7 +2441,7 @@
   for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
 StopInfoSP stop_info = thread_sp->GetStopInfo();
 if (!stop_info)
-  return false;
+  continue;
 
 const StopReason reason = stop_info->GetStopReason();
 if (reason == eStopReasonException ||
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[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 460723.

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/CodeGen/CGExprComplex.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/complex-conversion.cpp
  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
  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: 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
===
--- compiler-rt/test/ubsan/T

[Lldb-commits] [PATCH] D134039: [LLDB] Make instruction emulation context type private

[David Spickett via Phabricator via lldb-commits]

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

This is the first step to being able to handle non
trivial types in the union.

info_type effects the lifetime of the objects in the union,
so making it private means we know you have to call one of the
Set<...> functions to change it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134039

Files:
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/source/Core/EmulateInstruction.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp


Index: 
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- 
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ 
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -455,7 +455,7 @@
   case EmulateInstruction::eContextPushRegisterOnStack: {
 uint32_t reg_num = LLDB_INVALID_REGNUM;
 uint32_t generic_regnum = LLDB_INVALID_REGNUM;
-assert(context.info_type ==
+assert(context.GetInfoType() ==
EmulateInstruction::eInfoTypeRegisterToRegisterPlusOffset &&
"unhandled case, add code to handle this!");
 const uint32_t unwind_reg_kind = m_unwind_plan_ptr->GetRegisterKind();
@@ -574,7 +574,8 @@
 // with the same amount.
 lldb::RegisterKind kind = m_unwind_plan_ptr->GetRegisterKind();
 if (m_fp_is_cfa && reg_info->kinds[kind] == m_cfa_reg_info.kinds[kind] &&
-context.info_type == EmulateInstruction::eInfoTypeRegisterPlusOffset &&
+context.GetInfoType() ==
+EmulateInstruction::eInfoTypeRegisterPlusOffset &&
 context.info.RegisterPlusOffset.reg.kinds[kind] ==
 m_cfa_reg_info.kinds[kind]) {
   const int64_t offset = context.info.RegisterPlusOffset.signed_offset;
@@ -585,18 +586,19 @@
 
   case EmulateInstruction::eContextAbsoluteBranchRegister:
   case EmulateInstruction::eContextRelativeBranchImmediate: {
-if (context.info_type == EmulateInstruction::eInfoTypeISAAndImmediate &&
+if (context.GetInfoType() == EmulateInstruction::eInfoTypeISAAndImmediate 
&&
 context.info.ISAAndImmediate.unsigned_data32 > 0) {
   m_forward_branch_offset =
   context.info.ISAAndImmediateSigned.signed_data32;
-} else if (context.info_type ==
+} else if (context.GetInfoType() ==
EmulateInstruction::eInfoTypeISAAndImmediateSigned &&
context.info.ISAAndImmediateSigned.signed_data32 > 0) {
   m_forward_branch_offset = context.info.ISAAndImmediate.unsigned_data32;
-} else if (context.info_type == EmulateInstruction::eInfoTypeImmediate &&
+} else if (context.GetInfoType() ==
+   EmulateInstruction::eInfoTypeImmediate &&
context.info.unsigned_immediate > 0) {
   m_forward_branch_offset = context.info.unsigned_immediate;
-} else if (context.info_type ==
+} else if (context.GetInfoType() ==
EmulateInstruction::eInfoTypeImmediateSigned &&
context.info.signed_immediate > 0) {
   m_forward_branch_offset = context.info.signed_immediate;
@@ -609,7 +611,7 @@
 const uint32_t generic_regnum = reg_info->kinds[eRegisterKindGeneric];
 if (reg_num != LLDB_INVALID_REGNUM &&
 generic_regnum != LLDB_REGNUM_GENERIC_SP) {
-  switch (context.info_type) {
+  switch (context.GetInfoType()) {
   case EmulateInstruction::eInfoTypeAddress:
 if (m_pushed_regs.find(reg_num) != m_pushed_regs.end() &&
 context.info.address == m_pushed_regs[reg_num]) {
Index: lldb/source/Core/EmulateInstruction.cpp
===
--- lldb/source/Core/EmulateInstruction.cpp
+++ lldb/source/Core/EmulateInstruction.cpp
@@ -440,7 +440,7 @@
 break;
   }
 
-  switch (info_type) {
+  switch (GetInfoType()) {
   case eInfoTypeRegisterPlusOffset:
 strm.Printf(" (reg_plus_offset = %s%+" PRId64 ")",
 info.RegisterPlusOffset.reg.name,
Index: lldb/include/lldb/Core/EmulateInstruction.h
===
--- lldb/include/lldb/Core/EmulateInstruction.h
+++ lldb/include/lldb/Core/EmulateInstruction.h
@@ -183,7 +183,12 @@
 
   struct Context {
 ContextType type = eContextInvalid;
+
+  private:
 enum InfoType info_type = eInfoTypeNoArgs;
+
+  public:
+enum InfoType GetInfoType() const { return info_type; }
 union {
   struct RegisterPlusOffset {
 RegisterInfo reg;  // base register


Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyIns

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: atanasyan, jrtc27.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In future I want to extend RegisterInfo and that will likely be non trivial
types like string and vector. RegisterInfo gets stored in Context, so Context
must be changed to a discriminated enum that properly calls destructors.

(which would be std::variant but although llvm is using -std=c++17 I don't
think we can expect c++17 library features just yet)

As so much existing code does this:
Context ctx;
// Lots of code that interacts with ctx...
ctx.Set<...>

I wasn't able to switch to factory functions for the Set<...> methods without a 
lot
more changes. Instead I've made the default constructor construct to the NoArgs 
type
and then Set<...> will change the type later.

Whenever you call a Set<...> it will destroy whatever is currently in the union
before setting the new data. Potentially non trivial types like RegisterInfos 
here are
placement newed because otherwise operator= would try to destroy the existing 
data
which is in fact uninitialised memory as far as we're concerned.

For the destructor we switch on the current info_type to know what to destruct.
(info_type that was made private in a previous change, so we know we have 
control of it)

Some places in lldb were using memset to initalise RegisterInfos. I tracked 
these
down with GCC's -Wclass-memaccess.
https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#index-Wclass-memaccess

To replace these I've added sensible default values for each member in 
RegisterInfo
and filled kinds with LLDB_INVALID_REGNUM in the default constructor.

Depends on D134039 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134041

Files:
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
  lldb/source/Target/DynamicRegisterInfo.cpp

Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -236,7 +236,6 @@
 RegisterInfo reg_info;
 std::vector value_regs;
 std::vector invalidate_regs;
-memset(®_info, 0, sizeof(reg_info));
 
 ConstString name_val;
 ConstString alt_name_val;
Index: lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
===
--- lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
+++ lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
@@ -12,6 +12,7 @@
 #include "lldb/Core/EmulateInstruction.h"
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Utility/Status.h"
+#include "llvm/ADT/Optional.h"
 
 namespace llvm {
 class MCDisassembler;
@@ -168,6 +169,9 @@
 
   const char *GetRegisterName(unsigned reg_num, bool alternate_name);
 
+  llvm::Optional
+  GetMIPS64DWARFRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num);
+
 private:
   std::unique_ptr m_disasm;
   std::unique_ptr m_subtype_info;
Index: lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
===
--- lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
+++ lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
@@ -572,6 +572,58 @@
   return nullptr;
 }
 
+llvm::Optional
+EmulateInstructionMIPS64::GetMIPS64DWARFRegisterInfo(RegisterKind reg_kind,
+ uint32_t reg_num) {
+  RegisterInfo reg_info;
+
+  if (reg_num == dwarf_sr_mips64 || reg_num == dwarf_fcsr_mips64 ||
+  reg_num == dwarf_fir_mips64 || reg_num == dwarf_mcsr_mips64 ||
+  reg_num == dwarf_mir_mips64 || reg_num == dwarf_config5_mips64) {
+reg_info.byte_size = 4;
+reg_info.format = eFormatHex;
+reg_info.encoding = eEncodingUint;
+  } else if ((int)reg_num >= dwarf_zero_mips64 &&
+ (int)reg_num <= dwarf_f31_mips64) {
+reg_info.byte_size = 8;
+reg_info.format = eFormatHex;
+reg_info.encoding = eEncodingUint;
+  } else if ((int)reg_num >= dwarf_w0_mips64 &&
+ (int)reg_num <= dwarf_w31_mips64) {
+reg_info.byte_size = 16;
+reg_info.format = eFormatVectorOfUInt8;
+reg_info.encoding = eEncodingVector;
+  } else {
+return llvm::None;
+  }
+
+  reg_info.name = GetRegisterName(reg_num, false);
+  reg_info.alt_name = GetRegisterName(reg_num, true);
+  reg_info.kinds[eRegisterKin

[Lldb-commits] [PATCH] D134043: [lldb] Log when we cannot find an equivalent for a gdb register type

[David Spickett via Phabricator via lldb-commits]

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

This happens if the type is described elsewhere in target xml as a
 or .

Also hardcode the function names into the log messages because
if you use __FUNCTION__ in a lambda you just get "operator()".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134043

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4029,8 +4029,10 @@
   if (!feature_node)
 return false;
 
+  Log *log(GetLog(GDBRLog::Process));
+
   feature_node.ForEachChildElementWithName(
-  "reg", [&target_info, ®isters](const XMLNode ®_node) -> bool {
+  "reg", [&target_info, ®isters, log](const XMLNode ®_node) -> bool {
 std::string gdb_group;
 std::string gdb_type;
 DynamicRegisterInfo::Register reg_info;
@@ -4039,9 +4041,9 @@
 
 // FIXME: we're silently ignoring invalid data here
 reg_node.ForEachAttribute([&target_info, &gdb_group, &gdb_type,
-   &encoding_set, &format_set, ®_info](
-  const llvm::StringRef &name,
-  const llvm::StringRef &value) -> bool {
+   &encoding_set, &format_set, ®_info,
+   log](const llvm::StringRef &name,
+const llvm::StringRef &value) -> bool {
   if (name == "name") {
 reg_info.name.SetString(value);
   } else if (name == "bitsize") {
@@ -4098,10 +4100,10 @@
 SplitCommaSeparatedRegisterNumberString(
 value, reg_info.invalidate_regs, 0);
   } else {
-Log *log(GetLog(GDBRLog::Process));
 LLDB_LOGF(log,
-  "ProcessGDBRemote::%s unhandled reg attribute %s = %s",
-  __FUNCTION__, name.data(), value.data());
+  "ProcessGDBRemote::ParseRegisters unhandled reg "
+  "attribute %s = %s",
+  name.data(), value.data());
   }
   return true; // Keep iterating through all attributes
 });
@@ -4123,6 +4125,12 @@
 // them as vector (similarly to xmm/ymm)
 reg_info.format = eFormatVectorOfUInt8;
 reg_info.encoding = eEncodingVector;
+  } else {
+LLDB_LOGF(
+log,
+"ProcessGDBRemote::ParseRegisters Could not determine lldb"
+"format and encoding for gdb type %s",
+gdb_type.c_str());
   }
 }
 
@@ -4140,7 +4148,6 @@
 }
 
 if (reg_info.byte_size == 0) {
-  Log *log(GetLog(GDBRLog::Process));
   LLDB_LOGF(log,
 "ProcessGDBRemote::%s Skipping zero bitsize register %s",
 __FUNCTION__, reg_info.name.AsCString());


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4029,8 +4029,10 @@
   if (!feature_node)
 return false;
 
+  Log *log(GetLog(GDBRLog::Process));
+
   feature_node.ForEachChildElementWithName(
-  "reg", [&target_info, ®isters](const XMLNode ®_node) -> bool {
+  "reg", [&target_info, ®isters, log](const XMLNode ®_node) -> bool {
 std::string gdb_group;
 std::string gdb_type;
 DynamicRegisterInfo::Register reg_info;
@@ -4039,9 +4041,9 @@
 
 // FIXME: we're silently ignoring invalid data here
 reg_node.ForEachAttribute([&target_info, &gdb_group, &gdb_type,
-   &encoding_set, &format_set, ®_info](
-  const llvm::StringRef &name,
-  const llvm::StringRef &value) -> bool {
+   &encoding_set, &format_set, ®_info,
+   log](const llvm::StringRef &name,
+const llvm::StringRef &value) -> bool {
   if (name == "name") {
 reg_info.name.SetString(value);
   } else if (name == "bitsize") {
@@ -4098,10 +4100,10 @@
 SplitCommaSeparatedRegisterNumberString(
 value, reg_info.invalidate_regs, 0);
   } else {
-Log *log(GetLog(GDBRLog::Process));
 LLDB_LOGF(log,
-  "ProcessGDBRemote::%s unhandled reg attribute %s = %s",
-

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added reviewers: aprantl, clayborg.
DavidSpickett added a comment.

Note that this does not add any non trivial types but of course I did test it 
by adding some to RegisterInfo.

This work is early parts of 
https://discourse.llvm.org/t/rfc-showing-register-fields-in-lldb/64676.




Comment at: lldb/source/Target/DynamicRegisterInfo.cpp:239
 std::vector invalidate_regs;
-memset(®_info, 0, sizeof(reg_info));
 

This being an instance where there is a now a sensible default constructor that 
is used instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134041

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

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added a comment.

In D133973#3793398 , @JDevlieghere 
wrote:

> does this result in another search path or does that replace the former

I did some local testing and it seems to add another path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

If I read your summary correctly, then `std::variant` would simplify your code. 
LLVM still uses `llvm::optional`. As there is no LLVM variant, I would go for 
`std::variant` .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134041

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

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added inline comments.



Comment at: lldb/test/API/lit.cfg.py:176
   if platform.system() != 'Windows':
-if is_configured('llvm_include_dir') and is_configured('llvm_libs_dir'):
-  dotest_cmd += ['--libcxx-include-dir', 
os.path.join(config.llvm_include_dir, 'c++', 'v1')]
-  dotest_cmd += ['--libcxx-library-dir', config.llvm_libs_dir]
+if is_configured('libcxx_include_dir') and 
is_configured('libcxx_libs_dir'):
+  dotest_cmd += ['--libcxx-include-dir', config.libcxx_include_dir]

Isn't this always going to return true? (same for `is_configured 
(libcxx_include_target_dir)`).

`is_configured` returns the empty string when the CMake variables are empty, 
and the empty string converts to True


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> As there is no LLVM variant, I would go for std::variant

I was under the impression that we can use 17 language features but not library 
features, yet. Let me check that what the minimum toolchains have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134041

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

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

[Matheus Izvekov via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67e229831154: [clang] use getCommonSugar in an assortment of 
places (authored by mizvekov).

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/CodeGen/CGExprComplex.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/complex-conversion.cpp
  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
  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: 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: un

[Lldb-commits] [lldb] 67e2298 - [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via lldb-commits]

Author: Matheus Izvekov
Date: 2022-09-16T16:36:00+02:00
New Revision: 67e22983115451ef5512ad2813dd337762c52da3

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

LOG: [clang] use getCommonSugar in an assortment of places

For this patch, a simple search was performed for patterns where there are
two types (usually an LHS and an RHS) which are structurally the same, and there
is some result type which is resolved as either one of them (typically LHS for
consistency).

We change those cases to resolve as the common sugared type between those two,
utilizing the new infrastructure created for this purpose.

Signed-off-by: Matheus Izvekov 

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

Added: 
clang/test/Sema/sugar-common-types.c
clang/test/SemaCXX/sugar-common-types.cpp

Modified: 
clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
clang/lib/CodeGen/CGExprComplex.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/AST/ast-dump-fpfeatures.cpp
clang/test/CodeGen/complex-math.c
clang/test/CodeGen/compound-assign-overflow.c
clang/test/Sema/complex-int.c
clang/test/Sema/matrix-type-operators.c
clang/test/Sema/nullability.c
clang/test/SemaCXX/complex-conversion.cpp
clang/test/SemaCXX/matrix-type-operators.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
lldb/test/API/commands/expression/rdar42038760/main.c
lldb/test/API/commands/expression/rdar44436068/main.c

Removed: 




diff  --git 
a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp 
b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
index 70ecc202d0b28..6e38ea77ad68d 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
@@ -330,7 +330,7 @@ TEST_F(ExtractVariableTest, Test) {
  void bar() {
int (*placeholder)(int) = foo('c'); (void)placeholder;
  })cpp"},
-  // Arithmetic on typedef types yields plain integer types
+  // Arithmetic on typedef types preserves typedef types
   {R"cpp(typedef long NSInteger;
  void varDecl() {
 NSInteger a = 2 * 5;
@@ -339,7 +339,7 @@ TEST_F(ExtractVariableTest, Test) {
R"cpp(typedef long NSInteger;
  void varDecl() {
 NSInteger a = 2 * 5;
-long placeholder = a * 7; NSInteger b = placeholder + 3;
+NSInteger placeholder = a * 7; NSInteger b = placeholder + 3;
  })cpp"},
   };
   for (const auto &IO : InputOutputs) {

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
index 2fc5621c2fb8f..34da420fd0cc5 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -42,7 +42,7 @@ void narrowing_size_method() {
   // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
 
   i = j + v.size();
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'global_size_t' (aka 'long long') to signed type 'int' is 
implementation-defined [cppcoreguidelines-narrowing-conversions]
   // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
 }
 
@@ -51,7 +51,7 @@ void narrowing_size_method_binary_expr() {
   int j;
   vector v;
   i = j + v.size();
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'global_size_t' (aka 'long long') to signed

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added inline comments.



Comment at: lldb/test/API/lit.cfg.py:176
   if platform.system() != 'Windows':
-if is_configured('llvm_include_dir') and is_configured('llvm_libs_dir'):
-  dotest_cmd += ['--libcxx-include-dir', 
os.path.join(config.llvm_include_dir, 'c++', 'v1')]
-  dotest_cmd += ['--libcxx-library-dir', config.llvm_libs_dir]
+if is_configured('libcxx_include_dir') and 
is_configured('libcxx_libs_dir'):
+  dotest_cmd += ['--libcxx-include-dir', config.libcxx_include_dir]

fdeazeve wrote:
> Isn't this always going to return true? (same for `is_configured 
> (libcxx_include_target_dir)`).
> 
> `is_configured` returns the empty string when the CMake variables are empty, 
> and the empty string converts to True
Since this is a python file, it should follow the python rules where an empty 
string is treated as false:

```
$ python3
Python 3.10.6 (main, Aug 10 2022, 11:19:32) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> x = ''
>>> 'foo' if x else 'bar'
'bar'
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added inline comments.



Comment at: lldb/test/API/lit.cfg.py:176
   if platform.system() != 'Windows':
-if is_configured('llvm_include_dir') and is_configured('llvm_libs_dir'):
-  dotest_cmd += ['--libcxx-include-dir', 
os.path.join(config.llvm_include_dir, 'c++', 'v1')]
-  dotest_cmd += ['--libcxx-library-dir', config.llvm_libs_dir]
+if is_configured('libcxx_include_dir') and 
is_configured('libcxx_libs_dir'):
+  dotest_cmd += ['--libcxx-include-dir', config.libcxx_include_dir]

rupprecht wrote:
> fdeazeve wrote:
> > Isn't this always going to return true? (same for `is_configured 
> > (libcxx_include_target_dir)`).
> > 
> > `is_configured` returns the empty string when the CMake variables are 
> > empty, and the empty string converts to True
> Since this is a python file, it should follow the python rules where an empty 
> string is treated as false:
> 
> ```
> $ python3
> Python 3.10.6 (main, Aug 10 2022, 11:19:32) [GCC 11.3.0] on linux
> Type "help", "copyright", "credits" or "license" for more information.
> >>> x = ''
> >>> 'foo' if x else 'bar'
> 'bar'
> ```
Oh, my bad. I had been bitten by `if('0')` in a similar context recently, so I 
was a bit paranoid here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Variant is supported from:
libstdc++ 7.1
VS 2017 15.0
libcxx 4.0

So far so good relative to 
https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library.

https://en.cppreference.com/w/cpp/compiler_support/17#C.2B.2B17_library_features
 claims it is in apple clang 10.0.0 so unfortunately we're not quite there as 
the minimum is 9.3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134041

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

[Lldb-commits] [PATCH] D133961: [lldb] Use SWIG_fail in python-typemaps.swig (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e450d134755: [lldb] Use SWIG_fail in python-typemaps.swig 
(NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133961

Files:
  lldb/bindings/python/python-typemaps.swig

Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -18,7 +18,7 @@
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
 free($1);
-return nullptr;
+SWIG_fail;
   }
 
   $1[i] = const_cast(py_str.GetString().data());
@@ -28,7 +28,7 @@
 $1 = NULL;
   } else {
 PyErr_SetString(PyExc_TypeError, "not a list");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -70,7 +70,7 @@
   PythonObject obj = Retain($input);
   lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
-return nullptr;
+SWIG_fail;
   $1 = value;
 }
 
@@ -79,10 +79,10 @@
   unsigned long long state_type_value =
   unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
-return nullptr;
+SWIG_fail;
   if (state_type_value > lldb::StateType::kLastStateType) {
 PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
-return nullptr;
+SWIG_fail;
   }
   $1 = static_cast(state_type_value);
 }
@@ -93,12 +93,12 @@
 %typemap(in) (char *dst, size_t dst_len) {
   if (!PyInt_Check($input)) {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer");
-return NULL;
+SWIG_fail;
   }
   $2 = PyInt_AsLong($input);
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (char *)malloc($2);
 }
@@ -129,12 +129,12 @@
 %typemap(in) (char *dst_or_null, size_t dst_len) {
   if (!PyInt_Check($input)) {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer");
-return NULL;
+SWIG_fail;
   }
   $2 = PyInt_AsLong($input);
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (char *)malloc($2);
 }
@@ -166,7 +166,7 @@
 $2 = bytes.GetSize();
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting a string");
-return NULL;
+SWIG_fail;
   }
 }
 // For SBProcess::WriteMemory, SBTarget::GetInstructions and SBDebugger::DispatchInput.
@@ -186,7 +186,7 @@
 $2 = bytes.GetSize();
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting a buffer");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -199,11 +199,11 @@
 $2 = PyLong_AsLong($input);
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer or long object");
-return NULL;
+SWIG_fail;
   }
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (void *)malloc($2);
 }
@@ -287,12 +287,12 @@
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
 free($1);
-return NULL;
+SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
 free($1);
-return NULL;
+SWIG_fail;
   }
 }
   } else if ($input == Py_None) {
@@ -300,7 +300,7 @@
 $2 = 0;
   } else {
 PyErr_SetString(PyExc_TypeError, "not a list");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -353,7 +353,7 @@
   if (!($input == Py_None ||
 PyCallable_Check(reinterpret_cast($input {
 PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
-return NULL;
+SWIG_fail;
   }
 
   // FIXME (filcab): We can't currently check if our callback is already
@@ -377,11 +377,11 @@
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
 PyErr_SetString(PyExc_TypeError, "not a file");
-return nullptr;
+SWIG_fail;
   }
   auto sp = unwrapOrSetPythonException(py_file.ConvertToFile());
   if (!sp)
-return nullptr;
+SWIG_fail;
   $1 = sp;
 }
 
@@ -389,12 +389,12 @@
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
 PyErr_SetString(PyExc_TypeError, "not a file");
-return nullptr;
+SWIG_fail;
   }
   auto sp = unwrapOrSetPythonException(
   py_file.ConvertToFileForcingUseOfScriptingIOMethods());
   if (!sp)
-return nullptr;
+SWIG_fail;
   $1 = sp;
 }
 
@@ -402,12 +402,12 @@
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
 PyErr_SetString(PyExc_TypeError, "not a file");
-return nullptr;
+SWIG_fail;
   }
   auto sp =
   unwrapOrSetPythonException(py_file.ConvertToFile(/*borrowed=*/true));
   if (!sp)
-return nullptr;
+SWIG_fail;
   $1 = sp;
 }
 
@@ -415,12 +415,12 @@
   PythonFile py_file(PyRefType::Borrowed, $input);
   i

[Lldb-commits] [lldb] 6e450d1 - [lldb] Use SWIG_fail in python-typemaps.swig (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-09-16T08:25:01-07:00
New Revision: 6e450d134755c41bd4ac0ba356e31fc37ba88757

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

LOG: [lldb] Use SWIG_fail in python-typemaps.swig (NFC)

When attempting to use SWIG's `-builtin` flag, there were a few compile
failures caused by a mismatch between return type and return value. In those
cases, the return type was `int` but many of the type maps assume returning
`NULL`/`nullptr` (only the latter caused compile failures).

This fix abstracts failure paths to use the `SWIG_fail` macro, which performs
`goto fail;`. Each of the generated functions contain a `fail` label, which
performs any resource cleanup and returns the appropriate failure value.

This change isn't strictly necessary at this point, but seems like the right
thing to do, and for anyone who tries `-builtin` later, it resolves those
issues.

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

Added: 


Modified: 
lldb/bindings/python/python-typemaps.swig

Removed: 




diff  --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index bf3de66b91bf1..203be803d2ebd 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -18,7 +18,7 @@
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
 free($1);
-return nullptr;
+SWIG_fail;
   }
 
   $1[i] = const_cast(py_str.GetString().data());
@@ -28,7 +28,7 @@
 $1 = NULL;
   } else {
 PyErr_SetString(PyExc_TypeError, "not a list");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -70,7 +70,7 @@
   PythonObject obj = Retain($input);
   lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
-return nullptr;
+SWIG_fail;
   $1 = value;
 }
 
@@ -79,10 +79,10 @@
   unsigned long long state_type_value =
   unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
-return nullptr;
+SWIG_fail;
   if (state_type_value > lldb::StateType::kLastStateType) {
 PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
-return nullptr;
+SWIG_fail;
   }
   $1 = static_cast(state_type_value);
 }
@@ -93,12 +93,12 @@
 %typemap(in) (char *dst, size_t dst_len) {
   if (!PyInt_Check($input)) {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer");
-return NULL;
+SWIG_fail;
   }
   $2 = PyInt_AsLong($input);
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (char *)malloc($2);
 }
@@ -129,12 +129,12 @@
 %typemap(in) (char *dst_or_null, size_t dst_len) {
   if (!PyInt_Check($input)) {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer");
-return NULL;
+SWIG_fail;
   }
   $2 = PyInt_AsLong($input);
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (char *)malloc($2);
 }
@@ -166,7 +166,7 @@
 $2 = bytes.GetSize();
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting a string");
-return NULL;
+SWIG_fail;
   }
 }
 // For SBProcess::WriteMemory, SBTarget::GetInstructions and 
SBDebugger::DispatchInput.
@@ -186,7 +186,7 @@
 $2 = bytes.GetSize();
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting a buffer");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -199,11 +199,11 @@
 $2 = PyLong_AsLong($input);
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer or long object");
-return NULL;
+SWIG_fail;
   }
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (void *)malloc($2);
 }
@@ -287,12 +287,12 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
 free($1);
-return NULL;
+SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
 free($1);
-return NULL;
+SWIG_fail;
   }
 }
   } else if ($input == Py_None) {
@@ -300,7 +300,7 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
 $2 = 0;
   } else {
 PyErr_SetString(PyExc_TypeError, "not a list");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -353,7 +353,7 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   if (!($input == Py_None ||
 PyCallable_Check(reinterpret_cast($input {
 PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
-return NULL;
+SWIG_fail;
   }
 
   // FIXME (filcab): We can't currently check if 

[Lldb-commits] [PATCH] D133259: [lldb] Don't assume name of libc++ inline namespace in LibCxxUnorderedMap

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp:70
 
+static void consumeNamespace(llvm::StringRef &name) {
+  // Delete past an inline namespace, if any: __[a-zA-Z0-9_]+::

JDevlieghere wrote:
> rupprecht wrote:
> > nit: this consumes just the inline namespace, so I think 
> > `consumeInlineNamespace` might be a better name. I don't feel that strongly 
> > though so I'll leave it up to you.
> Rather than modifying the passed-in string, would it make sense to return a 
> `llvm::StringRef`? 
@JDevlieghere My answer is twofold: It follows the  "consume" APIs on 
StringRef. And I prefer to avoid find expressions where you assign to a var 
being used on the right side:

```
someVar = someVar.method(...)
```

If you'd like me to change it, I will.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp:70
 
+static void consumeNamespace(llvm::StringRef &name) {
+  // Delete past an inline namespace, if any: __[a-zA-Z0-9_]+::

kastiglione wrote:
> JDevlieghere wrote:
> > rupprecht wrote:
> > > nit: this consumes just the inline namespace, so I think 
> > > `consumeInlineNamespace` might be a better name. I don't feel that 
> > > strongly though so I'll leave it up to you.
> > Rather than modifying the passed-in string, would it make sense to return a 
> > `llvm::StringRef`? 
> @JDevlieghere My answer is twofold: It follows the  "consume" APIs on 
> StringRef. And I prefer to avoid find expressions where you assign to a var 
> being used on the right side:
> 
> ```
> someVar = someVar.method(...)
> ```
> 
> If you'd like me to change it, I will.
@rupprecht I agree, `consumeInlineNamespace` it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133259

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

[Lldb-commits] [PATCH] D133259: [lldb] Don't assume name of libc++ inline namespace in LibCxxUnorderedMap

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 460778.
kastiglione added a comment.

Rename to consumeInlineNamespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133259

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -67,11 +67,23 @@
   return m_num_elements;
 }
 
+static void consumeInlineNamespace(llvm::StringRef &name) {
+  // Delete past an inline namespace, if any: __[a-zA-Z0-9_]+::
+  auto scratch = name;
+  if (scratch.consume_front("__") && std::isalnum(scratch[0])) {
+scratch = scratch.drop_while([](char c) { return std::isalnum(c); });
+if (scratch.consume_front("::")) {
+  // Successfully consumed a namespace.
+  name = scratch;
+}
+  }
+}
+
 static bool isStdTemplate(ConstString type_name, llvm::StringRef type) {
   llvm::StringRef name = type_name.GetStringRef();
-  // The type name may or may not be prefixed with `std::` or `std::__1::`.
+  // The type name may be prefixed with `std::__::`.
   if (name.consume_front("std::"))
-name.consume_front("__1::");
+consumeInlineNamespace(name);
   return name.consume_front(type) && name.startswith("<");
 }
 


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -67,11 +67,23 @@
   return m_num_elements;
 }
 
+static void consumeInlineNamespace(llvm::StringRef &name) {
+  // Delete past an inline namespace, if any: __[a-zA-Z0-9_]+::
+  auto scratch = name;
+  if (scratch.consume_front("__") && std::isalnum(scratch[0])) {
+scratch = scratch.drop_while([](char c) { return std::isalnum(c); });
+if (scratch.consume_front("::")) {
+  // Successfully consumed a namespace.
+  name = scratch;
+}
+  }
+}
+
 static bool isStdTemplate(ConstString type_name, llvm::StringRef type) {
   llvm::StringRef name = type_name.GetStringRef();
-  // The type name may or may not be prefixed with `std::` or `std::__1::`.
+  // The type name may be prefixed with `std::__::`.
   if (name.consume_front("std::"))
-name.consume_front("__1::");
+consumeInlineNamespace(name);
   return name.consume_front(type) && name.startswith("<");
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3004a75 - [lldb][tests][gmodules] Test for expression evaluator crash for types referencing the same template

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-09-16T12:38:49-04:00
New Revision: 3004a759bcdf7b0a2a3a4220ed216d20defbd081

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

LOG: [lldb][tests][gmodules] Test for expression evaluator crash for types 
referencing the same template

The problem here is that the ASTImporter adds
the template class member FieldDecl to
the DeclContext twice. This happens because
we don't construct a `LookupPtr` for decls
that originate from modules and thus the
ASTImporter never realizes that the FieldDecl
has already been imported. These duplicate
decls then break the assumption of the LayoutBuilder
which expects only a single member decl to
exist.

The test will be fixed by a follow-up revision
and is thus skipped for now.

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

Added: 
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/Makefile

lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.cpp
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.h
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/main.cpp
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module.modulemap
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.cpp
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.h
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.cpp
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.h

Modified: 


Removed: 




diff  --git a/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/Makefile 
b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/Makefile
new file mode 100644
index 0..2d13591d6ccd7
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES   := main.cpp module1.cpp module2.cpp base_module.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
 
b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
new file mode 100644
index 0..0ad4603651a18
--- /dev/null
+++ 
b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -0,0 +1,72 @@
+"""
+Tests the scenario where we evaluate expressions
+of two types in 
diff erent modules that reference
+a class template instantiated with the same
+template argument.
+
+Note that,
+1. Since the decls originate from modules, LLDB
+   marks them as such and Clang doesn't create
+   a LookupPtr map on the corresponding DeclContext.
+   This prevents regular DeclContext::lookup from
+   succeeding.
+2. Because we reference the same class template
+   from two 
diff erent modules we get a redeclaration
+   chain for the class's ClassTemplateSpecializationDecl.
+   The importer will import all FieldDecls into the
+   same DeclContext on the redeclaration chain. If
+   we don't do the bookkeeping correctly we end up
+   with duplicate decls on the same DeclContext leading
+   to crashes down the line.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTemplateWithSameArg(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+
+@add_test_categories(["gmodules"])
+@skipIf(bugnumber='rdar://96581048')
+def test_same_template_arg(self):
+lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
+
+self.expect_expr("FromMod1", result_type="ClassInMod1", 
result_children=[
+ValueCheck(name="VecInMod1", children=[
+ValueCheck(name="Member", value="137")
+])
+])
+
+self.expect_expr("FromMod2", result_type="ClassInMod2", 
result_children=[
+ValueCheck(name="VecInMod2", children=[
+ValueCheck(name="Member", value="42")
+])
+])
+
+@add_test_categories(["gmodules"])
+@skipIf(bugnumber='rdar://96581048')
+def test_duplicate_decls(self):
+lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
+
+self.expect_expr("(intptr_t)&FromMod1 + (intptr_t)&FromMod2")
+
+# Make sure we only have a single 'Member' decl on the AST
+self.filecheck("target module dump ast", __file__)
+# CHECK:  ClassTemplateSpecializationDecl {{.*}} imported in Module2 
struct ClassInMod3 definition
+# CHECK-NEXT: |-DefinitionData pass_in_registers aggregate standard_layout 
trivially_co

[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe456d2ba8bca: [clang][ASTImporter] 
DeclContext::localUncachedLookup: Continue lookup into… (authored by 
Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -34,7 +34,6 @@
 self.main_source_file = lldb.SBFileSpec("main.cpp")
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_template_arg(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
 
@@ -51,7 +50,6 @@
 ])
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_duplicate_decls(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.


Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -34,7 +34,6 @@
 self.main_source_file = lldb.SBFileSpec("main.cpp")
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_template_arg(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", self.main_source_file)
 
@@ -51,7 +50,6 @@
 ])
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_duplicate_decls(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", self.main_source_file)
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e456d2b - [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-09-16T12:38:50-04:00
New Revision: e456d2ba8bcadaa63386b339a4f2abcb39505502

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

LOG: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup 
into decl chain when regular lookup fails

The uncached lookup is mainly used in the ASTImporter/LLDB code-path
where we're not allowed to load from external storage. When importing
a FieldDecl with a DeclContext that had no external visible storage
(but came from a Clang module or PCH) the above call to `lookup(Name)`
the regular `DeclContext::lookup` fails because:
1. `DeclContext::buildLookup` doesn't set `LookupPtr` for decls
   that came from a module
2. LLDB doesn't use the `SharedImporterState`

In such a case we would never continue with the "slow" path of iterating
through the decl chain on the DeclContext. In some cases this means that
ASTNodeImporter::VisitFieldDecl ends up importing a decl into the
DeclContext a second time.

The patch removes the short-circuit in the case where we don't find
any decls via the regular lookup.

**Tests**

* Un-skip the failing LLDB API tests

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

Added: 


Modified: 
clang/lib/AST/DeclBase.cpp
clang/unittests/AST/ASTImporterTest.cpp

lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py

Removed: 




diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index f22bf6b0937ee..837ff90d6e34f 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@ void DeclContext::localUncachedLookup(DeclarationName 
Name,
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index d9b7883119345..86a96305bed11 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@ TEST_P(ASTImporterLookupTableTest,
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);

diff  --git 
a/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
 
b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
index 0ad4603651a18..b4aec8ff95e4b 100644
--- 
a/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
+++ 
b/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -34,7 +34,6 @@ def setUp(self):
 self.main_source_file = lldb.SBFileSpec("main.cpp")
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_template_arg(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
 
@@ -51,7 +50,6 @@ def test_same_template_arg(self):
 ])
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_duplicate_decls(self):
 lldbutil.run_to_source_breakpoint(self, "Break here", 
self.main_source_file)
 



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

[Lldb-commits] [PATCH] D133944: [lldb][tests][gmodules] Test for expression evaluator crash for types that reference same template class

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3004a759bcdf: [lldb][tests][gmodules] Test for expression 
evaluator crash for types… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133944

Files:
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/Makefile
  
lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.h
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/main.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module.modulemap
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.h
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.cpp
  lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.h

Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.h
@@ -0,0 +1,10 @@
+#ifndef MOD2_H_IN
+#define MOD2_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod2 {
+  ClassInMod3 VecInMod2;
+};
+
+#endif
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module2.cpp
@@ -0,0 +1,3 @@
+#include "module2.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.h
@@ -0,0 +1,10 @@
+#ifndef MOD1_H_IN
+#define MOD1_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod1 {
+  ClassInMod3 VecInMod1;
+};
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module1.cpp
@@ -0,0 +1,3 @@
+#include "module1.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module.modulemap
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/module.modulemap
@@ -0,0 +1,14 @@
+module Module1 {
+  header "module1.h"
+  export *
+}
+
+module Module2 {
+  header "module2.h"
+  export *
+}
+
+module BaseModule {
+  header "base_module.h"
+  export *
+}
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/main.cpp
@@ -0,0 +1,15 @@
+#include "module1.h"
+#include "module2.h"
+
+#include 
+
+int main() {
+  ClassInMod1 FromMod1;
+  ClassInMod2 FromMod2;
+
+  FromMod1.VecInMod1.Member = 137;
+  FromMod2.VecInMod2.Member = 42;
+
+  std::puts("Break here");
+  return 0;
+}
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.h
@@ -0,0 +1,6 @@
+#ifndef MOD3_H_IN
+#define MOD3_H_IN
+
+template  struct ClassInMod3 { int Member = 0; };
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/base_module.cpp
@@ -0,0 +1,3 @@
+#include "base_module.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
@@ -0,0 +1,72 @@
+"""
+Tests the scenario where we evaluate expressions
+of two types in different modules that reference
+a class template instantiated with the same
+template argument.
+
+Note that,
+1. Since the decls originate from modules, LLDB
+   marks them as such and Clang doesn't create
+   a LookupPtr map on the corresponding DeclContext.
+   This prevents regular DeclContext::lookup from
+   succeeding.
+2. Because we reference the same class template
+   from two different modules we get a redeclaration
+   chain for the class's ClassTemplateSpecializationDecl.
+   The importer will import all FieldDecls into the
+   same DeclContext on the redeclaration chain. If
+   we don't do the bookkeeping correctly we end 

[Lldb-commits] [lldb] d21b417 - [LLDB][NativePDB] ResolveSymbolContext should return the innermost block

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-09-16T10:19:09-07:00
New Revision: d21b417025f8051638f467007af957f2ed9f614a

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

LOG: [LLDB][NativePDB] ResolveSymbolContext should return the innermost block

Before, it returns the outermost blocks if nested blocks have the same
address range. That casuses lldb unable to find variables that are inside
inner blocks.

Reviewed By: labath

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

Added: 
lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s

Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 3bcebb6d46f63..304b649043522 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1054,8 +1054,15 @@ uint32_t SymbolFileNativePDB::ResolveSymbolContext(
   }
 
   if (type == PDB_SymType::Block) {
-sc.block = &GetOrCreateBlock(csid);
-sc.function = sc.block->CalculateSymbolContextFunction();
+Block &block = GetOrCreateBlock(csid);
+sc.function = block.CalculateSymbolContextFunction();
+if (sc.function) {
+  sc.function->GetBlock(true);
+  addr_t func_base =
+  sc.function->GetAddressRange().GetBaseAddress().GetFileAddress();
+  addr_t offset = file_addr - func_base;
+  sc.block = block.FindInnermostBlockByOffset(offset);
+}
   }
   if (sc.function)
 resolved_flags |= eSymbolContextFunction;

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s 
b/lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s
new file mode 100644
index 0..0ab76dacdaded
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s
@@ -0,0 +1,397 @@
+# clang-format off
+# REQUIRES: lld, x86
+
+# Test when nested S_BLOCK32 have same address range, ResolveSymbolContext 
should return the innnermost block.
+# RUN: llvm-mc -triple=x86_64-windows-msvc --filetype=obj %s > %t.obj
+# RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe 
/base:0x14000
+# RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o "image lookup -a 
0x14000103c -v" -o "exit" | FileCheck %s
+
+# This file is compiled from following source file:
+# $ clang-cl /Z7 /GS- /c /O2 test.cpp /Fatest.s
+# __attribute__((optnone)) bool func(const char* cp, volatile char p[]) {
+#   return false;
+# }
+#
+# int main() {
+#   const char* const kMfDLLs[] = {"a"};
+#   asm("nop");
+#   for (const char* kMfDLL : kMfDLLs) {
+# volatile char path[10] = {0};
+# if (func(kMfDLL, path))
+#   break;
+#   }
+#   return 0;
+# }
+
+# CHECK:   Function: id = {{.*}}, name = "main", range = 
[0x000140001020-0x00014000104d)
+# CHECK-NEXT:  FuncType: id = {{.*}}, byte-size = 0, compiler_type = "int 
(void)"
+# CHECK-NEXT:Blocks: id = {{.*}}, range = [0x140001020-0x14000104d)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT: LineEntry: [0x000140001035-0x000140001046): 
/tmp/test.cpp:10
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "path", type = "volatile 
char[10]", valid ranges = , location = [0x000140001025, 
0x000140001046) -> DW_OP_breg7 RSP+40, decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "kMfDLL", type = "const char *", 
valid ranges = , location = [0x00014000103c, 0x000140001046) -> 
DW_OP_reg2 RCX, decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__range1", type = "const char 
*const (&)[1]", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__begin1", type = "const char 
*const *", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__end1", type = "const char 
*const *", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "kMfDLLs", type = "const char 
*const[1]", valid ranges = , location = [0x00014000103c, 
0x000140001046) -> DW_OP_reg2 RCX, decl =
+
+
+   .text
+   .def@feat.00;
+   .scl3;
+   .type   0;
+   .endef
+   .globl  @feat.00
+.set @feat.00, 0
+   .intel_syntax noprefix
+   .file   "test.cpp"
+   .def"?func@@YA_NPEBDQECD@Z";
+   .scl2;
+   .type   32;
+   .endef
+   .section.text,"xr",one_only,"?func@@YA_NPEBDQECD@Z"
+   .globl  "?func@

[Lldb-commits] [PATCH] D133601: [LLDB][NativePDB] ResolveSymbolContext should return the innermost block

[Zequan Wu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd21b417025f8: [LLDB][NativePDB] ResolveSymbolContext should 
return the innermost block (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133601

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s

Index: lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s
@@ -0,0 +1,397 @@
+# clang-format off
+# REQUIRES: lld, x86
+
+# Test when nested S_BLOCK32 have same address range, ResolveSymbolContext should return the innnermost block.
+# RUN: llvm-mc -triple=x86_64-windows-msvc --filetype=obj %s > %t.obj
+# RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe /base:0x14000
+# RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o "image lookup -a 0x14000103c -v" -o "exit" | FileCheck %s
+
+# This file is compiled from following source file:
+# $ clang-cl /Z7 /GS- /c /O2 test.cpp /Fatest.s
+# __attribute__((optnone)) bool func(const char* cp, volatile char p[]) {
+#   return false;
+# }
+#
+# int main() {
+#   const char* const kMfDLLs[] = {"a"};
+#   asm("nop");
+#   for (const char* kMfDLL : kMfDLLs) {
+# volatile char path[10] = {0};
+# if (func(kMfDLL, path))
+#   break;
+#   }
+#   return 0;
+# }
+
+# CHECK:   Function: id = {{.*}}, name = "main", range = [0x000140001020-0x00014000104d)
+# CHECK-NEXT:  FuncType: id = {{.*}}, byte-size = 0, compiler_type = "int (void)"
+# CHECK-NEXT:Blocks: id = {{.*}}, range = [0x140001020-0x14000104d)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT: LineEntry: [0x000140001035-0x000140001046): /tmp/test.cpp:10
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "path", type = "volatile char[10]", valid ranges = , location = [0x000140001025, 0x000140001046) -> DW_OP_breg7 RSP+40, decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "kMfDLL", type = "const char *", valid ranges = , location = [0x00014000103c, 0x000140001046) -> DW_OP_reg2 RCX, decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__range1", type = "const char *const (&)[1]", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__begin1", type = "const char *const *", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__end1", type = "const char *const *", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "kMfDLLs", type = "const char *const[1]", valid ranges = , location = [0x00014000103c, 0x000140001046) -> DW_OP_reg2 RCX, decl =
+
+
+	.text
+	.def	@feat.00;
+	.scl	3;
+	.type	0;
+	.endef
+	.globl	@feat.00
+.set @feat.00, 0
+	.intel_syntax noprefix
+	.file	"test.cpp"
+	.def	"?func@@YA_NPEBDQECD@Z";
+	.scl	2;
+	.type	32;
+	.endef
+	.section	.text,"xr",one_only,"?func@@YA_NPEBDQECD@Z"
+	.globl	"?func@@YA_NPEBDQECD@Z" # -- Begin function ?func@@YA_NPEBDQECD@Z
+	.p2align	4, 0x90
+"?func@@YA_NPEBDQECD@Z":# @"?func@@YA_NPEBDQECD@Z"
+.Lfunc_begin0:
+	.cv_func_id 0
+	.cv_file	1 "/tmp/test.cpp" "8CDAA03EE93954606427F9B409CE7638" 1
+	.cv_loc	0 1 1 0 # test.cpp:1:0
+.seh_proc "?func@@YA_NPEBDQECD@Z"
+# %bb.0:# %entry
+	sub	rsp, 16
+	.seh_stackalloc 16
+	.seh_endprologue
+	mov	qword ptr [rsp + 8], rdx
+	mov	qword ptr [rsp], rcx
+.Ltmp0:
+	.cv_loc	0 1 2 0 # test.cpp:2:0
+	xor	eax, eax
+	and	al, 1
+	movzx	eax, al
+	add	rsp, 16
+	ret
+.Ltmp1:
+.Lfunc_end0:
+	.seh_endproc
+# -- End function
+	.def	main;
+	.scl	2;
+	.type	32;
+	.endef
+	.section	.text,"xr",one_only,main
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+main:   # @main
+.Lfunc_begin1:
+	.cv_func_id 1
+	.cv_loc	1 1 5 0 # test.cpp:5:0
+.seh_proc main
+# %bb.0:# %entry
+	sub	rsp, 56
+	.seh_stackalloc 56
+	.seh_endprologue
+.Ltmp2:
+	.cv_loc	1 1 7 0 # test.cpp:7:0
+	#APP
+	nop
+	#NO_APP
+.Ltmp3:
+	#DEBUG_VALUE: __range1 <- undef
+	#DEBUG_VALUE: __begin1 <- undef
+	#DEBUG_VALUE: __end1 <- [DW_OP_plus_uconst 8, DW_OP_stack_value] undef
+	.cv_loc	1 1 9 0 # test.cpp:9:0
+	mov	word ptr [rsp + 48], 0
+	mov	qword ptr [rsp + 40], 0
+	.cv_loc	1 1 10 0# test.cpp:10:0
+	lea	rcx, [rip + "??_C@_01MCMALHOG@a?$AA@"]
+.Ltmp4:
+	#DEBUG_VALUE: main:k

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The more we gather up the "things you have to do before an operation starts" or 
after it ends, etc. into WillDoOperation methods, the less ad hoc code you have 
in the callers that you have to remember to copy over if you are introducing a 
different way of doing the same operation.  It also expresses when the 
operation needs to be done, which if you just had a call out in Process::Attach 
or whatever you would not know.  Either way will work, but I think packaging 
these "do before" and "do after" tasks into methods makes everything clearer 
and easier to maintain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

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

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

[Jim Ingham via Phabricator via lldb-commits]

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

This version looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

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

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

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

This LGTM and it worked without issues when I ran some tests on my machine, but 
please let @JDevlieghere have another look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

We never made any guarantee about the order threads would be listed in the 
array returned by GetThreadList.  I'm not sure it would be worth trying to do 
that, because often you don't know whether a thread's public StopInfo will be 
valid even if the private stop info is.  For instance, if a thread hits a 
thread specific breakpoint for a thread that isn't in the breakpoint's thread 
specifier, then we reset the stop info back to an empty one because formally 
that thread didn't hit our breakpoint...  So this change is clearly right.

I don't suppose there's a way to write a test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

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

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu created this revision.
zequanwu added reviewers: labath, martong, rnk.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When a record type doesn't have complete type debug info, forcefully complete it
to make clang not crashing at places calling `CXXRecordDecl::data()`.

Here's an example when a class has incomplete type debug info.

  0x17AB1 | LF_CLASS [size = 48] `v8::Data`
unique name: `.?AVData@v8@@`
vtable: , base list: , field list: 
options: forward ref (= 0x17AB1) | has unique name, sizeof 0


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134066

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/test/Shell/SymbolFile/NativePDB/missing-type.s

Index: lldb/test/Shell/SymbolFile/NativePDB/missing-type.s
===
--- lldb/test/Shell/SymbolFile/NativePDB/missing-type.s
+++ lldb/test/Shell/SymbolFile/NativePDB/missing-type.s
@@ -1,14 +1,18 @@
 # clang-format off
 # REQUIRES: lld, x86
 
-# Test when type index is missing in FieldList.
+# Test when class type is incomplete or type index is missing in FieldList.
 # RUN: llvm-mc -triple=x86_64-windows-msvc --filetype=obj %s > %t.obj
-# RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe /base:0x14000
-# RUN: lldb-test symbols --find=type --name=S %t.exe | FileCheck %s
+# RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe
+# RUN: lldb-test symbols --find=type --name=S %t.exe | FileCheck %s --check-prefix=MISSING-DATA-TYPE
+# RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o "expr a" -o "exit" \
+# RUN: | FileCheck %s --check-prefix=IMCOMPLETE-CLASS
 
-# CHECK:  name = "S", size = 4, compiler_type = {{.*}} struct S {
-# CHECK-NEXT: }
+# MISSING-DATA-TYPE:  name = "S", size = 4, compiler_type = {{.*}} struct S {
+# MISSING-DATA-TYPE-NEXT: }
 
+# IMCOMPLETE-CLASS:  (lldb) expr a
+# IMCOMPLETE-CLASS-NEXT: (A) $0 = {}
 
 
 	.text
@@ -43,10 +47,54 @@
 .Lfunc_end0:
 	.seh_endproc
 # -- End function
-	.section	.drectve,"yn"
-.Ltmp25:
+	.bss
+	.globl	"?a@@3VA@@A"# @"?a@@3VA@@A"
+	.p2align	2, 0x0
+"?a@@3VA@@A":
+	.zero	4
+	.section	.debug$S,"dr"
+	.p2align	2, 0x0
+	.long	4   # Debug section magic
+	.long	241
+	.long	.Ltmp3-.Ltmp2   # Subsection size
+.Ltmp2:
+	.short	.Ltmp3-.Ltmp6   # Record length
+.Ltmp6:
+	.short	4412# Record kind: S_COMPILE3
+	.long	1   # Flags and language
+	.short	208 # CPUType
+	.short	16  # Frontend version
+	.short	0
+	.short	0
+	.short	0
+	.short	16000   # Backend version
+	.short	0
+	.short	0
+	.short	0
+	.asciz	"clang version 16.0.0"  # Null-terminated compiler version string
+	.p2align	2, 0x0
+.Ltmp3:
+	.p2align	2, 0x0
+	.long	241 # Symbol subsection for main
+	.long   0
+	.p2align	2, 0x0
+	.cv_linetable	0, main, .Lfunc_end0
+	.long	241 # Symbol subsection for globals
+	.long	.Ltmp19-.Ltmp14 # Subsection size
+.Ltmp14:
+	.short	.Ltmp19-.Ltmp16 # Record length
+.Ltmp16:
+	.short	4365# Record kind: S_GDATA32
+	.long	4103# Type
+	.secrel32	"?a@@3VA@@A"# DataOffset
+	.secidx	"?a@@3VA@@A"# Segment
+	.asciz	"a" # Name
+	.p2align	2, 0x0
+
+.Ltmp19:
+	.p2align	2, 0x0
 	.section	.debug$T,"dr"
-	.p2align	2
+	.p2align	2, 0x0
 	.long	4   # Debug section magic
 	# Pointer (0x1000)
 	.short	0xa # Record length
@@ -87,6 +135,7 @@
 	.short	0x0 # SizeOf
 	.asciz	"S" # Name
 	.asciz	".?AUS@@"   # LinkageName
+	# Test when a class member has missing type info.
 	# FieldList (0x1005)
 	.short	0xe # Record length
 	.short	0x1203  # Record kind: LF_FIELDLIST
@@ -106,3 +155,15 @@
 	.short	0x4 # SizeOf
 	.asciz	"S" # Name
 	.asciz	".?AUS@@"   # LinkageName
+	# Test when class doesn't have full type info (forward reference to itself).
+# Class (0x1007)
+	.short	0x1e# Record length
+	.short	0x1504  # Record kind: LF_CLASS
+	.short	0x1 # MemberCount
+	.short	0x280   # Properties ( ForwardReference (0x80) | HasUniqueName (0x200) )
+	.long	0x0 # FieldList: 
+	.long	0x0   

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D134037#3796251 , @jingham wrote:

> We never made any guarantee about the order threads would be listed in the 
> array returned by GetThreadList.  I'm not sure it would be worth trying to do 
> that, because often you don't know whether a thread's public StopInfo will be 
> valid even if the private stop info is.  For instance, if a thread hits a 
> thread specific breakpoint for a thread that isn't in the breakpoint's thread 
> specifier, then we reset the stop info back to an empty one because formally 
> that thread didn't hit our breakpoint...  So this change is clearly right.
>
> I don't suppose there's a way to write a test for this?

Actually, yes, it is testable fairly easily - by firing up a test example with 
2 threads and hitting a crash or breakpoint in either of them, and running it 
with a `-k` option. Before this fix, the test would still pass maybe half of 
the time, but now it should pass reliably.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

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

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via Phabricator via lldb-commits]

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

Added a testcase. I'm not sure if this is the most accurate/correct place for 
the test, but the name is at least verbose enough to describe what it exercises.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
  lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp


Index: lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
===
--- /dev/null
+++ lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
@@ -0,0 +1,13 @@
+#include 
+
+void t_func() {
+  asm volatile(
+"int3\n\t"
+  );
+}
+
+int main() {
+  std::thread t(t_func);
+  t.join();
+  return 0;
+}
Index: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
@@ -0,0 +1,6 @@
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/CommandOnCrashMultiThreaded.cpp -o %t -pthread
+# RUN: %lldb -b -o "process launch" -k "process continue" -k "exit" %t | 
FileCheck %s
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2441,7 +2441,7 @@
   for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
 StopInfoSP stop_info = thread_sp->GetStopInfo();
 if (!stop_info)
-  return false;
+  continue;
 
 const StopReason reason = stop_info->GetStopReason();
 if (reason == eStopReasonException ||


Index: lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
===
--- /dev/null
+++ lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
@@ -0,0 +1,13 @@
+#include 
+
+void t_func() {
+  asm volatile(
+"int3\n\t"
+  );
+}
+
+int main() {
+  std::thread t(t_func);
+  t.join();
+  return 0;
+}
Index: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
@@ -0,0 +1,6 @@
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/CommandOnCrashMultiThreaded.cpp -o %t -pthread
+# RUN: %lldb -b -o "process launch" -k "process continue" -k "exit" %t | FileCheck %s
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2441,7 +2441,7 @@
   for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
 StopInfoSP stop_info = thread_sp->GetStopInfo();
 if (!stop_info)
-  return false;
+  continue;
 
 const StopReason reason = stop_info->GetStopReason();
 if (reason == eStopReasonException ||
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

[Reid Kleckner via Phabricator via lldb-commits]

rnk added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:681
+auto &ts = llvm::cast(*ct.GetTypeSystem());
+ts.GetMetadata(&tag)->SetIsForcefullyCompleted();
+  }

Is this what we do for DWARF? The same kind of situation seems like it can 
arise, where clang requires a type to be complete, but the information is 
missing, and we still try to proceed with evaluation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

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

[Lldb-commits] [PATCH] D133876: [lldb][tests][NFC] Move C++ gmodules tests into new gmodules/ subdirectory

[Douglas Yung via Phabricator via lldb-commits]

dyung added a comment.

@Michael137, your change is causing cmake to fail on one of our bots:

https://lab.llvm.org/buildbot/#/builders/217/builds/11819

  CMake Error at cmake/modules/AddLLVM.cmake:1867 (add_custom_target):
add_custom_target cannot create target
"check-lldb-api-lang-cpp-gmodules-templates" because another target with
the same name already exists.  The existing target is a custom target
created in source directory

"/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API".
See documentation for policy CMP0002 for more details.
  Call Stack (most recent call first):
cmake/modules/AddLLVM.cmake:1980 (add_lit_target)

/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API/CMakeLists.txt:4
 (add_lit_testsuites)

Can you take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133876

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

[Lldb-commits] [PATCH] D133876: [lldb][tests][NFC] Move C++ gmodules tests into new gmodules/ subdirectory

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D133876#3796847 , @dyung wrote:

> @Michael137, your change is causing cmake to fail on one of our bots:
>
> https://lab.llvm.org/buildbot/#/builders/217/builds/11819
>
>   CMake Error at cmake/modules/AddLLVM.cmake:1867 (add_custom_target):
> add_custom_target cannot create target
> "check-lldb-api-lang-cpp-gmodules-templates" because another target with
> the same name already exists.  The existing target is a custom target
> created in source directory
> 
> "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API".
> See documentation for policy CMP0002 for more details.
>   Call Stack (most recent call first):
> cmake/modules/AddLLVM.cmake:1980 (add_lit_target)
> 
> /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API/CMakeLists.txt:4
>  (add_lit_testsuites)
>
> Can you take a look?

Here I'm just renaming a directory. @dyung I think this might have to do with a 
stale `__pycache__` directory in the test directory. Can you force a fresh 
checkout on the build bot and restart?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133876

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

Re: [Lldb-commits] [PATCH] D133876: [lldb][tests][NFC] Move C++ gmodules tests into new gmodules/ subdirectory

[Jim Ingham via lldb-commits]

To be more specific, the problem is that there used to be a directory in the 
test suite called gmodules-template and Michael moved it to gmodules/template 
to better organize the tests.  But our rule for making a test name from 
subdirectories is to replace the "/" with a "-" so these two ended up being the 
same test name, which is an error.  

Michael's patch properly moved gmodules-template to gmodules/template so there 
actually shouldn't be a conflict.  But because python insists on sticking these 
__pycache__ directories alongside the test files git won't actually remove the 
directory on update, and thus until you either manually remove the 
gmodules-template directory or do a fresh checkout you'll get this error.

Is there a way to get the testsuite runs not to put these cache files into the 
source tree?  Running the testsuite really shouldn't be changing the source 
tree...  Or if we can't get it to stop that, is there a "git pull" that will 
clean up anything not actually in the repo?  I don't think we ever actually 
want to preserve anything of that sort from run to run.

Jim


> On Sep 16, 2022, at 3:24 PM, Michael Buch via Phabricator via lldb-commits 
>  wrote:
> 
> Michael137 added a comment.
> 
> In D133876#3796847 , @dyung wrote:
> 
>> @Michael137, your change is causing cmake to fail on one of our bots:
>> 
>> https://lab.llvm.org/buildbot/#/builders/217/builds/11819
>> 
>>  CMake Error at cmake/modules/AddLLVM.cmake:1867 (add_custom_target):
>>add_custom_target cannot create target
>>"check-lldb-api-lang-cpp-gmodules-templates" because another target with
>>the same name already exists.  The existing target is a custom target
>>created in source directory
>>
>> "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API".
>>See documentation for policy CMP0002 for more details.
>>  Call Stack (most recent call first):
>>cmake/modules/AddLLVM.cmake:1980 (add_lit_target)
>>
>> /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API/CMakeLists.txt:4
>>  (add_lit_testsuites)
>> 
>> Can you take a look?
> 
> Here I'm just renaming a directory. @dyung I think this might have to do with 
> a stale `__pycache__` directory in the test directory. Can you force a fresh 
> checkout on the build bot and restart?
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D133876/new/
> 
> https://reviews.llvm.org/D133876
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:681
+auto &ts = llvm::cast(*ct.GetTypeSystem());
+ts.GetMetadata(&tag)->SetIsForcefullyCompleted();
+  }

rnk wrote:
> Is this what we do for DWARF? The same kind of situation seems like it can 
> arise, where clang requires a type to be complete, but the information is 
> missing, and we still try to proceed with evaluation.
I think it's here: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L221-L250,
 relevant: https://reviews.llvm.org/D85968.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

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

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu planned changes to this revision.
zequanwu added a comment.

It seems not correct. The dwarf test 

 expects an error. I should figure out how to reduce that original crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

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

[Lldb-commits] [PATCH] D134043: [lldb] Log when we cannot find an equivalent for a gdb register type

[Greg Clayton via Phabricator via lldb-commits]

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

So you can use

  __PRETTY_FUNCTION__

in log messages if you want to get a demangled name for a C++ function, but 
that includes args, so it might be too verbose in a log message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134043

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

[Lldb-commits] [PATCH] D133876: [lldb][tests][NFC] Move C++ gmodules tests into new gmodules/ subdirectory

[Douglas Yung via Phabricator via lldb-commits]

dyung added a comment.

In D133876#3796923 , @Michael137 
wrote:

> In D133876#3796847 , @dyung wrote:
>
>> @Michael137, your change is causing cmake to fail on one of our bots:
>>
>> https://lab.llvm.org/buildbot/#/builders/217/builds/11819
>>
>>   CMake Error at cmake/modules/AddLLVM.cmake:1867 (add_custom_target):
>> add_custom_target cannot create target
>> "check-lldb-api-lang-cpp-gmodules-templates" because another target with
>> the same name already exists.  The existing target is a custom target
>> created in source directory
>> 
>> "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API".
>> See documentation for policy CMP0002 for more details.
>>   Call Stack (most recent call first):
>> cmake/modules/AddLLVM.cmake:1980 (add_lit_target)
>> 
>> /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API/CMakeLists.txt:4
>>  (add_lit_testsuites)
>>
>> Can you take a look?
>
> Here I'm just renaming a directory. @dyung I think this might have to do with 
> a stale `__pycache__` directory in the test directory. Can you force a fresh 
> checkout on the build bot and restart?

I only found one __pycache__ directory and deleting it didn't seem to make a 
difference. Any other suggestions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133876

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

[Lldb-commits] [PATCH] D133876: [lldb][tests][NFC] Move C++ gmodules tests into new gmodules/ subdirectory

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D133876#3797098 , @dyung wrote:

> In D133876#3796923 , @Michael137 
> wrote:
>
>> In D133876#3796847 , @dyung wrote:
>>
>>> @Michael137, your change is causing cmake to fail on one of our bots:
>>>
>>> https://lab.llvm.org/buildbot/#/builders/217/builds/11819
>>>
>>>   CMake Error at cmake/modules/AddLLVM.cmake:1867 (add_custom_target):
>>> add_custom_target cannot create target
>>> "check-lldb-api-lang-cpp-gmodules-templates" because another target with
>>> the same name already exists.  The existing target is a custom target
>>> created in source directory
>>> 
>>> "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API".
>>> See documentation for policy CMP0002 for more details.
>>>   Call Stack (most recent call first):
>>> cmake/modules/AddLLVM.cmake:1980 (add_lit_target)
>>> 
>>> /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API/CMakeLists.txt:4
>>>  (add_lit_testsuites)
>>>
>>> Can you take a look?
>>
>> Here I'm just renaming a directory. @dyung I think this might have to do 
>> with a stale `__pycache__` directory in the test directory. Can you force a 
>> fresh checkout on the build bot and restart?
>
> I only found one __pycache__ directory and deleting it didn't seem to make a 
> difference. Any other suggestions?

Could you try deleting the `lldb/test/API/lang/cpp/gmodules-templates` 
directory?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133876

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

[Lldb-commits] [PATCH] D133876: [lldb][tests][NFC] Move C++ gmodules tests into new gmodules/ subdirectory

[Douglas Yung via Phabricator via lldb-commits]

dyung added a comment.

In D133876#3797145 , @Michael137 
wrote:

> In D133876#3797098 , @dyung wrote:
>
>> In D133876#3796923 , @Michael137 
>> wrote:
>>
>>> In D133876#3796847 , @dyung wrote:
>>>
 @Michael137, your change is causing cmake to fail on one of our bots:

 https://lab.llvm.org/buildbot/#/builders/217/builds/11819

   CMake Error at cmake/modules/AddLLVM.cmake:1867 (add_custom_target):
 add_custom_target cannot create target
 "check-lldb-api-lang-cpp-gmodules-templates" because another target 
 with
 the same name already exists.  The existing target is a custom target
 created in source directory
 
 "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API".
 See documentation for policy CMP0002 for more details.
   Call Stack (most recent call first):
 cmake/modules/AddLLVM.cmake:1980 (add_lit_target)
 
 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/lldb/test/API/CMakeLists.txt:4
  (add_lit_testsuites)

 Can you take a look?
>>>
>>> Here I'm just renaming a directory. @dyung I think this might have to do 
>>> with a stale `__pycache__` directory in the test directory. Can you force a 
>>> fresh checkout on the build bot and restart?
>>
>> I only found one __pycache__ directory and deleting it didn't seem to make a 
>> difference. Any other suggestions?
>
> Could you try deleting the `lldb/test/API/lang/cpp/gmodules-templates` 
> directory?

That seemed to do the trick, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133876

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

[Lldb-commits] [lldb] edc1c9f - [LLDB][RISCV] Add RVM and RVA instruction support for EmulateInstructionRISCV

[via lldb-commits]

Author: Emmmer
Date: 2022-09-17T12:19:09+08:00
New Revision: edc1c9f6108a4eae24990928f914fe5a39f57ca6

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

LOG: [LLDB][RISCV] Add RVM and RVA instruction support for 
EmulateInstructionRISCV

Add:
- RVM and RVA instructions sets.
- corresponding unittests.

Further work:
- implement RVC, RVF, RVD, and RVV extension.

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp 
b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 4ca017dd98bbe..bcd18ff63d11b 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -19,7 +19,6 @@
 #include "lldb/Symbol/UnwindPlan.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/LLDBLog.h"
-#include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/STLExtras.h"
@@ -115,7 +114,7 @@ static bool WriteRegister(EmulateInstructionRISCV &emulator,
 static bool ExecJAL(EmulateInstructionRISCV &emulator, uint32_t inst, bool) {
   bool success = false;
   int64_t offset = SignExt(DecodeJImm(inst));
-  int64_t pc = emulator.ReadPC(&success);
+  int64_t pc = emulator.ReadPC(success);
   return success && emulator.WritePC(pc + offset) &&
  WriteRegister(emulator, DecodeRD(inst),
RegisterValue(uint64_t(pc + 4)));
@@ -127,7 +126,7 @@ static bool ExecJALR(EmulateInstructionRISCV &emulator, 
uint32_t inst, bool) {
   if (!ReadRegister(emulator, DecodeRS1(inst), value))
 return false;
   bool success = false;
-  int64_t pc = emulator.ReadPC(&success);
+  int64_t pc = emulator.ReadPC(success);
   int64_t rs1 = int64_t(value.GetAsUInt64());
   // JALR clears the bottom bit. According to riscv-spec:
   // "The JALR instruction now clears the lowest bit of the calculated target
@@ -157,10 +156,60 @@ static bool CompareB(uint64_t rs1, uint64_t rs2, uint32_t 
funct3) {
   }
 }
 
+struct Wrapped {
+  RegisterValue value;
+
+  template 
+  [[nodiscard]] std::enable_if_t, T> trunc() const {
+return T(value.GetAsUInt64());
+  }
+
+  template  T sext() const = delete;
+};
+
+template <> int32_t Wrapped::sext() const {
+  return int32_t(trunc());
+}
+
+template <> int64_t Wrapped::sext() const {
+  return int64_t(trunc());
+}
+
+template  struct RSRegs {
+private:
+  Wrapped rs1_value;
+
+public:
+  bool valid;
+
+  Wrapped &rs1() { return rs1_value; }
+};
+
+template <> struct RSRegs : RSRegs {
+private:
+  Wrapped rs2_value;
+
+public:
+  Wrapped &rs2() { return rs2_value; }
+};
+
+RSRegs readRS1RS2(EmulateInstructionRISCV &emulator, uint32_t inst) {
+  RSRegs value{};
+  value.valid = ReadRegister(emulator, DecodeRS1(inst), value.rs1().value) &&
+ReadRegister(emulator, DecodeRS2(inst), value.rs2().value);
+  return value;
+}
+
+RSRegs readRS1(EmulateInstructionRISCV &emulator, uint32_t inst) {
+  RSRegs value{};
+  value.valid = ReadRegister(emulator, DecodeRS1(inst), value.rs1().value);
+  return value;
+}
+
 static bool ExecB(EmulateInstructionRISCV &emulator, uint32_t inst,
   bool ignore_cond) {
   bool success = false;
-  uint64_t pc = emulator.ReadPC(&success);
+  uint64_t pc = emulator.ReadPC(success);
   if (!success)
 return false;
 
@@ -169,14 +218,12 @@ static bool ExecB(EmulateInstructionRISCV &emulator, 
uint32_t inst,
   if (ignore_cond)
 return emulator.WritePC(target);
 
-  RegisterValue value1;
-  RegisterValue value2;
-  if (!ReadRegister(emulator, DecodeRS1(inst), value1) ||
-  !ReadRegister(emulator, DecodeRS2(inst), value2))
+  auto rs = readRS1RS2(emulator, inst);
+  if (!rs.valid)
 return false;
 
   uint32_t funct3 = DecodeFunct3(inst);
-  if (CompareB(value1.GetAsUInt64(), value2.GetAsUInt64(), funct3))
+  if (CompareB(rs.rs1().trunc(), rs.rs2().trunc(), funct3))
 return emulator.WritePC(target);
 
   return true;
@@ -193,18 +240,21 @@ static bool ExecAUIPC(EmulateInstructionRISCV &emulator, 
uint32_t inst, bool) {
   uint32_t imm = DecodeUImm(inst);
   RegisterValue value;
   bool success = false;
-  value.SetUInt64(SignExt(imm) + emulator.ReadPC(&success));
+  value.SetUInt64(SignExt(imm) + emulator.ReadPC(success));
   return success && WriteRegister(emulator, DecodeRD(inst), value);
 }
 
 template 
-static std::enable_if_t, T>
-ReadMem(EmulateInstructionRISCV &emulator, uint64_t addr, bool *success) {
-
+static std::e

[Lldb-commits] [PATCH] D133670: [LLDB][RISCV] Add RVM and RVA instruction support for EmulateInstructionRISCV

[Emmmer S via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGedc1c9f6108a: [LLDB][RISCV] Add RVM and RVA instruction 
support for EmulateInstructionRISCV (authored by Emmmer).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133670

Files:
  lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
  lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
  lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

Index: lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
===
--- lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
+++ lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
@@ -24,11 +24,14 @@
 
 struct RISCVEmulatorTester : public EmulateInstructionRISCV, testing::Test {
   RegisterInfoPOSIX_riscv64::GPR gpr;
+  uint8_t memory[1024] = {0};
 
   RISCVEmulatorTester()
   : EmulateInstructionRISCV(ArchSpec("riscv64-unknown-linux-gnu")) {
 EmulateInstruction::SetReadRegCallback(ReadRegisterCallback);
 EmulateInstruction::SetWriteRegCallback(WriteRegisterCallback);
+EmulateInstruction::SetReadMemCallback(ReadMemoryCallback);
+EmulateInstruction::SetWriteMemCallback(WriteMemoryCallback);
   }
 
   static bool ReadRegisterCallback(EmulateInstruction *instruction, void *baton,
@@ -53,6 +56,25 @@
   tester->gpr.gpr[reg] = reg_value.GetAsUInt64();
 return true;
   }
+
+  static size_t ReadMemoryCallback(EmulateInstruction *instruction, void *baton,
+   const Context &context, lldb::addr_t addr,
+   void *dst, size_t length) {
+RISCVEmulatorTester *tester = (RISCVEmulatorTester *)instruction;
+assert(addr + length < sizeof(tester->memory));
+memcpy(dst, tester->memory + addr, length);
+return length;
+  };
+
+  static size_t WriteMemoryCallback(EmulateInstruction *instruction,
+void *baton, const Context &context,
+lldb::addr_t addr, const void *dst,
+size_t length) {
+RISCVEmulatorTester *tester = (RISCVEmulatorTester *)instruction;
+assert(addr + length < sizeof(tester->memory));
+memcpy(tester->memory + addr, dst, length);
+return length;
+  };
 };
 
 TEST_F(RISCVEmulatorTester, testJAL) {
@@ -64,7 +86,7 @@
   auto x1 = gpr.gpr[1];
 
   bool success = false;
-  auto pc = ReadPC(&success);
+  auto pc = ReadPC(success);
 
   ASSERT_TRUE(success);
   ASSERT_EQ(x1, old_pc + 4);
@@ -91,7 +113,7 @@
   auto x1 = gpr.gpr[1];
 
   bool success = false;
-  auto pc = ReadPC(&success);
+  auto pc = ReadPC(success);
 
   ASSERT_TRUE(success);
   ASSERT_EQ(x1, old_pc + 4);
@@ -144,7 +166,7 @@
   uint32_t inst = encoder(1, 2, -256);
   ASSERT_TRUE(tester->DecodeAndExecute(inst, false));
   bool success = false;
-  auto pc = tester->ReadPC(&success);
+  auto pc = tester->ReadPC(success);
   ASSERT_TRUE(success);
   ASSERT_EQ(pc, old_pc + (branched ? (-256) : 0));
 }
@@ -161,6 +183,13 @@
   ASSERT_EQ(tester->gpr.gpr[rd], value);
 }
 
+template 
+void CheckMem(RISCVEmulatorTester *tester, uint64_t addr, uint64_t value) {
+  bool success = false;
+  ASSERT_EQ(tester->ReadMem(*tester, addr, &success), value);
+  ASSERT_TRUE(success);
+}
+
 using RS1 = uint64_t;
 using RS2 = uint64_t;
 using PC = uint64_t;
@@ -171,21 +200,62 @@
 
   lldb::addr_t old_pc = 0x114514;
   tester->WritePC(old_pc);
-  auto rd = DecodeRD(inst);
-  auto rs1 = DecodeRS1(inst);
-  auto rs2 = 0;
+  uint32_t rd = DecodeRD(inst);
+  uint32_t rs1 = DecodeRS1(inst);
+  uint32_t rs2 = 0;
+
+  uint64_t rs1_val = 0x19;
+  uint64_t rs2_val = 0x81;
+
   if (rs1)
-tester->gpr.gpr[rs1] = 0x1919;
+tester->gpr.gpr[rs1] = rs1_val;
 
   if (has_rs2) {
 rs2 = DecodeRS2(inst);
-if (rs2)
-  tester->gpr.gpr[rs2] = 0x8181;
+if (rs2) {
+  if (rs1 == rs2)
+rs2_val = rs1_val;
+  tester->gpr.gpr[rs2] = rs2_val;
+}
   }
 
   ASSERT_TRUE(tester->DecodeAndExecute(inst, false));
-  CheckRD(tester, rd,
-  rd_val(tester->gpr.gpr[rs1], rs2 ? tester->gpr.gpr[rs2] : 0, old_pc));
+  CheckRD(tester, rd, rd_val(rs1_val, rs2 ? rs2_val : 0, old_pc));
+}
+
+template 
+void TestAtomic(RISCVEmulatorTester *tester, uint64_t inst, T rs1_val,
+T rs2_val, T rd_expected, T mem_expected) {
+  // Atomic inst must have rs1 and rs2
+
+  uint32_t rd = DecodeRD(inst);
+  uint32_t rs1 = DecodeRS1(inst);
+  uint32_t rs2 = DecodeRS2(inst);
+
+  // addr was stored in rs1
+  uint64_t atomic_addr = 0x100;
+
+  tester->gpr.gpr[rs1] = atomic_addr;
+  tester->gpr.gpr[rs2] = rs2_val;
+
+  // Write and check rs1_val in atomic_addr
+  ASSERT_TRUE(
+  tester->WriteMem(*tester, atomic_addr, RegisterValue(rs1_val)));
+  CheckMem(tester, atomic_addr, rs1_val);
+
+  ASSERT_TRUE(te

[Lldb-commits] [PATCH] D133042: Add auto deduce source map setting

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Just one nit about double checking if auto deducing is enabled. Fix that and 
this is good to go.




Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:305
 
+  if (GetBreakpoint()->GetTarget().GetAutoSourceMapRelative())
+DeduceSourceMapping(sc_list);

Either remove this, or leave the check in the DeduceSourceMapping function. I 
would vote to just always call DeduceSourceMapping in case someone else calls 
it from anywhere else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133042

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

[Lldb-commits] [PATCH] D133042: Add auto deduce source map setting

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan updated this revision to Diff 460973.
yinghuitan added a comment.

Fix nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133042

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Target/PathMappingList.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/unittests/Target/PathMappingListTest.cpp

Index: lldb/unittests/Target/PathMappingListTest.cpp
===
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -40,7 +40,7 @@
 map.RemapPath(ConstString(match.original.GetPath()), actual_remapped));
 EXPECT_EQ(FileSpec(actual_remapped.GetStringRef()), match.remapped);
 FileSpec unmapped_spec;
-EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec));
+EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec).hasValue());
 std::string unmapped_path = unmapped_spec.GetPath();
 EXPECT_EQ(unmapped_path, orig_normalized);
   }
Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -8,6 +8,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import json
 import os
 import side_effect
 
@@ -97,6 +98,8 @@
 "/x/main.cpp",
 "./x/y/a/d/c/main.cpp",
 ]
+# Reset source map.
+self.runCmd("settings clear target.source-map")
 for path in invalid_paths:
 bkpt = target.BreakpointCreateByLocation(path, 2)
 self.assertTrue(bkpt.GetNumLocations() == 0,
@@ -428,3 +431,68 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
+
+
+def get_source_map_json(self):
+stream = lldb.SBStream()
+self.dbg.GetSetting("target.source-map").GetAsJSON(stream)
+return json.loads(stream.GetData())
+
+def verify_source_map_entry_pair(self, entry, original, replacement):
+self.assertEquals(entry[0], original,
+"source map entry 'original' does not match")
+self.assertEquals(entry[1], replacement,
+"source map entry 'replacement' does not match")
+
+@skipIf(oslist=["windows"])
+@no_debug_info_test
+def test_breakpoints_auto_source_map_relative(self):
+"""
+Test that with target.auto-source-map-relative settings.
+
+The "relative.yaml" contains a line table that is:
+
+Line table for a/b/c/main.cpp in `a.out
+0x00013f94: a/b/c/main.cpp:1
+0x00013fb0: a/b/c/main.cpp:2:3
+0x00013fb8: a/b/c/main.cpp:2:3
+"""
+src_dir = self.getSourceDir()
+yaml_path = os.path.join(src_dir, "relative.yaml")
+yaml_base, ext = os.path.splitext(yaml_path)
+obj_path = self.getBuildArtifact("a.out")
+self.yaml2obj(yaml_path, obj_path)
+
+# Create a target with the object file we just created from YAML
+target = self.dbg.CreateTarget(obj_path)
+# We now have debug information with line table paths that start are
+# "./a/b/c/main.cpp".
+
+source_map_json = self.get_source_map_json()
+self.assertEquals(len(source_map_json), 0, "source map should be empty initially")
+
+# Verify auto deduced source map when file path in debug info
+# is a suffix of request breakpoint file path
+path = "/x/y/a/b/c/main.cpp"
+bp = target.BreakpointCreateByLocation(path, 2)
+self.assertTrue(bp.GetNumLocations() > 0,
+'Couldn\'t resolve breakpoint using full path "%s" in executate "%s" with '
+'debug info that has relative path with matching suffix' % (path, self.getBuildArtifact("a.out")))
+
+source_map_json = self.get_source_map_json()
+self.assertEquals(len(source_map_json), 1, "source map should not be empty")
+self.verify_source_map_entry_pair(source_map_json[0], ".", "/x/y")
+
+# Reset source map.
+self.runCmd("settings clear target.source-map")
+
+# Verify source map will not auto deduced when file path of request breakpoint
+# equals the file path in debug info.
+path = "a/b/c/main.cpp"
+bp = target.BreakpointCreateByLocation(path, 2)
+