[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

2023-04-27 Thread LU Hongyi via Phabricator via lldb-commits 
jwnhy added a comment.

Ping!


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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

2023-04-27 Thread LU Hongyi via Phabricator via lldb-commits 
jwnhy added a comment.

Ping!


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

https://reviews.llvm.org/D147606

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

2023-04-27 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 added a comment.

In D143347#4298613 , @kpdev42 wrote:

> So what are next steps? Are we going for implementation of 
> `DW_AT_no_unique_address` (which is going to be a non-standard extension) ? 
> @dblaikie @aprantl @Michael137

in my opinion that would be the most convenient way forward. Perhaps something 
like `DW_AT_LLVM_no_unique_address` even?

Btw what does GCC/gdb do in this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

2023-04-27 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 accepted this revision.
Michael137 added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:3
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \

other tests use `llvm-mc` as the assembler
E.g., `llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s`

I think it'll marginally speed things up



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:6
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:

Can you paste the source code here which you used to derive this test case? For 
future readers


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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

2023-04-27 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

This was added without a test.  Can you add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149312

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

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

2023-04-27 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added a comment.

Sure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149312

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

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

2023-04-27 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

This also seems a bit Quixotic to me.  Unless you are also planning to remove 
the `script` command, there's nothing you can do in the command line that you 
can't do with the SB API so you aren't actually removing any functionality, 
just making it slightly less convenient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149312

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

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

2023-04-27 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

In D149312#4302449 , @wallace wrote:

> Sure

You can probably just add something to TestCommandDelete.py.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149312

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

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

2023-04-27 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added inline comments.



Comment at: lldb/source/Expression/LLVMUserExpression.cpp:339
+  if (stack_frame_size == 0)
+stack_frame_size = arch == llvm::Triple::msp430 ? 512 : 512 * 1024;
 

kuilpd wrote:
> bulbazord wrote:
> > jingham wrote:
> > > This doesn't seem appropriate in generic code.  You should ask the Arch 
> > > plugin for this.
> > Oh, I missed this in the previous patch. Yeah, we shouldn't be hardcoding 
> > `llvm::Triple::msp430` in generic code. You're introducing even more uses 
> > of this in this patch, we should abstract this out into a plugin like Jim 
> > suggested.
> Do you mean creating a new MSP430 plugin in 
> lldb/source/Plugins/Architecture/, or accessing the existing ABI plugin? 
> Should I create new methods for getting stack frame size in each existing 
> plugin?
You could probably put this in the ABI plugin? The base ABI class could have a 
virtual method like `GetStackFrameSize` that returns `512 * 1024` and the 
MSP430 subclass can override it and return `512` instead. That should work, I 
think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149262

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

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

2023-04-27 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:89-91
+  dsym_file.append(".dSYM");
+  llvm::sys::path::append(dsym_file, path_style, "Contents", "Resources",
+  "DWARF", filename);

Why do you need the first call to `append` ?



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:102
 
-// See if we have "../CF.framework" - so we'll look for
-// CF.framework.dSYM/Contents/Resources/DWARF/CF
-// We need to drop the last suffix after '.' to match
-// 'CF' in the DWARF subdir.
-std::string binary_name(filename.AsCString());
-auto last_dot = binary_name.find_last_of('.');
-if (last_dot != std::string::npos) {
-  binary_name.erase(last_dot);
-  dsym_fspec = dsym_directory;
-  dsym_fspec.AppendPathComponent(binary_name);
-  if (FileSystem::Instance().Exists(dsym_fspec) &&
-  FileAtPathContainsArchAndUUID(dsym_fspec,
-mod_spec.GetArchitecturePtr(),
-mod_spec.GetUUIDPtr())) {
-return true;
+  // See if we have ".../CF.framework" - so we'll look for
+  // CF.framework.dSYM/Contents/Resources/DWARF/CF

Is this extra dot on purpose ?



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:105-108
+  if (filename.endswith(".framework")) {
+const auto last_dot = dsym_file.find_last_of('.');
+if (last_dot != llvm::StringRef::npos) {
+  dsym_file.truncate(last_dot - 1);

This part is a bit confusing ... may be a comment would help understand what 
we're trying to achieve here.



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:175-178
+llvm::StringRef filename =
+llvm::sys::path::filename(parent_path, path_style);
+if (filename.find('.') == llvm::StringRef::npos)
+  continue;

Same here ... this could use a comment to explain what we're doing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149096

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

[Lldb-commits] [PATCH] D149379: [lldb] Add tests for command removal

2023-04-27 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
wallace added a reviewer: jingham.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This adds some tests for removing lldb commands, user command and aliases. I've 
done in the SB API side for comleteness of this features. Besides that, I found 
a bug in the creation of the DummyCommand used in the test file: they were 
being allocated in the stack but they should have been allocated in the heap 
because CommandPluginInterfaceImplementation ends up owning the pointer passed 
to it, which is internally managed by a shared_ptr.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149379

Files:
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/API/SBCommandInterpreterTest.cpp

Index: lldb/unittests/API/SBCommandInterpreterTest.cpp
===
--- lldb/unittests/API/SBCommandInterpreterTest.cpp
+++ lldb/unittests/API/SBCommandInterpreterTest.cpp
@@ -44,15 +44,41 @@
   std::string m_message;
 };
 
+TEST_F(SBCommandInterpreterTest, RemoveCommandsAndAliases) {
+  // We first test removing non-existent commands
+  EXPECT_FALSE(m_interp.RemoveUserCommand("non-existent-user-command"));
+  EXPECT_FALSE(m_interp.RemoveCommand("non-existent-command"));
+  EXPECT_FALSE(m_interp.RemoveAlias("non-existent-alias"));
+
+  // We now test removing a built-in command
+  EXPECT_TRUE(m_interp.CommandExists("target"));
+  EXPECT_FALSE(m_interp.RemoveCommand("target"));
+  // Built-in commands need to be force removed.
+  EXPECT_TRUE(m_interp.RemoveCommand("target", /*force=*/true));
+  EXPECT_FALSE(m_interp.CommandExists("target"));
+
+  // Finally we test with a user defined command
+  m_interp.AddCommand("dummy", new DummyCommand("A dummy command"),
+  /*help=*/nullptr);
+  EXPECT_TRUE(m_interp.UserCommandExists("dummy"));
+  EXPECT_TRUE(m_interp.RemoveUserCommand("dummy"));
+  EXPECT_FALSE(m_interp.UserCommandExists("dummy"));
+
+  // Now we remove an alias
+  EXPECT_TRUE(m_interp.AliasExists("b"));
+  EXPECT_TRUE(m_interp.RemoveAlias("b"));
+  EXPECT_FALSE(m_interp.AliasExists("b"));
+}
+
 TEST_F(SBCommandInterpreterTest, SingleWordCommand) {
   // We first test a command without autorepeat
-  DummyCommand dummy("It worked");
-  m_interp.AddCommand("dummy", &dummy, /*help=*/nullptr);
+  m_interp.AddCommand("dummy", new DummyCommand("Dummy command"),
+  /*help=*/nullptr);
   {
 SBCommandReturnObject result;
 m_interp.HandleCommand("dummy", result, /*add_to_history=*/true);
 EXPECT_TRUE(result.Succeeded());
-EXPECT_STREQ(result.GetOutput(), "It worked\n");
+EXPECT_STREQ(result.GetOutput(), "Dummy command\n");
   }
   {
 SBCommandReturnObject result;
@@ -62,34 +88,36 @@
   }
 
   // Now we test a command with autorepeat
-  m_interp.AddCommand("dummy_with_autorepeat", &dummy, /*help=*/nullptr,
+  m_interp.AddCommand("dummy_with_autorepeat",
+  new DummyCommand("Command with autorepeat"),
+  /*help=*/nullptr,
   /*syntax=*/nullptr, /*auto_repeat_command=*/nullptr);
   {
 SBCommandReturnObject result;
 m_interp.HandleCommand("dummy_with_autorepeat", result,
/*add_to_history=*/true);
 EXPECT_TRUE(result.Succeeded());
-EXPECT_STREQ(result.GetOutput(), "It worked\n");
+EXPECT_STREQ(result.GetOutput(), "Command with autorepeat\n");
   }
   {
 SBCommandReturnObject result;
 m_interp.HandleCommand("", result);
 EXPECT_TRUE(result.Succeeded());
-EXPECT_STREQ(result.GetOutput(), "It worked\n");
+EXPECT_STREQ(result.GetOutput(), "Command with autorepeat\n");
   }
 }
 
 TEST_F(SBCommandInterpreterTest, MultiWordCommand) {
   auto command = m_interp.AddMultiwordCommand("multicommand", /*help=*/nullptr);
   // We first test a subcommand without autorepeat
-  DummyCommand subcommand("It worked again");
-  command.AddCommand("subcommand", &subcommand, /*help=*/nullptr);
+  command.AddCommand("subcommand", new DummyCommand("Dummy command"),
+ /*help=*/nullptr);
   {
 SBCommandReturnObject result;
 m_interp.HandleCommand("multicommand subcommand", result,
/*add_to_history=*/true);
 EXPECT_TRUE(result.Succeeded());
-EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+EXPECT_STREQ(result.GetOutput(), "Dummy command\n");
   }
   {
 SBCommandReturnObject result;
@@ -99,7 +127,8 @@
   }
 
   // We first test a subcommand with autorepeat
-  command.AddCommand("subcommand_with_autorepeat", &subcommand,
+  command.AddCommand("subcommand_with_autorepeat",
+ new DummyCommand("Dummy command with autorepeat"),
  /*help=*/nullptr, /*syntax=*/nullptr,
  /*auto_repeat_command=*/nullptr);

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

2023-04-27 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added a comment.

-> https://reviews.llvm.org/D149379


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149312

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

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

2023-04-27 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added inline comments.



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:89-91
+  dsym_file.append(".dSYM");
+  llvm::sys::path::append(dsym_file, path_style, "Contents", "Resources",
+  "DWARF", filename);

mib wrote:
> Why do you need the first call to `append` ?
The code suggestion you've given doesn't work for 2 reasons:
- `llvm::sys::path::append` only takes 4 Twine arguments, so this won't compile.
- `llvm::sys::path::append` appends things as if they were paths. So for 
`/bin/ls` you'll end up with `/bin/ls/.dSYM/Contents/Resources/DWARF` instead 
of `/bin/ls.dSYM/Contents/Resources/DWARF`.



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:102
 
-// See if we have "../CF.framework" - so we'll look for
-// CF.framework.dSYM/Contents/Resources/DWARF/CF
-// We need to drop the last suffix after '.' to match
-// 'CF' in the DWARF subdir.
-std::string binary_name(filename.AsCString());
-auto last_dot = binary_name.find_last_of('.');
-if (last_dot != std::string::npos) {
-  binary_name.erase(last_dot);
-  dsym_fspec = dsym_directory;
-  dsym_fspec.AppendPathComponent(binary_name);
-  if (FileSystem::Instance().Exists(dsym_fspec) &&
-  FileAtPathContainsArchAndUUID(dsym_fspec,
-mod_spec.GetArchitecturePtr(),
-mod_spec.GetUUIDPtr())) {
-return true;
+  // See if we have ".../CF.framework" - so we'll look for
+  // CF.framework.dSYM/Contents/Resources/DWARF/CF

mib wrote:
> Is this extra dot on purpose ?
I wanted to distinguish between `$parent_dir/CF.framework` because what is the 
parent_dir in this case? I used 3 dots to indicate `$some_path/CF.framework`.



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:105-108
+  if (filename.endswith(".framework")) {
+const auto last_dot = dsym_file.find_last_of('.');
+if (last_dot != llvm::StringRef::npos) {
+  dsym_file.truncate(last_dot - 1);

mib wrote:
> This part is a bit confusing ... may be a comment would help understand what 
> we're trying to achieve here.
I'll add more to the comment.



Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:175-178
+llvm::StringRef filename =
+llvm::sys::path::filename(parent_path, path_style);
+if (filename.find('.') == llvm::StringRef::npos)
+  continue;

mib wrote:
> Same here ... this could use a comment to explain what we're doing.
Will add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149096

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

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

2023-04-27 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord updated this revision to Diff 517725.
bulbazord added a comment.

Address @mib's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149096

Files:
  lldb/source/Symbol/LocateSymbolFile.cpp

Index: lldb/source/Symbol/LocateSymbolFile.cpp
===
--- lldb/source/Symbol/LocateSymbolFile.cpp
+++ lldb/source/Symbol/LocateSymbolFile.cpp
@@ -80,58 +80,50 @@
 // expanded/uncompressed dSYM and return that filepath in dsym_fspec.
 
 static bool LookForDsymNextToExecutablePath(const ModuleSpec &mod_spec,
-const FileSpec &exec_fspec,
+llvm::StringRef executable_path,
+llvm::sys::path::Style path_style,
 FileSpec &dsym_fspec) {
-  ConstString filename = exec_fspec.GetFilename();
-  FileSpec dsym_directory = exec_fspec;
-  dsym_directory.RemoveLastPathComponent();
-
-  std::string dsym_filename = filename.AsCString();
-  dsym_filename += ".dSYM";
-  dsym_directory.AppendPathComponent(dsym_filename);
-  dsym_directory.AppendPathComponent("Contents");
-  dsym_directory.AppendPathComponent("Resources");
-  dsym_directory.AppendPathComponent("DWARF");
-
-  if (FileSystem::Instance().Exists(dsym_directory)) {
-
-// See if the binary name exists in the dSYM DWARF
-// subdir.
-dsym_fspec = dsym_directory;
-dsym_fspec.AppendPathComponent(filename.AsCString());
-if (FileSystem::Instance().Exists(dsym_fspec) &&
-FileAtPathContainsArchAndUUID(dsym_fspec, mod_spec.GetArchitecturePtr(),
+  llvm::StringRef filename = llvm::sys::path::filename(executable_path, path_style);
+  llvm::SmallString<64> dsym_file = executable_path;
+
+  dsym_file.append(".dSYM");
+  llvm::sys::path::append(dsym_file, path_style, "Contents", "Resources",
+  "DWARF", filename);
+
+  // First, see if the binary name exists in the dSYM DWARF subdir
+  if (FileSystem::Instance().Exists(dsym_file)) {
+dsym_fspec.SetFile(dsym_file, path_style);
+if (FileAtPathContainsArchAndUUID(dsym_fspec, mod_spec.GetArchitecturePtr(),
   mod_spec.GetUUIDPtr())) {
   return true;
 }
+  }
 
-// See if we have "../CF.framework" - so we'll look for
-// CF.framework.dSYM/Contents/Resources/DWARF/CF
-// We need to drop the last suffix after '.' to match
-// 'CF' in the DWARF subdir.
-std::string binary_name(filename.AsCString());
-auto last_dot = binary_name.find_last_of('.');
-if (last_dot != std::string::npos) {
-  binary_name.erase(last_dot);
-  dsym_fspec = dsym_directory;
-  dsym_fspec.AppendPathComponent(binary_name);
-  if (FileSystem::Instance().Exists(dsym_fspec) &&
-  FileAtPathContainsArchAndUUID(dsym_fspec,
-mod_spec.GetArchitecturePtr(),
-mod_spec.GetUUIDPtr())) {
-return true;
+  // If the "executable path" is actually a framework bundle, e.g.
+  // /CF.framework, the dSYM's DWARF file may not have the `.framework`
+  // suffix (e.g. /CF.framework.dSYM/Contents/Resources/DWARF/CF).
+  // Given that `dsym_file` already will look like
+  // `/CF.framework.dSYM/Contents/Resources/DWARF/CF.framework` we an
+  // just drop the last `.framework` and check again.
+  if (filename.endswith(".framework")) {
+const auto last_dot = dsym_file.find_last_of('.');
+if (last_dot != llvm::StringRef::npos) {
+  dsym_file.truncate(last_dot - 1); // last_dot - 1 to include the dot
+  if (FileSystem::Instance().Exists(dsym_file)) {
+dsym_fspec.SetFile(dsym_file, path_style);
+if (FileAtPathContainsArchAndUUID(dsym_fspec,
+  mod_spec.GetArchitecturePtr(),
+  mod_spec.GetUUIDPtr())) {
+  return true;
+}
   }
 }
   }
 
   // See if we have a .dSYM.yaa next to this executable path.
-  FileSpec dsym_yaa_fspec = exec_fspec;
-  dsym_yaa_fspec.RemoveLastPathComponent();
-  std::string dsym_yaa_filename = filename.AsCString();
-  dsym_yaa_filename += ".dSYM.yaa";
-  dsym_yaa_fspec.AppendPathComponent(dsym_yaa_filename);
-
-  if (FileSystem::Instance().Exists(dsym_yaa_fspec)) {
+  dsym_file = executable_path;
+  dsym_file.append(".dSYM.yaa");
+  if (FileSystem::Instance().Exists(dsym_file)) {
 ModuleSpec mutable_mod_spec = mod_spec;
 Status error;
 if (Symbols::DownloadObjectAndSymbolFile(mutable_mod_spec, error, true) &&
@@ -160,52 +152,45 @@
   FileSpec &dsym_fspec) {
   Log *log = GetLog(LLDBLog::Host);
   const FileSpec &exec_fspec = module_spec.GetFileSpec();
-  if (exec_fspec) {
-if (::LookForDsymNextToExecutablePath(module_spec, ex

[Lldb-commits] [lldb] be5f35e - lldb: Fix usage of sve functions on arm64

2023-04-27 Thread Manoj Gupta via lldb-commits 

Author: Manoj Gupta
Date: 2023-04-27T14:59:49-07:00
New Revision: be5f35e24f4c15caf3c4aeccddc54c52560c28a0

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

LOG: lldb: Fix usage of sve functions on arm64

Use correct internal sve functions for arm64.
Otherwise, when cross-compling lld for AArch64 there are build
errors like:
NativeRegisterContextLinux_arm64.cpp:936:11:
   error: use of undeclared identifier 'sve_vl_valid
NativeRegisterContextLinux_arm64.cpp:63:28:
error: variable has incomplete type 'struct user_sve_header'

Reviewed By: omjavaid

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index c57b9499d5247..0ec152f0643a4 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -61,7 +61,7 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
   case llvm::Triple::aarch64: {
 // Configure register sets supported by this AArch64 target.
 // Read SVE header to check for SVE support.
-struct user_sve_header sve_header;
+struct sve::user_sve_header sve_header;
 struct iovec ioVec;
 ioVec.iov_base = &sve_header;
 ioVec.iov_len = sizeof(sve_header);
@@ -380,7 +380,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
   if (GetRegisterInfo().IsSVERegVG(reg)) {
 uint64_t vg_value = reg_value.GetAsUInt64();
 
-if (sve_vl_valid(vg_value * 8)) {
+if (sve::vl_valid(vg_value * 8)) {
   if (m_sve_header_is_valid && vg_value == GetSVERegVG())
 return error;
 
@@ -566,7 +566,7 @@ Status 
NativeRegisterContextLinux_arm64::WriteAllRegisterValues(
   if (contains_sve_reg_data) {
 // We have SVE register data first write SVE header.
 ::memcpy(GetSVEHeader(), src, GetSVEHeaderSize());
-if (!sve_vl_valid(m_sve_header.vl)) {
+if (!sve::vl_valid(m_sve_header.vl)) {
   m_sve_header_is_valid = false;
   error.SetErrorStringWithFormat("NativeRegisterContextLinux_arm64::%s "
  "Invalid SVE header in data_sp",
@@ -934,7 +934,7 @@ void 
NativeRegisterContextLinux_arm64::ConfigureRegisterContext() {
   // On every stop we configure SVE vector length by calling
   // ConfigureVectorLength regardless of current SVEState of this thread.
   uint32_t vq = RegisterInfoPOSIX_arm64::eVectorQuadwordAArch64SVE;
-  if (sve_vl_valid(m_sve_header.vl))
+  if (sve::vl_valid(m_sve_header.vl))
 vq = sve::vq_from_vl(m_sve_header.vl);
 
   GetRegisterInfo().ConfigureVectorLength(vq);



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

[Lldb-commits] [PATCH] D148752: lldb: Fix usage of sve functions on arm64

2023-04-27 Thread Manoj Gupta via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe5f35e24f4c: lldb: Fix usage of sve functions on arm64 
(authored by manojgupta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148752

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -61,7 +61,7 @@
   case llvm::Triple::aarch64: {
 // Configure register sets supported by this AArch64 target.
 // Read SVE header to check for SVE support.
-struct user_sve_header sve_header;
+struct sve::user_sve_header sve_header;
 struct iovec ioVec;
 ioVec.iov_base = &sve_header;
 ioVec.iov_len = sizeof(sve_header);
@@ -380,7 +380,7 @@
   if (GetRegisterInfo().IsSVERegVG(reg)) {
 uint64_t vg_value = reg_value.GetAsUInt64();
 
-if (sve_vl_valid(vg_value * 8)) {
+if (sve::vl_valid(vg_value * 8)) {
   if (m_sve_header_is_valid && vg_value == GetSVERegVG())
 return error;
 
@@ -566,7 +566,7 @@
   if (contains_sve_reg_data) {
 // We have SVE register data first write SVE header.
 ::memcpy(GetSVEHeader(), src, GetSVEHeaderSize());
-if (!sve_vl_valid(m_sve_header.vl)) {
+if (!sve::vl_valid(m_sve_header.vl)) {
   m_sve_header_is_valid = false;
   error.SetErrorStringWithFormat("NativeRegisterContextLinux_arm64::%s "
  "Invalid SVE header in data_sp",
@@ -934,7 +934,7 @@
   // On every stop we configure SVE vector length by calling
   // ConfigureVectorLength regardless of current SVEState of this thread.
   uint32_t vq = RegisterInfoPOSIX_arm64::eVectorQuadwordAArch64SVE;
-  if (sve_vl_valid(m_sve_header.vl))
+  if (sve::vl_valid(m_sve_header.vl))
 vq = sve::vq_from_vl(m_sve_header.vl);
 
   GetRegisterInfo().ConfigureVectorLength(vq);


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -61,7 +61,7 @@
   case llvm::Triple::aarch64: {
 // Configure register sets supported by this AArch64 target.
 // Read SVE header to check for SVE support.
-struct user_sve_header sve_header;
+struct sve::user_sve_header sve_header;
 struct iovec ioVec;
 ioVec.iov_base = &sve_header;
 ioVec.iov_len = sizeof(sve_header);
@@ -380,7 +380,7 @@
   if (GetRegisterInfo().IsSVERegVG(reg)) {
 uint64_t vg_value = reg_value.GetAsUInt64();
 
-if (sve_vl_valid(vg_value * 8)) {
+if (sve::vl_valid(vg_value * 8)) {
   if (m_sve_header_is_valid && vg_value == GetSVERegVG())
 return error;
 
@@ -566,7 +566,7 @@
   if (contains_sve_reg_data) {
 // We have SVE register data first write SVE header.
 ::memcpy(GetSVEHeader(), src, GetSVEHeaderSize());
-if (!sve_vl_valid(m_sve_header.vl)) {
+if (!sve::vl_valid(m_sve_header.vl)) {
   m_sve_header_is_valid = false;
   error.SetErrorStringWithFormat("NativeRegisterContextLinux_arm64::%s "
  "Invalid SVE header in data_sp",
@@ -934,7 +934,7 @@
   // On every stop we configure SVE vector length by calling
   // ConfigureVectorLength regardless of current SVEState of this thread.
   uint32_t vq = RegisterInfoPOSIX_arm64::eVectorQuadwordAArch64SVE;
-  if (sve_vl_valid(m_sve_header.vl))
+  if (sve::vl_valid(m_sve_header.vl))
 vq = sve::vq_from_vl(m_sve_header.vl);
 
   GetRegisterInfo().ConfigureVectorLength(vq);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D148752: lldb: Fix usage of sve functions on arm64

2023-04-27 Thread Manoj Gupta via Phabricator via lldb-commits 
manojgupta added a comment.

Thanks, regarding sve from Linux headers, LLVM currently supports building with 
GCC  7.1 or clang 5.0 and both were released in 2017. SVE support in Linux 
kernel believe  was added in Linux kernel 4.19 timeframe iso maybe late 2018. 
If SVE needs to be built from recent headers as a pre-requisite, then the Linux 
headers version requirement must also be added to llvm docs at 
https://llvm.org/docs/GettingStarted.html.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148752

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

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

2023-04-27 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

Thanks! LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149096

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

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

2023-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

IMHO this is optimizing the wrong thing. If FileSpec is slow, then we should 
try to make that faster, instead of updating the different places it's used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149096

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

[Lldb-commits] [PATCH] D149394: Finish the job of D145569

2023-04-27 Thread Jim Ingham via Phabricator via lldb-commits 
jingham created this revision.
jingham added reviewers: Michael137, JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The  InstrumentationRuntime's defined a data structure to gather report data, 
and repeated the pattern of defining it like:

struct data {
};
data t = {0};

That causes conflicts against other types called data, and could cause the 
expression not to compile.  Michael fixed this for some of the sanitizers in:

https://reviews.llvm.org/D145569

but neglected to do it for the thread sanitizer.

This patch adds the fix to that sanitizer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149394

Files:
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp


Index: 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -94,7 +94,7 @@
 const int REPORT_TRACE_SIZE = 128;
 const int REPORT_ARRAY_SIZE = 4;
 
-struct data {
+struct {
 void *report;
 const char *description;
 int report_count;


Index: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===
--- lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -94,7 +94,7 @@
 const int REPORT_TRACE_SIZE = 128;
 const int REPORT_ARRAY_SIZE = 4;
 
-struct data {
+struct {
 void *report;
 const char *description;
 int report_count;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149394: Finish the job of D145569

2023-04-27 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 accepted this revision.
Michael137 added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149394

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

[Lldb-commits] [lldb] 47f72ae - Make the TSan report capture data structure anonymous.

2023-04-27 Thread Jim Ingham via lldb-commits 

Author: Jim Ingham
Date: 2023-04-27T17:36:31-07:00
New Revision: 47f72aede163348ee474be4a3004dc0a9195fa9c

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

LOG: Make the TSan report capture data structure anonymous.

This was using `struct data` which is way to common a name to use in
an lldb expression, and was causing occasional failures in the
TSan report gatherer.  The structure doesn't need to have a tag,
so remove it to avoid future problems.

The same job was done for the other sanitizers in D145569, but this
one was overlooked.

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

Added: 


Modified: 

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
index cebe8d844160..946e4b356a36 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -94,7 +94,7 @@ const char *thread_sanitizer_retrieve_report_data_command = 
R"(
 const int REPORT_TRACE_SIZE = 128;
 const int REPORT_ARRAY_SIZE = 4;
 
-struct data {
+struct {
 void *report;
 const char *description;
 int report_count;



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

[Lldb-commits] [PATCH] D149394: Finish the job of D145569

2023-04-27 Thread Jim Ingham via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG47f72aede163: Make the TSan report capture data structure 
anonymous. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149394

Files:
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp


Index: 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -94,7 +94,7 @@
 const int REPORT_TRACE_SIZE = 128;
 const int REPORT_ARRAY_SIZE = 4;
 
-struct data {
+struct {
 void *report;
 const char *description;
 int report_count;


Index: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===
--- lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -94,7 +94,7 @@
 const int REPORT_TRACE_SIZE = 128;
 const int REPORT_ARRAY_SIZE = 4;
 
-struct data {
+struct {
 void *report;
 const char *description;
 int report_count;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

2023-04-27 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd created this revision.
compnerd added reviewers: JDevlieghere, kastiglione, aprantl.
compnerd added a project: LLDB.
Herald added a project: All.
compnerd requested review of this revision.

This generalises the `GetXcodeSDKPath` hook to a `GetSDKRoot` path which
will be re-used for the Windows support to compute a language specific
SDK path on the platform.  Because there may be other options that we
wish to use to compute the SDK path, sink the `XcodeSDK` parameter into
a structure which can pass a disaggregated set of options.  Furthermore,
optionalise the parameter as Xcode is not available for all platforms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149397

Files:
  lldb/include/lldb/Host/HostInfoBase.h
  lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
  lldb/source/Core/Module.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/unittests/Host/HostInfoTest.cpp

Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -57,7 +57,7 @@
 #if defined(__APPLE__)
 TEST_F(HostInfoTest, GetXcodeSDK) {
   auto get_sdk = [](std::string sdk, bool error = false) -> llvm::StringRef {
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+auto sdk_path_or_err = HostInfo::GetSDKRoot(SDKOptions{XcodeSDK(std::move(sdk))});
 if (!error) {
   EXPECT_TRUE((bool)sdk_path_or_err);
   return *sdk_path_or_err;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -124,7 +124,7 @@
   }
 
   // Use the default SDK as a fallback.
-  auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+  auto sdk_path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{XcodeSDK::GetAnyMacOS()});
   if (!sdk_path_or_err) {
 Debugger::ReportError("Error while searching for Xcode SDK: " +
   toString(sdk_path_or_err.takeError()));
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -284,7 +284,7 @@
   std::string secondary) {
   llvm::StringRef sdk;
   auto get_sdk = [&](std::string sdk) -> llvm::StringRef {
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+auto sdk_path_or_err = HostInfo::GetSDKRoot(SDKOptions{XcodeSDK(std::move(sdk))});
 if (!sdk_path_or_err) {
   Debugger::ReportError("Error while searching for Xcode SDK: " +
 toString(sdk_path_or_err.takeError()));
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -338,7 +338,7 @@
   }
 }
 
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+auto sdk_path_or_err = HostInfo::GetSDKRoot(SDKOptions{XcodeSDK::GetAnyMacOS()});
 if (!sdk_path_or_err) {
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOGF(log, "Error while searching for Xcode SDK: %s",
@@ -519,7 +519,7 @@
   return path;
 }
 
-llvm::Expected HostInfoMacOSX::GetXcodeSDKPath(XcodeSDK sdk) {
+llvm::Expected HostInfoMacOSX::GetSDKRoot(SDKOptions options) {
   struct ErrorOrPath {
 std::string str;
 bool is_error;
@@ -539,7 +539,7 @@
 else
   return it->second.str;
   }
-  auto path_or_err = GetXcodeSDK(sdk);
+  auto path_or_err = GetXcodeSDK(options.XcodeSDK);
   if (!path_or_err) {
 std::string error = toString(path_or_err.takeError());
 g_sdk_path.insert({key, {error, true}});
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1607,8 +1607,7 @@
 
 void Module::RegisterXcodeSDK(llvm::StringRef sdk_name,
   llvm::StringRef sysroot) {
-  XcodeSDK sdk(sdk_name.str());
-  auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(sdk);
+  auto sdk_path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk_name.str()});
 
   if (!sdk_path_or_err) {
 Debugger::ReportError("Error while searching for Xcode SDK: " +
Index: lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
===
--- lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
+++ lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
@@ -31,7 +31

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

2023-04-27 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. Thanks Saleem!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149397

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

2023-04-27 Thread LU Hongyi via Phabricator via lldb-commits 
jwnhy updated this revision to Diff 517784.
jwnhy marked an inline comment as done.
jwnhy added a comment.

Updated the patch to include the original code.

Could you please help me push it to the main codebase?, as I don't have the 
push right to the repo.


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

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
@@ -0,0 +1,468 @@
+  # Test handling of values represented via DW_OP_div
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:
+  # (uint32_t) $0 = 0
+  # CHECK: (uint32_t) $0 = 2
+	
+  # This case is generated from the following code:
+  # #include "stdint.h"
+  # static volatile uint64_t g = 0;
+  #  static const int32_t f()
+  #  {
+  #uint32_t i;
+  #for (i = 0; (i != 10); i++)
+  #  ++g;
+  #return 0;
+  #  
+  #  }
+  #
+  #  int main()
+  #  {
+  #f();
+  #return 0;
+  #
+  #  }
+
+  .text
+	.file	"signed_dw_op_div.c"
+	.file	1 "/usr/local/include/bits" "types.h" md5 0x96c0983c9cdaf387938a8268d00aa594
+	.file	2 "/usr/local/include/bits" "stdint-uintn.h" md5 0xbedfab747425222bb150968c14e40abd
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 13 0  # signed_dw_op_div.c:13:0
+	.cfi_startproc
+# %bb.0:
+	movl	$3, %eax
+	#DEBUG_VALUE: f:i <- 0
+	.p2align	4, 0x90
+.LBB0_1:# =>This Inner Loop Header: Depth=1
+.Ltmp0:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_stack_value] $eax
+	.loc	0 8 7 prologue_end  # signed_dw_op_div.c:8:7
+	incq	g(%rip)
+.Ltmp1:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_consts 1, DW_OP_plus, DW_OP_stack_value] $eax
+	.loc	0 7 20  # signed_dw_op_div.c:7:20
+	decl	%eax
+.Ltmp2:
+	.loc	0 7 5 is_stmt 0 # signed_dw_op_div.c:7:5
+	jne	.LBB0_1
+.Ltmp3:
+# %bb.2:
+	.loc	0 15 5 is_stmt 1# signed_dw_op_div.c:15:5
+	xorl	%eax, %eax
+	retq
+.Ltmp4:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+	.file	3 "/usr/local/include/bits" "stdint-intn.h" md5 0x90039fb90b44dcbf118222513050fe57
+# -- End function
+	.type	g,@object   # @g
+	.local	g
+	.comm	g,8,8
+	.section	.debug_loclists,"",@progbits
+	.long	.Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
+.Ldebug_list_header_start0:
+	.short	5   # Version
+	.byte	8   # Address size
+	.byte	0   # Segment selector size
+	.long	1   # Offset entry count
+.Lloclists_table_base0:
+	.long	.Ldebug_loc0-.Lloclists_table_base0
+.Ldebug_loc0:
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp0-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   ending offset
+	.byte	16  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 # 
+	.byte	255 # 
+	.byte	15  # 
+	.byte	26  # DW_OP_and
+	.byte	17  # DW_OP_consts
+	.byte	3   # 3
+	.byte	28  # DW_OP_minus
+	.byte	17  # DW_OP_consts
+	.byte	127 # -1
+	.byte	27  # DW_OP_div
+	.byte	159 # DW_OP_stack_value
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp2-.Lfunc_begin0   #   ending offset
+	.byte	19  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 # 
+	.byte	255 # 
+	.by

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

2023-04-27 Thread LU Hongyi via Phabricator via lldb-commits 
jwnhy marked an inline comment as done.
jwnhy added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:3
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \

Michael137 wrote:
> other tests use `llvm-mc` as the assembler
> E.g., `llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s`
> 
> I think it'll marginally speed things up
There are some issues on llvm-mc, it complains about .loc directives.
So I guess I will just go with Clang...



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:6
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:

Michael137 wrote:
> Can you paste the source code here which you used to derive this test case? 
> For future readers
Update the patch to include the original sources.


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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

2023-04-27 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added a comment.

@jingham, in my use case, I'm exposing a minified LLDB client remotely, so the 
user won't have access to script command nor the SB API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149312

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