[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via lldb-commits]

thieta added a comment.

In D130689#3689157 , @thakis wrote:

> Is it expected and intentional that this increases the mac deployment target 
> to 10.12?

I wasn't aware of that - but I think it's expected since the check in RWMutex 
checks for the C++ standard and doesn't care about the deployment target for 
macOS. It seems pretty easy to change, but I wonder if that matters? 10.12 was 
released in 2016 so it's pretty old and this wouldn't affect most users of LLVM.

My gut feeling say that we should be fine with requiring 10.12 and if that 
becomes a big problem during the development phase someone could propose a 
patch to improve the check in RWMutex.

But in that case we should probably have a check for the deployment target as 
part of the cmake config and error if CXX_STANDARD > 17 and 
OSX_DEPLOYMENT_TARGET < 10.12.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

[Nikita Popov via Phabricator via lldb-commits]

nikic added a comment.

In D130689#3690258 , @thieta wrote:

> In D130689#3689157 , @thakis wrote:
>
>> Is it expected and intentional that this increases the mac deployment target 
>> to 10.12?
>
> I wasn't aware of that - but I think it's expected since the check in RWMutex 
> checks for the C++ standard and doesn't care about the deployment target for 
> macOS. It seems pretty easy to change, but I wonder if that matters? 10.12 
> was released in 2016 so it's pretty old and this wouldn't affect most users 
> of LLVM.
>
> My gut feeling say that we should be fine with requiring 10.12 and if that 
> becomes a big problem during the development phase someone could propose a 
> patch to improve the check in RWMutex.
>
> But in that case we should probably have a check for the deployment target as 
> part of the cmake config and error if CXX_STANDARD > 17 and 
> OSX_DEPLOYMENT_TARGET < 10.12.

Given 
https://github.com/llvm/llvm-project/blob/2bb7c54621f31a957302a4deb3d25b752acb07bd/llvm/include/llvm/Support/RWMutex.h#L22-L27,
 it seems like this is supposed to be supported. This is probably just a matter 
of moving the shared_mutex use behind the LLVM_USE_RW_MUTEX_IMPL guard?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via lldb-commits]

thieta added a comment.

In D130689#3690274 , @nikic wrote:

> Given 
> https://github.com/llvm/llvm-project/blob/2bb7c54621f31a957302a4deb3d25b752acb07bd/llvm/include/llvm/Support/RWMutex.h#L22-L27,
>  it seems like this is supposed to be supported. This is probably just a 
> matter of moving the shared_mutex use behind the LLVM_USE_RW_MUTEX_IMPL guard?

Yeah - I just realized that when I checked this. Just building the rest of the 
tree now to confirm this is the only change we need and I will publish this a 
different diff first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[Lldb-commits] [PATCH] D130674: Accurate watchpoint hit counts redux

[Pavel Labath via Phabricator via lldb-commits]

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

This makes sense to me. Thanks for doing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130674

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

[Lldb-commits] [PATCH] D130796: [LLDB][NativePDB] Switch to use DWARFLocationList.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The general idea makes sense to me, although I haven't tried to understand the 
pdb parsing code.

> (otherwise internal binary search may fail)

The current search algorithm is buggy, but I think the `RangeDataVector` class 
has always intended to support overlapping ranges. If you find yourself needing 
to do extra work to work its limitations, we should fix that algorithm instead. 
(However, I know that the generic class will not be able to express the 
preference for expressions that describe the full value vs. those that describe 
only a part of it, so I'll believe you if you say that some outside 
filtering/deduplication is still needed.)




Comment at: lldb/source/Expression/DWARFExpressionList.cpp:37-39
+  if (m_exprs.FindEntryThatContains(base) ||
+  m_exprs.FindEntryThatContains(end - 1))
+return false;

What is this attempting to check? That the list does not contain an overlapping 
entry? That hardly seems like a correct algorithm...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130796

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

[Lldb-commits] [PATCH] D130803: [lldb] Allow SymbolTable regex search functions to match mangled name

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Who sets this flag? How do you intend to use it?




Comment at: lldb/include/lldb/Core/Module.h:267
+   SymbolContextList &sc_list,
+   bool match_against_demangled = false);
 

Could this be `Mangled::NamePreference` instead of `bool` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130803

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

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.
Herald added a subscriber: JDevlieghere.

Flake here: https://lab.llvm.org/buildbot/#/builders/68/builds/36967.

Presumably the same problem that cd18e2ea3f4e87f8804a7d6661d5596ef1f07b81 
 fixed for 
TestNonStop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128932

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

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like how you've extracted the name construction functionality. Just one 
(inline) comment about the filtering.




Comment at: lldb/source/Core/Module.cpp:740
+  bool user_provided_name_is_mangled =
+  Mangled::GetManglingScheme(m_name.GetStringRef()) !=
+  Mangled::eManglingSchemeNone;

I think this is overly aggressive. `_Z3foov` could be a method name in some 
particularly sadistic class. I think you can do this optimization only in one 
direction: if the names match, then return true without consulting the language 
plugin. At that point, I don't think you even need to check whether the names 
are mangled.


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

https://reviews.llvm.org/D129682

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

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Core/Module.cpp:740
+  bool user_provided_name_is_mangled =
+  Mangled::GetManglingScheme(m_name.GetStringRef()) !=
+  Mangled::eManglingSchemeNone;

labath wrote:
> I think this is overly aggressive. `_Z3foov` could be a method name in some 
> particularly sadistic class. I think you can do this optimization only in one 
> direction: if the names match, then return true without consulting the 
> language plugin. At that point, I don't think you even need to check whether 
> the names are mangled.
Actually, this could go wrong even for names like `_Zonk`, since 
`GetManglingScheme` does not check that the string is an actually valid mangled 
name -- just that it looks like one from very far away.


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

https://reviews.llvm.org/D129682

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

[Lldb-commits] [PATCH] D128989: [lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good modulo the comments.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1786
+if (pid == StringExtractorGDBRemote::AllProcesses) {
+  for (auto &process_it : m_debugged_processes)
+thread_actions[process_it.first].Append(thread_action);

https://sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets says "It is an 
error to specify all processes but a specific thread, such as ‘p-1.tid’", so I 
guess this should error out if `tid != AllThreads`.



Comment at: lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py:153
+{"direction": "send", "regex": "%Stop:T.*"},
+# see the comment in TestNonStop.py, test_stdio
+"read packet: $vStdio#00",

See cd18e2ea3f4e87f8804a7d6661d5596ef1f07b81 for why that does not work.


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

https://reviews.llvm.org/D128989

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

[Lldb-commits] [PATCH] D130899: [LLDB][RISCV] Add riscv register enums

[Emmmer S via Phabricator via lldb-commits]

Emmmer created this revision.
Herald added subscribers: sunshaoce, VincentWu, luke957, vkmr, frasercrmck, 
evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, 
rbar, asb, arichardson.
Herald added a project: All.
Emmmer edited the summary of this revision.
Emmmer added reviewers: DavidSpickett, labath, jingham.
Emmmer added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Emmmer published this revision for review.
Herald added subscribers: lldb-commits, pcwang-thead, eopXD.

According to RISC-V ISA Spec 
 and 
riscv-v-spec 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130899

Files:
  lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h

Index: lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
@@ -0,0 +1,139 @@
+//===-- lldb-riscv-register-enums.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
+#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
+
+// LLDB register codes (e.g. RegisterKind == eRegisterKindLLDB)
+
+// Internal codes for all riscv registers.
+enum {
+  k_first_gpr_riscv,
+  gpr_x0_riscv = k_first_gpr_riscv,
+  gpr_x1_riscv,
+  gpr_x2_riscv,
+  gpr_x3_riscv,
+  gpr_x4_riscv,
+  gpr_x5_riscv,
+  gpr_x6_riscv,
+  gpr_x7_riscv,
+  gpr_x8_riscv,
+  gpr_x9_riscv,
+  gpr_x10_riscv,
+  gpr_x11_riscv,
+  gpr_x12_riscv,
+  gpr_x13_riscv,
+  gpr_x14_riscv,
+  gpr_x15_riscv,
+  gpr_x16_riscv,
+  gpr_x17_riscv,
+  gpr_x18_riscv,
+  gpr_x19_riscv,
+  gpr_x20_riscv,
+  gpr_x21_riscv,
+  gpr_x22_riscv,
+  gpr_x23_riscv,
+  gpr_x24_riscv,
+  gpr_x25_riscv,
+  gpr_x26_riscv,
+  gpr_x27_riscv,
+  gpr_x28_riscv,
+  gpr_x29_riscv,
+  gpr_x30_riscv,
+  gpr_x31_riscv,
+  gpr_pc_riscv,
+
+  k_last_gpr_riscv = gpr_pc_riscv,
+
+  k_first_fpr_riscv,
+  fpr_f0_riscv = k_first_fpr_riscv,
+  fpr_f1_riscv,
+  fpr_f2_riscv,
+  fpr_f3_riscv,
+  fpr_f4_riscv,
+  fpr_f5_riscv,
+  fpr_f6_riscv,
+  fpr_f7_riscv,
+  fpr_f8_riscv,
+  fpr_f9_riscv,
+  fpr_f10_riscv,
+  fpr_f11_riscv,
+  fpr_f12_riscv,
+  fpr_f13_riscv,
+  fpr_f14_riscv,
+  fpr_f15_riscv,
+  fpr_f16_riscv,
+  fpr_f17_riscv,
+  fpr_f18_riscv,
+  fpr_f19_riscv,
+  fpr_f20_riscv,
+  fpr_f21_riscv,
+  fpr_f22_riscv,
+  fpr_f23_riscv,
+  fpr_f24_riscv,
+  fpr_f25_riscv,
+  fpr_f26_riscv,
+  fpr_f27_riscv,
+  fpr_f28_riscv,
+  fpr_f29_riscv,
+  fpr_f30_riscv,
+  fpr_f31_riscv,
+  fpr_fflags_riscv,
+  fpr_frm_riscv,
+  fpr_fcsr_riscv,
+  k_last_fpr_riscv = fpr_fcsr_riscv,
+
+  k_first_vcr_riscv,
+  vcr_v0_riscv = k_first_vcr_riscv,
+  vcr_v1_riscv,
+  vcr_v2_riscv,
+  vcr_v3_riscv,
+  vcr_v4_riscv,
+  vcr_v5_riscv,
+  vcr_v6_riscv,
+  vcr_v7_riscv,
+  vcr_v8_riscv,
+  vcr_v9_riscv,
+  vcr_v10_riscv,
+  vcr_v11_riscv,
+  vcr_v12_riscv,
+  vcr_v13_riscv,
+  vcr_v14_riscv,
+  vcr_v15_riscv,
+  vcr_v16_riscv,
+  vcr_v17_riscv,
+  vcr_v18_riscv,
+  vcr_v19_riscv,
+  vcr_v20_riscv,
+  vcr_v21_riscv,
+  vcr_v22_riscv,
+  vcr_v23_riscv,
+  vcr_v24_riscv,
+  vcr_v25_riscv,
+  vcr_v26_riscv,
+  vcr_v27_riscv,
+  vcr_v28_riscv,
+  vcr_v29_riscv,
+  vcr_v30_riscv,
+  vcr_v31_riscv,
+  vcr_vstart_riscv,
+  vcr_vxsat_riscv,
+  vcr_vxrm_riscv,
+  vcr_vcsr_riscv,
+  vcr_vl_riscv,
+  vcr_vtype_riscv,
+  vcr_vlenb_riscv,
+  k_last_vcr_riscv = vcr_vlenb_riscv,
+
+  k_num_registers_riscv,
+  k_num_gpr_registers_riscv = k_last_gpr_riscv - k_first_gpr_riscv + 1,
+  k_num_fpr_registers_riscv = k_last_fpr_riscv - k_first_fpr_riscv + 1,
+  k_num_vcr_registers_riscv = k_last_vcr_riscv - k_first_vcr_riscv + 1,
+};
+
+#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130899: [LLDB][RISCV] Add riscv register enums

[David Spickett via Phabricator via lldb-commits]

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

As I understand it, these registers are the same across riscv32 and riscv64. So 
LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130899

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

[Lldb-commits] [lldb] b53641c - [lldb] Fix flakyness in TestProcessList

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-08-01T16:05:01+02:00
New Revision: b53641cb72acae8973a16c4f5567885bd3fe85c0

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

LOG: [lldb] Fix flakyness in TestProcessList

If the test is too fast it can read the process list before the forked
child process actually manages to exec the process with the right
arguments.

Use our file-based synchronization primitives to ensure the child is
up-and-running before we fetch the process list.

Added: 


Modified: 
lldb/test/API/commands/platform/process/list/TestProcessList.py
lldb/test/API/commands/platform/process/list/main.cpp

Removed: 




diff  --git a/lldb/test/API/commands/platform/process/list/TestProcessList.py 
b/lldb/test/API/commands/platform/process/list/TestProcessList.py
index 5daf5dc49b52a..683b62e56e171 100644
--- a/lldb/test/API/commands/platform/process/list/TestProcessList.py
+++ b/lldb/test/API/commands/platform/process/list/TestProcessList.py
@@ -23,8 +23,10 @@ def test_process_list_with_args(self):
 exe = self.getBuildArtifact("TestProcess")
 
 # Spawn a new process
-popen = self.spawnSubprocess(exe, args=["arg1", "--arg2", "arg3"])
-
-substrs = [str(popen.pid), "TestProcess arg1 --arg2 arg3"]
+sync_file = lldbutil.append_to_process_working_directory(self,
+"ready.txt")
+popen = self.spawnSubprocess(exe, args=[sync_file, "arg1", "--arg2", 
"arg3"])
+lldbutil.wait_for_file_on_target(self, sync_file)
 
+substrs = [str(popen.pid), "TestProcess", "arg1 --arg2 arg3"]
 self.expect("platform process list -v", substrs=substrs)

diff  --git a/lldb/test/API/commands/platform/process/list/main.cpp 
b/lldb/test/API/commands/platform/process/list/main.cpp
index da43e60155ea0..955f6e4382d20 100644
--- a/lldb/test/API/commands/platform/process/list/main.cpp
+++ b/lldb/test/API/commands/platform/process/list/main.cpp
@@ -1,9 +1,9 @@
-#include 
-
 #include 
+#include 
 #include 
 
 int main(int argc, char const *argv[]) {
+  std::ofstream(argv[1]).close();
   std::this_thread::sleep_for(std::chrono::seconds(30));
   return 0;
 }



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

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

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

mgorny added a comment.

In D128932#3690798 , @labath wrote:

> Flake here: https://lab.llvm.org/buildbot/#/builders/68/builds/36967.
>
> Presumably the same problem that cd18e2ea3f4e87f8804a7d6661d5596ef1f07b81 
>  fixed 
> for TestNonStop.

Thanks for the ping and the suggestion. I'm testing a fix right now and will 
push if it works ;-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128932

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

[Lldb-commits] [PATCH] D130796: [LLDB][NativePDB] Switch to use DWARFLocationList.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

> If you find yourself needing to do extra work to work its limitations, we 
> should fix that algorithm instead.

That makes sense. I'll work on fixing `RangeVectorData`.




Comment at: lldb/source/Expression/DWARFExpressionList.cpp:37-39
+  if (m_exprs.FindEntryThatContains(base) ||
+  m_exprs.FindEntryThatContains(end - 1))
+return false;

labath wrote:
> What is this attempting to check? That the list does not contain an 
> overlapping entry? That hardly seems like a correct algorithm...
`RangeDataVector::Append` just append the new entry to the end of vector. It 
has no idea if overlaps happens or not. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130796

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

[Lldb-commits] [PATCH] D128989: [lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

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

mgorny marked 2 inline comments as done.
mgorny added a comment.

Thanks!




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1786
+if (pid == StringExtractorGDBRemote::AllProcesses) {
+  for (auto &process_it : m_debugged_processes)
+thread_actions[process_it.first].Append(thread_action);

labath wrote:
> https://sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets says "It is an 
> error to specify all processes but a specific thread, such as ‘p-1.tid’", so 
> I guess this should error out if `tid != AllThreads`.
Makes sense. For some reason, I thought we've covered it somewhere but maybe it 
got lost in refactoring.


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

https://reviews.llvm.org/D128989

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

[Lldb-commits] [lldb] f8603c1 - [lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-01T18:52:47+02:00
New Revision: f8603c1f6d9eb90bc6a674111bd3441458006601

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

LOG: [lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

Support using the vCont packet to resume multiple processes
simultaneously when in non-stop mode.  The new logic now assumes that:

- actions without a thread-id or with process id of "p-1" apply to all
  debugged processes

- actions with a thread-id without process id apply to the current
  process (m_continue_process)

As with the other continue packets, it is only possible to resume
processes that are currently stopped (or stop these that are running).
It is unsupported to resume or stop individual threads of a running
process.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128989

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 20650b5c8820d..d1b30f74f0bfb 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1762,6 +1762,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
   break;
 }
 
+// If there's no thread-id (e.g. "vCont;c"), it's "p-1.-1".
 lldb::pid_t pid = StringExtractorGDBRemote::AllProcesses;
 lldb::tid_t tid = StringExtractorGDBRemote::AllThreads;
 
@@ -1770,7 +1771,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
   // Consume the separator.
   packet.GetChar();
 
-  auto pid_tid = packet.GetPidTid(StringExtractorGDBRemote::AllProcesses);
+  auto pid_tid = packet.GetPidTid(LLDB_INVALID_PROCESS_ID);
   if (!pid_tid)
 return SendIllFormedResponse(packet, "Malformed thread-id");
 
@@ -1784,29 +1785,35 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
   packet, "'t' action not supported for individual threads");
 }
 
-if (pid == StringExtractorGDBRemote::AllProcesses) {
-  if (m_debugged_processes.size() > 1)
-return SendIllFormedResponse(
-packet, "Resuming multiple processes not supported yet");
+// If we get TID without PID, it's the current process.
+if (pid == LLDB_INVALID_PROCESS_ID) {
   if (!m_continue_process) {
-LLDB_LOG(log, "no debugged process");
+LLDB_LOG(log, "no process selected via Hc");
 return SendErrorResponse(0x36);
   }
   pid = m_continue_process->GetID();
 }
 
+assert(pid != LLDB_INVALID_PROCESS_ID);
 if (tid == StringExtractorGDBRemote::AllThreads)
   tid = LLDB_INVALID_THREAD_ID;
-
 thread_action.tid = tid;
 
-thread_actions[pid].Append(thread_action);
+if (pid == StringExtractorGDBRemote::AllProcesses) {
+  if (tid != LLDB_INVALID_THREAD_ID)
+return SendIllFormedResponse(
+packet, "vCont: p-1 is not valid with a specific tid");
+  for (auto &process_it : m_debugged_processes)
+thread_actions[process_it.first].Append(thread_action);
+} else
+  thread_actions[pid].Append(thread_action);
   }
 
   assert(thread_actions.size() >= 1);
-  if (thread_actions.size() > 1)
+  if (thread_actions.size() > 1 && !m_non_stop)
 return SendIllFormedResponse(
-packet, "Resuming multiple processes not supported yet");
+packet,
+"Resuming multiple processes is supported in non-stop mode only");
 
   for (std::pair x : thread_actions) {
 auto process_it = m_debugged_processes.find(x.first);

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
index daaa15e86ab4a..3714789d22337 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -150,3 +150,55 @@ def test_c_both_nonstop(self):
 self.assertEqual(output.count(b"PID: "), 2)
 self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(), output)
 self.assertIn("PID: {}".format(int(child_pid, 16)).encode(), output)
+
+@add_test_categories(["fork"])
+def test_vCont_both_nonstop(self):
+lock1 = self.getBuildArtifact("lock1")
+lock2 = self.getBuildArtifact("lock2")
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "process:sync:" + lock1, "print-pid",
+  "process:sync:" + lock2, "stop"],
+

[Lldb-commits] [lldb] 0806927 - [lldb] [test] Fix test_c_both_nonstop flakiness

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-01T18:52:47+02:00
New Revision: 080692797724eefd15de00500d8d802e98230622

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

LOG: [lldb] [test] Fix test_c_both_nonstop flakiness

Thanks to Pavel Labath for reporting this and suggesting a fix.

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
index 0868c32bdd322..daaa15e86ab4a 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -113,6 +113,16 @@ def test_vCont_interspersed_nonstop(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],
  use_vCont=True, nonstop=True)
 
+def get_all_output_via_vStdio(self, output_test):
+# The output may be split into an arbitrary number of messages.
+# Loop until we have everything. The first message is waiting for us
+# in the packet queue.
+output = self._server.get_raw_output_packet()
+while not output_test(output):
+self._server.send_packet(b"vStdio")
+output += self._server.get_raw_output_packet()
+return output
+
 @add_test_categories(["fork"])
 def test_c_both_nonstop(self):
 lock1 = self.getBuildArtifact("lock1")
@@ -132,13 +142,11 @@ def test_c_both_nonstop(self):
 "read packet: $c#00",
 "send packet: $OK#00",
 {"direction": "send", "regex": "%Stop:T.*"},
-# see the comment in TestNonStop.py, test_stdio
-"read packet: $vStdio#00",
-"read packet: $vStdio#00",
-"send packet: $OK#00",
 ], True)
-ret = self.expect_gdbremote_sequence()
-self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
-  ret["O_content"])
-self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
-  ret["O_content"])
+self.expect_gdbremote_sequence()
+
+output = self.get_all_output_via_vStdio(
+lambda output: output.count(b"PID: ") >= 2)
+self.assertEqual(output.count(b"PID: "), 2)
+self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(), output)
+self.assertIn("PID: {}".format(int(child_pid, 16)).encode(), output)



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

[Lldb-commits] [PATCH] D128989: [lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rGf8603c1f6d9e: [lldb] [llgs] Support resuming multiple 
processes via vCont w/ nonstop (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D128989?vs=441674&id=449053#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128989

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -150,3 +150,55 @@
 self.assertEqual(output.count(b"PID: "), 2)
 self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(), output)
 self.assertIn("PID: {}".format(int(child_pid, 16)).encode(), output)
+
+@add_test_categories(["fork"])
+def test_vCont_both_nonstop(self):
+lock1 = self.getBuildArtifact("lock1")
+lock2 = self.getBuildArtifact("lock2")
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "process:sync:" + lock1, "print-pid",
+  "process:sync:" + lock2, "stop"],
+ nonstop=True))
+
+self.test_sequence.add_log_lines([
+"read packet: $vCont;c:p{}.{};c:p{}.{}#00".format(
+parent_pid, parent_tid, child_pid, child_tid),
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:T.*"},
+], True)
+self.expect_gdbremote_sequence()
+
+output = self.get_all_output_via_vStdio(
+lambda output: output.count(b"PID: ") >= 2)
+self.assertEqual(output.count(b"PID: "), 2)
+self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(), output)
+self.assertIn("PID: {}".format(int(child_pid, 16)).encode(), output)
+
+def vCont_both_nonstop_test(self, vCont_packet):
+lock1 = self.getBuildArtifact("lock1")
+lock2 = self.getBuildArtifact("lock2")
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "process:sync:" + lock1, "print-pid",
+  "process:sync:" + lock2, "stop"],
+ nonstop=True))
+
+self.test_sequence.add_log_lines([
+"read packet: ${}#00".format(vCont_packet),
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:T.*"},
+], True)
+self.expect_gdbremote_sequence()
+
+output = self.get_all_output_via_vStdio(
+lambda output: output.count(b"PID: ") >= 2)
+self.assertEqual(output.count(b"PID: "), 2)
+self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(), output)
+self.assertIn("PID: {}".format(int(child_pid, 16)).encode(), output)
+
+@add_test_categories(["fork"])
+def test_vCont_both_implicit_nonstop(self):
+self.vCont_both_nonstop_test("vCont;c")
+
+@add_test_categories(["fork"])
+def test_vCont_both_minus_one_nonstop(self):
+self.vCont_both_nonstop_test("vCont;c:p-1.-1")
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
@@ -1762,6 +1762,7 @@
   break;
 }
 
+// If there's no thread-id (e.g. "vCont;c"), it's "p-1.-1".
 lldb::pid_t pid = StringExtractorGDBRemote::AllProcesses;
 lldb::tid_t tid = StringExtractorGDBRemote::AllThreads;
 
@@ -1770,7 +1771,7 @@
   // Consume the separator.
   packet.GetChar();
 
-  auto pid_tid = packet.GetPidTid(StringExtractorGDBRemote::AllProcesses);
+  auto pid_tid = packet.GetPidTid(LLDB_INVALID_PROCESS_ID);
   if (!pid_tid)
 return SendIllFormedResponse(packet, "Malformed thread-id");
 
@@ -1784,29 +1785,35 @@
   packet, "'t' action not supported for individual threads");
 }
 
-if (pid == StringExtractorGDBRemote::AllProcesses) {
-  if (m_debugged_processes.size() > 1)
-return SendIllFormedResponse(
-packet, "Resuming multiple processes not supported yet");
+// If we get TID without PID, it's the current process.
+if (pid == LLDB_INVALID_PROCESS_ID) {
   if (!m_continue_process) {
-LLDB_LOG(log, "no debugged process");
+LLDB_LOG(log, "no process selected via Hc");
 return SendErrorRespo

[Lldb-commits] [PATCH] D130803: [lldb] Allow SymbolTable regex search functions to match mangled name

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

In D130803#3690754 , @labath wrote:

> Who sets this flag? How do you intend to use it?

I need this change downstream to search for certain swift mangled names by 
querying `Module::FindSymbolsMatchingRegExAndType`. It seemed general enough 
that we might want this upstream.




Comment at: lldb/include/lldb/Core/Module.h:267
+   SymbolContextList &sc_list,
+   bool match_against_demangled = false);
 

labath wrote:
> Could this be `Mangled::NamePreference` instead of `bool` ?
Ok! Wasn't aware of that enum. I'll change that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130803

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

[Lldb-commits] [PATCH] D130924: [trace][intelpt] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API changes

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 created this revision.
jj10306 added reviewers: clayborg, wallace, persona0220.
Herald added a project: All.
jj10306 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

D130309  introduced changes to the FileSpec 
API which broke usages of
`GetCString()` in TraceIntelPTBundleSaver.cpp. This diff replaces usages
of `GetCString()` with `GetPath().c_str()` as suggested by D130309 
.

Test Plan:
Building with the trace plug-in now succeeds


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130924

Files:
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
@@ -103,7 +103,7 @@
 
   FileSpec threads_dir = directory;
   threads_dir.AppendPathComponent("threads");
-  sys::fs::create_directories(threads_dir.GetCString());
+  sys::fs::create_directories(threads_dir.GetPath().c_str());
 
   for (ThreadSP thread_sp : process.Threads()) {
 lldb::tid_t tid = thread_sp->GetID();
@@ -200,7 +200,7 @@
   std::vector json_cpus;
   FileSpec cpus_dir = directory;
   cpus_dir.AppendPathComponent("cpus");
-  sys::fs::create_directories(cpus_dir.GetCString());
+  sys::fs::create_directories(cpus_dir.GetPath().c_str());
 
   for (lldb::cpu_id_t cpu_id : trace_ipt.GetTracedCpus()) {
 JSONCpu json_cpu;


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
@@ -103,7 +103,7 @@
 
   FileSpec threads_dir = directory;
   threads_dir.AppendPathComponent("threads");
-  sys::fs::create_directories(threads_dir.GetCString());
+  sys::fs::create_directories(threads_dir.GetPath().c_str());
 
   for (ThreadSP thread_sp : process.Threads()) {
 lldb::tid_t tid = thread_sp->GetID();
@@ -200,7 +200,7 @@
   std::vector json_cpus;
   FileSpec cpus_dir = directory;
   cpus_dir.AppendPathComponent("cpus");
-  sys::fs::create_directories(cpus_dir.GetCString());
+  sys::fs::create_directories(cpus_dir.GetPath().c_str());
 
   for (lldb::cpu_id_t cpu_id : trace_ipt.GetTracedCpus()) {
 JSONCpu json_cpu;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130924: [NFC][trace] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API changes

[walter erquinigo via Phabricator via lldb-commits]

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

thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130924

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

[Lldb-commits] [PATCH] D130925: [trace] Replace TraceCursorUP with TraceCursorSP

[Jakob Johnson via Phabricator via lldb-commits]

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

The use of `std::unique_ptr` with `TraceCursor` adds unnecessary complexity to 
adding `SBTraceCursor` bindings
Specifically, since `TraceCursor` is an abstract class there's no clean
way to provide "deep clone" semantics for `TraceCursorUP` short of
creating a pure virtual `clone()` method (afaict).

After discussing with @wallace, we decided there is no strong reason to
favor wrapping `TraceCursor` with `std::unique_ptr` over `std::shared_ptr`, 
thus this diff
replaces all usages of `std::unique_ptr` with 
`std::shared_ptr`.

This sets the stage for future diffs to introduce `SBTraceCursor`
bindings in a more clean fashion.

Test Plan:


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130925

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceDumper.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -295,32 +295,32 @@
 new OutputWriterCLI(s, options, thread));
 }
 
-TraceDumper::TraceDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+TraceDumper::TraceDumper(lldb::TraceCursorSP cursor_sp, Stream &s,
  const TraceDumperOptions &options)
-: m_cursor_up(std::move(cursor_up)), m_options(options),
+: m_cursor_sp(std::move(cursor_sp)), m_options(options),
   m_writer_up(CreateWriter(
-  s, m_options, *m_cursor_up->GetExecutionContextRef().GetThreadSP())) {
+  s, m_options, *m_cursor_sp->GetExecutionContextRef().GetThreadSP())) {
 
   if (m_options.id)
-m_cursor_up->GoToId(*m_options.id);
+m_cursor_sp->GoToId(*m_options.id);
   else if (m_options.forwards)
-m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
+m_cursor_sp->Seek(0, TraceCursor::SeekType::Beginning);
   else
-m_cursor_up->Seek(0, TraceCursor::SeekType::End);
+m_cursor_sp->Seek(0, TraceCursor::SeekType::End);
 
-  m_cursor_up->SetForwards(m_options.forwards);
+  m_cursor_sp->SetForwards(m_options.forwards);
   if (m_options.skip) {
-m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
+m_cursor_sp->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
   TraceCursor::SeekType::Current);
   }
 }
 
 TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
   TraceItem item = {};
-  item.id = m_cursor_up->GetId();
+  item.id = m_cursor_sp->GetId();
 
   if (m_options.show_timestamps)
-item.timestamp = m_cursor_up->GetWallClockTime();
+item.timestamp = m_cursor_sp->GetWallClockTime();
   return item;
 }
 
@@ -378,7 +378,7 @@
 }
 
 Optional TraceDumper::DumpInstructions(size_t count) {
-  ThreadSP thread_sp = m_cursor_up->GetExecutionContextRef().GetThreadSP();
+  ThreadSP thread_sp = m_cursor_sp->GetExecutionContextRef().GetThreadSP();
 
   SymbolInfo prev_symbol_info;
   Optional last_id;
@@ -386,32 +386,32 @@
   ExecutionContext exe_ctx;
   thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx);
 
-  for (size_t insn_seen = 0; insn_seen < count && m_cursor_up->HasValue();
-   m_cursor_up->Next()) {
+  for (size_t insn_seen = 0; insn_seen < count && m_cursor_sp->HasValue();
+   m_cursor_sp->Next()) {
 
-last_id = m_cursor_up->GetId();
+last_id = m_cursor_sp->GetId();
 TraceItem item = CreatRawTraceItem();
 
-if (m_cursor_up->IsEvent()) {
+if (m_cursor_sp->IsEvent()) {
   if (!m_options.show_events)
 continue;
-  item.event = m_cursor_up->GetEventType();
+  item.event = m_cursor_sp->GetEventType();
   switch (*item.event) {
   case eTraceEventCPUChanged:
-item.cpu_id = m_cursor_up->GetCPU();
+item.cpu_id = m_cursor_sp->GetCPU();
 break;
   case eTraceEventHWClockTick:
-item.hw_clock = m_cursor_up->GetHWClock();
+item.hw_clock = m_cursor_sp->GetHWClock();
 break;
   case eTraceEventDisabledHW:
   case eTraceEventDisabledSW:
 break;
   }
-} else if (m_cursor_up->IsError()) {
-  item.error = m_cursor_up->GetError();
+} else if (m_cursor_sp->IsError()) {
+  item.error = m_cursor_sp->GetError();
 } else {
   insn_seen++;
-  item.load_address = m_cursor_up->GetLoadAddress();
+  item.load_address = m_cursor_sp->GetLoadAddress();
 
   if (!m_options.raw) {
 SymbolInfo symbol_info;
@@ -429,7 +429,7 @@
  

[Lldb-commits] [lldb] 9bab358 - [trace][intelpt] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API changes

[Jakob Johnson via lldb-commits]

Author: Jakob Johnson
Date: 2022-08-01T11:52:15-07:00
New Revision: 9bab358e39225a657be829962d7f9532b492ca93

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

LOG: [trace][intelpt] Update TraceIntelPTBundleSaver.cpp to accommodate 
FileSpec API changes

D130309 introduced changes to the FileSpec API which broke usages of
`GetCString()` in TraceIntelPTBundleSaver.cpp. This diff replaces usages
of `GetCString()` with `GetPath().c_str()` as suggested by D130309.

Test Plan:
Building with the trace plug-in now succeeds

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

Added: 


Modified: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
index 8be70dc2139be..f35914f26ab7c 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
@@ -103,7 +103,7 @@ BuildThreadsSection(Process &process, FileSpec directory) {
 
   FileSpec threads_dir = directory;
   threads_dir.AppendPathComponent("threads");
-  sys::fs::create_directories(threads_dir.GetCString());
+  sys::fs::create_directories(threads_dir.GetPath().c_str());
 
   for (ThreadSP thread_sp : process.Threads()) {
 lldb::tid_t tid = thread_sp->GetID();
@@ -200,7 +200,7 @@ BuildCpusSection(TraceIntelPT &trace_ipt, FileSpec 
directory, bool compact) {
   std::vector json_cpus;
   FileSpec cpus_dir = directory;
   cpus_dir.AppendPathComponent("cpus");
-  sys::fs::create_directories(cpus_dir.GetCString());
+  sys::fs::create_directories(cpus_dir.GetPath().c_str());
 
   for (lldb::cpu_id_t cpu_id : trace_ipt.GetTracedCpus()) {
 JSONCpu json_cpu;



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

[Lldb-commits] [PATCH] D130924: [NFC][trace] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API changes

[Jakob Johnson via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9bab358e3922: [trace][intelpt] Update 
TraceIntelPTBundleSaver.cpp to accommodate FileSpec API… (authored by jj10306).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130924

Files:
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
@@ -103,7 +103,7 @@
 
   FileSpec threads_dir = directory;
   threads_dir.AppendPathComponent("threads");
-  sys::fs::create_directories(threads_dir.GetCString());
+  sys::fs::create_directories(threads_dir.GetPath().c_str());
 
   for (ThreadSP thread_sp : process.Threads()) {
 lldb::tid_t tid = thread_sp->GetID();
@@ -200,7 +200,7 @@
   std::vector json_cpus;
   FileSpec cpus_dir = directory;
   cpus_dir.AppendPathComponent("cpus");
-  sys::fs::create_directories(cpus_dir.GetCString());
+  sys::fs::create_directories(cpus_dir.GetPath().c_str());
 
   for (lldb::cpu_id_t cpu_id : trace_ipt.GetTracedCpus()) {
 JSONCpu json_cpu;


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp
@@ -103,7 +103,7 @@
 
   FileSpec threads_dir = directory;
   threads_dir.AppendPathComponent("threads");
-  sys::fs::create_directories(threads_dir.GetCString());
+  sys::fs::create_directories(threads_dir.GetPath().c_str());
 
   for (ThreadSP thread_sp : process.Threads()) {
 lldb::tid_t tid = thread_sp->GetID();
@@ -200,7 +200,7 @@
   std::vector json_cpus;
   FileSpec cpus_dir = directory;
   cpus_dir.AppendPathComponent("cpus");
-  sys::fs::create_directories(cpus_dir.GetCString());
+  sys::fs::create_directories(cpus_dir.GetPath().c_str());
 
   for (lldb::cpu_id_t cpu_id : trace_ipt.GetTracedCpus()) {
 JSONCpu json_cpu;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

[walter erquinigo via Phabricator via lldb-commits]

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

Are the files in `lldb/test/API/commands/trace/intelpt-kernel-trace/cores/` 
actual kernel traces? If not, just use some trace files that are already 
present in the repo. You can use relative paths of the form 
../../trace_sample/cores/... to refer to them.

Good start for this task, btw!




Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:176
 
+  enum TraceMode {
+UserMode,

enum class



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:176
 
+  enum TraceMode {
+UserMode,

wallace wrote:
> enum class
@jj10306 , TraceMode or TracingMode?



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:199
+  /// \param[in] trace_mode
+  /// The tracing mode in the live session. One of TraceMode enum value.
+  ///





Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:133-148
+  TargetSP target_sp;
+  Status error = m_debugger.GetTargetList().CreateTarget(
+  m_debugger, /*user_exe_path*/ StringRef(), "",
+  eLoadDependentsNo,
+  /*platform_options*/ nullptr, target_sp);
+
+  if (!target_sp)

this part is the same as the one in ParseProcess. Create a function to dedup 
this code. Maybe a good name is CreateEmptyProcess



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:155-163
+char thread_name[20];
+sprintf(thread_name, "kernel_cpu_%d", cpu.id);
+lldb::tid_t tid = static_cast(cpu.id);
+
+Optional trace_file;
+trace_file = FileSpec(cpu.ipt_trace);
+

you can make this simpler



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:172
+  module_spec.GetFileSpec() = system_file_spec;
+  module_spec.GetPlatformFileSpec() = system_file_spec;
+

try without setting this one. Hopefully you only need to set one file spec. The 
PlatformFileSpec helps distinguish the binary path on the lldb's machine from 
the one were collection happened, but I don't think that feature will be useful 
for this kernel thing



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:291
+"loadAddress"?: integer | string decimal | hex string,
+// Kernel's start address. 0x8100 on default.
+"file": string,





Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:302
 - "tscPerfZeroConversion" must be provided if "cpus" is provided.
+- If tracing mode is "kernel", then the "processes" section must be empty and 
the "kernel" and "cpus" section must be provided.
  })";





Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleSaver.cpp:362
 
+  //TODO: Add kernel section
+

add this now. You have to update the corresponding tests to make sure that 
saving a loaded trace bundle produces the same information.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h:25
 const bool kDefaultDisableCgroupFiltering = false;
+const uint64_t kDefaultKernelLoadAddress = 0x8100;
 





Comment at: lldb/test/API/commands/trace/intelpt-kernel-trace/trace.json:20-21
+  },
+  "processes": [
+  ],
+  "tscPerfZeroConversion": {

Make "processes" optional, so that it's not even necessary to specify this 
field if "kernel" is provided. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130805

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

[Lldb-commits] [PATCH] D130925: [trace] Replace TraceCursorUP with TraceCursorSP

[walter erquinigo via Phabricator via lldb-commits]

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

great! that will make all the bindings very easy to handle


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130925

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

[Lldb-commits] [PATCH] D130929: [LLDB][Reliability] Remove dead code.

[Slava Gurevich via Phabricator via lldb-commits]

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

Remove redundant code that can never execute due to preceeding logic checks in 
the code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130929

Files:
  lldb/source/API/SBFrame.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
  lldb/source/Target/Platform.cpp


Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1773,25 +1773,21 @@
   error.Clear();
 
   if (!target) {
-ArchSpec arch;
-if (target && target->GetArchitecture().IsValid())
-  arch = target->GetArchitecture();
-else
-  arch = Target::GetDefaultArchitecture();
+ArchSpec arch = Target::GetDefaultArchitecture();
 
-const char *triple = "";
-if (arch.IsValid())
-  triple = arch.GetTriple().getTriple().c_str();
+const char *triple =
+arch.IsValid() ? arch.GetTriple().getTriple().c_str() : "";
 
 TargetSP new_target_sp;
 error = debugger.GetTargetList().CreateTarget(
 debugger, "", triple, eLoadDependentsNo, nullptr, new_target_sp);
+
 target = new_target_sp.get();
+if (!target || error.Fail()) {
+  return nullptr;
+}
   }
 
-  if (!target || error.Fail())
-return nullptr;
-
   lldb::ProcessSP process_sp =
   target->CreateProcess(debugger.GetListener(), plugin_name, nullptr, 
true);
 
Index: lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
===
--- lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
+++ lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
@@ -65,9 +65,6 @@
 return true;
   }
 
-  if (timescale == 0)
-return false;
-
   switch (timescale) {
   case 0:
 return false;
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
===
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -582,9 +582,6 @@
   ++NGRN;
 }
 
-if (reg_info == nullptr)
-  return false;
-
 const lldb::addr_t value_addr =
 reg_ctx->ReadRegisterAsUnsigned(reg_info, LLDB_INVALID_ADDRESS);
 
Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
===
--- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -612,9 +612,6 @@
   ++NGRN;
 }
 
-if (reg_info == nullptr)
-  return false;
-
 const lldb::addr_t value_addr =
 reg_ctx->ReadRegisterAsUnsigned(reg_info, LLDB_INVALID_ADDRESS);
 
Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -739,7 +739,7 @@
 lldb::DynamicValueType use_dynamic =
 frame->CalculateTarget()->GetPreferDynamicValue();
 const bool include_runtime_support_values =
-target ? target->GetDisplayRuntimeSupportValues() : false;
+target->GetDisplayRuntimeSupportValues();
 
 SBVariablesOptions options;
 options.SetIncludeArguments(arguments);


Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1773,25 +1773,21 @@
   error.Clear();
 
   if (!target) {
-ArchSpec arch;
-if (target && target->GetArchitecture().IsValid())
-  arch = target->GetArchitecture();
-else
-  arch = Target::GetDefaultArchitecture();
+ArchSpec arch = Target::GetDefaultArchitecture();
 
-const char *triple = "";
-if (arch.IsValid())
-  triple = arch.GetTriple().getTriple().c_str();
+const char *triple =
+arch.IsValid() ? arch.GetTriple().getTriple().c_str() : "";
 
 TargetSP new_target_sp;
 error = debugger.GetTargetList().CreateTarget(
 debugger, "", triple, eLoadDependentsNo, nullptr, new_target_sp);
+
 target = new_target_sp.get();
+if (!target || error.Fail()) {
+  return nullptr;
+}
   }
 
-  if (!target || error.Fail())
-return nullptr;
-
   lldb::ProcessSP process_sp =
   target->CreateProcess(debugger.GetListener(), plugin_name, nullptr, true);
 
Index: lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
===
--- lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
+++ lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
@@ -65,9 +65,6 @@
 return true;
   }
 
-  if (timescale == 0)
-return false;
-
   switch (times

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 created this revision.
jj10306 added reviewers: wallace, persona0220.
Herald added a subscriber: mgorny.
Herald added a project: All.
jj10306 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Add bindings for the `TraceCursor` to allow for programatic traversal of
traces.
This diff adds bindings for all public `TraceCursor` methods except
`GetHwClock` and also adds `SBTrace::CreateNewCursor`. A new unittest
has been added to TestTraceLoad.py that uses the new `SBTraceCursor` API
to test that the sequential and random access APIs of the `TraceCursor`
are equivalent.

This diff depends on D130925 .

Test Plan:
`ninja lldb-dotest && ./bin/lldb-dotest -p TestTraceLoad`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130930

Files:
  lldb/bindings/interface/SBTrace.i
  lldb/bindings/interface/SBTraceCursor.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBTrace.h
  lldb/include/lldb/API/SBTraceCursor.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBTrace.cpp
  lldb/source/API/SBTraceCursor.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -264,3 +264,84 @@
 expected_substrs = ['error: missing value at traceBundle.processes[1].pid']
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, error=True, substrs=expected_substrs)
 self.assertEqual(self.dbg.GetNumTargets(), 0)
+
+def testLoadTraceCursor(self):
+src_dir = self.getSourceDir()
+trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
+traceDescriptionFile = lldb.SBFileSpec(trace_description_file_path, True)
+
+error = lldb.SBError()
+trace = self.dbg.LoadTraceFromFile(error, traceDescriptionFile)
+self.assertSBError(error)
+
+target = self.dbg.GetSelectedTarget()
+process = target.process
+
+# Helper function to check equality of the current item of two trace cursors.
+def assertCurrentTraceCursorItemEqual(lhs, rhs):
+self.assertTrue(lhs.HasValue() and rhs.HasValue())
+
+self.assertEqual(lhs.GetId(), rhs.GetId())
+self.assertEqual(lhs.GetItemKind(), rhs.GetItemKind())
+if lhs.IsError():
+self.assertEqual(lhs.GetError(), rhs.GetError())
+elif lhs.IsEvent():
+self.assertEqual(lhs.GetEventType(), rhs.GetEventType())
+self.assertEqual(lhs.GetEventTypeAsString(), rhs.GetEventTypeAsString())
+elif lhs.IsInstruction():
+self.assertEqual(lhs.GetLoadAddress(), rhs.GetLoadAddress())
+else:
+self.fail("Unknown trace item kind")
+
+for thread in process.threads:
+sequentialTraversalCursor = trace.CreateNewCursor(error, thread) 
+self.assertSBError(error)
+# Skip threads with no trace items
+if not sequentialTraversalCursor.HasValue():
+continue
+
+
+# Test "End" boundary of the trace by advancing past the trace's last item. 
+sequentialTraversalCursor.Seek(0, lldb.SBTraceCursor.End)
+self.assertTrue(sequentialTraversalCursor.HasValue())
+sequentialTraversalCursor.SetForwards(True)
+sequentialTraversalCursor.Next()
+self.assertFalse(sequentialTraversalCursor.HasValue())
+
+
+
+# Test sequential traversal using sequential access API (ie Next())
+# and random access API (ie GoToId()) simultaneously.
+randomAccessCursor = trace.CreateNewCursor(error, thread) 
+self.assertSBError(error)
+# Reset the sequential cursor 
+sequentialTraversalCursor.Seek(0, lldb.SBTraceCursor.Beginning)
+sequentialTraversalCursor.SetForwards(True)
+self.assertTrue(sequentialTraversalCursor.IsForwards())
+
+while sequentialTraversalCursor.HasValue():
+itemId = sequentialTraversalCursor.GetId()
+randomAccessCursor.GoToId(itemId)
+assertCurrentTraceCursorItemEqual(sequentialTraversalCursor, randomAccessCursor)
+sequentialTraversalCursor.Next()
+
+
+
+# Test a random access with random access API (ie Seek()) and
+# sequential access API (ie consecutive calls to Next()).
+TEST_SEEK_ID = 3
+randomAccessCursor.GoToId(TEST_SEEK_ID )
+# Reset the sequential cursor 
+sequentialTraversalCursor.Seek(0, lldb.SBTraceCursor.Beginning)
+sequentialTraversalCursor.SetForwards(True)
+f

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/API/SBTraceCursor.h:1-2
+//===-- SBTraceCursor.h ---*- C++
+//-*-===//
+//

Broken ASCII art



Comment at: lldb/source/API/SBTrace.cpp:47
+SBTraceCursor SBTrace::CreateNewCursor(SBError &error, SBThread &thread) {
+  LLDB_INSTRUMENT_VA(this);
+  if (!m_opaque_sp || !thread.get()) {

There should be a newline after `LLDB_INSTRUMENT_VA` to match the output of 
`lldb-instr`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130930

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

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

[walter erquinigo via Phabricator via lldb-commits]

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

pretty nice!! Just some few minor changes and good to go




Comment at: lldb/bindings/interface/SBTraceCursor.i:1-7
+//===-- SWIG Interface for SBTraceCursor.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//

the first and last lines should have 80 cols



Comment at: lldb/bindings/interface/SBTraceCursor.i:61
+
+  lldb::cpu_id_t GetCPU(SBError &error) const;
+};

add an IsValid method. It's common in the SB API to offer such method as an 
alternative to heck the error object.



Comment at: lldb/include/lldb/API/SBTrace.h:38-39
+  /// \return
+  /// A \a SBTraceCursor. If the thread is not traced or its trace
+  /// information failed to load, an \a llvm::Error is returned.
+  SBTraceCursor CreateNewCursor(SBError &error, SBThread &thread);





Comment at: lldb/include/lldb/API/SBTraceCursor.h:1-2
+//===-- SBTraceCursor.h ---*- C++
+//-*-===//
+//

JDevlieghere wrote:
> Broken ASCII art
fix this



Comment at: lldb/include/lldb/API/SBTraceCursor.h:166
+  /// The specific kind of event the cursor is pointing at, or \b
+  /// TraceEvent::eTraceEventNone if the cursor not pointing to an event.
+  lldb::TraceEvent GetEventType() const;

The event eTraceEventNone was deleted, so don't mention it here.



Comment at: lldb/include/lldb/API/SBTraceCursor.h:181-182
+
+  // TODO: should we define LLDB_INVALID_CPU_ID so this matches the behavior of
+  // `GetLoadAddress()`?
+  lldb::cpu_id_t GetCPU(SBError &error) const;

yes, you should. And you can set it to -1



Comment at: lldb/source/API/SBTrace.cpp:48-51
+  if (!m_opaque_sp || !thread.get()) {
+error.SetErrorString("error: invalid trace");
+return SBTraceCursor();
+  }

you should have one error for invalid thread and one for invalid trace



Comment at: lldb/source/API/SBTraceCursor.cpp:17-18
+
+// relevant to the clone issue:
+// 
https://stackoverflow.com/questions/16030081/copy-constructor-for-a-class-with-unique-ptr
+SBTraceCursor::SBTraceCursor() { LLDB_INSTRUMENT_VA(this); }

no need to mention this. You can just create a invalid SBTraceCursor. Don't 
forget to add the IsValid method



Comment at: lldb/source/API/SBTraceCursor.cpp:63-64
+  LLDB_INSTRUMENT_VA(this, offset, origin);
+  // Convert from public `SBTraceCursor::SeekType` enum to private
+  // `TraceCursor::SeekType` enum.
+  auto convert_seek_type_enum = [](SeekType seek_type) {

You should instead create a new public enumeration TraceCursorSeekType. That 
will simplify the code



Comment at: lldb/source/API/SBTraceCursor.cpp:119-120
+
+// TODO: should we define LLDB_INVALID_CPU_ID so this matches the behavior of
+// `GetLoadAddress()`?
+lldb::cpu_id_t SBTraceCursor::GetCPU(SBError &error) const {

yes, define a LLDB_INVALID_CPU_ID and don't return an error here



Comment at: lldb/test/API/commands/trace/TestTraceLoad.py:342-346
+
+
+
+
+

add one more test that simply walks through a few items expecting some 
hardcoded values. That kind of test are easier to fix when something minor 
breaks. The other kind of tests, like the one you just wrote above, are more 
complicated to fix but they are much stronger test. Both kinds of tests are 
useful


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130930

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

[Lldb-commits] [PATCH] D130937: [LLDB][NFC][Correctness] Fix bad null check

[Slava Gurevich via Phabricator via lldb-commits]

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

Fix incorrect null-check logic, likely cause by copy-paste


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130937

Files:
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp


Index: lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
===
--- lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
+++ lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
@@ -161,7 +161,7 @@
   GetAddressByteSize(), bytes_read);
   if (!status.Success())
 return status.ToError();
-  if (address == 0)
+  if (link_map == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Invalid link_map address");
 


Index: lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
===
--- lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
+++ lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
@@ -161,7 +161,7 @@
   GetAddressByteSize(), bytes_read);
   if (!status.Success())
 return status.ToError();
-  if (address == 0)
+  if (link_map == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Invalid link_map address");
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3bec33b - [trace] Replace TraceCursorUP with TraceCursorSP

[Jakob Johnson via lldb-commits]

Author: Jakob Johnson
Date: 2022-08-01T13:53:53-07:00
New Revision: 3bec33b16db11c67d43bda134520a2132ff606c9

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

LOG: [trace] Replace TraceCursorUP with TraceCursorSP

The use of `std::unique_ptr` with `TraceCursor` adds unnecessary complexity to 
adding `SBTraceCursor` bindings
Specifically, since `TraceCursor` is an abstract class there's no clean
way to provide "deep clone" semantics for `TraceCursorUP` short of
creating a pure virtual `clone()` method (afaict).

After discussing with @wallace, we decided there is no strong reason to
favor wrapping `TraceCursor` with `std::unique_ptr` over `std::shared_ptr`, 
thus this diff
replaces all usages of `std::unique_ptr` with 
`std::shared_ptr`.

This sets the stage for future diffs to introduce `SBTraceCursor`
bindings in a more clean fashion.

Test Plan:

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

Added: 


Modified: 
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Target/TraceCursor.h
lldb/include/lldb/Target/TraceDumper.h
lldb/include/lldb/lldb-forward.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
lldb/source/Target/TraceDumper.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index beae9e28417d..917e66992ad1 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -169,9 +169,9 @@ class Trace : public PluginInterface,
   /// Get a \a TraceCursor for the given thread's trace.
   ///
   /// \return
-  /// A \a TraceCursorUP. If the thread is not traced or its trace
+  /// A \a TraceCursorSP. If the thread is not traced or its trace
   /// information failed to load, an \a llvm::Error is returned.
-  virtual llvm::Expected
+  virtual llvm::Expected
   CreateNewCursor(Thread &thread) = 0;
 
   /// Dump general info about a given thread's trace. Each Trace plug-in

diff  --git a/lldb/include/lldb/Target/TraceCursor.h 
b/lldb/include/lldb/Target/TraceCursor.h
index 95b022331634..4e405aeaab7c 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -52,7 +52,7 @@ namespace lldb_private {
 ///
 /// Sample usage:
 ///
-///  TraceCursorUP cursor = trace.GetTrace(thread);
+///  TraceCursorSP cursor = trace.GetTrace(thread);
 ///
 ///  for (; cursor->HasValue(); cursor->Next()) {
 /// TraceItemKind kind = cursor->GetItemKind();

diff  --git a/lldb/include/lldb/Target/TraceDumper.h 
b/lldb/include/lldb/Target/TraceDumper.h
index ada779990e07..ab3f77916751 100644
--- a/lldb/include/lldb/Target/TraceDumper.h
+++ b/lldb/include/lldb/Target/TraceDumper.h
@@ -93,7 +93,7 @@ class TraceDumper {
   ///
   /// \param[in] options
   /// Additional options for configuring the dumping.
-  TraceDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+  TraceDumper(lldb::TraceCursorSP cursor_sp, Stream &s,
   const TraceDumperOptions &options);
 
   /// Dump \a count instructions of the thread trace starting at the current
@@ -114,7 +114,7 @@ class TraceDumper {
   /// Create a trace item for the current position without symbol information.
   TraceItem CreatRawTraceItem();
 
-  lldb::TraceCursorUP m_cursor_up;
+  lldb::TraceCursorSP m_cursor_sp;
   TraceDumperOptions m_options;
   std::unique_ptr m_writer_up;
 };

diff  --git a/lldb/include/lldb/lldb-forward.h 
b/lldb/include/lldb/lldb-forward.h
index c51e1850338f..fd88a45ba06f 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -420,7 +420,7 @@ typedef std::weak_ptr 
ThreadPlanWP;
 typedef std::shared_ptr ThreadPlanTracerSP;
 typedef std::shared_ptr TraceSP;
 typedef std::unique_ptr TraceExporterUP;
-typedef std::unique_ptr TraceCursorUP;
+typedef std::shared_ptr TraceCursorSP;
 typedef std::shared_ptr TypeSP;
 typedef std::weak_ptr TypeWP;
 typedef std::shared_ptr TypeCategoryImplSP;

diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index fe0cb0945cde..9aa128aaa288 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -2269,17 +2269,17 @@ class CommandObjectTraceDumpInstructions : public 
CommandObjectParsed {
   m_options.m_dumper_options.id = m_last_id;
 }
 
-llvm::Expected cursor_or_error =
+llvm::Expected cursor_or_error =
 m_exe_ctx.GetTargetSP()->GetTrace()->CreateNewCursor(*thread_sp);
 
 if (!cursor_or_error) {
   result.AppendError(llvm::toString(cursor_or_error.takeError()));
   return f

[Lldb-commits] [PATCH] D130925: [trace] Replace TraceCursorUP with TraceCursorSP

[Jakob Johnson via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3bec33b16db1: [trace] Replace TraceCursorUP with 
TraceCursorSP (authored by jj10306).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130925

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceDumper.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -295,32 +295,32 @@
 new OutputWriterCLI(s, options, thread));
 }
 
-TraceDumper::TraceDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+TraceDumper::TraceDumper(lldb::TraceCursorSP cursor_sp, Stream &s,
  const TraceDumperOptions &options)
-: m_cursor_up(std::move(cursor_up)), m_options(options),
+: m_cursor_sp(std::move(cursor_sp)), m_options(options),
   m_writer_up(CreateWriter(
-  s, m_options, *m_cursor_up->GetExecutionContextRef().GetThreadSP())) {
+  s, m_options, *m_cursor_sp->GetExecutionContextRef().GetThreadSP())) {
 
   if (m_options.id)
-m_cursor_up->GoToId(*m_options.id);
+m_cursor_sp->GoToId(*m_options.id);
   else if (m_options.forwards)
-m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
+m_cursor_sp->Seek(0, TraceCursor::SeekType::Beginning);
   else
-m_cursor_up->Seek(0, TraceCursor::SeekType::End);
+m_cursor_sp->Seek(0, TraceCursor::SeekType::End);
 
-  m_cursor_up->SetForwards(m_options.forwards);
+  m_cursor_sp->SetForwards(m_options.forwards);
   if (m_options.skip) {
-m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
+m_cursor_sp->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
   TraceCursor::SeekType::Current);
   }
 }
 
 TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
   TraceItem item = {};
-  item.id = m_cursor_up->GetId();
+  item.id = m_cursor_sp->GetId();
 
   if (m_options.show_timestamps)
-item.timestamp = m_cursor_up->GetWallClockTime();
+item.timestamp = m_cursor_sp->GetWallClockTime();
   return item;
 }
 
@@ -378,7 +378,7 @@
 }
 
 Optional TraceDumper::DumpInstructions(size_t count) {
-  ThreadSP thread_sp = m_cursor_up->GetExecutionContextRef().GetThreadSP();
+  ThreadSP thread_sp = m_cursor_sp->GetExecutionContextRef().GetThreadSP();
 
   SymbolInfo prev_symbol_info;
   Optional last_id;
@@ -386,32 +386,32 @@
   ExecutionContext exe_ctx;
   thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx);
 
-  for (size_t insn_seen = 0; insn_seen < count && m_cursor_up->HasValue();
-   m_cursor_up->Next()) {
+  for (size_t insn_seen = 0; insn_seen < count && m_cursor_sp->HasValue();
+   m_cursor_sp->Next()) {
 
-last_id = m_cursor_up->GetId();
+last_id = m_cursor_sp->GetId();
 TraceItem item = CreatRawTraceItem();
 
-if (m_cursor_up->IsEvent()) {
+if (m_cursor_sp->IsEvent()) {
   if (!m_options.show_events)
 continue;
-  item.event = m_cursor_up->GetEventType();
+  item.event = m_cursor_sp->GetEventType();
   switch (*item.event) {
   case eTraceEventCPUChanged:
-item.cpu_id = m_cursor_up->GetCPU();
+item.cpu_id = m_cursor_sp->GetCPU();
 break;
   case eTraceEventHWClockTick:
-item.hw_clock = m_cursor_up->GetHWClock();
+item.hw_clock = m_cursor_sp->GetHWClock();
 break;
   case eTraceEventDisabledHW:
   case eTraceEventDisabledSW:
 break;
   }
-} else if (m_cursor_up->IsError()) {
-  item.error = m_cursor_up->GetError();
+} else if (m_cursor_sp->IsError()) {
+  item.error = m_cursor_sp->GetError();
 } else {
   insn_seen++;
-  item.load_address = m_cursor_up->GetLoadAddress();
+  item.load_address = m_cursor_sp->GetLoadAddress();
 
   if (!m_options.raw) {
 SymbolInfo symbol_info;
@@ -429,7 +429,7 @@
 }
 m_writer_up->TraceItem(item);
   }
-  if (!m_cursor_up->HasValue())
+  if (!m_cursor_sp->HasValue())
 m_writer_up->NoMoreData();
   return last_id;
 }
Index: lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
===
--- lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
+++ lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
@@ -81,7 +81,7 @@
 return false;
   } else {
 auto do_work = [&]() -> Error {
-  Expected cursor = trace_sp->CreateNewCursor(*thread);

[Lldb-commits] [PATCH] D130939: [LLDB][NFC] Fix potential div by 0 "count" can be zero potentially causing div by 0

[Slava Gurevich via Phabricator via lldb-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130939

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


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2536,7 +2536,9 @@
   (1024.0 * 1024.0);
 float packets_per_second =
 ((float)packet_count) / duration(total_time).count();
-const auto average_per_packet = total_time / packet_count;
+const auto average_per_packet = packet_count > 0
+? total_time / packet_count
+: duration::zero();
 
 if (json) {
   strm.Format("{0}\n {{\"send_size\" : {1,6}, \"recv_size\" : "


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2536,7 +2536,9 @@
   (1024.0 * 1024.0);
 float packets_per_second =
 ((float)packet_count) / duration(total_time).count();
-const auto average_per_packet = total_time / packet_count;
+const auto average_per_packet = packet_count > 0
+? total_time / packet_count
+: duration::zero();
 
 if (json) {
   strm.Format("{0}\n {{\"send_size\" : {1,6}, \"recv_size\" : "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130942: [LLDB][DWARF] Set MSInheritanceAttr for record decl when it's using Microsoft compatible ABI.

[Zequan Wu via Phabricator via lldb-commits]

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

This is needed for object files with MS ABI and debug info in Dwarf.
MSInheritanceAttr is required for record decls under MS ABI.

Fixes #56458


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130942

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/ms_abi.cpp

Index: lldb/test/Shell/SymbolFile/DWARF/x86/ms_abi.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/ms_abi.cpp
@@ -0,0 +1,56 @@
+// REQUIRES: lld
+
+// RUN: %clang -c --target=x86_64-windows-msvc -gdwarf %s -o %t.obj
+// RUN: lld-link /out:%t.exe %t.obj /nodefaultlib /entry:main /debug
+// RUN: %lldb -f %t.exe \
+// RUN:   -o "target variable mp1 mp2 mp3 mp4 mp5 mp6 mp7 mp8 mp9" -o "exit" \
+// RUN:   | FileCheck %s
+
+// The following tests that MSInheritanceAttr are set for record decls.
+class SI {
+  int si;
+};
+struct SI2 {
+  int si2;
+};
+class MI : SI, SI2 {
+  int mi;
+};
+class MI2 : MI {
+  int mi2;
+};
+class VI : virtual MI {
+  int vi;
+};
+class VI2 : virtual SI, virtual SI2 {
+  int vi;
+};
+class /* __unspecified_inheritance*/ UI;
+
+typedef void (SI::*SITYPE)();
+typedef void (MI::*MITYPE)();
+typedef void (MI2::*MI2TYPE)();
+typedef void (VI::*VITYPE)();
+typedef void (VI2::*VI2TYPE)();
+typedef void (UI::*UITYPE)();
+SITYPE mp1 = nullptr;
+MITYPE mp2 = nullptr;
+MI2TYPE mp3 = nullptr;
+VITYPE mp4 = nullptr;
+VI2TYPE mp5 = nullptr;
+UITYPE mp6 = nullptr;
+MITYPE *mp7 = nullptr;
+VI2TYPE *mp8 = nullptr;
+int SI::*mp9 = nullptr;
+
+// CHECK: (SITYPE) mp1 = 00 00 00 00 00 00 00 00
+// CHECK: (MITYPE) mp2 = 00 00 00 00 00 00 00 00
+// CHECK: (MI2TYPE) mp3 = 00 00 00 00 00 00 00 00
+// CHECK: (VITYPE) mp4 = 00 00 00 00 00 00 00 00
+// CHECK: (VI2TYPE) mp5 = 00 00 00 00 00 00 00 00
+// CHECK: (UITYPE) mp6 = 00 00 00 00 00 00 00 00
+// CHECK: (MITYPE *) mp7 = 0x
+// CHECK: (VI2TYPE *) mp8 = 0x
+// CHECK: (int SI::*) mp9 = ff ff ff ff
+
+int main() {}
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -816,7 +816,7 @@
 clang::QualType class_type = GetOrCreateType(mpi.ContainingType);
 if (class_type.isNull())
   return {};
-if (clang::TagDecl *tag = class_type->getAsTagDecl()) {
+if (clang::CXXRecordDecl *record_decl = class_type->getAsCXXRecordDecl()) {
   clang::MSInheritanceAttr::Spelling spelling;
   switch (mpi.Representation) {
   case llvm::codeview::PointerToMemberRepresentation::SingleInheritanceData:
@@ -847,7 +847,7 @@
 spelling = clang::MSInheritanceAttr::Spelling::SpellingNotCalculated;
 break;
   }
-  tag->addAttr(clang::MSInheritanceAttr::CreateImplicit(
+  record_decl->addAttr(clang::MSInheritanceAttr::CreateImplicit(
   m_clang.getASTContext(), spelling));
 }
 return m_clang.getASTContext().getMemberPointerType(
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1743,6 +1743,18 @@
 }
   }
 
+  // Microsoft abi requires tag decl to have MSInheritanceAttr.
+  if (m_ast.getASTContext().getTargetInfo().getCXXABI().isMicrosoft() &&
+  clang_type_was_created) {
+clang::QualType class_qt =
+clang::QualType::getFromOpaquePtr(clang_type.GetOpaqueQualType());
+if (clang::CXXRecordDecl *record_decl = class_qt->getAsCXXRecordDecl()) {
+  record_decl->addAttr(clang::MSInheritanceAttr::CreateImplicit(
+  m_ast.getASTContext(),
+  clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance));
+}
+  }
+
   // Store a forward declaration to this class type in case any
   // parameters in any class methods need it for the clang types for
   // function prototypes.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130937: [LLDB][NFC][Correctness] Fix bad null check

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130937

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

[Lldb-commits] [lldb] 30b3911 - [LLDB][NFC][Correctness] Fix bad null check

[Slava Gurevich via lldb-commits]

Author: Slava Gurevich
Date: 2022-08-01T14:45:26-07:00
New Revision: 30b39111973798451397a1360dc7abc3e2490c84

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

LOG: [LLDB][NFC][Correctness] Fix bad null check

Fix incorrect null-check logic, likely cause by copy-paste

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

Added: 


Modified: 
lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp 
b/lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
index cbaa1fc7a2b12..fe64da873d22a 100644
--- a/lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
+++ b/lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
@@ -161,7 +161,7 @@ NativeProcessELF::GetLoadedSVR4Libraries() {
   GetAddressByteSize(), bytes_read);
   if (!status.Success())
 return status.ToError();
-  if (address == 0)
+  if (link_map == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Invalid link_map address");
 



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

[Lldb-commits] [PATCH] D130937: [LLDB][NFC][Correctness] Fix bad null check

[Slava Gurevich via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG30b391119737: [LLDB][NFC][Correctness] Fix bad null check 
(authored by fixathon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130937

Files:
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp


Index: lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
===
--- lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
+++ lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
@@ -161,7 +161,7 @@
   GetAddressByteSize(), bytes_read);
   if (!status.Success())
 return status.ToError();
-  if (address == 0)
+  if (link_map == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Invalid link_map address");
 


Index: lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
===
--- lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
+++ lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
@@ -161,7 +161,7 @@
   GetAddressByteSize(), bytes_read);
   if (!status.Success())
 return status.ToError();
-  if (address == 0)
+  if (link_map == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Invalid link_map address");
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130942: [LLDB][DWARF] Set MSInheritanceAttr for record decl when it's using Microsoft compatible ABI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

This looks sensible to me, although it might be good if someone else more 
familiar with this code has a look too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130942

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

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

[Alex Langford via Phabricator via lldb-commits]

bulbazord marked an inline comment as done.
bulbazord added inline comments.



Comment at: lldb/source/Core/Module.cpp:740
+  bool user_provided_name_is_mangled =
+  Mangled::GetManglingScheme(m_name.GetStringRef()) !=
+  Mangled::eManglingSchemeNone;

labath wrote:
> labath wrote:
> > I think this is overly aggressive. `_Z3foov` could be a method name in some 
> > particularly sadistic class. I think you can do this optimization only in 
> > one direction: if the names match, then return true without consulting the 
> > language plugin. At that point, I don't think you even need to check 
> > whether the names are mangled.
> Actually, this could go wrong even for names like `_Zonk`, since 
> `GetManglingScheme` does not check that the string is an actually valid 
> mangled name -- just that it looks like one from very far away.
Then in that case, perhaps this method should always take a demangled name? If 
it can take mangled names, it seems like a giant pain and potentially pretty 
expensive to figure out if it's mangled or not, especially given that in the 
sadistic case a name could look mangled but actually be the demangled name. Am 
I following your thought correctly?


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

https://reviews.llvm.org/D129682

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

[Lldb-commits] [PATCH] D130796: [LLDB][NativePDB] Switch to use DWARFLocationList.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

In D130796#3691227 , @zequanwu wrote:

>> If you find yourself needing to do extra work to work its limitations, we 
>> should fix that algorithm instead.
>
> That makes sense. I'll work on fixing `RangeVectorData`.

After thinking it again, not just `RangeVectorData` need to be written but also 
`RangeVector`, although they are similar. It requires rewrite the whole file 
`RangeMap.h` and all other places uses its APIs.
Can we just use `RangeVectorData` as it is right now and fix it later? So, this 
patch is not blocked. Otherwise, this local variables functionality is not 
almost unusable for NativePDB plugin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130796

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

[Lldb-commits] [PATCH] D62764: Detect x86 mid-function epilogues that end in a jump

[Slava Gurevich via Phabricator via lldb-commits]

fixathon added a comment.
Herald added subscribers: JDevlieghere, pengfei.
Herald added a project: All.

flagging suspicious duplicate code




Comment at: 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:707-713
+  if (second_byte_sans_reg == 0x24)
+return true;
+
+  // use SIB byte
+  // ff 24 fe  jmpq   *(%rsi,%rdi,8)
+  if (second_byte_sans_reg == 0x24)
+return true;

This looks like duplicate code


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62764

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

[Lldb-commits] [PATCH] D130813: Add ability for lldb to load binaries in a process/corefile given only a load address & no dynamic loader

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 449159.
jasonmolenda added a comment.

I was using an obsoleted linker flag to seg the vmaddrs of two dylibs and 
laying them out there in the corefile.  This won't work long-term, and I really 
wanted to make sure lldb will slide binaries correctly, so I added a new 
feature to my corefile creator which will apply a slide to the binaries it puts 
in the corefiles, modifying the segment/section load commands with the slid 
vmaddrs as if dyld had put them there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130813

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/test/API/macosx/lc-note/multiple-binary-corefile/Makefile
  
lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
  
lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
  lldb/test/API/macosx/lc-note/multiple-binary-corefile/main.c
  lldb/test/API/macosx/lc-note/multiple-binary-corefile/one.c
  lldb/test/API/macosx/lc-note/multiple-binary-corefile/two.c

Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/two.c
===
--- /dev/null
+++ lldb/test/API/macosx/lc-note/multiple-binary-corefile/two.c
@@ -0,0 +1 @@
+int two() { return 10; }
Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/one.c
===
--- /dev/null
+++ lldb/test/API/macosx/lc-note/multiple-binary-corefile/one.c
@@ -0,0 +1 @@
+int one() { return 5; }
Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/lc-note/multiple-binary-corefile/main.c
@@ -0,0 +1,7 @@
+#include 
+int one();
+int two();
+int main() {
+  puts("this is the standalone binary test program");
+  return one() + two();
+}
Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
===
--- /dev/null
+++ lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
@@ -0,0 +1,478 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Given a list of binaries that load into different vmaddrs,
+// create a corefile with `main bin spec` and `load binary`
+// LC_NOTEs that *only* lists the vmaddrs, no uuid or filename,
+// and copy the binaries into the corefile.  Test whether lldb
+// can read the binaries out of memory, get the UUID, and find
+// the correct binaries.
+
+struct main_bin_spec_payload {
+  uint32_t version;
+  uint32_t type;
+  uint64_t address;
+  uint64_t slide;
+  uuid_t uuid;
+  uint32_t log2_pagesize;
+  uint32_t platform;
+};
+
+struct load_binary_payload {
+  uint32_t version;
+  uuid_t uuid;
+  uint64_t address;
+  uint64_t slide;
+  const char name[4];
+};
+
+union uint32_buf {
+  uint8_t bytebuf[4];
+  uint32_t val;
+};
+
+union uint64_buf {
+  uint8_t bytebuf[8];
+  uint64_t val;
+};
+
+void add_uint64(std::vector &buf, uint64_t val) {
+  uint64_buf conv;
+  conv.val = val;
+  for (int i = 0; i < 8; i++)
+buf.push_back(conv.bytebuf[i]);
+}
+
+void add_uint32(std::vector &buf, uint32_t val) {
+  uint32_buf conv;
+  conv.val = val;
+  for (int i = 0; i < 4; i++)
+buf.push_back(conv.bytebuf[i]);
+}
+
+std::vector lc_thread_load_command(cpu_type_t cputype) {
+  std::vector data;
+  // Emit an LC_THREAD register context appropriate for the cputype
+  // of the binary we're embedded.  The tests in this case do not
+  // use the register values, so 0's are fine, lldb needs to see at
+  // least one LC_THREAD in the corefile.
+#if defined(__x86_64__)
+  if (cputype == CPU_TYPE_X86_64) {
+add_uint32(data, LC_THREAD); // thread_command.cmd
+add_uint32(data,
+   16 + (x86_THREAD_STATE64_COUNT * 4)); // thread_command.cmdsize
+add_uint32(data, x86_THREAD_STATE64);// thread_command.flavor
+add_uint32(data, x86_THREAD_STATE64_COUNT);  // thread_command.count
+for (int i = 0; i < x86_THREAD_STATE64_COUNT; i++) {
+  add_uint32(data, 0); // whatever, just some empty register values
+}
+  }
+#endif
+#if defined(__arm64__) || defined(__aarch64__)
+  if (cputype == CPU_TYPE_ARM64) {
+add_uint32(data, LC_THREAD); // thread_command.cmd
+add_uint32(data,
+ 

[Lldb-commits] [PATCH] D62764: Detect x86 mid-function epilogues that end in a jump

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:707-713
+  if (second_byte_sans_reg == 0x24)
+return true;
+
+  // use SIB byte
+  // ff 24 fe  jmpq   *(%rsi,%rdi,8)
+  if (second_byte_sans_reg == 0x24)
+return true;

fixathon wrote:
> This looks like duplicate code
At least it does the same thing in both of them!  :) 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62764

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

[Lldb-commits] [PATCH] D62764: Detect x86 mid-function epilogues that end in a jump

[Slava Gurevich via Phabricator via lldb-commits]

fixathon added inline comments.



Comment at: 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:707-713
+  if (second_byte_sans_reg == 0x24)
+return true;
+
+  // use SIB byte
+  // ff 24 fe  jmpq   *(%rsi,%rdi,8)
+  if (second_byte_sans_reg == 0x24)
+return true;

jasonmolenda wrote:
> fixathon wrote:
> > This looks like duplicate code
> At least it does the same thing in both of them!  :) 
It does indeed, but the comments above the code are different for the 2 
identical blocks of code. Do you want to fix it then? :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62764

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

[Lldb-commits] [lldb] 768e59d - [LLDB][RISCV] Add riscv register enums

[via lldb-commits]

Author: Emmmer
Date: 2022-08-02T11:55:33+08:00
New Revision: 768e59d959c7e23e98cda1b08c5b6b68dbc1d2a7

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

LOG: [LLDB][RISCV] Add riscv register enums

According to [RISC-V ISA 
Spec](https://riscv.org/wp-content/uploads/2017/05/riscv-spec-v2.2.pdf) and 
[riscv-v-spec](https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#3-vector-extension-programmers-model)

Reviewed By: DavidSpickett

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

Added: 
lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h

Modified: 


Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h 
b/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
new file mode 100644
index ..9acf181b4a56
--- /dev/null
+++ b/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
@@ -0,0 +1,139 @@
+//===-- lldb-riscv-register-enums.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
+#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
+
+// LLDB register codes (e.g. RegisterKind == eRegisterKindLLDB)
+
+// Internal codes for all riscv registers.
+enum {
+  k_first_gpr_riscv,
+  gpr_x0_riscv = k_first_gpr_riscv,
+  gpr_x1_riscv,
+  gpr_x2_riscv,
+  gpr_x3_riscv,
+  gpr_x4_riscv,
+  gpr_x5_riscv,
+  gpr_x6_riscv,
+  gpr_x7_riscv,
+  gpr_x8_riscv,
+  gpr_x9_riscv,
+  gpr_x10_riscv,
+  gpr_x11_riscv,
+  gpr_x12_riscv,
+  gpr_x13_riscv,
+  gpr_x14_riscv,
+  gpr_x15_riscv,
+  gpr_x16_riscv,
+  gpr_x17_riscv,
+  gpr_x18_riscv,
+  gpr_x19_riscv,
+  gpr_x20_riscv,
+  gpr_x21_riscv,
+  gpr_x22_riscv,
+  gpr_x23_riscv,
+  gpr_x24_riscv,
+  gpr_x25_riscv,
+  gpr_x26_riscv,
+  gpr_x27_riscv,
+  gpr_x28_riscv,
+  gpr_x29_riscv,
+  gpr_x30_riscv,
+  gpr_x31_riscv,
+  gpr_pc_riscv,
+
+  k_last_gpr_riscv = gpr_pc_riscv,
+
+  k_first_fpr_riscv,
+  fpr_f0_riscv = k_first_fpr_riscv,
+  fpr_f1_riscv,
+  fpr_f2_riscv,
+  fpr_f3_riscv,
+  fpr_f4_riscv,
+  fpr_f5_riscv,
+  fpr_f6_riscv,
+  fpr_f7_riscv,
+  fpr_f8_riscv,
+  fpr_f9_riscv,
+  fpr_f10_riscv,
+  fpr_f11_riscv,
+  fpr_f12_riscv,
+  fpr_f13_riscv,
+  fpr_f14_riscv,
+  fpr_f15_riscv,
+  fpr_f16_riscv,
+  fpr_f17_riscv,
+  fpr_f18_riscv,
+  fpr_f19_riscv,
+  fpr_f20_riscv,
+  fpr_f21_riscv,
+  fpr_f22_riscv,
+  fpr_f23_riscv,
+  fpr_f24_riscv,
+  fpr_f25_riscv,
+  fpr_f26_riscv,
+  fpr_f27_riscv,
+  fpr_f28_riscv,
+  fpr_f29_riscv,
+  fpr_f30_riscv,
+  fpr_f31_riscv,
+  fpr_fflags_riscv,
+  fpr_frm_riscv,
+  fpr_fcsr_riscv,
+  k_last_fpr_riscv = fpr_fcsr_riscv,
+
+  k_first_vcr_riscv,
+  vcr_v0_riscv = k_first_vcr_riscv,
+  vcr_v1_riscv,
+  vcr_v2_riscv,
+  vcr_v3_riscv,
+  vcr_v4_riscv,
+  vcr_v5_riscv,
+  vcr_v6_riscv,
+  vcr_v7_riscv,
+  vcr_v8_riscv,
+  vcr_v9_riscv,
+  vcr_v10_riscv,
+  vcr_v11_riscv,
+  vcr_v12_riscv,
+  vcr_v13_riscv,
+  vcr_v14_riscv,
+  vcr_v15_riscv,
+  vcr_v16_riscv,
+  vcr_v17_riscv,
+  vcr_v18_riscv,
+  vcr_v19_riscv,
+  vcr_v20_riscv,
+  vcr_v21_riscv,
+  vcr_v22_riscv,
+  vcr_v23_riscv,
+  vcr_v24_riscv,
+  vcr_v25_riscv,
+  vcr_v26_riscv,
+  vcr_v27_riscv,
+  vcr_v28_riscv,
+  vcr_v29_riscv,
+  vcr_v30_riscv,
+  vcr_v31_riscv,
+  vcr_vstart_riscv,
+  vcr_vxsat_riscv,
+  vcr_vxrm_riscv,
+  vcr_vcsr_riscv,
+  vcr_vl_riscv,
+  vcr_vtype_riscv,
+  vcr_vlenb_riscv,
+  k_last_vcr_riscv = vcr_vlenb_riscv,
+
+  k_num_registers_riscv,
+  k_num_gpr_registers_riscv = k_last_gpr_riscv - k_first_gpr_riscv + 1,
+  k_num_fpr_registers_riscv = k_last_fpr_riscv - k_first_fpr_riscv + 1,
+  k_num_vcr_registers_riscv = k_last_vcr_riscv - k_first_vcr_riscv + 1,
+};
+
+#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H



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

[Lldb-commits] [PATCH] D130899: [LLDB][RISCV] Add riscv register enums

[Emmmer S via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG768e59d959c7: [LLDB][RISCV] Add riscv register enums 
(authored by Emmmer).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130899

Files:
  lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h

Index: lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
@@ -0,0 +1,139 @@
+//===-- lldb-riscv-register-enums.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
+#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
+
+// LLDB register codes (e.g. RegisterKind == eRegisterKindLLDB)
+
+// Internal codes for all riscv registers.
+enum {
+  k_first_gpr_riscv,
+  gpr_x0_riscv = k_first_gpr_riscv,
+  gpr_x1_riscv,
+  gpr_x2_riscv,
+  gpr_x3_riscv,
+  gpr_x4_riscv,
+  gpr_x5_riscv,
+  gpr_x6_riscv,
+  gpr_x7_riscv,
+  gpr_x8_riscv,
+  gpr_x9_riscv,
+  gpr_x10_riscv,
+  gpr_x11_riscv,
+  gpr_x12_riscv,
+  gpr_x13_riscv,
+  gpr_x14_riscv,
+  gpr_x15_riscv,
+  gpr_x16_riscv,
+  gpr_x17_riscv,
+  gpr_x18_riscv,
+  gpr_x19_riscv,
+  gpr_x20_riscv,
+  gpr_x21_riscv,
+  gpr_x22_riscv,
+  gpr_x23_riscv,
+  gpr_x24_riscv,
+  gpr_x25_riscv,
+  gpr_x26_riscv,
+  gpr_x27_riscv,
+  gpr_x28_riscv,
+  gpr_x29_riscv,
+  gpr_x30_riscv,
+  gpr_x31_riscv,
+  gpr_pc_riscv,
+
+  k_last_gpr_riscv = gpr_pc_riscv,
+
+  k_first_fpr_riscv,
+  fpr_f0_riscv = k_first_fpr_riscv,
+  fpr_f1_riscv,
+  fpr_f2_riscv,
+  fpr_f3_riscv,
+  fpr_f4_riscv,
+  fpr_f5_riscv,
+  fpr_f6_riscv,
+  fpr_f7_riscv,
+  fpr_f8_riscv,
+  fpr_f9_riscv,
+  fpr_f10_riscv,
+  fpr_f11_riscv,
+  fpr_f12_riscv,
+  fpr_f13_riscv,
+  fpr_f14_riscv,
+  fpr_f15_riscv,
+  fpr_f16_riscv,
+  fpr_f17_riscv,
+  fpr_f18_riscv,
+  fpr_f19_riscv,
+  fpr_f20_riscv,
+  fpr_f21_riscv,
+  fpr_f22_riscv,
+  fpr_f23_riscv,
+  fpr_f24_riscv,
+  fpr_f25_riscv,
+  fpr_f26_riscv,
+  fpr_f27_riscv,
+  fpr_f28_riscv,
+  fpr_f29_riscv,
+  fpr_f30_riscv,
+  fpr_f31_riscv,
+  fpr_fflags_riscv,
+  fpr_frm_riscv,
+  fpr_fcsr_riscv,
+  k_last_fpr_riscv = fpr_fcsr_riscv,
+
+  k_first_vcr_riscv,
+  vcr_v0_riscv = k_first_vcr_riscv,
+  vcr_v1_riscv,
+  vcr_v2_riscv,
+  vcr_v3_riscv,
+  vcr_v4_riscv,
+  vcr_v5_riscv,
+  vcr_v6_riscv,
+  vcr_v7_riscv,
+  vcr_v8_riscv,
+  vcr_v9_riscv,
+  vcr_v10_riscv,
+  vcr_v11_riscv,
+  vcr_v12_riscv,
+  vcr_v13_riscv,
+  vcr_v14_riscv,
+  vcr_v15_riscv,
+  vcr_v16_riscv,
+  vcr_v17_riscv,
+  vcr_v18_riscv,
+  vcr_v19_riscv,
+  vcr_v20_riscv,
+  vcr_v21_riscv,
+  vcr_v22_riscv,
+  vcr_v23_riscv,
+  vcr_v24_riscv,
+  vcr_v25_riscv,
+  vcr_v26_riscv,
+  vcr_v27_riscv,
+  vcr_v28_riscv,
+  vcr_v29_riscv,
+  vcr_v30_riscv,
+  vcr_v31_riscv,
+  vcr_vstart_riscv,
+  vcr_vxsat_riscv,
+  vcr_vxrm_riscv,
+  vcr_vcsr_riscv,
+  vcr_vl_riscv,
+  vcr_vtype_riscv,
+  vcr_vlenb_riscv,
+  k_last_vcr_riscv = vcr_vlenb_riscv,
+
+  k_num_registers_riscv,
+  k_num_gpr_registers_riscv = k_last_gpr_riscv - k_first_gpr_riscv + 1,
+  k_num_fpr_registers_riscv = k_last_fpr_riscv - k_first_fpr_riscv + 1,
+  k_num_vcr_registers_riscv = k_last_vcr_riscv - k_first_vcr_riscv + 1,
+};
+
+#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_RISCV_REGISTER_ENUMS_H
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62764: Detect x86 mid-function epilogues that end in a jump

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: 
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:707-713
+  if (second_byte_sans_reg == 0x24)
+return true;
+
+  // use SIB byte
+  // ff 24 fe  jmpq   *(%rsi,%rdi,8)
+  if (second_byte_sans_reg == 0x24)
+return true;

fixathon wrote:
> jasonmolenda wrote:
> > fixathon wrote:
> > > This looks like duplicate code
> > At least it does the same thing in both of them!  :) 
> It does indeed, but the comments above the code are different for the 2 
> identical blocks of code. Do you want to fix it then? :)
:) I see there's a comment intended for this first entry a dozen lines earlier 
in the source file, it's probably gotten confusing over time & edits.  yah I'll 
fix it, thanks for catching it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62764

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