[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82093e8fb7d6: [lldb/Driver] Fix handling on positional 
arguments (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80165

Files:
  lldb/docs/man/lldb.rst
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestPositionalArgs.test
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
@@ -765,10 +760,15 @@
   The debugger can be started in several modes.
 
   Passing an executable as a positional argument prepares lldb to debug the
-  given executable. Arguments passed after -- are considered arguments to the
-  debugged executable.
+  given executable. To disambiguate between arguments passed to lldb and
+  arguments passed to the debugged executable, arguments starting with a - must
+  be passed after --.
+
+lldb --arch x86_64 /path/to/program program argument -- --arch arvm7
+
+  For convenience, passing the executable after -- is also supported.
 
-lldb --arch x86_64 /path/to/program -- --arch arvm7
+lldb --arch x86_64 -- /path/to/program program argument --arch arvm7
 
   Passing one of the attach options causes lldb to immediately attach to the
   given process.
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,30 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb %t.foo -x bar -b baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s
+
+CHECK: Current executable set to {{.*}}foo
+CHECK: target.run-args "bar" "baz" "quux"
+
+RUN: %lldb -x -b %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -- %t.foo bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -f %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+
+DASH: Current executable set to {{.*}}foo
+DASH: target.run-args "bar" "-baz" "--quux"
+
+RUN: %lldb -x -b %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix UNKNOWN
+RUN: %lldb -x -b -f %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix UNKNOWN
+
+UNKNOWN: warning: ignoring unknown option: -baz
+UNKNOWN: warning: ignoring unknown option: --quux
+UNKNOWN: Current executable set to {{.*}}foo
+UNKNOWN: target.run-args "bar"
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,4 @@
-# RUN: %lldb --no-use-color -s %s | FileCheck %s
+# RUN: %lldb --no-use-colors -s %s | FileCheck %s
 settings show use-color
 # CHECK: use-color (boolean) = false
 q
Index: lldb/docs/man/lldb.rst
===
--- lldb/docs/man/lldb.rst
+++ lldb/docs/man/lldb.rst
@@ -251,11 +251,16 @@
 
 The debugger can be started in several modes.
 
-Passing an executable as a positional argument prepares :program:`lldb` to
-debug the given executable. Arguments passed after -- are considered arguments
-to the debugged executable.
+Passing an executable as a positional argument prepares lldb to debug the given
+executable. To disambiguate between arguments passed to lldb and arguments
+passed to the debugged executable, arguments starting with a - must be passed
+after --.
 
-  lldb --arch x86_64 /path/to/program -- --arch arvm7
+  lldb --arch x86_64 /path/to/program program argument -- --arch arvm7
+
+For 

[Lldb-commits] [lldb] 82093e8 - [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-05-18T18:52:18-07:00
New Revision: 82093e8fb7d65486ff450d33bf386aabd0d194f7

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

LOG: [lldb/Driver] Fix handling on positional arguments

Before the transition to libOption it was possible to specify arguments
for the inferior without -- as long as they didn't start with a dash.

For example, the following invocations should all behave the same:

  $ lldb inferior inferior-arg
  $ lldb inferior -- inferior-arg
  $ lldb -- inferior inferior-arg

This patch fixes that behavior, documents it and adds a test to cover
the different combinations.

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

Added: 
lldb/test/Shell/Driver/TestPositionalArgs.test

Modified: 
lldb/docs/man/lldb.rst
lldb/test/Shell/Driver/TestNoUseColor.test
lldb/tools/driver/Driver.cpp

Removed: 




diff  --git a/lldb/docs/man/lldb.rst b/lldb/docs/man/lldb.rst
index b4972df1b601..a3a0736680ad 100644
--- a/lldb/docs/man/lldb.rst
+++ b/lldb/docs/man/lldb.rst
@@ -251,11 +251,16 @@ EXAMPLES
 
 The debugger can be started in several modes.
 
-Passing an executable as a positional argument prepares :program:`lldb` to
-debug the given executable. Arguments passed after -- are considered arguments
-to the debugged executable.
+Passing an executable as a positional argument prepares lldb to debug the given
+executable. To disambiguate between arguments passed to lldb and arguments
+passed to the debugged executable, arguments starting with a - must be passed
+after --.
 
-  lldb --arch x86_64 /path/to/program -- --arch arvm7
+  lldb --arch x86_64 /path/to/program program argument -- --arch arvm7
+
+For convenience, passing the executable after -- is also supported.
+
+  lldb --arch x86_64 -- /path/to/program program argument --arch arvm7
 
 Passing one of the attach options causes :program:`lldb` to immediately attach
 to the given process.

diff  --git a/lldb/test/Shell/Driver/TestNoUseColor.test 
b/lldb/test/Shell/Driver/TestNoUseColor.test
index 62735301fca8..61d03b308f78 100644
--- a/lldb/test/Shell/Driver/TestNoUseColor.test
+++ b/lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,4 @@
-# RUN: %lldb --no-use-color -s %s | FileCheck %s
+# RUN: %lldb --no-use-colors -s %s | FileCheck %s
 settings show use-color
 # CHECK: use-color (boolean) = false
 q

diff  --git a/lldb/test/Shell/Driver/TestPositionalArgs.test 
b/lldb/test/Shell/Driver/TestPositionalArgs.test
new file mode 100644
index ..327d006d814e
--- /dev/null
+++ b/lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,30 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb %t.foo -x bar -b baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s
+
+CHECK: Current executable set to {{.*}}foo
+CHECK: target.run-args "bar" "baz" "quux"
+
+RUN: %lldb -x -b %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -- %t.foo bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -f %t.foo -- bar -baz --quux | FileCheck %s --check-prefix 
DASH
+
+DASH: Current executable set to {{.*}}foo
+DASH: target.run-args "bar" "-baz" "--quux"
+
+RUN: %lldb -x -b %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+RUN: %lldb -x -b -f %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+
+UNKNOWN: warning: ignoring unknown option: -baz
+UNKNOWN: warning: ignoring unknown option: --quux
+UNKNOWN: Current executable set to {{.*}}foo
+UNKNOWN: target.run-args "bar"

diff  --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index b38423b28559..7b783ad9c3f4 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@ SBError Driver::ProcessArgs(const opt::InputArgList , 
bool ) {
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : 

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Much better!


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

https://reviews.llvm.org/D80165



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

[Lldb-commits] [lldb] 47cc6db - Re-land [Debug][CodeView] Emit fully qualified names for globals

[Reid Kleckner via lldb-commits]

Author: Reid Kleckner
Date: 2020-05-18T17:31:00-07:00
New Revision: 47cc6db928d063d96e11e70c196bd5601b2bdd06

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

LOG: Re-land [Debug][CodeView] Emit fully qualified names for globals

This reverts commit 525a591f0f48b9d54018bf5245f2abee09c9c1c8.

Fixed an issue with pointers to members based on typedefs. In this case,
LLVM would emit a second UDT. I fixed it by not passing the class type
to getTypeIndex when the base type is not a function type. lowerType
only uses the class type for direct function types. This suggests if we
have a PMF with a function typedef, there may be an issue, but that can
be solved separately.

Added: 
llvm/test/DebugInfo/COFF/udts-fixpoint.ll

Modified: 
lldb/test/Shell/SymbolFile/PDB/variables.test
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
llvm/test/DebugInfo/COFF/global-constants.ll
llvm/test/DebugInfo/COFF/global_visibility.ll
llvm/test/DebugInfo/COFF/globals.ll
llvm/test/DebugInfo/COFF/types-array-unsized.ll

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/PDB/variables.test 
b/lldb/test/Shell/SymbolFile/PDB/variables.test
index ae14f02754ce..a6c715360958 100644
--- a/lldb/test/Shell/SymbolFile/PDB/variables.test
+++ b/lldb/test/Shell/SymbolFile/PDB/variables.test
@@ -13,12 +13,12 @@ GLOBALS: SymbolFile pdb ([[MOD]])
 GLOBALS: CompileUnit{{.*}}, language = "c++", file = 
'{{.*}}\VariablesTest.cpp'
 GLOBALS-DAG:   Variable{{.*}}, name = "g_IntVar"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
-GLOBALS-DAG:   Variable{{.*}}, name = "m_StaticClassMember"
-GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "g_pConst"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "same_name_var"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
+GLOBALS-DAG:   Variable{{.*}}, name = "Class::m_StaticClassMember"
+GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "g_EnumVar"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "g_tls"

diff  --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp 
b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index f167cead4e2c..de2b9bcc58c7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -343,18 +343,6 @@ static std::string formatNestedName(ArrayRef 
QualifiedNameComponents,
   return FullyQualifiedName;
 }
 
-std::string CodeViewDebug::getFullyQualifiedName(const DIScope *Scope,
- StringRef Name) {
-  SmallVector QualifiedNameComponents;
-  collectParentScopeNames(Scope, QualifiedNameComponents);
-  return formatNestedName(QualifiedNameComponents, Name);
-}
-
-std::string CodeViewDebug::getFullyQualifiedName(const DIScope *Ty) {
-  const DIScope *Scope = Ty->getScope();
-  return getFullyQualifiedName(Scope, getPrettyScopeName(Ty));
-}
-
 struct CodeViewDebug::TypeLoweringScope {
   TypeLoweringScope(CodeViewDebug ) : CVD(CVD) { ++CVD.TypeEmissionLevel; }
   ~TypeLoweringScope() {
@@ -367,6 +355,22 @@ struct CodeViewDebug::TypeLoweringScope {
   CodeViewDebug 
 };
 
+std::string CodeViewDebug::getFullyQualifiedName(const DIScope *Scope,
+ StringRef Name) {
+  // Ensure types in the scope chain are emitted as soon as possible.
+  // This can create otherwise a situation where S_UDTs are emitted while
+  // looping in emitDebugInfoForUDTs.
+  TypeLoweringScope S(*this);
+  SmallVector QualifiedNameComponents;
+  collectParentScopeNames(Scope, QualifiedNameComponents);
+  return formatNestedName(QualifiedNameComponents, Name);
+}
+
+std::string CodeViewDebug::getFullyQualifiedName(const DIScope *Ty) {
+  const DIScope *Scope = Ty->getScope();
+  return getFullyQualifiedName(Scope, getPrettyScopeName(Ty));
+}
+
 TypeIndex CodeViewDebug::getScopeIndex(const DIScope *Scope) {
   // No scope means global scope and that uses the zero index.
   if (!Scope || isa(Scope))
@@ -1784,11 +1788,12 @@ translatePtrToMemberRep(unsigned SizeInBytes, bool 
IsPMF, unsigned Flags) {
 TypeIndex CodeViewDebug::lowerTypeMemberPointer(const DIDerivedType *Ty,
 PointerOptions PO) {
   assert(Ty->getTag() == dwarf::DW_TAG_ptr_to_member_type);
+  bool IsPMF = isa(Ty->getBaseType());
   TypeIndex ClassTI = getTypeIndex(Ty->getClassType());
-  TypeIndex PointeeTI = getTypeIndex(Ty->getBaseType(), Ty->getClassType());
+  TypeIndex PointeeTI =
+  getTypeIndex(Ty->getBaseType(), IsPMF ? Ty->getClassType() : nullptr);
   

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM.  These were useful behaviors, thanks for restoring them!


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

https://reviews.llvm.org/D80165



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

[Lldb-commits] [lldb] e3aa4cd - [lldb/test] Disable NSDate format check under _WIN32

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2020-05-18T16:51:47-07:00
New Revision: e3aa4cd9dbcee6441f51102e3958c35321698c67

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

LOG: [lldb/test] Disable NSDate format check under _WIN32

Disable the test which attempts to format an NSDate with a date_value of
0 on _WIN32.

When _WIN32 is defined, GetOSXEpoch returns a date that should be in
2001, but after this is passed through timegm (which, afaict isn't
portable?) the result is a date in 1970:

```
lldb-x64-windows-ninja\llvm-project\lldb\unittests\DataFormatter\MockTests.cpp(39):
 error:   Expected: *formatDateValue(0)
  Which is: "1970-01-01 00:00:00 Pacific Standard Time"
  To be equal to: "2001-01-01 00:00:00 UTC"
```

http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/4520/steps/test/logs/stdio

Added: 


Modified: 
lldb/unittests/DataFormatter/MockTests.cpp

Removed: 




diff  --git a/lldb/unittests/DataFormatter/MockTests.cpp 
b/lldb/unittests/DataFormatter/MockTests.cpp
index 752e3987dac9..1185d7bf2c9c 100644
--- a/lldb/unittests/DataFormatter/MockTests.cpp
+++ b/lldb/unittests/DataFormatter/MockTests.cpp
@@ -37,6 +37,10 @@ TEST(DataFormatterMockTest, NSDate) {
   EXPECT_EQ(formatDateValue(std::numeric_limits::max()), llvm::None);
   EXPECT_EQ(formatDateValue(std::numeric_limits::min()), llvm::None);
 
+  // FIXME: The formatting result is wrong on Windows because we adjust the
+  // epoch when _WIN32 is defined (see GetOSXEpoch).
+#ifndef _WIN32
   EXPECT_TRUE(
   llvm::StringRef(*formatDateValue(0)).startswith("2001-01-01 00:00:00"));
+#endif
 }



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

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 264753.
JDevlieghere added a comment.

Document the behavior in the help and man page.


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

https://reviews.llvm.org/D80165

Files:
  lldb/docs/man/lldb.rst
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestPositionalArgs.test
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
@@ -765,10 +760,15 @@
   The debugger can be started in several modes.
 
   Passing an executable as a positional argument prepares lldb to debug the
-  given executable. Arguments passed after -- are considered arguments to the
-  debugged executable.
+  given executable. To disambiguate between arguments passed to lldb and
+  arguments passed to the debugged executable, arguments starting with a - must
+  be passed after --.
+
+lldb --arch x86_64 /path/to/program program argument -- --arch arvm7
+
+  For convenience, passing the executable after -- is also supported.
 
-lldb --arch x86_64 /path/to/program -- --arch arvm7
+lldb --arch x86_64 -- /path/to/program program argument --arch arvm7
 
   Passing one of the attach options causes lldb to immediately attach to the
   given process.
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,30 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb %t.foo -x bar -b baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s
+
+CHECK: Current executable set to {{.*}}foo
+CHECK: target.run-args "bar" "baz" "quux"
+
+RUN: %lldb -x -b %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -- %t.foo bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -f %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+
+DASH: Current executable set to {{.*}}foo
+DASH: target.run-args "bar" "-baz" "--quux"
+
+RUN: %lldb -x -b %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix UNKNOWN
+RUN: %lldb -x -b -f %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix UNKNOWN
+
+UNKNOWN: warning: ignoring unknown option: -baz
+UNKNOWN: warning: ignoring unknown option: --quux
+UNKNOWN: Current executable set to {{.*}}foo
+UNKNOWN: target.run-args "bar"
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,4 @@
-# RUN: %lldb --no-use-color -s %s | FileCheck %s
+# RUN: %lldb --no-use-colors -s %s | FileCheck %s
 settings show use-color
 # CHECK: use-color (boolean) = false
 q
Index: lldb/docs/man/lldb.rst
===
--- lldb/docs/man/lldb.rst
+++ lldb/docs/man/lldb.rst
@@ -251,11 +251,16 @@
 
 The debugger can be started in several modes.
 
-Passing an executable as a positional argument prepares :program:`lldb` to
-debug the given executable. Arguments passed after -- are considered arguments
-to the debugged executable.
+Passing an executable as a positional argument prepares lldb to debug the given
+executable. To disambiguate between arguments passed to lldb and arguments
+passed to the debugged executable, arguments starting with a - must be passed
+after --.
 
-  lldb --arch x86_64 /path/to/program -- --arch arvm7
+  lldb --arch x86_64 /path/to/program program argument -- --arch arvm7
+
+For convenience, passing the executable after -- is also supported.
+
+  lldb --arch x86_64 -- /path/to/program program argument 

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 264745.
JDevlieghere edited the summary of this revision.
JDevlieghere added a comment.

Apparently I misremembered the old behavior. Jim refreshed my memory about what 
he expects the behavior to be link. I've reworked the patch to match that. The 
result is actually less complex than it was before.

Please ignore the old patch & description.


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

https://reviews.llvm.org/D80165

Files:
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestPositionalArgs.test
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
@@ -857,8 +852,9 @@
   }
 
   for (auto *arg : input_args.filtered(OPT_UNKNOWN)) {
-WithColor::warning() << "ignoring unknown option: " << arg->getSpelling()
- << '\n';
+if (!arg->isClaimed())
+  WithColor::warning() << "ignoring unknown option: " << arg->getSpelling()
+   << '\n';
   }
 
   if (auto exit_code = InitializeReproducer(input_args)) {
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,29 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s
+
+CHECK: Current executable set to {{.*}}foo
+CHECK: target.run-args "bar" "baz" "quux"
+
+RUN: %lldb -x -b %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -- %t.foo bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -f %t.foo -- bar -baz --quux | FileCheck %s --check-prefix 
DASH
+
+DASH: Current executable set to {{.*}}foo
+DASH: target.run-args "bar" "-baz" "--quux"
+
+RUN: %lldb -x -b %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+RUN: %lldb -x -b -f %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+
+UNKNOWN: warning: ignoring unknown option: -baz
+UNKNOWN: warning: ignoring unknown option: --quux
+UNKNOWN: Current executable set to {{.*}}foo
+UNKNOWN: target.run-args "bar"
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,4 @@
-# RUN: %lldb --no-use-color -s %s | FileCheck %s
+# RUN: %lldb --no-use-colors -s %s | FileCheck %s
 settings show use-color
 # CHECK: use-color (boolean) = false
 q


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
@@ -857,8 +852,9 @@
   }
 
   for (auto *arg : input_args.filtered(OPT_UNKNOWN)) {
-WithColor::warning() << "ignoring unknown option: " << arg->getSpelling()
- << '\n';
+if (!arg->isClaimed())
+  WithColor::warning() << 

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 264747.
JDevlieghere added a comment.

Add test for `lldb inferior -b inferior-arg -- inferior-arg2` as Greg pointed 
out.


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

https://reviews.llvm.org/D80165

Files:
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestPositionalArgs.test
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,30 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb %t.foo -x bar -b baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s
+
+CHECK: Current executable set to {{.*}}foo
+CHECK: target.run-args "bar" "baz" "quux"
+
+RUN: %lldb -x -b %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -- %t.foo bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -f %t.foo -- bar -baz --quux | FileCheck %s --check-prefix 
DASH
+
+DASH: Current executable set to {{.*}}foo
+DASH: target.run-args "bar" "-baz" "--quux"
+
+RUN: %lldb -x -b %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+RUN: %lldb -x -b -f %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+
+UNKNOWN: warning: ignoring unknown option: -baz
+UNKNOWN: warning: ignoring unknown option: --quux
+UNKNOWN: Current executable set to {{.*}}foo
+UNKNOWN: target.run-args "bar"
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,4 @@
-# RUN: %lldb --no-use-color -s %s | FileCheck %s
+# RUN: %lldb --no-use-colors -s %s | FileCheck %s
 settings show use-color
 # CHECK: use-color (boolean) = false
 q


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,30 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb %t.foo -x bar -b baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 264746.

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

https://reviews.llvm.org/D80165

Files:
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestPositionalArgs.test
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,29 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s
+
+CHECK: Current executable set to {{.*}}foo
+CHECK: target.run-args "bar" "baz" "quux"
+
+RUN: %lldb -x -b %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -- %t.foo bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -f %t.foo -- bar -baz --quux | FileCheck %s --check-prefix 
DASH
+
+DASH: Current executable set to {{.*}}foo
+DASH: target.run-args "bar" "-baz" "--quux"
+
+RUN: %lldb -x -b %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+RUN: %lldb -x -b -f %t.foo bar -baz --quux 2>&1 | FileCheck %s --check-prefix 
UNKNOWN
+
+UNKNOWN: warning: ignoring unknown option: -baz
+UNKNOWN: warning: ignoring unknown option: --quux
+UNKNOWN: Current executable set to {{.*}}foo
+UNKNOWN: target.run-args "bar"
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,4 @@
-# RUN: %lldb --no-use-color -s %s | FileCheck %s
+# RUN: %lldb --no-use-colors -s %s | FileCheck %s
 settings show use-color
 # CHECK: use-color (boolean) = false
 q


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -361,13 +361,8 @@
   if (m_option_data.m_process_name.empty() &&
   m_option_data.m_process_pid == LLDB_INVALID_PROCESS_ID) {
 
-// If the option data args array is empty that means the file was not
-// specified with -f and we need to get it from the input args.
-if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
-  }
-}
+for (auto *arg : args.filtered(OPT_INPUT))
+  m_option_data.m_args.push_back(arg->getAsString((args)));
 
 // Any argument following -- is an argument for the inferior.
 if (auto *arg = args.getLastArgNoClaim(OPT_REM)) {
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,29 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -- %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo -- bar baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar -- baz quux | FileCheck %s
+RUN: %lldb -x -b -f %t.foo bar baz -- quux | FileCheck %s
+
+CHECK: Current executable set to {{.*}}foo
+CHECK: target.run-args "bar" "baz" "quux"
+
+RUN: %lldb -x -b %t.foo -- bar -baz --quux | FileCheck %s --check-prefix DASH
+RUN: %lldb -x -b -- %t.foo bar -baz --quux | FileCheck 

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Not sure why you need to track "beginning and end".  Does libOption not pull 
out all the options (and their option values if they have them) before handing 
you the command line?  If not, then this probably is not worth doing.  But if 
it does, this should be straightforward since the only arguments lldb takes 
(prior to seeing a --) are meant to be arguments to the program when you run it.


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

https://reviews.llvm.org/D80165



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

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So when talking about "prior to libOption", are we talking getopt()? I believe 
that getopt would have an issue with:

  $ lldb inferior --inferior-arg 

As it would not understand the argument. Or was there an intermediate llvm 
option parser we were using after getopt()?

And BTW, getopt allows arguments and options to be interspersed:

  $ lldb --lldb-option1 inferior --lldb-option2=123 inferior-arg1 -- 
--inferior-opt=444 inferior-arg2


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

https://reviews.llvm.org/D80165



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

[Lldb-commits] [lldb] fff3a84 - [lldb/test] Relax NSDate mock test for non-Apple platforms

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2020-05-18T16:00:10-07:00
New Revision: fff3a8464d4d518c7086c928fba967908eb294d7

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

LOG: [lldb/test] Relax NSDate mock test for non-Apple platforms

On Ubuntu, a formatted date prints as "GMT" instead of "UTC", which is
ok.

http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/4520/steps/test/logs/stdio

Added: 


Modified: 
lldb/unittests/DataFormatter/MockTests.cpp

Removed: 




diff  --git a/lldb/unittests/DataFormatter/MockTests.cpp 
b/lldb/unittests/DataFormatter/MockTests.cpp
index 0042888243f7..752e3987dac9 100644
--- a/lldb/unittests/DataFormatter/MockTests.cpp
+++ b/lldb/unittests/DataFormatter/MockTests.cpp
@@ -9,6 +9,7 @@
 #include "lldb/DataFormatters/Mock.h"
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -36,5 +37,6 @@ TEST(DataFormatterMockTest, NSDate) {
   EXPECT_EQ(formatDateValue(std::numeric_limits::max()), llvm::None);
   EXPECT_EQ(formatDateValue(std::numeric_limits::min()), llvm::None);
 
-  EXPECT_EQ(*formatDateValue(0), "2001-01-01 00:00:00 UTC");
+  EXPECT_TRUE(
+  llvm::StringRef(*formatDateValue(0)).startswith("2001-01-01 00:00:00"));
 }



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

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

One thing the current approach allows is `foo bar -o 'run' baz` which will `set 
target.run-args` to ` "bar" "baz" `. I guess that's not really desirable, but 
I'm not sure it outweighs the added complexity of tracking the beginning and 
thee end.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D80165



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

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, labath, clayborg.
Herald added a subscriber: teemperor.

Before the transition to `libOption` it was possible to specify arguments for 
the inferior without `--`. For instance, the next two invocations behaved the 
same.

  $ lldb inferior --inferior-arg 
  $ lldb inferior -- --inferior-arg 

Now, this prints a warning:

  $ lldb inferior --inferior-arg 
  warning: ignoring unknown option: --inferior-arg

This patch reinstates the old behavior by using a heuristic:

1. Look for the index of the inferior in the argument list.
2. Append all the positional or unknown argument with an index greater than the 
inferior to the inferior argument list.
3. Append all the arguments passed after `--`.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D80165

Files:
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestPositionalArgs.test
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -364,8 +364,24 @@
 // If the option data args array is empty that means the file was not
 // specified with -f and we need to get it from the input args.
 if (m_option_data.m_args.empty()) {
-  if (auto *arg = args.getLastArgNoClaim(OPT_INPUT)) {
-m_option_data.m_args.push_back(arg->getAsString((args)));
+  // Find the first input argument.
+  unsigned first_input_arg = std::numeric_limits::max();
+  for (auto *arg : args.filtered(OPT_INPUT)) {
+first_input_arg = arg->getIndex();
+break;
+  }
+
+  // Iterate over all the positional, unknown and remaining arguments. The
+  // order will match how they were specified on the command line.
+  for (auto *arg : args.filtered(OPT_INPUT, OPT_UNKNOWN)) {
+// Any argument with an index smaller than the first input argument
+// cannot be an argument to the inferior.
+if (arg->getIndex() >= first_input_arg) {
+  // Claim the argument so we don't report it as unknown.
+  arg->claim();
+  // Add it to the argument list.
+  m_option_data.m_args.push_back(arg->getAsString((args)));
+}
   }
 }
 
@@ -856,11 +872,6 @@
 return 0;
   }
 
-  for (auto *arg : input_args.filtered(OPT_UNKNOWN)) {
-WithColor::warning() << "ignoring unknown option: " << arg->getSpelling()
- << '\n';
-  }
-
   if (auto exit_code = InitializeReproducer(input_args)) {
 return *exit_code;
   }
@@ -892,6 +903,13 @@
 
 bool exiting = false;
 SBError error(driver.ProcessArgs(input_args, exiting));
+
+for (auto *arg : input_args.filtered(OPT_UNKNOWN)) {
+  if (!arg->isClaimed())
+WithColor::warning() << "ignoring unknown option: " << 
arg->getSpelling()
+<< '\n';
+}
+
 if (error.Fail()) {
   exit_code = 1;
   if (const char *error_cstr = error.GetCString())
Index: lldb/test/Shell/Driver/TestPositionalArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestPositionalArgs.test
@@ -0,0 +1,24 @@
+RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t.foo
+
+RUN: %lldb -x -b %t.foo -- bar baz quux | FileCheck %s --check-prefix=FOO
+RUN: %lldb -x -b %t.foo bar -- baz quux | FileCheck %s --check-prefix=FOO
+RUN: %lldb -x -b %t.foo bar baz -- quux | FileCheck %s --check-prefix=FOO
+RUN: %lldb -x -b %t.foo bar baz quux | FileCheck %s --check-prefix=FOO
+RUN: not %lldb -x -b %t.foo bar baz quux -o 'bogus' 2>&1 | FileCheck %s 
--check-prefix=FOO --check-prefix LLDB-BOGUS
+RUN: %lldb -x -b --unknown %t.foo bar baz quux 2>&1 | FileCheck %s 
--check-prefix LLDB-UNKNOWN --check-prefix FOO
+
+LLDB-UNKNOWN: warning: ignoring unknown option: --unknown
+FOO: Current executable set to {{.*}}foo
+FOO: target.run-args  "bar" "baz" "quux"
+LLDB-BOGUS: error: 'bogus' is not a valid command.
+
+RUN: %lldb -x -b %t.foo bar baz quux -- -o 'bogus' 2>&1 | FileCheck %s 
--check-prefix INFERIOR-BOGUS
+
+INFERIOR-BOGUS: Current executable set to {{.*}}foo
+INFERIOR-BOGUS: target.run-args  "bar" "baz" "quux" "-o" "bogus"
+
+RUN: %lldb -x -b %t.foo bar baz quux --unknown 2>&1 | FileCheck %s 
--check-prefix INFERIOR-UNKNOWN
+RUN: %lldb -x -b %t.foo bar baz quux -- --unknown 2>&1 | FileCheck %s 
--check-prefix INFERIOR-UNKNOWN
+
+INFERIOR-UNKNOWN: Current executable set to {{.*}}foo
+INFERIOR-UNKNOWN: target.run-args  "bar" "baz" "quux" "--unknown"
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,4 @@
-# RUN: %lldb --no-use-color -s %s | FileCheck %s
+# RUN: %lldb --no-use-colors -s %s | FileCheck %s
 settings show use-color
 # CHECK: 

[Lldb-commits] [PATCH] D80159: [lldb/Properties] Move OSPluginReportsAllThreads from Target to Process

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: jingham.

This is what Jim wanted originally.

rdar://problem/61236293


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D80159

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py

Index: lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
===
--- lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
+++ lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
@@ -44,7 +44,7 @@
 """Test that the Python operating system plugin works correctly"""
 
 # Our OS plugin does NOT report all threads:
-result = self.dbg.HandleCommand("settings set target.experimental.os-plugin-reports-all-threads false")
+result = self.dbg.HandleCommand("settings set process.experimental.os-plugin-reports-all-threads false")
 
 python_os_plugin_path = os.path.join(self.getSourceDir(),
  "operating_system.py")
Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -1,13 +1,9 @@
 include "../../include/lldb/Core/PropertiesBase.td"
 
-let Definition = "experimental" in {
+let Definition = "target_experimental" in {
   def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
 Global, DefaultTrue,
 Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
-  def OSPluginReportsAllThreads: Property<"os-plugin-reports-all-threads", "Boolean">,
-Global,
-DefaultTrue,
-Desc<"Set to False if your OS Plugins doesn't report all threads on each stop.">;
 }
 
 let Definition = "target" in {
@@ -169,6 +165,13 @@
 Desc<"Always install the main executable when connected to a remote platform.">;
 }
 
+let Definition = "process_experimental" in {
+  def OSPluginReportsAllThreads: Property<"os-plugin-reports-all-threads", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"Set to False if your OS Plugins doesn't report all threads on each stop.">;
+}
+
 let Definition = "process" in {
   def DisableMemCache: Property<"disable-memory-cache", "Boolean">,
 DefaultFalse,
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3373,11 +3373,11 @@
 };
 
 // TargetProperties
-#define LLDB_PROPERTIES_experimental
+#define LLDB_PROPERTIES_target_experimental
 #include "TargetProperties.inc"
 
 enum {
-#define LLDB_PROPERTIES_experimental
+#define LLDB_PROPERTIES_target_experimental
 #include "TargetPropertiesEnum.inc"
 };
 
@@ -3391,7 +3391,7 @@
 TargetExperimentalProperties::TargetExperimentalProperties()
 : Properties(OptionValuePropertiesSP(
   new TargetExperimentalOptionValueProperties())) {
-  m_collection_sp->Initialize(g_experimental_properties);
+  m_collection_sp->Initialize(g_target_experimental_properties);
 }
 
 // TargetProperties
@@ -3487,34 +3487,6 @@
 true);
 }
 
-bool TargetProperties::GetOSPluginReportsAllThreads() const {
-  const bool fail_value = true;
-  const Property *exp_property =
-  m_collection_sp->GetPropertyAtIndex(nullptr, true, ePropertyExperimental);
-  OptionValueProperties *exp_values =
-  exp_property->GetValue()->GetAsProperties();
-  if (!exp_values)
-return fail_value;
-
-  return 
-  exp_values->GetPropertyAtIndexAsBoolean(nullptr, 
-  ePropertyOSPluginReportsAllThreads,
-  fail_value);
-}
-
-void TargetProperties::SetOSPluginReportsAllThreads(bool does_report) {
-  const Property *exp_property =
-  m_collection_sp->GetPropertyAtIndex(nullptr, true, ePropertyExperimental);
-  OptionValueProperties *exp_values =
-  exp_property->GetValue()->GetAsProperties();
-  if (exp_values)
-exp_values->SetPropertyAtIndexAsBoolean(nullptr, 
-ePropertyOSPluginReportsAllThreads,
-does_report);
-}
-
-
-
 ArchSpec TargetProperties::GetDefaultArchitecture() const {
   OptionValueArch *value = m_collection_sp->GetPropertyAtIndexAsOptionValueArch(
   nullptr, ePropertyDefaultArch);
@@ -4084,7 +4056,7 @@
   return 

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Vedant Kumar via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
vsk marked an inline comment as done.
Closed by commit rGb783f70a4257: [lldb/DataFormatter] Check for overflow when 
finding NSDate epoch (authored by vsk).

Changed prior to commit:
  https://reviews.llvm.org/D80150?vs=264695=264708#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150

Files:
  lldb/include/lldb/DataFormatters/Mock.h
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/unittests/DataFormatter/CMakeLists.txt
  lldb/unittests/DataFormatter/MockTests.cpp

Index: lldb/unittests/DataFormatter/MockTests.cpp
===
--- /dev/null
+++ lldb/unittests/DataFormatter/MockTests.cpp
@@ -0,0 +1,40 @@
+//===-- MockTests.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/DataFormatters/Mock.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/Optional.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace lldb_private;
+
+// Try to format the date_value field of a NSDate.
+static llvm::Optional formatDateValue(double date_value) {
+  StreamString s;
+  bool succcess = formatters::NSDate::FormatDateValue(date_value, s);
+  if (succcess)
+return std::string(s.GetData());
+  return llvm::None;
+}
+
+TEST(DataFormatterMockTest, NSDate) {
+  EXPECT_EQ(*formatDateValue(-63114076800), "0001-12-30 00:00:00 +");
+
+  // Can't convert the date_value to a time_t.
+  EXPECT_EQ(formatDateValue(std::numeric_limits::max() + 1),
+llvm::None);
+  EXPECT_EQ(formatDateValue(std::numeric_limits::min() - 1),
+llvm::None);
+
+  // Can't add the macOS epoch to the converted date_value (the add overflows).
+  EXPECT_EQ(formatDateValue(std::numeric_limits::max()), llvm::None);
+  EXPECT_EQ(formatDateValue(std::numeric_limits::min()), llvm::None);
+
+  EXPECT_EQ(*formatDateValue(0), "2001-01-01 00:00:00 UTC");
+}
Index: lldb/unittests/DataFormatter/CMakeLists.txt
===
--- lldb/unittests/DataFormatter/CMakeLists.txt
+++ lldb/unittests/DataFormatter/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(LLDBFormatterTests
   FormatManagerTests.cpp
+  MockTests.cpp
   StringPrinterTests.cpp
 
   LINK_LIBS
Index: lldb/source/Plugins/Language/ObjC/Cocoa.cpp
===
--- lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/DataFormatters/Mock.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/DataFormatters/TypeSummary.h"
 #include "lldb/Host/Time.h"
@@ -27,6 +28,7 @@
 
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/bit.h"
+#include "llvm/Support/CheckedArithmetic.h"
 
 #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h"
 
@@ -785,6 +787,34 @@
   return llvm::bit_cast(decodedBits);
 }
 
+bool lldb_private::formatters::NSDate::FormatDateValue(double date_value,
+   Stream ) {
+  if (date_value == -63114076800) {
+stream.Printf("0001-12-30 00:00:00 +");
+return true;
+  }
+
+  if (date_value > std::numeric_limits::max() ||
+  date_value < std::numeric_limits::min())
+return false;
+
+  time_t epoch = GetOSXEpoch();
+  if (auto osx_epoch = llvm::checkedAdd(epoch, (time_t)date_value))
+epoch = *osx_epoch;
+  else
+return false;
+  tm *tm_date = gmtime();
+  if (!tm_date)
+return false;
+  std::string buffer(1024, 0);
+  if (strftime([0], 1023, "%Z", tm_date) == 0)
+return false;
+  stream.Printf("%04d-%02d-%02d %02d:%02d:%02d %s", tm_date->tm_year + 1900,
+tm_date->tm_mon + 1, tm_date->tm_mday, tm_date->tm_hour,
+tm_date->tm_min, tm_date->tm_sec, buffer.c_str());
+  return true;
+}
+
 bool lldb_private::formatters::NSDateSummaryProvider(
 ValueObject , Stream , const TypeSummaryOptions ) {
   ProcessSP process_sp = valobj.GetProcessSP();
@@ -828,6 +858,16 @@
 if (descriptor->GetTaggedPointerInfo(_bits, _bits)) {
   date_value_bits = ((value_bits << 8) | (info_bits << 4));
   memcpy(_value, _value_bits, sizeof(date_value_bits));
+
+  // Accomodate the __NSTaggedDate format introduced in Foundation 1600.
+  if (class_name == g___NSTaggedDate) {
+auto *apple_runtime = llvm::dyn_cast_or_null(
+ObjCLanguageRuntime::Get(*process_sp));
+

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Vedant Kumar via Phabricator via lldb-commits]

vsk marked 2 inline comments as done.
vsk added a comment.

Thanks!




Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:801-804
+  // this snippet of code assumes that time_t == seconds since Jan-1-1970 this
+  // is generally true and POSIXly happy, but might break if a library vendor
+  // decides to get creative
+  time_t epoch = GetOSXEpoch();

davide wrote:
> I think you can drop this comment [you just moved it, but it feels irrelevant 
> at this point because there's only one vendor].
I'll fix this before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150



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

[Lldb-commits] [lldb] b783f70 - [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2020-05-18T13:12:00-07:00
New Revision: b783f70a42575a5d9147bea1ac97e872370fe55b

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

LOG: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

Summary:
Fixes UBSan-reported issues where the date value inside of an
uninitialized NSDate overflows the 64-bit epoch.

rdar://61774575

Reviewers: JDevlieghere, mib, teemperor

Subscribers: mgorny, lldb-commits

Tags: #lldb

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

Added: 
lldb/include/lldb/DataFormatters/Mock.h
lldb/unittests/DataFormatter/MockTests.cpp

Modified: 
lldb/source/Plugins/Language/ObjC/Cocoa.cpp
lldb/unittests/DataFormatter/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/Mock.h 
b/lldb/include/lldb/DataFormatters/Mock.h
new file mode 100644
index ..b3fc10cd2e51
--- /dev/null
+++ b/lldb/include/lldb/DataFormatters/Mock.h
@@ -0,0 +1,26 @@
+//===-- Mock.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_DATAFORMATTERS_MOCK_H
+#define LLDB_DATAFORMATTERS_MOCK_H
+
+namespace lldb_private {
+
+class Stream;
+
+namespace formatters {
+namespace NSDate {
+
+/// Format the date_value field of a NSDate.
+bool FormatDateValue(double date_value, Stream );
+
+} // namespace NSDate
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // LLDB_DATAFORMATTERS_MOCK_H

diff  --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp 
b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
index 8a44811dd36b..1ad443b8b74e 100644
--- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/DataFormatters/Mock.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/DataFormatters/TypeSummary.h"
 #include "lldb/Host/Time.h"
@@ -27,6 +28,7 @@
 
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/bit.h"
+#include "llvm/Support/CheckedArithmetic.h"
 
 #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h"
 
@@ -785,6 +787,34 @@ static double decodeTaggedTimeInterval(uint64_t 
encodedTimeInterval) {
   return llvm::bit_cast(decodedBits);
 }
 
+bool lldb_private::formatters::NSDate::FormatDateValue(double date_value,
+   Stream ) {
+  if (date_value == -63114076800) {
+stream.Printf("0001-12-30 00:00:00 +");
+return true;
+  }
+
+  if (date_value > std::numeric_limits::max() ||
+  date_value < std::numeric_limits::min())
+return false;
+
+  time_t epoch = GetOSXEpoch();
+  if (auto osx_epoch = llvm::checkedAdd(epoch, (time_t)date_value))
+epoch = *osx_epoch;
+  else
+return false;
+  tm *tm_date = gmtime();
+  if (!tm_date)
+return false;
+  std::string buffer(1024, 0);
+  if (strftime([0], 1023, "%Z", tm_date) == 0)
+return false;
+  stream.Printf("%04d-%02d-%02d %02d:%02d:%02d %s", tm_date->tm_year + 1900,
+tm_date->tm_mon + 1, tm_date->tm_mday, tm_date->tm_hour,
+tm_date->tm_min, tm_date->tm_sec, buffer.c_str());
+  return true;
+}
+
 bool lldb_private::formatters::NSDateSummaryProvider(
 ValueObject , Stream , const TypeSummaryOptions ) {
   ProcessSP process_sp = valobj.GetProcessSP();
@@ -828,6 +858,16 @@ bool lldb_private::formatters::NSDateSummaryProvider(
 if (descriptor->GetTaggedPointerInfo(_bits, _bits)) {
   date_value_bits = ((value_bits << 8) | (info_bits << 4));
   memcpy(_value, _value_bits, sizeof(date_value_bits));
+
+  // Accomodate the __NSTaggedDate format introduced in Foundation 1600.
+  if (class_name == g___NSTaggedDate) {
+auto *apple_runtime = llvm::dyn_cast_or_null(
+ObjCLanguageRuntime::Get(*process_sp));
+if (!apple_runtime)
+  return false;
+if (apple_runtime->GetFoundationVersion() >= 1600)
+  date_value = decodeTaggedTimeInterval(value_bits << 4);
+  }
 } else {
   llvm::Triple triple(
   process_sp->GetTarget().GetArchitecture().GetTriple());
@@ -850,34 +890,7 @@ bool lldb_private::formatters::NSDateSummaryProvider(
   } else
 return false;
 
-  if (date_value == -63114076800) {
-stream.Printf("0001-12-30 00:00:00 +");
-return true;
-  }
-
-  // Accomodate for the __NSTaggedDate format 

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Davide already covered what I was going to say :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150



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

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/DataFormatters/Mock.h:1-8
+//===-- Mock.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//

Neat way to test.



Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:801-804
+  // this snippet of code assumes that time_t == seconds since Jan-1-1970 this
+  // is generally true and POSIXly happy, but might break if a library vendor
+  // decides to get creative
+  time_t epoch = GetOSXEpoch();

I think you can drop this comment [you just moved it, but it feels irrelevant 
at this point because there's only one vendor].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150



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

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Looking, as I touched `NSDate` last year.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150



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

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: JDevlieghere, mib, teemperor.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

Fixes UBSan-reported issues where the date value inside of an
uninitialized NSDate overflows the 64-bit epoch.

rdar://61774575


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80150

Files:
  lldb/include/lldb/DataFormatters/Mock.h
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/unittests/DataFormatter/CMakeLists.txt
  lldb/unittests/DataFormatter/MockTests.cpp

Index: lldb/unittests/DataFormatter/MockTests.cpp
===
--- /dev/null
+++ lldb/unittests/DataFormatter/MockTests.cpp
@@ -0,0 +1,40 @@
+//===-- MockTests.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/DataFormatters/Mock.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/Optional.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace lldb_private;
+
+// Try to format the date_value field of a NSDate.
+static llvm::Optional formatDateValue(double date_value) {
+  StreamString s;
+  bool succcess = formatters::NSDate::FormatDateValue(date_value, s);
+  if (succcess)
+return std::string(s.GetData());
+  return llvm::None;
+}
+
+TEST(DataFormatterMockTest, NSDate) {
+  EXPECT_EQ(*formatDateValue(-63114076800), "0001-12-30 00:00:00 +");
+
+  // Can't convert the date_value to a time_t.
+  EXPECT_EQ(formatDateValue(std::numeric_limits::max() + 1),
+llvm::None);
+  EXPECT_EQ(formatDateValue(std::numeric_limits::min() - 1),
+llvm::None);
+
+  // Can't add the macOS epoch to converted the date_value (the add overflows).
+  EXPECT_EQ(formatDateValue(std::numeric_limits::max()), llvm::None);
+  EXPECT_EQ(formatDateValue(std::numeric_limits::min()), llvm::None);
+
+  EXPECT_EQ(*formatDateValue(0), "2001-01-01 00:00:00 UTC");
+}
Index: lldb/unittests/DataFormatter/CMakeLists.txt
===
--- lldb/unittests/DataFormatter/CMakeLists.txt
+++ lldb/unittests/DataFormatter/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(LLDBFormatterTests
   FormatManagerTests.cpp
+  MockTests.cpp
   StringPrinterTests.cpp
 
   LINK_LIBS
Index: lldb/source/Plugins/Language/ObjC/Cocoa.cpp
===
--- lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/DataFormatters/Mock.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/DataFormatters/TypeSummary.h"
 #include "lldb/Host/Time.h"
@@ -27,6 +28,7 @@
 
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/bit.h"
+#include "llvm/Support/CheckedArithmetic.h"
 
 #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h"
 
@@ -785,6 +787,37 @@
   return llvm::bit_cast(decodedBits);
 }
 
+bool lldb_private::formatters::NSDate::FormatDateValue(double date_value,
+   Stream ) {
+  if (date_value == -63114076800) {
+stream.Printf("0001-12-30 00:00:00 +");
+return true;
+  }
+
+  if (date_value > std::numeric_limits::max() ||
+  date_value < std::numeric_limits::min())
+return false;
+
+  // this snippet of code assumes that time_t == seconds since Jan-1-1970 this
+  // is generally true and POSIXly happy, but might break if a library vendor
+  // decides to get creative
+  time_t epoch = GetOSXEpoch();
+  if (auto osx_epoch = llvm::checkedAdd(epoch, (time_t)date_value))
+epoch = *osx_epoch;
+  else
+return false;
+  tm *tm_date = gmtime();
+  if (!tm_date)
+return false;
+  std::string buffer(1024, 0);
+  if (strftime([0], 1023, "%Z", tm_date) == 0)
+return false;
+  stream.Printf("%04d-%02d-%02d %02d:%02d:%02d %s", tm_date->tm_year + 1900,
+tm_date->tm_mon + 1, tm_date->tm_mday, tm_date->tm_hour,
+tm_date->tm_min, tm_date->tm_sec, buffer.c_str());
+  return true;
+}
+
 bool lldb_private::formatters::NSDateSummaryProvider(
 ValueObject , Stream , const TypeSummaryOptions ) {
   ProcessSP process_sp = valobj.GetProcessSP();
@@ -828,6 +861,16 @@
 if (descriptor->GetTaggedPointerInfo(_bits, _bits)) {
   date_value_bits = ((value_bits << 8) | (info_bits << 4));
   memcpy(_value, _value_bits, sizeof(date_value_bits));
+
+  // Accomodate for the __NSTaggedDate format introduced in Foundation 1600.
+  if (class_name == g___NSTaggedDate) {
+auto *apple_runtime 

[Lldb-commits] [PATCH] D79447: [Debug][CodeView] Emit fully qualified names for globals

[Reid Kleckner via Phabricator via lldb-commits]

rnk added a comment.

I will take a look and try to reland this, since I requested the assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79447



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

[Lldb-commits] [PATCH] D78603: remove unused function LLDBSwigPython_GetIndexOfChildWithName

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna abandoned this revision.
lawrence_danna added a comment.

despite there being no references to this function, it is not actually unused.

This change causes `TestFormattersSBAPI.py` to fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78603



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

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I admit I don't fully understand the details of that CL and how it may interact 
with this one. However, I can say that I verified this CL with an Android 
device connected over IPv6.

So, I think this CL is ready to be submitted. (I don't have commit access).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757



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

[Lldb-commits] [lldb] 2084330 - [lldb/Reproducers] Add skipIfReproducer to more tests

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-05-18T10:13:01-07:00
New Revision: 2084330e41d301cf9eaa3495d8968bff70846c7b

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

LOG: [lldb/Reproducers] Add skipIfReproducer to more tests

Mark more tests as unsupported with reproducers.

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
lldb/test/API/functionalities/load_unload/TestLoadUnload.py
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
lldb/test/API/functionalities/postmortem/netbsd-core/TestNetBSDCore.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
 
b/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
index b20490f3cefd..931326b32291 100644
--- 
a/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
+++ 
b/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
@@ -51,6 +51,7 @@ def setUp(self):
 self.thread = 
lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoint1)
 self.assertIsNotNone(self.thread, "Didn't stop at breakpoint 1.")
 
+@skipIfReproducer
 def test_step_instruction(self):
 # Count instructions between breakpoint_1 and breakpoint_4
 contextList = self.target.FindFunctions('main', 
lldb.eFunctionNameTypeAuto)

diff  --git 
a/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py 
b/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
index ed17d9b36b6b..78f3feae6ff6 100644
--- a/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
+++ b/lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -20,6 +20,7 @@ class TestDeletedExecutable(TestBase):
 triple=no_match('aarch64-.*-android'))
 # determining the architecture of the process fails
 @expectedFailureNetBSD
+@skipIfReproducer # File synchronization is not supported during replay.
 def test(self):
 self.build()
 exe = self.getBuildArtifact("a.out")

diff  --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py 
b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
index 7188fa32a154..e0013ccd93fa 100644
--- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -95,6 +95,7 @@ def setSvr4Support(self, enabled):
 @not_remote_testsuite_ready
 @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic 
libraries work 
diff erently
 @expectedFailureNetBSD
+@skipIfReproducer # VFS is a snapshot.
 def test_modules_search_paths(self):
 """Test target modules list after loading a 
diff erent copy of the library libd.dylib, and verifies that it works with 
'target modules search-paths add'."""
 if self.platformIsDarwin():

diff  --git 
a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py 
b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
index 63bb02e5eb60..e0046f710889 100644
--- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -38,29 +38,34 @@ class LinuxCoreTestCase(TestBase):
 
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("AArch64")
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_aarch64(self):
 """Test that lldb can read the process information from an aarch64 
linux core file."""
 self.do_test("linux-aarch64", self._aarch64_pid, 
self._aarch64_regions, "a.out")
 
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_i386(self):
 """Test that lldb can read the process information from an i386 linux 
core file."""
 self.do_test("linux-i386", self._i386_pid, self._i386_regions, "a.out")
 
 @skipIfLLVMTargetMissing("Mips")
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_mips_o32(self):
 """Test that lldb can read the process information from an MIPS O32 
linux core file."""
 self.do_test("linux-mipsel-gnuabio32", self._mips_o32_pid,
 self._mips_regions, "linux-mipsel-gn")
 
 @skipIfLLVMTargetMissing("Mips")
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_mips_n32(self):
 """Test that lldb can read the process 

[Lldb-commits] [PATCH] D77043: Fix process gdb-remote usage of value_regs/invalidate_regs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Sorry, about the delay. I had to find some time to dig into the code again, to 
understand all of the interactions.

It seems that we (lldb) have painted ourselves in the corner somewhat. The 
thing I did not realize until now is that the "lldb" scheme is documented to be 
the same as the index into the register info array. (I believed it was used 
that way, but I didn't expect this level of intent in it.)  The relationship 
between `qRegisterInfo` and `p` register numbers is not mentioned explicitly 
anywhere, but reading the documentation of `qRegisterInfo`, I do get the 
impression that it was intended for them to match.

If we take these two things as a given, we are left with a bunch of sub-optimal 
alternatives, most of which we have tried already. Basically, once we start 
having targets with multiple optional register sets, the only way to reconcile 
these two requirements is to introduce an extra renumbering _somewhere_. The 
only reason we got away with this so far is because register sets tend to be 
additive -- the presence of one register set usually implies the presence of 
all "older" sets as well. And the register context interfaces offers us a very 
crude tool (GetUserRegisterCount) to weed out any weird registers, by placing 
them last. This isn't going to be a maintainable long term state, as today's 
architectures are starting to have a lot of different "optional" registers. 
(Intel is not promising that all cpus will always have the new MPX registers, 
for example).

I do believe something will need to be done about that. And I hope that 
something does not involve adding a new numbering scheme, because I believe we 
have too many of those already. I think the best solution would be to drop the 
"lldb regnum == reginfo index" requirement. It's redundant because the index 
can be computed by subtracting the RegisterInfo pointer from the beginning of 
the reginfo vector, and it enforces a linear order on the register descriptions 
(or forces the usage of complicated renumbering schemes). Without that 
restriction, it would be a simple thing for NativeRegisterContexts to only 
expose the registers they really want to expose, and there wouldn't be a need 
for renumberings at the gdb-remote level. I've done some experimenting with 
that, and while it required changes in a lot if places, the changes were not 
too complicated, and I did not run into any fundamental problems. Most of the 
time, the changes actually ended up simplifying the code.

Now, as for your patch set, I don't think that it should be on you to implement 
all of this, though I would strongly encourage it, and promise to help along. I 
think this last version is something that we could go with, even though it's 
definitely still less than ideal. But before we go through with that, let me 
circle back to one of the previous ideas. A bunch of revisions ago, you 
mentioned the possibility of inserting the sve registers before the 
watch/breakpoint registers in the register info vector (and the lldb 
enumeration).

Do we know of any reason why that would not work? After playing around with 
this code (for far too long), I've come to believe that this should work right 
now. It won't solve any of the underlying problems, but it should unblock this 
patch set, and it would match what was e.g. done when we added the intel mpx 
registers (D24255 ). While it won't work for 
targets that wish to expose debug registers, I'm pretty sure we don't have any 
targets like that right now. Based on everything I've learned so far, I believe 
that change should only impact servers who actually depend on the the registers 
embedded in the source code (and not on lldb client communicating with them). 
Right now, only lldb-server depends on these, and it does not expose the debug 
registers...


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

https://reviews.llvm.org/D77043



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

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't think we need to have different behavior for repl and --top-level. I'm 
mainly just confused about what is the right behavior to aim for.

So, assuming we want these to behave as if everything was in a single TU, my 
next question is: What is the purpose of the `external` flag? In this setup, it 
seems like everything should be "external" (i.e., made available to subsequent 
expressions)...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972



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

[Lldb-commits] [PATCH] D80104: [LLDB] Remove code duplication from RegisterContextPOSIX_*

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: wuzish.

Looks good. Thanks.


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

https://reviews.llvm.org/D80104



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

[Lldb-commits] [PATCH] D79447: [Debug][CodeView] Emit fully qualified names for globals

[Hans Wennborg via Phabricator via lldb-commits]

hans added a comment.

In D79447#2041250 , @hans wrote:

> > - Added assert in `emitDebugInfoForUDTs` to ensure no lowering can occur 
> > during UDT emission, as suggested by @rnk
>
> It seems we hit the assert in Chromium. Here's a reproducer: 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1083877#c3 (I'll try to 
> create a shorter one.)
>
> I've reverted in 525a591f0f48b9d54018bf5245f2abee09c9c1c8 
>  in the 
> meantime.


Reduced repro:

  $ cat /tmp/x.ii
  typedef int a;
  struct b;
  class c {
c();
a b::*d;
  };
  c::c() = default;
  
  $ clang -cc1 -triple x86_64-pc-windows-msvc19.16.0 -emit-obj -gcodeview 
-debug-info-kind=limited /tmp/x.ii
  clang: 
/work/llvm.monorepo/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2997: void 
llvm::CodeViewDebug::emitDebugInfoForUDTs(const 
std::vector> &): Assertion `OriginalSize 
== UDTs.size()' failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79447



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum updated this revision to Diff 264576.
fallkrum added a reviewer: LLDB.
fallkrum removed a subscriber: lldb-commits.

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

https://reviews.llvm.org/D80112

Files:
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -378,7 +378,9 @@
 const uint32_t process_stop_id = process_sp->GetStopID();
 if (m_stop_info_stop_id != process_stop_id) {
   if (m_stop_info_sp) {
-if (m_stop_info_sp->IsValid() || IsStillAtLastBreakpointHit() ||
+if (m_stop_info_sp->IsValid() ||
+(IsStillAtLastBreakpointHit() &&
+ m_resume_state != eStateSuspended) ||
 GetCurrentPlan()->IsVirtualStep())
   SetStopInfo(m_stop_info_sp);
 else


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -378,7 +378,9 @@
 const uint32_t process_stop_id = process_sp->GetStopID();
 if (m_stop_info_stop_id != process_stop_id) {
   if (m_stop_info_sp) {
-if (m_stop_info_sp->IsValid() || IsStillAtLastBreakpointHit() ||
+if (m_stop_info_sp->IsValid() ||
+(IsStillAtLastBreakpointHit() &&
+ m_resume_state != eStateSuspended) ||
 GetCurrentPlan()->IsVirtualStep())
   SetStopInfo(m_stop_info_sp);
 else
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum created this revision.
fallkrum added reviewers: clayborg, jingham.
fallkrum added a project: LLDB.

Encountered the following situation: Let we started thread T1 and it hit 
breakpoint on B1  location.  We suspended T1 and 
continued the process. Then we started thread T2 which hit for example the same 
location B1 . This time in a breakpoint callback 
we decided not to stop returning false.

Expected result: process continues (as if T2 did not hit breakpoint) its 
workflow with T1 still suspended.
Real result: process do stops (as if T2 callback returned true).

Solution: We need somehow invalidate StopInfo for threads that was previously 
suspended just because something that is already inactive can not be the reason 
of stop.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80112

Files:
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -378,7 +378,8 @@
 const uint32_t process_stop_id = process_sp->GetStopID();
 if (m_stop_info_stop_id != process_stop_id) {
   if (m_stop_info_sp) {
-if (m_stop_info_sp->IsValid() || IsStillAtLastBreakpointHit() ||
+if (m_stop_info_sp->IsValid() ||
+(IsStillAtLastBreakpointHit() && m_resume_state != 
eStateSuspended) ||
 GetCurrentPlan()->IsVirtualStep())
   SetStopInfo(m_stop_info_sp);
 else


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -378,7 +378,8 @@
 const uint32_t process_stop_id = process_sp->GetStopID();
 if (m_stop_info_stop_id != process_stop_id) {
   if (m_stop_info_sp) {
-if (m_stop_info_sp->IsValid() || IsStillAtLastBreakpointHit() ||
+if (m_stop_info_sp->IsValid() ||
+(IsStillAtLastBreakpointHit() && m_resume_state != eStateSuspended) ||
 GetCurrentPlan()->IsVirtualStep())
   SetStopInfo(m_stop_info_sp);
 else
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D79447: [Debug][CodeView] Emit fully qualified names for globals

[Hans Wennborg via Phabricator via lldb-commits]

hans added a comment.

> - Added assert in `emitDebugInfoForUDTs` to ensure no lowering can occur 
> during UDT emission, as suggested by @rnk

It seems we hit the assert in Chromium. Here's a reproducer: 
https://bugs.chromium.org/p/chromium/issues/detail?id=1083877#c3 (I'll try to 
create a shorter one.)

I've reverted in 525a591f0f48b9d54018bf5245f2abee09c9c1c8 
 in the 
meantime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79447



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

[Lldb-commits] [lldb] 525a591 - Revert 76c5f277f2 "Re-land [Debug][CodeView] Emit fully qualified names for globals"

[Hans Wennborg via lldb-commits]

Author: Hans Wennborg
Date: 2020-05-18T11:26:30+02:00
New Revision: 525a591f0f48b9d54018bf5245f2abee09c9c1c8

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

LOG: Revert 76c5f277f2 "Re-land [Debug][CodeView] Emit fully qualified names 
for globals"

> Before this patch, S_[L|G][THREAD32|DATA32] records were emitted with a 
> simple name, not the fully qualified name (namespace + class scope).
>
> Differential Revision: https://reviews.llvm.org/D79447

This causes asserts in Chromium builds:

CodeViewDebug.cpp:2997: void llvm::CodeViewDebug::emitDebugInfoForUDTs(const 
std::vector> &):
Assertion `OriginalSize == UDTs.size()' failed.

I will follow up on the Phabricator issue.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/PDB/variables.test
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
llvm/test/DebugInfo/COFF/global-constants.ll
llvm/test/DebugInfo/COFF/global_visibility.ll
llvm/test/DebugInfo/COFF/globals.ll
llvm/test/DebugInfo/COFF/types-array-unsized.ll

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/PDB/variables.test 
b/lldb/test/Shell/SymbolFile/PDB/variables.test
index a6c715360958..ae14f02754ce 100644
--- a/lldb/test/Shell/SymbolFile/PDB/variables.test
+++ b/lldb/test/Shell/SymbolFile/PDB/variables.test
@@ -13,12 +13,12 @@ GLOBALS: SymbolFile pdb ([[MOD]])
 GLOBALS: CompileUnit{{.*}}, language = "c++", file = 
'{{.*}}\VariablesTest.cpp'
 GLOBALS-DAG:   Variable{{.*}}, name = "g_IntVar"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
+GLOBALS-DAG:   Variable{{.*}}, name = "m_StaticClassMember"
+GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "g_pConst"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "same_name_var"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
-GLOBALS-DAG:   Variable{{.*}}, name = "Class::m_StaticClassMember"
-GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "g_EnumVar"
 GLOBALS-SAME:  scope = global, location = {{.*}}, external
 GLOBALS-DAG:   Variable{{.*}}, name = "g_tls"

diff  --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp 
b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 6f75dbd889fe..f167cead4e2c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -343,24 +343,8 @@ static std::string formatNestedName(ArrayRef 
QualifiedNameComponents,
   return FullyQualifiedName;
 }
 
-struct CodeViewDebug::TypeLoweringScope {
-  TypeLoweringScope(CodeViewDebug ) : CVD(CVD) { ++CVD.TypeEmissionLevel; }
-  ~TypeLoweringScope() {
-// Don't decrement TypeEmissionLevel until after emitting deferred types, 
so
-// inner TypeLoweringScopes don't attempt to emit deferred types.
-if (CVD.TypeEmissionLevel == 1)
-  CVD.emitDeferredCompleteTypes();
---CVD.TypeEmissionLevel;
-  }
-  CodeViewDebug 
-};
-
 std::string CodeViewDebug::getFullyQualifiedName(const DIScope *Scope,
  StringRef Name) {
-  // Ensure types in the scope chain are emitted as soon as possible.
-  // This can create otherwise a situation where S_UDTs are emitted while
-  // looping in emitDebugInfoForUDTs.
-  TypeLoweringScope S(*this);
   SmallVector QualifiedNameComponents;
   collectParentScopeNames(Scope, QualifiedNameComponents);
   return formatNestedName(QualifiedNameComponents, Name);
@@ -371,6 +355,18 @@ std::string CodeViewDebug::getFullyQualifiedName(const 
DIScope *Ty) {
   return getFullyQualifiedName(Scope, getPrettyScopeName(Ty));
 }
 
+struct CodeViewDebug::TypeLoweringScope {
+  TypeLoweringScope(CodeViewDebug ) : CVD(CVD) { ++CVD.TypeEmissionLevel; }
+  ~TypeLoweringScope() {
+// Don't decrement TypeEmissionLevel until after emitting deferred types, 
so
+// inner TypeLoweringScopes don't attempt to emit deferred types.
+if (CVD.TypeEmissionLevel == 1)
+  CVD.emitDeferredCompleteTypes();
+--CVD.TypeEmissionLevel;
+  }
+  CodeViewDebug 
+};
+
 TypeIndex CodeViewDebug::getScopeIndex(const DIScope *Scope) {
   // No scope means global scope and that uses the zero index.
   if (!Scope || isa(Scope))
@@ -2985,16 +2981,10 @@ void CodeViewDebug::emitEndSymbolRecord(SymbolKind 
EndKind) {
 }
 
 void CodeViewDebug::emitDebugInfoForUDTs(
-const std::vector> ) {
-#ifndef NDEBUG
-  size_t OriginalSize = UDTs.size();
-#endif
+ArrayRef> UDTs) {
   for (const auto  : UDTs) {
 const DIType *T = UDT.second;
 assert(shouldEmitUdt(T));
-// Ensure no new types are discovered when executing the code below.
-// See discussion in https://reviews.llvm.org/D79512
-

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added subscribers: danielkiss, atanasyan, kristof.beyls, emaste.

This patch aims to combine similar arm64 register set definitions defined in 
NativeRegisterContextLinux_arm64 and RegisterContextPOSIX_arm64.
I have implemented a register set interface out of RegisterInfoInterface class 
and moved arm64 register sets into RegisterInfosPOSIX_arm64 which is similar to 
Utility/RegisterContextLinux_* implemented by various other targets. This will 
help in managing register sets of new ARM64 architecture features in one place.

Built and tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabihf 
targets.


https://reviews.llvm.org/D80105

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.h
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_mips.h
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_mips64.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp

Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -61,7 +61,7 @@
 return false;
 
   offset -= GetGPRSize();
-  if (IsFPR(reg) && offset + reg_info->byte_size <= sizeof(FPU)) {
+  if (IsFPR(reg) && offset + reg_info->byte_size <= GetFPUSize()) {
 Status error;
 value.SetFromMemoryData(reg_info, m_fpregset.GetDataStart() + offset,
 reg_info->byte_size, lldb::eByteOrderLittle, error);
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -13,8 +13,20 @@
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/lldb-private.h"
 
+enum { RegisterSetPOSIX_ARM64GPR = 0, RegisterSetPOSIX_ARM64FPR };
+
 class RegisterInfoPOSIX_arm64 : public lldb_private::RegisterInfoInterface {
 public:
+  struct RegInfo {
+uint32_t num_registers;
+uint32_t num_gpr_registers;
+uint32_t num_fpr_registers;
+
+uint32_t last_gpr;
+uint32_t first_fpr;
+uint32_t last_fpr;
+  };
+
   // based on RegisterContextDarwin_arm64.h
   struct GPR {
 uint64_t x[29]; // x0-x28
@@ -61,7 +73,19 @@
 
   uint32_t GetRegisterCount() const override;
 
+  uint32_t GetRegisterOffset(uint32_t reg_index) const override;
+
+  uint32_t GetRegisterSize(uint32_t reg_index) const override;
+
+  const lldb_private::RegisterSet *
+  GetRegisterSet(size_t reg_set) const override;
+
+  size_t GetRegisterSetCount() const override;
+
+  size_t GetRegisterSetFromRegisterIndex(uint32_t reg_index) const override;
+
 private:
+  RegInfo m_reg_info;
   const lldb_private::RegisterInfo *m_register_info_p;
   uint32_t m_register_info_count;
 };
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -65,6 +65,58 @@
   }
 }
 
+// Number of register sets provided by this context.
+enum {
+  k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
+  k_num_fpr_registers = fpu_fpcr - fpu_v0 + 1,
+  k_num_register_sets = 2
+};
+// clang-format off
+// ARM64 general purpose registers.
+static const uint32_t g_gpr_regnums_arm64[] = {
+gpr_x0,  gpr_x1,   gpr_x2,  gpr_x3,  gpr_x4,  gpr_x5,  gpr_x6,  gpr_x7,
+gpr_x8,  gpr_x9,   gpr_x10, gpr_x11, gpr_x12, gpr_x13, gpr_x14, gpr_x15,
+gpr_x16, gpr_x17,  gpr_x18, gpr_x19, gpr_x20, gpr_x21, gpr_x22, gpr_x23,
+gpr_x24, gpr_x25,  gpr_x26, gpr_x27, gpr_x28, gpr_fp,  gpr_lr,  gpr_sp,
+gpr_pc,  gpr_cpsr, gpr_w0,  gpr_w1,  gpr_w2,  gpr_w3,  gpr_w4,  gpr_w5,
+gpr_w6,  gpr_w7,   gpr_w8,  gpr_w9,  gpr_w10, gpr_w11, gpr_w12, gpr_w13,
+gpr_w14, gpr_w15,  gpr_w16, gpr_w17, gpr_w18, gpr_w19, gpr_w20, gpr_w21,
+gpr_w22, gpr_w23,  gpr_w24, gpr_w25, gpr_w26, gpr_w27, gpr_w28,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+
+static_assert(((sizeof g_gpr_regnums_arm64 /