[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu added a comment.

In D156086#4536992 , @jasonmolenda 
wrote:

> In D156086#4530507 , @RamNalamothu 
> wrote:
>
>> In D156086#4529791 , @jasonmolenda 
>> wrote:
>>
>>> 
>
>
>
>>> Does `isBranch` include other variants like `isUnconditionalBranch`?
>>
>> No. They are implemented as separate methods. You can see that with a full 
>> context diff of MCInstrAnalysis.h changes in this revision or 
>> MCInstrAnalysis.h 
>> 
>
> `mayAffectControlFlow` doesn't test for `isUnconditionalBranch`.  Is that a 
> problem?   I didn't look through the different property check methods like 
> this, but I happened to notice this one and see it wasn't detected in 
> `mayAffectControlFlow`.  Maybe I misunderstood something.

The idea is MCInstrAnalysis's default implementation just replicates what 
MCInstrDesc does (MCInstrDesc::mayAffectControlFlow 
)
 and the individual targets can refine those methods as needed.

In D156086#4537284 , @MaskRay wrote:

> It seems that a lldb specific test is needed. Adding a new method to 
> `llvm/include/llvm/MC/MCInstrAnalysis.h` is fine with me, though I haven't 
> checked the semantics.

I will try to add a lldb specific test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156086

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

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

It seems that a lldb specific test is needed. Adding a new method to 
`llvm/include/llvm/MC/MCInstrAnalysis.h` is fine with me, though I haven't 
checked the semantics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156086

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

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D155905#4537036 , @clayborg wrote:

> 



> I like it the above approach with more enums for the high and low code/data. 
> Not sure if eTypeAny makes sense in the GetAddressMask(eTypeAny) scenario, 
> but I can see the use for a eTypeAll in case you wanted to set all of the 
> various address masks to zero though using SetAddressMask(eTypeAll, mask). We 
> would need to document this in the enum header file if we do add a eTypeAny 
> or eTypeAll.

Yeah I agree SetAddressMask(eTypeAll) and GetAddressMask(eTypeAny) would be the 
clearest names, that was my first thought too.  Maybe adding two enum names for 
the same value.  And I'm a little worried about encouraging script writers to 
assume there is one mask active -- all of the targets I work on today have the 
same mask for code and data, but I could imagine some harvard architecture 
target that behaved differently (surely this is why Linux has two address 
masks), and scripts wouldn't work correctly then.  But let's be honest, even if 
it's not easy they're probably going to pick one mask anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

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

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

[Jason Molenda via Phabricator via lldb-commits]

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

Looking at Ted's earlier riscv disassembly with this enabled, it is more 
readable, I'm surprised these instructions don't print this way by default like 
they are for other targets.

My two cents, I'm fine with landing this, and if we find that another target's 
disassembly is poorer, we can look into that more closely.  I had a quick 
attempt at aarch64 that would disassemble differently but didn't succeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155107

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

[Lldb-commits] [PATCH] D156375: Fix lldb-vscode frame id integer overflow

[jeffrey tan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGca849352936d: Fix lldb-vscode frame id integer overflow 
(authored by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156375

Files:
  lldb/tools/lldb-vscode/LLDBUtils.cpp


Index: lldb/tools/lldb-vscode/LLDBUtils.cpp
===
--- lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -79,8 +79,8 @@
 }
 
 int64_t MakeVSCodeFrameID(lldb::SBFrame ) {
-  return (int64_t)(frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT |
-   frame.GetFrameID());
+  return ((int64_t)frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT) |
+ frame.GetFrameID();
 }
 
 } // namespace lldb_vscode


Index: lldb/tools/lldb-vscode/LLDBUtils.cpp
===
--- lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -79,8 +79,8 @@
 }
 
 int64_t MakeVSCodeFrameID(lldb::SBFrame ) {
-  return (int64_t)(frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT |
-   frame.GetFrameID());
+  return ((int64_t)frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT) |
+ frame.GetFrameID();
 }
 
 } // namespace lldb_vscode
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ca84935 - Fix lldb-vscode frame id integer overflow

[Jeffrey Tan via lldb-commits]

Author: Jeffrey Tan
Date: 2023-07-26T16:12:41-07:00
New Revision: ca849352936dadadd232cf9ec74ac006ce410f51

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

LOG: Fix lldb-vscode frame id integer overflow

This patch fixes a 32bit integer overflow in lldb-vscode.
The current implementation of frame_id does `(thread_index << 19 | 
frame_index)`. Since thread_index is a 32 bit integer this leaves only 32 - 19 
== 13 bits available for the thread_index. As a result, lldb-vscode can only 
handle 2^13 == 8192 threads. Normally, this would be sufficient, but we have 
seen crazy process having +12000 threads, causing the frame_id algorithm above 
to integer overflow during casting.

The patch fixes the overflow by up casting to 64 bit integer first before bit 
shifiting.

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

Added: 


Modified: 
lldb/tools/lldb-vscode/LLDBUtils.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/LLDBUtils.cpp 
b/lldb/tools/lldb-vscode/LLDBUtils.cpp
index 621f4ec37c83da..464195bdc6444c 100644
--- a/lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ b/lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -79,8 +79,8 @@ uint32_t GetLLDBFrameID(uint64_t dap_frame_id) {
 }
 
 int64_t MakeVSCodeFrameID(lldb::SBFrame ) {
-  return (int64_t)(frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT |
-   frame.GetFrameID());
+  return ((int64_t)frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT) |
+ frame.GetFrameID();
 }
 
 } // namespace lldb_vscode



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

[Lldb-commits] [PATCH] D156375: Fix lldb-vscode frame id integer overflow

[Greg Clayton via Phabricator via lldb-commits]

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

We ran into this with a process that had many threads. No easy way to test this 
without creating a process with 8K threads.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156375

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

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D155905#4536917 , @jasonmolenda 
wrote:

> Also interesting to consider if there should be an "Any" define.  e.g.
>
>   enum AddressMaskType {
> eTypeCode = 0,
> eTypeData,
> eTypeHighmemCode,
> eTypeHighmemData,
> eTypeAny
>   };
>   lldb::addr_t GetAddressMask(AddressMaskType mask_type);
>   void SetAddressMask(AddressMaskType mask_type, lldb::addr_t mask);
>
> The patch also adds SBProcess::FixCodeAddress, SBProcess::FixDataAddress, and 
> SBProcess::FixAddress -- FixAddress is always calling FixDataAddress right 
> now, because Data can be at any address on all the targets we support today, 
> whereas CodeAddress may have low bits masked off (e.g. armv7 where the low 
> bit is sometimes used to indicate arm/thub, but it could be for AArch64 as 
> well).  So `GetAddressMask(eTypeAny)` would probably return the data mask, 
> and `SetAddressMask(eTypeAny, mask)` would set all the address masks to the 
> same value.

I like it the above approach with more enums for the high and low code/data. 
Not sure if eTypeAny makes sense in the GetAddressMask(eTypeAny) scenario, but 
I can see the use for a eTypeAll in case you wanted to set all of the various 
address masks to zero though using SetAddressMask(eTypeAll, mask). We would 
need to document this in the enum header file if we do add a eTypeAny or 
eTypeAll.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

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

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D156086#4530507 , @RamNalamothu 
wrote:

> In D156086#4529791 , @jasonmolenda 
> wrote:
>
>> 



>> Does `isBranch` include other variants like `isUnconditionalBranch`?
>
> No. They are implemented as separate methods. You can see that with a full 
> context diff of MCInstrAnalysis.h changes in this revision or 
> MCInstrAnalysis.h 
> 

`mayAffectControlFlow` doesn't test for `isUnconditionalBranch`.  Is that a 
problem?   I didn't look through the different property check methods like 
this, but I happened to notice this one and see it wasn't detected in 
`mayAffectControlFlow`.  Maybe I misunderstood something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156086

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

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Also interesting to consider if there should be an "Any" define.  e.g.

  enum AddressMaskType {
eTypeCode = 0,
eTypeData,
eTypeHighmemCode,
eTypeHighmemData,
eTypeAny
  };
  lldb::addr_t GetAddressMask(AddressMaskType mask_type);
  void SetAddressMask(AddressMaskType mask_type, lldb::addr_t mask);

The patch also adds SBProcess::FixCodeAddress, SBProcess::FixDataAddress, and 
SBProcess::FixAddress -- FixAddress is always calling FixDataAddress right now, 
because Data can be at any address on all the targets we support today, whereas 
CodeAddress may have low bits masked off (e.g. armv7 where the low bit is 
sometimes used to indicate arm/thub, but it could be for AArch64 as well).  So 
`GetAddressMask(eTypeAny)` would probably return the data mask, and 
`SetAddressMask(eTypeAny, mask)` would set all the address masks to the same 
value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

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

[Lldb-commits] [PATCH] D156375: Fix lldb-vscode frame id integer overflow

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere, 
kusmour, GeorgeHuyubo.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch fixes a 32bit integer overflow in lldb-vscode.
The current implementation of frame_id does `(thread_index << 19 | 
frame_index)`. Since thread_index is a 32 bit integer this leaves only 32 - 19 
== 13 bits available for the thread_index. As a result, lldb-vscode can only 
handle 2^13 == 8192 threads. Normally, this would be sufficient, but we have 
seen crazy process having +12000 threads, causing the frame_id algorithm above 
to integer overflow during casting.

The patch fixes the overflow by up casting to 64 bit integer first before bit 
shifiting.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156375

Files:
  lldb/tools/lldb-vscode/LLDBUtils.cpp


Index: lldb/tools/lldb-vscode/LLDBUtils.cpp
===
--- lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -79,8 +79,8 @@
 }
 
 int64_t MakeVSCodeFrameID(lldb::SBFrame ) {
-  return (int64_t)(frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT |
-   frame.GetFrameID());
+  return ((int64_t)frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT) |
+ frame.GetFrameID();
 }
 
 } // namespace lldb_vscode


Index: lldb/tools/lldb-vscode/LLDBUtils.cpp
===
--- lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -79,8 +79,8 @@
 }
 
 int64_t MakeVSCodeFrameID(lldb::SBFrame ) {
-  return (int64_t)(frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT |
-   frame.GetFrameID());
+  return ((int64_t)frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT) |
+ frame.GetFrameID();
 }
 
 } // namespace lldb_vscode
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/include/lldb/API/SBProcess.h:415-416
+  /// (unsigned long long) 0xfc00
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+

clayborg wrote:
> Will there only ever be two variants of this for code and data? If we think 
> there might be another address mask type later it might be a good idea to use 
> an enumeration for these two functions and all functions below? I was 
> thinking of something like:
> ```
> enum AddressMaskType {
>   eTypeCode = 0,
>   eTypeData
> };
> lldb::addr_t GetAddressMask(AddressMaskType mask_type);
> ```
> 
> This enum could be used in all subsequent calls. I am just thinking of any 
> needed extra API changes needed in the future. If we need another address 
> mask type in the future we just add another enum and all functions stay the 
> same if we do it as suggested above. Else we will need to add many more API 
> functions if we need a new type later.
That's a very good idea.  The HighCode/HighData can be rolled into the same 
list of enums.



Comment at: lldb/include/lldb/API/SBProcess.h:430-434
+  /// If the low ten bits are the only valid bits for addressing,
+  /// this would be the mask returned:
+  ///
+  /// (lldb) p/x ~((1ULL << 10) - 1)
+  /// (unsigned long long) 0xfc00

clayborg wrote:
> Is this comment needed for the set accessors? These functions don't return 
> anything
I wanted to include this concrete example on both the Get and Set methods 
because the Linux address masks are the opposite of what I would have expected 
if someone told me it was an address mask.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

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

[Lldb-commits] [PATCH] D154990: [lldb-vsocde] Cleaning up the usage of the Separate helper in Options.td.

[David Goldman via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG011cc6d8bfba: [lldb-vsocde] Cleaning up the usage of the 
Separate helper in Options.td. (authored by ashgti, committed by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154990

Files:
  lldb/tools/lldb-vscode/Options.td


Index: lldb/tools/lldb-vscode/Options.td
===
--- lldb/tools/lldb-vscode/Options.td
+++ lldb/tools/lldb-vscode/Options.td
@@ -17,25 +17,25 @@
   Alias,
   HelpText<"Alias for --wait-for-debugger">;
 
-def port: Separate<["--", "-"], "port">,
+def port: S<"port">,
   MetaVarName<"">,
   HelpText<"Communicate with the lldb-vscode tool over the defined port.">;
 def: Separate<["-"], "p">,
   Alias,
   HelpText<"Alias for --port">;
 
-def launch_target: Separate<["--", "-"], "launch-target">,
+def launch_target: S<"launch-target">,
   MetaVarName<"">,
   HelpText<"Launch a target for the launchInTerminal request. Any argument "
 "provided after this one will be passed to the target. The parameter "
 "--comm-file must also be specified.">;
 
-def comm_file: Separate<["--", "-"], "comm-file">,
+def comm_file: S<"comm-file">,
   MetaVarName<"">,
   HelpText<"The fifo file used to communicate the with the debug adaptor "
 "when using --launch-target.">;
 
-def debugger_pid: Separate<["--", "-"], "debugger-pid">,
+def debugger_pid: S<"debugger-pid">,
   MetaVarName<"">,
   HelpText<"The PID of the lldb-vscode instance that sent the launchInTerminal 
"
 "request when using --launch-target.">;


Index: lldb/tools/lldb-vscode/Options.td
===
--- lldb/tools/lldb-vscode/Options.td
+++ lldb/tools/lldb-vscode/Options.td
@@ -17,25 +17,25 @@
   Alias,
   HelpText<"Alias for --wait-for-debugger">;
 
-def port: Separate<["--", "-"], "port">,
+def port: S<"port">,
   MetaVarName<"">,
   HelpText<"Communicate with the lldb-vscode tool over the defined port.">;
 def: Separate<["-"], "p">,
   Alias,
   HelpText<"Alias for --port">;
 
-def launch_target: Separate<["--", "-"], "launch-target">,
+def launch_target: S<"launch-target">,
   MetaVarName<"">,
   HelpText<"Launch a target for the launchInTerminal request. Any argument "
 "provided after this one will be passed to the target. The parameter "
 "--comm-file must also be specified.">;
 
-def comm_file: Separate<["--", "-"], "comm-file">,
+def comm_file: S<"comm-file">,
   MetaVarName<"">,
   HelpText<"The fifo file used to communicate the with the debug adaptor "
 "when using --launch-target.">;
 
-def debugger_pid: Separate<["--", "-"], "debugger-pid">,
+def debugger_pid: S<"debugger-pid">,
   MetaVarName<"">,
   HelpText<"The PID of the lldb-vscode instance that sent the launchInTerminal "
 "request when using --launch-target.">;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 011cc6d - [lldb-vsocde] Cleaning up the usage of the Separate helper in Options.td.

[David Goldman via lldb-commits]

Author: John Harrison
Date: 2023-07-26T15:27:17-04:00
New Revision: 011cc6d8bfba44ae7e057854c4e7110ae0a4ba37

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

LOG: [lldb-vsocde] Cleaning up the usage of the Separate helper in Options.td.

Reviewed By: wallace

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

Added: 


Modified: 
lldb/tools/lldb-vscode/Options.td

Removed: 




diff  --git a/lldb/tools/lldb-vscode/Options.td 
b/lldb/tools/lldb-vscode/Options.td
index 8c51a994ebc270..a6ba0a318f388a 100644
--- a/lldb/tools/lldb-vscode/Options.td
+++ b/lldb/tools/lldb-vscode/Options.td
@@ -17,25 +17,25 @@ def: Flag<["-"], "g">,
   Alias,
   HelpText<"Alias for --wait-for-debugger">;
 
-def port: Separate<["--", "-"], "port">,
+def port: S<"port">,
   MetaVarName<"">,
   HelpText<"Communicate with the lldb-vscode tool over the defined port.">;
 def: Separate<["-"], "p">,
   Alias,
   HelpText<"Alias for --port">;
 
-def launch_target: Separate<["--", "-"], "launch-target">,
+def launch_target: S<"launch-target">,
   MetaVarName<"">,
   HelpText<"Launch a target for the launchInTerminal request. Any argument "
 "provided after this one will be passed to the target. The parameter "
 "--comm-file must also be specified.">;
 
-def comm_file: Separate<["--", "-"], "comm-file">,
+def comm_file: S<"comm-file">,
   MetaVarName<"">,
   HelpText<"The fifo file used to communicate the with the debug adaptor "
 "when using --launch-target.">;
 
-def debugger_pid: Separate<["--", "-"], "debugger-pid">,
+def debugger_pid: S<"debugger-pid">,
   MetaVarName<"">,
   HelpText<"The PID of the lldb-vscode instance that sent the launchInTerminal 
"
 "request when using --launch-target.">;



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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0b2b6bab4d2: [lldb][PlatformDarwin] Parse SDK path for 
module compilation from debug-info (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const _paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:

[Lldb-commits] [lldb] b0b2b6b - [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-07-26T18:26:08+01:00
New Revision: b0b2b6bab4d25122b2259dfac500fd83d60fa154

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

LOG: [lldb][PlatformDarwin] Parse SDK path for module compilation from 
debug-info

When we build the Clang module compilation command (e.g., when
a user requests import of a module via `expression @import Foundation`),
LLDB will try to determine which SDK directory to use as the `sysroot`.
However, it currently does so by simply enumerating the `SDKs` directory
and picking the last one that's appropriate for module compilation
(see `PlatformDarwin::GetSDKDirectoryForModules`). That means if we have
multiple platform SDKs installed (e.g., a public and internal one), we
may pick the wrong one by chance.

On Darwin platforms we emit the SDK path that a object
file was compiled against into DWARF (using `DW_AT_LLVM_sysroot`
and `DW_AT_APPLE_sdk`). For Swift debugging, we already parse the SDK
path from debug-info if we can.

This patch mimicks the Swift behaviour for non-Swift languages. I.e.,
if we can get the SDK path from debug-info, do so. Otherwise, fall back
to the old heuristic.

rdar://110407148

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

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 5fd666b8ce8b58..fb652d9f02e0ed 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -42,6 +42,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VersionTuple.h"
@@ -1095,8 +1096,21 @@ void 
PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
   }
 
   FileSpec sysroot_spec;
-  // Scope for mutex locker below
-  {
+
+  if (target) {
+if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
+  auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
+  if (path_or_err) {
+sysroot_spec = FileSpec(*path_or_err);
+  } else {
+LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host),
+   path_or_err.takeError(),
+   "Failed to resolve SDK path: {0}");
+  }
+}
+  }
+
+  if (!FileSystem::Instance().IsDirectory(sysroot_spec.GetPath())) {
 std::lock_guard guard(m_mutex);
 sysroot_spec = GetSDKDirectoryForModules(sdk_type);
   }
@@ -1335,3 +1349,53 @@ llvm::Triple::OSType PlatformDarwin::GetHostOSType() {
 #endif
 #endif // __APPLE__
 }
+
+llvm::Expected>
+PlatformDarwin::GetSDKPathFromDebugInfo(Module ) {
+  SymbolFile *sym_file = module.GetSymbolFile();
+  if (!sym_file)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+llvm::formatv("No symbol file available for module '{0}'",
+  module.GetFileSpec().GetFilename().AsCString("")));
+
+  bool found_public_sdk = false;
+  bool found_internal_sdk = false;
+  XcodeSDK merged_sdk;
+  for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) {
+if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) {
+  auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp);
+  bool is_internal_sdk = cu_sdk.IsAppleInternalSDK();
+  found_public_sdk |= !is_internal_sdk;
+  found_internal_sdk |= is_internal_sdk;
+
+  merged_sdk.Merge(cu_sdk);
+}
+  }
+
+  const bool found_mismatch = found_internal_sdk && found_public_sdk;
+
+  return std::pair{std::move(merged_sdk), found_mismatch};
+}
+
+llvm::Expected
+PlatformDarwin::ResolveSDKPathFromDebugInfo(Module ) {
+  auto sdk_or_err = GetSDKPathFromDebugInfo(module);
+  if (!sdk_or_err)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+llvm::formatv("Failed to parse SDK path from debug-info: {0}",
+  llvm::toString(sdk_or_err.takeError(;
+
+  auto [sdk, _] = std::move(*sdk_or_err);
+
+  auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
+  if (!path_or_err)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+llvm::formatv("Error while searching for SDK (XcodeSDK '{0}'): {1}",
+  sdk.GetString(),
+  llvm::toString(path_or_err.takeError(;
+
+  return path_or_err->str();
+}

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h 

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1363
+  XcodeSDK sdk;
+  for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i)
+if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i))

Michael137 wrote:
> Only remaining question is whether we want to limit this to just Objective-C 
> and Swift. Going through each compile unit for C++ seems like a lot of work 
> for something that we won't use
> 
> @aprantl 
The work to detect the language and to get the SDK attribute is almost the 
same, both are stored in the top-level CU DIE.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

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

- EXPECT -> ASSERT


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const _paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

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

- Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const _paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  

[Lldb-commits] [PATCH] D156268: [lldb] Treat ARM64X images as ARM64.

[Jacek Caban via Phabricator via lldb-commits]

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

Thanks for review and testing. I created 
https://github.com/llvm/llvm-project/issues/64131 for the backport.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156268

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

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

- Parameterize tests
- Return bool to indicate SDK mismatch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const _paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- 

[Lldb-commits] [PATCH] D156268: [lldb] Treat ARM64X images as ARM64.

[Jacek Caban via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG48feef277a24: [lldb] Treat ARM64X images as ARM64. (authored 
by jacek).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156268

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -24,6 +24,7 @@
 MachineArm = 0x1c0,
 MachineArmNt = 0x1c4,
 MachineArm64 = 0xaa64,
+MachineArm64X = 0xa64e,
 MachineEbc = 0xebc,
 MachineX86 = 0x14c,
 MachineIA64 = 0x200,
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -343,6 +343,7 @@
 specs.Append(module_spec);
 break;
   case MachineArm64:
+  case MachineArm64X:
 spec.SetTriple("aarch64-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -24,6 +24,7 @@
 MachineArm = 0x1c0,
 MachineArmNt = 0x1c4,
 MachineArm64 = 0xaa64,
+MachineArm64X = 0xa64e,
 MachineEbc = 0xebc,
 MachineX86 = 0x14c,
 MachineIA64 = 0x200,
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -343,6 +343,7 @@
 specs.Append(module_spec);
 break;
   case MachineArm64:
+  case MachineArm64X:
 spec.SetTriple("aarch64-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 48feef2 - [lldb] Treat ARM64X images as ARM64.

[Jacek Caban via lldb-commits]

Author: Jacek Caban
Date: 2023-07-26T14:45:48+02:00
New Revision: 48feef277a24b1b9c0ff33267a91e70d9584012e

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

LOG: [lldb] Treat ARM64X images as ARM64.

With D149091, ARM64X binaries are no longer reported as ARM64. This broke
lldb tests as Windows 11 system DLLs are mostly ARM64X binaries and lldb
doesn't know how to handle them. Ideally lldb would understand a bit more
about ARM64X and handle them as AMD64 in x64 processes, but this is
enough to preserve previous behavior and fix tests.

Reviewed By: mstorsjo
Differential Revision: https://reviews.llvm.org/D156268

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 30ff0e5f3ef6a7..77506b40ff5c36 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -343,6 +343,7 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
 specs.Append(module_spec);
 break;
   case MachineArm64:
+  case MachineArm64X:
 spec.SetTriple("aarch64-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index 3da3a752a15733..5d0465b4ee44f6 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -24,6 +24,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
 MachineArm = 0x1c0,
 MachineArmNt = 0x1c4,
 MachineArm64 = 0xaa64,
+MachineArm64X = 0xa64e,
 MachineEbc = 0xebc,
 MachineX86 = 0x14c,
 MachineIA64 = 0x200,



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

[Lldb-commits] [PATCH] D156268: [lldb] Treat ARM64X images as ARM64.

[antoine moynault via Phabricator via lldb-commits]

antmo added a comment.

I confirm that the 7 failed tests are now Ok


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

https://reviews.llvm.org/D156268

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

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGefa43d785ee6: [lldb][AArch64] Add the tpidr2 TLS register 
that comes with SME (authored by DavidSpickett).

Changed prior to commit:
  https://reviews.llvm.org/D154930?vs=542932=544269#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c
  lldb/test/API/linux/aarch64/tls_registers/Makefile
  lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
  lldb/test/API/linux/aarch64/tls_registers/main.c

Index: lldb/test/API/linux/aarch64/tls_registers/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -0,0 +1,60 @@
+#include 
+#include 
+
+uint64_t get_tpidr(void) {
+  uint64_t tpidr = 0;
+  __asm__ volatile("mrs %0, tpidr_el0" : "=r"(tpidr));
+  return tpidr;
+}
+
+uint64_t get_tpidr2(void) {
+  uint64_t tpidr2 = 0;
+  // S3_3_C13_C0_5 means tpidr2, and will work with older tools.
+  __asm__ volatile("mrs %0, S3_3_C13_C0_5" : "=r"(tpidr2));
+  return tpidr2;
+}
+
+void set_tpidr(uint64_t value) {
+  __asm__ volatile("msr tpidr_el0, %0" ::"r"(value));
+}
+
+void set_tpidr2(uint64_t value) {
+  __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
+}
+
+int main(int argc, char *argv[]) {
+  bool use_tpidr2 = argc > 1;
+
+  uint64_t original_tpidr = get_tpidr();
+  // Accessing this on a core without it produces SIGILL. Only do this if
+  // requested.
+  uint64_t original_tpidr2 = 0;
+  if (use_tpidr2)
+original_tpidr2 = get_tpidr2();
+
+  uint64_t tpidr_pattern = 0x1122334455667788;
+  set_tpidr(tpidr_pattern);
+
+  uint64_t tpidr2_pattern = 0x8877665544332211;
+  if (use_tpidr2)
+set_tpidr2(tpidr2_pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern(s) for us to find.
+
+  uint64_t new_tpidr = get_tpidr();
+  volatile bool tpidr_was_set = new_tpidr == 0x;
+
+  uint64_t new_tpidr2 = 0;
+  volatile bool tpidr2_was_set = false;
+  if (use_tpidr2) {
+new_tpidr2 = get_tpidr2();
+tpidr2_was_set = new_tpidr2 == 0x;
+  }
+
+  set_tpidr(original_tpidr);
+  if (use_tpidr2)
+set_tpidr2(original_tpidr2);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -0,0 +1,115 @@
+"""
+Test lldb's ability to read and write the AArch64 TLS registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTLSRegisters(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup(self, registers):
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point at this line."),
+num_expected_locations=1,
+)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point 2 at this line."),
+num_expected_locations=1,
+)
+
+if "tpidr2" in registers:
+self.runCmd("settings set target.run-args 1")
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+def check_tls_reg(self, registers):
+self.setup(registers)
+
+regs = self.thread().GetSelectedFrame().GetRegisters()
+tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
+self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
+
+# Since we can't predict what the value will be, the program has set
+# a target value for us to find.
+initial_values = {
+"tpidr": 0x1122334455667788,
+"tpidr2": 0x8877665544332211,
+}
+
+for register in registers:
+  

[Lldb-commits] [lldb] efa43d7 - [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-07-26T10:34:13+01:00
New Revision: efa43d785ee600ef4cc14589e4777264f0613ec9

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

LOG: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

This changes the TLS regset to not only be dynamic in that it could
exist or not (though it always does) but also of a dynamic size.

If SME is present then the regset is 16 bytes and contains both tpidr
and tpidr2.

Testing is the same as tpidr. Write from assembly, read from lldb and
vice versa since we have no way to predict what its value should be
by just running a program.

Reviewed By: omjavaid

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

Added: 
lldb/test/API/linux/aarch64/tls_registers/Makefile
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
lldb/test/API/linux/aarch64/tls_registers/main.c

Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h

Removed: 
lldb/test/API/linux/aarch64/tls_register/Makefile
lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
lldb/test/API/linux/aarch64/tls_register/main.c



diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index ba6ec995dc2201..ae9fe7039a9204 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -132,9 +132,9 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   ::memset(_hbp_regs, 0, sizeof(m_hbp_regs));
   ::memset(_sve_header, 0, sizeof(m_sve_header));
   ::memset(_pac_mask, 0, sizeof(m_pac_mask));
+  ::memset(_tls_regs, 0, sizeof(m_tls_regs));
 
   m_mte_ctrl_reg = 0;
-  m_tls_tpidr_reg = 0;
 
   // 16 is just a maximum value, query hardware for actual watchpoint count
   m_max_hwp_supported = 16;
@@ -148,7 +148,11 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_sve_header_is_valid = false;
   m_pac_mask_is_valid = false;
   m_mte_ctrl_is_valid = false;
-  m_tls_tpidr_is_valid = false;
+  m_tls_is_valid = false;
+
+  // SME adds the tpidr2 register
+  m_tls_size = GetRegisterInfo().IsSSVEEnabled() ? sizeof(m_tls_regs)
+ : 
sizeof(m_tls_regs.tpidr_reg);
 
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled())
 m_sve_state = SVEState::Unknown;
@@ -255,13 +259,13 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
   src = (uint8_t *)GetSVEBuffer() + offset;
 }
   } else if (IsTLS(reg)) {
-error = ReadTLSTPIDR();
+error = ReadTLS();
 if (error.Fail())
   return error;
 
 offset = reg_info->byte_offset - GetRegisterInfo().GetTLSOffset();
-assert(offset < GetTLSTPIDRSize());
-src = (uint8_t *)GetTLSTPIDR() + offset;
+assert(offset < GetTLSBufferSize());
+src = (uint8_t *)GetTLSBuffer() + offset;
   } else if (IsSVE(reg)) {
 if (m_sve_state == SVEState::Disabled || m_sve_state == SVEState::Unknown)
   return Status("SVE disabled or not supported");
@@ -480,16 +484,16 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
 
 return WriteMTEControl();
   } else if (IsTLS(reg)) {
-error = ReadTLSTPIDR();
+error = ReadTLS();
 if (error.Fail())
   return error;
 
 offset = reg_info->byte_offset - GetRegisterInfo().GetTLSOffset();
-assert(offset < GetTLSTPIDRSize());
-dst = (uint8_t *)GetTLSTPIDR() + offset;
+assert(offset < GetTLSBufferSize());
+dst = (uint8_t *)GetTLSBuffer() + offset;
 ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
 
-return WriteTLSTPIDR();
+return WriteTLS();
   }
 
   return Status("Failed to write register value");
@@ -533,9 +537,9 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   return error;
   }
 
-  // tpidr is always present but there will be more in future.
-  reg_data_byte_size += GetTLSTPIDRSize();
-  error = ReadTLSTPIDR();
+  // tpidr is always present but tpidr2 depends on SME.
+  reg_data_byte_size += GetTLSBufferSize();
+  error = ReadTLS();
   if (error.Fail())
 return error;
 
@@ -558,7 +562,7 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   if (GetRegisterInfo().IsMTEEnabled())
 ::memcpy(dst, GetMTEControl(), GetMTEControlSize());
 
-  ::memcpy(dst, GetTLSTPIDR(), GetTLSTPIDRSize());
+  ::memcpy(dst, GetTLSBuffer(), 

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG03d8cd1d722d: [lldb][AArch64] Add support for SMEs SVE 
streaming mode registers (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -0,0 +1,108 @@
+#include 
+#include 
+
+void write_simd_regs() {
+#define WRITE_SIMD(NUM)\
+  asm volatile("MOV v" #NUM ".d[0], %0\n\t"\
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+
+  WRITE_SIMD(0);
+  WRITE_SIMD(1);
+  WRITE_SIMD(2);
+  WRITE_SIMD(3);
+  WRITE_SIMD(4);
+  WRITE_SIMD(5);
+  WRITE_SIMD(6);
+  WRITE_SIMD(7);
+  WRITE_SIMD(8);
+  WRITE_SIMD(9);
+  WRITE_SIMD(10);
+  WRITE_SIMD(11);
+  WRITE_SIMD(12);
+  WRITE_SIMD(13);
+  WRITE_SIMD(14);
+  WRITE_SIMD(15);
+  WRITE_SIMD(16);
+  WRITE_SIMD(17);
+  WRITE_SIMD(18);
+  WRITE_SIMD(19);
+  WRITE_SIMD(20);
+  WRITE_SIMD(21);
+  WRITE_SIMD(22);
+  WRITE_SIMD(23);
+  WRITE_SIMD(24);
+  WRITE_SIMD(25);
+  WRITE_SIMD(26);
+  WRITE_SIMD(27);
+  WRITE_SIMD(28);
+  WRITE_SIMD(29);
+  WRITE_SIMD(30);
+  WRITE_SIMD(31);
+}
+
+unsigned verify_simd_regs() {
+  uint64_t got_low = 0;
+  uint64_t got_high = 0;
+  uint64_t target = 0;
+
+#define VERIFY_SIMD(NUM)   \
+  do { \
+got_low = 0;   \
+got_high = 0;  \
+asm volatile("MOV %0, v" #NUM ".d[0]\n\t"  \
+ "MOV %1, v" #NUM ".d[1]\n\t"  \
+ : "=r"(got_low), "=r"(got_high)); \
+target = NUM + 1;  \
+if ((got_low != target) || (got_high != target))   \
+  return 1;\
+  } while (0)
+
+  VERIFY_SIMD(0);
+  VERIFY_SIMD(1);
+  VERIFY_SIMD(2);
+  VERIFY_SIMD(3);
+  VERIFY_SIMD(4);
+  VERIFY_SIMD(5);
+  VERIFY_SIMD(6);
+  VERIFY_SIMD(7);
+  VERIFY_SIMD(8);
+  VERIFY_SIMD(9);
+  VERIFY_SIMD(10);
+  VERIFY_SIMD(11);
+  VERIFY_SIMD(12);
+  VERIFY_SIMD(13);
+  VERIFY_SIMD(14);
+  VERIFY_SIMD(15);
+  VERIFY_SIMD(16);
+  VERIFY_SIMD(17);
+  VERIFY_SIMD(18);
+  VERIFY_SIMD(19);
+  VERIFY_SIMD(20);
+  VERIFY_SIMD(21);
+  VERIFY_SIMD(22);
+  VERIFY_SIMD(23);
+  VERIFY_SIMD(24);
+  VERIFY_SIMD(25);
+  VERIFY_SIMD(26);
+  VERIFY_SIMD(27);
+  VERIFY_SIMD(28);
+  VERIFY_SIMD(29);
+  VERIFY_SIMD(30);
+  VERIFY_SIMD(31);
+
+  return 0;
+}
+
+int main() {
+#ifdef SSVE
+  asm volatile("msr  s0_3_c4_c7_3, xzr" /*smstart*/);
+#elif defined SVE
+  // Make the non-streaming SVE registers active.
+  asm volatile("cpy  z0.b, p0/z, #1\n\t");
+#endif
+  // else test plain SIMD access.
+
+  write_simd_regs();
+
+  return verify_simd_regs(); // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
@@ -0,0 +1,108 @@
+"""
+Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,
+streaming SVE and normal SIMD modes.
+
+There are a few operating modes and we use different strategies for each:
+* Without SVE, in SIMD mode - read the SIMD regset.

[Lldb-commits] [lldb] 03d8cd1 - [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-07-26T09:36:50+01:00
New Revision: 03d8cd1d722daad86c45e242eb7d75d0a88bb00c

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

LOG: [lldb][AArch64] Add support for SME's SVE streaming mode registers

The Scalable Matrix Extension (SME) adds a new Scalable Vector mode
called "streaming SVE mode".

In this mode a lot of things change, but my understanding overall
is that this mode assumes you are not going to move data out of
the vector unit very often or read flags.

Based on "E1.3" of "Arm® Architecture Reference Manual Supplement,
The Scalable Matrix Extension (SME), for Armv9-A".

https://developer.arm.com/documentation/ddi0616/latest/

The important details for debug are that this adds another set
of SVE registers. This set is only active when we are in streaming
mode and is read from a new ptrace regset NT_ARM_SSVE.
We are able to read the header of either mode at all times but
only one will be active and contain register data.

For this reason, I have reused the existing SVE state. Streaming
mode is just another mode value attached to that state.

The streaming mode registers do not have different names in the
architecture, so I do not plan to allow users to read or write the
inactive mode's registers. "z0" will always mean "z0" of the active
mode.

Ptrace does allow reading inactive modes, but the data is of little
use. Writing to inactive modes will switch to that mode which would
not be what a debugger user would expect. So lldb will do neither.

Existing SVE tests have been updated to check streaming mode and
mode switches. However, we are limited in what we can check given
that state for the other mode is invalidated on mode switch.

The only way to know what mode you are in for testing purposes would
be to execute a streaming only, or non-streaming only instruction in
the opposite mode. However, the CPU feature smefa64 actually allows
all non-streaming mode instructions in streaming mode.

This is enabled by default in QEMU emulation and rather than mess
about trying to disable it I'm just going to use the pseduo streaming
control register added in a later patch to make these tests more
robust.

A new test has been added to check SIMD read/write from all the modes
as there is a subtlety there that needs noting, though lldb
doesn't have to make extra effort to do so.

If you are in streaming mode and write to v0, when you later exit
streaming mode that value may not be in the non-streaming state.
This can depend on how the core works but is a valid behaviour.

For example, say I am stopped here:
mov x0, v0.d[0]

And I want to update v0 in lldb. "register write v0 ..." should update
the v0 that this instruction is about to see. Not the potential other
copy of v0 in the non-streaming state (which is what I attempted in
earlier versions of this patch).

Not to mention, switching out of streaming mode here would be unexpected
and difficult to signal to the user.

Reviewed By: omjavaid

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

Added: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/Makefile

lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c

Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h

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

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index c9384a651e81b2..ba6ec995dc2201 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -36,6 +36,11 @@
 #define NT_ARM_SVE 0x405 /* ARM Scalable Vector Extension */
 #endif
 
+#ifndef NT_ARM_SSVE
+#define NT_ARM_SSVE
\
+  0x40b /* ARM Scalable Matrix Extension, Streaming SVE mode */
+#endif
+
 

[Lldb-commits] [PATCH] D156268: [lldb] Treat ARM64X images as ARM64.

[antoine moynault via Phabricator via lldb-commits]

antmo added a comment.

In D156268#4534366 , @mstorsjo wrote:

> I think this looks good. Did @antmo verify that it actually fixes the issue? 
> (It seems plausible, although there may be more similar cases hiding 
> somewhere?)

Not yet. Struggling a bit with the windows setup


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

https://reviews.llvm.org/D156268

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