[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

> Yes, that was the decision at the last time we looked - because removing 
> decls would degrade this - if we have new information that changes our 
> preferred design, then fine.

I remember the major reason for the last time to not remove the decls are that 
the design of AST doesn't support to remove decls. And my current idea is, we 
can refuse to write the discardable Decls into the BMIs.

> One solution is to place the elision behind a flag so that the user can 
> choose slower compilation with better diagnostics or faster compilation but 
> maybe harder-to-find errors?

I proposed to add a flag. But that was a helper for developers to find if we 
did anything wrong. I don't want to provide such a flag since on the one hand, 
there are already many flags  and on the other hand, form my user experience, 
it is not so hard to find the unexported decls. It is really much much more 
easier than template programming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Iain Sandoe via Phabricator via lldb-commits]

iains added a comment.

In D126694#4470297 , @ChuanqiXu wrote:

>> Yes, that was the decision at the last time we looked - because removing 
>> decls would degrade this - if we have new information that changes our 
>> preferred design, then fine.
>
> I remember the major reason for the last time to not remove the decls are 
> that the design of AST doesn't support to remove decls. And my current idea 
> is, we can refuse to write the discardable Decls into the BMIs.

@rsmith pointed out that the API was not intended for that purpose - so, yes, 
you are correct that the next place to look was in the serialisation [but ISTR 
that this also has some challenges because of the way in which the structures 
are interconnected].  It might be necessary to do a pass before the 
serialisation and actually prune the AST there. (in a similar manner we'd need 
to prune it to remove non-inline function bodies in some future version)

>> One solution is to place the elision behind a flag so that the user can 
>> choose slower compilation with better diagnostics or faster compilation but 
>> maybe harder-to-find errors?
>
> I proposed to add a flag. But that was a helper for developers to find if we 
> did anything wrong. I don't want to provide such a flag since on the one 
> hand, there are already many flags  and on the other hand, form my user 
> experience, it is not so hard to find the unexported decls. It is really much 
> much more easier than template programming.

Yeah, (as you know) I definitely prefer not to add more flags - there are too 
many - it was only an option in case there were many people against degrading 
diagnostic output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

In D126694#4470358 , @iains wrote:

> In D126694#4470297 , @ChuanqiXu 
> wrote:
>
>>> Yes, that was the decision at the last time we looked - because removing 
>>> decls would degrade this - if we have new information that changes our 
>>> preferred design, then fine.
>>
>> I remember the major reason for the last time to not remove the decls are 
>> that the design of AST doesn't support to remove decls. And my current idea 
>> is, we can refuse to write the discardable Decls into the BMIs.
>
> @rsmith pointed out that the API was not intended for that purpose - so, yes, 
> you are correct that the next place to look was in the serialisation [but 
> ISTR that this also has some challenges because of the way in which the 
> structures are interconnected].  It might be necessary to do a pass before 
> the serialisation and actually prune the AST there. (in a similar manner we'd 
> need to prune it to remove non-inline function bodies in some future version)

Yeah, I just feel it is implementable. But I need to give it a try.

>>> One solution is to place the elision behind a flag so that the user can 
>>> choose slower compilation with better diagnostics or faster compilation but 
>>> maybe harder-to-find errors?
>>
>> I proposed to add a flag. But that was a helper for developers to find if we 
>> did anything wrong. I don't want to provide such a flag since on the one 
>> hand, there are already many flags  and on the other hand, form my user 
>> experience, it is not so hard to find the unexported decls. It is really 
>> much much more easier than template programming.
>
> Yeah, (as you know) I definitely prefer not to add more flags - there are too 
> many - it was only an option in case there were many people against degrading 
> diagnostic output.

Got it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D154413: [lldb] Fix crash when completing register names after program exit

[David Spickett via Phabricator via lldb-commits]

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

Previously the following would crash:
(lldb) run
Process 2594053 launched: '/tmp/test.o' (aarch64)
Process 2594053 exited with status = 0 (0x)
(lldb) register read 

As the completer assumed that the execution context would always
have a register context. After a program has finished, it does not.

Split out the generic parts of the test from the x86 specific tests,
and added "register info" there as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154413

Files:
  lldb/source/Commands/CommandCompletions.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -736,13 +736,25 @@
 self.runCmd("type synthetic add -x Hoo -l test")
 self.complete_from_to("type synthetic delete ", ["Hoo"])
 
-@skipIf(archs=no_match(["x86_64"]))
-def test_register_read_and_write_on_x86(self):
-"""Test the completion of the commands register read and write on 
x86"""
-
+def test_register_no_complete(self):
 # The tab completion for "register read/write"  won't work without a 
running process.
 self.complete_from_to("register read ", "register read ")
 self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+self.build()
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.runCmd("run")
+
+# Once a program has finished you have an execution context but no 
register
+# context so completion cannot work.
+self.complete_from_to("register read ", "register read ")
+self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+@skipIf(archs=no_match(["x86_64"]))
+def test_register_read_and_write_on_x86(self):
+"""Test the completion of the commands register read and write on 
x86"""
 
 self.build()
 self.main_source_spec = lldb.SBFileSpec("main.cpp")
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -611,6 +611,9 @@
 
   RegisterContext *reg_ctx =
   interpreter.GetExecutionContext().GetRegisterContext();
+  if (!reg_ctx)
+return;
+
   const size_t reg_num = reg_ctx->GetRegisterCount();
   for (size_t reg_idx = 0; reg_idx < reg_num; ++reg_idx) {
 const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(reg_idx);


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -736,13 +736,25 @@
 self.runCmd("type synthetic add -x Hoo -l test")
 self.complete_from_to("type synthetic delete ", ["Hoo"])
 
-@skipIf(archs=no_match(["x86_64"]))
-def test_register_read_and_write_on_x86(self):
-"""Test the completion of the commands register read and write on x86"""
-
+def test_register_no_complete(self):
 # The tab completion for "register read/write"  won't work without a running process.
 self.complete_from_to("register read ", "register read ")
 self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+self.build()
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.runCmd("run")
+
+# Once a program has finished you have an execution context but no register
+# context so completion cannot work.
+self.complete_from_to("register read ", "register read ")
+self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+@skipIf(archs=no_match(["x86_64"]))
+def test_register_read_and_write_on_x86(self):
+"""Test the completion of the commands register read and write on x86"""
 
 self.build()
 self.main_source_spec = lldb.SBFileSpec("main.cpp")
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -611,6 +611,9 @@
 
   RegisterContext *reg_ctx =
   interpre

[Lldb-commits] [PATCH] D154413: [lldb] Fix crash when completing register names after program exit

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added reviewers: labath, JDevlieghere.
DavidSpickett added inline comments.



Comment at: lldb/test/API/functionalities/completion/TestCompletion.py:782
 self.complete_from_to("register write rbx ", [])
 
 def test_common_completion_target_stophook_ids(self):

I will add register info here later once I've got an x86 build going again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154413

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

[Lldb-commits] [PATCH] D154413: [lldb] Fix crash when completing register names after program exit

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 537021.
DavidSpickett added a comment.

Added register info to the x86 test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154413

Files:
  lldb/source/Commands/CommandCompletions.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -736,13 +736,25 @@
 self.runCmd("type synthetic add -x Hoo -l test")
 self.complete_from_to("type synthetic delete ", ["Hoo"])
 
-@skipIf(archs=no_match(["x86_64"]))
-def test_register_read_and_write_on_x86(self):
-"""Test the completion of the commands register read and write on 
x86"""
-
+def test_register_no_complete(self):
 # The tab completion for "register read/write"  won't work without a 
running process.
 self.complete_from_to("register read ", "register read ")
 self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+self.build()
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.runCmd("run")
+
+# Once a program has finished you have an execution context but no 
register
+# context so completion cannot work.
+self.complete_from_to("register read ", "register read ")
+self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+@skipIf(archs=no_match(["x86_64"]))
+def test_register_read_and_write_on_x86(self):
+"""Test the completion of the commands register read and write on 
x86"""
 
 self.build()
 self.main_source_spec = lldb.SBFileSpec("main.cpp")
@@ -768,6 +780,14 @@
 # register write can only take exact one register name as argument
 self.complete_from_to("register write rbx ", [])
 
+# test cases for register info
+self.complete_from_to("register info ", ["rax", "rbx", "rcx"])
+self.complete_from_to("register info r", ["rax", "rbx", "rcx"])
+self.complete_from_to("register info ra", "register info rax")
+self.complete_from_to("register info rb", ["rbx", "rbp"])
+# register info can only take exact one register name as argument
+self.complete_from_to("register info rbx ", [])
+
 def test_common_completion_target_stophook_ids(self):
 subcommands = ["delete", "enable", "disable"]
 
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -611,6 +611,9 @@
 
   RegisterContext *reg_ctx =
   interpreter.GetExecutionContext().GetRegisterContext();
+  if (!reg_ctx)
+return;
+
   const size_t reg_num = reg_ctx->GetRegisterCount();
   for (size_t reg_idx = 0; reg_idx < reg_num; ++reg_idx) {
 const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(reg_idx);


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -736,13 +736,25 @@
 self.runCmd("type synthetic add -x Hoo -l test")
 self.complete_from_to("type synthetic delete ", ["Hoo"])
 
-@skipIf(archs=no_match(["x86_64"]))
-def test_register_read_and_write_on_x86(self):
-"""Test the completion of the commands register read and write on x86"""
-
+def test_register_no_complete(self):
 # The tab completion for "register read/write"  won't work without a running process.
 self.complete_from_to("register read ", "register read ")
 self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+self.build()
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.runCmd("run")
+
+# Once a program has finished you have an execution context but no register
+# context so completion cannot work.
+self.complete_from_to("register read ", "register read ")
+self.complete_from_to("register write ", "register write ")
+self.complete_from_to("register info ", "register info ")
+
+@skipIf(archs=no_match(["x86_64"]))
+def test_register_read_and_write_on_x86(self):
+"""Test the completion of the commands register read and write on x86"""
 
 self.build()
 self.main_source_spec = lldb.SBFileSpec("main.cpp")
@@ -768,6 +7

[Lldb-commits] [lldb] 518320f - [lldb][AArch64] Account for extra libc frames in PAC unwind test

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-07-04T11:15:18+01:00
New Revision: 518320fd98d51e2151d2d256275860e99cae695f

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

LOG: [lldb][AArch64] Account for extra libc frames in PAC unwind test

Running this on Amazon Ubuntu the final backtrace is:
```
(lldb) thread backtrace
* thread #1, name = 'a.out', stop reason = breakpoint 1.1
  * frame #0: 0x07d0 a.out`func_c at main.c:10:3
frame #1: 0x07c4 a.out`func_b at main.c:14:3
frame #2: 0x07b4 a.out`func_a at main.c:18:3
frame #3: 0x07a4 a.out`main(argc=, 
argv=) at main.c:22:3
frame #4: 0xf7b373fc libc.so.6`___lldb_unnamed_symbol2962 + 108
frame #5: 0xf7b374cc libc.so.6`__libc_start_main + 152
frame #6: 0x06b0 a.out`_start + 48
```
This causes the test to fail because of the extra ___lldb_unnamed_symbol2962 
frame
(an inlined function?).

To fix this, strictly check all the frames in main.c then for the rest
just check we find __libc_start_main and _start in that order regardless
of other frames in between.

Reviewed By: omjavaid

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

Added: 


Modified: 

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
 
b/lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
index dbdab5a4d03660..949962ae42992a 100644
--- 
a/lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ 
b/lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -42,17 +42,31 @@ def test(self):
 "func_b",
 "func_a",
 "main",
+]
+
+libc_backtrace = [
 "__libc_start_main",
 "_start",
 ]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
-for frame_idx, frame in enumerate(thread.frames):
-frame = thread.GetFrameAtIndex(frame_idx)
+
+self.assertTrue(thread.GetNumFrames() >= (len(backtrace) + 
len(libc_backtrace)))
+
+# Strictly check frames that are in the test program's source.
+for frame_idx, frame in enumerate(thread.frames[:len(backtrace)]):
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
-# Check line number for functions in main.c
-if frame_idx < 4:
-self.assertEqual(
-frame.GetLineEntry().GetLine(),
-line_number("main.c", "Frame " + backtrace[frame_idx]),
-)
+self.assertEqual(
+frame.GetLineEntry().GetLine(),
+line_number("main.c", "Frame " + backtrace[frame_idx]),
+)
+
+# After the program comes some libc frames. The number varies by
+# system, so ensure we have at least these two in this order,
+# skipping frames in between.
+start_idx = frame_idx + 1
+for frame_idx, frame in enumerate(thread.frames[start_idx:], 
start=start_idx):
+self.assertTrue(frame)
+if libc_backtrace[0] == frame.GetFunctionName():
+libc_backtrace.pop(0)
+
+self.assertFalse(libc_backtrace, "Did not find expected libc frames.")



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

[Lldb-commits] [PATCH] D154204: [lldb][AArch64] Account for extra libc frames in PAC unwind test

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG518320fd98d5: [lldb][AArch64] Account for extra libc frames 
in PAC unwind test (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154204

Files:
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py


Index: 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -42,17 +42,31 @@
 "func_b",
 "func_a",
 "main",
+]
+
+libc_backtrace = [
 "__libc_start_main",
 "_start",
 ]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
-for frame_idx, frame in enumerate(thread.frames):
-frame = thread.GetFrameAtIndex(frame_idx)
+
+self.assertTrue(thread.GetNumFrames() >= (len(backtrace) + 
len(libc_backtrace)))
+
+# Strictly check frames that are in the test program's source.
+for frame_idx, frame in enumerate(thread.frames[:len(backtrace)]):
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
-# Check line number for functions in main.c
-if frame_idx < 4:
-self.assertEqual(
-frame.GetLineEntry().GetLine(),
-line_number("main.c", "Frame " + backtrace[frame_idx]),
-)
+self.assertEqual(
+frame.GetLineEntry().GetLine(),
+line_number("main.c", "Frame " + backtrace[frame_idx]),
+)
+
+# After the program comes some libc frames. The number varies by
+# system, so ensure we have at least these two in this order,
+# skipping frames in between.
+start_idx = frame_idx + 1
+for frame_idx, frame in enumerate(thread.frames[start_idx:], 
start=start_idx):
+self.assertTrue(frame)
+if libc_backtrace[0] == frame.GetFunctionName():
+libc_backtrace.pop(0)
+
+self.assertFalse(libc_backtrace, "Did not find expected libc frames.")


Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -42,17 +42,31 @@
 "func_b",
 "func_a",
 "main",
+]
+
+libc_backtrace = [
 "__libc_start_main",
 "_start",
 ]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
-for frame_idx, frame in enumerate(thread.frames):
-frame = thread.GetFrameAtIndex(frame_idx)
+
+self.assertTrue(thread.GetNumFrames() >= (len(backtrace) + len(libc_backtrace)))
+
+# Strictly check frames that are in the test program's source.
+for frame_idx, frame in enumerate(thread.frames[:len(backtrace)]):
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
-# Check line number for functions in main.c
-if frame_idx < 4:
-self.assertEqual(
-frame.GetLineEntry().GetLine(),
-line_number("main.c", "Frame " + backtrace[frame_idx]),
-)
+self.assertEqual(
+frame.GetLineEntry().GetLine(),
+line_number("main.c", "Frame " + backtrace[frame_idx]),
+)
+
+# After the program comes some libc frames. The number varies by
+# system, so ensure we have at least these two in this order,
+# skipping frames in between.
+start_idx = frame_idx + 1
+for frame_idx, frame in enumerate(thread.frames[start_idx:], start=start_idx):
+self.assertTrue(frame)
+if libc_backtrace[0] == frame.GetFunctionName():
+libc_backtrace.pop(0)
+
+self.assertFalse(libc_backtrace, "Did not find expected libc frames.")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 8e76093 - [lldb][AArch64] Fix tagged watch test on Graviton 3

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-07-04T11:16:07+01:00
New Revision: 8e76093ae8fe1184006ca14d7ffb01df982f0f43

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

LOG: [lldb][AArch64] Fix tagged watch test on Graviton 3

During __do_global_dtors_aux glibc sets a flag that is right
next to the global variable. This is done using a store byte.

On QEMU the watchpoints are handled with a finer granularity
than real hardware, so this wasn't a problem. On Graviton 3
(and Mountain Jade, though this test won't run there) watchpoints
look at larger chunks of memory.

This means that the final continue actually stops in  __do_global_dtors_aux
instead of exiting.

We could fix this by padding the global to be away from the flag,
but that is fiddly and it is easier just to remove the watchpoint
before the final continue. We have already verified it worked by that
point.

Reviewed By: omjavaid

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

Added: 


Modified: 

lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py

Removed: 




diff  --git 
a/lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
 
b/lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
index a2b6af5d770e6e..6369a3034b810a 100644
--- 
a/lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
+++ 
b/lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
@@ -129,12 +129,15 @@ def verify_watch_hits(self):
 substrs=["stop reason = watchpoint"],
 )
 
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should now be 2.
+self.expect("watchpoint list -v", substrs=["hit_count = 2"])
+
+# On some hardware, during __do_global_dtors_aux a flag is set near
+# the global which can trigger the watchpoint. So we must remove it.
+self.runCmd("watchpoint delete 1")
 self.runCmd("process continue")
 
 # There should be no more watchpoint hit and the process status should
 # be 'exited'.
 self.expect("process status", substrs=["exited"])
-
-# Use the '-v' option to do verbose listing of the watchpoint.
-# The hit count should now be 2.
-self.expect("watchpoint list -v", substrs=["hit_count = 2"])



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

[Lldb-commits] [PATCH] D154201: [lldb][AArch64] Fix tagged watch test on Graviton 3

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8e76093ae8fe: [lldb][AArch64] Fix tagged watch test on 
Graviton 3 (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154201

Files:
  lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py


Index: 
lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
===
--- 
lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
+++ 
lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
@@ -129,12 +129,15 @@
 substrs=["stop reason = watchpoint"],
 )
 
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should now be 2.
+self.expect("watchpoint list -v", substrs=["hit_count = 2"])
+
+# On some hardware, during __do_global_dtors_aux a flag is set near
+# the global which can trigger the watchpoint. So we must remove it.
+self.runCmd("watchpoint delete 1")
 self.runCmd("process continue")
 
 # There should be no more watchpoint hit and the process status should
 # be 'exited'.
 self.expect("process status", substrs=["exited"])
-
-# Use the '-v' option to do verbose listing of the watchpoint.
-# The hit count should now be 2.
-self.expect("watchpoint list -v", substrs=["hit_count = 2"])


Index: lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
===
--- lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
+++ lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
@@ -129,12 +129,15 @@
 substrs=["stop reason = watchpoint"],
 )
 
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should now be 2.
+self.expect("watchpoint list -v", substrs=["hit_count = 2"])
+
+# On some hardware, during __do_global_dtors_aux a flag is set near
+# the global which can trigger the watchpoint. So we must remove it.
+self.runCmd("watchpoint delete 1")
 self.runCmd("process continue")
 
 # There should be no more watchpoint hit and the process status should
 # be 'exited'.
 self.expect("process status", substrs=["exited"])
-
-# Use the '-v' option to do verbose listing of the watchpoint.
-# The hit count should now be 2.
-self.expect("watchpoint list -v", substrs=["hit_count = 2"])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9b37bfa - [lldb][AArch64] Handle different default vector length in SVE testing

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-07-04T11:17:54+01:00
New Revision: 9b37bfa15ecfd0b87c176bb20e3ec4c193926d36

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

LOG: [lldb][AArch64] Handle different default vector length in SVE testing

This test previously ran on QEMU or A64FX both of which can/do have
512 bit SVE by default.

Graviton 3 has 256 bit SVE so the first part of the test failed.

To fix this, probe the supported vector lengths before starting
the test. The first check will use the default vector length and
the rest use either 256 or 128 bit.

Therefore this test will be skipped on a machine with only 128 bit SVE.

Reviewed By: omjavaid

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

Added: 


Modified: 

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py

Removed: 




diff  --git 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
index 37ff75bf4cb143..c10a2e0cc77b13 100644
--- 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -1,5 +1,10 @@
 """
 Test the AArch64 SVE registers dynamic resize with multiple threads.
+
+This test assumes a minimum supported vector length (VL) of 256 bits
+and will test 512 bits if possible. We refer to "vg" which is the
+register shown in lldb. This is in units of 64 bits. 256 bit VL is
+the same as a vg of 4.
 """
 
 import lldb
@@ -9,6 +14,39 @@
 
 
 class RegisterCommandsTestCase(TestBase):
+def get_supported_vg(self):
+# Changing VL trashes the register state, so we need to run the program
+# just to test this. Then run it again for the test.
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+main_thread_stop_line = line_number("main.c", "// Break in main 
thread")
+lldbutil.run_break_set_by_file_and_line(self, "main.c", 
main_thread_stop_line)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"thread info 1",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint"],
+)
+
+# Write back the current vg to confirm read/write works at all.
+current_vg = self.match("register read vg", ["(0x[0-9]+)"])
+self.assertTrue(current_vg is not None)
+self.expect("register write vg {}".format(current_vg.group()))
+
+# Aka 128, 256 and 512 bit.
+supported_vg = []
+for vg in [2, 4, 8]:
+# This could mask other errors but writing vg is tested elsewhere
+# so we assume the hardware rejected the value.
+self.runCmd("register write vg {}".format(vg), check=False)
+if not self.res.GetError():
+supported_vg.append(vg)
+
+return supported_vg
+
 def check_sve_registers(self, vg_test_value):
 z_reg_size = vg_test_value * 8
 p_reg_size = int(z_reg_size / 8)
@@ -56,13 +94,18 @@ def check_sve_registers(self, vg_test_value):
 def test_sve_registers_dynamic_config(self):
 """Test AArch64 SVE registers multi-threaded dynamic resize."""
 
+if not self.isAArch64SVE():
+self.skipTest("SVE registers must be supported.")
+
 self.build()
+supported_vg = self.get_supported_vg()
+
+if not (2 in supported_vg and 4 in supported_vg):
+self.skipTest("Not all required SVE vector lengths are supported.")
+
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
-if not self.isAArch64SVE():
-self.skipTest("SVE registers must be supported.")
-
 main_thread_stop_line = line_number("main.c", "// Break in main 
thread")
 lldbutil.run_break_set_by_file_and_line(self, "main.c", 
main_thread_stop_line)
 
@@ -90,7 +133,10 @@ def test_sve_registers_dynamic_config(self):
 substrs=["stop reason = breakpoint"],
 )
 
-self.check_sve_registers(8)
+if 8 in supported_vg:
+self.check_sve_registers(8)
+else:
+self.check_sve_registers(4)
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 



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

[Lldb-commits] [PATCH] D154208: [lldb][AArch64] Handle different default vector length in SVE testing

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9b37bfa15ecf: [lldb][AArch64] Handle different default 
vector length in SVE testing (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154208

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py


Index: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -1,5 +1,10 @@
 """
 Test the AArch64 SVE registers dynamic resize with multiple threads.
+
+This test assumes a minimum supported vector length (VL) of 256 bits
+and will test 512 bits if possible. We refer to "vg" which is the
+register shown in lldb. This is in units of 64 bits. 256 bit VL is
+the same as a vg of 4.
 """
 
 import lldb
@@ -9,6 +14,39 @@
 
 
 class RegisterCommandsTestCase(TestBase):
+def get_supported_vg(self):
+# Changing VL trashes the register state, so we need to run the program
+# just to test this. Then run it again for the test.
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+main_thread_stop_line = line_number("main.c", "// Break in main 
thread")
+lldbutil.run_break_set_by_file_and_line(self, "main.c", 
main_thread_stop_line)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"thread info 1",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint"],
+)
+
+# Write back the current vg to confirm read/write works at all.
+current_vg = self.match("register read vg", ["(0x[0-9]+)"])
+self.assertTrue(current_vg is not None)
+self.expect("register write vg {}".format(current_vg.group()))
+
+# Aka 128, 256 and 512 bit.
+supported_vg = []
+for vg in [2, 4, 8]:
+# This could mask other errors but writing vg is tested elsewhere
+# so we assume the hardware rejected the value.
+self.runCmd("register write vg {}".format(vg), check=False)
+if not self.res.GetError():
+supported_vg.append(vg)
+
+return supported_vg
+
 def check_sve_registers(self, vg_test_value):
 z_reg_size = vg_test_value * 8
 p_reg_size = int(z_reg_size / 8)
@@ -56,13 +94,18 @@
 def test_sve_registers_dynamic_config(self):
 """Test AArch64 SVE registers multi-threaded dynamic resize."""
 
+if not self.isAArch64SVE():
+self.skipTest("SVE registers must be supported.")
+
 self.build()
+supported_vg = self.get_supported_vg()
+
+if not (2 in supported_vg and 4 in supported_vg):
+self.skipTest("Not all required SVE vector lengths are supported.")
+
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
-if not self.isAArch64SVE():
-self.skipTest("SVE registers must be supported.")
-
 main_thread_stop_line = line_number("main.c", "// Break in main 
thread")
 lldbutil.run_break_set_by_file_and_line(self, "main.c", 
main_thread_stop_line)
 
@@ -90,7 +133,10 @@
 substrs=["stop reason = breakpoint"],
 )
 
-self.check_sve_registers(8)
+if 8 in supported_vg:
+self.check_sve_registers(8)
+else:
+self.check_sve_registers(4)
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 


Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -1,5 +1,10 @@
 """
 Test the AArch64 SVE registers dynamic resize with multiple threads.
+
+This test assumes a minimum supported vector length (VL) of 256 bits
+and will test 512 bits if possible. We refer to "vg" which is the
+register shown in lldb. This is in units of 64 bits. 256 bit VL is
+the same as a vg of 4.
 """
 
 import lldb
@@ -9,6 +14,39 @@
 
 
 class RegisterCommandsTestCase(TestBase):
+def get_supported_vg(self):
+# Changing VL trashes the register state, so we need to run the program
+# just to test this. Then run it again for the test.
+exe = self.getBuildArtifact("a.out")