date:20210604

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

2021-06-04 Thread Bruce Mitchener via Phabricator via lldb-commits

brucem created this revision.
brucem requested review of this revision.
Herald added a project: LLDB.

This is another step towards implementing the equivalent of
`platform process list` and related functionality.

`uint32_t` is used for the argument count and index despite the
underlying value being `size_t` to be consistent with other
index-based access to arguments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103675

Files:
  lldb/bindings/interface/SBProcessInfo.i
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBProcessInfo.h
  lldb/source/API/SBProcessInfo.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py

Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -335,6 +335,8 @@
 # Launch the process and stop at the entry point.
 launch_info = target.GetLaunchInfo()
 launch_info.SetWorkingDirectory(self.get_process_working_directory())
+launch_info.SetEnvironmentEntries(["FOO=BAR"], False)
+launch_info.SetArguments(["--abc"], False)
 launch_flags = launch_info.GetLaunchFlags()
 launch_flags |= lldb.eLaunchFlagStopAtEntry
 launch_info.SetLaunchFlags(launch_flags)
@@ -358,6 +360,11 @@
 "Process ID is valid")
 triple = process_info.GetTriple()
 self.assertIsNotNone(triple, "Process has a triple")
+env = process_info.GetEnvironment()
+self.assertGreater(env.GetNumValues(), 0)
+self.assertEqual("BAR", env.Get("FOO"))
+self.assertEqual(process_info.GetNumArguments(), 1)
+self.assertEqual("--abc", process_info.GetArgumentAtIndex(0))
 
 # Additional process info varies by platform, so just check that
 # whatever info was retrieved is consistent and nothing blows up.
Index: lldb/source/API/SBProcessInfo.cpp
===
--- lldb/source/API/SBProcessInfo.cpp
+++ lldb/source/API/SBProcessInfo.cpp
@@ -9,6 +9,7 @@
 #include "lldb/API/SBProcessInfo.h"
 #include "SBReproducerPrivate.h"
 #include "Utils.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/Utility/ProcessInfo.h"
 
@@ -194,6 +195,38 @@
   return triple;
 }
 
+uint32_t SBProcessInfo::GetNumArguments() {
+  LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBProcessInfo, GetNumArguments);
+
+  uint32_t num = 0;
+  if (m_opaque_up) {
+num = m_opaque_up->GetArguments().size();
+  }
+  return num;
+}
+
+const char *SBProcessInfo::GetArgumentAtIndex(uint32_t index) {
+  LLDB_RECORD_METHOD(const char *, SBProcessInfo, GetArgumentAtIndex,
+ (uint32_t), index);
+
+  const char *argument = nullptr;
+  if (m_opaque_up) {
+argument = m_opaque_up->GetArguments().GetArgumentAtIndex(index);
+  }
+  return argument;
+}
+
+SBEnvironment SBProcessInfo::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBProcessInfo,
+ GetEnvironment);
+
+  if (m_opaque_up) {
+return LLDB_RECORD_RESULT(SBEnvironment(m_opaque_up->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -220,6 +253,10 @@
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveGroupIDIsValid, ());
   LLDB_REGISTER_METHOD(lldb::pid_t, SBProcessInfo, GetParentProcessID, ());
   LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetTriple, ());
+  LLDB_REGISTER_METHOD(uint32_t, SBProcessInfo, GetNumArguments, ());
+  LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetArgumentAtIndex,
+   (uint32_t));
+  LLDB_REGISTER_METHOD(lldb::SBEnvironment, SBProcessInfo, GetEnvironment, ());
 }
 
 }
Index: lldb/include/lldb/API/SBProcessInfo.h
===
--- lldb/include/lldb/API/SBProcessInfo.h
+++ lldb/include/lldb/API/SBProcessInfo.h
@@ -53,6 +53,19 @@
   /// Return the target triple (arch-vendor-os) for the described process.
   const char *GetTriple();
 
+  // Return the number of arguments given to the described process.
+  uint32_t GetNumArguments();
+
+  // Return the specified argument given to the described process.
+  const char *GetArgumentAtIndex(uint32_t index);
+
+  /// Return the environment variables for the described process.
+  ///
+  /// \return
+  /// An lldb::SBEnvironment object which is a copy of the process
+  /// environment.
+  SBEnvironment GetEnvironment();
+
 private:
   friend class SBProcess;
 
Index: lldb/include/lldb/API/SBEnvironment.h
===
--- lldb/include/lldb/API/SBEnvironment.h
+++ lldb/include/lldb/API/SBEnvironment.h
@@ -122,6 +122,7 @@
 protected:
   friend class SBPlatform;
   friend class SBTarget;
+  friend class SBProcessInfo;
   friend

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

2021-06-04 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib accepted this revision as: mib.
mib 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/D103675/new/

https://reviews.llvm.org/D103675

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

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

2021-06-04 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added inline comments.



Comment at: lldb/bindings/interface/SBProcessInfo.i:78
+
+%feature("docstring",
+"Return the specified argument given to the described process."

Can you add this line here?

```
%feature("autodoc", "GetArgumentAtIndex(int index) -> string") 
GetArgumentAtIndex;
```

Otherwise the Python docs will mention that this returns `const char *` which 
is always kinda weird for users to see.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103675

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

[Lldb-commits] [PATCH] D103652: [lldb][NFC] Refactor name to index maps in Symtab

2021-06-04 Thread Raphael Isemann via Phabricator via lldb-commits

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

Only have some comments about the way `FindFunctionSymbols` is now implemented, 
but otherwise this LGTM.




Comment at: lldb/include/lldb/Symbol/Symtab.h:177
   FileRangeToIndexMap m_file_addr_to_index;
-  UniqueCStringMap m_name_to_index;
-  UniqueCStringMap m_basename_to_index;
-  UniqueCStringMap m_method_to_index;
-  UniqueCStringMap m_selector_to_index;
+  std::map>
+  m_functype_to_map;

`/// Maps function names to symbol indices (grouped by FunctionNameTypes)`
and maybe let's call this variable:
`m_name_to_symbol_indizes`



Comment at: lldb/source/Symbol/Symtab.cpp:1031
   std::vector symbol_indexes;
+  std::vector> maps_to_search;
 

Those large maps seem rather expensive to copy around. Could you just make this 
a for loop over the few types we care about? Then we can delete all the 
duplicated code.

```
lang=c++
for (lldb::FunctionNameType type : {lldb::eFunctionNameTypeBase, 
lldb::eFunctionNameTypeMethod, lldb::eFunctionNameTypeSelector}) {
  if (name_type_mask & type) {
auto &map = GetFunctypeMap(type);
[search map]
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103652

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

[Lldb-commits] [lldb] 0a655c6 - [lldb][NFC] Remove a redundant call to weak_ptr::expired

2021-06-04 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-06-04T12:06:53+02:00
New Revision: 0a655c62eca878cd5f366c08a4a5fee1b8723ce8

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

LOG: [lldb][NFC] Remove a redundant call to weak_ptr::expired

The `lock` call directly will check for us if the `weak_ptr` is expired and
returns an invalid `shared_ptr` (which we correctly handle), so this check is
redundant.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index ec3bee2cbc5d5..494622fdd24ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3792,7 +3792,7 @@ void SymbolFileDWARF::DumpClangAST(Stream &s) {
 }
 
 SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
-  if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
+  if (m_debug_map_symfile == nullptr) {
 lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
 if (module_sp) {
   m_debug_map_symfile =



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

[Lldb-commits] [PATCH] D103442: [lldb][NFC] Remove a redundant call to weak_ptr::expired

2021-06-04 Thread Raphael Isemann 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 rG0a655c62eca8: [lldb][NFC] Remove a redundant call to 
weak_ptr::expired (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103442

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


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3792,7 +3792,7 @@
 }
 
 SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
-  if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
+  if (m_debug_map_symfile == nullptr) {
 lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
 if (module_sp) {
   m_debug_map_symfile =


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3792,7 +3792,7 @@
 }
 
 SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
-  if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
+  if (m_debug_map_symfile == nullptr) {
 lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
 if (module_sp) {
   m_debug_map_symfile =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Michael Benfield via Phabricator via lldb-commits

mbenfield updated this revision to Diff 347111.
mbenfield added a comment.

Move fixes into a separate change https://reviews.llvm.org/D102942.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
  clang/test/CodeGen/2007-10-30-Volatile.c
  clang/test/CodeGen/X86/x86_32-xsave.c
  clang/test/CodeGen/X86/x86_64-xsave.c
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-riscv.c
  clang/test/FixIt/fixit.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/shift.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/Sema/warn-unused-but-set-parameters.c
  clang/test/Sema/warn-unused-but-set-variables.c
  clang/test/SemaCXX/goto.cpp
  clang/test/SemaCXX/shift.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
  clang/test/SemaObjC/foreach.m

Index: clang/test/SemaObjC/foreach.m
===
--- clang/test/SemaObjC/foreach.m
+++ clang/test/SemaObjC/foreach.m
@@ -1,4 +1,4 @@
-/* RUN: %clang_cc1 -Wall -fsyntax-only -verify -std=c89 -pedantic %s
+/* RUN: %clang_cc1 -Wall -Wno-unused-but-set-variable -fsyntax-only -verify -std=c89 -pedantic %s
  */
 
 @class NSArray;
Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s
+
+struct S {
+  int i;
+};
+
+struct __attribute__((warn_unused)) SWarnUnused {
+  int j;
+};
+
+int f0() {
+  int y; // expected-warning{{variable 'y' set but not used}}
+  y = 0;
+
+  int z __attribute__((unused));
+  z = 0;
+
+  // In C++, don't warn for structs. (following gcc's behavior)
+  struct S s;
+  struct S t;
+  s = t;
+
+  // Unless it's marked with the warn_unused attribute.
+  struct SWarnUnused swu; // expected-warning{{variable 'swu' set but not used}}
+  struct SWarnUnused swu2;
+  swu = swu2;
+
+  int x;
+  x = 0;
+  return x + 5;
+}
+
+void f1(void) {
+  (void)^() {
+int y; // expected-warning{{variable 'y' set but not used}}
+y = 0;
+
+int x;
+x = 0;
+return x;
+  };
+}
+
+void f2() {
+  // Don't warn for either of these cases.
+  constexpr int x = 2;
+  const int y = 1;
+  char a[x];
+  char b[y];
+}
Index: clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s
+
+int f0(int x,
+   int y, // expected-warning{{parameter 'y' set but not used}}
+   int z __attribute__((unused))) {
+  y = 0;
+  return x;
+}
+
+void f1(void) {
+  (void)^(int x,
+  int y, // expected-warning{{parameter 'y' set but not used}}
+  int z __attribute__((unused))) {
+y = 0;
+return x;
+  };
+}
+
+struct S {
+  int i;
+};
+
+// In C++, don't warn for a struct (following gcc).
+void f3(struct S s) {
+  struct S t;
+  s = t;
+}
+
+// Also don't warn for a reference.
+void f4(int &x) {
+  x = 0;
+}
+
+// Make sure this doesn't warn.
+struct A {
+  int i;
+  A(int j) : i(j) {}
+};
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unu

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-06-04 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added inline comments.



Comment at: llvm/include/llvm/Support/TargetRegistry.h:26
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/Support/CodeGen.h"

`include/llvm/Support/TargetRegistry.h now has cyclic dependency on 
include/MC/*.h`.

The previous style isn't good as well: `include/llvm/Support/TargetRegistry.h` 
has forward declarations of a number of MC* classes.

I think this header probably should be moved to lib/Target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-06-04 Thread Philipp Krones via Phabricator via lldb-commits

flip1995 added inline comments.



Comment at: llvm/include/llvm/Support/TargetRegistry.h:26
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/Support/CodeGen.h"

MaskRay wrote:
> `include/llvm/Support/TargetRegistry.h now has cyclic dependency on 
> include/MC/*.h`.
> 
> The previous style isn't good as well: 
> `include/llvm/Support/TargetRegistry.h` has forward declarations of a number 
> of MC* classes.
> 
> I think this header probably should be moved to lib/Target.
> `include/llvm/Support/TargetRegistry.h now has cyclic dependency on 
> include/MC/*.h`

I'll submit a quick fix changing this to a forward decl.

> I think this header probably should be moved to lib/Target.

The whole `TargetRegistry.h` header? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-06-04 Thread Philipp Krones via Phabricator via lldb-commits

flip1995 added inline comments.



Comment at: llvm/include/llvm/Support/TargetRegistry.h:26
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/Support/CodeGen.h"

flip1995 wrote:
> MaskRay wrote:
> > `include/llvm/Support/TargetRegistry.h now has cyclic dependency on 
> > include/MC/*.h`.
> > 
> > The previous style isn't good as well: 
> > `include/llvm/Support/TargetRegistry.h` has forward declarations of a 
> > number of MC* classes.
> > 
> > I think this header probably should be moved to lib/Target.
> > `include/llvm/Support/TargetRegistry.h now has cyclic dependency on 
> > include/MC/*.h`
> 
> I'll submit a quick fix changing this to a forward decl.
> 
> > I think this header probably should be moved to lib/Target.
> 
> The whole `TargetRegistry.h` header? 
> I'll submit a quick fix changing this to a forward decl.

I just noticed, that this isn't as easy, as I would've thought, since the MOFI 
is constructed if no `createMCObjectFileInfo` callback function is defined by 
the target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-06-04 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added subscribers: compnerd, dblaikie.
MaskRay added a comment.

Because of `new MCObjectFileInfo`, we cannot use a forward declaration 
(incomplete class) to replace `#include "llvm/MC/MCObjectFileInfo.h"` in 
`TargetRegistry.h`.

I thought about moving `TargetRegistry.{h,cpp}` from Support to Target. 
However, it doesn't work because Bitcode/Object call 
`TargetRegistry::lookupTarget` and Bitcode/Object are lower than Target.

@compnerd @dblaikie @mehdi_amini Do you have suggestions on fixing the layering?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-06-04 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added a comment.

In D101921#2786245 , @MaskRay wrote:

> Because of `new MCObjectFileInfo`, we cannot use a forward declaration 
> (incomplete class) to replace `#include "llvm/MC/MCObjectFileInfo.h"` in 
> `TargetRegistry.h`.
>
> I thought about moving `TargetRegistry.{h,cpp}` from Support to Target. 
> However, it doesn't work because Bitcode/Object call 
> `TargetRegistry::lookupTarget` and Bitcode/Object are lower than Target.
>
> @compnerd @dblaikie @mehdi_amini Do you have suggestions on fixing the 
> layering?

Looks like a big patch and a bit hard for me to page in all the context. Could 
you summarize the layering constraints/what headers/types/functions are in 
which libraries and why they are there/what constrains them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Roman Lebedev via Phabricator via lldb-commits

lebedev.ri resigned from this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Removing from queue - i don't expect to review this.
Looks like this has been reverted twice now, presumably llvm stage 2/linux 
kernel/chrome
should be enough of a coverage to be sure there isn't any obvious 
false-positives this time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-06-04 Thread Fangrui Song 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 rGc2f819af73c5: [MC] Refactor MCObjectFileInfo initialization 
and allow targets to create… (authored by flip1995, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/Support/TargetRegistry.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.h
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.h
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/Disassembler.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/CodeGen/MachineInstrTest.cpp
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/CodeGen/TestAsmPrinter.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
  mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Index: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
===
--- mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -168,9 +168,10 @@
   target->createMCAsmInfo(*mri, this->triple, mcOptions));
   mai->setRelaxELFRelocations(true);
 
-  llvm::MCObjectFileInfo mofi;
-  llvm::MCContext ctx(triple, mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
-  mofi.initMCObjectFileInfo(ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
+  llvm::MCContext ctx(triple, mai.get(), mri.get(), &srcMgr, &mcOptions);
+  std::unique_ptr mofi(target->createMCObjectFileInfo(
+  ctx, /*PIC=*/false, /*LargeCodeModel=*/false));
+  ctx.setObjectFileInfo(mofi.get());
 
   SmallString<128> cwd;
   if (!llvm::sys::fs::current_path(cwd))
Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -62,6 +62,7 @@
   std::unique_ptr MRI;
   std::unique_ptr MUPMAI;
   std::unique_ptr MII;
+  std::unique_ptr MOFI;
   std::unique_ptr Str;
   std::unique_ptr Parser;
   std::unique_ptr Ctx;
@@ -74,7 +75,6 @@
   const Target *TheTarget;
 
   const MCTargetOptions MCOptions;
-  MCObjectFileInfo MOFI;
 
   SystemZAsmLexerTest() {
 // We will use the SystemZ triple, because of missing
@@ -112,9 +112,11 @@
 SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 EXPECT_EQ(Buffer, nullptr);
 
-Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, STI.get(),
-&SrcMgr, &MCOptions));
-MOFI.initMCObjectFileInfo(*Ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
+Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), STI.get(), &SrcMgr,
+&MCOptions));
+MOFI.reset(TheTarget->createMCObjectFileInfo(*Ctx, /*PIC=*/false,
+ /*LargeCodeModel=*/false));
+Ctx->setObjectFileInfo(MOFI.get());
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -41,7 +41,7 @@
 MCTargetOptions MCOptions;
 MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
 Ctx = std::make_unique(Triple(TripleName), MAI.get(), MRI.get(),
-  /*MOFI=*/nullptr, /*MSTI=*/nullptr);
+  /*MSTI=*/nullptr);
   }
 
   operator bool() { return Ctx.get(); }
Index: llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

2021-06-04 Thread zhongchengyong via Phabricator via lldb-commits

sven added a comment.

@luismarques 
I have tried the patch with llvm12, test case is provide by 
jade(https://gist.github.com/e2efac2f780ed820277dbaf608805f4e), but it didn't 
worked for me.
After execute 'gdb-remote 1234', the terminal shows:

  Process 1 stopped
  * Thread #1, stop reason = signal SIGTRAP
 frame #0: 0x
  }

It seems that the unwind didn't succeed, can you figure out the problem?
my qemu vesion: 4.2.1
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

2021-06-04 Thread Luís Marques via Phabricator via lldb-commits

luismarques added a comment.

In D62732#2789409 , @sven wrote:

> I have tried the patch with llvm12, test case is provide by 
> jade(https://gist.github.com/e2efac2f780ed820277dbaf608805f4e), but it didn't 
> worked for me.
> It seems that the unwind didn't succeed, can you figure out the problem?

That's surprising. I'll see if I can figure out what the issue might be. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

2021-06-04 Thread Luís Marques via Phabricator via lldb-commits

luismarques added a comment.

In D62732#2790028 , @luismarques wrote:

> That's surprising. I'll see if I can figure out what the issue might be. 
> Thanks.

Confirmed. Something must have broken since the last patch revision. I'll see 
if I can figure out / fix this soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

2021-06-04 Thread zhongchengyong via Phabricator via lldb-commits

sven added a comment.

In D62732#2790087 , @luismarques wrote:

> In D62732#2790028 , @luismarques 
> wrote:
>
>> That's surprising. I'll see if I can figure out what the issue might be. 
>> Thanks.
>
> Confirmed. Something must have broken since the last patch revision. I'll see 
> if I can figure out / fix this soon.

Hi @luismarques, @jade  I have fixed the issue by install **libxml2-dev**, then 
recompile lldb and it works.
The cause of this issue is that LLDB doesn't send qXfer command for register 
info which defined in qemu-gdb-stub xml if libxml2 is not installed.
See ProcessGDBRemote.cpp::GetGDBServerRegisterInfo().
Thank you for your help.

  // query the target of gdb-remote for extended target information returns
  // true on success (got register definitions), false on failure (did not).
  bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
// Make sure LLDB has an XML parser it can use first
if (!XMLDocument::XMLEnabled())
  return false;

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

2021-06-04 Thread Jade via Phabricator via lldb-commits

jade added a comment.

In D62732#2789409 , @sven wrote:

> It seems that the unwind didn't succeed, can you figure out the problem?
> my qemu vesion: 4.2.1
> Thanks.

Unfortunately, from my last adventures through the lldb codebase in debugging 
that (see bugs.llvm.org/show_bug.cgi?id=49941 for pertinent places to look at 
internals), that particular symptom may not provide enough detail to track this 
down to a root cause necessarily (in the case of missing this patch for 
instance, it would be that the debugger didn't know what the stack pointer was 
called). It could be that the patch is not applied, it could be something else 
entirely. I believe if you call Dump() with the debugger on an instance of one 
of the gdb protocol classes that has the register information (sorry, away from 
a computer and can't remember which one it is), you can see what registers it 
knows about. But overall our error reporting needs to be vastly improved on 
this case. I want to write a patch for it but I've not had the chance.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread George Burgess IV via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf49cae278b4: [Clang] -Wunused-but-set-parameter and 
-Wunused-but-set-variable (authored by mbenfield, committed by 
george.burgess.iv).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
  clang/test/CodeGen/2007-10-30-Volatile.c
  clang/test/CodeGen/X86/x86_32-xsave.c
  clang/test/CodeGen/X86/x86_64-xsave.c
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-riscv.c
  clang/test/FixIt/fixit.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/shift.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/Sema/warn-unused-but-set-parameters.c
  clang/test/Sema/warn-unused-but-set-variables.c
  clang/test/SemaCXX/goto.cpp
  clang/test/SemaCXX/shift.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
  clang/test/SemaObjC/foreach.m

Index: clang/test/SemaObjC/foreach.m
===
--- clang/test/SemaObjC/foreach.m
+++ clang/test/SemaObjC/foreach.m
@@ -1,4 +1,4 @@
-/* RUN: %clang_cc1 -Wall -fsyntax-only -verify -std=c89 -pedantic %s
+/* RUN: %clang_cc1 -Wall -Wno-unused-but-set-variable -fsyntax-only -verify -std=c89 -pedantic %s
  */
 
 @class NSArray;
Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s
+
+struct S {
+  int i;
+};
+
+struct __attribute__((warn_unused)) SWarnUnused {
+  int j;
+};
+
+int f0() {
+  int y; // expected-warning{{variable 'y' set but not used}}
+  y = 0;
+
+  int z __attribute__((unused));
+  z = 0;
+
+  // In C++, don't warn for structs. (following gcc's behavior)
+  struct S s;
+  struct S t;
+  s = t;
+
+  // Unless it's marked with the warn_unused attribute.
+  struct SWarnUnused swu; // expected-warning{{variable 'swu' set but not used}}
+  struct SWarnUnused swu2;
+  swu = swu2;
+
+  int x;
+  x = 0;
+  return x + 5;
+}
+
+void f1(void) {
+  (void)^() {
+int y; // expected-warning{{variable 'y' set but not used}}
+y = 0;
+
+int x;
+x = 0;
+return x;
+  };
+}
+
+void f2() {
+  // Don't warn for either of these cases.
+  constexpr int x = 2;
+  const int y = 1;
+  char a[x];
+  char b[y];
+}
Index: clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s
+
+int f0(int x,
+   int y, // expected-warning{{parameter 'y' set but not used}}
+   int z __attribute__((unused))) {
+  y = 0;
+  return x;
+}
+
+void f1(void) {
+  (void)^(int x,
+  int y, // expected-warning{{parameter 'y' set but not used}}
+  int z __attribute__((unused))) {
+y = 0;
+return x;
+  };
+}
+
+struct S {
+  int i;
+};
+
+// In C++, don't warn for a struct (following gcc).
+void f3(struct S s) {
+  struct S t;
+  s = t;
+}
+
+// Also don't warn for a reference.
+void f4(int &x) {
+  x = 0;
+}
+
+// Make sure this doesn't warn.
+struct A {
+  int i;
+  A(int j) : i(j) {}
+};
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Andi via Phabricator via lldb-commits

Abpostelnicu added a comment.

I think there is a false positive with this @george.burgess.iv:
In 
[this)(https://searchfox.org/mozilla-central/source/mozglue/baseprofiler/core/platform-linux-android.cpp#222-227)
 we have the warning triggered: 
mozglue/baseprofiler/core/platform-linux-android.cpp:216:9: error: variable 'r' 
set but not used [-Werror,-Wunused-but-set-variable]

  SigHandlerCoordinator() {
PodZero(&mUContext);
int r = sem_init(&mMessage2, /* pshared */ 0, 0);
r |= sem_init(&mMessage3, /* pshared */ 0, 0);
r |= sem_init(&mMessage4, /* pshared */ 0, 0);
MOZ_ASSERT(r == 0);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Dávid Bolvanský via Phabricator via lldb-commits

xbolva00 added a comment.

In D100581#2792854 , @Abpostelnicu 
wrote:

> I think there is a false positive with this @george.burgess.iv:
> In this 
> 
>  we have the warning triggered: 
> mozglue/baseprofiler/core/platform-linux-android.cpp:216:9: error: variable 
> 'r' set but not used [-Werror,-Wunused-but-set-variable]
>
>   SigHandlerCoordinator() {
> PodZero(&mUContext);
> int r = sem_init(&mMessage2, /* pshared */ 0, 0);
> r |= sem_init(&mMessage3, /* pshared */ 0, 0);
> r |= sem_init(&mMessage4, /* pshared */ 0, 0);
> MOZ_ASSERT(r == 0);
>   }

If MOZ_ASSERT is expanded to nothing, than the warning is correct. (void)r; 
trick should work here..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Louis Dionne via Phabricator via lldb-commits

ldionne added a comment.

Hello! There are still some false positives, for example this one is breaking 
the libc++ CI: 
https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848

Would it be possible to either fix this quickly or revert the change until the 
issue has been fixed? Our pre-commit CI is going to be stalled until this is 
fixed. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

Re: [Lldb-commits] Prevent type canonization when dereferencing

2021-06-04 Thread Andy Yankovsky via lldb-commits

I think the problem is specific to references, at least that's the case I
ran into in lldb-eval --
https://github.com/google/lldb-eval/blob/master/docs/lldb-bugs.md#dereferencing-a-value-canonizes-the-type

On Wed, 2 Jun 2021 at 17:26, Raphael “Teemperor” Isemann <
teempe...@gmail.com> wrote:

> I assume your bug is for dereferencing references? In your test taking the
> ref variable and then dereferencing it via the SB API reproduces that I
> think you're running into:
>
> >>> lldb.frame.FindVariable("p_ref").GetType().GetName()
> 'TPair &'
> >>> lldb.frame.FindVariable("p_ref").Dereference().GetType().GetName()
> 'TTuple'
>
> (I made `p_ref` a local to avoid the expression evaluation machinery)
>
> Cheers,
> - Raphael
>
> > On 2 Jun 2021, at 15:27, Lasse Folger  wrote:
> >
> > Hi Raphael,
> >
> > I have a very similar test for a tool that integrates with lldb which
> failed without the patch.
> > I thought the test in the patch would behave the same which is
> apparently not the case.
> > Thanks for pointing that out. I will need to take another look and will
> get back to you once I figure out what's wrong.
> > Sorry for the inconvenience.
> >
> > kind regards,
> > Lasse
> >
> >
> > On Wed, Jun 2, 2021 at 1:15 PM Raphael “Teemperor” Isemann <
> teempe...@gmail.com> wrote:
> > Hi Lasse,
> >
> > the test from the patch passes for me even without your non-test
> changes. Not sure if you attached the wrong diff or it needs to be applied
> on a specific commit that is not ToT? Can you maybe try pushing your code
> to some git repo?
> >
> > Your change to TypeSystemClang (which I assume removes the
> canonicalization of parent_qual_type) is from what I can see not actually
> changing the result value of `GetChildCompilerTypeAtIndex`. It looks like
> the return value for pointer types is computed independently from
> `parent_qual_type` without any canonicalization.
> >
> > Cheers,
> > - Raphael
> >
> >> On 2 Jun 2021, at 11:39, Lasse Folger via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >>
> >> <0001-lldb-prevent-canonization-of-type-when-dereferencing.patch>
> >
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added a comment.

In D100581#2793775 , @ldionne wrote:

> Hello! There are still some false positives, for example this one is breaking 
> the libc++ CI: 
> https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848
>
> Would it be possible to either fix this quickly or revert the change until 
> the issue has been fixed? Our pre-commit CI is going to be stalled until this 
> is fixed. Thanks!

Looks like a true positive in libc++ ( 
https://github.com/llvm/llvm-project/blob/main/libcxx/include/regex ) - the 
"__j" variable is initialized, then incremented, but never read (except to 
increment). Is that a bug in libc++? Was __j meant to be used for something?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Michael Benfield via Phabricator via lldb-commits

mbenfield added a comment.

In D100581#2792854 , @Abpostelnicu 
wrote:

> I think there is a false positive with this @george.burgess.iv:
> In this 
> 
>  we have the warning triggered: 
> mozglue/baseprofiler/core/platform-linux-android.cpp:216:9: error: variable 
> 'r' set but not used [-Werror,-Wunused-but-set-variable]
>
>   SigHandlerCoordinator() {
> PodZero(&mUContext);
> int r = sem_init(&mMessage2, /* pshared */ 0, 0);
> r |= sem_init(&mMessage3, /* pshared */ 0, 0);
> r |= sem_init(&mMessage4, /* pshared */ 0, 0);
> MOZ_ASSERT(r == 0);
>   }

Indeed as pointed out above I think this is a true positive unfortunately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Louis Dionne via Phabricator via lldb-commits

ldionne added a comment.

In D100581#2793926 , @dblaikie wrote:

> In D100581#2793775 , @ldionne wrote:
>
>> Hello! There are still some false positives, for example this one is 
>> breaking the libc++ CI: 
>> https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848
>>
>> Would it be possible to either fix this quickly or revert the change until 
>> the issue has been fixed? Our pre-commit CI is going to be stalled until 
>> this is fixed. Thanks!
>
> Looks like a true positive in libc++ ( 
> https://github.com/llvm/llvm-project/blob/main/libcxx/include/regex ) - the 
> "__j" variable is initialized, then incremented, but never read (except to 
> increment). Is that a bug in libc++? Was __j meant to be used for something?

Oh, you're right! I was tripped by the fact that we did in fact "use" `__j` (in 
the sense that we do `__j += ...`), but indeed we never read the result. I'll 
work on fixing that issue. I'm not sure whether it's a bug or not yet, that 
code was modified 11 years ago, but I'll do my research.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Stephan Bergmann via Phabricator via lldb-commits

sberg added a comment.

Is it intentional that this warns about volatile variables as in

  void f(char * p) {
  volatile char c = 0;
  c ^= *p;
  }

(I see that GCC warns about volatile too, at least when you replace the `^=` 
with `=`, so assume the answer is "yes", but would just like to see that 
confirmed (ideally with a test case even?).)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Michael Benfield via Phabricator via lldb-commits

mbenfield added a comment.

In D100581#2795966 , @sberg wrote:

> Is it intentional that this warns about volatile variables as in

Yes, volatile was intentional. Alright, I will add a test for this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Dávid Bolvanský via Phabricator via lldb-commits

xbolva00 added a comment.

In D100581#2798079 , @raj.khem wrote:

> http://sprunge.us/FJzZXL is a file from harfbuzz and it warns
>
>   a.cc:28670:32: error: variable 'supp_size' set but not used 
> [-Werror,-Wunused-but-set-variable]
>   unsigned int size0, size1, supp_size = 0;
>
> I do not have -Werror enabled but it still is reported as error. There is no 
> way to disable this warning ?

Well, you have

>> #pragma GCC diagnostic error "-Wunused"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Khem Raj via Phabricator via lldb-commits

raj.khem added a comment.

http://sprunge.us/FJzZXL is a file from harfbuzz and it warns

  a.cc:28670:32: error: variable 'supp_size' set but not used 
[-Werror,-Wunused-but-set-variable]
  unsigned int size0, size1, supp_size = 0;

I do not have -Werror enabled but it still is reported as error. There is no 
way to disable this warning ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-06-04 Thread Andi via Phabricator via lldb-commits

Abpostelnicu added a comment.

In D100581#2798079 , @raj.khem wrote:

> http://sprunge.us/FJzZXL is a file from harfbuzz and it warns
>
>   a.cc:28670:32: error: variable 'supp_size' set but not used 
> [-Werror,-Wunused-but-set-variable]
>   unsigned int size0, size1, supp_size = 0;

You have, with pragma gcc error, look at hb.hh. The problem is that clang has 
an error dealing with cascading pragma when setting different levels.

> I do not have -Werror enabled but it still is reported as error. There is no 
> way to disable this warning ?

Go to the hb GitHub repo, I’ve fixed this in master branch, it will propagate 
soon to a new release.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

2021-06-04 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added subscribers: pengfei, kristof.beyls.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

There is a common pattern:
result.AppendError(...);
result.SetStatus(eReturnStatusFailed);

I found that some commands don't actually "fail" but only
print "error: ..." because the second line got missed.

This can cause you to miss a failed command when you're
using the Python interface during testing.
(and produce some confusing script results)

I did not find any place where you would want to add
an error without setting the return status, so just
set eReturnStatusFailed whenever you add an error to
a command result.

This change does not remove any of the now redundant
SetStatus. This should allow us to see if there are any
tests that have commands unexpectedly fail with this change.
(the test suite passes for me but I don't have access to all
the systems we cover so there could be some corner cases)

Some tests that failed on x86 and AArch64 have been modified
to work with the new behaviour.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103701

Files:
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/Shell/Commands/command-backtrace-parser-1.test
  lldb/test/Shell/Commands/command-backtrace-parser-2.test
  lldb/test/Shell/Commands/command-backtrace.test

Index: lldb/test/Shell/Commands/command-backtrace-parser-2.test
===
--- lldb/test/Shell/Commands/command-backtrace-parser-2.test
+++ lldb/test/Shell/Commands/command-backtrace-parser-2.test
@@ -1,12 +1,6 @@
-# Check basic functionality of command bt.
 # RUN: %lldb -s %s 2>&1 | FileCheck %s
 
 # Make sure this is not rejected by the parser as invalid syntax.
-# Blank characters after the '1' are important, as we're testing the parser.
-bt 1  
-# CHECK: error: invalid target
-
-# Make sure this is not rejected by the parser as invalid syntax.
 # Blank characters after the 'all' are important, as we're testing the parser.
 bt all   
 # CHECK: error: invalid target
Index: lldb/test/Shell/Commands/command-backtrace-parser-1.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-backtrace-parser-1.test
@@ -0,0 +1,6 @@
+# RUN: %lldb -s %s 2>&1 | FileCheck %s
+
+# Make sure this is not rejected by the parser as invalid syntax.
+# Blank characters after the '1' are important, as we're testing the parser.
+bt 1  
+# CHECK: error: invalid target
Index: lldb/test/API/commands/register/register/register_command/TestRegisters.py
===
--- lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -41,13 +41,18 @@
 self.expect("register read -a", MISSING_EXPECTED_REGISTERS,
 substrs=['registers were unavailable'], matching=False)
 
+all_registers = self.res.GetOutput()
+
 if self.getArchitecture() in ['amd64', 'i386', 'x86_64']:
 self.runCmd("register read xmm0")
-self.runCmd("register read ymm15")  # may be available
-self.runCmd("register read bnd0")  # may be available
+if "ymm15 = " in all_registers:
+  self.runCmd("register read ymm15")  # may be available
+if "bnd0 = " in all_registers:
+  self.runCmd("register read bnd0")  # may be available
 elif self.getArchitecture() in ['arm', 'armv7', 'armv7k', 'arm64', 'arm64e', 'arm64_32']:
 self.runCmd("register read s0")
-self.runCmd("register read q15")  # may be available
+if "q15 = " in all_registers:
+  self.runCmd("register read q15")  # may be available
 
 self.expect(
 "register read -s 4",
@@ -413,7 +418,8 @@
 self.write_and_read(currentFrame, "ymm7", new_value)
 self.expect("expr $ymm0", substrs=['vector_type'])
 else:
-self.runCmd("register read ymm0")
+self.expect("register read ymm0", substrs=["Invalid register name 'ymm0'"],
+error=True)
 
 if has_mpx:
 # Test write and read for bnd0.
@@ -428,7 +434,8 @@
 self.write_and_read(currentFrame, "bndstatus", new_value)
 self.expect("expr $bndstatus", substrs = ['vector_type'])
 else:
-self.runCmd("register read bnd0")
+self.expect("register read bnd0", substrs=["Invalid register name 'bnd0'"],
+error=True)
 
 def convenience_registers(self):
 """Test convenience registers."""
@@ -450,7 +457,7 @@
 #

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

2021-06-04 Thread David Spickett via Phabricator via lldb-commits

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

There are likely other tests that aren't enabled for x86/AArch64 or sets of 
registers that I don't have on my machines. So if this change is welcome then 
the plan would be to land this as is and monitor the bots for a week or so, 
revert and reland with test fixups as needed.

Then I can remove the now redundant SetStatus calls, which I've done locally 
and goes from ~580 to ~50. Again, in batches to account for tests I'm not 
running.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103701

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

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

2021-06-04 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

I can run this for you on macOS and Linux x86 which I think should cover every 
test.

FWIW, there is even more redundancy from what I remember as we IIRC return 
false within `bool DoExecute` everywhere. I can't recall why I didn't remove 
this yet (beside that it's a lot of work).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103701

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

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

2021-06-04 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

Oh I see, you're not just concerned about just having every command in the test 
suite covered before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103701

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

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

2021-06-04 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

> I can run this for you on macOS and Linux x86 which I think should cover 
> every test.

That would be great, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103701

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

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

2021-06-04 Thread Bruce Mitchener via Phabricator via lldb-commits

brucem updated this revision to Diff 349895.
brucem added a comment.

Rebase and address feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103675

Files:
  lldb/bindings/interface/SBProcessInfo.i
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBProcessInfo.h
  lldb/source/API/SBProcessInfo.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py

Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -335,6 +335,8 @@
 # Launch the process and stop at the entry point.
 launch_info = target.GetLaunchInfo()
 launch_info.SetWorkingDirectory(self.get_process_working_directory())
+launch_info.SetEnvironmentEntries(["FOO=BAR"], False)
+launch_info.SetArguments(["--abc"], False)
 launch_flags = launch_info.GetLaunchFlags()
 launch_flags |= lldb.eLaunchFlagStopAtEntry
 launch_info.SetLaunchFlags(launch_flags)
@@ -358,6 +360,11 @@
 "Process ID is valid")
 triple = process_info.GetTriple()
 self.assertIsNotNone(triple, "Process has a triple")
+env = process_info.GetEnvironment()
+self.assertGreater(env.GetNumValues(), 0)
+self.assertEqual("BAR", env.Get("FOO"))
+self.assertEqual(process_info.GetNumArguments(), 1)
+self.assertEqual("--abc", process_info.GetArgumentAtIndex(0))
 
 # Additional process info varies by platform, so just check that
 # whatever info was retrieved is consistent and nothing blows up.
Index: lldb/source/API/SBProcessInfo.cpp
===
--- lldb/source/API/SBProcessInfo.cpp
+++ lldb/source/API/SBProcessInfo.cpp
@@ -9,6 +9,7 @@
 #include "lldb/API/SBProcessInfo.h"
 #include "SBReproducerPrivate.h"
 #include "Utils.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/Utility/ProcessInfo.h"
 
@@ -194,6 +195,38 @@
   return triple;
 }
 
+uint32_t SBProcessInfo::GetNumArguments() {
+  LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBProcessInfo, GetNumArguments);
+
+  uint32_t num = 0;
+  if (m_opaque_up) {
+num = m_opaque_up->GetArguments().size();
+  }
+  return num;
+}
+
+const char *SBProcessInfo::GetArgumentAtIndex(uint32_t index) {
+  LLDB_RECORD_METHOD(const char *, SBProcessInfo, GetArgumentAtIndex,
+ (uint32_t), index);
+
+  const char *argument = nullptr;
+  if (m_opaque_up) {
+argument = m_opaque_up->GetArguments().GetArgumentAtIndex(index);
+  }
+  return argument;
+}
+
+SBEnvironment SBProcessInfo::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBProcessInfo,
+ GetEnvironment);
+
+  if (m_opaque_up) {
+return LLDB_RECORD_RESULT(SBEnvironment(m_opaque_up->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -220,6 +253,10 @@
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveGroupIDIsValid, ());
   LLDB_REGISTER_METHOD(lldb::pid_t, SBProcessInfo, GetParentProcessID, ());
   LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetTriple, ());
+  LLDB_REGISTER_METHOD(uint32_t, SBProcessInfo, GetNumArguments, ());
+  LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetArgumentAtIndex,
+   (uint32_t));
+  LLDB_REGISTER_METHOD(lldb::SBEnvironment, SBProcessInfo, GetEnvironment, ());
 }
 
 }
Index: lldb/include/lldb/API/SBProcessInfo.h
===
--- lldb/include/lldb/API/SBProcessInfo.h
+++ lldb/include/lldb/API/SBProcessInfo.h
@@ -53,6 +53,19 @@
   /// Return the target triple (arch-vendor-os) for the described process.
   const char *GetTriple();
 
+  // Return the number of arguments given to the described process.
+  uint32_t GetNumArguments();
+
+  // Return the specified argument given to the described process.
+  const char *GetArgumentAtIndex(uint32_t index);
+
+  /// Return the environment variables for the described process.
+  ///
+  /// \return
+  /// An lldb::SBEnvironment object which is a copy of the process
+  /// environment.
+  SBEnvironment GetEnvironment();
+
 private:
   friend class SBProcess;
 
Index: lldb/include/lldb/API/SBEnvironment.h
===
--- lldb/include/lldb/API/SBEnvironment.h
+++ lldb/include/lldb/API/SBEnvironment.h
@@ -122,6 +122,7 @@
 protected:
   friend class SBPlatform;
   friend class SBTarget;
+  friend class SBProcessInfo;
   friend class SBLaunchInfo;
 
   SBEnvironment(lldb_private::Environment rhs);
Index: lldb/bindings/interface/SBProcessInfo.i
===
--- lldb/bindings/i

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

2021-06-04 Thread Bruce Mitchener via Phabricator via lldb-commits

brucem marked an inline comment as done.
brucem added inline comments.



Comment at: lldb/bindings/interface/SBProcessInfo.i:78
+
+%feature("docstring",
+"Return the specified argument given to the described process."

teemperor wrote:
> Can you add this line here?
> 
> ```
> %feature("autodoc", "GetArgumentAtIndex(int index) -> string") 
> GetArgumentAtIndex;
> ```
> 
> Otherwise the Python docs will mention that this returns `const char *` which 
> is always kinda weird for users to see.
I changed this to have the whole thing be an autodoc rather than both an 
autodoc and a docstring. This isn't consistently done throughout the code 
though and in this case, the resulting syntax help is a bit different.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103675

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

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

2021-06-04 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

This seems like a good change to me.  And even if you wanted to do something 
cheesy like put an error into the result on spec, then later decide it was 
successful after all, you can always reset the result's status to the 
appropriate success status.  This just makes the common practice less error 
prone.  We should also remove all the redundant settings, leaving them in would 
be confusing.  So I'd just make the change then fix whatever the bots bring up.

As to the bool from DoExecute, that also seems like a nice cleanup, but since 
you have to return something or the code doesn't compile, that's one that is 
harder to get wrong.  Plus it would be a tedious to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103701

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

[Lldb-commits] [PATCH] D103575: Allow signposts to take advantage of deferred string substitution

2021-06-04 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

I think we should just remove the functionality form the timer class again. I 
only added it there because of the macro.


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

https://reviews.llvm.org/D103575

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

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

2021-06-04 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.

HandleCommands really does need to do this suppression, and this is a fine way 
to do it.  I can't think of any other place where you would need to do this 
suppression after an admittedly non-exhaustive search.  So LGTM.


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

https://reviews.llvm.org/D103349

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

[Lldb-commits] [lldb] 8d33437 - [LLDB/API] Expose args and env from SBProcessInfo.

2021-06-04 Thread Bruce Mitchener via lldb-commits


Author: Bruce Mitchener
Date: 2021-06-05T13:42:18+07:00
New Revision: 8d33437d030af27fff21dd3fd0e66893b0148217

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

LOG: [LLDB/API] Expose args and env from SBProcessInfo.

This is another step towards implementing the equivalent of
`platform process list` and related functionality.

`uint32_t` is used for the argument count and index despite the
underlying value being `size_t` to be consistent with other
index-based access to arguments.

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

Added: 


Modified: 
lldb/bindings/interface/SBProcessInfo.i
lldb/include/lldb/API/SBEnvironment.h
lldb/include/lldb/API/SBProcessInfo.h
lldb/source/API/SBProcessInfo.cpp
lldb/test/API/python_api/process/TestProcessAPI.py

Removed: 




diff  --git a/lldb/bindings/interface/SBProcessInfo.i 
b/lldb/bindings/interface/SBProcessInfo.i
index 17b2761a344e..361975a571ad 100644
--- a/lldb/bindings/interface/SBProcessInfo.i
+++ b/lldb/bindings/interface/SBProcessInfo.i
@@ -68,6 +68,25 @@ public:
 ) GetTriple;
 const char *
 GetTriple ();
+
+%feature("docstring",
+"Return the number of arguments given to the described process."
+) GetNumArguments;
+uint32_t
+GetNumArguments ();
+
+%feature("autodoc", "
+GetArgumentAtIndex(int index) -> string
+Return the specified argument given to the described process."
+) GetArgumentAtIndex;
+const char *
+GetArgumentAtIndex (uint32_t index);
+
+%feature("docstring",
+"Return the environment variables for the described process."
+) GetEnvironment;
+SBEnvironment
+GetEnvironment ();
 };
 
 } // namespace lldb

diff  --git a/lldb/include/lldb/API/SBEnvironment.h 
b/lldb/include/lldb/API/SBEnvironment.h
index f40ee01a42ab..fcf41684d0be 100644
--- a/lldb/include/lldb/API/SBEnvironment.h
+++ b/lldb/include/lldb/API/SBEnvironment.h
@@ -122,6 +122,7 @@ class LLDB_API SBEnvironment {
 protected:
   friend class SBPlatform;
   friend class SBTarget;
+  friend class SBProcessInfo;
   friend class SBLaunchInfo;
 
   SBEnvironment(lldb_private::Environment rhs);

diff  --git a/lldb/include/lldb/API/SBProcessInfo.h 
b/lldb/include/lldb/API/SBProcessInfo.h
index 36fae9e842a6..ae5e6072aa74 100644
--- a/lldb/include/lldb/API/SBProcessInfo.h
+++ b/lldb/include/lldb/API/SBProcessInfo.h
@@ -53,6 +53,19 @@ class LLDB_API SBProcessInfo {
   /// Return the target triple (arch-vendor-os) for the described process.
   const char *GetTriple();
 
+  // Return the number of arguments given to the described process.
+  uint32_t GetNumArguments();
+
+  // Return the specified argument given to the described process.
+  const char *GetArgumentAtIndex(uint32_t index);
+
+  /// Return the environment variables for the described process.
+  ///
+  /// \return
+  /// An lldb::SBEnvironment object which is a copy of the process
+  /// environment.
+  SBEnvironment GetEnvironment();
+
 private:
   friend class SBProcess;
 

diff  --git a/lldb/source/API/SBProcessInfo.cpp 
b/lldb/source/API/SBProcessInfo.cpp
index cba3bdc179f3..6f6381367398 100644
--- a/lldb/source/API/SBProcessInfo.cpp
+++ b/lldb/source/API/SBProcessInfo.cpp
@@ -9,6 +9,7 @@
 #include "lldb/API/SBProcessInfo.h"
 #include "SBReproducerPrivate.h"
 #include "Utils.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/Utility/ProcessInfo.h"
 
@@ -194,6 +195,38 @@ const char *SBProcessInfo::GetTriple() {
   return triple;
 }
 
+uint32_t SBProcessInfo::GetNumArguments() {
+  LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBProcessInfo, GetNumArguments);
+
+  uint32_t num = 0;
+  if (m_opaque_up) {
+num = m_opaque_up->GetArguments().size();
+  }
+  return num;
+}
+
+const char *SBProcessInfo::GetArgumentAtIndex(uint32_t index) {
+  LLDB_RECORD_METHOD(const char *, SBProcessInfo, GetArgumentAtIndex,
+ (uint32_t), index);
+
+  const char *argument = nullptr;
+  if (m_opaque_up) {
+argument = m_opaque_up->GetArguments().GetArgumentAtIndex(index);
+  }
+  return argument;
+}
+
+SBEnvironment SBProcessInfo::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBProcessInfo,
+ GetEnvironment);
+
+  if (m_opaque_up) {
+return LLDB_RECORD_RESULT(SBEnvironment(m_opaque_up->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -220,6 +253,10 @@ void RegisterMethods(Registry &R) {
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveGroupIDIsValid, ());
   LLDB_REGISTER_METHOD(lldb::pid_t, SBProcessInfo, GetParentProcessID, ());
   LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetTriple, ());
+  LLDB_REGISTER_METHOD(uint32_t,

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

2021-06-04 Thread Bruce Mitchener via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
brucem marked an inline comment as done.
Closed by commit rG8d33437d030a: [LLDB/API] Expose args and env from 
SBProcessInfo. (authored by brucem).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103675

Files:
  lldb/bindings/interface/SBProcessInfo.i
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBProcessInfo.h
  lldb/source/API/SBProcessInfo.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py

Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -335,6 +335,8 @@
 # Launch the process and stop at the entry point.
 launch_info = target.GetLaunchInfo()
 launch_info.SetWorkingDirectory(self.get_process_working_directory())
+launch_info.SetEnvironmentEntries(["FOO=BAR"], False)
+launch_info.SetArguments(["--abc"], False)
 launch_flags = launch_info.GetLaunchFlags()
 launch_flags |= lldb.eLaunchFlagStopAtEntry
 launch_info.SetLaunchFlags(launch_flags)
@@ -358,6 +360,11 @@
 "Process ID is valid")
 triple = process_info.GetTriple()
 self.assertIsNotNone(triple, "Process has a triple")
+env = process_info.GetEnvironment()
+self.assertGreater(env.GetNumValues(), 0)
+self.assertEqual("BAR", env.Get("FOO"))
+self.assertEqual(process_info.GetNumArguments(), 1)
+self.assertEqual("--abc", process_info.GetArgumentAtIndex(0))
 
 # Additional process info varies by platform, so just check that
 # whatever info was retrieved is consistent and nothing blows up.
Index: lldb/source/API/SBProcessInfo.cpp
===
--- lldb/source/API/SBProcessInfo.cpp
+++ lldb/source/API/SBProcessInfo.cpp
@@ -9,6 +9,7 @@
 #include "lldb/API/SBProcessInfo.h"
 #include "SBReproducerPrivate.h"
 #include "Utils.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/Utility/ProcessInfo.h"
 
@@ -194,6 +195,38 @@
   return triple;
 }
 
+uint32_t SBProcessInfo::GetNumArguments() {
+  LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBProcessInfo, GetNumArguments);
+
+  uint32_t num = 0;
+  if (m_opaque_up) {
+num = m_opaque_up->GetArguments().size();
+  }
+  return num;
+}
+
+const char *SBProcessInfo::GetArgumentAtIndex(uint32_t index) {
+  LLDB_RECORD_METHOD(const char *, SBProcessInfo, GetArgumentAtIndex,
+ (uint32_t), index);
+
+  const char *argument = nullptr;
+  if (m_opaque_up) {
+argument = m_opaque_up->GetArguments().GetArgumentAtIndex(index);
+  }
+  return argument;
+}
+
+SBEnvironment SBProcessInfo::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBProcessInfo,
+ GetEnvironment);
+
+  if (m_opaque_up) {
+return LLDB_RECORD_RESULT(SBEnvironment(m_opaque_up->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -220,6 +253,10 @@
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveGroupIDIsValid, ());
   LLDB_REGISTER_METHOD(lldb::pid_t, SBProcessInfo, GetParentProcessID, ());
   LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetTriple, ());
+  LLDB_REGISTER_METHOD(uint32_t, SBProcessInfo, GetNumArguments, ());
+  LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetArgumentAtIndex,
+   (uint32_t));
+  LLDB_REGISTER_METHOD(lldb::SBEnvironment, SBProcessInfo, GetEnvironment, ());
 }
 
 }
Index: lldb/include/lldb/API/SBProcessInfo.h
===
--- lldb/include/lldb/API/SBProcessInfo.h
+++ lldb/include/lldb/API/SBProcessInfo.h
@@ -53,6 +53,19 @@
   /// Return the target triple (arch-vendor-os) for the described process.
   const char *GetTriple();
 
+  // Return the number of arguments given to the described process.
+  uint32_t GetNumArguments();
+
+  // Return the specified argument given to the described process.
+  const char *GetArgumentAtIndex(uint32_t index);
+
+  /// Return the environment variables for the described process.
+  ///
+  /// \return
+  /// An lldb::SBEnvironment object which is a copy of the process
+  /// environment.
+  SBEnvironment GetEnvironment();
+
 private:
   friend class SBProcess;
 
Index: lldb/include/lldb/API/SBEnvironment.h
===
--- lldb/include/lldb/API/SBEnvironment.h
+++ lldb/include/lldb/API/SBEnvironment.h
@@ -122,6 +122,7 @@
 protected:
   friend class SBPlatform;
   friend class SBTarget;
+  friend class SBProcessInfo;
   friend class SBLaunchInfo;
 
   SB

44 matches

Mail list logo