[Lldb-commits] [PATCH] D154907: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate (2nd attempt)

2023-07-10 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.
Herald added subscribers: Michael137, JDevlieghere.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154907

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


[Lldb-commits] [PATCH] D154907: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate (2nd attempt)

2023-07-10 Thread Caroline Tice via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3885ceafa934: [LLDB] Fix buffer overflow problem in 
DWARFExpression::Evaluate (authored by cmtice).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154907

Files:
  lldb/source/Expression/DWARFExpression.cpp


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1069,6 +1069,13 @@
 return false;
   }
   uint8_t size = opcodes.GetU8(&offset);
+  if (size > 8) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "Invalid address size for DW_OP_deref_size: %d\n",
+  size);
+return false;
+  }
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1141,7 +1148,7 @@
   } else {
 if (error_ptr)
   error_ptr->SetErrorStringWithFormat(
-  "Failed to dereference pointer for for DW_OP_deref_size: "
+  "Failed to dereference pointer for DW_OP_deref_size: "
   "%s\n",
   error.AsCString());
 return false;


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1069,6 +1069,13 @@
 return false;
   }
   uint8_t size = opcodes.GetU8(&offset);
+  if (size > 8) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "Invalid address size for DW_OP_deref_size: %d\n",
+  size);
+return false;
+  }
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1141,7 +1148,7 @@
   } else {
 if (error_ptr)
   error_ptr->SetErrorStringWithFormat(
-  "Failed to dereference pointer for for DW_OP_deref_size: "
+  "Failed to dereference pointer for DW_OP_deref_size: "
   "%s\n",
   error.AsCString());
 return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 3885cea - [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate

2023-07-10 Thread Caroline Tice via lldb-commits

Author: Caroline Tice
Date: 2023-07-10T19:47:38-07:00
New Revision: 3885ceafa9347dd729eba5cf872091a689b63d98

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

LOG: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate

In two calls to ReadMemory in DWARFExpression.cpp, the buffer size
passed to ReadMemory is not checked and can be bigger than the actual
size of the buffer. This caused a buffer overflow bug, which we
found through Address Sanitizer. This patch fixes the problem by
checking the address size when it is first read out of the DWARF, and
setting an error and returning immediatley if the size is invalid.

This is the second attempt to fix this issue; I reverted the first one,
as it was not quite correct.

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

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 2e512bf7581e86..b829a6f7c86477 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1069,6 +1069,13 @@ bool DWARFExpression::Evaluate(
 return false;
   }
   uint8_t size = opcodes.GetU8(&offset);
+  if (size > 8) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "Invalid address size for DW_OP_deref_size: %d\n",
+  size);
+return false;
+  }
   Value::ValueType value_type = stack.back().GetValueType();
   switch (value_type) {
   case Value::ValueType::HostAddress: {
@@ -1141,7 +1148,7 @@ bool DWARFExpression::Evaluate(
   } else {
 if (error_ptr)
   error_ptr->SetErrorStringWithFormat(
-  "Failed to dereference pointer for for DW_OP_deref_size: "
+  "Failed to dereference pointer for DW_OP_deref_size: "
   "%s\n",
   error.AsCString());
 return false;



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


[Lldb-commits] [PATCH] D154030: [lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands.

2023-07-10 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

Could you split the changes to the selected thread out of this? I'm seeing two 
features being implemented in this patch.
Other than that, it looks pretty good!




Comment at: lldb/tools/lldb-vscode/Options.td:20-38
+def port: S<"port">,
   MetaVarName<"">,
   HelpText<"Communicate with the lldb-vscode tool over the defined port.">;
 def: Separate<["-"], "p">,
   Alias,
   HelpText<"Alias for --port">;
 

+1



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:768
 
+  return true; 
+}

use clang-format please



Comment at: lldb/tools/lldb-vscode/VSCode.h:89
+
+/// A huersitic for determining the context of an evaluation.
+enum class ExpressionContext {

typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154030

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


[Lldb-commits] [PATCH] D154759: [lldb][NFCI] Change type of UnwindPlan::m_source_name

2023-07-10 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

In D154759#4487381 , @jasonmolenda 
wrote:

> I guess simplest is we can have all the UnwindPlan creators have a `static 
> g_unwindplan_name` and save a pointer to that.  Each one is unique, don't 
> need anything fancier than that. There's the risk that someone will create a 
> new UnwindPlan and forget to make it a static string, so unwind logging would 
> try to print random memory, but it wouldn't break normal debugging.

I was thinking about this earlier, since we know what all the possible strings 
are going to be, we can create them all statically at compile time and just 
point to them. We can refactor a bit and then most of these source names just 
become `static const char *` or `static constexpr llvm::StringLiteral` or 
something like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154759

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


[Lldb-commits] [PATCH] D153735: [lldb][TargetGetModuleCallback] Implement Python interface

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added inline comments.



Comment at: lldb/include/lldb/API/SBDefines.h:129
 
+typedef SBError (*SBTargetGetModuleCallback)(SBDebugger debugger,
+ SBModuleSpec &module_spec,

clayborg wrote:
> If we are putting this into SBPlatform, this should probably be named 
> "SBPlatformGetModuleCallback". 
> 
> The name might be a bit more clear if it was 
> "SBPlatformLocateModuleCallback"? Open to suggestions or fine to lave this as 
> "SBPlatformGetModuleCallback" if everyone likes that.
As commented on D153734, I'll rename it to `SBPlatformLocateModuleCallback`.

```
typedef SBError (*SBPlatformLocateModuleCallback)(
SBDebugger debugger,
SBModuleSpec &module_spec,
SBFileSpec &module_file_spec,
SBFileSpec &symbol_file_spec);
```



Comment at: lldb/include/lldb/API/SBPlatform.h:181
+  /// nullptr or None is set.
+  SBError SetTargetGetModuleCallback(lldb::SBTargetGetModuleCallback callback,
+ void *callback_baton);

clayborg wrote:
> remove "Target" from the function name? Or rename to 
> "SetLocateModuleCallback(...)" if we end up renaming the callback type to 
> SBPlatformLocateModuleCallback
I'll rename it to `SetLocateModuleCallback`.
```
SBError SetLocateModuleCallback(lldb::SBPlatformLocateModuleCallback callback,
   void *callback_baton);
```



Comment at: lldb/source/API/SBPlatform.cpp:668
+// Platform. 'callback_baton' is the actual callback Python callable 
object.
+platform_sp->SetTargetGetModuleCallback(callback_baton);
+return SBError();

clayborg wrote:
> You are not passing the "callback" into Platform::SetTargetGetModuleCallback??
> 
> Are we missing changes to the file "lldb/include/lldb/Target/Platform.h"? 
will update it to support C callback function as well as Python.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153735

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


[Lldb-commits] [PATCH] D153734: [lldb][TargetGetModuleCallback] Call get module callback

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added a comment.

@clayborg 
Sure, I'll rename the type and function to `Locate` instead of `Get`.

  typedef SBError (*SBPlatformLocateModuleCallback)(
  SBDebugger debugger,
  SBModuleSpec &module_spec,
  SBFileSpec &module_file_spec,
  SBFileSpec &symbol_file_spec);

This `SBPlatformLocateModuleCallback` typedef is required for the SWIG type 
conversion.

However it will introduce the SB* dependencies to `Platform.h` if we use the 
type. Is it ok?

  void SetLocateModuleCallback(SBPlatformLocateModuleCallback callback, void 
*baton);

I think `Platform.h` should use `void *` instead of 
`SBPlatformLocateModuleCallback`. Is it correct?

  void SetLocateModuleCallback(void *callback, void *baton);




Comment at: lldb/source/Core/Debugger.cpp:2170-2181
+
+Status Debugger::CallTargetGetModuleCallback(
+void *target_get_module_callback_baton, const ModuleSpec &module_spec,
+FileSpec &module_file_spec, FileSpec &symbol_file_spec) {
+  ScriptInterpreter *script_interpreter = GetScriptInterpreter();
+  if (!script_interpreter)
+return Status("Cannot find ScriptInterpreter");

clayborg wrote:
> I don't think we need any of this in debugger. when someone, through python 
> calls the "SetTargetGetModuleCallback" that code should register a static 
> function as the callback to use that will end up calling into the script 
> interpreter.
> 
> We don't always want to call the script interpreter for all cases. Users 
> should be able to register a native callback for this, and it shouldn't force 
> us to always use the script interpreter.
> 
There was another reason why `Debugger` has this method. The reason was that 
`Debugger` is easily `gMock`-able but `Target` is not.
Please take a look at `MockDebugger` in 
`lldb/unittests/Target/GetModuleCallbackTest.cpp` in this diff.

I will update this diff to allow C function callback aside from Python 
function, but probably we need to keep this kind of method outside `Target` for 
`gMock` to write unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153734

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


[Lldb-commits] [PATCH] D154759: [lldb][NFCI] Change type of UnwindPlan::m_source_name

2023-07-10 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I guess simplest is we can have all the UnwindPlan creators have a `static 
g_unwindplan_name` and save a pointer to that.  Each one is unique, don't need 
anything fancier than that. There's the risk that someone will create a new 
UnwindPlan and forget to make it a static string, so unwind logging would try 
to print random memory, but it wouldn't break normal debugging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154759

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


[Lldb-commits] [PATCH] D154759: [lldb][NFCI] Change type of UnwindPlan::m_source_name

2023-07-10 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

probably the real correct change is to have constant c-strings in as globals 
and have pointers to those.  Then you're using the same space as a ConstString 
in each UnwindPlan (a pointer) but creation is cheaper.  There's no dynamic 
aspect to the names when the UnwindPlans are created, each method creating them 
knows what the name will be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154759

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


[Lldb-commits] [PATCH] D154759: [lldb][NFCI] Change type of UnwindPlan::m_source_name

2023-07-10 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

(to say it more clearly: I'm fine if you'd like to reduce the places where 
ConstStrings are being used, but I don't think there's any good reason to 
remove this use of them.  It will increase our memory footprint a bit.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154759

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


[Lldb-commits] [PATCH] D154759: [lldb][NFCI] Change type of UnwindPlan::m_source_name

2023-07-10 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

I'm fine with this.  It does increase the size of the UnwindPlan object, and 
there are a couple of them for every function that we see on a stack during a 
program execution, but I believe that won't be more than a few hundred.  These 
are short strings so they won't expand to use heap, they will fit inline.  
Strictly I think it is more memory efficient to leave these half-dozen strings 
in the constant string pool than to have an extra 24 bytes in each UnwindPlan 
with them duplicated, but it's not enough memory use for me to be actually 
worried about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154759

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


[Lldb-commits] [lldb] 5f6c558 - Revert "[LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate."

2023-07-10 Thread Caroline Tice via lldb-commits

Author: Caroline Tice
Date: 2023-07-10T16:24:31-07:00
New Revision: 5f6c55836fb4666f3160400dc273deefdac82e06

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

LOG: Revert "[LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate."

This reverts commit ee476996bec7f170928505a4c5b7715183cfbada.

That commit was not the right way to fix the issue (it could result in
reading too many bytes).  A better fix is in the works.

Original review: https://reviews.llvm.org/D153840

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index c9524870f316f4..2e512bf7581e86 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1127,16 +1127,15 @@ bool DWARFExpression::Evaluate(
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
   uint8_t addr_bytes[8];
-  size_t buf_size = sizeof(addr_bytes);
   Status error;
 
   if (target &&
-  target->ReadMemory(so_addr, &addr_bytes, buf_size, error,
- /*force_live_memory=*/false) == buf_size) {
+  target->ReadMemory(so_addr, &addr_bytes, size, error,
+ /*force_live_memory=*/false) == size) {
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, size, objfile->GetByteOrder(), buf_size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();
 break;
   } else {
@@ -1160,13 +1159,13 @@ bool DWARFExpression::Evaluate(
 lldb::addr_t pointer_addr =
 stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 uint8_t addr_bytes[sizeof(lldb::addr_t)];
-size_t buf_size = sizeof(addr_bytes);
 Status error;
-if (process->ReadMemory(pointer_addr, &addr_bytes, buf_size, error)
-== buf_size) {
+if (process->ReadMemory(pointer_addr, &addr_bytes, size, error) ==
+size) {
+
   stack.back().GetScalar() =
   DerefSizeExtractDataHelper(addr_bytes, sizeof(addr_bytes),
- process->GetByteOrder(), 
buf_size);
+ process->GetByteOrder(), size);
   stack.back().ClearContext();
 } else {
   if (error_ptr)



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


[Lldb-commits] [PATCH] D153734: [lldb][TargetGetModuleCallback] Call get module callback

2023-07-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So the "SetTargetGetModuleCallback" functions in this patch need to pass both a 
callback _and_ a callback_baton. That way people can register native callbacks 
that can be used. The python interpreter, when SetTargetGetModuleCallback is 
called from python, can take care of registering a callback that will call 
through the interpreter. See the following function as a model to follow:

  void SBDebugger::SetDestroyCallback(lldb::SBDebuggerDestroyCallback 
destroy_callback, void *baton);

See this patch for details:

  $ git show b461398f1ce30




Comment at: lldb/include/lldb/Target/Platform.h:888
+  /// from symbol servers.
+  void SetTargetGetModuleCallback(void *callback_baton);
+

You actually need a callback along with the baton here. We probably don't need 
the word "Target" in the function name?

Maybe better named as 
```
void SetLocationModuleCallback(PlatformLocateModuleCallback callback, void 
*baton);
```



Comment at: lldb/include/lldb/Target/Platform.h:890
+
+  void *GetTargetGetModuleCallback() const;
+

Usually there there are two functions:
- one that returns the function callback
- one that returns the callback baton



Comment at: lldb/include/lldb/Target/Platform.h:939
   const std::unique_ptr m_module_cache;
+  void *m_target_get_module_callback_baton = nullptr;
 

We need to store the callback itself here too. Remove "target" from the name



Comment at: lldb/source/Core/Debugger.cpp:2170-2181
+
+Status Debugger::CallTargetGetModuleCallback(
+void *target_get_module_callback_baton, const ModuleSpec &module_spec,
+FileSpec &module_file_spec, FileSpec &symbol_file_spec) {
+  ScriptInterpreter *script_interpreter = GetScriptInterpreter();
+  if (!script_interpreter)
+return Status("Cannot find ScriptInterpreter");

I don't think we need any of this in debugger. when someone, through python 
calls the "SetTargetGetModuleCallback" that code should register a static 
function as the callback to use that will end up calling into the script 
interpreter.

We don't always want to call the script interpreter for all cases. Users should 
be able to register a native callback for this, and it shouldn't force us to 
always use the script interpreter.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153734

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


[Lldb-commits] [PATCH] D153735: [lldb][TargetGetModuleCallback] Implement Python interface

2023-07-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/include/lldb/API/SBDefines.h:129
 
+typedef SBError (*SBTargetGetModuleCallback)(SBDebugger debugger,
+ SBModuleSpec &module_spec,

If we are putting this into SBPlatform, this should probably be named 
"SBPlatformGetModuleCallback". 

The name might be a bit more clear if it was "SBPlatformLocateModuleCallback"? 
Open to suggestions or fine to lave this as "SBPlatformGetModuleCallback" if 
everyone likes that.



Comment at: lldb/include/lldb/API/SBPlatform.h:181
+  /// nullptr or None is set.
+  SBError SetTargetGetModuleCallback(lldb::SBTargetGetModuleCallback callback,
+ void *callback_baton);

remove "Target" from the function name? Or rename to 
"SetLocateModuleCallback(...)" if we end up renaming the callback type to 
SBPlatformLocateModuleCallback



Comment at: lldb/source/API/SBPlatform.cpp:668
+// Platform. 'callback_baton' is the actual callback Python callable 
object.
+platform_sp->SetTargetGetModuleCallback(callback_baton);
+return SBError();

You are not passing the "callback" into Platform::SetTargetGetModuleCallback??

Are we missing changes to the file "lldb/include/lldb/Target/Platform.h"? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153735

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


[Lldb-commits] [PATCH] D154643: [lldb] Prevent crash when completing ambiguous subcommands

2023-07-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfb10b01cca85: [lldb] Prevent crash when completing ambiguous 
subcommands (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D154643?vs=537829&id=538837#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154643

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


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -880,3 +880,11 @@
 self.complete_from_to("breakpoint set -N n", "breakpoint set -N n")
 self.assertTrue(bp1.AddNameWithErrorHandling("nn"))
 self.complete_from_to("breakpoint set -N ", "breakpoint set -N nn")
+
+def test_ambiguous_command(self):
+"""Test completing an ambiguous commands"""
+self.complete_from_to("settings s", ['set', 'show'])
+
+def test_ambiguous_subcommand(self):
+"""Test completing a subcommand of an ambiguous command"""
+self.complete_from_to("settings s ta", [])
Index: lldb/source/Commands/CommandObjectMultiword.cpp
===
--- lldb/source/Commands/CommandObjectMultiword.cpp
+++ lldb/source/Commands/CommandObjectMultiword.cpp
@@ -274,10 +274,10 @@
 
   StringList new_matches;
   CommandObject *sub_command_object = GetSubcommandObject(arg0, &new_matches);
-  if (sub_command_object == nullptr) {
-request.AddCompletions(new_matches);
+
+  // The subcommand is ambiguous. The completion isn't meaningful.
+  if (!sub_command_object)
 return;
-  }
 
   // Remove the one match that we got from calling GetSubcommandObject.
   new_matches.DeleteStringAtIndex(0);


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -880,3 +880,11 @@
 self.complete_from_to("breakpoint set -N n", "breakpoint set -N n")
 self.assertTrue(bp1.AddNameWithErrorHandling("nn"))
 self.complete_from_to("breakpoint set -N ", "breakpoint set -N nn")
+
+def test_ambiguous_command(self):
+"""Test completing an ambiguous commands"""
+self.complete_from_to("settings s", ['set', 'show'])
+
+def test_ambiguous_subcommand(self):
+"""Test completing a subcommand of an ambiguous command"""
+self.complete_from_to("settings s ta", [])
Index: lldb/source/Commands/CommandObjectMultiword.cpp
===
--- lldb/source/Commands/CommandObjectMultiword.cpp
+++ lldb/source/Commands/CommandObjectMultiword.cpp
@@ -274,10 +274,10 @@
 
   StringList new_matches;
   CommandObject *sub_command_object = GetSubcommandObject(arg0, &new_matches);
-  if (sub_command_object == nullptr) {
-request.AddCompletions(new_matches);
+
+  // The subcommand is ambiguous. The completion isn't meaningful.
+  if (!sub_command_object)
 return;
-  }
 
   // Remove the one match that we got from calling GetSubcommandObject.
   new_matches.DeleteStringAtIndex(0);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] fb10b01 - [lldb] Prevent crash when completing ambiguous subcommands

2023-07-10 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2023-07-10T14:50:40-07:00
New Revision: fb10b01cca85306c8a94826e31e8a4dfb8aff502

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

LOG: [lldb] Prevent crash when completing ambiguous subcommands

Fix a crash when trying to complete an ambiguous subcommand. Take `set s
tar` for example: for the subcommand `s` there's ambiguity between set
and show. Pressing TAB after this input currently crashes LLDB. The
problem is that we're trying to complete `tar` but give up at `s`
because of the ambiguity. LLDB doesn't expect the completed string to be
shorter than the current string and crashes when trying to eliminate the
common prefix.

rdar://111848598

Differential revision: https://reviews.llvm.org/D154643

Added: 


Modified: 
lldb/source/Commands/CommandObjectMultiword.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectMultiword.cpp 
b/lldb/source/Commands/CommandObjectMultiword.cpp
index bae2717ffe68ec..7ef829afaab6e7 100644
--- a/lldb/source/Commands/CommandObjectMultiword.cpp
+++ b/lldb/source/Commands/CommandObjectMultiword.cpp
@@ -274,10 +274,10 @@ void 
CommandObjectMultiword::HandleCompletion(CompletionRequest &request) {
 
   StringList new_matches;
   CommandObject *sub_command_object = GetSubcommandObject(arg0, &new_matches);
-  if (sub_command_object == nullptr) {
-request.AddCompletions(new_matches);
+
+  // The subcommand is ambiguous. The completion isn't meaningful.
+  if (!sub_command_object)
 return;
-  }
 
   // Remove the one match that we got from calling GetSubcommandObject.
   new_matches.DeleteStringAtIndex(0);

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index 2d83abd7a6573e..4b413260279b9c 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -880,3 +880,11 @@ def test_complete_breakpoint_with_names(self):
 self.complete_from_to("breakpoint set -N n", "breakpoint set -N n")
 self.assertTrue(bp1.AddNameWithErrorHandling("nn"))
 self.complete_from_to("breakpoint set -N ", "breakpoint set -N nn")
+
+def test_ambiguous_command(self):
+"""Test completing an ambiguous commands"""
+self.complete_from_to("settings s", ['set', 'show'])
+
+def test_ambiguous_subcommand(self):
+"""Test completing a subcommand of an ambiguous command"""
+self.complete_from_to("settings s ta", [])



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


[Lldb-commits] [PATCH] D153735: [lldb][TargetGetModuleCallback] Implement Python interface

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 538835.
splhack added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153735

Files:
  lldb/bindings/python/python-typemaps.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/API/python_api/target/TestGetModuleCallback.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -321,3 +321,11 @@
 lldb_private::python::SWIGBridge::ToSWIGWrapper(lldb::DataExtractorSP) {
   return python::PythonObject();
 }
+
+Status
+lldb_private::python::SWIGBridge::LLDBSWIGPythonCallTargetGetModuleCallback(
+void *callback_baton, lldb::DebuggerSP debugger_sp,
+const ModuleSpec &module_spec, FileSpec &module_file_spec,
+FileSpec &symbol_file_spec) {
+  return Status();
+}
Index: lldb/test/API/python_api/target/TestGetModuleCallback.py
===
--- /dev/null
+++ lldb/test/API/python_api/target/TestGetModuleCallback.py
@@ -0,0 +1,303 @@
+"""
+Test Target get module callback functionality
+"""
+
+import ctypes
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from pathlib import Path
+
+import lldb
+
+UNITTESTS_TARGET_INPUTS_PATH = "../../../../unittests/Target/Inputs"
+MODULE_PLATFORM_PATH = "/system/lib64/AndroidModule.so"
+MODULE_TRIPLE = "aarch64-none-linux"
+MODULE_RESOLVED_TRIPLE = "aarch64--linux-android"
+MODULE_UUID = "80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"
+MODULE_FUNCTION = "boom"
+MODULE_HIDDEN_FUNCTION = "boom_hidden"
+MODULE_FILE = "AndroidModule.so"
+MODULE_NON_EXISTENT_FILE = "non-existent-file"
+SYMBOL_FILE = "AndroidModule.unstripped.so"
+BREAKPAD_SYMBOL_FILE = "AndroidModule.so.sym"
+SYMBOL_STRIPPED = "stripped"
+SYMBOL_UNSTRIPPED = "unstripped"
+
+
+class GetModuleCallbackTestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.platform = self.dbg.GetSelectedPlatform()
+self.target = self.dbg.CreateTarget("")
+self.assertTrue(self.target)
+
+self.input_dir = (
+Path(self.getSourceDir()) / UNITTESTS_TARGET_INPUTS_PATH
+).resolve()
+self.assertTrue(self.input_dir.is_dir())
+
+def check_module_spec(self, module_spec: lldb.SBModuleSpec):
+self.assertEqual(
+MODULE_UUID.replace("-", ""),
+ctypes.string_at(
+int(module_spec.GetUUIDBytes()),
+module_spec.GetUUIDLength(),
+)
+.hex()
+.upper(),
+)
+
+self.assertEqual(MODULE_TRIPLE, module_spec.GetTriple())
+
+self.assertEqual(MODULE_PLATFORM_PATH, module_spec.GetFileSpec().fullpath)
+
+def check_module(self, module: lldb.SBModule, symbol_file: str, symbol_kind: str):
+self.assertTrue(module.IsValid())
+
+self.assertEqual(
+MODULE_UUID,
+module.GetUUIDString(),
+)
+
+self.assertEqual(MODULE_RESOLVED_TRIPLE, module.GetTriple())
+
+self.assertEqual(MODULE_PLATFORM_PATH, module.GetPlatformFileSpec().fullpath)
+
+self.assertEqual(
+str(self.input_dir / MODULE_FILE),
+module.GetFileSpec().fullpath,
+)
+
+self.assertEqual(
+str(self.input_dir / symbol_file),
+module.GetSymbolFileSpec().fullpath,
+)
+
+sc_list = module.FindFunctions(MODULE_FUNCTION, lldb.eSymbolTypeCode)
+self.assertEqual(1, sc_list.GetSize())
+sc_list = module.FindFunctions(MODULE_HIDDEN_FUNCTION, lldb.eSymbolTypeCode)
+self.assertEqual(0 if symbol_kind == SYMBOL_STRIPPED else 1, sc_list.GetSize())
+
+def test_set_non_callable(self):
+# The callback should be callable.
+non_callable = "a"
+
+with self.assertRaises(TypeError, msg="Need a callable object or None!"):
+self.platform.SetTargetGetModuleCallback(non_callable)
+
+def test_set_wrong_args(self):
+# The callback should accept 4 argument.
+def test_args3(a, b, c):
+pass
+
+with self.assertRaises(TypeError, msg="Expected 4 argument callable object"):
+self.platform.SetTargetGetModuleCallback(test_args3)
+
+def test_set_none(self):
+# SetTargetGetModuleCallback should succeed to clear the callback with None.
+self.assert

[Lldb-commits] [lldb] c1885d2 - "settings set -g target.load-script-from-symbol-file" shouldn't crash.

2023-07-10 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2023-07-10T14:40:18-07:00
New Revision: c1885d2dfa950d0f78978546f92be15bc6cca474

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

LOG: "settings set -g target.load-script-from-symbol-file" shouldn't crash.

-g is specified by passing in nullptr ExecutionContext, but in some
load-script-from-symbol-file specific code, the ExecutionContext was
asked for its Target w/o checking whether the pointer was null.

Fix that and add a test.

Added: 


Modified: 
lldb/source/Core/Debugger.cpp
lldb/test/API/commands/settings/TestSettings.py

Removed: 




diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 3e92c125aa4ee6..4f06ed07274914 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -212,7 +212,7 @@ Status Debugger::SetPropertyValue(const ExecutionContext 
*exe_ctx,
 
   TargetSP target_sp;
   LoadScriptFromSymFile load_script_old_value = eLoadScriptFromSymFileFalse;
-  if (is_load_script && exe_ctx->GetTargetSP()) {
+  if (is_load_script && exe_ctx && exe_ctx->GetTargetSP()) {
 target_sp = exe_ctx->GetTargetSP();
 load_script_old_value =
 target_sp->TargetProperties::GetLoadScriptFromSymbolFile();

diff  --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index 5c37264281c88b..7c90ef4ba2a446 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -971,3 +971,11 @@ def test_settings_api(self):
 
 # Test OptionValueLanguage
 self.verify_setting_value_json("repl-lang", "c++")
+
+def test_global_option(self):
+# This command used to crash the settings because -g was signaled by a
+# NULL execution context (not one with an empty Target...) and in the
+# special handling for load-script-from-symbol-file this wasn't 
checked.
+self.runCmd("settings set -g target.load-script-from-symbol-file true")
+
+  



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


[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

2023-07-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

In D151683#4486321 , @aaron.ballman 
wrote:

> Aside from the failing precommit CI test case in C, I think this LGTM. I've 
> added @MaskRay as the code owner for the command line option changes in case 
> he's got concerns regarding the deprecation/removal plans.



  - ``-fdouble-square-bracket-attributes`` has been deprecated. It is ignored 
now
and will be removed in CLang 18.

sounds good. (Minor case typo in `CLang`). As you said in

https://discourse.llvm.org/t/rfc-enable-c-11-c2x-attributes-in-all-standard-modes-as-an-extension-and-remove-fdouble-square-bracket-attributes/71268/2

> Iā€™m in support of this idea. I think we should enable the extension 
> unconditionally for Clang 17 with a release note mentioning that 
> -fdouble-square-bracket-attributes will be removed in Clang 18 just as a 
> kindness to users with proprietary code bases that might be using it.

I think a clear summary/commit message should mention that this patch makes 
`[[...]]`` available to C++03 and C17 with no warning by default.

It's also worth calling out that GCC since 10 supports `[[]]` in all C language 
modes (AFAICT there is no warning even with `gcc -std=c89 -c a.c`) (there is a 
warning `-pedantic`).

  % g++ -c a.cc -std=c++03
  a.cc:2:1: error: expected unqualified-id before ā€˜[ā€™ token
  2 | [[nodiscard]] int without_underscores(void);
| ^
  % myclang++ -c a.cc -std=c++03  # no warning with this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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


[Lldb-commits] [PATCH] D154890: [lldb][NFCI] Avoid construction of temporary std::strings in Variable

2023-07-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154890

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


[Lldb-commits] [PATCH] D154643: [lldb] Prevent crash when completing ambiguous subcommands

2023-07-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

This looks like it might be the place where we also say "`set s` could either 
be `set` or `show`".  But that's actually done elsewhere so this change is 
okay.  We should maybe make sure we have a test for `setting s` -> {`set` 
or `show`} options.


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

https://reviews.llvm.org/D154643

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


[Lldb-commits] [PATCH] D154890: [lldb][NFCI] Avoid construction of temporary std::strings in Variable

2023-07-10 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, fdeazeve.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

A common thing to do is to call `str().c_str()` to get a null-terminated
string out of an existing StringRef. Most of the time this is to be able
to use a printf-style format string. However, llvm::formatv can handle
StringRefs without the need for the additional allocation. Using that
makes more sense.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154890

Files:
  lldb/source/Symbol/Variable.cpp


Index: lldb/source/Symbol/Variable.cpp
===
--- lldb/source/Symbol/Variable.cpp
+++ lldb/source/Symbol/Variable.cpp
@@ -380,9 +380,8 @@
 llvm::SmallVector matches;
 variable_list.Clear();
 if (!g_regex.Execute(variable_expr_path, &matches)) {
-  error.SetErrorStringWithFormat(
-  "unable to extract a variable name from '%s'",
-  variable_expr_path.str().c_str());
+  error.SetErrorStringWithFormatv(
+  "unable to extract a variable name from '{0}'", variable_expr_path);
   return error;
 }
 std::string variable_name = matches[1].str();
@@ -411,10 +410,9 @@
 valobj_sp = variable_valobj_sp->GetValueForExpressionPath(
 variable_sub_expr_path);
 if (!valobj_sp) {
-  error.SetErrorStringWithFormat(
-  "invalid expression path '%s' for variable '%s'",
-  variable_sub_expr_path.str().c_str(),
-  var_sp->GetName().GetCString());
+  error.SetErrorStringWithFormatv(
+  "invalid expression path '{0}' for variable '{1}'",
+  variable_sub_expr_path, var_sp->GetName().GetCString());
   variable_list.RemoveVariableAtIndex(i);
   continue;
 }


Index: lldb/source/Symbol/Variable.cpp
===
--- lldb/source/Symbol/Variable.cpp
+++ lldb/source/Symbol/Variable.cpp
@@ -380,9 +380,8 @@
 llvm::SmallVector matches;
 variable_list.Clear();
 if (!g_regex.Execute(variable_expr_path, &matches)) {
-  error.SetErrorStringWithFormat(
-  "unable to extract a variable name from '%s'",
-  variable_expr_path.str().c_str());
+  error.SetErrorStringWithFormatv(
+  "unable to extract a variable name from '{0}'", variable_expr_path);
   return error;
 }
 std::string variable_name = matches[1].str();
@@ -411,10 +410,9 @@
 valobj_sp = variable_valobj_sp->GetValueForExpressionPath(
 variable_sub_expr_path);
 if (!valobj_sp) {
-  error.SetErrorStringWithFormat(
-  "invalid expression path '%s' for variable '%s'",
-  variable_sub_expr_path.str().c_str(),
-  var_sp->GetName().GetCString());
+  error.SetErrorStringWithFormatv(
+  "invalid expression path '{0}' for variable '{1}'",
+  variable_sub_expr_path, var_sp->GetName().GetCString());
   variable_list.RemoveVariableAtIndex(i);
   continue;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D154883: [lldb][NFCI] Methods to load scripting resources should take a Stream by reference

2023-07-10 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added subscribers: lldb-commits, wangpc.
Herald added a project: LLDB.

These methods all take a `Stream *` to get feedback about what's going
on. By default, it's a nullptr, but we always feed it with a valid
pointer. It would therefore make more sense to have this take a
reference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154883

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1393,8 +1393,8 @@
Target *target) {
   Status error;
   StreamString feedback_stream;
-  if (module_sp && !module_sp->LoadScriptingResourceInTarget(
-   target, error, &feedback_stream)) {
+  if (module_sp && !module_sp->LoadScriptingResourceInTarget(target, error,
+ feedback_stream)) {
 if (error.AsCString())
   target->GetDebugger().GetErrorStream().Printf(
   "unable to load scripting data for module %s - error reported was "
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -158,7 +158,7 @@
 
 FileSpecList
 Platform::LocateExecutableScriptingResources(Target *target, Module &module,
- Stream *feedback_stream) {
+ Stream &feedback_stream) {
   return FileSpecList();
 }
 
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -69,7 +69,7 @@
 
   FileSpecList
   LocateExecutableScriptingResources(Target *target, Module &module,
- Stream *feedback_stream) override;
+ Stream &feedback_stream) override;
 
   Status GetSharedModule(const ModuleSpec &module_spec, Process *process,
  lldb::ModuleSP &module_sp,
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -194,7 +194,7 @@
 }
 
 FileSpecList PlatformDarwin::LocateExecutableScriptingResources(
-Target *target, Module &module, Stream *feedback_stream) {
+Target *target, Module &module, Stream &feedback_stream) {
   FileSpecList file_list;
   if (target &&
   target->GetDebugger().GetScriptLanguage() == eScriptLanguagePython) {
@@ -266,33 +266,31 @@
   // if we did some replacements of reserved characters, and a
   // file with the untampered name exists, then warn the user
   // that the file as-is shall not be loaded
-  if (feedback_stream) {
-if (module_basename != original_module_basename &&
-FileSystem::Instance().Exists(orig_script_fspec)) {
-  const char *reason_for_complaint =
-  was_keyword ? "conflicts with a keyword"
-  : "contains reserved characters";
-  if (FileSystem::Instance().Exists(script_fspec))
-feedback_stream->Printf(
-"warning: the symbol file '%s' contains a debug "
-"script. However, its name"
-" '%s' %s and as such cannot be loaded. LLDB will"
-" load '%s' instead. Consider removing the file with "
-"the malformed name to"
-" eliminate this warning.\n",
-symfile_spec.GetPath().c_str(),
-original_path_string.GetData(), reason_for_complaint,
-path_string.GetData());
-  else
-feedback_stream->Printf(
-"warning: the symbol file '%s' contains a debug "
-"script. However, its name"
-" %s and as such cannot be loaded. If you intend"
- 

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

2023-07-10 Thread Aaron Ballman via Phabricator via lldb-commits
aaron.ballman added a reviewer: MaskRay.
aaron.ballman added a comment.

Aside from the failing precommit CI test case in C, I think this LGTM. I've 
added @MaskRay as the code owner for the command line option changes in case 
he's got concerns regarding the deprecation/removal plans.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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


[Lldb-commits] [PATCH] D154757: [lldb][NFCI] TestEmulation should take a Stream ref

2023-07-10 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e7101a3d958: [lldb][NFCI] TestEmulation should take a 
Stream ref (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154757

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
  lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
  lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
  lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
  lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
  lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
  lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
  lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

Index: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
===
--- lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -61,7 +61,7 @@
   bool SetTargetTriple(const ArchSpec &arch) override;
   bool ReadInstruction() override;
   bool EvaluateInstruction(uint32_t options) override;
-  bool TestEmulation(Stream *out_stream, ArchSpec &arch,
+  bool TestEmulation(Stream &out_stream, ArchSpec &arch,
  OptionValueDictionary *test_data) override;
   std::optional GetRegisterInfo(lldb::RegisterKind reg_kind,
   uint32_t reg_num) override;
Index: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
===
--- lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -1748,7 +1748,7 @@
   return SupportsThisArch(arch);
 }
 
-bool EmulateInstructionRISCV::TestEmulation(Stream *out_stream, ArchSpec &arch,
+bool EmulateInstructionRISCV::TestEmulation(Stream &out_stream, ArchSpec &arch,
 OptionValueDictionary *test_data) {
   return false;
 }
Index: lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
===
--- lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
+++ lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
@@ -57,7 +57,7 @@
 
   bool EvaluateInstruction(uint32_t evaluate_options) override;
 
-  bool TestEmulation(Stream *out_stream, ArchSpec &arch,
+  bool TestEmulation(Stream &out_stream, ArchSpec &arch,
  OptionValueDictionary *test_data) override {
 return false;
   }
Index: lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
===
--- lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
+++ lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
@@ -67,7 +67,7 @@
 
   bool EvaluateInstruction(uint32_t evaluate_options) override;
 
-  bool TestEmulation(lldb_private::Stream *out_stream,
+  bool TestEmulation(lldb_private::Stream &out_stream,
  lldb_private::ArchSpec &arch,
  lldb_private::OptionValueDictionary *test_data) override {
 return false;
Index: lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
===
--- lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
+++ lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
@@ -75,7 +75,7 @@
   const lldb_private::Address &inst_addr,
   lldb_private::Target *target) override;
 
-  bool TestEmulation(lldb_private::Stream *out_stream,
+  bool TestEmulation(lldb_private::Stream &out_stream,
  lldb_private::ArchSpec &arch,
  lldb_private::OptionValueDictionary *test_data) override {
 return false;
Index: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
===
--- lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
+++ lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
@@ -52,7 +52,7 @@
   bool SetTargetTriple(const ArchSpec &arch) override;
   bool ReadInstruction() override;
   bool EvaluateInstruction(uint32_t options) override;
-  bo

[Lldb-commits] [lldb] 1e7101a - [lldb][NFCI] TestEmulation should take a Stream ref

2023-07-10 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-07-10T11:17:25-07:00
New Revision: 1e7101a3d95884c791d566b7b6192fbac55f55e0

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

LOG: [lldb][NFCI] TestEmulation should take a Stream ref

`Instruction::TestEmulation` takes a `Stream *` and checks it for validity.
However, this is unnecessary as we can always ensure that we never pass
`nullptr` for the `Stream` argument. The only use of
`Instruction::TestEmulation` currently is `SBInstruction::TestEmulation`
which gets the `Stream` from an `SBStream`, and `SBStream::ref` can
return a `Stream &` guaranteed.

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

Added: 


Modified: 
lldb/include/lldb/Core/Disassembler.h
lldb/include/lldb/Core/EmulateInstruction.h
lldb/source/API/SBInstruction.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
lldb/source/Plugins/Instruction/ARM/EmulationStateARM.h
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

Removed: 




diff  --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index b9ac0a5bca39c4..181d0b5d0e2902 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -176,14 +176,14 @@ class Instruction {
   virtual void SetDescription(llvm::StringRef) {
   } // May be overridden in sub-classes that have descriptions.
 
-  lldb::OptionValueSP ReadArray(FILE *in_file, Stream *out_stream,
+  lldb::OptionValueSP ReadArray(FILE *in_file, Stream &out_stream,
 OptionValue::Type data_type);
 
-  lldb::OptionValueSP ReadDictionary(FILE *in_file, Stream *out_stream);
+  lldb::OptionValueSP ReadDictionary(FILE *in_file, Stream &out_stream);
 
   bool DumpEmulation(const ArchSpec &arch);
 
-  virtual bool TestEmulation(Stream *stream, const char *test_file_name);
+  virtual bool TestEmulation(Stream &stream, const char *test_file_name);
 
   bool Emulate(const ArchSpec &arch, uint32_t evaluate_options, void *baton,
EmulateInstruction::ReadMemoryCallback read_mem_callback,

diff  --git a/lldb/include/lldb/Core/EmulateInstruction.h 
b/lldb/include/lldb/Core/EmulateInstruction.h
index 6d76380ce65927..93c16537adba12 100644
--- a/lldb/include/lldb/Core/EmulateInstruction.h
+++ b/lldb/include/lldb/Core/EmulateInstruction.h
@@ -375,7 +375,7 @@ class EmulateInstruction : public PluginInterface {
 return UnconditionalCondition;
   }
 
-  virtual bool TestEmulation(Stream *out_stream, ArchSpec &arch,
+  virtual bool TestEmulation(Stream &out_stream, ArchSpec &arch,
  OptionValueDictionary *test_data) = 0;
 
   virtual std::optional

diff  --git a/lldb/source/API/SBInstruction.cpp 
b/lldb/source/API/SBInstruction.cpp
index 46da6e18b6b2b7..ea14e90abfd23d 100644
--- a/lldb/source/API/SBInstruction.cpp
+++ b/lldb/source/API/SBInstruction.cpp
@@ -345,6 +345,6 @@ bool SBInstruction::TestEmulation(lldb::SBStream 
&output_stream,
 
   lldb::InstructionSP inst_sp(GetOpaque());
   if (inst_sp)
-return inst_sp->TestEmulation(output_stream.get(), test_file);
+return inst_sp->TestEmulation(output_stream.ref(), test_file);
   return false;
 }

diff  --git a/lldb/source/Core/Disassembler.cpp 
b/lldb/source/Core/Disassembler.cpp
index 5e79aa40cb33e8..104e9100e38830 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -687,7 +687,7 @@ bool Instruction::HasDelaySlot() {
   return false;
 }
 
-OptionValueSP Instruction::ReadArray(FILE *in_file, Stream *out_stream,
+OptionValueSP Instruction::ReadArray(FILE *in_file, Stream &out_stream,
  OptionValue::Type data_type) {
   bool done = false;
   char buffer[1024];
@@ -697,7 +697,7 @@ OptionValueSP Instruction::ReadArray(FILE *in_file, Stream 
*out_stream,
   int idx = 0;
   while (!done) {
 if (!fgets(buffer, 1023, in_file)) {
-  out_stream->Printf(
+  out_stream.Printf(
   "Instruction::ReadArray:  Error reading file (fgets).\n");
   option_value_sp.reset();
   return option_value_sp;
@@ -746

[Lldb-commits] [PATCH] D153734: [lldb][TargetGetModuleCallback] Call get module callback

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 538741.
splhack added a comment.

Fix comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153734

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/GetModuleCallbackTest.cpp
  lldb/unittests/Target/Inputs/AndroidModule.c
  lldb/unittests/Target/Inputs/AndroidModule.so
  lldb/unittests/Target/Inputs/AndroidModule.so.sym
  lldb/unittests/Target/Inputs/AndroidModule.unstripped.so

Index: lldb/unittests/Target/Inputs/AndroidModule.so.sym
===
--- /dev/null
+++ lldb/unittests/Target/Inputs/AndroidModule.so.sym
@@ -0,0 +1,21 @@
+MODULE Linux arm64 38830080A082E5515922C905D23890DA0 AndroidModule.so
+INFO CODE_ID 8000833882A051E55922C905D23890DABDDEFECC
+FILE 0 /private/tmp/test/AndroidModule.c
+FUNC 162c 8 0 boom
+162c 8 8 0
+FUNC 1634 8 0 boom_hidden
+1634 8 12 0
+PUBLIC 15cc 0 __on_dlclose
+PUBLIC 15dc 0 __emutls_unregister_key
+PUBLIC 15e4 0 __on_dlclose_late
+PUBLIC 15ec 0 __atexit_handler_wrapper
+PUBLIC 1600 0 atexit
+PUBLIC 161c 0 pthread_atfork
+STACK CFI INIT 15cc 10 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15dc 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15e4 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15ec 14 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 1600 1c .cfa: sp 0 + .ra: x30
+STACK CFI INIT 161c 10 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 162c 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 1634 8 .cfa: sp 0 + .ra: x30
Index: lldb/unittests/Target/Inputs/AndroidModule.c
===
--- /dev/null
+++ lldb/unittests/Target/Inputs/AndroidModule.c
@@ -0,0 +1,13 @@
+// aarch64-linux-android29-clang -shared -Os -glldb -g3 -Wl,--build-id=sha1 \
+// AndroidModule.c -o AndroidModule.so
+// dump_syms AndroidModule.so > AndroidModule.so.sym
+// cp AndroidModule.so AndroidModule.unstripped.so
+// llvm-strip --strip-unneeded AndroidModule.so
+
+int boom(void) {
+  return 47;
+}
+
+__attribute__((visibility("hidden"))) int boom_hidden(void) {
+  return 48;
+}
Index: lldb/unittests/Target/GetModuleCallbackTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/GetModuleCallbackTest.cpp
@@ -0,0 +1,675 @@
+//===-- GetModuleCallbackTest.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 "Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h"
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/Platform/Android/PlatformAndroid.h"
+#include "Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Target.h"
+#include "gmock/gmock.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_android;
+using namespace lldb_private::breakpad;
+using namespace testing;
+
+namespace {
+
+constexpr llvm::StringLiteral k_process_plugin("mock-process-plugin");
+constexpr llvm::StringLiteral k_platform_dir("remote-android");
+constexpr llvm::StringLiteral k_cache_dir(".cache");
+constexpr llvm::StringLiteral k_module_file("AndroidModule.so");
+constexpr llvm::StringLiteral k_symbol_file("AndroidModule.unstripped.so");
+constexpr llvm::StringLiteral k_breakpad_symbol_file("AndroidModule.so.sym");
+constexpr llvm::StringLiteral k_arch("aarch64-none-linux");
+constexpr llvm::StringLiteral
+k_module_uuid("80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC");
+constexpr llvm::StringLiteral k_function_symbol("boom");
+constexpr llvm::StringLiteral k_hidden_function_symbol("boom_hidden");
+const size_t k_module_size = 3784;
+
+class MockDebugger : public Debugger {
+public:
+  MockDebugger() : Debugger(nullptr, nullptr) {}
+
+  MOCK_METHOD4(CallTargetGetModuleCallback,
+   Status(void *, const ModuleSpec &, FileSpec &, FileSpec &));
+};
+
+ModuleSpec GetTestModuleSpec();
+
+class MockProcess : public Process {
+public:
+  MockProcess(TargetSP target_sp, ListenerSP listener_sp)
+  : Process(target_sp, listener_sp) {}
+
+  llvm::StringRef GetPluginName() override { return k_process_plugin; };
+
+  bool CanDebug(TargetSP target, bool plugin_specified_by_name)

[Lldb-commits] [PATCH] D153735: [lldb][TargetGetModuleCallback] Implement Python interface

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 538713.
splhack added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153735

Files:
  lldb/bindings/python/python-typemaps.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/API/python_api/target/TestGetModuleCallback.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -321,3 +321,11 @@
 lldb_private::python::SWIGBridge::ToSWIGWrapper(lldb::DataExtractorSP) {
   return python::PythonObject();
 }
+
+Status
+lldb_private::python::SWIGBridge::LLDBSWIGPythonCallTargetGetModuleCallback(
+void *callback_baton, lldb::DebuggerSP debugger_sp,
+const ModuleSpec &module_spec, FileSpec &module_file_spec,
+FileSpec &symbol_file_spec) {
+  return Status();
+}
Index: lldb/test/API/python_api/target/TestGetModuleCallback.py
===
--- /dev/null
+++ lldb/test/API/python_api/target/TestGetModuleCallback.py
@@ -0,0 +1,303 @@
+"""
+Test Target get module callback functionality
+"""
+
+import ctypes
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from pathlib import Path
+
+import lldb
+
+UNITTESTS_TARGET_INPUTS_PATH = "../../../../unittests/Target/Inputs"
+MODULE_PLATFORM_PATH = "/system/lib64/AndroidModule.so"
+MODULE_TRIPLE = "aarch64-none-linux"
+MODULE_RESOLVED_TRIPLE = "aarch64--linux-android"
+MODULE_UUID = "80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"
+MODULE_FUNCTION = "boom"
+MODULE_HIDDEN_FUNCTION = "boom_hidden"
+MODULE_FILE = "AndroidModule.so"
+MODULE_NON_EXISTENT_FILE = "non-existent-file"
+SYMBOL_FILE = "AndroidModule.unstripped.so"
+BREAKPAD_SYMBOL_FILE = "AndroidModule.so.sym"
+SYMBOL_STRIPPED = "stripped"
+SYMBOL_UNSTRIPPED = "unstripped"
+
+
+class GetModuleCallbackTestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.platform = self.dbg.GetSelectedPlatform()
+self.target = self.dbg.CreateTarget("")
+self.assertTrue(self.target)
+
+self.input_dir = (
+Path(self.getSourceDir()) / UNITTESTS_TARGET_INPUTS_PATH
+).resolve()
+self.assertTrue(self.input_dir.is_dir())
+
+def check_module_spec(self, module_spec: lldb.SBModuleSpec):
+self.assertEqual(
+MODULE_UUID.replace("-", ""),
+ctypes.string_at(
+int(module_spec.GetUUIDBytes()),
+module_spec.GetUUIDLength(),
+)
+.hex()
+.upper(),
+)
+
+self.assertEqual(MODULE_TRIPLE, module_spec.GetTriple())
+
+self.assertEqual(MODULE_PLATFORM_PATH, module_spec.GetFileSpec().fullpath)
+
+def check_module(self, module: lldb.SBModule, symbol_file: str, symbol_kind: str):
+self.assertTrue(module.IsValid())
+
+self.assertEqual(
+MODULE_UUID,
+module.GetUUIDString(),
+)
+
+self.assertEqual(MODULE_RESOLVED_TRIPLE, module.GetTriple())
+
+self.assertEqual(MODULE_PLATFORM_PATH, module.GetPlatformFileSpec().fullpath)
+
+self.assertEqual(
+str(self.input_dir / MODULE_FILE),
+module.GetFileSpec().fullpath,
+)
+
+self.assertEqual(
+str(self.input_dir / symbol_file),
+module.GetSymbolFileSpec().fullpath,
+)
+
+sc_list = module.FindFunctions(MODULE_FUNCTION, lldb.eSymbolTypeCode)
+self.assertEqual(1, sc_list.GetSize())
+sc_list = module.FindFunctions(MODULE_HIDDEN_FUNCTION, lldb.eSymbolTypeCode)
+self.assertEqual(0 if symbol_kind == SYMBOL_STRIPPED else 1, sc_list.GetSize())
+
+def test_set_non_callable(self):
+# The callback should be callable.
+non_callable = "a"
+
+with self.assertRaises(TypeError, msg="Need a callable object or None!"):
+self.platform.SetTargetGetModuleCallback(non_callable)
+
+def test_set_wrong_args(self):
+# The callback should accept 4 argument.
+def test_args3(a, b, c):
+pass
+
+with self.assertRaises(TypeError, msg="Expected 4 argument callable object"):
+self.platform.SetTargetGetModuleCallback(test_args3)
+
+def test_set_none(self):
+# SetTargetGetModuleCallback should succeed to clear the callback with None.
+self.assert

[Lldb-commits] [PATCH] D153734: [lldb][TargetGetModuleCallback] Call get module callback

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 538696.
splhack added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153734

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/GetModuleCallbackTest.cpp
  lldb/unittests/Target/Inputs/AndroidModule.c
  lldb/unittests/Target/Inputs/AndroidModule.so
  lldb/unittests/Target/Inputs/AndroidModule.so.sym
  lldb/unittests/Target/Inputs/AndroidModule.unstripped.so

Index: lldb/unittests/Target/Inputs/AndroidModule.so.sym
===
--- /dev/null
+++ lldb/unittests/Target/Inputs/AndroidModule.so.sym
@@ -0,0 +1,21 @@
+MODULE Linux arm64 38830080A082E5515922C905D23890DA0 AndroidModule.so
+INFO CODE_ID 8000833882A051E55922C905D23890DABDDEFECC
+FILE 0 /private/tmp/test/AndroidModule.c
+FUNC 162c 8 0 boom
+162c 8 8 0
+FUNC 1634 8 0 boom_hidden
+1634 8 12 0
+PUBLIC 15cc 0 __on_dlclose
+PUBLIC 15dc 0 __emutls_unregister_key
+PUBLIC 15e4 0 __on_dlclose_late
+PUBLIC 15ec 0 __atexit_handler_wrapper
+PUBLIC 1600 0 atexit
+PUBLIC 161c 0 pthread_atfork
+STACK CFI INIT 15cc 10 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15dc 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15e4 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15ec 14 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 1600 1c .cfa: sp 0 + .ra: x30
+STACK CFI INIT 161c 10 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 162c 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 1634 8 .cfa: sp 0 + .ra: x30
Index: lldb/unittests/Target/Inputs/AndroidModule.c
===
--- /dev/null
+++ lldb/unittests/Target/Inputs/AndroidModule.c
@@ -0,0 +1,13 @@
+// aarch64-linux-android29-clang -shared -Os -glldb -g3 -Wl,--build-id=sha1 \
+// AndroidModule.c -o AndroidModule.so
+// dump_syms AndroidModule.so > AndroidModule.so.sym
+// cp AndroidModule.so AndroidModule.unstripped.so
+// llvm-strip --strip-unneeded AndroidModule.so
+
+int boom(void) {
+  return 47;
+}
+
+__attribute__((visibility("hidden"))) int boom_hidden(void) {
+  return 48;
+}
Index: lldb/unittests/Target/GetModuleCallbackTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/GetModuleCallbackTest.cpp
@@ -0,0 +1,675 @@
+//===-- GetModuleCallbackTest.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 "Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h"
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/Platform/Android/PlatformAndroid.h"
+#include "Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Target.h"
+#include "gmock/gmock.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_android;
+using namespace lldb_private::breakpad;
+using namespace testing;
+
+namespace {
+
+constexpr llvm::StringLiteral k_process_plugin("mock-process-plugin");
+constexpr llvm::StringLiteral k_platform_dir("remote-android");
+constexpr llvm::StringLiteral k_cache_dir(".cache");
+constexpr llvm::StringLiteral k_module_file("AndroidModule.so");
+constexpr llvm::StringLiteral k_symbol_file("AndroidModule.unstripped.so");
+constexpr llvm::StringLiteral k_breakpad_symbol_file("AndroidModule.so.sym");
+constexpr llvm::StringLiteral k_arch("aarch64-none-linux");
+constexpr llvm::StringLiteral
+k_module_uuid("80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC");
+constexpr llvm::StringLiteral k_function_symbol("boom");
+constexpr llvm::StringLiteral k_hidden_function_symbol("boom_hidden");
+const size_t k_module_size = 3784;
+
+class MockDebugger : public Debugger {
+public:
+  MockDebugger() : Debugger(nullptr, nullptr) {}
+
+  MOCK_METHOD4(CallTargetGetModuleCallback,
+   Status(void *, const ModuleSpec &, FileSpec &, FileSpec &));
+};
+
+ModuleSpec GetTestModuleSpec();
+
+class MockProcess : public Process {
+public:
+  MockProcess(TargetSP target_sp, ListenerSP listener_sp)
+  : Process(target_sp, listener_sp) {}
+
+  llvm::StringRef GetPluginName() override { return k_process_plugin; };
+
+  bool CanDebug(TargetSP target, bool plugin_specified_by_name) overr

[Lldb-commits] [PATCH] D153733: [lldb][TargetGetModuleCallback] Update SBFileSpec/SBModuleSpec

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 538662.
splhack added a comment.

fix LLDB_INSTRUMENT_VA for SBModuleSpec constructor too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153733

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/include/lldb/API/SBFileSpec.h
  lldb/include/lldb/API/SBModuleSpec.h
  lldb/source/API/SBModuleSpec.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/test/API/python_api/module_spec/TestModuleSpec.py

Index: lldb/test/API/python_api/module_spec/TestModuleSpec.py
===
--- /dev/null
+++ lldb/test/API/python_api/module_spec/TestModuleSpec.py
@@ -0,0 +1,24 @@
+"""
+Test some SBModuleSpec APIs.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class ModuleSpecAPIsTestCase(TestBase):
+def test_object_offset_and_size(self):
+module_spec = lldb.SBModuleSpec()
+self.assertEqual(module_spec.GetObjectOffset(), 0)
+self.assertEqual(module_spec.GetObjectSize(), 0)
+
+module_spec.SetObjectOffset(4096)
+self.assertEqual(module_spec.GetObjectOffset(), 4096)
+
+module_spec.SetObjectSize(3600)
+self.assertEqual(module_spec.GetObjectSize(), 3600)
+
+module_spec.Clear()
+self.assertEqual(module_spec.GetObjectOffset(), 0)
+self.assertEqual(module_spec.GetObjectSize(), 0)
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -30,6 +30,8 @@
 class SBValue;
 class SBStream;
 class SBStructuredData;
+class SBFileSpec;
+class SBModuleSpec;
 } // namespace lldb
 
 namespace lldb_private {
@@ -102,6 +104,10 @@
   static PythonObject ToSWIGWrapper(std::unique_ptr stream_sb);
   static PythonObject
   ToSWIGWrapper(std::unique_ptr data_sb);
+  static PythonObject
+  ToSWIGWrapper(std::unique_ptr file_spec_sb);
+  static PythonObject
+  ToSWIGWrapper(std::unique_ptr module_spec_sb);
 
   static python::ScopedPythonObject
   ToSWIGWrapper(CommandReturnObject &cmd_retobj);
Index: lldb/source/API/SBModuleSpec.cpp
===
--- lldb/source/API/SBModuleSpec.cpp
+++ lldb/source/API/SBModuleSpec.cpp
@@ -29,6 +29,11 @@
   m_opaque_up = clone(rhs.m_opaque_up);
 }
 
+SBModuleSpec::SBModuleSpec(const lldb_private::ModuleSpec &module_spec)
+: m_opaque_up(new lldb_private::ModuleSpec(module_spec)) {
+  LLDB_INSTRUMENT_VA(this, module_spec);
+}
+
 const SBModuleSpec &SBModuleSpec::operator=(const SBModuleSpec &rhs) {
   LLDB_INSTRUMENT_VA(this, rhs);
 
@@ -145,6 +150,30 @@
   return true;
 }
 
+uint64_t SBModuleSpec::GetObjectOffset() {
+  LLDB_INSTRUMENT_VA(this);
+
+  return m_opaque_up->GetObjectOffset();
+}
+
+void SBModuleSpec::SetObjectOffset(uint64_t object_offset) {
+  LLDB_INSTRUMENT_VA(this, object_offset);
+
+  m_opaque_up->SetObjectOffset(object_offset);
+}
+
+uint64_t SBModuleSpec::GetObjectSize() {
+  LLDB_INSTRUMENT_VA(this);
+
+  return m_opaque_up->GetObjectSize();
+}
+
+void SBModuleSpec::SetObjectSize(uint64_t object_size) {
+  LLDB_INSTRUMENT_VA(this, object_size);
+
+  m_opaque_up->SetObjectSize(object_size);
+}
+
 SBModuleSpecList::SBModuleSpecList() : m_opaque_up(new ModuleSpecList()) {
   LLDB_INSTRUMENT_VA(this);
 }
Index: lldb/include/lldb/API/SBModuleSpec.h
===
--- lldb/include/lldb/API/SBModuleSpec.h
+++ lldb/include/lldb/API/SBModuleSpec.h
@@ -12,6 +12,12 @@
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBFileSpec.h"
 
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+} // namespace lldb_private
+
 namespace lldb {
 
 class LLDB_API SBModuleSpec {
@@ -77,6 +83,14 @@
 
   bool SetUUIDBytes(const uint8_t *uuid, size_t uuid_len);
 
+  uint64_t GetObjectOffset();
+
+  void SetObjectOffset(uint64_t object_offset);
+
+  uint64_t GetObjectSize();
+
+  void SetObjectSize(uint64_t object_size);
+
   bool GetDescription(lldb::SBStream &description);
 
 private:
@@ -84,6 +98,10 @@
   friend class SBModule;
   friend class SBTarget;
 
+  friend class lldb_private::python::SWIGBridge;
+
+  SBModuleSpec(const lldb_private::ModuleSpec &module_spec);
+
   std::unique_ptr m_opaque_up;
 };
 
Index: lldb/include/lldb/API/SBFileSpec.h
===
--- lldb/include/lldb/API/SBFileSpec.h
+++ lldb/include/lldb/API/SBFileSpec.h
@@ -11,6 +11,12 @@
 
 #include "lldb/API/SBDefines.h"
 
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+} // namespace lldb_private
+
 namespace lldb {
 
 class LLDB_API SBFileSpec {
@@ -7

[Lldb-commits] [PATCH] D153733: [lldb][TargetGetModuleCallback] Update SBFileSpec/SBModuleSpec

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 538660.
splhack added a comment.

Fix LLDB_INSTRUMENT_VA args


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153733

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/include/lldb/API/SBFileSpec.h
  lldb/include/lldb/API/SBModuleSpec.h
  lldb/source/API/SBModuleSpec.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/test/API/python_api/module_spec/TestModuleSpec.py

Index: lldb/test/API/python_api/module_spec/TestModuleSpec.py
===
--- /dev/null
+++ lldb/test/API/python_api/module_spec/TestModuleSpec.py
@@ -0,0 +1,24 @@
+"""
+Test some SBModuleSpec APIs.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class ModuleSpecAPIsTestCase(TestBase):
+def test_object_offset_and_size(self):
+module_spec = lldb.SBModuleSpec()
+self.assertEqual(module_spec.GetObjectOffset(), 0)
+self.assertEqual(module_spec.GetObjectSize(), 0)
+
+module_spec.SetObjectOffset(4096)
+self.assertEqual(module_spec.GetObjectOffset(), 4096)
+
+module_spec.SetObjectSize(3600)
+self.assertEqual(module_spec.GetObjectSize(), 3600)
+
+module_spec.Clear()
+self.assertEqual(module_spec.GetObjectOffset(), 0)
+self.assertEqual(module_spec.GetObjectSize(), 0)
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -30,6 +30,8 @@
 class SBValue;
 class SBStream;
 class SBStructuredData;
+class SBFileSpec;
+class SBModuleSpec;
 } // namespace lldb
 
 namespace lldb_private {
@@ -102,6 +104,10 @@
   static PythonObject ToSWIGWrapper(std::unique_ptr stream_sb);
   static PythonObject
   ToSWIGWrapper(std::unique_ptr data_sb);
+  static PythonObject
+  ToSWIGWrapper(std::unique_ptr file_spec_sb);
+  static PythonObject
+  ToSWIGWrapper(std::unique_ptr module_spec_sb);
 
   static python::ScopedPythonObject
   ToSWIGWrapper(CommandReturnObject &cmd_retobj);
Index: lldb/source/API/SBModuleSpec.cpp
===
--- lldb/source/API/SBModuleSpec.cpp
+++ lldb/source/API/SBModuleSpec.cpp
@@ -29,6 +29,11 @@
   m_opaque_up = clone(rhs.m_opaque_up);
 }
 
+SBModuleSpec::SBModuleSpec(const lldb_private::ModuleSpec &module_spec)
+: m_opaque_up(new lldb_private::ModuleSpec(module_spec)) {
+  LLDB_INSTRUMENT_VA(this);
+}
+
 const SBModuleSpec &SBModuleSpec::operator=(const SBModuleSpec &rhs) {
   LLDB_INSTRUMENT_VA(this, rhs);
 
@@ -145,6 +150,30 @@
   return true;
 }
 
+uint64_t SBModuleSpec::GetObjectOffset() {
+  LLDB_INSTRUMENT_VA(this);
+
+  return m_opaque_up->GetObjectOffset();
+}
+
+void SBModuleSpec::SetObjectOffset(uint64_t object_offset) {
+  LLDB_INSTRUMENT_VA(this, object_offset);
+
+  m_opaque_up->SetObjectOffset(object_offset);
+}
+
+uint64_t SBModuleSpec::GetObjectSize() {
+  LLDB_INSTRUMENT_VA(this);
+
+  return m_opaque_up->GetObjectSize();
+}
+
+void SBModuleSpec::SetObjectSize(uint64_t object_size) {
+  LLDB_INSTRUMENT_VA(this, object_size);
+
+  m_opaque_up->SetObjectSize(object_size);
+}
+
 SBModuleSpecList::SBModuleSpecList() : m_opaque_up(new ModuleSpecList()) {
   LLDB_INSTRUMENT_VA(this);
 }
Index: lldb/include/lldb/API/SBModuleSpec.h
===
--- lldb/include/lldb/API/SBModuleSpec.h
+++ lldb/include/lldb/API/SBModuleSpec.h
@@ -12,6 +12,12 @@
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBFileSpec.h"
 
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+} // namespace lldb_private
+
 namespace lldb {
 
 class LLDB_API SBModuleSpec {
@@ -77,6 +83,14 @@
 
   bool SetUUIDBytes(const uint8_t *uuid, size_t uuid_len);
 
+  uint64_t GetObjectOffset();
+
+  void SetObjectOffset(uint64_t object_offset);
+
+  uint64_t GetObjectSize();
+
+  void SetObjectSize(uint64_t object_size);
+
   bool GetDescription(lldb::SBStream &description);
 
 private:
@@ -84,6 +98,10 @@
   friend class SBModule;
   friend class SBTarget;
 
+  friend class lldb_private::python::SWIGBridge;
+
+  SBModuleSpec(const lldb_private::ModuleSpec &module_spec);
+
   std::unique_ptr m_opaque_up;
 };
 
Index: lldb/include/lldb/API/SBFileSpec.h
===
--- lldb/include/lldb/API/SBFileSpec.h
+++ lldb/include/lldb/API/SBFileSpec.h
@@ -11,6 +11,12 @@
 
 #include "lldb/API/SBDefines.h"
 
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+} // namespace lldb_private
+
 namespace lldb {
 
 class LLDB_API SBFileSpec {
@@ -79,6 +85,8 @@
   friend class SBThread;
  

[Lldb-commits] [PATCH] D153733: [lldb][TargetGetModuleCallback] Update SBFileSpec/SBModuleSpec

2023-07-10 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 538649.
splhack added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153733

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/include/lldb/API/SBFileSpec.h
  lldb/include/lldb/API/SBModuleSpec.h
  lldb/source/API/SBModuleSpec.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/test/API/python_api/module_spec/TestModuleSpec.py

Index: lldb/test/API/python_api/module_spec/TestModuleSpec.py
===
--- /dev/null
+++ lldb/test/API/python_api/module_spec/TestModuleSpec.py
@@ -0,0 +1,24 @@
+"""
+Test some SBModuleSpec APIs.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class ModuleSpecAPIsTestCase(TestBase):
+def test_object_offset_and_size(self):
+module_spec = lldb.SBModuleSpec()
+self.assertEqual(module_spec.GetObjectOffset(), 0)
+self.assertEqual(module_spec.GetObjectSize(), 0)
+
+module_spec.SetObjectOffset(4096)
+self.assertEqual(module_spec.GetObjectOffset(), 4096)
+
+module_spec.SetObjectSize(3600)
+self.assertEqual(module_spec.GetObjectSize(), 3600)
+
+module_spec.Clear()
+self.assertEqual(module_spec.GetObjectOffset(), 0)
+self.assertEqual(module_spec.GetObjectSize(), 0)
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -30,6 +30,8 @@
 class SBValue;
 class SBStream;
 class SBStructuredData;
+class SBFileSpec;
+class SBModuleSpec;
 } // namespace lldb
 
 namespace lldb_private {
@@ -102,6 +104,10 @@
   static PythonObject ToSWIGWrapper(std::unique_ptr stream_sb);
   static PythonObject
   ToSWIGWrapper(std::unique_ptr data_sb);
+  static PythonObject
+  ToSWIGWrapper(std::unique_ptr file_spec_sb);
+  static PythonObject
+  ToSWIGWrapper(std::unique_ptr module_spec_sb);
 
   static python::ScopedPythonObject
   ToSWIGWrapper(CommandReturnObject &cmd_retobj);
Index: lldb/source/API/SBModuleSpec.cpp
===
--- lldb/source/API/SBModuleSpec.cpp
+++ lldb/source/API/SBModuleSpec.cpp
@@ -29,6 +29,11 @@
   m_opaque_up = clone(rhs.m_opaque_up);
 }
 
+SBModuleSpec::SBModuleSpec(const lldb_private::ModuleSpec &module_spec)
+: m_opaque_up(new lldb_private::ModuleSpec(module_spec)) {
+  LLDB_INSTRUMENT_VA(this);
+}
+
 const SBModuleSpec &SBModuleSpec::operator=(const SBModuleSpec &rhs) {
   LLDB_INSTRUMENT_VA(this, rhs);
 
@@ -145,6 +150,30 @@
   return true;
 }
 
+uint64_t SBModuleSpec::GetObjectOffset() {
+  LLDB_INSTRUMENT_VA(this);
+
+  return m_opaque_up->GetObjectOffset();
+}
+
+void SBModuleSpec::SetObjectOffset(uint64_t object_offset) {
+  LLDB_INSTRUMENT_VA(this);
+
+  m_opaque_up->SetObjectOffset(object_offset);
+}
+
+uint64_t SBModuleSpec::GetObjectSize() {
+  LLDB_INSTRUMENT_VA(this);
+
+  return m_opaque_up->GetObjectSize();
+}
+
+void SBModuleSpec::SetObjectSize(uint64_t object_size) {
+  LLDB_INSTRUMENT_VA(this);
+
+  m_opaque_up->SetObjectSize(object_size);
+}
+
 SBModuleSpecList::SBModuleSpecList() : m_opaque_up(new ModuleSpecList()) {
   LLDB_INSTRUMENT_VA(this);
 }
Index: lldb/include/lldb/API/SBModuleSpec.h
===
--- lldb/include/lldb/API/SBModuleSpec.h
+++ lldb/include/lldb/API/SBModuleSpec.h
@@ -12,6 +12,12 @@
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBFileSpec.h"
 
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+} // namespace lldb_private
+
 namespace lldb {
 
 class LLDB_API SBModuleSpec {
@@ -77,6 +83,14 @@
 
   bool SetUUIDBytes(const uint8_t *uuid, size_t uuid_len);
 
+  uint64_t GetObjectOffset();
+
+  void SetObjectOffset(uint64_t object_offset);
+
+  uint64_t GetObjectSize();
+
+  void SetObjectSize(uint64_t object_size);
+
   bool GetDescription(lldb::SBStream &description);
 
 private:
@@ -84,6 +98,10 @@
   friend class SBModule;
   friend class SBTarget;
 
+  friend class lldb_private::python::SWIGBridge;
+
+  SBModuleSpec(const lldb_private::ModuleSpec &module_spec);
+
   std::unique_ptr m_opaque_up;
 };
 
Index: lldb/include/lldb/API/SBFileSpec.h
===
--- lldb/include/lldb/API/SBFileSpec.h
+++ lldb/include/lldb/API/SBFileSpec.h
@@ -11,6 +11,12 @@
 
 #include "lldb/API/SBDefines.h"
 
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+} // namespace lldb_private
+
 namespace lldb {
 
 class LLDB_API SBFileSpec {
@@ -79,6 +85,8 @@
   friend class SBThread;
   friend class SBTrace;
 
+  friend class lldb_pri

[Lldb-commits] [PATCH] D154843: [lldb] Disable TestNamespaceLookup in DWARF 5 + dSYM setting

2023-07-10 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6890ad3f41c5: [lldb] Disable TestNamespaceLookup in DWARF 5 
+ dSYM setting (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154843

Files:
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py


Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -164,7 +164,11 @@
 
 # Continue to BP_file_scope at file scope
 self.runToBkpt("continue")
-self.expect_expr("func()", result_type="int", result_value="2")
+# FIXME: In DWARF 5 with dsyms, the ordering of functions is slightly
+# different, which also hits the same issues mentioned previously.
+if (configuration.dwarf_version <= 4 or
+self.getDebugInfo() == 'dwarf'):
+self.expect_expr("func()", result_type="int", result_value="2")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")


Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -164,7 +164,11 @@
 
 # Continue to BP_file_scope at file scope
 self.runToBkpt("continue")
-self.expect_expr("func()", result_type="int", result_value="2")
+# FIXME: In DWARF 5 with dsyms, the ordering of functions is slightly
+# different, which also hits the same issues mentioned previously.
+if (configuration.dwarf_version <= 4 or
+self.getDebugInfo() == 'dwarf'):
+self.expect_expr("func()", result_type="int", result_value="2")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 6890ad3 - [lldb] Disable TestNamespaceLookup in DWARF 5 + dSYM setting

2023-07-10 Thread Felipe de Azevedo Piovezan via lldb-commits

Author: Felipe de Azevedo Piovezan
Date: 2023-07-10T10:10:15-04:00
New Revision: 6890ad3f41c51ba9008c26f5b23397d5e0bc2aad

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

LOG: [lldb] Disable TestNamespaceLookup in DWARF 5 + dSYM setting

The ordering in which functions are presented to the expression evaluator in
this test setting triggers a known bug in LLDB.

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

Added: 


Modified: 
lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py 
b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
index 2040a2d8f76a64..2b1e2ac00f07ff 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -164,7 +164,11 @@ def test_scope_lookup_with_run_command(self):
 
 # Continue to BP_file_scope at file scope
 self.runToBkpt("continue")
-self.expect_expr("func()", result_type="int", result_value="2")
+# FIXME: In DWARF 5 with dsyms, the ordering of functions is slightly
+# 
diff erent, which also hits the same issues mentioned previously.
+if (configuration.dwarf_version <= 4 or
+self.getDebugInfo() == 'dwarf'):
+self.expect_expr("func()", result_type="int", result_value="2")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")



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


[Lldb-commits] [PATCH] D154843: [lldb] Disable TestNamespaceLookup in DWARF 5 + dSYM setting

2023-07-10 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve created this revision.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The ordering in which functions are presented to the expression evaluator in
this test setting triggers a known bug in LLDB.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154843

Files:
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py


Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -164,7 +164,11 @@
 
 # Continue to BP_file_scope at file scope
 self.runToBkpt("continue")
-self.expect_expr("func()", result_type="int", result_value="2")
+# FIXME: In DWARF 5 with dsyms, the ordering of functions is slightly
+# different, which also hits the same issues mentioned previously.
+if (configuration.dwarf_version <= 4 or
+self.getDebugInfo() == 'dwarf'):
+self.expect_expr("func()", result_type="int", result_value="2")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")


Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -164,7 +164,11 @@
 
 # Continue to BP_file_scope at file scope
 self.runToBkpt("continue")
-self.expect_expr("func()", result_type="int", result_value="2")
+# FIXME: In DWARF 5 with dsyms, the ordering of functions is slightly
+# different, which also hits the same issues mentioned previously.
+if (configuration.dwarf_version <= 4 or
+self.getDebugInfo() == 'dwarf'):
+self.expect_expr("func()", result_type="int", result_value="2")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D154757: [lldb][NFCI] TestEmulation should take a Stream ref

2023-07-10 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve accepted this revision.
fdeazeve added a comment.

Thanks for finding those!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154757

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


[Lldb-commits] [PATCH] D154823: [lldb][AArch64] Add test predicate for systems with SME enabled

2023-07-10 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

"sme" is just one of many cpuinfo features for SME but it's the
only one we need for testing.

The rest are related to the use of certain instructions and
don't change the register state available.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154823

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1271,6 +1271,9 @@
 def isAArch64SVE(self):
 return self.isAArch64() and "sve" in self.getCPUInfo()
 
+def isAArch64SME(self):
+return self.isAArch64() and "sme" in self.getCPUInfo()
+
 def isAArch64MTE(self):
 return self.isAArch64() and "mte" in self.getCPUInfo()
 


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1271,6 +1271,9 @@
 def isAArch64SVE(self):
 return self.isAArch64() and "sve" in self.getCPUInfo()
 
+def isAArch64SME(self):
+return self.isAArch64() and "sme" in self.getCPUInfo()
+
 def isAArch64MTE(self):
 return self.isAArch64() and "mte" in self.getCPUInfo()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D154757: [lldb][NFCI] TestEmulation should take a Stream ref

2023-07-10 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

LGTM. Always happy to see never-null pointers get updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154757

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