[Lldb-commits] [lldb] 4faf000 - Recommit of 8ae18303f97d5dcfaecc90b4d87effb2011ed82e - part 2

2022-12-09 Thread via lldb-commits 

Author: serge-sans-paille
Date: 2022-12-09T10:07:02+01:00
New Revision: 4faf6cf989f3ae212912994022c0486a2dc4

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

LOG: Recommit of 8ae18303f97d5dcfaecc90b4d87effb2011ed82e - part 2

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
llvm/include/llvm/Option/OptTable.h
llvm/lib/Option/OptTable.cpp
llvm/unittests/Option/OptionMarshallingTest.cpp
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 4621850f13772..60d62e2b9c5c1 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -331,8 +331,8 @@ static bool getStaticPIE(const ArgList &Args, const 
ToolChain &TC) {
   if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
 const Driver &D = TC.getDriver();
 const llvm::opt::OptTable &Opts = D.getOpts();
-const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
-const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
+StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
 D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
   }
   return HasStaticPIE;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 9d89148616be1..fde098840be4b 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1055,7 +1055,7 @@ void 
PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
   // Only add the version-min options if we got a version from somewhere
   if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
 #define OPTION(PREFIX, NAME, VAR, ...) 
\
-  const char *opt_##VAR = NAME;
\
+  llvm::StringRef opt_##VAR = NAME;
\
   (void)opt_##VAR;
 #include "clang/Driver/Options.inc"
 #undef OPTION

diff  --git a/llvm/include/llvm/Option/OptTable.h 
b/llvm/include/llvm/Option/OptTable.h
index 07d9870f71b33..390462d762e61 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -102,9 +102,7 @@ class OptTable {
   const Option getOption(OptSpecifier Opt) const;
 
   /// Lookup the name of the given option.
-  const char *getOptionName(OptSpecifier id) const {
-return getInfo(id).Name;
-  }
+  StringRef getOptionName(OptSpecifier id) const { return getInfo(id).Name; }
 
   /// Get the kind of the given option.
   unsigned getOptionKind(OptSpecifier id) const {

diff  --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index ef4873eb7f9c4..f4426bc34b3f9 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -36,16 +36,10 @@ namespace opt {
 // Ordering on Info. The ordering is *almost* case-insensitive lexicographic,
 // with an exception. '\0' comes at the end of the alphabet instead of the
 // beginning (thus options precede any other options which prefix them).
-static int StrCmpOptionNameIgnoreCase(const char *A, const char *B) {
-  const char *X = A, *Y = B;
-  char a = tolower(*A), b = tolower(*B);
-  while (a == b) {
-if (a == '\0')
-  return 0;
-
-a = tolower(*++X);
-b = tolower(*++Y);
-  }
+static int StrCmpOptionNameIgnoreCase(StringRef A, StringRef B) {
+  size_t MinSize = std::min(A.size(), B.size());
+  if (int Res = A.substr(0, MinSize).compare_insensitive(B.substr(0, MinSize)))
+return Res;
 
   if (a == '\0') // A is a prefix of B.
 return 1;
@@ -60,7 +54,7 @@ static int StrCmpOptionNameIgnoreCase(const char *A, const 
char *B) {
 static int StrCmpOptionName(const char *A, const char *B) {
   if (int N = StrCmpOptionNameIgnoreCase(A, B))
 return N;
-  return strcmp(A, B);
+  return A.compare(B);
 }
 
 static inline bool operator<(const OptTable::Info &A, const OptTable::Info &B) 
{
@@ -186,7 +180,7 @@ static unsigned matchOption(const OptTable::Info *I, 
StringRef Str,
   bool Matched = IgnoreCase ? Rest.startswith_insensitive(I->Name)
 : Rest.startswith(I->Name);
   if (Matched)
-return Prefix.size() + StringRef(I->Name).size();
+return Prefix.size() + I->Name.size();
 }
   }
   return 0;
@@ -347,8 +341,8 @@ std::unique_ptr 
OptTable::parseOneArgGrouped(InputArgList &Args,
 
   const Info *End = OptionInfos.data() + OptionInfos.size();
   StringRef Name = St

[Lldb-commits] [lldb] d881fdf - Revert "Recommit of 8ae18303f97d5dcfaecc90b4d87effb2011ed82e - part 2"

2022-12-09 Thread via lldb-commits 

Author: serge-sans-paille
Date: 2022-12-09T10:15:41+01:00
New Revision: d881fdf72047fd18b88c6a65d0966cad542c95cd

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

LOG: Revert "Recommit of 8ae18303f97d5dcfaecc90b4d87effb2011ed82e - part 2"

This reverts commit 4faf6cf989f3ae212912994022c0486a2dc4.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
llvm/include/llvm/Option/OptTable.h
llvm/lib/Option/OptTable.cpp
llvm/unittests/Option/OptionMarshallingTest.cpp
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 60d62e2b9c5c1..4621850f13772 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -331,8 +331,8 @@ static bool getStaticPIE(const ArgList &Args, const 
ToolChain &TC) {
   if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
 const Driver &D = TC.getDriver();
 const llvm::opt::OptTable &Opts = D.getOpts();
-StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
-StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
+const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
 D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
   }
   return HasStaticPIE;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index fde098840be4b..9d89148616be1 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1055,7 +1055,7 @@ void 
PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
   // Only add the version-min options if we got a version from somewhere
   if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
 #define OPTION(PREFIX, NAME, VAR, ...) 
\
-  llvm::StringRef opt_##VAR = NAME;
\
+  const char *opt_##VAR = NAME;
\
   (void)opt_##VAR;
 #include "clang/Driver/Options.inc"
 #undef OPTION

diff  --git a/llvm/include/llvm/Option/OptTable.h 
b/llvm/include/llvm/Option/OptTable.h
index 390462d762e61..07d9870f71b33 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -102,7 +102,9 @@ class OptTable {
   const Option getOption(OptSpecifier Opt) const;
 
   /// Lookup the name of the given option.
-  StringRef getOptionName(OptSpecifier id) const { return getInfo(id).Name; }
+  const char *getOptionName(OptSpecifier id) const {
+return getInfo(id).Name;
+  }
 
   /// Get the kind of the given option.
   unsigned getOptionKind(OptSpecifier id) const {

diff  --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index f4426bc34b3f9..ef4873eb7f9c4 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -36,10 +36,16 @@ namespace opt {
 // Ordering on Info. The ordering is *almost* case-insensitive lexicographic,
 // with an exception. '\0' comes at the end of the alphabet instead of the
 // beginning (thus options precede any other options which prefix them).
-static int StrCmpOptionNameIgnoreCase(StringRef A, StringRef B) {
-  size_t MinSize = std::min(A.size(), B.size());
-  if (int Res = A.substr(0, MinSize).compare_insensitive(B.substr(0, MinSize)))
-return Res;
+static int StrCmpOptionNameIgnoreCase(const char *A, const char *B) {
+  const char *X = A, *Y = B;
+  char a = tolower(*A), b = tolower(*B);
+  while (a == b) {
+if (a == '\0')
+  return 0;
+
+a = tolower(*++X);
+b = tolower(*++Y);
+  }
 
   if (a == '\0') // A is a prefix of B.
 return 1;
@@ -54,7 +60,7 @@ static int StrCmpOptionNameIgnoreCase(StringRef A, StringRef 
B) {
 static int StrCmpOptionName(const char *A, const char *B) {
   if (int N = StrCmpOptionNameIgnoreCase(A, B))
 return N;
-  return A.compare(B);
+  return strcmp(A, B);
 }
 
 static inline bool operator<(const OptTable::Info &A, const OptTable::Info &B) 
{
@@ -180,7 +186,7 @@ static unsigned matchOption(const OptTable::Info *I, 
StringRef Str,
   bool Matched = IgnoreCase ? Rest.startswith_insensitive(I->Name)
 : Rest.startswith(I->Name);
   if (Matched)
-return Prefix.size() + I->Name.size();
+return Prefix.size() + StringRef(I->Name).size();
 }
   }
   return 0;
@@ -341,8 +347,8 @@ std::unique_ptr 
OptTable::parseOneArgGrouped(InputArgList &Args,
 
   const Info *End = OptionInfos.data() + OptionInfos.size();
   StringRe

[Lldb-commits] [lldb] 12a877a - Adapt lldb to use StringRef for option storage

2022-12-09 Thread via lldb-commits 

Author: serge-sans-paille
Date: 2022-12-09T11:25:26+01:00
New Revision: 12a877a32c73e5364ace453dad0ab7cb63a6506c

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

LOG: Adapt lldb to use StringRef for option storage

As a consequence to 138942c833b3baa12d19216797efca6d4dd010d2
This fixes lldb build https://lab.llvm.org/buildbot/#/builders/83/builds/26991

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 9d89148616be1..fde098840be4b 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1055,7 +1055,7 @@ void 
PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
   // Only add the version-min options if we got a version from somewhere
   if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
 #define OPTION(PREFIX, NAME, VAR, ...) 
\
-  const char *opt_##VAR = NAME;
\
+  llvm::StringRef opt_##VAR = NAME;
\
   (void)opt_##VAR;
 #include "clang/Driver/Options.inc"
 #undef OPTION



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

[Lldb-commits] [PATCH] D139674: Fix ValueObject::GetAddressOf for subclasses using ValueObjectConstResultImpl backends

2022-12-09 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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139674

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

[Lldb-commits] [PATCH] D139649: [lldb] Make ParseTemplateParameterInfos return false if there are no template params

2022-12-09 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1251
+  if (has_template_params &&
+  ParseTemplateParameterInfos(die, template_param_infos)) {
+template_function_decl = m_ast.CreateFunctionDeclaration(

aeubanks wrote:
> dblaikie wrote:
> > This part changes behavior, yeah? (previously the code was only conditional 
> > on `has_template_params` and is now conditional on both that and there 
> > actually being template parameter DIEs) Was that intended? If so, is there 
> > any testing that could be done to demonstrate the change in behavior?
> presumably any visible change would only happen with broken debug info
> 
> `has_template_params` is true if there are any template parameter child DIEs, 
> but now we do more checking that those DIEs are actually reasonable
> 
> I can revert this back if you want, but this seems better and I don't think 
> I'm going to find time to add a test for broken debug info which may or may 
> not actually hit this code path
Agree with David that if it's behaviour changing we should either test or leave 
as is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139649

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

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

2022-12-09 Thread John Ericson via Phabricator via lldb-commits 
Ericson2314 added inline comments.



Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
-  llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+  llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
   if (!llvm::sys::fs::exists(LibPath)) {

compnerd wrote:
> serge-sans-paille wrote:
> > Ericson2314 wrote:
> > > mgorny wrote:
> > > > Well, one thing I immediately notice is that technically 
> > > > `CMAKE_INSTALL_LIBDIR` can be an absolute path, while in many locations 
> > > > you are assuming it will always be relative. Not saying that's a 
> > > > blocker but I think you should add checks for that into 
> > > > `CMakeLists.txt`.
> > > Yes I was working on this, and I intend to use `CMAKE_INSTALL_LIBDIR` 
> > > with an absolute path.
> > > 
> > > OK if this isn't supported initially but we should ovoid regressing where 
> > > possible.
> > Turns out LLVM_LIBDIR_SUFFIX is used in several situations: (a) for the 
> > library install dir or (b) for the location where build artifact as stored 
> > in CMAKE_BINARY_DIR. (a) could accept a path but not (b), unless we derive 
> > it from (a) but I can see a lot of complication there for the testing step. 
> > Would that be ok to choke on CMAKE_INSTALL_LIBDIR being a path and not a 
> > directory component?
> > 
> I think that is absolutely reasonable.  There is the 
> `CMAKE_INSTALL_FULL_LIBDIR` which should be the relatively absolute path (it 
> is relative to `DESTDIR`).  The `CMAKE_INSTALL_LIBDIR` should be the relative 
> component which is added to `CMAKE_INSTALL_PREFIX`.
Please do not do this. In NixOS we uses absolute paths for these which are 
*not* within `CMAKE_INSTALL_PREFIX` and I would very much like that to continue 
to work, and have put a lot of effort into it.

(I am sorry I have been a bit AWAL on LLVM things in general but hopefully will 
have more time as we head into the new year.)


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

https://reviews.llvm.org/D137337

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

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-12-09 Thread Alex Lorenz via Phabricator via lldb-commits 
arphaman added a comment.

Ping @mizvekov.

Unfortunately I'm unable to revert this commit now so we won't be able to get 
the bot back to green until it's fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D139684: Switch the "command script add" interactive editor to use the exe_ctx interface

2022-12-09 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.

LGTM! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139684

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

[Lldb-commits] [PATCH] D139684: Switch the "command script add" interactive editor to use the exe_ctx interface

2022-12-09 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

Just to make sure I understand: this works because we already supported both 
variants, and Python picks up the correct one based on the function signature? 
If so then this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139684

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

[Lldb-commits] [PATCH] D139684: Switch the "command script add" interactive editor to use the exe_ctx interface

2022-12-09 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

In D139684#3984997 , @JDevlieghere 
wrote:

> Just to make sure I understand: this works because we already supported both 
> variants, and Python picks up the correct one based on the function 
> signature? If so then this LGTM.

Exactly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139684

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

[Lldb-commits] [lldb] 9c5877f - Switch the "command script add" interactive input to use the new command form.

2022-12-09 Thread Jim Ingham via lldb-commits 

Author: Jim Ingham
Date: 2022-12-09T10:58:15-08:00
New Revision: 9c5877f33d9f7fd317c07920fd24a041ad69077a

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

LOG: Switch the "command script add" interactive input to use the new command 
form.

We're suggesting people use the form of the command that takes an exe_ctx - it
is both more convenient and more correct - since you should not be using
GetSelected{Target, Process, etc.} in commands.

Added: 
lldb/test/Shell/Commands/command-script-add.test

Modified: 
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index 9bf07f3eba780..422291f1cd5a5 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -201,7 +201,7 @@ class CommandObjectCommandsSource : public 
CommandObjectParsed {
 static const char *g_python_command_instructions =
 "Enter your Python command(s). Type 'DONE' to end.\n"
 "You must define a Python function with this signature:\n"
-"def my_command_impl(debugger, args, result, internal_dict):\n";
+"def my_command_impl(debugger, args, exe_ctx, result, internal_dict):\n";
 
 class CommandObjectCommandsAlias : public CommandObjectRaw {
 protected:

diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 810c77925f5b7..117a057d8d7c5 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1369,7 +1369,7 @@ bool 
ScriptInterpreterPythonImpl::GenerateScriptAliasFunction(
   std::string auto_generated_function_name(GenerateUniqueName(
   "lldb_autogen_python_cmd_alias_func", num_created_functions));
 
-  sstr.Printf("def %s (debugger, args, result, internal_dict):",
+  sstr.Printf("def %s (debugger, args, exe_ctx, result, internal_dict):",
   auto_generated_function_name.c_str());
 
   if (!GenerateFunction(sstr.GetData(), user_input).Success())

diff  --git a/lldb/test/Shell/Commands/command-script-add.test 
b/lldb/test/Shell/Commands/command-script-add.test
new file mode 100644
index 0..5ffada069a610
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-script-add.test
@@ -0,0 +1,9 @@
+# Test that command script add with no arguments prompts for
+# and generates the modern (exe_ctx) version of the command.
+
+# RUN: %lldb < %s | FileCheck %s
+command script add doit
+print(exe_ctx.target)
+DONE
+doit
+# CHECK: No value



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

[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld

2022-12-09 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:307-308
+
+int addr_size =
+process->GetTarget().GetArchitecture().GetAddressByteSize();
 for (uint64_t i = 0; i < image_infos_count; i++) {

Nit: pretty sure `GetAddressByteSize()` returns an unsigned (`uint32_t` or 
`uint8_t`). 



Comment at: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:314
+  process->ReadPointerFromMemory(dyld_image_info, error);
+  if (error.Success()) {
 image_load_addresses.push_back(addr);

jingham wrote:
> Is it expected that this memory read fail?  It seems weird to just not handle 
> that case at all?  Maybe at least log something here, otherwise it looks like 
> we just get nothing w/o knowing why.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139453

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

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-12-09 Thread Matheus Izvekov via Phabricator via lldb-commits 
mizvekov added a comment.

In D131858#3984855 , @arphaman wrote:

> Ping @mizvekov.
>
> Unfortunately I'm unable to revert this commit now so we won't be able to get 
> the bot back to green until it's fixed.

Hello, sorry for the long time to respond.

Can you tell if this has been fixed by 
https://github.com/llvm/llvm-project/commit/303f20a2cab4554a10ec225fd853709146f8c51c
 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[Lldb-commits] [lldb] 3b7ac5b - Fix GetAddressOf for children of pointer ValueObjectConstResult* variables.

2022-12-09 Thread Jim Ingham via lldb-commits 

Author: Jim Ingham
Date: 2022-12-09T11:16:10-08:00
New Revision: 3b7ac5b295df7381cee277342085f52fe468e633

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

LOG: Fix GetAddressOf for children of pointer ValueObjectConstResult* variables.

The original code always set the m_live_address of children of the ValueObjects 
that
use ValueObjectConstResultImpl backends to the parent m_live_address + 
child_byte_offset.
That is correct for structure types, but wrong for pointer types, since 
m_live_address
for a pointer type is the address of the storage for the pointer, not of the 
pointee.

Also added a test which was failing before this patch.

Added: 
lldb/test/API/lang/c/parray_vrs_char_array/Makefile
lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
lldb/test/API/lang/c/parray_vrs_char_array/main.c

Modified: 
lldb/source/Core/ValueObjectConstResultImpl.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObjectConstResultImpl.cpp 
b/lldb/source/Core/ValueObjectConstResultImpl.cpp
index fee1da138bbc6..e2db3ace19247 100644
--- a/lldb/source/Core/ValueObjectConstResultImpl.cpp
+++ b/lldb/source/Core/ValueObjectConstResultImpl.cpp
@@ -89,13 +89,20 @@ ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(
 if (!child_name_str.empty())
   child_name.SetCString(child_name_str.c_str());
 
+lldb::addr_t child_live_addr = LLDB_INVALID_ADDRESS;
+// Transfer the live address (with offset) to the child.  But if
+// the parent is a pointer, the live address is where that pointer
+// value lives in memory, so the children live addresses aren't
+// offsets from that value, they are just other load addresses that
+// are recorded in the Value of the child ValueObjects.
+if (m_live_address != LLDB_INVALID_ADDRESS) {
+  if (!compiler_type.IsPointerType())
+child_live_addr = m_live_address + child_byte_offset;
+}
 valobj = new ValueObjectConstResultChild(
 *m_impl_backend, child_compiler_type, child_name, child_byte_size,
 child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
-child_is_base_class, child_is_deref_of_parent,
-m_live_address == LLDB_INVALID_ADDRESS
-? m_live_address
-: m_live_address + child_byte_offset,
+child_is_base_class, child_is_deref_of_parent, child_live_addr,
 language_flags);
   }
 

diff  --git a/lldb/test/API/lang/c/parray_vrs_char_array/Makefile 
b/lldb/test/API/lang/c/parray_vrs_char_array/Makefile
new file mode 100644
index 0..695335e068c0c
--- /dev/null
+++ b/lldb/test/API/lang/c/parray_vrs_char_array/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py 
b/lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
new file mode 100644
index 0..f1795516c74e1
--- /dev/null
+++ b/lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
@@ -0,0 +1,37 @@
+"""
+Test that parray of a struct with an embedded char array works.
+This was failing because the "live address" of the child elements
+was calculated incorrectly - as a offset from the pointer live 
+address.  It only happened for char[] children because they used
+GetAddressOf which relies on the live address.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestParrayVrsCharArrayChild(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_parray_struct_with_char_array_child(self):
+"""This is the basic test for does parray get the char values right."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.do_array_test()
+
+def do_array_test(self):
+(target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+   "Set a breakpoint here", 
self.main_source_file)
+
+frame = thread.GetFrameAtIndex(0)
+ 
+self.expect("expr -Z 3 -- struct_ptr",
+substrs = ['before = 112, var = "abcd", after = 221',
+   'before = 313, var = "efgh", after = 414',
+   'before = 515, var = "ijkl", after = 616'])
+
+

diff  --git a/lldb/test/API/lang/c/parray_vrs_char_array/main.c 
b/lldb/test/API/lang/c/parray_vrs_char_array/main.c
new file mode 100644
index 0..0571a6bc5f66d
--- /dev/null
+++ b/lldb/test/API/lang/c/parray_vrs_char_array/main.c
@@ -0,0 +1,15 @@
+struct MyStruct {
+  int before;
+  char var[5];
+  int after;
+};
+
+int
+main()
+{
+  struct MyStruct struct_arr[3] = {{112

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

2022-12-09 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd added inline comments.



Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
-  llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+  llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
   if (!llvm::sys::fs::exists(LibPath)) {

Ericson2314 wrote:
> compnerd wrote:
> > serge-sans-paille wrote:
> > > Ericson2314 wrote:
> > > > mgorny wrote:
> > > > > Well, one thing I immediately notice is that technically 
> > > > > `CMAKE_INSTALL_LIBDIR` can be an absolute path, while in many 
> > > > > locations you are assuming it will always be relative. Not saying 
> > > > > that's a blocker but I think you should add checks for that into 
> > > > > `CMakeLists.txt`.
> > > > Yes I was working on this, and I intend to use `CMAKE_INSTALL_LIBDIR` 
> > > > with an absolute path.
> > > > 
> > > > OK if this isn't supported initially but we should ovoid regressing 
> > > > where possible.
> > > Turns out LLVM_LIBDIR_SUFFIX is used in several situations: (a) for the 
> > > library install dir or (b) for the location where build artifact as 
> > > stored in CMAKE_BINARY_DIR. (a) could accept a path but not (b), unless 
> > > we derive it from (a) but I can see a lot of complication there for the 
> > > testing step. Would that be ok to choke on CMAKE_INSTALL_LIBDIR being a 
> > > path and not a directory component?
> > > 
> > I think that is absolutely reasonable.  There is the 
> > `CMAKE_INSTALL_FULL_LIBDIR` which should be the relatively absolute path 
> > (it is relative to `DESTDIR`).  The `CMAKE_INSTALL_LIBDIR` should be the 
> > relative component which is added to `CMAKE_INSTALL_PREFIX`.
> Please do not do this. In NixOS we uses absolute paths for these which are 
> *not* within `CMAKE_INSTALL_PREFIX` and I would very much like that to 
> continue to work, and have put a lot of effort into it.
> 
> (I am sorry I have been a bit AWAL on LLVM things in general but hopefully 
> will have more time as we head into the new year.)
Why can NixOS not use `CMAKE_INSTALL_FULL_LIBDIR`?  That is computed to 
`${CMAKE_INSTALL_PREFIX}${CMAKE_INSTALL_LIBDIR}` only if it is not already 
defined to an absolute path.


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

https://reviews.llvm.org/D137337

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

[Lldb-commits] [PATCH] D139649: [lldb] Make ParseTemplateParameterInfos return false if there are no template params

2022-12-09 Thread Arthur Eubanks via Phabricator via lldb-commits 
aeubanks updated this revision to Diff 481711.
aeubanks added a comment.

revert one change to make this nfc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139649

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -747,9 +747,7 @@
 
   clang::DeclContext *decl_ctx = GetClangDeclContextContainingDIE(die, 
nullptr);
   TypeSystemClang::TemplateParameterInfos template_param_infos;
-  if (ParseTemplateParameterInfos(die, template_param_infos) &&
-  (!template_param_infos.args.empty() ||
-   template_param_infos.packed_args)) {
+  if (ParseTemplateParameterInfos(die, template_param_infos)) {
 // Most of the parameters here don't matter, but we make sure the base name
 // is empty so when we print the name we only get the template parameters.
 clang::ClassTemplateDecl *class_template_decl =
@@ -1787,9 +1785,7 @@
 metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
 
 TypeSystemClang::TemplateParameterInfos template_param_infos;
-if (ParseTemplateParameterInfos(die, template_param_infos) &&
-(!template_param_infos.args.empty() ||
- template_param_infos.packed_args)) {
+if (ParseTemplateParameterInfos(die, template_param_infos)) {
   clang::ClassTemplateDecl *class_template_decl =
   m_ast.ParseClassTemplateDecl(
   decl_ctx, GetOwningClangModule(die), attrs.accessibility,
@@ -2123,7 +2119,10 @@
   break;
 }
   }
-  return template_param_infos.args.size() == template_param_infos.names.size();
+  return template_param_infos.args.size() ==
+ template_param_infos.names.size() &&
+ (!template_param_infos.args.empty() ||
+  template_param_infos.packed_args);
 }
 
 bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -747,9 +747,7 @@
 
   clang::DeclContext *decl_ctx = GetClangDeclContextContainingDIE(die, nullptr);
   TypeSystemClang::TemplateParameterInfos template_param_infos;
-  if (ParseTemplateParameterInfos(die, template_param_infos) &&
-  (!template_param_infos.args.empty() ||
-   template_param_infos.packed_args)) {
+  if (ParseTemplateParameterInfos(die, template_param_infos)) {
 // Most of the parameters here don't matter, but we make sure the base name
 // is empty so when we print the name we only get the template parameters.
 clang::ClassTemplateDecl *class_template_decl =
@@ -1787,9 +1785,7 @@
 metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
 
 TypeSystemClang::TemplateParameterInfos template_param_infos;
-if (ParseTemplateParameterInfos(die, template_param_infos) &&
-(!template_param_infos.args.empty() ||
- template_param_infos.packed_args)) {
+if (ParseTemplateParameterInfos(die, template_param_infos)) {
   clang::ClassTemplateDecl *class_template_decl =
   m_ast.ParseClassTemplateDecl(
   decl_ctx, GetOwningClangModule(die), attrs.accessibility,
@@ -2123,7 +2119,10 @@
   break;
 }
   }
-  return template_param_infos.args.size() == template_param_infos.names.size();
+  return template_param_infos.args.size() ==
+ template_param_infos.names.size() &&
+ (!template_param_infos.args.empty() ||
+  template_param_infos.packed_args);
 }
 
 bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

2022-12-09 Thread Alexander Yermolovich via Phabricator via lldb-commits 
ayermolo added a comment.

@dblaikie ping? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139379

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

[Lldb-commits] [PATCH] D139740: [lldb] Disable macro redefinition warnings in expression wrapper

2022-12-09 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor created this revision.
teemperor added a reviewer: Michael137.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
teemperor requested review of this revision.
Herald added a subscriber: lldb-commits.

GCC emits macro definitions into debug info when compiling with `-g3`.
LLDB is translating this information into `#define` directives which are 
injected into the
source code of user expressions. While this mechanism itself works fine, it can 
lead to
spurious "... macro redefined" warnings when the defined macro is also a 
builtin Clang macro:

  warning: :46:9: '__VERSION__' macro redefined
  #define __VERSION__ "12.1.0"
  ^
  :19:9: previous definition is here
  #define __VERSION__ "Clang 14.0.6"
  [repeated about a 100 more times for every builtin macro]

This patch just disables the diagnostic when parsing LLDB's generated list of
macros definitions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139740

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/test/API/commands/expression/macros/TestMacros.py


Index: lldb/test/API/commands/expression/macros/TestMacros.py
===
--- lldb/test/API/commands/expression/macros/TestMacros.py
+++ lldb/test/API/commands/expression/macros/TestMacros.py
@@ -129,3 +129,8 @@
 result = frame.EvaluateExpression("MACRO_2")
 self.assertTrue(result.GetError().Fail(),
 "Printing MACRO_2 fails in the header file")
+
+# Check that the macro definitions do not trigger bogus Clang
+# diagnostics about macro redefinitions.
+result = frame.EvaluateExpression("does_not_parse")
+self.assertNotIn("macro redefined", str(result.GetError()))
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -14,6 +14,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
@@ -141,6 +142,16 @@
   if (dm == nullptr)
 return;
 
+  // The macros directives below can potentially redefine builtin macros of the
+  // Clang instance which parses the user expression. The Clang diagnostics
+  // caused by this are not useful for the user as the source code here is
+  // generated by LLDB.
+  stream << "#pragma clang diagnostic push\n";
+  stream << "#pragma clang diagnostic ignored \"-Wmacro-redefined\"\n";
+  auto pop_warning = llvm::make_scope_exit([&stream](){
+stream << "#pragma clang diagnostic pop\n";
+  });
+
   for (size_t i = 0; i < dm->GetNumMacroEntries(); i++) {
 const DebugMacroEntry &entry = dm->GetMacroEntryAtIndex(i);
 uint32_t line;


Index: lldb/test/API/commands/expression/macros/TestMacros.py
===
--- lldb/test/API/commands/expression/macros/TestMacros.py
+++ lldb/test/API/commands/expression/macros/TestMacros.py
@@ -129,3 +129,8 @@
 result = frame.EvaluateExpression("MACRO_2")
 self.assertTrue(result.GetError().Fail(),
 "Printing MACRO_2 fails in the header file")
+
+# Check that the macro definitions do not trigger bogus Clang
+# diagnostics about macro redefinitions.
+result = frame.EvaluateExpression("does_not_parse")
+self.assertNotIn("macro redefined", str(result.GetError()))
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -14,6 +14,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
@@ -141,6 +142,16 @@
   if (dm == nullptr)
 return;
 
+  // The macros directives below can potentially redefine builtin macros of the
+  // Clang instance which parses the user expression. The Clang diagnostics
+  // caused by this are not useful for the user as the source code here is
+  // generated by LLDB.
+  stream << "#pragma clang diagnostic push\n";
+  stream << "#pragma clang diagnostic ignored \"-Wmacro-redefined\"\n";
+  auto pop_warning = llvm::make_scope_exit([&stream](){
+stream << "#pragma clang diagnostic pop\n";
+  });
+
   for (size_t i = 0; i < dm->GetNumMacroEntries(); i++) {
 const DebugM