[Lldb-commits] [PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

[Diana Picus via Phabricator via lldb-commits]

rovka added a comment.

Is this going to break for everyone that still passes TARGET_TRIPLE to cmake? 
If so, we should probably have a period where we support both.

In particular, this might break some of our buildbots 

 who explicitly set TARGET_TRIPLE. Could you send a patch to zorg cleaning that 
up (and maybe wait for that to go through before removing TARGET_TRIPLE 
completely)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119918

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

[Lldb-commits] [PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

[Diana Picus via Phabricator via lldb-commits]

rovka accepted this revision.
rovka added a comment.

I'm happy if the buildbots are happy :D thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119918

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

[Lldb-commits] [PATCH] D96458: [LLDB] Add support for Arm64/Linux dynamic register sets

[Diana Picus via Phabricator via lldb-commits]

rovka added a comment.

Hi Omair,

I had a look and I think the commit message is slightly misleading because it 
says "Arm64 has dynamic features like SVE, Pointer Authentication and MTE", but 
then you don't set m_reg_infos_is_dynamic to true for SVE. This got me a bit 
confused at first. Maybe it would help to elaborate a bit on this point in the 
commit message (and maybe also in the comments if you think it makes sense).

I also have a few nits inline.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:81
+  Status error = ReadSVEHeader();
+  uint32_t opt_regset = RegisterInfoPOSIX_arm64::eRegsetEnableDefault;
+  if (error.Success()) {

I was about to suggest using lldb_private::Flags for this, but after looking a 
bit more through the code it's not clear how this is intended to be used. The 
definition of eRegsetEnableSVE doesn't look flag-like at all (I would expect to 
see 1 << 1 rather than just plain 1, to make it clear what future values should 
look like), and also in ConfigureRegisterContext you check if (opt_regsets > 
eRegsetEnableSVE), which to me doesn't seem very idiomatic for working with 
flags. 

What do you think about increasing the level of abstraction a bit and using a 
small wrapper class for the regset options, that can answer directly questions 
like IsRegsetDynamic()?



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:88
+opt_regset |= RegisterInfoPOSIX_arm64::eRegsetEnableSVE;
+GetRegisterInfo().ConfigureRegisterInfos(opt_regset);
+  } else {

Nitpick: You should hoist the call out of the if, so it's easier to add other 
regsets in the future.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:132
+  // Contains pair of (start, end) register numbers of a register set with 
start
+  // and end included.
+  std::map> m_per_regset_regnum_range;

Nitpick: Please use square brackets instead of parantheses, otherwise it's 
confusing (parentheses suggest an open interval).


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

https://reviews.llvm.org/D96458

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

[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

[Diana Picus via Phabricator via lldb-commits]

rovka added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:843
-  response.PutCString(";QThreadSuffixSupported+");
-  response.PutCString(";QListThreadsInStopReply+");
-  response.PutCString(";qEcho+");

This seems to have disappeared, shouldn't it be in 
GDBRemoteCommunicationServerCommon::HandleFeatures?


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

https://reviews.llvm.org/D100140

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

[Lldb-commits] [PATCH] D100146: [lldb] [gdb-remote client] Refactor handling qSupported

[Diana Picus via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D100146

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

[Lldb-commits] [PATCH] D101153: [lldb][NFC] Specify guidelines for API tests

[Diana Picus via Phabricator via lldb-commits]

rovka added a comment.

This is awesome, thanks for writing this up!




Comment at: lldb/docs/resources/test.rst:224
+tests the `SBProcess`, `SBThread`, and `SBFrame` classes. The same is true
+for tests that exercise to breakpoints, watchpoints and sanitizers.
+Languages such as Objective-C that have a dependency on a runtime

I don't think you need 'to' here.



Comment at: lldb/docs/resources/test.rst:256
+**Identifiers in tests should be simple and descriptive.**
+Often test program need to declare functions and classes which require
+choosing some form of identifier for them. These identifiers should always

program -> programs



Comment at: lldb/docs/resources/test.rst:310
+generate an explanation how the received values differ from the expected
+ones. See the documentation of Python's `unittest` module to see what
+asserts are available. If you can't find a specific assert that fits your

You're using 'see' twice in the same sentence. Some alternatives:
"Check the documentation [...]", 
"Read the documentation [...]", 
"[...] learn what asserts are available", 
or anything else along those lines :)



Comment at: lldb/docs/resources/test.rst:313
+needs and you fall back to a generic assert, make sure you put useful
+information into the assert's `msg` argument that help explain the failure.
+

help -> helps


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

https://reviews.llvm.org/D101153

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

[Lldb-commits] [PATCH] D100459: [lldb] [gdb-remote client] Refactor SetCurrentThread*()

[Diana Picus via Phabricator via lldb-commits]

rovka added a comment.

This looks good to me, but I'll let others comment on whether or not we want to 
send extra leading zeroes for the thread ID. In my opinion it would be nice to 
1) be consistent between all packages that deal with thread ids, and 2) send as 
little clutter as possible. But these are just theoretical concerns, I'm not 
sure if it matters in practice.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:337
 
+  llvm::Optional SendSetCurrentThreadPacket(uint64_t tid, char type);
+

Nitpick: You could call this 'op' instead of 'type', to match the [[ 
https://sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets | documentation 
]] for the H packet.


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

https://reviews.llvm.org/D100459

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

[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg

[Diana Picus via Phabricator via lldb-commits]

rovka added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2130
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "No current process and no PID provided"));
+

Just a drive-by comment: The previous behaviour was to send 0x15 when the 
current process ID was invalid. Should we do that here?


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

https://reviews.llvm.org/D100261

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

[Lldb-commits] [PATCH] D70303: Fix TestFormatters.py stepping too far

[Diana Picus via Phabricator via lldb-commits]

rovka created this revision.
rovka added reviewers: omjavaid, lawrence_danna.
Herald added a subscriber: kristof.beyls.

TestFormatters.py has a sequence of three 'next' commands to get past
all the initializations in the test function. On AArch64 (and
potentially other platforms), this was one 'next' too many and we ended
up outside our frame.

This patch replaces the sequence with a 'thread until ' the line of the
return from the function, so we should stop after all the
initializations but before actually returning.


https://reviews.llvm.org/D70303

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py


Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "return")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])


Index: lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "return")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70303: Fix TestFormatters.py stepping too far

[Diana Picus via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5f0c3bad2f03: Fix TestFormatters.py stepping too far 
(authored by rovka).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70303

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
  lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp


Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
@@ -42,7 +42,7 @@
foo1.b.i = ;

int numbers[5] = {1,2,3,4,5};
-   
-   return 0;
-   
+
+   return 0; // Done initializing
+
 }
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "Done initializing")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])


Index: lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
@@ -42,7 +42,7 @@
 	foo1.b.i = ;
 	
 	int numbers[5] = {1,2,3,4,5};
-	
-	return 0;
-	
+
+	return 0; // Done initializing
+
 }
Index: lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "Done initializing")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70303: Fix TestFormatters.py stepping too far

[Diana Picus via Phabricator via lldb-commits]

rovka updated this revision to Diff 229505.
rovka added a comment.

Addressed @labath's comment (test run in progress). Thanks for having a look!


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

https://reviews.llvm.org/D70303

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
  lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp


Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
@@ -42,7 +42,7 @@
foo1.b.i = ;

int numbers[5] = {1,2,3,4,5};
-   
-   return 0;
-   
+
+   return 0; // Done initializing
+
 }
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "Done initializing")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])


Index: lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/main.cpp
@@ -42,7 +42,7 @@
 	foo1.b.i = ;
 	
 	int numbers[5] = {1,2,3,4,5};
-	
-	return 0;
-	
+
+	return 0; // Done initializing
+
 }
Index: lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "Done initializing")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70381: Mark PR44037 tests as XFAIL on AArch64 Linux dwo

[Diana Picus via Phabricator via lldb-commits]

rovka created this revision.
rovka added reviewers: omjavaid, aprantl, labath.
Herald added a subscriber: kristof.beyls.

These tests are failing with various assertion failures, but they all
throw the following error message first:

error: a.out 0x002d: adding range [0x14-0x24) which has a base that
is less than the function's low PC 0x40060c.

See llvm.org/pr44037.


https://reviews.llvm.org/D70381

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
  
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
  
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
  
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py


Index: 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
===
--- 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
+++ 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
@@ -26,6 +26,9 @@
 self.source, '// This should correspond to the second break stop.')
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_stop_at_outer_inline(self):
 """Exercise SBFrame.IsInlined() and SBFrame.GetFunctionName()."""
 self.build()
Index: 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
@@ -7,6 +7,9 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_when_stopped_in_method(self):
 self._load_exe()
 
@@ -109,6 +112,9 @@
 self.assertTrue(val.IsValid())
 self.assertEqual(val.GetValueAsUnsigned(), 778899)
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_when_stopped_in_function(self):
 self._load_exe()
 
Index: 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
@@ -6,4 +6,8 @@
   [decorators.expectedFailureAll(compiler="clang",
  compiler_version=["<",

"3.5"],
- 
bugnumber="llvm.org/pr27845")])
+ 
bugnumber="llvm.org/pr27845"),
+   decorators.expectedFailureAll(archs="aarch64",
+ oslist="linux",
+ debug_info="dwo",
+ 
bugnumber="llvm.org/pr44037"])
Index: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
@@ -16,17 +16,26 @@
 mydir = TestBase.compute_mydir(__file__)
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_enable(self):
 """Test breakpoint enable/disable for a breakpoint ID with multiple 
locations."""
 self.build()
 self.breakpoint_locations_test()
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_shadowed_cond_options(self):
 """Test that options set on the breakpoint and location behave 
correctly."""
 self.build()
 

[Lldb-commits] [PATCH] D70381: Mark PR44037 tests as XFAIL on AArch64 Linux dwo

[Diana Picus via Phabricator via lldb-commits]

rovka updated this revision to Diff 229794.

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

https://reviews.llvm.org/D70381

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
  
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
  
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
  
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py


Index: 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
===
--- 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
+++ 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
@@ -26,6 +26,9 @@
 self.source, '// This should correspond to the second break stop.')
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_stop_at_outer_inline(self):
 """Exercise SBFrame.IsInlined() and SBFrame.GetFunctionName()."""
 self.build()
Index: 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
@@ -1,4 +1,5 @@
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -7,6 +8,9 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_when_stopped_in_method(self):
 self._load_exe()
 
@@ -109,6 +113,9 @@
 self.assertTrue(val.IsValid())
 self.assertEqual(val.GetValueAsUnsigned(), 778899)
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_when_stopped_in_function(self):
 self._load_exe()
 
Index: 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
@@ -6,4 +6,8 @@
   [decorators.expectedFailureAll(compiler="clang",
  compiler_version=["<",

"3.5"],
- 
bugnumber="llvm.org/pr27845")])
+ 
bugnumber="llvm.org/pr27845"),
+   decorators.expectedFailureAll(archs="aarch64",
+ oslist="linux",
+ debug_info="dwo",
+ 
bugnumber="llvm.org/pr44037")])
Index: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
@@ -16,17 +16,26 @@
 mydir = TestBase.compute_mydir(__file__)
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_enable(self):
 """Test breakpoint enable/disable for a breakpoint ID with multiple 
locations."""
 self.build()
 self.breakpoint_locations_test()
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_shadowed_cond_options(self):
 """Test that options set on the breakpoint and location behave 
correctly."""
 self.build()
 self.shadowed_bkpt_cond_test()
 
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+   

[Lldb-commits] [PATCH] D70381: Mark PR44037 tests as XFAIL on AArch64 Linux dwo

[Diana Picus via Phabricator via lldb-commits]

rovka added a subscriber: labath.
rovka added a comment.

In D70381#1749616 , @labath wrote:

> Looks fine to me, but I am not running aarch64 tests. You might want to check 
> of @omjavaid is running them and whether he's ok with that. Otherwise, you 
> can just manage the decorators yourself without any special reviews.


Actually, we have a buildbot on the silent master running them:
http://lab.llvm.org:8014/builders/lldb-aarch64-ubuntu

I'm currently just triaging those failures so we can move the bot to the proper 
master at some point.


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

https://reviews.llvm.org/D70381



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

[Lldb-commits] [PATCH] D70381: Mark PR44037 tests as XFAIL on AArch64 Linux dwo

[Diana Picus via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbb7c8e984f87: Mark PR44037 tests as XFAIL on AArch64 Linux 
dwo (authored by rovka).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70381

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
  
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
  
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
  
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py


Index: 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
===
--- 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
+++ 
lldb/packages/Python/lldbsuite/test/python_api/frame/inlines/TestInlinedFrame.py
@@ -26,6 +26,9 @@
 self.source, '// This should correspond to the second break stop.')
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_stop_at_outer_inline(self):
 """Exercise SBFrame.IsInlined() and SBFrame.GetFunctionName()."""
 self.build()
Index: 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
@@ -1,4 +1,5 @@
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -7,6 +8,9 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_when_stopped_in_method(self):
 self._load_exe()
 
@@ -109,6 +113,9 @@
 self.assertTrue(val.IsValid())
 self.assertEqual(val.GetValueAsUnsigned(), 778899)
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_when_stopped_in_function(self):
 self._load_exe()
 
Index: 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/c/inlines/TestRedefinitionsInInlines.py
@@ -6,4 +6,8 @@
   [decorators.expectedFailureAll(compiler="clang",
  compiler_version=["<",

"3.5"],
- 
bugnumber="llvm.org/pr27845")])
+ 
bugnumber="llvm.org/pr27845"),
+   decorators.expectedFailureAll(archs="aarch64",
+ oslist="linux",
+ debug_info="dwo",
+ 
bugnumber="llvm.org/pr44037")])
Index: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
@@ -16,17 +16,26 @@
 mydir = TestBase.compute_mydir(__file__)
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_enable(self):
 """Test breakpoint enable/disable for a breakpoint ID with multiple 
locations."""
 self.build()
 self.breakpoint_locations_test()
 
+@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
+debug_info=["dwo"],
+bugnumber="llvm.org/pr44037")
 def test_shadowed_cond_options(self):
 """Test tha

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

[Diana Picus via Phabricator via lldb-commits]

rovka added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h:29
+  DynamicRegisterInfo(DynamicRegisterInfo &) = default;
   void operator=(DynamicRegisterInfo &) = delete;
 

I think operator= should be default too if the copy constructor is default.


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

https://reviews.llvm.org/D82857

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

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

[Diana Picus via Phabricator via lldb-commits]

rovka added a comment.

Sorry, I had one more comment :) I think that should be all from my side.




Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:403
+  m_register_info_sp = std::make_shared();
+  m_register_info_sp->Clear();
 

Is it still necessary to call Clear? I think the default constructor should 
give us a "cleared" object.


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

https://reviews.llvm.org/D82857

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

[Lldb-commits] [PATCH] D82064: [ARM64] Add QEMU testing environment setup guide for SVE testing

[Diana Picus via Phabricator via lldb-commits]

rovka added a comment.

Hi Omair,

If this is intended to be more generally useful, I wonder if we should rename 
it to something else (e.g. lldb-qemu-howto.txt) and only use AArch64 SVE as a 
case study.

In any case, thanks for writing this up!




Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:5
+
+QEMU can be used to test LLDB in emulation environment in absence of actual
+hardware. This write up will help setup an QEMU environment for testing LLDB

Nitpick: "in **an** emulation environment in **the** absence of actual [...]



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:7
+hardware. This write up will help setup an QEMU environment for testing LLDB
+patches supporting AArch64 features like SVE, MTE, Pointer Authentication etc.
+

Isn't this guide still useful after the patches are merged?



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:9
+
+Ubuntu Bionic/Focal x86_64 host machine was used to test all instruction in 
this
+document. Please update it according to your host distribution/architecture.

Typo: all the instructions



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:19
+
+#!/bin/bash
+

Would it be better for this to be an actual script in lldb/utils or somewhere 
similar?



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:78
+#
+# Build QEMU from source for AArch64 system-mode emulation
+

Is this difficult to infer from QEMU's own docs? If not, maybe just add a link 
and mention only what's specific to this guide (I'm guessing the target-list).



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:96
+#
+# Cross compile Linux kernel
+

Same as above, could this be just a link to the official docs?



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:124
+
+#!/bin/bash
+

Same as step 1: can this live in utils?



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:176
+# Your default username will be the one with whom you created RootFS image.
+# Internet access inside your emulation environment run: sudo dhclient enp0s2.
+

Nitpick: **For** Internet access [...]



Comment at: lldb/docs/lldb-qemu-aarch64-sve-howto.txt:217
+
+# Cross compile LLDB server for AArch64 Linux
+# Transfer LLDB server executable to emulation environment

For consistency, maybe there should be some commands here, or a link to the 
docs for cross-compiling.


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

https://reviews.llvm.org/D82064



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