[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[WÁNG Xuěruì via Phabricator via lldb-commits]

xen0n added a comment.

Hi, I've edited the summary and patch title for you. It's generally not 
necessary to add //that// much "politeness" when most of it is obvious from 
context (e.g. the fact that you're new face here, that there's obviously no 
LoongArch support in LLDB, and most of the methods being stubs). It didn't help 
that much of the text was in Chinglish either.

As for the changes, they look reasonable to me, but as I haven't tested it out 
myself yet (and unable to, due to it being stub-only), I'll not give the 
approval myself this time. Thanks for your contribution and welcome!


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I don't have the expertise to review, but have one question.

Does this account for the situation where you spawn many gdbserver from the 
platform and therefore need more ports? Does it even need to? (just guessing 
that that could have been the reason for selecting random ports, though not for 
the platform)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[Luka Markušić via Phabricator via lldb-commits]

mark2185 added a comment.

In D136465#3878581 , @DavidSpickett 
wrote:

> Does this account for the situation where you spawn many gdbserver from the 
> platform and therefore need more ports? Does it even need to? (just guessing 
> that that could have been the reason for selecting random ports, though not 
> for the platform)

To be honest, I don't know. In all of my testing of practical use cases (i.e. 
launching an app and debugging it either through `CodeLLDB/vscode-lldb` or 
`Android Studio`), I've only ever seen two ports. One for the platform (when 
running `platform connect`), and one for `gdb` (when trying to `attach` to a 
`PID`).

`git blame` points to this , as previously it 
was using the port specified when launching `lldb-server` for the `local` port, 
but it turns out it's better to have a random one and then forward it. And it 
makes sense, since the device you're trying to debug is //very likely// plugged 
into the same machine as you're running `ADB` on, which is perfect for 
//local// development.

But that wasn't good enough for the use case I have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136551: [lldb] Include gtest in standalone build only if LLDB_INCLUDE_TESTS

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/CMakeLists.txt:20
 
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
+

mgorny wrote:
> To be honest, I don't exactly like moving this `option`. The alternative I 
> can think of is moving gtest and LLVMTestingSupport logic from 
> `LLDBStandalone` into unittests.
So, how does this work in non-standalone builds? Is gtest built 
unconditionally, or is it guarded by something else (LLVM_INCLUDE_TESTS 
perhaps?)


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

https://reviews.llvm.org/D136551

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Ok, so this change means that the randomisation still happens, unless you 
override it with these environment variables? This seems like a good way to do 
it.

Where do these env vars need to be set? On the local machine, on the 
lldb-server machine, on the device?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136584: [LLDB] Check that RegisterInfo and ContextInfo are trivial

[David Spickett via Phabricator via lldb-commits]

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

RegisterInfo is often initialised with a memcpy, and ContextInfo
does not run destructors for anything within it.

This was discussed in https://reviews.llvm.org/D134041.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136584

Files:
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/include/lldb/lldb-private-types.h


Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -15,6 +15,8 @@
 
 #include "llvm/ADT/ArrayRef.h"
 
+#include 
+
 namespace llvm {
 namespace sys {
 class DynamicLibrary;
@@ -70,6 +72,8 @@
   byte_size);
   }
 };
+static_assert(std::is_trivial::value,
+  "RegisterInfo must be trivial.");
 
 /// Registers are grouped into register sets
 struct RegisterSet {
Index: lldb/include/lldb/Core/EmulateInstruction.h
===
--- lldb/include/lldb/Core/EmulateInstruction.h
+++ lldb/include/lldb/Core/EmulateInstruction.h
@@ -189,7 +189,7 @@
 
   public:
 enum InfoType GetInfoType() const { return info_type; }
-union {
+union ContextInfo {
   struct RegisterPlusOffset {
 RegisterInfo reg;  // base register
 int64_t signed_offset; // signed offset added to base register
@@ -241,6 +241,8 @@
 
   uint32_t isa;
 } info;
+static_assert(std::is_trivial::value,
+  "ContextInfo must be trivial.");
 
 Context() = default;
 


Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -15,6 +15,8 @@
 
 #include "llvm/ADT/ArrayRef.h"
 
+#include 
+
 namespace llvm {
 namespace sys {
 class DynamicLibrary;
@@ -70,6 +72,8 @@
   byte_size);
   }
 };
+static_assert(std::is_trivial::value,
+  "RegisterInfo must be trivial.");
 
 /// Registers are grouped into register sets
 struct RegisterSet {
Index: lldb/include/lldb/Core/EmulateInstruction.h
===
--- lldb/include/lldb/Core/EmulateInstruction.h
+++ lldb/include/lldb/Core/EmulateInstruction.h
@@ -189,7 +189,7 @@
 
   public:
 enum InfoType GetInfoType() const { return info_type; }
-union {
+union ContextInfo {
   struct RegisterPlusOffset {
 RegisterInfo reg;  // base register
 int64_t signed_offset; // signed offset added to base register
@@ -241,6 +241,8 @@
 
   uint32_t isa;
 } info;
+static_assert(std::is_trivial::value,
+  "ContextInfo must be trivial.");
 
 Context() = default;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[Luka Markušić via Phabricator via lldb-commits]

mark2185 added a comment.

In D136465#3878640 , @DavidSpickett 
wrote:

> Ok, so this change means that the randomisation still happens, unless you 
> override it with these environment variables? This seems like a good way to 
> do it.

Exactly, this is completely opt-in. This approach with `environment` variables 
was easy enough, and it was used in other things related to `adb` so it seemed 
fitting. Maybe it would be better to set it through `platform settings`, but 
I'd rather have someone chip in on that.

Also the burden of making sure the ports are //actually// free is on the user, 
but I'm guessing the user is aware of this since they are using custom ports.

> Where do these env vars need to be set? On the local machine, on the 
> lldb-server machine, on the device?

These are **client-side** variables, since that is where the user is launching 
`lldb` from. `adb` will only get a request `forward tcp: 
tcp:` instead of `forward tcp: tcp:`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136462: [LLDB] Add color to output text when searching for symbols

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This is definitely not a good way to implement this functionality. There's no 
telling who else might be accessing the symbols while their names are being 
changed.

The colouration should be implemented inside the dumping function, without 
modifying the state of the debugger. I realize that's not currently easy to do, 
but that's not a reason to do this. It's possible the dumping machinery needs 
to be refactored to support this kind of customized colouration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136462

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

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Always good to see another architecture in lldb.

A few points up front.

Changes like this are fine and we can review them, but can only be landed once 
we see that they'll be built on. So please stack changes so that we can see 
what is upcoming. https://llvm.org/docs/Phabricator.html#creating-a-patch-series

Do you have a plan to test this configuration? I don't see any public buildbots 
for Loongson (please correct me if I am wrong) but that is not a requirement as 
long as you have some testing plan (RISCV doesn't have a public bot, as one 
example).

Do you have community members who can review these changes for architectural 
correctness? Speaking just for myself I am happy to review bits of lldb 
machinery but when it comes to architecture details I am not, and will not, be 
an expert in Loongson.

I'm ok accepting patches without a second reviewer, but I advise for your own 
benefit to try to find someone who already knows the details of Loongson.


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[Thorsten via Phabricator via lldb-commits]

tschuett added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.cpp:43
+uint32_t RegisterInfoPOSIX_loongarch64::GetRegisterCount() const {
+  return 0;
+}

Stupid question: why is the register count 0?



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h:26
+public:
+  enum { GPRegSet = 0, FPRegSet };
+

Firstly, I prefer `ènum class`. Second, it is unused?


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D135621: [lldb] Add an always-on log channel

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D135621#3875218 , @JDevlieghere 
wrote:

> In D135621#3850248 , @clayborg 
> wrote:
>
>> I am also questioning if the these even belong in the LLDB logging stuff? 
>> Seems like it would be just as easy to create a diagnostic message by 
>> calling Diagnostics::Report(...). Do we really want to modify the log 
>> channels here? Seems like always on diagnostics should just a dedicated API.
>
> I expect the majority of places where we want to log to the diagnostic log 
> channel to be places where we already log today.

Yeah, but that could be achieved by having the "dedicated diagnostic API" 
forward the message to a regular log when it deems it necessary (or always).

Speaking of dedicated APIs, we already have the Debugger::ReportWarning/Error 
functions. I would expect we want those to show up in the diagnostic log, as 
these are things that we've already deemed important enough to print to the 
user. Wouldn't it be better to use that as a starting point? Possibly extend it 
by adding some sort of an "info" priority, which does not get printed to the 
user (but makes its way to the log)?

> Similarly, I also plan to change `LLDB_LOG_ERROR` to always log to the 
> diagnostic channel. I didn't include it in this patch as it requires moving 
> the macro in the diagnostics header and generates a lot of churn.

I'm not sure that's a good default. Some of those messages could be fairly 
innocuous, and the ratio of the innocuous ones will probably increase as we use 
llvm::Error more.




Comment at: lldb/source/Utility/Diagnostics.cpp:23
+static constexpr Log::Category g_categories[] = {
+{{"lldb"}, {"diagnostics log for lldb"}, DiagnosticsLog::LLDB},
+};

JDevlieghere wrote:
> kastiglione wrote:
> > To me, it's not ideal that there's an "lldb" channel, and this channel has 
> > an "lldb" category.
> > 
> > Do you have other categories in mind for later updates, that should be 
> > moved into this patch? For example, will there eventually be "warning" and 
> > "error" categories? If so, maybe start with those instead of "lldb".
> Yes, for the errors I was going to add an error category. Maybe we can call 
> this one general or something?
I'm not entirely sure of what's the purpose of having multiple categories, if 
they're all supposed to be on all the time (the category name is not recorded 
anywhere, so the only thing they do is enable one to selectively enable/disable 
some log messages).



Comment at: lldb/test/Shell/Diagnostics/TestLogChannel.test:2
+# RUN: %lldb -o 'log list' 2>&1 | FileCheck %s
+# CHECK-NOT: Logging categories for 'diagnostics'

Judging by the patch, I think the user will still be able to disable 
diagnostics if he is able to "guess" that there is a log channel called that 
way (`log disable diagnostics`). Is that intentional?


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

https://reviews.llvm.org/D135621

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

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[Tiezhu Yang via Phabricator via lldb-commits]

seehearfeel marked an inline comment as done.
seehearfeel added a comment.

In D136578#3878476 , @xen0n wrote:

> Hi, I've edited the summary and patch title for you. It's generally not 
> necessary to add //that// much "politeness" when most of it is obvious from 
> context (e.g. the fact that you're new face here, that there's obviously no 
> LoongArch support in LLDB, and most of the methods being stubs). It didn't 
> help that much of the text was in Chinglish either.
>
> As for the changes, they look reasonable to me, but as I haven't tested it 
> out myself yet (and unable to, due to it being stub-only), I'll not give the 
> approval myself this time. Thanks for your contribution and welcome!

OK, thank you.




Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.cpp:43
+uint32_t RegisterInfoPOSIX_loongarch64::GetRegisterCount() const {
+  return 0;
+}

tschuett wrote:
> Stupid question: why is the register count 0?
It is just a stub function here to build successfully, actually it should 
return k_num_gpr_registers
after the following header file is added in the later patch.

lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[Lu Weining via Phabricator via lldb-commits]

SixWeining added a comment.

In D136578#3878744 , @DavidSpickett 
wrote:

> Always good to see another architecture in lldb.
>
> A few points up front.
>
> Changes like this are fine and we can review them, but can only be landed 
> once we see that they'll be built on. So please stack changes so that we can 
> see what is upcoming. 
> https://llvm.org/docs/Phabricator.html#creating-a-patch-series
>
> Do you have a plan to test this configuration? I don't see any public 
> buildbots for Loongson (please correct me if I am wrong) but that is not a 
> requirement as long as you have some testing plan (RISCV doesn't have a 
> public bot, as one example).
>
> Do you have community members who can review these changes for architectural 
> correctness? Speaking just for myself I am happy to review bits of lldb 
> machinery but when it comes to architecture details I am not, and will not, 
> be an expert in Loongson.
>
> I'm ok accepting patches without a second reviewer, but I advise for your own 
> benefit to try to find someone who already knows the details of Loongson.

Hi @DavidSpickett, thanks for your advice. I (as the LoongArch backend code 
owner) can review these changes for architectural correctness. And I think 
other LoongArch community members (non Loongson employees) also can take it. 
Especially @xen0n and @xry111 who are quite familar with LoongArch.

What's more, AFAIK, @seehearfeel (yangtie...@loongson.cn) is the LoongArch port 
maintainer of gdb 
.

Regarding the public buildbot, I'm trying to set it up and maybe we can see it 
one or two weeks.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h:24
+
+class NativeRegisterContextLinux_loongarch64 : public 
NativeRegisterContextLinux {
+public:

Pls limit the columns count to 80 which can be done by `clang-format`. See: 
https://llvm.org/docs/Contributing.html#how-to-submit-a-patch



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h:9
+
+#if defined(__loongarch__) && __loongarch_grlen == 64
+

Seems `#if __loongarch_grlen == 64` is enough? But I'm fine with both.


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[Tiezhu Yang via Phabricator via lldb-commits]

seehearfeel added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h:26
+public:
+  enum { GPRegSet = 0, FPRegSet };
+

tschuett wrote:
> Firstly, I prefer `ènum class`. Second, it is unused?
Now, GPRegSet and FPRegSet are not used,
I will remove them and update the diff in this commit,
and use
enum class { GPRegSet = 0, FPRegSet };
in the later patch, thank you.


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D136209: [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

So if I understand correctly, when we deserialize incorrectly, this causes us 
to misclassify some function parameters (arguments) as local variables. Is that 
correct? If yes, then that is something that can be checked. Even if the 
misclassified variables behave perfectly, their kind is still visible, e.g. in 
the output of "frame variable --scope".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136209

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

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> What's more, AFAIK, @seehearfeel (yangtie...@loongson.cn) is the LoongArch 
> port maintainer of gdb.
> Regarding the public buildbot, I'm trying to set it up and maybe we can see 
> it in one or two weeks.

Great!


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D136584: [LLDB] Check that RegisterInfo and ContextInfo are trivial

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d06ef565818: [LLDB] Check that RegisterInfo and ContextInfo 
are trivial (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136584

Files:
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/include/lldb/lldb-private-types.h


Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -15,6 +15,8 @@
 
 #include "llvm/ADT/ArrayRef.h"
 
+#include 
+
 namespace llvm {
 namespace sys {
 class DynamicLibrary;
@@ -70,6 +72,8 @@
   byte_size);
   }
 };
+static_assert(std::is_trivial::value,
+  "RegisterInfo must be trivial.");
 
 /// Registers are grouped into register sets
 struct RegisterSet {
Index: lldb/include/lldb/Core/EmulateInstruction.h
===
--- lldb/include/lldb/Core/EmulateInstruction.h
+++ lldb/include/lldb/Core/EmulateInstruction.h
@@ -189,7 +189,7 @@
 
   public:
 enum InfoType GetInfoType() const { return info_type; }
-union {
+union ContextInfo {
   struct RegisterPlusOffset {
 RegisterInfo reg;  // base register
 int64_t signed_offset; // signed offset added to base register
@@ -241,6 +241,8 @@
 
   uint32_t isa;
 } info;
+static_assert(std::is_trivial::value,
+  "ContextInfo must be trivial.");
 
 Context() = default;
 


Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -15,6 +15,8 @@
 
 #include "llvm/ADT/ArrayRef.h"
 
+#include 
+
 namespace llvm {
 namespace sys {
 class DynamicLibrary;
@@ -70,6 +72,8 @@
   byte_size);
   }
 };
+static_assert(std::is_trivial::value,
+  "RegisterInfo must be trivial.");
 
 /// Registers are grouped into register sets
 struct RegisterSet {
Index: lldb/include/lldb/Core/EmulateInstruction.h
===
--- lldb/include/lldb/Core/EmulateInstruction.h
+++ lldb/include/lldb/Core/EmulateInstruction.h
@@ -189,7 +189,7 @@
 
   public:
 enum InfoType GetInfoType() const { return info_type; }
-union {
+union ContextInfo {
   struct RegisterPlusOffset {
 RegisterInfo reg;  // base register
 int64_t signed_offset; // signed offset added to base register
@@ -241,6 +241,8 @@
 
   uint32_t isa;
 } info;
+static_assert(std::is_trivial::value,
+  "ContextInfo must be trivial.");
 
 Context() = default;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 8d06ef5 - [LLDB] Check that RegisterInfo and ContextInfo are trivial

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-10-24T10:16:29Z
New Revision: 8d06ef5658187bf6a3eda61a5ec3627c1ff33fb8

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

LOG: [LLDB] Check that RegisterInfo and ContextInfo are trivial

RegisterInfo is often initialised with a memcpy, and ContextInfo
does not run destructors for anything within it.

This was discussed in https://reviews.llvm.org/D134041.

Reviewed By: labath

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

Added: 


Modified: 
lldb/include/lldb/Core/EmulateInstruction.h
lldb/include/lldb/lldb-private-types.h

Removed: 




diff  --git a/lldb/include/lldb/Core/EmulateInstruction.h 
b/lldb/include/lldb/Core/EmulateInstruction.h
index 65b7982c36a0b..64633e9cf29be 100644
--- a/lldb/include/lldb/Core/EmulateInstruction.h
+++ b/lldb/include/lldb/Core/EmulateInstruction.h
@@ -189,7 +189,7 @@ class EmulateInstruction : public PluginInterface {
 
   public:
 enum InfoType GetInfoType() const { return info_type; }
-union {
+union ContextInfo {
   struct RegisterPlusOffset {
 RegisterInfo reg;  // base register
 int64_t signed_offset; // signed offset added to base register
@@ -241,6 +241,8 @@ class EmulateInstruction : public PluginInterface {
 
   uint32_t isa;
 } info;
+static_assert(std::is_trivial::value,
+  "ContextInfo must be trivial.");
 
 Context() = default;
 

diff  --git a/lldb/include/lldb/lldb-private-types.h 
b/lldb/include/lldb/lldb-private-types.h
index 1b0d263e2073b..edc1c78985bdd 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -15,6 +15,8 @@
 
 #include "llvm/ADT/ArrayRef.h"
 
+#include 
+
 namespace llvm {
 namespace sys {
 class DynamicLibrary;
@@ -70,6 +72,8 @@ struct RegisterInfo {
   byte_size);
   }
 };
+static_assert(std::is_trivial::value,
+  "RegisterInfo must be trivial.");
 
 /// Registers are grouped into register sets
 struct RegisterSet {



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

[Lldb-commits] [PATCH] D136551: [lldb] Include gtest in standalone build only if LLDB_INCLUDE_TESTS

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

mgorny added inline comments.



Comment at: lldb/CMakeLists.txt:20
 
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
+

labath wrote:
> mgorny wrote:
> > To be honest, I don't exactly like moving this `option`. The alternative I 
> > can think of is moving gtest and LLVMTestingSupport logic from 
> > `LLDBStandalone` into unittests.
> So, how does this work in non-standalone builds? Is gtest built 
> unconditionally, or is it guarded by something else (LLVM_INCLUDE_TESTS 
> perhaps?)
It's guarded by `LLVM_INCLUDE_UTILS` + `LLVM_INCLUDE_TESTS`.


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

https://reviews.llvm.org/D136551

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

[Lldb-commits] [lldb] d152393 - [lldb] Add LLVM include dirs prior to gtest target in standalone build

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-10-24T12:18:14+02:00
New Revision: d15239388cca8a5031119b4660e456c26ca5f379

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

LOG: [lldb] Add LLVM include dirs prior to gtest target in standalone build

Move include_directories() declaration before gtest targets are created
in standalone build.  This fixes build failure due to gtest targets
being unable to find LLVM headers, e.g.:


/var/tmp/portage/dev-util/lldb-16.0.0_pre20221023/work/llvm/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h:43:10:
 fatal error: llvm/ADT/Optional.h: No such file or directory

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

Added: 


Modified: 
lldb/cmake/modules/LLDBStandalone.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBStandalone.cmake 
b/lldb/cmake/modules/LLDBStandalone.cmake
index 9cd3f1e8186ba..98a52969ba30f 100644
--- a/lldb/cmake/modules/LLDBStandalone.cmake
+++ b/lldb/cmake/modules/LLDBStandalone.cmake
@@ -97,6 +97,12 @@ include(LLVMDistributionSupport)
 set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
+set(CMAKE_INCLUDE_CURRENT_DIR ON)
+include_directories(
+  "${CMAKE_BINARY_DIR}/include"
+  "${LLVM_INCLUDE_DIRS}"
+  "${CLANG_INCLUDE_DIRS}")
+
 # Build the gtest library needed for unittests, if we have LLVM sources
 # handy.
 if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
@@ -117,12 +123,6 @@ endif()
 set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "lldb misc")
 set_target_properties(intrinsics_gen PROPERTIES FOLDER "lldb misc")
 
-set(CMAKE_INCLUDE_CURRENT_DIR ON)
-include_directories(
-  "${CMAKE_BINARY_DIR}/include"
-  "${LLVM_INCLUDE_DIRS}"
-  "${CLANG_INCLUDE_DIRS}")
-
 if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
   set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 endif()



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

[Lldb-commits] [PATCH] D136552: [lldb] Add LLVM include dirs prior to gtest target in standalone build

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd15239388cca: [lldb] Add LLVM include dirs prior to gtest 
target in standalone build (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136552

Files:
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -97,6 +97,12 @@
 set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
+set(CMAKE_INCLUDE_CURRENT_DIR ON)
+include_directories(
+  "${CMAKE_BINARY_DIR}/include"
+  "${LLVM_INCLUDE_DIRS}"
+  "${CLANG_INCLUDE_DIRS}")
+
 # Build the gtest library needed for unittests, if we have LLVM sources
 # handy.
 if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
@@ -117,12 +123,6 @@
 set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "lldb misc")
 set_target_properties(intrinsics_gen PROPERTIES FOLDER "lldb misc")
 
-set(CMAKE_INCLUDE_CURRENT_DIR ON)
-include_directories(
-  "${CMAKE_BINARY_DIR}/include"
-  "${LLVM_INCLUDE_DIRS}"
-  "${CLANG_INCLUDE_DIRS}")
-
 if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
   set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 endif()


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -97,6 +97,12 @@
 set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
+set(CMAKE_INCLUDE_CURRENT_DIR ON)
+include_directories(
+  "${CMAKE_BINARY_DIR}/include"
+  "${LLVM_INCLUDE_DIRS}"
+  "${CLANG_INCLUDE_DIRS}")
+
 # Build the gtest library needed for unittests, if we have LLVM sources
 # handy.
 if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
@@ -117,12 +123,6 @@
 set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "lldb misc")
 set_target_properties(intrinsics_gen PROPERTIES FOLDER "lldb misc")
 
-set(CMAKE_INCLUDE_CURRENT_DIR ON)
-include_directories(
-  "${CMAKE_BINARY_DIR}/include"
-  "${LLVM_INCLUDE_DIRS}"
-  "${CLANG_INCLUDE_DIRS}")
-
 if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
   set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136462: [LLDB] Add color to output text when searching for symbols

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I agree that this change names - print - restore names is going to cause 
problems. Pavel is right that this should happen in some dump function 
somewhere.

But, assuming what you've got works to an extent, you should add some testing 
first. That will allow you to refactor with less risk. You'll be able to reuse 
a lot of the tests later, so it's not wasted effort and it'll flush out some 
corner cases you haven't thought of.

For how to test this I would look to the other commands that have colour 
(`frame` was one I think) and anything interacting with the colour settings.
(if you want a cheeky way to find the tests, delete some of the colouring code 
and run the test suite - whatever fails is going to be interesting to you)




Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1514
+  for (size_t i = 0; i < positions.size(); ++i) {
+if (pos >= positions[i].first && pos <= positions[i].second) {
+  return true;

Doesn't seem like you need to enumerate them, so this could be:
```
for (const auto &p : positions)
   if (pos >= )
   return true;
return false;
```
(https://en.cppreference.com/w/cpp/language/range-for)

Or `std::find(..., ) != positions.end()` but same thing really.

Basically prefer iterators unless `i` is needed for something other than 
indexing.

If you do need the index but don't want the hassle of handling it yourself, 
there is an `llvm::enumerate` in `llvm/include/llvm/ADT/STLExtras.h` which acts 
like Python's `enumerate`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136462

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

[Lldb-commits] [PATCH] D136462: [LLDB] Add color to output text when searching for symbols

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

And if it helps to refactor in parent patches to make this change easier, go 
for it. Maybe you add generic colouring support to something, then the last 
patch hooks it up to the symbol lookup, something along those lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136462

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

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

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

mgorny updated this revision to Diff 470108.
mgorny retitled this revision from "[lld] Copy cmake_policy() to silence CMake 
warnings in standalone builds" to "Harmonize cmake_policy() across standalone 
builds of all projects".
mgorny edited the summary of this revision.
mgorny added reviewers: mehdi_amini, labath.
mgorny added subscribers: cfe-commits, lldb-commits.
mgorny added a comment.
Herald added subscribers: zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, 
cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, 
Joonsoo, stephenneuendorffer, liufengdb, aartbik, mgester, arpith-jacob, 
nicolasvasilache, antiagainst, shauheen, rriddle, kristof.beyls.
Herald added a reviewer: sscalpone.
Herald added a project: MLIR.

Replace with one patch for all the projects.


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

https://reviews.llvm.org/D136572

Files:
  clang/CMakeLists.txt
  flang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  mlir/CMakeLists.txt

Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -1,7 +1,16 @@
 # MLIR project.
 
+cmake_minimum_required(VERSION 3.13.4)
+
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  # Please keep policies in sync with llvm/CMakeLists.txt.
+  if(POLICY CMP0114)
+cmake_policy(SET CMP0114 OLD)
+  endif()
+  if(POLICY CMP0116)
+cmake_policy(SET CMP0116 OLD)
+  endif()
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
 endif()
@@ -10,8 +19,6 @@
 include(GNUInstallDirs)
 
 if(MLIR_STANDALONE_BUILD)
-  cmake_minimum_required(VERSION 3.13.4)
-
   find_package(LLVM CONFIG REQUIRED)
   set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${LLVM_CMAKE_DIR})
   include(HandleLLVMOptions)
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -1,9 +1,3 @@
-# CMP0116: Ninja generators transform `DEPFILE`s from `add_custom_command()`
-# New in CMake 3.20. https://cmake.org/cmake/help/latest/policy/CMP0116.html
-if(POLICY CMP0116)
-  cmake_policy(SET CMP0116 OLD)
-endif()
-
 if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
   set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 endif()
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -1,19 +1,26 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-# Add path for custom modules.
-set(CMAKE_MODULE_PATH
-  ${CMAKE_MODULE_PATH}
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
-  )
-
 # If we are not building as part of LLVM, build LLDB as a standalone project,
 # using LLVM as an external library.
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  # Please keep policies in sync with llvm/CMakeLists.txt.
+  if(POLICY CMP0114)
+cmake_policy(SET CMP0114 OLD)
+  endif()
+  if(POLICY CMP0116)
+cmake_policy(SET CMP0116 OLD)
+  endif()
   project(lldb)
   set(LLDB_BUILT_STANDALONE TRUE)
 endif()
 
+# Add path for custom modules.
+set(CMAKE_MODULE_PATH
+  ${CMAKE_MODULE_PATH}
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+  )
+
 # Must go below project(..)
 include(GNUInstallDirs)
 
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -3,6 +3,13 @@
 # If we are not building as a part of LLVM, build LLD as an
 # standalone project, using LLVM as an external library:
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  # Please keep policies in sync with llvm/CMakeLists.txt.
+  if(POLICY CMP0114)
+cmake_policy(SET CMP0114 OLD)
+  endif()
+  if(POLICY CMP0116)
+cmake_policy(SET CMP0116 OLD)
+  endif()
   project(lld)
   set(LLD_BUILT_STANDALONE TRUE)
 endif()
Index: flang/CMakeLists.txt
===
--- flang/CMakeLists.txt
+++ flang/CMakeLists.txt
@@ -1,5 +1,22 @@
 cmake_minimum_required(VERSION 3.13.4)
 
+# Check for a standalone build and configure as appropriate from
+# there.
+if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  # Please keep policies in sync with llvm/CMakeLists.txt.
+  if(POLICY CMP0114)
+cmake_policy(SET CMP0114 OLD)
+  endif()
+  if(POLICY CMP0116)
+cmake_policy(SET CMP0116 OLD)
+  endif()
+  message("Building Flang as a standalone project.")
+  project(Flang)
+  set(FLANG_STANDALONE_BUILD ON)
+else()
+  set(FLANG_STANDALONE_BUILD OFF)
+endif()
+
 set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
 # Flang requires C++17.
@@ -19,16 +36,6 @@
 
 option(FLANG_ENABLE_WERROR "Fail and stop building flang if a warning is triggered." OFF)
 
-# Check for a standalone build and co

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[Tiezhu Yang via Phabricator via lldb-commits]

seehearfeel added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h:24
+
+class NativeRegisterContextLinux_loongarch64 : public 
NativeRegisterContextLinux {
+public:

SixWeining wrote:
> Pls limit the columns count to 80 which can be done by `clang-format`. See: 
> https://llvm.org/docs/Contributing.html#how-to-submit-a-patch
OK, will do it and then update.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h:9
+
+#if defined(__loongarch__) && __loongarch_grlen == 64
+

SixWeining wrote:
> Seems `#if __loongarch_grlen == 64` is enough? But I'm fine with both.
Let us leave it as it is like riscv does, thank you.


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D132510: [RISCV][LLDB] Add initial SysV ABI support

[kasper via Phabricator via lldb-commits]

kasper81 requested review of this revision.
kasper81 added a comment.

@MaskRay could you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132510

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

[Lldb-commits] [PATCH] D136551: [lldb] Include gtest in standalone build only if LLDB_INCLUDE_TESTS

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/CMakeLists.txt:20
 
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
+

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > To be honest, I don't exactly like moving this `option`. The alternative 
> > > I can think of is moving gtest and LLVMTestingSupport logic from 
> > > `LLDBStandalone` into unittests.
> > So, how does this work in non-standalone builds? Is gtest built 
> > unconditionally, or is it guarded by something else (LLVM_INCLUDE_TESTS 
> > perhaps?)
> It's guarded by `LLVM_INCLUDE_UTILS` + `LLVM_INCLUDE_TESTS`.
Ok, I think that's fine. I'm not particularly upset by that. Doing this work in 
the unittest directory would probably be fine too...


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

https://reviews.llvm.org/D136551

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

[Lldb-commits] [lldb] 52f3985 - [lldb] Include gtest in standalone build only if LLDB_INCLUDE_TESTS

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-10-24T15:51:43+02:00
New Revision: 52f39853abd46495a6d636c4b035e1b92cf4b833

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

LOG: [lldb] Include gtest in standalone build only if LLDB_INCLUDE_TESTS

Build gtest targets when building standalone only if LLDB_INCLUDE_TESTS
is true.  Prior to this change, they were built whenever
LLVM_MAIN_SRC_DIR was available, independently whether tests themselves
would be run.

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

Added: 


Modified: 
lldb/CMakeLists.txt
lldb/cmake/modules/LLDBStandalone.cmake

Removed: 




diff  --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
index 88a8f0a651399..12ba8a9d79e20 100644
--- a/lldb/CMakeLists.txt
+++ b/lldb/CMakeLists.txt
@@ -17,6 +17,8 @@ endif()
 # Must go below project(..)
 include(GNUInstallDirs)
 
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
+
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
@@ -129,7 +131,6 @@ if (NOT TARGET llvm_gtest)
   set(LLDB_INCLUDE_UNITTESTS OFF)
 endif()
 
-option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
 if(LLDB_INCLUDE_TESTS)
   add_subdirectory(test)
   if (LLDB_INCLUDE_UNITTESTS)

diff  --git a/lldb/cmake/modules/LLDBStandalone.cmake 
b/lldb/cmake/modules/LLDBStandalone.cmake
index 98a52969ba30f..1a03c5a5ef749 100644
--- a/lldb/cmake/modules/LLDBStandalone.cmake
+++ b/lldb/cmake/modules/LLDBStandalone.cmake
@@ -103,16 +103,18 @@ include_directories(
   "${LLVM_INCLUDE_DIRS}"
   "${CLANG_INCLUDE_DIRS}")
 
-# Build the gtest library needed for unittests, if we have LLVM sources
-# handy.
-if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
-  add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/unittest utils/unittest)
-endif()
-# LLVMTestingSupport library is needed for Process/gdb-remote.
-if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
-AND NOT TARGET LLVMTestingSupport)
-  add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
-lib/Testing/Support)
+if(LLDB_INCLUDE_TESTS)
+  # Build the gtest library needed for unittests, if we have LLVM sources
+  # handy.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/unittest utils/unittest)
+  endif()
+  # LLVMTestingSupport library is needed for Process/gdb-remote.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
 endif()
 
 option(LLVM_USE_FOLDERS "Enable solution folders in Visual Studio. Disable for 
Express versions." ON)



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

[Lldb-commits] [PATCH] D136551: [lldb] Include gtest in standalone build only if LLDB_INCLUDE_TESTS

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG52f39853abd4: [lldb] Include gtest in standalone build only 
if LLDB_INCLUDE_TESTS (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D136551?vs=469980&id=470133#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136551

Files:
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -103,16 +103,18 @@
   "${LLVM_INCLUDE_DIRS}"
   "${CLANG_INCLUDE_DIRS}")
 
-# Build the gtest library needed for unittests, if we have LLVM sources
-# handy.
-if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
-  add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/unittest utils/unittest)
-endif()
-# LLVMTestingSupport library is needed for Process/gdb-remote.
-if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
-AND NOT TARGET LLVMTestingSupport)
-  add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
-lib/Testing/Support)
+if(LLDB_INCLUDE_TESTS)
+  # Build the gtest library needed for unittests, if we have LLVM sources
+  # handy.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/unittest utils/unittest)
+  endif()
+  # LLVMTestingSupport library is needed for Process/gdb-remote.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
 endif()
 
 option(LLVM_USE_FOLDERS "Enable solution folders in Visual Studio. Disable for 
Express versions." ON)
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -17,6 +17,8 @@
 # Must go below project(..)
 include(GNUInstallDirs)
 
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
+
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
@@ -129,7 +131,6 @@
   set(LLDB_INCLUDE_UNITTESTS OFF)
 endif()
 
-option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
 if(LLDB_INCLUDE_TESTS)
   add_subdirectory(test)
   if (LLDB_INCLUDE_UNITTESTS)


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -103,16 +103,18 @@
   "${LLVM_INCLUDE_DIRS}"
   "${CLANG_INCLUDE_DIRS}")
 
-# Build the gtest library needed for unittests, if we have LLVM sources
-# handy.
-if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
-  add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/unittest utils/unittest)
-endif()
-# LLVMTestingSupport library is needed for Process/gdb-remote.
-if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
-AND NOT TARGET LLVMTestingSupport)
-  add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
-lib/Testing/Support)
+if(LLDB_INCLUDE_TESTS)
+  # Build the gtest library needed for unittests, if we have LLVM sources
+  # handy.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/unittest AND NOT TARGET llvm_gtest)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/unittest utils/unittest)
+  endif()
+  # LLVMTestingSupport library is needed for Process/gdb-remote.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
 endif()
 
 option(LLVM_USE_FOLDERS "Enable solution folders in Visual Studio. Disable for Express versions." ON)
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -17,6 +17,8 @@
 # Must go below project(..)
 include(GNUInstallDirs)
 
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." ${LLVM_INCLUDE_TESTS})
+
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
@@ -129,7 +131,6 @@
   set(LLDB_INCLUDE_UNITTESTS OFF)
 endif()
 
-option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." ${LLVM_INCLUDE_TESTS})
 if(LLDB_INCLUDE_TESTS)
   add_subdirectory(test)
   if (LLDB_INCLUDE_UNITTESTS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136551: [lldb] Include gtest in standalone build only if LLDB_INCLUDE_TESTS

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

mgorny added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136551

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

[Lldb-commits] [PATCH] D134604: [clang] Implement sugared substitution changes to infrastructure

[Erich Keane via Phabricator via lldb-commits]

erichkeane accepted this revision.
erichkeane added a comment.

A couple of nits, otherwise I don't have to see this again.




Comment at: clang/include/clang/AST/TemplateName.h:158
+  // When true the substitution will be finalized (subst node won't be placed).
+  bool getFinal() const;
+

Based on this comment, should this be 'Finalize' instead of 'Final'?  Or 
something like "ShouldFinalize"?



Comment at: clang/include/clang/Sema/Template.h:104
 /// Construct a single-level template argument list.
-MultiLevelTemplateArgumentList(Decl *D, ArgList Args) {
-  addOuterTemplateArguments(D, Args);
+MultiLevelTemplateArgumentList(Decl *D, ArgList Args, bool Final) {
+  addOuterTemplateArguments(D, Args, Final);

Hrmph, this additional flag likely requires rebasing/rebuilding on a patch I 
just committed, which uses this MLTAL again.  Hopefully not too bad to 
propagate.



Comment at: clang/lib/AST/ASTContext.cpp:3045
+static bool
+getCanonicalTemplateArguments(const ASTContext &C,
+  ArrayRef OrigArgs,

typically we name these sorts of things based on their return type, so this 
would be more `hasCanonicalTemplateArguments`.  BUT the modifying nature of it 
makes this an awkward function all the same.

I think I'd suggest putting both of the 'output's here on the same side of the 
function, either both a return, or both an out-param, so the name isn't 
ambiguous/confusing like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134604

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

[Lldb-commits] [PATCH] D136600: [lldb-tests] Force use of system stdlib for Objective-C test

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

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

The test TestObjCDirectMethods loads the Objective C runtime, which
doesn't work well with custom a libcxx, resulting in two copies of the
standard library to be loaded at runtime. Like what was done for
`TestObjCExceptions`, this commit forces the usage of the system's
library instead. The minimum required Clang version is set to the oldest
Clang that can compile the libraries available in the lldb-matrix bots.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136600

Files:
  lldb/test/API/lang/objc/objc_direct-methods/Makefile
  lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,4 +1,6 @@
 from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
 
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
 lldbinline.MakeInlineTest(
-__file__, globals(), [])
+__file__, globals(), decor)
Index: lldb/test/API/lang/objc/objc_direct-methods/Makefile
===
--- lldb/test/API/lang/objc/objc_direct-methods/Makefile
+++ lldb/test/API/lang/objc/objc_direct-methods/Makefile
@@ -1,4 +1,5 @@
 OBJC_SOURCES := main.m
 LD_EXTRAS := -lobjc -framework Foundation
+USE_SYSTEM_STDLIB := 1
 
 include Makefile.rules


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,4 +1,6 @@
 from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
 
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
 lldbinline.MakeInlineTest(
-__file__, globals(), [])
+__file__, globals(), decor)
Index: lldb/test/API/lang/objc/objc_direct-methods/Makefile
===
--- lldb/test/API/lang/objc/objc_direct-methods/Makefile
+++ lldb/test/API/lang/objc/objc_direct-methods/Makefile
@@ -1,4 +1,5 @@
 OBJC_SOURCES := main.m
 LD_EXTRAS := -lobjc -framework Foundation
+USE_SYSTEM_STDLIB := 1
 
 include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136600: [lldb-tests] Force use of system stdlib for Objective-C test

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

fdeazeve updated this revision to Diff 470146.
fdeazeve added a comment.
Herald added a subscriber: JDevlieghere.

Edit commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136600

Files:
  lldb/test/API/lang/objc/objc_direct-methods/Makefile
  lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,4 +1,6 @@
 from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
 
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
 lldbinline.MakeInlineTest(
-__file__, globals(), [])
+__file__, globals(), decor)
Index: lldb/test/API/lang/objc/objc_direct-methods/Makefile
===
--- lldb/test/API/lang/objc/objc_direct-methods/Makefile
+++ lldb/test/API/lang/objc/objc_direct-methods/Makefile
@@ -1,4 +1,5 @@
 OBJC_SOURCES := main.m
 LD_EXTRAS := -lobjc -framework Foundation
+USE_SYSTEM_STDLIB := 1
 
 include Makefile.rules


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,4 +1,6 @@
 from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
 
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
 lldbinline.MakeInlineTest(
-__file__, globals(), [])
+__file__, globals(), decor)
Index: lldb/test/API/lang/objc/objc_direct-methods/Makefile
===
--- lldb/test/API/lang/objc/objc_direct-methods/Makefile
+++ lldb/test/API/lang/objc/objc_direct-methods/Makefile
@@ -1,4 +1,5 @@
 OBJC_SOURCES := main.m
 LD_EXTRAS := -lobjc -framework Foundation
+USE_SYSTEM_STDLIB := 1
 
 include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136600: [lldb-tests] Force use of system stdlib for Objective-C test

[Michael Buch via Phabricator via lldb-commits]

Michael137 accepted this revision.
Michael137 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/D136600/new/

https://reviews.llvm.org/D136600

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

[Lldb-commits] [PATCH] D136608: [lldb-tests] Remove libstdc++ requirement from test

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

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

This requirement dates back to ten years ago and the test seems to work
nowadays with either libc++ or libstdc++.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136608

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
@@ -1,5 +1,4 @@
 CXX_SOURCES := main.cpp
-USE_LIBCPP := 1
 
 CXXFLAGS_EXTRAS := -O0
 include Makefile.rules


Index: lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
+++ lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
@@ -1,5 +1,4 @@
 CXX_SOURCES := main.cpp
-USE_LIBCPP := 1
 
 CXXFLAGS_EXTRAS := -O0
 include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136608: [lldb-tests] Remove libstdc++ requirement from test

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

fdeazeve updated this revision to Diff 470165.
fdeazeve added a comment.

Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136608

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
@@ -1,7 +1,4 @@
 CXX_SOURCES := main.cpp
-USE_LIBSTDCPP := 0
-
-
 
 CXXFLAGS_EXTRAS := -O0
 include Makefile.rules


Index: lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
+++ lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
@@ -1,7 +1,4 @@
 CXX_SOURCES := main.cpp
-USE_LIBSTDCPP := 0
-
-
 
 CXXFLAGS_EXTRAS := -O0
 include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136610: [trace][intelpt] Fix multi CPU decoding TSC assertion error

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

Occasionally the assertion that enforces increasing TSC values in 
`DecodedThread::NotifyTsc`
would get tripped during large multi CPU trace decoding.
The root cause of this issue was an assumption that all the data of a
PSB will fit within the start,end TSC of the "owning"
`ThreadContinuousExecution`. After investigating, this is not the case
because PSBs can have multiple TSCs.
This diff works around this issue by introducing a TSC upper bound for
each `PSBBlockDecoder`. This fixes the assertion failure by simply
"dropping" the remaining data of PSB whenever the TSC upper bound is
exceeded during decoding.
Future work will do a larger refactor of the multi CPU decoding to
remove the dependencies on this incorrect assumption so that PSB blocks
that span multiple `ThreadContinuousExecutions` are correctly handled.
correctly

Test Plan:


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136610

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

Index: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -155,9 +155,11 @@
   /// appended to. It might have already some instructions.
   PSBBlockDecoder(PtInsnDecoderUP &&decoder_up, const PSBBlock &psb_block,
   Optional next_block_ip,
-  DecodedThread &decoded_thread)
+  DecodedThread &decoded_thread,
+  llvm::Optional tsc_upper_bound)
   : m_decoder_up(std::move(decoder_up)), m_psb_block(psb_block),
-m_next_block_ip(next_block_ip), m_decoded_thread(decoded_thread) {}
+m_next_block_ip(next_block_ip), m_decoded_thread(decoded_thread),
+m_tsc_upper_bound(tsc_upper_bound) {}
 
   /// \param[in] trace_intel_pt
   /// The main Trace object that own the PSB block.
@@ -185,14 +187,15 @@
   static Expected
   Create(TraceIntelPT &trace_intel_pt, const PSBBlock &psb_block,
  ArrayRef buffer, Process &process,
- Optional next_block_ip, DecodedThread &decoded_thread) {
+ Optional next_block_ip, DecodedThread &decoded_thread,
+ llvm::Optional tsc_upper_bound) {
 Expected decoder_up =
 CreateInstructionDecoder(trace_intel_pt, buffer, process);
 if (!decoder_up)
   return decoder_up.takeError();
 
 return PSBBlockDecoder(std::move(*decoder_up), psb_block, next_block_ip,
-   decoded_thread);
+   decoded_thread, tsc_upper_bound);
   }
 
   void DecodePSBBlock() {
@@ -279,8 +282,34 @@
 return status;
   }
 
-  if (event.has_tsc)
-m_decoded_thread.NotifyTsc(event.tsc);
+  if (event.has_tsc) {
+if (m_tsc_upper_bound && event.tsc > m_tsc_upper_bound.value()) {
+  // This event and all the remaining events of this PSB have a TSC
+  // outside the range of the "owning" ThreadContinuousExecution. For
+  // now we drop all of these events/instructions, future work can
+  // improve upon this by determining the "owning"
+  // ThreadContinuousExecution of the remaining PSB data.
+  std::string err_msg =
+  formatv(
+  "TSC {0} exceeds maximum TSC value {1}. Will skip decoding"
+  " the remining the remaining data of the PSB.",
+  event.tsc, m_tsc_upper_bound.value())
+  .str();
+
+  uint64_t offset;
+  int status = pt_insn_get_offset(m_decoder_up.get(), &offset);
+  if (!IsLibiptError(status)) {
+err_msg =
+formatv("At byte offset {0} of PSB with size {1} bytes, {2}",
+offset, m_psb_block.size, err_msg)
+.str();
+  }
+  m_decoded_thread.AppendCustomError(err_msg);
+  return -1;
+} else {
+  m_decoded_thread.NotifyTsc(event.tsc);
+}
+  }
 
   switch (event.type) {
   case ptev_disabled:
@@ -313,6 +342,8 @@
   PSBBlock m_psb_block;
   Optional m_next_block_ip;
   DecodedThread &m_decoded_thread;
+  // Maximum allowed value of TSCs decoded from m_psb_block.
+  llvm::Optional m_tsc_upper_bound;
 };
 
 Error lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
@@ -330,7 +361,7 @@
 trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
 *decoded_thread.GetThread()->GetProcess(),
 i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
-decoded_thread);
+decoded_thread, llvm::None);
 if (!decoder)
   return decoder.takeError();
 
@@ -392,13 +423,13 @@
 
   Expected decoder = PSBBlo

[Lldb-commits] [PATCH] D135621: [lldb] Add an always-on log channel

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere abandoned this revision.
JDevlieghere added a comment.

Seems like the consensus is that we prefer a dedicated API rather than using 
the existing logging infrastructure.


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

https://reviews.llvm.org/D135621

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

[Lldb-commits] [lldb] 73200d1 - [lldb-tests] Remove libstdc++ requirement from test

[Felipe de Azevedo Piovezan via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2022-10-24T16:27:10Z
New Revision: 73200d155a5882f5bda918596abdc143c991cf0f

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

LOG: [lldb-tests] Remove libstdc++ requirement from test

This requirement dates back to ten years ago and the test seems to work
nowadays with either libc++ or libstdc++.

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

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
index 6bfb97cd2c2b..a149c7f81bfa 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
@@ -1,7 +1,4 @@
 CXX_SOURCES := main.cpp
-USE_LIBSTDCPP := 0
-
-
 
 CXXFLAGS_EXTRAS := -O0
 include Makefile.rules



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

[Lldb-commits] [PATCH] D136608: [lldb-tests] Remove libstdc++ requirement from test

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG73200d155a58: [lldb-tests] Remove libstdc++ requirement from 
test (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136608

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
@@ -1,7 +1,4 @@
 CXX_SOURCES := main.cpp
-USE_LIBSTDCPP := 0
-
-
 
 CXXFLAGS_EXTRAS := -O0
 include Makefile.rules


Index: lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
+++ lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/Makefile
@@ -1,7 +1,4 @@
 CXX_SOURCES := main.cpp
-USE_LIBSTDCPP := 0
-
-
 
 CXXFLAGS_EXTRAS := -O0
 include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c413df0 - [lldb-tests] Force use of system stdlib for Objective-C test

[Felipe de Azevedo Piovezan via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2022-10-24T12:39:48-04:00
New Revision: c413df064e85623003ae624835177d19a23fffe9

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

LOG: [lldb-tests] Force use of system stdlib for Objective-C test

The test TestObjCDirectMethods loads the Objective C runtime, which
doesn't work well with custom a libcxx, resulting in two copies of the
standard library being loaded at runtime.

Like what was done for `TestObjCExceptions`, this commit forces the
usage of the system's library instead. The minimum required Clang
version is set to the oldest Clang that can compile the libraries
available in the lldb-matrix bots.

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

Added: 


Modified: 
lldb/test/API/lang/objc/objc_direct-methods/Makefile
lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py

Removed: 




diff  --git a/lldb/test/API/lang/objc/objc_direct-methods/Makefile 
b/lldb/test/API/lang/objc/objc_direct-methods/Makefile
index afecbf969483e..22567a2e5a6d8 100644
--- a/lldb/test/API/lang/objc/objc_direct-methods/Makefile
+++ b/lldb/test/API/lang/objc/objc_direct-methods/Makefile
@@ -1,4 +1,5 @@
 OBJC_SOURCES := main.m
 LD_EXTRAS := -lobjc -framework Foundation
+USE_SYSTEM_STDLIB := 1
 
 include Makefile.rules

diff  --git 
a/lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py 
b/lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
index 9fbf972ad8ea6..ed89377d9b7ac 100644
--- a/lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ b/lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,4 +1,6 @@
 from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
 
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
 lldbinline.MakeInlineTest(
-__file__, globals(), [])
+__file__, globals(), decor)



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

[Lldb-commits] [PATCH] D136600: [lldb-tests] Force use of system stdlib for Objective-C test

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc413df064e85: [lldb-tests] Force use of system stdlib for 
Objective-C test (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136600

Files:
  lldb/test/API/lang/objc/objc_direct-methods/Makefile
  lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,4 +1,6 @@
 from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
 
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
 lldbinline.MakeInlineTest(
-__file__, globals(), [])
+__file__, globals(), decor)
Index: lldb/test/API/lang/objc/objc_direct-methods/Makefile
===
--- lldb/test/API/lang/objc/objc_direct-methods/Makefile
+++ lldb/test/API/lang/objc/objc_direct-methods/Makefile
@@ -1,4 +1,5 @@
 OBJC_SOURCES := main.m
 LD_EXTRAS := -lobjc -framework Foundation
+USE_SYSTEM_STDLIB := 1
 
 include Makefile.rules


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,4 +1,6 @@
 from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
 
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
 lldbinline.MakeInlineTest(
-__file__, globals(), [])
+__file__, globals(), decor)
Index: lldb/test/API/lang/objc/objc_direct-methods/Makefile
===
--- lldb/test/API/lang/objc/objc_direct-methods/Makefile
+++ lldb/test/API/lang/objc/objc_direct-methods/Makefile
@@ -1,4 +1,5 @@
 OBJC_SOURCES := main.m
 LD_EXTRAS := -lobjc -framework Foundation
+USE_SYSTEM_STDLIB := 1
 
 include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers added inline comments.



Comment at: clang/CMakeLists.txt:6
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  # Please keep policies in sync with llvm/CMakeLists.txt.
+  if(POLICY CMP0114)

Seems error prone. Does cmake have an include system for including fragments 
from elsewhere? If so, that seems like a more concise and less error prone 
approach.

https://cmake.org/cmake/help/latest/command/include.html


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

https://reviews.llvm.org/D136572

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

[Lldb-commits] [PATCH] D136620: Change how debugserver clears auth bits from pc/fp/sp/lr with thread_get_state on Darwin

[Jason Molenda via Phabricator via lldb-commits]

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

When debugserver thread_get_state/thread_set_state's registers from an 
inferior, if the inferior is arm64e, debugserver must also be built arm64e, and 
debugserver passes the values through a series of macros provided by the kernel 
to authorize & clear auth bits off of the values that thread_get_state 
provides.  When the inferior process has crashed -- jumping through an 
improperly signed function pointer, or jumped to invalid memory, the pc value 
will fail to authenticate in these kernel macros.  On M2 
 era Mac hardware, this auth failure results in 
debugserver crashing.

We don't need to authenticate sp/pc/fp/lr, we only need to clear the auth bits 
from the address values.  This patch replaces the kernel macro accesses after 
thread_get_state to do that.  The macros like 
__darwin_arm_thread_state64_get_pc() are gated on `__has_feature(ptrauth_calls) 
&& defined(__LP64__)`, and in the case where we have `ptrauth_calls`, the 
register context structure in  are `void *` instead of 
`uint64_t`, so I needed to add a reinterpret cast of those values before 
clearing them.

It would probably be better to move my checks of `__has_feature(ptrauth_calls) 
&& defined(__LP64__)` into this `clear_pac_bits()` function and call it 
unconditionally, instead of testing at all of the caller sites.  (these two 
tests are distinguishing between arm64_32 v. arm64 v. arm64e)

In the case of thread_set_state, we will still use the kernel provided macros 
-- in this case, we are passing unsigned addresses and the signing will never 
fail.

With this patch, we still trigger the warning that the program has halted 
because of a PAC auth failure and show the most relevant pc value to explain 
it; no change in behavior there.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136620

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -4781,24 +4781,6 @@
   return g_host_cputype != 0;
 }
 
-static bool GetAddressingBits(uint32_t &addressing_bits) {
-  static uint32_t g_addressing_bits = 0;
-  static bool g_tried_addressing_bits_syscall = false;
-  if (g_tried_addressing_bits_syscall == false) {
-size_t len = sizeof (uint32_t);
-if (::sysctlbyname("machdep.virtual_address_size",
-  &g_addressing_bits, &len, NULL, 0) != 0) {
-  g_addressing_bits = 0;
-}
-  }
-  g_tried_addressing_bits_syscall = true;
-  addressing_bits = g_addressing_bits;
-  if (addressing_bits > 0)
-return true;
-  else
-return false;
-}
-
 rnb_err_t RNBRemote::HandlePacket_qHostInfo(const char *p) {
   std::ostringstream strm;
 
@@ -4812,7 +4794,7 @@
   }
 
   uint32_t addressing_bits = 0;
-  if (GetAddressingBits(addressing_bits)) {
+  if (DNBGetAddressingBits(addressing_bits)) {
 strm << "addressing_bits:" << std::dec << addressing_bits << ';';
   }
 
Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -94,11 +94,30 @@
 
 uint32_t DNBArchMachARM64::GetCPUType() { return CPU_TYPE_ARM64; }
 
+static uint64_t clear_pac_bits(uint64_t value) {
+  uint32_t addressing_bits = 0;
+  if (!DNBGetAddressingBits(addressing_bits) || addressing_bits == 0)
+return value;
+
+  uint64_t mask = ((1ULL << addressing_bits) - 1);
+
+  // Normally PAC bit clearing needs to check b55 and either set the
+  // non-addressing bits, or clear them.  But the register values we
+  // get from thread_get_state on an arm64e process don't follow this
+  // convention?, at least when there's been a PAC auth failure in
+  // the inferior.
+  // Userland processes are always in low memory, so this
+  // hardcoding b55 == 0 PAC stripping behavior here.
+
+  return value & mask; // high bits cleared to 0
+}
+
 uint64_t DNBArchMachARM64::GetPC(uint64_t failValue) {
   // Get program counter
   if (GetGPRState(false) == KERN_SUCCESS)
-#if defined(__LP64__)
-return arm_thread_state64_get_pc(m_state.context.gpr);
+#if __has_feature(ptrauth_calls) && defined(__LP64__)
+return clear_pac_bits(
+reinterpret_cast(m_state.context.gpr.__opaque_pc));
 #else
 return m_state.context.gpr.__pc;
 #endif
@@ -128,8 +147,9 @@
 uint64_t DNBArchMachARM64::Get

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

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

mgorny added inline comments.



Comment at: clang/CMakeLists.txt:6
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  # Please keep policies in sync with llvm/CMakeLists.txt.
+  if(POLICY CMP0114)

nickdesaulniers wrote:
> Seems error prone. Does cmake have an include system for including fragments 
> from elsewhere? If so, that seems like a more concise and less error prone 
> approach.
> 
> https://cmake.org/cmake/help/latest/command/include.html
I suppose that makes sense. I think that at this point all these components 
require the top-level `cmake` directory from monorepo anyway.


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

https://reviews.llvm.org/D136572

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

[Lldb-commits] [PATCH] D136610: [trace][intelpt] Fix multi CPU decoding TSC assertion error

[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 good! I just left cosmetic requests




Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:285-312
+  if (event.has_tsc) {
+if (m_tsc_upper_bound && event.tsc > m_tsc_upper_bound.value()) {
+  // This event and all the remaining events of this PSB have a TSC
+  // outside the range of the "owning" ThreadContinuousExecution. For
+  // now we drop all of these events/instructions, future work can
+  // improve upon this by determining the "owning"
+  // ThreadContinuousExecution of the remaining PSB data.

I suggest moving all of this to another function because it's a bit long



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:286
+  if (event.has_tsc) {
+if (m_tsc_upper_bound && event.tsc > m_tsc_upper_bound.value()) {
+  // This event and all the remaining events of this PSB have a TSC

use * instead of value(). value() is rarely used in LLVM code. Also use >= 
instead of >



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:294-295
+  formatv(
+  "TSC {0} exceeds maximum TSC value {1}. Will skip decoding"
+  " the remining the remaining data of the PSB.",
+  event.tsc, m_tsc_upper_bound.value())

add a prefix message for easier regexp search
  "decoding truncated: TSC packet {0} exceeds maximum TSC value {1} for this 
PSB block . Will resume decoding with the next PSB."

Use a better prefix if you can, and btw, you wrote `the remining the remaining 
` here. 



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:296
+  " the remining the remaining data of the PSB.",
+  event.tsc, m_tsc_upper_bound.value())
+  .str();

ditto



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:299-307
+  uint64_t offset;
+  int status = pt_insn_get_offset(m_decoder_up.get(), &offset);
+  if (!IsLibiptError(status)) {
+err_msg =
+formatv("At byte offset {0} of PSB with size {1} bytes, {2}",
+offset, m_psb_block.size, err_msg)
+.str();

I think let's better not include this to keep the error a bit smaller. In any 
case, you can do `thread trace dump instructions  -E` and then look for 
the error prefix when debugging.

but if you insist, the byte offset message should come after the textual 
description of the error



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:308
+  m_decoded_thread.AppendCustomError(err_msg);
+  return -1;
+} else {

return -pte_internal;



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:345
   DecodedThread &m_decoded_thread;
+  // Maximum allowed value of TSCs decoded from m_psb_block.
+  llvm::Optional m_tsc_upper_bound;

move this to the constructor so that it's highlighted by IDEs and appears in 
the public documentation

  Maximum allowed value of TSCs decoded from this psb block. If this value is 
hit, then decoding for this block is stopped and an error is appended to the 
trace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136610

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

[Lldb-commits] [PATCH] D136620: Change how debugserver clears auth bits from pc/fp/sp/lr with thread_get_state on Darwin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/tools/debugserver/source/DNB.cpp:1830-1837
+  static bool g_tried_addressing_bits_syscall = false;
+  if (g_tried_addressing_bits_syscall == false) {
+size_t len = sizeof(uint32_t);
+if (::sysctlbyname("machdep.virtual_address_size", &g_addressing_bits, 
&len,
+   NULL, 0) != 0) {
+  g_addressing_bits = 0;
+}

A more C++ and thread safe way of doing this is:

```
static std::once_flag g_once_flag;
std::call_once(g_once_flag, [&](){
size_t len = sizeof(uint32_t);
if (::sysctlbyname("machdep.virtual_address_size", &g_addressing_bits, 
&len,
   NULL, 0) != 0) {
  g_addressing_bits = 0;
}
}
```



Comment at: lldb/tools/debugserver/source/DNB.cpp:1840-1843
+  if (addressing_bits > 0)
+return true;
+  else
+return false;

```return g_addressing_bits > 0```



Comment at: 
lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:98-100
+  uint32_t addressing_bits = 0;
+  if (!DNBGetAddressingBits(addressing_bits) || addressing_bits == 0)
+return value;

Doesn't `DNBGetAddressingBits` already cover the case of `addressing_bits` 
being `0`? Why not have `DNBGetAddressingBits` return the addressing bits and 
do the check here?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136620

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

[Lldb-commits] [PATCH] D136209: [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

In D136209#3878821 , @labath wrote:

> So if I understand correctly, when we deserialize incorrectly, this causes us 
> to misclassify some function parameters (arguments) as local variables. Is 
> that correct? If yes, then that is something that can be checked. Even if the 
> misclassified variables behave perfectly, their kind is still visible, e.g. 
> in the output of "frame variable --scope".

It's the opposite. Some non parameters local variables are misclassified as 
parameters, since deserialized parameter count is incorrect/larger. Those 
misclassified local variables will have the parameter bit set. See the inline 
comment for detail.

Or maybe we should just rely on individual variable's debug info to determine 
if a local var is parameter or not, not the parameter count found in function 
debug info.




Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:1825
 
   is_param |= var_info.is_param;
   ValueType var_scope =

Because we also check individual variable's debug info kind (parameter or not), 
if the deserialized parameter count number is smaller than actual number, this 
would fix it. So, it only causes problem when the deserialized result is larger 
which causes non-parameter local variables count as parameters.

The reason for this check, I vaguely remember, is that some function debug info 
has missing parameter count (e.g. 0), but the individual variable debug info 
says it's a parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136209

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

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

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

mgorny updated this revision to Diff 470225.
mgorny edited the summary of this revision.
mgorny added a comment.
Herald added a project: LLVM.

Use a shared policy file via `include()` instead.


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

https://reviews.llvm.org/D136572

Files:
  clang/CMakeLists.txt
  cmake/Modules/CMakePolicy.cmake
  flang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  mlir/CMakeLists.txt

Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -1,4 +1,7 @@
 # MLIR project.
+cmake_minimum_required(VERSION 3.13.4)
+include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
 
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
@@ -10,8 +13,6 @@
 include(GNUInstallDirs)
 
 if(MLIR_STANDALONE_BUILD)
-  cmake_minimum_required(VERSION 3.13.4)
-
   find_package(LLVM CONFIG REQUIRED)
   set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${LLVM_CMAKE_DIR})
   include(HandleLLVMOptions)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -1,17 +1,8 @@
 # See docs/CMake.html for instructions about how to build LLVM with CMake.
 
 cmake_minimum_required(VERSION 3.13.4)
-
-# CMP0114: ExternalProject step targets fully adopt their steps.
-# New in CMake 3.19: https://cmake.org/cmake/help/latest/policy/CMP0114.html
-if(POLICY CMP0114)
-  cmake_policy(SET CMP0114 OLD)
-endif()
-# CMP0116: Ninja generators transform `DEPFILE`s from `add_custom_command()`
-# New in CMake 3.20. https://cmake.org/cmake/help/latest/policy/CMP0116.html
-if(POLICY CMP0116)
-  cmake_policy(SET CMP0116 OLD)
-endif()
+include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
 
 set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -1,9 +1,3 @@
-# CMP0116: Ninja generators transform `DEPFILE`s from `add_custom_command()`
-# New in CMake 3.20. https://cmake.org/cmake/help/latest/policy/CMP0116.html
-if(POLICY CMP0116)
-  cmake_policy(SET CMP0116 OLD)
-endif()
-
 if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
   set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 endif()
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -1,4 +1,6 @@
 cmake_minimum_required(VERSION 3.13.4)
+include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
 
 # Add path for custom modules.
 set(CMAKE_MODULE_PATH
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -1,4 +1,6 @@
 cmake_minimum_required(VERSION 3.13.4)
+include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
 
 # If we are not building as a part of LLVM, build LLD as an
 # standalone project, using LLVM as an external library:
Index: flang/CMakeLists.txt
===
--- flang/CMakeLists.txt
+++ flang/CMakeLists.txt
@@ -1,4 +1,6 @@
 cmake_minimum_required(VERSION 3.13.4)
+include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
 
 set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
Index: cmake/Modules/CMakePolicy.cmake
===
--- /dev/null
+++ cmake/Modules/CMakePolicy.cmake
@@ -0,0 +1,12 @@
+# CMake policy settings shared between LLVM projects
+
+# CMP0114: ExternalProject step targets fully adopt their steps.
+# New in CMake 3.19: https://cmake.org/cmake/help/latest/policy/CMP0114.html
+if(POLICY CMP0114)
+  cmake_policy(SET CMP0114 OLD)
+endif()
+# CMP0116: Ninja generators transform `DEPFILE`s from `add_custom_command()`
+# New in CMake 3.20. https://cmake.org/cmake/help/latest/policy/CMP0116.html
+if(POLICY CMP0116)
+  cmake_policy(SET CMP0116 OLD)
+endif()
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -1,4 +1,6 @@
 cmake_minimum_required(VERSION 3.13.4)
+include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
 
 # If we are not building as a part of LLVM, build Clang as an
 # standalone project, using LLVM as an external library:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5b17eb1 - [lldb] Fix stale diagnostic event comments (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-10-24T11:14:52-07:00
New Revision: 5b17eb1c1e152a6d7dbcb94d7613bc2a7ea0b92b

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

LOG: [lldb] Fix stale diagnostic event comments (NFC)

The diagnostic events were heavily inspired by the progress events and
several comments incorrectly referenced "progress" rather than
"diagnostic" events.

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/source/Core/Debugger.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index cfab213dcd026..394dac3662345 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -383,17 +383,17 @@ class Debugger : public 
std::enable_shared_from_this,
 
   /// Report warning events.
   ///
-  /// Progress events will be delivered to any debuggers that have listeners
-  /// for the eBroadcastBitError.
+  /// Warning events will be delivered to any debuggers that have listeners
+  /// for the eBroadcastBitWarning.
   ///
   /// \param[in] message
   ///   The warning message to be reported.
   ///
   /// \param [in] debugger_id
   ///   If this optional parameter has a value, it indicates the unique
-  ///   debugger identifier that this progress should be delivered to. If this
-  ///   optional parameter does not have a value, the progress will be
-  ///   delivered to all debuggers.
+  ///   debugger identifier that this diagnostic should be delivered to. If
+  ///   this optional parameter does not have a value, the diagnostic event
+  ///   will be delivered to all debuggers.
   ///
   /// \param [in] once
   ///   If a pointer is passed to a std::once_flag, then it will be used to
@@ -405,7 +405,7 @@ class Debugger : public 
std::enable_shared_from_this,
 
   /// Report error events.
   ///
-  /// Progress events will be delivered to any debuggers that have listeners
+  /// Error events will be delivered to any debuggers that have listeners
   /// for the eBroadcastBitError.
   ///
   /// \param[in] message
@@ -413,9 +413,9 @@ class Debugger : public 
std::enable_shared_from_this,
   ///
   /// \param [in] debugger_id
   ///   If this optional parameter has a value, it indicates the unique
-  ///   debugger identifier that this progress should be delivered to. If this
-  ///   optional parameter does not have a value, the progress will be
-  ///   delivered to all debuggers.
+  ///   debugger identifier that this diagnostic should be delivered to. If
+  ///   this optional parameter does not have a value, the diagnostic event
+  ///   will be delivered to all debuggers.
   ///
   /// \param [in] once
   ///   If a pointer is passed to a std::once_flag, then it will be used to

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 6ce811b825c56..e12979234bb96 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1338,7 +1338,7 @@ void 
Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
 llvm::Optional 
debugger_id,
 std::once_flag *once) {
   auto ReportDiagnosticLambda = [&]() {
-// Check if this progress is for a specific debugger.
+// Check if this diagnostic is for a specific debugger.
 if (debugger_id) {
   // It is debugger specific, grab it and deliver the event if the debugger
   // still exists.
@@ -1347,8 +1347,8 @@ void 
Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
 PrivateReportDiagnostic(*debugger_sp, type, std::move(message), true);
   return;
 }
-// The progress event is not debugger specific, iterate over all debuggers
-// and deliver a progress event to each one.
+// The diagnostic event is not debugger specific, iterate over all 
debuggers
+// and deliver a diagnostic event to each one.
 if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
   std::lock_guard guard(*g_debugger_list_mutex_ptr);
   for (const auto &debugger : *g_debugger_list_ptr)
@@ -1372,7 +1372,6 @@ void Debugger::ReportWarning(std::string message,
 void Debugger::ReportError(std::string message,
llvm::Optional debugger_id,
std::once_flag *once) {
-
   ReportDiagnosticImpl(DiagnosticEventData::Type::Error, std::move(message),
debugger_id, once);
 }



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

[Lldb-commits] [lldb] 7590776 - [lldb] Skip TestFullLtoStepping in older clangs

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2022-10-24T12:12:36-07:00
New Revision: 7590776b852f28ba8d569e17d989060f33d0e7a3

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

LOG: [lldb] Skip TestFullLtoStepping in older clangs

Added: 


Modified: 
lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py

Removed: 




diff  --git a/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py 
b/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py
index e697a53103b31..9a553e8afff6e 100644
--- a/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py
+++ b/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py
@@ -9,6 +9,7 @@
 class TestFullLtoStepping(TestBase):
 
 @skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 @skipUnlessDarwin
 def test(self):
 self.build()



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

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

In D136011#3870677 , @labath wrote:

> In D136011#3866854 , @aeubanks 
> wrote:
>
>> In D136011#3862150 , @labath wrote:
>>
>>> 
>>
>> Maybe looking for a "char" entry and setting 
>> `ast.getLangOpts().CharIsSigned` from it would probably work in most cases, 
>> but not sure that it's worth the effort. Rendering the wrong signedness for 
>> chars doesn't seem like the worst thing, and this patch improves things (as 
>> the removed `expectedFailureAll` show, plus this helps with some testing of 
>> lldb I've been doing). I can add a TODO if you'd like.
>
> It's not just a question of rendering. This can result in wrong/unexpected 
> results of expressions as well. Something as simple as `p 
> (int)unspecified_char_value` can return a different result depending on 
> whether `unspecified_char_value` is considered signed or unsigned.
> However, I am not convinced that we would be better off trying to detect this 
> would, as that detection can fail as well, only in more unpredictable ways.
>
> In D136011#3868755 , @dblaikie 
> wrote:
>
>> Yeah, this line of reasoning (non-homogenaeity) is one I'm on board with, 
>> thanks for the framing. Basically I/we can think of the debugger's 
>> expression context as some code that's compiled with the default char 
>> signedness always. Since char signedness isn't part of the ABI, bits of the 
>> program can be built with one and bits with the other - and the debugger is 
>> just a bit that's built with the default.
>
> Are you sure about the ABI part? I guess it may depend on how you define the 
> ABI.. It definitely does not affect the calling conventions, but I'm pretty 
> sure it would be an ODR violation if you even had a simple function like `int 
> to_int(char c) { return c; }` in a header, and compiled it with both -fsigned 
> and -funsigned-char. Not that this will stop people from doing it, but the 
> most likely scenario for this is that your linking your application with the 
> C library compiled in a different mode, where it is kinda ok, because the C 
> library does not have many inline functions, and doesn't do much char 
> manipulation.

"ODR violation" gets a bit fuzzy - but yeah, you'd get a different int out of 
that function. My point was since apps would already end up with a mix of 
signed and unsigned most likely, the debugger expression evaluation is one of 
those things mixed one way rather than the other.

But yeah, it's all a bit awkward (& probably will be more for googlers where 
everything's built with the non-default signedness - and now the expression 
evaluator will do the other thing/not that - but hopefully a rather small 
number of users ever notice or care about this). I guess char buffers for 
raw/binary data will be a bit more annoying to look at, dumped as signed 
integers instead of unsigned...

Anyway, yeah, it's all a bit awkward but this seems like the best thing for 
now. Thanks for weighing in!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136011

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

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

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

mgorny added a comment.

Thanks. Since this is not urgent, I'll let it wait for others to chime in a 
while more.


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

https://reviews.llvm.org/D136572

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

[Lldb-commits] [PATCH] D135622: [lldb] Add a "diagnostics dump" command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 470283.
JDevlieghere marked 8 inline comments as done.
JDevlieghere added a comment.

Address code review feedback


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

https://reviews.llvm.org/D135622

Files:
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectDiagnostics.cpp
  lldb/source/Commands/CommandObjectDiagnostics.h
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Utility/Diagnostics.cpp
  lldb/test/Shell/Diagnostics/TestDump.test

Index: lldb/test/Shell/Diagnostics/TestDump.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestDump.test
@@ -0,0 +1,15 @@
+# Check that the diagnostics dump command uses the correct directory and
+# creates one if needed.
+
+# Dump to an existing directory.
+# RUN: rm -rf %t.existing
+# RUN: mkdir -p %t.existing
+# RUN: %lldb -o 'diagnostics dump -d %t.existing'
+# RUN: file %t.existing | FileCheck %s
+
+# Dump to a non-existing directory.
+# RUN: rm -rf %t.nonexisting
+# RUN: %lldb -o 'diagnostics dump -d %t.nonexisting'
+# RUN: file %t.nonexisting | FileCheck %s
+
+# CHECK: : directory
Index: lldb/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -44,19 +44,21 @@
 }
 
 bool Diagnostics::Dump(raw_ostream &stream) {
-  SmallString<128> diagnostics_dir;
-  std::error_code ec =
-  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
-  if (ec) {
+  Expected diagnostics_dir = CreateUniqueDirectory();
+  if (!diagnostics_dir) {
 stream << "unable to create diagnostic dir: "
-   << toString(errorCodeToError(ec)) << '\n';
+   << toString(diagnostics_dir.takeError()) << '\n';
 return false;
   }
 
-  stream << "LLDB diagnostics written to " << diagnostics_dir << "\n";
+  return Dump(stream, *diagnostics_dir);
+}
+
+bool Diagnostics::Dump(raw_ostream &stream, const FileSpec &dir) {
+  stream << "LLDB diagnostics written to " << dir.GetPath() << "\n";
   stream << "Please include the directory content when filing a bug report\n";
 
-  Error error = Create(FileSpec(diagnostics_dir.str()));
+  Error error = Create(dir);
   if (error) {
 stream << toString(std::move(error)) << '\n';
 return false;
@@ -65,6 +67,15 @@
   return true;
 }
 
+llvm::Expected Diagnostics::CreateUniqueDirectory() {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
+  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
+  if (ec)
+return errorCodeToError(ec);
+  return FileSpec(diagnostics_dir.str());
+}
+
 Error Diagnostics::Create(const FileSpec &dir) {
   for (Callback c : m_callbacks) {
 if (Error err = c(dir))
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -15,6 +15,7 @@
 #include "Commands/CommandObjectApropos.h"
 #include "Commands/CommandObjectBreakpoint.h"
 #include "Commands/CommandObjectCommands.h"
+#include "Commands/CommandObjectDiagnostics.h"
 #include "Commands/CommandObjectDisassemble.h"
 #include "Commands/CommandObjectExpression.h"
 #include "Commands/CommandObjectFrame.h"
@@ -518,6 +519,7 @@
   REGISTER_COMMAND_OBJECT("apropos", CommandObjectApropos);
   REGISTER_COMMAND_OBJECT("breakpoint", CommandObjectMultiwordBreakpoint);
   REGISTER_COMMAND_OBJECT("command", CommandObjectMultiwordCommands);
+  REGISTER_COMMAND_OBJECT("diagnostics", CommandObjectDiagnostics);
   REGISTER_COMMAND_OBJECT("disassemble", CommandObjectDisassemble);
   REGISTER_COMMAND_OBJECT("expression", CommandObjectExpression);
   REGISTER_COMMAND_OBJECT("frame", CommandObjectMultiwordFrame);
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -343,6 +343,11 @@
 Desc<"Force disassembly of large functions.">;
 }
 
+let Command = "diagnostics dump" in {
+  def diagnostics_dump_directory : Option<"directory", "d">, Group<1>,
+Arg<"Path">, Desc<"Dump the diagnostics to the given directory.">;
+}
+
 let Command = "expression" in {
   def expression_options_all_threads : Option<"all-threads", "a">,
 Groups<[1,2]>, Arg<"Boolean">, Desc<"Should we run all threads if the "
Index: lldb/source/Commands/CommandObjectDiagnostics.h
===
--- /dev/null
+++ lldb/source/Commands/CommandObjectDiagnostics.h
@@ -0,0 +1,29 @@
+//===-- CommandObjectDiagnostics.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/

[Lldb-commits] [PATCH] D135622: [lldb] Add a "diagnostics dump" command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Commands/CommandObjectDiagnostics.cpp:84
+if (!directory) {
+  result.AppendError(llvm::toString(directory.takeError()));
+  result.SetStatus(eReturnStatusFailed);

clayborg wrote:
> Do we want to just create a temp directory if the user didn't specify one 
> here and them emit the path to where things were saved instead of erroring 
> out? 
Yes, that's exactly what we do. If we get here we were unable to create either 
of them.



Comment at: lldb/source/Commands/Options.td:348
+  def diagnostics_dump_directory : Option<"directory", "d">, Group<1>,
+Arg<"Path">, Desc<"Dump the diagnostics to the given directory.">;
+}

DavidSpickett wrote:
> Worth clarifying whether the path can be absolute or relative to the current 
> working dir?
> 
> I'd guess either works.
Given that either works I think we don't need to be explicit about it.



Comment at: lldb/source/Utility/Diagnostics.cpp:62
 stream << "unable to create diagnostic dir: "
-   << toString(errorCodeToError(ec)) << '\n';
+   << toString(diagnostics_dir.takeError()) << '\n';
 return false;

DavidSpickett wrote:
> You `std::move`'d some errors above does that apply here?
> 
> (I'm not sure exactly what requires it and what doesn't)
`toString` takes ownership of the error, so it needs to be moved here. Errors 
cannot be copied, so they always need to be moved, except when RVO kicks in. 



Comment at: lldb/source/Utility/Diagnostics.cpp:70
 
-  Error error = Create(FileSpec(diagnostics_dir.str()));
+  Error error = Create(*diagnostics_dir);
   if (error) {

DavidSpickett wrote:
> Silly question, is it intentional that we write out files after saying they 
> have been written?
> 
> Of course it makes sense to print something up front, otherwise the error 
> here would come out of the blue, but perhaps "LLDB diagnostics will be 
> written to..." instead?
> 
> (this is probably better changed in that earlier patch not sure if that went 
> in yet)
Yes, that was @labath's suggestion because when we call this from the signal 
handler, we might crash again in the process and wouldn't print anything at 
all. 


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

https://reviews.llvm.org/D135622

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

[Lldb-commits] [PATCH] D136620: Change how debugserver clears auth bits from pc/fp/sp/lr with thread_get_state on Darwin

[Jason Molenda via Phabricator via lldb-commits]

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

Update patch to address Jonas' comments -- using a std::call_once to initialize 
the number of addressing bits, and making the return clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136620

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -4781,24 +4781,6 @@
   return g_host_cputype != 0;
 }
 
-static bool GetAddressingBits(uint32_t &addressing_bits) {
-  static uint32_t g_addressing_bits = 0;
-  static bool g_tried_addressing_bits_syscall = false;
-  if (g_tried_addressing_bits_syscall == false) {
-size_t len = sizeof (uint32_t);
-if (::sysctlbyname("machdep.virtual_address_size",
-  &g_addressing_bits, &len, NULL, 0) != 0) {
-  g_addressing_bits = 0;
-}
-  }
-  g_tried_addressing_bits_syscall = true;
-  addressing_bits = g_addressing_bits;
-  if (addressing_bits > 0)
-return true;
-  else
-return false;
-}
-
 rnb_err_t RNBRemote::HandlePacket_qHostInfo(const char *p) {
   std::ostringstream strm;
 
@@ -4812,7 +4794,7 @@
   }
 
   uint32_t addressing_bits = 0;
-  if (GetAddressingBits(addressing_bits)) {
+  if (DNBGetAddressingBits(addressing_bits)) {
 strm << "addressing_bits:" << std::dec << addressing_bits << ';';
   }
 
Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -94,11 +94,35 @@
 
 uint32_t DNBArchMachARM64::GetCPUType() { return CPU_TYPE_ARM64; }
 
+static uint64_t clear_pac_bits(uint64_t value) {
+  uint32_t addressing_bits = 0;
+  if (!DNBGetAddressingBits(addressing_bits))
+return value;
+
+// On arm64_32, no ptrauth bits to clear
+#if !defined(__LP64__)
+  return value;
+#endif
+
+  uint64_t mask = ((1ULL << addressing_bits) - 1);
+
+  // Normally PAC bit clearing needs to check b55 and either set the
+  // non-addressing bits, or clear them.  But the register values we
+  // get from thread_get_state on an arm64e process don't follow this
+  // convention?, at least when there's been a PAC auth failure in
+  // the inferior.
+  // Userland processes are always in low memory, so this
+  // hardcoding b55 == 0 PAC stripping behavior here.
+
+  return value & mask; // high bits cleared to 0
+}
+
 uint64_t DNBArchMachARM64::GetPC(uint64_t failValue) {
   // Get program counter
   if (GetGPRState(false) == KERN_SUCCESS)
-#if defined(__LP64__)
-return arm_thread_state64_get_pc(m_state.context.gpr);
+#if __has_feature(ptrauth_calls) && defined(__LP64__)
+return clear_pac_bits(
+reinterpret_cast(m_state.context.gpr.__opaque_pc));
 #else
 return m_state.context.gpr.__pc;
 #endif
@@ -128,8 +152,9 @@
 uint64_t DNBArchMachARM64::GetSP(uint64_t failValue) {
   // Get stack pointer
   if (GetGPRState(false) == KERN_SUCCESS)
-#if defined(__LP64__)
-return arm_thread_state64_get_sp(m_state.context.gpr);
+#if __has_feature(ptrauth_calls) && defined(__LP64__)
+return clear_pac_bits(
+reinterpret_cast(m_state.context.gpr.__opaque_sp));
 #else
 return m_state.context.gpr.__sp;
 #endif
@@ -150,16 +175,20 @@
   if (DNBLogEnabledForAny(LOG_THREAD)) {
 uint64_t *x = &m_state.context.gpr.__x[0];
 
-#if defined(__LP64__)
-uint64_t log_fp = arm_thread_state64_get_fp(m_state.context.gpr);
-uint64_t log_lr = arm_thread_state64_get_lr(m_state.context.gpr);
-uint64_t log_sp = arm_thread_state64_get_sp(m_state.context.gpr);
-uint64_t log_pc = arm_thread_state64_get_pc(m_state.context.gpr);
+#if __has_feature(ptrauth_calls) && defined(__LP64__)
+uint64_t log_fp = clear_pac_bits(
+reinterpret_cast(m_state.context.gpr.__opaque_fp));
+uint64_t log_lr = clear_pac_bits(
+reinterpret_cast(m_state.context.gpr.__opaque_lr));
+uint64_t log_sp = clear_pac_bits(
+reinterpret_cast(m_state.context.gpr.__opaque_sp));
+uint64_t log_pc = clear_pac_bits(
+reinterpret_cast(m_state.context.gpr.__opaque_pc));
 #else
-uint64_t log_fp = m_state.context.gpr.__fp;
-uint64_t log_lr = m_state.context.gpr.__lr;
-uint64_t log_sp = m_state.context.gpr.__sp;
-uint64_t log_pc = m_state.context.gpr.__pc;
+uint64_t log_fp = m_state.context.gpr.__fp;
+uint64_t log_lr = m_state.context.gpr.__lr;
+uint64_t log_sp = m_state.context.gpr.__sp;
+uint64_t log_pc = m_state.context.gpr.__pc;
 #

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

[Will Hawkins via Phabricator via lldb-commits]

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

I updated this revision with the following change and I *think* that things are 
happy again:

  diff --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
  index 226a2c3f690f..1f939c6fa2cd 100644
  --- a/lldb/source/Core/ValueObject.cpp
  +++ b/lldb/source/Core/ValueObject.cpp
  @@ -2004,6 +2004,11 @@ void ValueObject::GetExpressionPath(Stream &s,
 s.PutChar(')');
 }
   
  +  if (m_flags.m_is_array_item_for_pointer &&
  +  epformat == eGetExpressionPathFormatHonorPointers) {
  +s.PutCString(m_name.GetStringRef());
  +  }
  +
 if (print_obj_access)
   s.PutChar('.');
 if (print_ptr_access)

Let me know if that is helpful.

Sincerely,
Will


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132734

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Is there any way to test this?




Comment at: 
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:95-97
+  if (gdbstub_port) {
+local_port = std::stoi(gdbstub_port);
+  }

remove {} for single line if statements per LLVM coding guidelines



Comment at: 
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:137-139
+  if (platform_local_port) {
+local_port = std::stoi(platform_local_port);
+  }

remove {} for single line if statements per LLVM coding guidelines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136648: [lldb] Emit diagnostic events in the diagnostic dump

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg, kastiglione.
Herald added a project: All.
JDevlieghere requested review of this revision.

Emit diagnostic events (errors and warnings) in the diagnostic dump. This patch 
also adds an "information" diagnostic event, which doesn't get broadcast, but 
ends up in the diagnostics as well.


https://reviews.llvm.org/D136648

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/DebuggerEvents.cpp
  lldb/source/Utility/Diagnostics.cpp
  lldb/test/Shell/Diagnostics/TestDiagnosticsLog.test

Index: lldb/test/Shell/Diagnostics/TestDiagnosticsLog.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestDiagnosticsLog.test
@@ -0,0 +1,33 @@
+# RUN: yaml2obj %s -o %t
+# RUN: mkdir -p %t.diags
+# RUN: %lldb -c %t -o 'diagnostics dump -d %t.diags' 2> %t.stderr
+
+# RUN: cat %t.stderr | FileCheck %s --check-prefix ERROR
+# RUN: cat %t.diags/diagnostics.log | FileCheck %s --check-prefix ERROR
+
+# ERROR: unable to retrieve process ID from minidump file, setting process ID to 1
+
+--- !minidump
+Streams:
+  - Type:ThreadList
+Threads:
+  - Thread Id:   0x3E81
+Context: 0B001000330006020100100010A234EBFC7F10A234EBFC7FF09C34EBFC7FC0A91ABCE97FA0163FBCE97F4602921C400030A434EBFC7FC61D40007F03801F0000FF00252525252525252525252525252525250000FF00FF00
+Stack:
+  Start of Memory Range: 0x7FFCEB34A000
+  Content: ''
+  - Type:ModuleList
+Modules:
+  - Base of Image:   0x0040
+Size of Image:   0x00017000
+Module Name: 'a.out'
+CodeView Record: ''
+  - Type:SystemInfo
+Processor Arch:  AMD64
+Platform ID: Linux
+CSD Version: 'Linux 3.13'
+CPU:
+  Vendor ID:   GenuineIntel
+  Version Info:0x
+  Feature Info:0x
+...
Index: lldb/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -17,6 +17,8 @@
 using namespace lldb;
 using namespace llvm;
 
+static constexpr size_t g_num_log_messages = 100;
+
 void Diagnostics::Initialize() {
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace();
@@ -34,7 +36,7 @@
 
 Diagnostics &Diagnostics::Instance() { return *InstanceImpl(); }
 
-Diagnostics::Diagnostics() {}
+Diagnostics::Diagnostics() : m_log_handler(g_num_log_messag

[Lldb-commits] [lldb] dd00c4d - Fix breakpoint setting so it always works when there is a line entry in a compile unit's line table.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2022-10-24T16:28:39-07:00
New Revision: dd00c4db747e27f53a5621a8b9c50711f00894ef

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

LOG: Fix breakpoint setting so it always works when there is a line entry in a 
compile unit's line table.

Prior to this fix, if the compile unit function:

  void CompileUnit::ResolveSymbolContext(const SourceLocationSpec 
&src_location_spec, SymbolContextItem resolve_scope, SymbolContextList 
&sc_list);

was called with a resolve scope that wasn't just eSymbolContextLineEntry, we 
would end up calling:

  line_entry.range.GetBaseAddress().CalculateSymbolContext(&sc, resolve_scope);

This is ok as long as the line entry's base address is able to be resolved back 
to the same information, but there were problems when it didn't. The example I 
found was we have a file with a bad .debug_aranges section where the address to 
compile unit mapping was incomplete. When this happens, the above function call 
to calculate the symbol context would end up matching the module and it would 
NULL out the compile unit and line entry, which means we would fail to set this 
breakpoint. We have many other clients that ask for eSymbolContextEverything as 
the resolve_scope, so all other locations could end up failing as well.

The solutions is to make sure the compile unit matches the current compile unit 
after calling the calculate symbol context. If the compile unit is NULL, then 
we report an error via the module/debugger as this indicates an entry in the 
line table fails to resolve back to any compile unit. If the compile unit is 
not NULL and it differs from the current compile unit, we restore the current 
compile unit and line entry to ensure the call to .CalculateSymbolContext 
doesn't match something completely different, as can easily happen if LTO or 
other link time optimizations are enabled that could end up outlining or 
merging functions.

This patch allows breakpoint succeeding to work as expected and not get short 
circuited by our address lookup logic failing.

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

Added: 
lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml

Modified: 
lldb/source/Symbol/CompileUnit.cpp

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

Removed: 




diff  --git a/lldb/source/Symbol/CompileUnit.cpp 
b/lldb/source/Symbol/CompileUnit.cpp
index da9dc95d61fb6..8b979d352fc82 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -330,14 +330,44 @@ void CompileUnit::ResolveSymbolContext(
 // If they only asked for the line entry, then we're done, we can
 // just copy that over. But if they wanted more than just the line
 // number, fill it in.
+SymbolContext resolved_sc;
+sc.line_entry = line_entry;
 if (resolve_scope == eSymbolContextLineEntry) {
-  sc.line_entry = line_entry;
+  sc_list.Append(sc);
 } else {
-  line_entry.range.GetBaseAddress().CalculateSymbolContext(&sc,
+  line_entry.range.GetBaseAddress().CalculateSymbolContext(&resolved_sc,
resolve_scope);
+  // Sometimes debug info is bad and isn't able to resolve the line entry's
+  // address back to the same compile unit and/or line entry. If the 
compile
+  // unit changed, then revert back to just the compile unit and line 
entry.
+  // Prior to this fix, the above code might end up not being able to 
lookup
+  // the address, and then it would clear compile unit and the line entry 
in
+  // the symbol context and the breakpoint would fail to get set even 
though
+  // we have a valid line table entry in this compile unit. The address
+  // lookup can also end up finding another function in another compiler
+  // unit if the DWARF has overlappging address ranges. So if we end up 
with
+  // no compile unit or a 
diff erent one after the above function call,
+  // revert back to the same results as if resolve_scope was set exactly to
+  // eSymbolContextLineEntry.
+  if (resolved_sc.comp_unit == this) {
+sc_list.Append(resolved_sc);
+  } else {
+if (resolved_sc.comp_unit == nullptr && resolved_sc.module_sp) {
+  // Only report an error if we don't map back to any compile unit. 
With
+  // link time optimizations, the debug info might have many compile
+  // units that have the same address range due to function outlining
+  // or other link time optimizations. If the compile unit is NULL, 
then
+  // address resolving is completely failing and more deserving of an
+  // error message the user can see.
+   

[Lldb-commits] [PATCH] D136207: [lldb] Fix breakpoint setting so it always works when there is a line entry in a compile unit's line table.

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd00c4db747e: Fix breakpoint setting so it always works when 
there is a line entry in a… (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136207

Files:
  lldb/source/Symbol/CompileUnit.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml

Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml
@@ -0,0 +1,445 @@
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1240
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:030921EA-6D76-3A68-B515-386B9AF6D568
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   786432
+sdk: 787200
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   2
+stroff:  4128
+strsize: 28
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  16384
+fileoff: 0
+filesize:0
+maxprot: 5
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x13F94
+size:36
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x8400
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B00
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x13FB8
+size:72
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B001800030921EA6D763A68B515386B9AF6D5683200180001000C00
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __LINKEDIT
+vmaddr:  4294983680
+vmsize:  4096
+fileoff: 4096
+filesize:60
+maxprot: 1
+initprot:1
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 792
+segname: __DWARF
+vmaddr:  4294987776
+vmsize:  4096
+fileoff: 8192
+filesize:796
+maxprot: 7
+initprot:3
+nsects:  9
+flags:   0
+Sections:
+  - sectname:__debug_line
+segname: __DWARF
+addr:0x15000
+size:64
+offset:  0x2000
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_aranges
+segname: __DWARF
+addr:0x15040
+size:48
+offset:  0x2040
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_info
+segname: __DWARF
+addr:0x15070
+size:148
+offset:  0x2070
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_abbrev
+segname: __DWARF
+addr: 

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks updated this revision to Diff 470321.
aeubanks marked 3 inline comments as done.
aeubanks added a comment.

address comments

rebasing to ToT, TestCPPBreakpointsLocations.py is failing when using 
-gsimple-template-names. still working on that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
  lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
  lldb/test/API/lang/cpp/unique-types2/Makefile
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -0,0 +1,22 @@
+template  class Foo {
+  T t;
+};
+
+template  class FooPack {
+  T t;
+};
+
+int main() {
+  Foo t1;
+  Foo t2;
+  Foo> t3;
+
+  FooPack p1;
+  FooPack p2;
+  FooPack> p3;
+  FooPack t4;
+  FooPack t5;
+  FooPack t6;
+  FooPack t7;
+  // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
@@ -0,0 +1,40 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase(TestBase):
+def do_test(self, debug_flags):
+"""Test that we only display the requested Foo instantiation, not all Foo instantiations."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_name(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_name(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
+
Index: lldb/test/API/lang/cpp/unique-types2/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -51,7 +51,7 @@
 # Record types without a defining declaration are not complete.
 self.assertPointeeIncomplete("FwdClass *", "fwd_class")
 self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+self.assertPointeeIncomplete("FwdTemplateClass *", "fwd_template_class")
 
 # A pointer type is complete even when it points to an incomplete

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1551
+}
+if (!all_template_names.empty()) {
+  all_template_names.append(">");

labath wrote:
> When can this be empty? Should we still include the `<>` in those cases?
it was empty when we're working with a non-templated die, so a lot of cases. 
I've added an early exit and changed this to an assert



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2500
+const llvm::StringRef nameRef = name.GetStringRef();
+auto it = nameRef.find('<');
+if (it != llvm::StringRef::npos) {

labath wrote:
> Michael137 wrote:
> > Is there some other way to determine whether the debug-info was generated 
> > from `-gsimple-template-names`? Then we wouldn't have to check the 
> > existence of a `<` in the name in multiple places and wouldn't have to do 
> > this lookup speculatively. With this change, if we fail to find a template 
> > type in the index, we would do the lookup all over again, only to not find 
> > it again. Could that get expensive? Just trying to figure out if we can 
> > avoid doing this `GetTypes` call twice.
> > 
> > There have been [talks](https://github.com/llvm/llvm-project/issues/58362) 
> > about doing a base-name search by default followed by fuzzy matching on 
> > template parameters (though in the context of function names). The 
> > `simple-template-names` came up as a good motivator/facilitator for doing 
> > so. But for that idea to work here we'd have to be able to retrieve from 
> > the index by basename, so perhaps out of the scope of what we're trying to 
> > do here
> > 
> > tagging people involved in that discussion: @dblaikie @aprantl @jingham 
> > @labath
> > 
> > Other than this, generally LGTM
> Negative matches should be fast, and since typically only one of the branches 
> will produce any results, I think this should be fine. Filtering matches in 
> the simplified case would be slower, since you'd have build the full name of 
> all potential candidates (and that could be thousands of instantiations), but 
> I don't know how slow. But that's the price we pay for better filtering 
> (which we don't do right now, but we could start doing afterwards).
Done.

I believe the only difference with `-gsimple-template-names` is that the name 
doesn't contain the template parameters, so I don't think there's any other way 
of determining if the debug info was generated from `-gsimple-template-names`.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2577
 
   ConstString name = pattern.back().name;
 

labath wrote:
> Do we need to do something similar in this FindTypes overload as well?
I was wondering when this code path is even taken, turns out it's only called 
from lldb-test, and only in two tests:
```
  lldb-shell :: SymbolFile/DWARF/x86/compilercontext.ll
  lldb-shell :: SymbolFile/DWARF/x86/module-ownership.mm
```
so probably doesn't need to be updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D136650: Add a check for TypeSystem use-after-free problems

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: kastiglione, jingham, labath.
Herald added a project: All.
aprantl requested review of this revision.

When a process gets restarted TypeSystem objects associated with it may get 
deleted, and any CompilerType objects holding on to a reference to that type 
system are a use-after-free in waiting. Because of the SBAPI, we don't have 
tight control over where CompilerTypes go and when they are used. This is 
particularly a problem in the Swift plugin, where the scratch TypeSystem can be 
restarted while the process is still running. The Swift plugin has a lock to 
prevent abuse, but where there's a lock there can be bugs.

To help diagnose these hard-to-debug problems (and because Halloween is right 
around the corner) this patch introduces a global TypeSystem graveyard that 
collects all dead TypeSystem pointers and checks against it in 
CompilerType::IsValid(). This is intended as a bug-finding tool, which is why 
this triggers lldbassert(). Compared to everything else LLDB is doing the extra 
DenseMap lookup and lock should be negligible, which is why this feature is 
turned on even in release mode.

rdar://101505232


https://reviews.llvm.org/D136650

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Symbol/TypeSystem.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp

Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -29,6 +29,7 @@
   void SetUp() override {
 m_ast.reset(
 new TypeSystemClang("test ASTContext", HostInfo::GetTargetTriple()));
+GetTypeSystemGraveyard().Erase(m_ast.get());
   }
 
   void TearDown() override { m_ast.reset(); }
Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -17,6 +17,12 @@
 using namespace lldb_private;
 using namespace lldb;
 
+TypeSystemSet &lldb_private::GetTypeSystemGraveyard() {
+  // Intentionally leaked to avoid C++ destructor ordering issue.
+  static TypeSystemSet *g_typesystem_graveyard = new TypeSystemSet();
+  return *g_typesystem_graveyard;
+}
+
 /// A 64-bit SmallBitVector is only small up to 64-7 bits, and the
 /// setBitsInMask interface wants to write full bytes.
 static const size_t g_num_small_bitvector_bits = 64 - 8;
@@ -35,7 +41,11 @@
 bool LanguageSet::Empty() const { return bitvector.none(); }
 bool LanguageSet::operator[](unsigned i) const { return bitvector[i]; }
 
-TypeSystem::~TypeSystem() = default;
+TypeSystem::TypeSystem() {
+  // Remove this pointer from the graveyard in case it got reused.
+  GetTypeSystemGraveyard().Erase(this);
+}
+void TypeSystem::Finalize() { GetTypeSystemGraveyard().Insert(this, false); }
 
 static lldb::TypeSystemSP CreateInstanceHelper(lldb::LanguageType language,
Module *module, Target *target) {
Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -11,11 +11,13 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Scalar.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
@@ -26,6 +28,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
+bool CompilerType::IsValid() const {
+  if (!m_type || !m_type_system)
+return false;
+  bool unused;
+  if (GetTypeSystemGraveyard().Lookup(m_type_system, unused)) {
+lldbassert(false && "CompilerType belongs to deleted Typesystem");
+return false;
+  }
+  return true;
+}
+
 // Tests
 
 bool CompilerType::IsAggregateType() const {
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -552,7 +552,8 @@
 }
 
 TypeSystemClang::TypeSystemClang(llvm::StringRef name,
- llvm::Triple target_triple) {
+ llvm::Triple target_triple)
+: TypeSystem() {
   m_display_name = name.str();
   if (!target_triple.str().empty())
 SetTargetTriple(target_triple.str());
@@ -562,7 +563,8 @@
 }
 
 TypeSystemClang::TypeSystemClang(llvm::StringRe

[Lldb-commits] [PATCH] D136557: [trace][intel pt] Simple detection of infinite decoding loops

[Jakob Johnson via Phabricator via lldb-commits]

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

looks good overall, mainly some questions and a few nits




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:123
 /// of a DenseMap because DenseMap can't understand enums.
-std::unordered_map events_counts;
-size_t total_count = 0;
+std::unordered_map events_counts;
+uint64_t total_count = 0;





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:130
+  // Struct holding counts for errors
+  struct ErrorStats {
+/// The following counters are mutually exclusive

nice, I was about to add this as part of my diff (:



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:206
+  }
+  m_next_infinite_decoding_loop_threshold *= 2;
+}

can you explain why we are increasing the threshold?



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:235-241
+if (m_decoded_thread.GetInstructionLoadAddress(last_insn_index) !=
+eTraceItemKindInstruction) {
+  if (Optional prev_index = prev_insn_index(last_insn_index)) {
+last_insn_index = *prev_index;
+  } else {
+return None;
+  }

if you move the `--item_index` in `prev_insn_index` lambda, would that allow 
you to remove this duplicated `eTraceItemKindInstruction` check and instead 
unconditionally call `prev_insn_index`?
or would this not work because the intention of the lamda is to skip the 
current event even if it's already an instruction



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:281
+lldb::addr_t new_packet_offset;
+if (!IsLibiptError(pt_insn_get_offset(&m_decoder, &new_packet_offset)) &&
+new_packet_offset != m_last_packet_offset) {

help me understand this please. I thought `pt_insn_get_offset` would always 
return a new, increasing offset every time this function is called.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136557

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

[Lldb-commits] [PATCH] D136557: [trace][intel pt] Simple detection of infinite decoding loops

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:123
 /// of a DenseMap because DenseMap can't understand enums.
-std::unordered_map events_counts;
-size_t total_count = 0;
+std::unordered_map events_counts;
+uint64_t total_count = 0;

jj10306 wrote:
> 
ahh good one



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:130
+  // Struct holding counts for errors
+  struct ErrorStats {
+/// The following counters are mutually exclusive

jj10306 wrote:
> nice, I was about to add this as part of my diff (:
oh nice!



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:206
+  }
+  m_next_infinite_decoding_loop_threshold *= 2;
+}

jj10306 wrote:
> can you explain why we are increasing the threshold?
the idea is to check for infinite loops sporadically without making the total 
checks in O(N^2) and instead do it in O(N)

If we first do a linear check in the trace, which is O(T) after T instructions 
are appended and there are no loops, we might want to check again in the 
future. We could wait for the next T instructions and then run another check, 
and if we fail, wait for the next T and so on. This result in a total time 
spent of O(T + 2T + 3T + 4T + ... + N) which is O(N^2). Instead, we can run the 
check after 2T, and then after 4T and then after 8T and so on. This gives us a 
geometric progression of (N + N/2 + N / 4 + ... + T) which is amortized total 
O(N). A similar algorithm is vector::push_back 
(https://cs.stackexchange.com/questions/9380/why-is-push-back-in-c-vectors-constant-amortized)
 which is total O(N) using a similar approach.



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:235-241
+if (m_decoded_thread.GetInstructionLoadAddress(last_insn_index) !=
+eTraceItemKindInstruction) {
+  if (Optional prev_index = prev_insn_index(last_insn_index)) {
+last_insn_index = *prev_index;
+  } else {
+return None;
+  }

jj10306 wrote:
> if you move the `--item_index` in `prev_insn_index` lambda, would that allow 
> you to remove this duplicated `eTraceItemKindInstruction` check and instead 
> unconditionally call `prev_insn_index`?
> or would this not work because the intention of the lamda is to skip the 
> current event even if it's already an instruction
I like your idea. I think I can simplify the code



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:281
+lldb::addr_t new_packet_offset;
+if (!IsLibiptError(pt_insn_get_offset(&m_decoder, &new_packet_offset)) &&
+new_packet_offset != m_last_packet_offset) {

jj10306 wrote:
> help me understand this please. I thought `pt_insn_get_offset` would always 
> return a new, increasing offset every time this function is called.
Not really. pt_insn_get_offset returns the offset of the last packet that was 
processed, and that single could lead to many individual sequential 
instructions until the next packet is needed.
 
Let's imagine that you have this trace

PSB with starting address of 0xAAA
TNT with 4 bits
TIP with address 0xFFF

What the decoder will do is to first read the PSB and start at IP 0xAAA. It'll 
then decode sequential instructions until it reaches the first branch or jump. 
It then needs to read the next packet, which is the TNT with 4 bits, so it will 
help decode the next 4 branches but not the fifth one. So the decoder will 
change the offset and resume decoding instructions sequentially until that 
fifth branch (or jump) is reached. Then the decoder will read the next packet, 
which is a TIP and tells the decoder to jump to address 0xFFF.
So this means that with the PSB, the decoder produced, let's say, 10 
instructions, and with the TNT maybe 1000 were produced,, and then the decoder 
moved to the offset of the TIP for the next instruction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136557

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

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[Lu Weining via Phabricator via lldb-commits]

SixWeining added a comment.

Build is OK on my local LoongArch machine with 229 failed tests and 2 timed out 
tests.


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

https://reviews.llvm.org/D136578

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

[Lldb-commits] [PATCH] D136557: [trace][intel pt] Simple detection of infinite decoding loops

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 470343.
wallace added a comment.

address issues and comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136557

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTProperties.td
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  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
@@ -37,6 +37,12 @@
   "totalCount": 0,
   "individualCounts": {}
 },
+"errors": {
+  "totalCount": 0,
+  "libiptErrors": {},
+  "fatalErrors": 0,
+  "otherErrors": 0
+},
 "continuousExecutions": 0,
 "PSBBlocks": 0
   },
@@ -72,6 +78,12 @@
 "HW clock tick": 8
   }
 },
+"errors": {
+  "totalCount": 1,
+  "libiptErrors": {},
+  "fatalErrors": 0,
+  "otherErrors": 1
+},
 "continuousExecutions": 1,
 "PSBBlocks": 1
   },
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -78,6 +78,12 @@
 "software disabled tracing": 2,
 "trace synchronization point": 1
   }
+},
+"errors": {
+  "totalCount": 0,
+  "libiptErrors": {},
+  "fatalErrors": 0,
+  "otherErrors": 0
 }
   },
   "globalStats": {
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTProperties.td
===
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTProperties.td
@@ -0,0 +1,24 @@
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "traceintelpt" in {
+  def InfiniteDecodingLoopVerificationThreshold:
+  Property<"infinite-decoding-loop-verification-threshold", "UInt64">,
+Global,
+DefaultUnsignedValue<1>,
+Desc<"Specify how many instructions following an individual Intel PT "
+  "packet must have been decoded before triggering the verification of "
+  "infinite decoding loops. If no decoding loop has been found after this "
+  "threshold T, another attempt will be done after 2T instructions, then "
+  "4T, 8T and so on, which guarantees a total linear time spent checking "
+  "this anomaly. If a loop is found, then decoding of the corresponding "
+  "PSB block is stopped. An error is hence emitted in the trace and "
+  "decoding is resumed in the next PSB block.">;
+  def ExtremelyLargeDecodingThreshold:
+  Property<"extremely-large-decoding-threshold", "UInt64">,
+Global,
+DefaultUnsignedValue<50>,
+Desc<"Specify how many instructions following an individual Intel PT "
+  "packet must have been decoded before stopping the decoding of the "
+  "corresponding PSB block. An error is hence emitted in the trace and "
+  "decoding is resumed in the next PSB block.">;
+}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -22,6 +22,23 @@
 
 class TraceIntelPT : public Trace {
 public:
+  /// Properties to be used with the `settings` command.
+  class PluginProperties : public Properties {
+  public:
+static ConstString GetSettingName();
+
+PluginProperties();
+
+~PluginProperties() override = default;
+
+uint64_t GetInfiniteDecodingLoopVerificationThreshold();
+
+uint64_t GetExtremelyLargeDecodingThreshold();
+  };
+
+  /// Return the global properties for this trace plug-in.
+  static PluginProperties &GetGlobalProperties();
+
   void Dump(Stream *s) const override;
 
   llvm::Expected SaveToDisk(FileSpec directory,
@@ -59,6 +76,8 @@
   CreateInstanceForLiveProcess(Process &process);
 
   static llvm::StringRef GetPluginNameStatic() { return "intel-pt"; }
+
+  static void DebuggerInitialize(Debugger &debugger);
   /// \}
 
   lldb::CommandObjectSP
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -16,6 +16,7 @@
 #include "TraceIntelPTBundleSaver.h"
 #include "T

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

[Tiezhu Yang via Phabricator via lldb-commits]

seehearfeel updated this revision to Diff 470345.
seehearfeel edited the summary of this revision.
seehearfeel set the repository for this revision to rG LLVM Github Monorepo.
seehearfeel added a comment.

Remove GPRegSet and FPRegSet definitions which are not used in this commit.
Use git clang-format to format the patch properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136578

Files:
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.h
@@ -0,0 +1,64 @@
+//===-- RegisterInfoPOSIX_loongarch64.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_REGISTERINFOPOSIX_LOONGARCH64_H
+#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERINFOPOSIX_LOONGARCH64_H
+
+#include "RegisterInfoAndSetInterface.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/lldb-private.h"
+#include 
+
+class RegisterInfoPOSIX_loongarch64
+: public lldb_private::RegisterInfoAndSetInterface {
+public:
+  static const lldb_private::RegisterInfo *
+  GetRegisterInfoPtr(const lldb_private::ArchSpec &target_arch);
+  static uint32_t
+  GetRegisterInfoCount(const lldb_private::ArchSpec &target_arch);
+
+public:
+  struct GPR {
+uint64_t gpr[32];
+
+uint64_t orig_a0;
+uint64_t csr_era;
+uint64_t csr_badv;
+uint64_t reserved[10];
+  };
+
+  struct FPR {
+uint64_t fpr[32];
+uint64_t fcc;
+uint32_t fcsr;
+  };
+
+  RegisterInfoPOSIX_loongarch64(const lldb_private::ArchSpec &target_arch,
+lldb_private::Flags flags);
+
+  size_t GetGPRSize() const override;
+
+  size_t GetFPRSize() const override;
+
+  const lldb_private::RegisterInfo *GetRegisterInfo() const override;
+
+  uint32_t GetRegisterCount() const override;
+
+  const lldb_private::RegisterSet *
+  GetRegisterSet(size_t reg_set) const override;
+
+  size_t GetRegisterSetCount() const override;
+
+  size_t GetRegisterSetFromRegisterIndex(uint32_t reg_index) const override;
+
+private:
+  const lldb_private::RegisterInfo *m_register_info_p;
+  uint32_t m_register_info_count;
+};
+
+#endif
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.cpp
@@ -0,0 +1,68 @@
+//===-- RegisterInfoPOSIX_loongarch64.cpp ===//
+//
+// 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
+//
+//===-===//
+
+#include 
+#include 
+#include 
+
+#include "lldb/lldb-defines.h"
+#include "llvm/Support/Compiler.h"
+
+#include "RegisterInfoPOSIX_loongarch64.h"
+
+const lldb_private::RegisterInfo *
+RegisterInfoPOSIX_loongarch64::GetRegisterInfoPtr(
+const lldb_private::ArchSpec &target_arch) {
+  switch (target_arch.GetMachine()) {
+  default:
+assert(false && "Unhandled target architecture.");
+return nullptr;
+  }
+}
+
+uint32_t RegisterInfoPOSIX_loongarch64::GetRegisterInfoCount(
+const lldb_private::ArchSpec &target_arch) {
+  switch (target_arch.GetMachine()) {
+  default:
+assert(false && "Unhandled target architecture.");
+return 0;
+  }
+}
+
+RegisterInfoPOSIX_loongarch64::RegisterInfoPOSIX_loongarch64(
+const lldb_private::ArchSpec &target_arch, lldb_private::Flags flags)
+: lldb_private::RegisterInfoAndSetInterface(target_arch),
+  m_register_info_p(GetRegisterInfoPtr(target_arch)),
+  m_register_info_count(GetRegisterInfoCount(target_arch)) {}
+
+uint32_t RegisterInfoPOSIX_loongarch64::GetRegisterCount() const { return 0; }
+
+size_t RegisterInfoPOSIX_loongarch64::GetGPRSize() const {
+  return sizeof(struct RegisterInfoPOSIX_loongarch64::GPR);
+}
+
+size_t RegisterInfoPOSIX_loongarch64::GetFPRSize() const {
+  return sizeof(struct RegisterInfoPOSIX_loongarch64::FPR);
+}
+
+const lldb_private::

[Lldb-commits] [PATCH] D134604: [clang] Implement sugared substitution changes to infrastructure

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov updated this revision to Diff 470348.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134604

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/TemplateName.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/TemplateName.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Sema/TypeLocBuilder.cpp
  clang/lib/Sema/TypeLocBuilder.h
  clang/test/SemaTemplate/nested-name-spec-template.cpp

Index: clang/test/SemaTemplate/nested-name-spec-template.cpp
===
--- clang/test/SemaTemplate/nested-name-spec-template.cpp
+++ clang/test/SemaTemplate/nested-name-spec-template.cpp
@@ -147,3 +147,10 @@
 
   template void f(); // expected-note{{in instantiation of}}
 }
+
+namespace sugared_template_instantiation {
+  // Test that we ignore local qualifiers.
+  template  struct A {};
+  struct B { typedef int type1; };
+  typedef A type2;
+} // namespace sugated_template_instantiation
Index: clang/lib/Sema/TypeLocBuilder.h
===
--- clang/lib/Sema/TypeLocBuilder.h
+++ clang/lib/Sema/TypeLocBuilder.h
@@ -64,6 +64,10 @@
   /// must be empty for this to work.
   void pushFullCopy(TypeLoc L);
 
+  /// Pushes 'T' with all locations pointing to 'Loc'.
+  /// The builder must be empty for this to work.
+  void pushTrivial(ASTContext &Context, QualType T, SourceLocation Loc);
+
   /// Pushes space for a typespec TypeLoc.  Invalidates any TypeLocs
   /// previously retrieved from this builder.
   TypeSpecTypeLoc pushTypeSpec(QualType T) {
Index: clang/lib/Sema/TypeLocBuilder.cpp
===
--- clang/lib/Sema/TypeLocBuilder.cpp
+++ clang/lib/Sema/TypeLocBuilder.cpp
@@ -41,6 +41,29 @@
   }
 }
 
+void TypeLocBuilder::pushTrivial(ASTContext &Context, QualType T,
+ SourceLocation Loc) {
+  auto L = TypeLoc(T, nullptr);
+  reserve(L.getFullDataSize());
+
+  SmallVector TypeLocs;
+  for (auto CurTL = L; CurTL; CurTL = CurTL.getNextTypeLoc())
+TypeLocs.push_back(CurTL);
+
+  for (const auto &CurTL : llvm::reverse(TypeLocs)) {
+switch (CurTL.getTypeLocClass()) {
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT) \
+  case TypeLoc::CLASS: {   \
+auto NewTL = push(CurTL.getType());  \
+NewTL.initializeLocal(Context, Loc);   \
+break; \
+  }
+#include "clang/AST/TypeLocNodes.def"
+}
+  }
+}
+
 void TypeLocBuilder::grow(size_t NewCapacity) {
   assert(NewCapacity > Capacity);
 
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -636,6 +636,14 @@
   QualType Transform##CLASS##Type(TypeLocBuilder &TLB, CLASS##TypeLoc T);
 #include "clang/AST/TypeLocNodes.def"
 
+  QualType TransformTemplateTypeParmType(TypeLocBuilder &TLB,
+ TemplateTypeParmTypeLoc TL,
+ bool SuppressObjCLifetime);
+  QualType
+  TransformSubstTemplateTypeParmPackType(TypeLocBuilder &TLB,
+ SubstTemplateTypeParmPackTypeLoc TL,
+ bool SuppressObjCLifetime);
+
   template
   QualType TransformFunctionProtoType(TypeLocBuilder &TLB,
   FunctionProtoTypeLoc TL,
@@ -1285,9 +1293,10 @@
   /// template name. Subclasses may override this routine to provide different
   /// behavior.
   TemplateName RebuildTemplateName(const TemplateArgument &ArgPack,
-   Decl *AssociatedDecl, unsigned Index) {
+   Decl *AssociatedDecl, unsigned Index,
+   bool Final) {
 return getSema().Context.getSubstTemplateTemplateParmPack(
-ArgPack, AssociatedDecl, Index);
+ArgPack, AssociatedDecl, Index, Final);
   }
 
   /// Build a new compound statement.
@@ -4152,9 +4161,13 @@
 NestedNameSpecifierLoc NNS, QualType ObjectType,
 NamedDecl *FirstQualifierInScope) {
   SmallVector Qualifiers;
- 

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[Luka Markušić via Phabricator via lldb-commits]

mark2185 added a comment.

In D136465#3880809 , @clayborg wrote:

> Is there any way to test this?

Through the test suite? I couldn't find //any// `android platform` related 
tests, and this would require `adb` to be running in the background, so I'm not 
sure how to approach it.

As for manually testing, three shells are needed (one for `lldb-server`, one 
for `lldb`, and one to check the ports).

  (shell-1) $> adb push /path/to/correct/cpu/arch/lldb-server /data/local/tmp   
# (android studio comes with prebuilt servers)
  (shell-1) $> adb shell /data/local/tmp/lldb-server platform --listen "*:"



  (shell-2) $> ANDROID_PLATFORM_LOCAL_PORT= lldb
  (lldb) $> platform select remote-android
  (lldb) $> platform connect connect://localhost:



  (shell-3) $> adb forward --list
   tcp: tcp:

If one were to omit the `ANDROID_PLATFORM_LOCAL_PORT` variable, the forwarded 
port will be random.

As for testing the `ANDROID_PLATFORM_LOCAL_GDB_PORT`, one would need to:

- install an app on the device
- run the `lldb-server` as that app ( `adb shell run-as com.full.package.App 
./lldb-server platform...` ) so it can attach to its `PID`
- after `platform connect` run `attach `
- `adb forward --list` will now show two forwarded ports
  - the `ANDROID_PLATFORM_LOCAL_PORT` from before
  - the  `ANDROID_PLATFORM_LOCAL_GDB_PORT`, forwarded to some port on the 
device (that destination port is also random, unless `lldb-server` is started 
with the `--gdbserver-port` argument, or `--min-gdbserver-port` and 
`--max-gdbserver-port` that limit its possible range)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[Luka Markušić via Phabricator via lldb-commits]

mark2185 updated this revision to Diff 470381.
mark2185 added a comment.

Remove {} for single line if statements per LLVM coding guidelines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

Files:
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp


Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -92,9 +92,8 @@
 
   uint16_t local_port = 0;
   const char *gdbstub_port = std::getenv("ANDROID_PLATFORM_LOCAL_GDB_PORT");
-  if (gdbstub_port) {
+  if (gdbstub_port)
 local_port = std::stoi(gdbstub_port);
-  }
 
   auto error = MakeConnectURL(pid, local_port, remote_port, 
socket_name.c_str(),
   connect_url);
@@ -134,9 +133,8 @@
 
   uint16_t local_port = 0;
   const char *platform_local_port = std::getenv("ANDROID_PLATFORM_LOCAL_PORT");
-  if (platform_local_port) {
+  if (platform_local_port)
 local_port = std::stoi(platform_local_port);
-  }
 
   std::string connect_url;
   auto error = MakeConnectURL(g_remote_platform_pid, local_port,
@@ -201,9 +199,8 @@
 return error;
   };
 
-  if (local_port != 0) {
+  if (local_port != 0)
 return forward(local_port, remote_port);
-  }
 
   // There is a race possibility that somebody will occupy a port while we're
   // in between FindUnusedPort and ForwardPortWithAdb - adding the loop to
@@ -214,9 +211,8 @@
 if (error.Fail())
   return error;
 
-if (forward(local_port, remote_port).Success()) {
+if (forward(local_port, remote_port).Success())
   break;
-}
   }
 
   return error;


Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -92,9 +92,8 @@
 
   uint16_t local_port = 0;
   const char *gdbstub_port = std::getenv("ANDROID_PLATFORM_LOCAL_GDB_PORT");
-  if (gdbstub_port) {
+  if (gdbstub_port)
 local_port = std::stoi(gdbstub_port);
-  }
 
   auto error = MakeConnectURL(pid, local_port, remote_port, socket_name.c_str(),
   connect_url);
@@ -134,9 +133,8 @@
 
   uint16_t local_port = 0;
   const char *platform_local_port = std::getenv("ANDROID_PLATFORM_LOCAL_PORT");
-  if (platform_local_port) {
+  if (platform_local_port)
 local_port = std::stoi(platform_local_port);
-  }
 
   std::string connect_url;
   auto error = MakeConnectURL(g_remote_platform_pid, local_port,
@@ -201,9 +199,8 @@
 return error;
   };
 
-  if (local_port != 0) {
+  if (local_port != 0)
 return forward(local_port, remote_port);
-  }
 
   // There is a race possibility that somebody will occupy a port while we're
   // in between FindUnusedPort and ForwardPortWithAdb - adding the loop to
@@ -214,9 +211,8 @@
 if (error.Fail())
   return error;
 
-if (forward(local_port, remote_port).Success()) {
+if (forward(local_port, remote_port).Success())
   break;
-}
   }
 
   return error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[Luka Markušić via Phabricator via lldb-commits]

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

Did I just overwrite the initial commit with a new one, instead of just 
creating a diff based on the two comments?

I'm terribly sorry, should I just squash my two commits and run `arc diff 
--update=D136465` again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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