date:20190225

[Lldb-commits] [lldb] r354766 - Finish revert of r354706

2019-02-25 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Feb 25 01:30:41 2019
New Revision: 354766

URL: http://llvm.org/viewvc/llvm-project?rev=354766&view=rev
Log:
Finish revert of r354706

The revert in r354711 wasn't complete. Finish the job.

Modified:
lldb/trunk/lit/ExecControl/StopHook/stop-hook.test

Modified: lldb/trunk/lit/ExecControl/StopHook/stop-hook.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/stop-hook.test?rev=354766&r1=354765&r2=354766&view=diff
==
--- lldb/trunk/lit/ExecControl/StopHook/stop-hook.test (original)
+++ lldb/trunk/lit/ExecControl/StopHook/stop-hook.test Mon Feb 25 01:30:41 2019
@@ -46,9 +46,7 @@ target stop-hook list
 run
 # Stopping inside of the stop hook range
 # CHECK: (lldb) run
-# CHECK-NEXT: Process {{.*}} launched:
 # CHECK-NEXT: (void *) $0 = 0x
-# CHECK-NEXT: Process {{.*}} stopped
 
 thread step-over
 # Stepping inside of the stop hook range


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

[Lldb-commits] [PATCH] D58394: Add --auto-continue to stop-hooks, fix up a few tests

2019-02-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I'm not sure why you ended up reverting this, but it looks like the revert 
wasn't complete (which is why we got the test failures that Jan mentions). I've 
finished the revert in r354766.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58394



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

[Lldb-commits] [PATCH] D58394: Add --auto-continue to stop-hooks, fix up a few tests

2019-02-25 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

In D58394#1408117 , @jankratochvil 
wrote:

> Getting now failure for every run: `LLDB :: 
> ExecControl/StopHook/stop-hook.test`


It is because after this commit (rL354706  = 
GIT `ff8c7a0947663ce7515f0b8ee52b9d0fe8883bc3`) there was its revert (rL354711 
 = GIT 
0513a24d62e1eef4e8179ced177c3672dab21bce 
) but it 
did not revert one testcase patch hunk 
:

  lldb/lit/ExecControl/StopHook/stop-hook.test
   # CHECK: (lldb) run
  +# CHECK-NEXT: Process {{.*}} launched:
   # CHECK-NEXT: (void *) $0 = 0x
  +# CHECK-NEXT: Process {{.*}} stopped

while on Fedora 29 x86_64 the LLDB output is reversed:

  (lldb) run
  (void *) $0 = 0x00405260
  
  Process 460013 stopped
  * thread #1, name = 'stop-hook.test.', stop reason = breakpoint 1.1
  frame #0: 0x004011c4 stop-hook.test.tmp`b(val=1) at 
stop-hook.c:29:10
 26 {
 27 int rc = c(val);
 28 void *ptr = malloc(1024);
  -> 29 if (!ptr)  // Set breakpoint here to test target stop-hook.
 30 return -1;
 31 else
 32 printf("ptr=%p\n", ptr); // We should stop here after 
stepping.
  
  Process 460013 launched: 
'.../tools/lldb/lit/ExecControl/StopHook/Output/stop-hook.test.tmp' (x86_64)
  (lldb) thread step-over


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58394



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

[Lldb-commits] [PATCH] D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

2019-02-25 Thread Hafiz Abid Qadeer via Phabricator via lldb-commits

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

The lldb-mi bits look ok to me.




Comment at: tools/lldb-mi/MIUtilString.h:36
 va_list vArgs);
+
   static bool IsAllValidAlphaAndNumeric(const char *vpText);

May be spurious new line


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

https://reviews.llvm.org/D55653



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

[Lldb-commits] [PATCH] D58610: [lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

2019-02-25 Thread Michał Górny via Phabricator via lldb-commits

mgorny created this revision.
mgorny added a reviewer: labath.
mgorny added a project: LLDB.
Herald added subscribers: abidh, fedor.sergeev.

Set LD_LIBRARY_PATH or local platform's equivalent of it when running
the 'Suite' tests.  This is necessary when running tests inside build
tree with BUILD_SHARED_LIBS enabled, in order to make the LLDB modules
load freshly built LLVM libraries.

The code is copied from clang (test/Unit/lit.cfg) with minimal change
of removing support for disjoint LLVM/Clang library directory.
A similar option (along with additional site configuration variable)
will become necessary if LLDB ever starts supporting building shared
libraries, for stand-alone builds (where LLDB library dir != LLVM
library dir).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58610

Files:
  lldb/lit/Suite/lit.cfg


Index: lldb/lit/Suite/lit.cfg
===
--- lldb/lit/Suite/lit.cfg
+++ lldb/lit/Suite/lit.cfg
@@ -3,6 +3,7 @@
 # Configuration file for the 'lit' test runner.
 
 import os
+import platform
 import shlex
 
 import lit.formats
@@ -32,6 +33,25 @@
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
+def find_shlibpath_var():
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
+yield 'LD_LIBRARY_PATH'
+elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+shlibpath = os.path.pathsep.join(
+(config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+config.environment[shlibpath_var] = shlibpath
+break
+else:
+lit_config.warning("unable to inject shared library path on '{}'"
+   .format(platform.system()))
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))


Index: lldb/lit/Suite/lit.cfg
===
--- lldb/lit/Suite/lit.cfg
+++ lldb/lit/Suite/lit.cfg
@@ -3,6 +3,7 @@
 # Configuration file for the 'lit' test runner.
 
 import os
+import platform
 import shlex
 
 import lit.formats
@@ -32,6 +33,25 @@
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
+def find_shlibpath_var():
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
+yield 'LD_LIBRARY_PATH'
+elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+shlibpath = os.path.pathsep.join(
+(config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+config.environment[shlibpath_var] = shlibpath
+break
+else:
+lit_config.warning("unable to inject shared library path on '{}'"
+   .format(platform.system()))
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

2019-02-25 Thread serge via Phabricator via lldb-commits

serge-sans-paille abandoned this revision.
serge-sans-paille added a comment.

@sgraenitz It's fixed! I'm dropping this patch then, thanks for investigating o/


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [PATCH] D58339: Changes for running LLDB test suite for Swift on PowerPC64LE

2019-02-25 Thread Sarvesh Tamba via Phabricator via lldb-commits

sarveshtamba added a comment.

Hi, can someone point me the right repo to submit these changes?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58339



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

[Lldb-commits] [PATCH] D58339: Changes for running LLDB test suite for Swift on PowerPC64LE

2019-02-25 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

That should be the right repo: https://github.com/apple/swift-lldb


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58339



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

[Lldb-commits] [PATCH] D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

2019-02-25 Thread Tatyana Krasnukha via Phabricator via lldb-commits

tatyana-krasnukha updated this revision to Diff 188188.
tatyana-krasnukha added a comment.

Removed new lines from MIUtilString.h


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

https://reviews.llvm.org/D55653

Files:
  tools/lldb-mi/MICmdCmdMiscellanous.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
  tools/lldb-mi/MICmnLLDBDebugger.cpp
  tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
  tools/lldb-mi/MICmnMIResultRecord.cpp
  tools/lldb-mi/MIDriverMgr.cpp
  tools/lldb-mi/MIUtilString.cpp
  tools/lldb-mi/MIUtilString.h
  unittests/tools/CMakeLists.txt
  unittests/tools/lldb-mi/CMakeLists.txt
  unittests/tools/lldb-mi/utils/CMakeLists.txt
  unittests/tools/lldb-mi/utils/StringTest.cpp

Index: unittests/tools/lldb-mi/utils/StringTest.cpp
===
--- unittests/tools/lldb-mi/utils/StringTest.cpp
+++ unittests/tools/lldb-mi/utils/StringTest.cpp
@@ -0,0 +1,32 @@
+//===-- StringTest.cpp --*- 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
+//
+//===--===//
+
+#include "MIUtilString.h"
+#include "gtest/gtest.h"
+
+TEST(StringTest, ConstructFromNullptr) {
+  auto str = CMIUtilString(nullptr);
+  EXPECT_TRUE(str.empty());
+  EXPECT_NE(nullptr, str.c_str());
+  EXPECT_EQ(CMIUtilString(""), str);
+}
+
+TEST(StringTest, AssignNullptr) {
+  CMIUtilString str;
+  str = nullptr;
+  EXPECT_TRUE(str.empty());
+  EXPECT_NE(nullptr, str.c_str());
+  EXPECT_EQ(CMIUtilString(""), str);
+}
+
+TEST(StringTest, IsAllValidAlphaAndNumeric) {
+  EXPECT_TRUE(CMIUtilString::IsAllValidAlphaAndNumeric("123abc"));
+  EXPECT_FALSE(CMIUtilString::IsAllValidAlphaAndNumeric(""));
+  EXPECT_FALSE(CMIUtilString::IsAllValidAlphaAndNumeric(nullptr));
+}
+
Index: unittests/tools/lldb-mi/utils/CMakeLists.txt
===
--- unittests/tools/lldb-mi/utils/CMakeLists.txt
+++ unittests/tools/lldb-mi/utils/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_library(lldb-mi-utils OBJECT
+  ${LLDB_SOURCE_DIR}/tools/lldb-mi/MIUtilString.cpp
+  )
+
+add_lldb_unittest(LLDBMiUtilTests
+  StringTest.cpp
+
+  LINK_COMPONENTS
+Support
+  )
+
+target_sources(LLDBMiUtilTests PRIVATE $)
Index: unittests/tools/lldb-mi/CMakeLists.txt
===
--- unittests/tools/lldb-mi/CMakeLists.txt
+++ unittests/tools/lldb-mi/CMakeLists.txt
@@ -0,0 +1,2 @@
+include_directories(${LLDB_SOURCE_DIR}/tools/lldb-mi)
+add_subdirectory(utils)
Index: unittests/tools/CMakeLists.txt
===
--- unittests/tools/CMakeLists.txt
+++ unittests/tools/CMakeLists.txt
@@ -1,3 +1,4 @@
+add_subdirectory(lldb-mi)
 if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD")
   if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_TEST_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD))
 # These tests are meant to test lldb-server/debugserver in isolation, and
Index: tools/lldb-mi/MIUtilString.h
===
--- tools/lldb-mi/MIUtilString.h
+++ tools/lldb-mi/MIUtilString.h
@@ -34,6 +34,9 @@
   static CMIUtilString FormatValist(const CMIUtilString &vrFormating,
 va_list vArgs);
   static bool IsAllValidAlphaAndNumeric(const char *vpText);
+  static const char *WithNullAsEmpty(const char *vpText) {
+return vpText ? vpText : "";
+  }
   static bool Compare(const CMIUtilString &vrLhs, const CMIUtilString &vrRhs);
   static CMIUtilString ConvertToPrintableASCII(const char vChar,
bool bEscapeQuotes = false);
Index: tools/lldb-mi/MIUtilString.cpp
===
--- tools/lldb-mi/MIUtilString.cpp
+++ tools/lldb-mi/MIUtilString.cpp
@@ -37,7 +37,8 @@
 // Return:  None.
 // Throws:  None.
 //--
-CMIUtilString::CMIUtilString(const char *vpData) : std::string(vpData) {}
+CMIUtilString::CMIUtilString(const char *vpData)
+  : std::string(WithNullAsEmpty(vpData)) {}
 
 //++
 //
@@ -58,7 +59,7 @@
 // Throws:  None.
 //--
 CMIUtilString &CMIUtilString::operator=(const char *vpRhs) {
-  assign(vpRhs);
+  assign(WithNullAsEmpty(vpRhs));
   return *this;
 }
 
@@ -103,12 +104,10 @@
   MIint n = vrFormat.size();
 
   // IOR: mysterious crash in this function on some windows builds not able to
-  // duplicate
-  // but found article which may be related. Crash occurs in vsnprintf() or
-  // va_copy()
+  // duplicate but found article which may be related. Crash occurs in
+  // vsnprintf() or va_copy().
   /

[Lldb-commits] [lldb] r354798 - [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

2019-02-25 Thread Tatyana Krasnukha via lldb-commits

Author: tkrasnukha
Date: Mon Feb 25 08:40:11 2019
New Revision: 354798

URL: http://llvm.org/viewvc/llvm-project?rev=354798&view=rev
Log:
[lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

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

Added:
lldb/trunk/unittests/tools/lldb-mi/
lldb/trunk/unittests/tools/lldb-mi/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-mi/utils/
lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-mi/utils/StringTest.cpp
Modified:
lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp
lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp
lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
lldb/trunk/tools/lldb-mi/MICmnMIResultRecord.cpp
lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
lldb/trunk/tools/lldb-mi/MIUtilString.cpp
lldb/trunk/tools/lldb-mi/MIUtilString.h
lldb/trunk/unittests/tools/CMakeLists.txt

Modified: lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp?rev=354798&r1=354797&r2=354798&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp Mon Feb 25 08:40:11 2019
@@ -339,7 +339,9 @@ bool CMICmdCmdListThreadGroups::Acknowle
   const char *pDir = sbTrgt.GetExecutable().GetDirectory();
   const char *pFileName = sbTrgt.GetExecutable().GetFilename();
   const CMIUtilString strFile(
-  CMIUtilString::Format("%s/%s", pDir, pFileName));
+  CMIUtilString::Format("%s/%s",
+CMIUtilString::WithNullAsEmpty(pDir),
+CMIUtilString::WithNullAsEmpty(pFileName)));
   const CMICmnMIValueConst miValueConst4(strFile);
   const CMICmnMIValueResult miValueResult4("executable", miValueConst4);
   miTuple.Add(miValueResult4);

Modified: lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp?rev=354798&r1=354797&r2=354798&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp Mon Feb 25 08:40:11 
2019
@@ -375,11 +375,9 @@ bool CMICmnLLDBDebugSessionInfo::MIRespo
 
   // Add "target-id"
   const char *pThreadName = rThread.GetName();
-  const MIuint len =
-  (pThreadName != nullptr) ? CMIUtilString(pThreadName).length() : 0;
-  const bool bHaveName = ((pThreadName != nullptr) && (len > 0) && (len < 32) 
&&
-  CMIUtilString::IsAllValidAlphaAndNumeric(
-  pThreadName)); // 32 is arbitrary number
+  const MIuint len = CMIUtilString(pThreadName).length();
+  const bool bHaveName = (len > 0) && (len < 32) && // 32 is arbitrary number
+ CMIUtilString::IsAllValidAlphaAndNumeric(pThreadName);
   const char *pThrdFmt = bHaveName ? "%s" : "Thread %d";
   CMIUtilString strThread;
   if (bHaveName)

Modified: lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp?rev=354798&r1=354797&r2=354798&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp Mon Feb 25 08:40:11 2019
@@ -43,10 +43,11 @@ static inline bool MI_char_summary_provi
 
   lldb::BasicType type_code = value_type.GetBasicType();
   if (type_code == lldb::eBasicTypeSignedChar)
-stream.Printf("%d %s", (int)value.GetValueAsSigned(), value.GetValue());
+stream.Printf("%d %s", (int)value.GetValueAsSigned(),
+  CMIUtilString::WithNullAsEmpty(value.GetValue()));
   else if (type_code == lldb::eBasicTypeUnsignedChar)
 stream.Printf("%u %s", (unsigned)value.GetValueAsUnsigned(),
-  value.GetValue());
+  CMIUtilString::WithNullAsEmpty(value.GetValue()));
   else
 return false;
 

Modified: lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp?rev=354798&r1=354797&r2=354798&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp Mon Feb 25 08:40:11 2019
@@ -109,8 +109,10 @@ MapOutOfBandToToken(CMICmnMIOutOfBandRec
 //--
 static CMIUtilString
 BuildAsyncRecord(CMICmnMIOutOfBandRecord::OutOfBand_e veType) {
-  return CMIUtilString::Format("%s%s", MapOutOfBandToToken(veType),
-

[Lldb-commits] [PATCH] D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

2019-02-25 Thread Phabricator via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354798: [lldb-mi] Check raw pointers before passing them to 
std::string ctor/assignment (authored by tkrasnukha, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55653?vs=188188&id=188192#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55653

Files:
  lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp
  lldb/trunk/tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
  lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp
  lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
  lldb/trunk/tools/lldb-mi/MICmnMIResultRecord.cpp
  lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
  lldb/trunk/tools/lldb-mi/MIUtilString.cpp
  lldb/trunk/tools/lldb-mi/MIUtilString.h
  lldb/trunk/unittests/tools/CMakeLists.txt
  lldb/trunk/unittests/tools/lldb-mi/CMakeLists.txt
  lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt
  lldb/trunk/unittests/tools/lldb-mi/utils/StringTest.cpp

Index: lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt
===
--- lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt
+++ lldb/trunk/unittests/tools/lldb-mi/utils/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_library(lldb-mi-utils OBJECT
+  ${LLDB_SOURCE_DIR}/tools/lldb-mi/MIUtilString.cpp
+  )
+
+add_lldb_unittest(LLDBMiUtilTests
+  StringTest.cpp
+
+  LINK_COMPONENTS
+Support
+  )
+
+target_sources(LLDBMiUtilTests PRIVATE $)
Index: lldb/trunk/unittests/tools/lldb-mi/utils/StringTest.cpp
===
--- lldb/trunk/unittests/tools/lldb-mi/utils/StringTest.cpp
+++ lldb/trunk/unittests/tools/lldb-mi/utils/StringTest.cpp
@@ -0,0 +1,32 @@
+//===-- StringTest.cpp --*- 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
+//
+//===--===//
+
+#include "MIUtilString.h"
+#include "gtest/gtest.h"
+
+TEST(StringTest, ConstructFromNullptr) {
+  auto str = CMIUtilString(nullptr);
+  EXPECT_TRUE(str.empty());
+  EXPECT_NE(nullptr, str.c_str());
+  EXPECT_EQ(CMIUtilString(""), str);
+}
+
+TEST(StringTest, AssignNullptr) {
+  CMIUtilString str;
+  str = nullptr;
+  EXPECT_TRUE(str.empty());
+  EXPECT_NE(nullptr, str.c_str());
+  EXPECT_EQ(CMIUtilString(""), str);
+}
+
+TEST(StringTest, IsAllValidAlphaAndNumeric) {
+  EXPECT_TRUE(CMIUtilString::IsAllValidAlphaAndNumeric("123abc"));
+  EXPECT_FALSE(CMIUtilString::IsAllValidAlphaAndNumeric(""));
+  EXPECT_FALSE(CMIUtilString::IsAllValidAlphaAndNumeric(nullptr));
+}
+
Index: lldb/trunk/unittests/tools/lldb-mi/CMakeLists.txt
===
--- lldb/trunk/unittests/tools/lldb-mi/CMakeLists.txt
+++ lldb/trunk/unittests/tools/lldb-mi/CMakeLists.txt
@@ -0,0 +1,2 @@
+include_directories(${LLDB_SOURCE_DIR}/tools/lldb-mi)
+add_subdirectory(utils)
Index: lldb/trunk/unittests/tools/CMakeLists.txt
===
--- lldb/trunk/unittests/tools/CMakeLists.txt
+++ lldb/trunk/unittests/tools/CMakeLists.txt
@@ -1,3 +1,4 @@
+add_subdirectory(lldb-mi)
 if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD")
   if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_TEST_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD))
 # These tests are meant to test lldb-server/debugserver in isolation, and
Index: lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp
+++ lldb/trunk/tools/lldb-mi/MICmdCmdMiscellanous.cpp
@@ -339,7 +339,9 @@
   const char *pDir = sbTrgt.GetExecutable().GetDirectory();
   const char *pFileName = sbTrgt.GetExecutable().GetFilename();
   const CMIUtilString strFile(
-  CMIUtilString::Format("%s/%s", pDir, pFileName));
+  CMIUtilString::Format("%s/%s",
+CMIUtilString::WithNullAsEmpty(pDir),
+CMIUtilString::WithNullAsEmpty(pFileName)));
   const CMICmnMIValueConst miValueConst4(strFile);
   const CMICmnMIValueResult miValueResult4("executable", miValueConst4);
   miTuple.Add(miValueResult4);
Index: lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
+++ lldb/trunk/tools/lldb-mi/MICmnMIOutOfBandRecord.cpp
@@ -109,8 +109,10 @@
 //--
 static CMIUtilString
 BuildAsyncRecord(CMICmnMIOutOfBandRecord::OutOfBand_e veType) {
-

[Lldb-commits] [PATCH] D58630: [lldb] [test] Pass appropriate -L for just-built libc++

2019-02-25 Thread Michał Górny via Phabricator via lldb-commits

mgorny created this revision.
mgorny added reviewers: krytarowski, labath, zturner.
mgorny added a project: LLDB.
Herald added a reviewer: EricWF.
Herald added a subscriber: teemperor.
Herald added a reviewer: serge-sans-paille.

Pass appropriate -L flags pointing out to the LLVM library directory
when linking tests.  This is necessary on platforms that default to
libc++ but don't ship with one, e.g. on NetBSD buildbot.

This patch includes -L passing for tests using clang via toolchain
substitutions, via build.py and via Makefiles.  This is not pretty.
I'm open to better solutions.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58630

Files:
  lldb/lit/BuildScript/toolchain-clang.test
  lldb/lit/Suite/lit.cfg
  lldb/lit/helper/build.py
  lldb/lit/helper/toolchain.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -297,6 +297,9 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
+ifneq (,$(LLVM_LIBS_DIR))
+   LDFLAGS += -L$(LLVM_LIBS_DIR)
+endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread
Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -31,6 +31,8 @@
 
build_script_args.append('--tools-dir={0}'.format(config.lldb_lit_tools_dir))
 if config.lldb_tools_dir:
 
build_script_args.append('--tools-dir={0}'.format(config.lldb_tools_dir))
+if config.llvm_libs_dir:
+build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
 
 primary_tools = [
 ToolSubst('%lldb',
@@ -99,6 +101,8 @@
 elif platform.system() in ['OpenBSD', 'Linux']:
 flags = ['-pthread']
 
+# needed e.g. to use freshly built libc++
+flags.append('-L' + config.llvm_libs_dir)
 
 additional_tool_dirs=[]
 if config.lldb_lit_tools_dir:
Index: lldb/lit/helper/build.py
===
--- lldb/lit/helper/build.py
+++ lldb/lit/helper/build.py
@@ -35,6 +35,13 @@
 required=True,
 help='Path to a compiler executable, or one of the values 
[any, msvc, clang-cl, gcc, clang]')
 
+parser.add_argument('--libs-dir',
+metavar='directory',
+dest='libs_dir',
+required=False,
+action='append',
+help='If specified, a path to linked libraries to be 
passed via -L')
+
 parser.add_argument('--tools-dir',
 metavar='directory',
 dest='tools_dir',
@@ -225,6 +232,7 @@
 self.nodefaultlib = args.nodefaultlib
 self.verbose = args.verbose
 self.obj_ext = obj_ext
+self.lib_paths = args.libs_dir
 
 def _exe_file_name(self):
 assert self.mode != 'compile'
@@ -648,6 +656,7 @@
 if sys.platform == 'darwin':
 main_symbol = '_main'
 args.append('-Wl,-e,' + main_symbol)
+args.extend(['-L' + x for x in self.lib_paths])
 args.extend(['-o', self._exe_file_name()])
 args.extend(self._obj_file_names())
 
Index: lldb/lit/Suite/lit.cfg
===
--- lldb/lit/Suite/lit.cfg
+++ lldb/lit/Suite/lit.cfg
@@ -32,6 +32,9 @@
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Library path may be needed to locate just-built clang.
+config.environment['LLVM_LIBS_DIR'] = config.llvm_libs_dir
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))
Index: lldb/lit/BuildScript/toolchain-clang.test
===
--- lldb/lit/BuildScript/toolchain-clang.test
+++ lldb/lit/BuildScript/toolchain-clang.test
@@ -10,5 +10,5 @@
 CHECK-32: {{.*}}clang++{{(.exe)?}} -m32 -g -O0 -c -o {{.*}}foo.exe-foobar.o 
{{.*}}foobar.c
 CHECK-64: {{.*}}clang++{{(.exe)?}} -m64 -g -O0 -c -o {{.*}}foo.exe-foobar.o 
{{.*}}foobar.c
 CHECK: linking foo.exe-foobar.o -> foo.exe
-CHECK-32: {{.*}}clang++{{(.exe)?}} -m32 -o {{.*}}foo.exe {{.*}}foo.exe-foobar.o
-CHECK-64: {{.*}}clang++{{(.exe)?}} -m64 -o {{.*}}foo.exe {{.*}}foo.exe-foobar.o
+CHECK-32: {{.*}}clang++{{(.exe)?}} -m32 {{(-L.*)?}} -o {{.*}}foo.exe 
{{.*}}foo.exe-foobar.o
+CHECK-64: {{.*}}clang++{{(.exe)?}} -m64 {{(-L.*)?}} -o {{.*}}foo.exe 
{{.*}}foo.exe-foobar.o


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/M

[Lldb-commits] [lldb] r354803 - [lldb-mi] Fix conversion warning for 64-bit build

2019-02-25 Thread Tatyana Krasnukha via lldb-commits

Author: tkrasnukha
Date: Mon Feb 25 10:23:44 2019
New Revision: 354803

URL: http://llvm.org/viewvc/llvm-project?rev=354803&view=rev
Log:
[lldb-mi] Fix conversion warning for 64-bit build

Modified:
lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp

Modified: lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp?rev=354803&r1=354802&r2=354803&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnLLDBDebugger.cpp Mon Feb 25 10:23:44 2019
@@ -558,7 +558,7 @@ bool CMICmnLLDBDebugger::UnregisterForEv
   ClientGetMaskForAllClients(vBroadcasterClass);
   MIuint newEventMask = 0;
   for (MIuint i = 0; i < 32; i++) {
-const MIuint bit = 1 << i;
+const MIuint bit = MIuint(1) << i;
 const MIuint clientBit = bit & clientsEventMask;
 const MIuint othersBit = bit & otherClientsEventMask;
 if ((clientBit != 0) && (othersBit == 0)) {


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

[Lldb-commits] [lldb] r354804 - [lldb-mi] Return source line number in proper format

2019-02-25 Thread Tatyana Krasnukha via lldb-commits

Author: tkrasnukha
Date: Mon Feb 25 10:32:46 2019
New Revision: 354804

URL: http://llvm.org/viewvc/llvm-project?rev=354804&view=rev
Log:
[lldb-mi] Return source line number in proper format

Line number is a decimal number and is printed as such, however for some
reason it was prefixed with '0x', thus turning printed value invalid.

Patch by Anton Kolesov 

Modified:
lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp

Modified: lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp?rev=354804&r1=354803&r2=354804&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp Mon Feb 25 10:32:46 2019
@@ -418,7 +418,7 @@ bool CMICmdCmdDataDisassemble::Execute()
 
   // MI "src_and_asm_line={line=\"%u\",file=\"%s\",line_asm_insn=[ ]}"
   const CMICmnMIValueConst miValueConst(
-  CMIUtilString::Format("0x%u", nLine));
+  CMIUtilString::Format("%u", nLine));
   const CMICmnMIValueResult miValueResult("line", miValueConst);
   CMICmnMIValueTuple miValueTuple2(miValueResult);
   const CMICmnMIValueConst miValueConst2(pFileName);


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

Re: [Lldb-commits] [lldb] r354711 - Revert r354706 - lit touched my thigh

2019-02-25 Thread Zachary Turner via lldb-commits

While it’s a good chuckle, can you elaborate on what exactly the commit
message means?
On Fri, Feb 22, 2019 at 5:07 PM Jim Ingham via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: jingham
> Date: Fri Feb 22 17:08:17 2019
> New Revision: 354711
>
> URL: http://llvm.org/viewvc/llvm-project?rev=354711&view=rev
> Log:
> Revert r354706 - lit touched my thigh
>
> Modified:
> lldb/trunk/include/lldb/Target/Process.h
> lldb/trunk/include/lldb/Target/Target.h
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
> lldb/trunk/lit/ExecControl/StopHook/stop-hook-threads.test
> lldb/trunk/lit/ExecControl/StopHook/stop-hook.test
> lldb/trunk/source/Commands/CommandObjectTarget.cpp
> lldb/trunk/source/Target/Process.cpp
> lldb/trunk/source/Target/Target.cpp
>
> Modified: lldb/trunk/include/lldb/Target/Process.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=354711&r1=354710&r2=354711&view=diff
>
> ==
> --- lldb/trunk/include/lldb/Target/Process.h (original)
> +++ lldb/trunk/include/lldb/Target/Process.h Fri Feb 22 17:08:17 2019
> @@ -2519,8 +2519,6 @@ public:
>///
>//--
>void RestoreProcessEvents();
> -
> -  bool IsHijackedForSynchronousResume();
>
>const lldb::ABISP &GetABI();
>
>
> Modified: lldb/trunk/include/lldb/Target/Target.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=354711&r1=354710&r2=354711&view=diff
>
> ==
> --- lldb/trunk/include/lldb/Target/Target.h (original)
> +++ lldb/trunk/include/lldb/Target/Target.h Fri Feb 22 17:08:17 2019
> @@ -1153,10 +1153,6 @@ public:
>
>  void SetIsActive(bool is_active) { m_active = is_active; }
>
> -void SetAutoContinue(bool auto_continue) {m_auto_continue =
> auto_continue;}
> -
> -bool GetAutoContinue() const { return m_auto_continue; }
> -
>  void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
>
>private:
> @@ -1164,8 +1160,7 @@ public:
>  StringList m_commands;
>  lldb::SymbolContextSpecifierSP m_specifier_sp;
>  std::unique_ptr m_thread_spec_up;
> -bool m_active = true;
> -bool m_auto_continue = false;
> +bool m_active;
>
>  // Use CreateStopHook to make a new empty stop hook. The
> GetCommandPointer
>  // and fill it with commands, and SetSpecifier to set the specifier
> shared
>
> Modified: lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit?rev=354711&r1=354710&r2=354711&view=diff
>
> ==
> --- lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
> (original)
> +++ lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit Fri
> Feb 22 17:08:17 2019
> @@ -1 +1 @@
> -target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
> +target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
> \ No newline at end of file
>
> Modified: lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit?rev=354711&r1=354710&r2=354711&view=diff
>
> ==
> --- lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
> (original)
> +++ lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit Fri
> Feb 22 17:08:17 2019
> @@ -1,3 +1,3 @@
>  target stop-hook add -f stop-hook.c -l 29 -e 34
>  expr ptr
> -DONE
> +DONE
> \ No newline at end of file
>
> Modified:
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit?rev=354711&r1=354710&r2=354711&view=diff
>
> ==
> ---
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
> (original)
> +++
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit Fri
> Feb 22 17:08:17 2019
> @@ -1,7 +1,7 @@
> -break set -f stop-hook-threads.cpp -p "Break here to set up the stop hook"
>  break set -f stop-hook-threads.cpp -p "Break here to test that the
> stop-hook"
>  run
> -target stop-hook add -G true
> -expr lldb_val += 1
> +target stop-hook add
> +frame variable --show-g

[Lldb-commits] [PATCH] D58565: [Reproducers] Enable replay from SBRepro

2019-02-25 Thread Davide Italiano via Phabricator via lldb-commits

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

LGTM


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58565



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

Re: [Lldb-commits] [lldb] r354711 - Revert r354706 - lit touched my thigh

2019-02-25 Thread Jim Ingham via lldb-commits

A test that was passing locally failed on the bot after submitting this patch.  
Not sure what the difference is yet.

Jim

> On Feb 25, 2019, at 12:22 PM, Zachary Turner  wrote:
> 
> While it’s a good chuckle, can you elaborate on what exactly the commit 
> message means? 
> On Fri, Feb 22, 2019 at 5:07 PM Jim Ingham via lldb-commits 
>  wrote:
> Author: jingham
> Date: Fri Feb 22 17:08:17 2019
> New Revision: 354711
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=354711&view=rev
> Log:
> Revert r354706 - lit touched my thigh
> 
> Modified:
> lldb/trunk/include/lldb/Target/Process.h
> lldb/trunk/include/lldb/Target/Target.h
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
> lldb/trunk/lit/ExecControl/StopHook/stop-hook-threads.test
> lldb/trunk/lit/ExecControl/StopHook/stop-hook.test
> lldb/trunk/source/Commands/CommandObjectTarget.cpp
> lldb/trunk/source/Target/Process.cpp
> lldb/trunk/source/Target/Target.cpp
> 
> Modified: lldb/trunk/include/lldb/Target/Process.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=354711&r1=354710&r2=354711&view=diff
> ==
> --- lldb/trunk/include/lldb/Target/Process.h (original)
> +++ lldb/trunk/include/lldb/Target/Process.h Fri Feb 22 17:08:17 2019
> @@ -2519,8 +2519,6 @@ public:
>///
>//--
>void RestoreProcessEvents();
> -  
> -  bool IsHijackedForSynchronousResume();
> 
>const lldb::ABISP &GetABI();
> 
> 
> Modified: lldb/trunk/include/lldb/Target/Target.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=354711&r1=354710&r2=354711&view=diff
> ==
> --- lldb/trunk/include/lldb/Target/Target.h (original)
> +++ lldb/trunk/include/lldb/Target/Target.h Fri Feb 22 17:08:17 2019
> @@ -1153,10 +1153,6 @@ public:
> 
>  void SetIsActive(bool is_active) { m_active = is_active; }
> 
> -void SetAutoContinue(bool auto_continue) {m_auto_continue = 
> auto_continue;}
> -
> -bool GetAutoContinue() const { return m_auto_continue; }
> -
>  void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
> 
>private:
> @@ -1164,8 +1160,7 @@ public:
>  StringList m_commands;
>  lldb::SymbolContextSpecifierSP m_specifier_sp;
>  std::unique_ptr m_thread_spec_up;
> -bool m_active = true;
> -bool m_auto_continue = false;
> +bool m_active;
> 
>  // Use CreateStopHook to make a new empty stop hook. The 
> GetCommandPointer
>  // and fill it with commands, and SetSpecifier to set the specifier 
> shared
> 
> Modified: lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit?rev=354711&r1=354710&r2=354711&view=diff
> ==
> --- lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit (original)
> +++ lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit Fri Feb 
> 22 17:08:17 2019
> @@ -1 +1 @@
> -target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
> +target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
> \ No newline at end of file
> 
> Modified: lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit?rev=354711&r1=354710&r2=354711&view=diff
> ==
> --- lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit (original)
> +++ lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit Fri Feb 
> 22 17:08:17 2019
> @@ -1,3 +1,3 @@
>  target stop-hook add -f stop-hook.c -l 29 -e 34
>  expr ptr
> -DONE
> +DONE
> \ No newline at end of file
> 
> Modified: 
> lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit?rev=354711&r1=354710&r2=354711&view=diff
> ==
> --- lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit 
> (original)
> +++ lldb/trunk/lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit 
> Fri Feb 22 17:08:17 2019
> @@ -1,7 +1,7 @@
> -break set -f stop-hook-threads.cpp -p "Break here to set up the stop hook"
>  break set -f stop-h

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

2019-02-25 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:31
+class CommandProvider;
+struct CommandProviderInfo {
+  static const char *name;

Doxygen comments?



Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:615
 
+  // Reproducer provider.
+  CommandProvider *m_provider = nullptr;

`///`



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:121
 
+class lldb_private::CommandProvider
+: public repro::Provider {

Any toplevel Doxygen comments that would be useful to add here?



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:132
+
+  void Keep() override {
+FileSpec file =

At least from the outside it's not quite obvious what this function does. Could 
you perhaps add a high-level comment?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58564



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

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

2019-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 188244.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Add comments


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

https://reviews.llvm.org/D58564

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
@@ -74,6 +75,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace llvm;
 
 static const char *k_white_space = " \t\v";
 
@@ -116,6 +118,51 @@
   eEchoCommentCommands = 5
 };
 
+/// Provider for the command interpreter. Every command is logged to file which
+/// is used as input during replay. The latter takes place at the SB API layer
+/// by changing the input file handle.
+class lldb_private::CommandProvider
+: public repro::Provider {
+public:
+  typedef CommandProviderInfo info;
+
+  CommandProvider(const FileSpec &directory) : Provider(directory) {}
+
+  /// Capture a single command.
+  void CaptureCommand(std::string command) {
+m_commands.push_back(std::move(command));
+  }
+
+  /// Commands are kept in memory and written to a file when the reproducer
+  /// needs to be kept.
+  void Keep() override {
+FileSpec file =
+GetRoot().CopyByAppendingPathComponent(CommandProviderInfo::file);
+
+std::error_code ec;
+llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::F_Text);
+
+if (ec)
+  return;
+
+for (auto &command : m_commands)
+  os << command << '\n';
+  }
+
+  /// Commands are kept in memory and are cleared when the reproducer is
+  /// discarded.
+  void Discard() override { m_commands.clear(); }
+
+  static char ID;
+
+private:
+  std::vector m_commands;
+};
+
+char CommandProvider::ID = 0;
+const char *CommandProviderInfo::name = "command-interpreter";
+const char *CommandProviderInfo::file = "command-interpreter.txt";
+
 ConstString &CommandInterpreter::GetStaticBroadcasterClass() {
   static ConstString class_name("lldb.commandInterpreter");
   return class_name;
@@ -141,6 +188,9 @@
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
   CheckInWithManager();
   m_collection_sp->Initialize(g_properties);
+
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+m_provider = &g->GetOrCreate();
 }
 
 bool CommandInterpreter::GetExpandRegexAliases() const {
@@ -1694,6 +1744,9 @@
 
   Status error(PreprocessCommand(command_string));
 
+  if (m_provider)
+m_provider->CaptureCommand(original_command_string);
+
   if (error.Fail()) {
 result.AppendError(error.AsCString());
 result.SetStatus(eReturnStatusFailed);
Index: lldb/source/Commands/CommandObjectReproducer.cpp
===
--- lldb/source/Commands/CommandObjectReproducer.cpp
+++ lldb/source/Commands/CommandObjectReproducer.cpp
@@ -37,8 +37,13 @@
 auto &r = repro::Reproducer::Instance();
 if (auto generator = r.GetGenerator()) {
   generator->Keep();
+} else if (r.GetLoader()) {
+  // Make this operation a NOP in replay mode.
+  result.SetStatus(eReturnStatusSuccessFinishNoResult);
+  return result.Succeeded();
 } else {
   result.AppendErrorWithFormat("Unable to get the reproducer generator");
+  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -27,6 +27,14 @@
 
 namespace lldb_private {
 
+/// Reproducer provider for the command interpreter. The info struct needs to
+/// be public because replay takes place at the SB API layer.
+class CommandProvider;
+struct CommandProviderInfo {
+  static const char *name;
+  static const char *file;
+};
+
 class CommandInterpreterRunOptions {
 public:
   //--
@@ -606,6 +614,9 @@
   bool m_quit_requested;
   bool m_stopped_for_crash;
 
+  /// Reproducer provider.
+  CommandProvider *m_provider = nullptr;
+
   // The exit code the user has requested when calling the 'quit' command.
   // No value means the user hasn't set a custom exit code so far.
   llvm::Optional m_quit_exit_code;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bi

[Lldb-commits] [PATCH] D58648: Improve process launch comments for Windows

2019-02-25 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth created this revision.
amccarth added a reviewer: zturner.

The existing comment about over-allocating the command line was incorrect.  The 
contents of the command line may be changed, but it's not necessary to over 
allocate.  The changes will be limited to the existing contents of the string 
(e.g., by replacing spaces with L'\0' to tokenize the command line).

Also added a comment explaining a possible cause of failure to save the next 
programmer some time when they try to debug a 64-bit process from a 32-bit LLDB.


https://reviews.llvm.org/D58648

Files:
  lldb/source/Host/windows/ProcessLauncherWindows.cpp


Index: lldb/source/Host/windows/ProcessLauncherWindows.cpp
===
--- lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -104,17 +104,21 @@
   llvm::ConvertUTF8toWide(commandLine, wcommandLine);
   llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetCString(),
   wworkingDirectory);
+  // If the command line is empty, it's best to pass a null pointer to tell
+  // CreateProcessW to use the executable name as the command line.  If the
+  // command line is not empty, its contents may be modified by CreateProcessW.
+  WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0];
 
-  wcommandLine.resize(PATH_MAX); // Needs to be over-allocated because
- // CreateProcessW can modify it
   BOOL result = ::CreateProcessW(
-  wexecutable.c_str(), &wcommandLine[0], NULL, NULL, TRUE, flags, 
env_block,
+  wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
   wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
   &startupinfo, &pi);
 
   if (!result) {
 // Call GetLastError before we make any other system calls.
 error.SetError(::GetLastError(), eErrorTypeWin32);
+// Note that error 50 ("The request is not supported") will occur if you
+// try debug a 64-bit inferior from a 32-bit LLDB.
   }
 
   if (result) {


Index: lldb/source/Host/windows/ProcessLauncherWindows.cpp
===
--- lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -104,17 +104,21 @@
   llvm::ConvertUTF8toWide(commandLine, wcommandLine);
   llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetCString(),
   wworkingDirectory);
+  // If the command line is empty, it's best to pass a null pointer to tell
+  // CreateProcessW to use the executable name as the command line.  If the
+  // command line is not empty, its contents may be modified by CreateProcessW.
+  WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0];
 
-  wcommandLine.resize(PATH_MAX); // Needs to be over-allocated because
- // CreateProcessW can modify it
   BOOL result = ::CreateProcessW(
-  wexecutable.c_str(), &wcommandLine[0], NULL, NULL, TRUE, flags, env_block,
+  wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
   wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
   &startupinfo, &pi);
 
   if (!result) {
 // Call GetLastError before we make any other system calls.
 error.SetError(::GetLastError(), eErrorTypeWin32);
+// Note that error 50 ("The request is not supported") will occur if you
+// try debug a 64-bit inferior from a 32-bit LLDB.
   }
 
   if (result) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58648: Improve process launch comments for Windows

2019-02-25 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth updated this revision to Diff 188263.
amccarth added a comment.

Forgot to include context in original diff.


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

https://reviews.llvm.org/D58648

Files:
  lldb/source/Host/windows/ProcessLauncherWindows.cpp


Index: lldb/source/Host/windows/ProcessLauncherWindows.cpp
===
--- lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -104,17 +104,21 @@
   llvm::ConvertUTF8toWide(commandLine, wcommandLine);
   llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetCString(),
   wworkingDirectory);
+  // If the command line is empty, it's best to pass a null pointer to tell
+  // CreateProcessW to use the executable name as the command line.  If the
+  // command line is not empty, its contents may be modified by CreateProcessW.
+  WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0];
 
-  wcommandLine.resize(PATH_MAX); // Needs to be over-allocated because
- // CreateProcessW can modify it
   BOOL result = ::CreateProcessW(
-  wexecutable.c_str(), &wcommandLine[0], NULL, NULL, TRUE, flags, 
env_block,
+  wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
   wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
   &startupinfo, &pi);
 
   if (!result) {
 // Call GetLastError before we make any other system calls.
 error.SetError(::GetLastError(), eErrorTypeWin32);
+// Note that error 50 ("The request is not supported") will occur if you
+// try debug a 64-bit inferior from a 32-bit LLDB.
   }
 
   if (result) {


Index: lldb/source/Host/windows/ProcessLauncherWindows.cpp
===
--- lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -104,17 +104,21 @@
   llvm::ConvertUTF8toWide(commandLine, wcommandLine);
   llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetCString(),
   wworkingDirectory);
+  // If the command line is empty, it's best to pass a null pointer to tell
+  // CreateProcessW to use the executable name as the command line.  If the
+  // command line is not empty, its contents may be modified by CreateProcessW.
+  WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0];
 
-  wcommandLine.resize(PATH_MAX); // Needs to be over-allocated because
- // CreateProcessW can modify it
   BOOL result = ::CreateProcessW(
-  wexecutable.c_str(), &wcommandLine[0], NULL, NULL, TRUE, flags, env_block,
+  wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
   wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
   &startupinfo, &pi);
 
   if (!result) {
 // Call GetLastError before we make any other system calls.
 error.SetError(::GetLastError(), eErrorTypeWin32);
+// Note that error 50 ("The request is not supported") will occur if you
+// try debug a 64-bit inferior from a 32-bit LLDB.
   }
 
   if (result) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58566: [Reproducers] Add more logging capabilities to reproducer instrumentation

2019-02-25 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:591
 
-LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording ({0}) 
'{1}'",
+#ifndef LLDB_REPRO_INSTR_TRACE
+LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording {0}: {1}",

Who defines this macro? I'm asking because it's unusual for LLDB to emit 
anything on stderr.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58566



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

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

2019-02-25 Thread Alex Langford via Phabricator via lldb-commits

xiaobai created this revision.
xiaobai added reviewers: aprantl, labath, JDevlieghere.
Herald added subscribers: jdoerfert, srhines.

I saw that `llvm::Triple::normalize` will set vendor, os, and
environment to "unknown" if they aren't set. Because we use that function when
creating a triple with a string, we need to make sure that this is accomodated
in functions that check for unknown.


https://reviews.llvm.org/D58653

Files:
  include/lldb/Utility/ArchSpec.h
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -232,3 +232,58 @@
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+ArchSpec A("x86-unknown-unknown");
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("arm-pc-linux-android");
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("aarch64-unknown-linux-android");
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("");
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("arm---");
+// Note: llvm::Triple::normalize interprets empty-string as "unknown"
+// instead of unspecified.
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+}
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -903,10 +903,10 @@
 if (other.GetCore() != eCore_uknownMach64)
   UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-  !TripleVendorWasSpecified()) {
-if (other.TripleVendorWasSpecified())
-  GetTriple().setEnvironment(other.GetTriple().getEnvironment());
+  if (TripleEnvironmentIsUnspecifiedUnknown() &&
+  !other.TripleEnvironmentIsUnspecifiedUnknown() &&
+  !TripleVendorWasSpecified() && other.TripleVendorWasSpecified()) {
+GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic
   // "some kind of arm" spec but the other ArchSpec is a specific arm core,
Index: include/lldb/Utility/ArchSpec.h
===
--- include/lldb/Utility/ArchSpec.h
+++ include/lldb/Utility/ArchSpec.h
@@ -378,18 +378,23 @@
 
   bool TripleVendorIsUnspecifiedUnknown() const {
 return m_triple.getVendor() == llvm::Triple::UnknownVendor &&
-   m_triple.getVendorName().empty();
+   (m_triple.getVendorName().empty() ||
+m_triple.getVendorName() == "unknown");
   }
 
   bool TripleOSWasSpecified() const { return !m_triple.getOSName().empty(); }
 
-  bool TripleEnvironmentWasSpecified() const {
-return !m_triple.getEnvironmentName().empty();
-  }
-
   bool TripleOSIsUnspecifiedUnknown() const {
 return m_triple.getOS() == llvm::Triple::UnknownOS &&
-   m_triple.getOSName().empty();
+   (m_triple.getOSName().empty() || m_triple.getOSName() == "unknown");
+  }
+
+  bool TripleEnvironmentWasSpecified() const { return m_triple.hasEnvironment(); }
+
+  bool TripleEnvironmentIsUnspecifiedUnknown() const {
+return m_triple.getEnvironment() == llvm::Triple::UnknownEnvironment &&
+   (!m_triple.hasEnvironment() ||
+m_triple.getEnvironmentName() == "unknown");
   }
 
   //-

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

2019-02-25 Thread Zachary Turner via Phabricator via lldb-commits

zturner created this revision.
zturner added reviewers: labath, jgorbe, JDevlieghere, davide.
Herald added subscribers: mgorny, emaste.

`Host/Config.h` is where we have platform specific preprocessor defines that 
are configured at CMake time.  Then, we can include this file 
`lldb/Host/Config.h` and read the value of defines.  This is basically 
identical to what llvm does with `llvm/Support/llvm-config.h`, the only 
fundamental differences being: a) LLDB configures it into `lldb/Host` where as 
the spiritual equivalent to `llvm/Support` is `lldb/Utility`, and b) LLDB calls 
it `Config.h` and LLVM calls it `llvm-config.h`.

This patch brings LLDB in line with LLVM here by configuring into 
`lldb/Utility/lldb-config.h`, in part for consistency and in part for more 
practical reasons.

The practical reasons are that I want to use `Socket.h` from a new tool / 
library without having to link all of LLDB such as Clang, Python, etc, and 
that's not currently possible with the current layering, as linking to Host 
will link to everything.

So this is a necessary first step.  `llvm/Support`'s model for handling 
platform specific differences is quite convenient both from a usability as well 
as a maintenance perspective, and I'd like to gradually move towards that 
whenever an opportunity / need arises.  So the immediate plan is to move 
`config.h` to Utility, and then start by moving pieces -- as necessary -- from 
Host to Utility until I can get `Socket` just by linking to `lldbUtility`.


https://reviews.llvm.org/D58654

Files:
  lldb/cmake/XcodeHeaderGenerator/CMakeLists.txt
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/include/lldb/Host/Config.h
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/MainLoop.h
  lldb/include/lldb/Host/Terminal.h
  lldb/include/lldb/Host/linux/Uio.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/lldb-config.h
  lldb/include/lldb/Utility/lldb-config.h.cmake
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Host/common/PseudoTerminal.cpp
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/common/Terminal.cpp
  lldb/source/Host/common/UDPSocket.cpp
  lldb/source/Host/linux/HostInfoLinux.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/unittests/Host/SocketTest.cpp

Index: lldb/unittests/Host/SocketTest.cpp
===
--- lldb/unittests/Host/SocketTest.cpp
+++ lldb/unittests/Host/SocketTest.cpp
@@ -12,10 +12,10 @@
 
 #include "gtest/gtest.h"
 
-#include "lldb/Host/Config.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "lldb/Host/common/UDPSocket.h"
+#include "lldb/Utility/lldb-config.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
+#include "lldb/Utility/lldb-config.h"
 
 #include 
 #include 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -19,7 +19,6 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 
-#include "lldb/Host/Config.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileAction.h"
 #include "lldb/

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

2019-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Makes sense, no objections from me.


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

https://reviews.llvm.org/D58654



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

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

2019-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Do we actually need to check the string values in addition to the enum values? 
It looks like the llvm class tries pretty hard to make that the canonical way 
to query it. I know triples and arches are pretty tricky in lldb though, so 
maybe there's a reason that we can't.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

2019-02-25 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

Utility is supposed to be a bunch of stand-alone utility files & headers that 
we gather together for convenience's sake.

Host is where we put all the code that is specific to one or another host, and 
any support files that requires.  For instance, that's why PlatformLinux and 
friends are there, and all the independent platform directories.  So it is odd 
to move platform specific defines from Host to Utility.

I don't care much about the name, but having this in Utility when it is setting 
Host defines and we have a place clearly set out for host support files doesn't 
seem right.


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

https://reviews.llvm.org/D58654



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

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

2019-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

- Add testcase.
- Don't log sourced commands twice.

For some reason I thought that we no longer needed to differentiate between 
commands being sourced from a file. I remember an earlier discussing with Pavel 
on this topic where we considered doing this at a lower level, but there we 
face the same issue, without an easy way to differentiate.


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

https://reviews.llvm.org/D58564

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/lit/Reproducer/Inputs/CommandRepro.in
  lldb/lit/Reproducer/TestCommandRepro.test
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
@@ -74,6 +75,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace llvm;
 
 static const char *k_white_space = " \t\v";
 
@@ -116,6 +118,51 @@
   eEchoCommentCommands = 5
 };
 
+/// Provider for the command interpreter. Every command is logged to file which
+/// is used as input during replay. The latter takes place at the SB API layer
+/// by changing the input file handle.
+class lldb_private::CommandProvider
+: public repro::Provider {
+public:
+  typedef CommandProviderInfo info;
+
+  CommandProvider(const FileSpec &directory) : Provider(directory) {}
+
+  /// Capture a single command.
+  void CaptureCommand(std::string command) {
+m_commands.push_back(std::move(command));
+  }
+
+  /// Commands are kept in memory and written to a file when the reproducer
+  /// needs to be kept.
+  void Keep() override {
+FileSpec file =
+GetRoot().CopyByAppendingPathComponent(CommandProviderInfo::file);
+
+std::error_code ec;
+llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::F_Text);
+
+if (ec)
+  return;
+
+for (auto &command : m_commands)
+  os << command << '\n';
+  }
+
+  /// Commands are kept in memory and are cleared when the reproducer is
+  /// discarded.
+  void Discard() override { m_commands.clear(); }
+
+  static char ID;
+
+private:
+  std::vector m_commands;
+};
+
+char CommandProvider::ID = 0;
+const char *CommandProviderInfo::name = "command-interpreter";
+const char *CommandProviderInfo::file = "command-interpreter.txt";
+
 ConstString &CommandInterpreter::GetStaticBroadcasterClass() {
   static ConstString class_name("lldb.commandInterpreter");
   return class_name;
@@ -141,6 +188,9 @@
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
   CheckInWithManager();
   m_collection_sp->Initialize(g_properties);
+
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+m_provider = &g->GetOrCreate();
 }
 
 bool CommandInterpreter::GetExpandRegexAliases() const {
@@ -1604,7 +1654,8 @@
CommandReturnObject &result,
ExecutionContext *override_context,
bool repeat_on_empty_command,
-   bool no_context_switching)
+   bool no_context_switching,
+   bool add_to_reproducer)
 
 {
 
@@ -1694,6 +1745,9 @@
 
   Status error(PreprocessCommand(command_string));
 
+  if (m_provider && add_to_reproducer)
+m_provider->CaptureCommand(original_command_string);
+
   if (error.Fail()) {
 result.AppendError(error.AsCString());
 result.SetStatus(eReturnStatusFailed);
@@ -2175,6 +2229,7 @@
 options.SetSilent(true);
 options.SetStopOnError(false);
 options.SetStopOnContinue(true);
+options.SetAddToReproducer(false);
 
 HandleCommandsFromFile(init_file,
nullptr, // Execution context
@@ -2252,7 +2307,8 @@
 HandleCommand(cmd, options.m_add_to_history, tmp_result,
   nullptr, /* override_context */
   true,/* repeat_on_empty_command */
-  override_context != nullptr /* no_context_switching */);
+  override_context != nullptr /* no_context_switching */,
+  options.GetAddToReproducer());
 if (!options.GetAddToHistory())
   m_command_source_depth--;
 
@@ -2364,7 +2420,8 @@
   eHandleCommandFlagEchoCommand = (1u << 2),
   eHandleCommandFlagEchoCommentCommand = (1u << 3),
   eHandleCommandFlagPrintResult = (1u << 4),
-  eHandleCommandFlagStopOnCrash = (1u << 5)
+  eHandleCommandFlagStopOnCrash

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

2019-02-25 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

In D58654#1409974 , @jingham wrote:

> Utility is supposed to be a bunch of stand-alone utility files & headers that 
> we gather together for convenience's sake.
>
> Host is where we put all the code that is specific to one or another host, 
> and any support files that requires.  For instance, that's why PlatformLinux 
> and friends are there, and all the independent platform directories.  So it 
> is odd to move platform specific defines from Host to Utility.
>
> I don't care much about the name, but having this in Utility when it is 
> setting Host defines and we have a place clearly set out for host support 
> files doesn't seem right.

That is how Host started out, but I think that that distinction both a) hasn't 
played out the way we hoped for in practice, and b) isn't necessarily the right 
line to draw anyway.  We have quite a few ifdefs scattered around the codebase 
for different platforms that are not in Host in places where we couldn't easily 
(or cleanly) make the code fit into that separation model.  On the flip side, 
lots of stuff has crept into Host that isn't really host-specific, which is why 
the dependencies are so tangled up.

I think following the LLVM/Support model makes sense.  It's "Utility", plus 
what we would normally think of as "Host".  This is a lot cleaner than it 
sounds, and all of the ifdefs and platform specific defines end up only in 
private implementation files, with none exposed through headers.

To be honest, I don't mind if we call it something other than Host (for example 
it could be called System or something), but the practical issue with using 
Host today is that it still has a ways to go before it can be linked against 
without pulling in the rest of LLDB, and the changes needed to get there are 
not trivial, so it will take some more continued effort.

Regardless, in my mind Utility is exactly what you said, but that definition 
doesn't actually preclude Host-specific things.  In fact, putting Host-specific 
abstractions there solves a lot of problems.  Imagine, for example, someone 
wanted to create a debugging related tool that was not a full-fledged debugger. 
 They might want to use lots of functionality that is in Utility and Host, but 
not bring in everything else.  That's what Utility is to me.  It's a set of 
abstractions that are useful for creating debugging-related tooling that 
someone can use for that purpose.

I don't necessarily care if it's in Utility or some other newly created library 
we call something else, but we need some immediate way of using these 
abstractions without linking against the rest of LLDB so that we're not blocked 
on finishing the layering split.

One option would be to just keep growing Utility with these kinds of classes 
and abstractions until we decide that it's too big, at which point a separation 
would probably fall out naturally (FWIW, this is how LLVM/Support and LLVM/ADT 
evolved into what they are)

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

https://reviews.llvm.org/D58654

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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

2019-02-25 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

There aren't many more platforms (OpenVMS?) that are likely to show up and need 
support for lldb, so maybe this is making too much of the matter.  But having 
there be a well defined set of places where you need to look when porting lldb 
to a new host platform seems a useful design to me.  The fact that we weren't 
strict enough and allowed Host platform dependencies to creep in where they 
don't belong doesn't argue we should just abandon trying to keep the places 
where a port sockets into lldb regular.

It's fine to have a staging area ("WillBeHost"?) to help pull out the classes 
that it is simple to pull out of host.  But I'd rather not glom them all into 
Utility, which is already becoming a little incoherent.


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

https://reviews.llvm.org/D58654



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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

2019-02-25 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

In D58654#1410080 , @jingham wrote:

> There aren't many more platforms (OpenVMS?) that are likely to show up and 
> need support for lldb, so maybe this is making too much of the matter.  But 
> having there be a well defined set of places where you need to look when 
> porting lldb to a new host platform seems a useful design to me.  The fact 
> that we weren't strict enough and allowed Host platform dependencies to creep 
> in where they don't belong doesn't argue we should just abandon trying to 
> keep the places where a port sockets into lldb regular.
>
> It's fine to have a staging area ("WillBeHost"?) to help pull out the classes 
> that it is simple to pull out of host.  But I'd rather not glom them all into 
> Utility, which is already becoming a little incoherent.

How about "System" then?  FTR, I'm also fine eventually renaming it back to 
Host once all the layering issues are addressed.  The important thing is that 
if it's really just a set of platform-specific abstractions, it shouldn't 
really depend on anything else (except perhaps Utility).

I guess one advantage to having them be together though is that often even the 
Utility code itself needs to make use of platform-specific abstractions.  You 
can already see this anywhere that lldb/Utility calls into llvm/Support to do 
things like read from the file system or start a thread (TBH, I'm actually not 
even sure it does any of those things, but it seems reasonable that it might 
want to).  So if we make a library called System, then it's possible that 
System ends up depending on Utility and Utility ends up depending on System.  
Despite all of mine (and others') push for proper layering, this is one area 
where I'm actually ok with it.  LLVM has the same issue here, where ADT and 
Support depend on each other and just linked together into one big .lib, 
despite being in separate folders.

But it's something to keep in mind.

Anyway, does that seem reasonable?  Put it in a new folder called `System`?

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

https://reviews.llvm.org/D58654

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

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

2019-02-25 Thread Alex Langford via Phabricator via lldb-commits

xiaobai updated this revision to Diff 188309.
xiaobai added a comment.

Remove redundant checks for triple components


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

https://reviews.llvm.org/D58653

Files:
  include/lldb/Utility/ArchSpec.h
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -232,3 +232,58 @@
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+ArchSpec A("x86-unknown-unknown");
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("arm-pc-linux-android");
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("aarch64-unknown-linux-android");
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("");
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("arm---");
+// Note: llvm::Triple::normalize interprets empty-string as "unknown"
+// instead of unspecified.
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+}
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -903,10 +903,10 @@
 if (other.GetCore() != eCore_uknownMach64)
   UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-  !TripleVendorWasSpecified()) {
-if (other.TripleVendorWasSpecified())
-  GetTriple().setEnvironment(other.GetTriple().getEnvironment());
+  if (TripleEnvironmentIsUnspecifiedUnknown() &&
+  !other.TripleEnvironmentIsUnspecifiedUnknown() &&
+  !TripleVendorWasSpecified() && other.TripleVendorWasSpecified()) {
+GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic
   // "some kind of arm" spec but the other ArchSpec is a specific arm core,
Index: include/lldb/Utility/ArchSpec.h
===
--- include/lldb/Utility/ArchSpec.h
+++ include/lldb/Utility/ArchSpec.h
@@ -377,19 +377,19 @@
   }
 
   bool TripleVendorIsUnspecifiedUnknown() const {
-return m_triple.getVendor() == llvm::Triple::UnknownVendor &&
-   m_triple.getVendorName().empty();
+return m_triple.getVendor() == llvm::Triple::UnknownVendor;
   }
 
   bool TripleOSWasSpecified() const { return !m_triple.getOSName().empty(); }
 
-  bool TripleEnvironmentWasSpecified() const {
-return !m_triple.getEnvironmentName().empty();
+  bool TripleOSIsUnspecifiedUnknown() const {
+return m_triple.getOS() == llvm::Triple::UnknownOS;
   }
 
-  bool TripleOSIsUnspecifiedUnknown() const {
-return m_triple.getOS() == llvm::Triple::UnknownOS &&
-   m_triple.getOSName().empty();
+  bool TripleEnvironmentWasSpecified() const { return m_triple.hasEnvironment(); }
+
+  bool TripleEnvironmentIsUnspecifiedUnknown() const {
+return m_triple.getEnvironment() == llvm::Triple::UnknownEnvironment;
   }
 
   //--
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

2019-02-25 Thread Alex Langford via Phabricator via lldb-commits

xiaobai added a comment.

In D58653#1409978 , @JDevlieghere 
wrote:

> Do we actually need to check the string values in addition to the enum 
> values? It looks like the llvm class tries pretty hard to make that the 
> canonical way to query it. I know triples and arches are pretty tricky in 
> lldb though, so maybe there's a reason that we can't.


I wasn't sure when I was writing this, but I just tried and the tests seem to 
pass when we just check the enum values so I presume it's fine.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

2019-02-25 Thread Alex Langford via Phabricator via lldb-commits

xiaobai created this revision.
xiaobai added reviewers: aprantl, labath, JDevlieghere, clayborg.
Herald added subscribers: jdoerfert, srhines.

This behavior was originally added in rL252264 
 (git commit 76a7f365da)
in order to be extra careful with handling platforms like watchos and tvos.
However, as far as triples go, those two (and others) are treated as OSes and
not environments, so that should not really apply here.

Additionally, this behavior is incorrect and can lead to incorrect ArchSpecs.
Because android is specified as an environment and not an OS, not propogating
the environment can lead to modules and targets being misidentified.


https://reviews.llvm.org/D58664

Files:
  include/lldb/Utility/ArchSpec.h
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
+
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+A.MergeFrom(B);
+ASSERT_TRUE(A.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  }
+  {
+ArchSpec A("aarch64-unknown-unknown-unknown");
+ArchSpec B("aarch64-unknown-linux-android");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
@@ -232,3 +256,58 @@
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+ArchSpec A("x86-unknown-unknown");
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("arm-pc-linux-android");
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("aarch64-unknown-linux-android");
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_TRUE(A.TripleOSWasSpecified());
+ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+ArchSpec A("");
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

2019-02-25 Thread Alex Langford via Phabricator via lldb-commits

xiaobai updated this revision to Diff 188312.
xiaobai set the repository for this revision to rLLDB LLDB.
xiaobai added a comment.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

Accidentally added a commit to this diff that I didn't intend to add.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58664

Files:
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
+
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+A.MergeFrom(B);
+ASSERT_TRUE(A.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  }
+  {
+ArchSpec A("aarch64-unknown-unknown-unknown");
+ArchSpec B("aarch64-unknown-linux-android");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -904,8 +904,7 @@
   UpdateCore();
   }
   if (TripleEnvironmentIsUnspecifiedUnknown() &&
-  !other.TripleEnvironmentIsUnspecifiedUnknown() &&
-  !TripleVendorWasSpecified() && other.TripleVendorWasSpecified()) {
+  !other.TripleEnvironmentIsUnspecifiedUnknown()) {
 GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
+
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

2019-02-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added a reviewer: clayborg.
labath added a comment.

I think this basically defeats the purpose of the `IsUnspecifiedUnknown` 
functions, which was to differentiate between foo-unknown-bar and foo--bar 
triples. That in itself was a pretty big hack, but it seems that there is 
functionality which depends on this. There was a discussion about this back in 
december http://lists.llvm.org/pipermail/lldb-dev/2018-December/014437.html, 
and IIRC it converged to some sort of a conclusion, but I don't think there 
hasn't been any effort to try to implement it. I'm not sure what exactly it 
means about the future of this patch, but I'd be cautious about it.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [lldb] r354766 - Finish revert of r354706

[Lldb-commits] [PATCH] D58394: Add --auto-continue to stop-hooks, fix up a few tests

[Lldb-commits] [PATCH] D58394: Add --auto-continue to stop-hooks, fix up a few tests

[Lldb-commits] [PATCH] D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

[Lldb-commits] [PATCH] D58610: [lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Lldb-commits] [PATCH] D58339: Changes for running LLDB test suite for Swift on PowerPC64LE

[Lldb-commits] [PATCH] D58339: Changes for running LLDB test suite for Swift on PowerPC64LE

[Lldb-commits] [PATCH] D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

[Lldb-commits] [lldb] r354798 - [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

[Lldb-commits] [PATCH] D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment

[Lldb-commits] [PATCH] D58630: [lldb] [test] Pass appropriate -L for just-built libc++

[Lldb-commits] [lldb] r354803 - [lldb-mi] Fix conversion warning for 64-bit build

[Lldb-commits] [lldb] r354804 - [lldb-mi] Return source line number in proper format

Re: [Lldb-commits] [lldb] r354711 - Revert r354706 - lit touched my thigh

[Lldb-commits] [PATCH] D58565: [Reproducers] Enable replay from SBRepro

Re: [Lldb-commits] [lldb] r354711 - Revert r354706 - lit touched my thigh

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

[Lldb-commits] [PATCH] D58648: Improve process launch comments for Windows

[Lldb-commits] [PATCH] D58648: Improve process launch comments for Windows

[Lldb-commits] [PATCH] D58566: [Reproducers] Add more logging capabilities to reproducer instrumentation

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

36 matches

Site Navigation

Mail list logo

Footer information