[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

[Stephen Neuendorffer via Phabricator via lldb-commits]

stephenneuendorffer added a comment.

Perfect!  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84691



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

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

[Petr Hosek via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG64d99cc6abed: [CMake] Move find_package(ZLIB) to LLVMConfig 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84691

Files:
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  mlir/examples/standalone/CMakeLists.txt


Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -29,10 +29,6 @@
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -50,6 +50,9 @@
 set(LLVM_ENABLE_UNWIND_TABLES @LLVM_ENABLE_UNWIND_TABLES@)
 
 set(LLVM_ENABLE_ZLIB @LLVM_ENABLE_ZLIB@)
+if(LLVM_ENABLE_ZLIB)
+  find_package(ZLIB)
+endif()
 
 set(LLVM_LIBXML2_ENABLED @LLVM_LIBXML2_ENABLED@)
 
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -74,10 +74,6 @@
 # CMake modules to be in that directory as well.
 list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(AddLLVM)
 include(TableGen)
 include(HandleLLVMOptions)
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -51,10 +51,6 @@
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} 
NO_DEFAULT_PATH)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -114,10 +114,6 @@
   option(CLANG_ENABLE_BOOTSTRAP "Generate the clang bootstrap target" OFF)
   option(LLVM_ENABLE_LIBXML2 "Use libxml2 if available." ON)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)


Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -29,10 +29,6 @@
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -50,6 +50,9 @@
 set(LLVM_ENABLE_UNWIND_TABLES @LLVM_ENABLE_UNWIND_TABLES@)
 
 set(LLVM_ENABLE_ZLIB @LLVM_ENABLE_ZLIB@)
+if(LLVM_ENABLE_ZLIB)
+  find_package(ZLIB)
+endif()
 
 set(LLVM_LIBXML2_ENABLED @LLVM_LIBXML2_ENABLED@)
 
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -74,10 +74,6 @@
 # CMake modules to be in that directory as well.
 list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(AddLLVM)
 include(TableGen)
 include(HandleLLVMOptions)
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -51,10 +51,6 @@
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -114,10 +114,6 @@
   option(CLANG_ENABLE_BOOTSTRAP "Generate the clang bootstrap target" OFF)
   option(LLVM_ENABLE_LIBXML2 "Use libxml2 if available." ON)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)
___
lldb-commits mailing list

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

[Mehdi AMINI via Phabricator via lldb-commits]

mehdi_amini added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84691



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

[Lldb-commits] [lldb] 8120eba - [lldb/ArchSpec] Always match simulator environment in IsEqualTo

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-07-27T17:33:37-07:00
New Revision: 8120eba5fce378083ef22651f2b7b6dcaa54a098

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

LOG: [lldb/ArchSpec] Always match simulator environment in IsEqualTo

Summary:
Initially, Apple simulator binarie triples didn't use a `-simulator`
environment and were just differentiated based on the architecture.
For example, `x86_64-apple-ios` would obviously be a simualtor as iOS
doesn't run on x86_64. With Catalyst, we made the disctinction
explicit and today, all simulator triples (even the legacy ones) are
constructed with an environment. This is especially important on Apple
Silicon were the architecture is not different from the one of the
simulated device.

This change makes the simulator part of the environment always part of
the criteria to detect whether 2 `ArchSpec`s are equal or compatible.

Reviewers: aprantl

Subscribers: inglorion, dexonsmith, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Utility/ArchSpec.cpp
lldb/unittests/Utility/ArchSpecTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index cd382a322da7..6e4f1b5326dd 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -987,6 +987,12 @@ static bool 
IsCompatibleEnvironment(llvm::Triple::EnvironmentType lhs,
   if (lhs == rhs)
 return true;
 
+  // Apple simulators are a 
diff erent platform than what they simulate.
+  // As the environments are 
diff erent at this point, if one of them is a
+  // simulator, then they are 
diff erent.
+  if (lhs == llvm::Triple::Simulator || rhs == llvm::Triple::Simulator)
+return false;
+
   // If any of the environment is unknown then they are compatible
   if (lhs == llvm::Triple::UnknownEnvironment ||
   rhs == llvm::Triple::UnknownEnvironment)

diff  --git a/lldb/unittests/Utility/ArchSpecTest.cpp 
b/lldb/unittests/Utility/ArchSpecTest.cpp
index a8f43ed7dc7c..ad0a8ac18cd1 100644
--- a/lldb/unittests/Utility/ArchSpecTest.cpp
+++ b/lldb/unittests/Utility/ArchSpecTest.cpp
@@ -306,6 +306,14 @@ TEST(ArchSpecTest, Compatibility) {
 ASSERT_FALSE(A.IsExactMatch(B));
 ASSERT_FALSE(A.IsCompatibleMatch(B));
   }
+  {
+ArchSpec A("arm64-apple-ios");
+ArchSpec B("arm64-apple-ios-simulator");
+ASSERT_FALSE(A.IsExactMatch(B));
+ASSERT_FALSE(A.IsCompatibleMatch(B));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+  }
   {
 ArchSpec A("arm64-*-*");
 ArchSpec B("arm64-apple-ios");



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

[Lldb-commits] [PATCH] D84716: [lldb/ArchSpec] Always match simulator environment in IsEqualTo

[Frederic Riss via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8120eba5fce3: [lldb/ArchSpec] Always match simulator 
environment in IsEqualTo (authored by friss).

Changed prior to commit:
  https://reviews.llvm.org/D84716?vs=281084=281088#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84716

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


Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -306,6 +306,14 @@
 ASSERT_FALSE(A.IsExactMatch(B));
 ASSERT_FALSE(A.IsCompatibleMatch(B));
   }
+  {
+ArchSpec A("arm64-apple-ios");
+ArchSpec B("arm64-apple-ios-simulator");
+ASSERT_FALSE(A.IsExactMatch(B));
+ASSERT_FALSE(A.IsCompatibleMatch(B));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+  }
   {
 ArchSpec A("arm64-*-*");
 ArchSpec B("arm64-apple-ios");
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -987,6 +987,12 @@
   if (lhs == rhs)
 return true;
 
+  // Apple simulators are a different platform than what they simulate.
+  // As the environments are different at this point, if one of them is a
+  // simulator, then they are different.
+  if (lhs == llvm::Triple::Simulator || rhs == llvm::Triple::Simulator)
+return false;
+
   // If any of the environment is unknown then they are compatible
   if (lhs == llvm::Triple::UnknownEnvironment ||
   rhs == llvm::Triple::UnknownEnvironment)


Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -306,6 +306,14 @@
 ASSERT_FALSE(A.IsExactMatch(B));
 ASSERT_FALSE(A.IsCompatibleMatch(B));
   }
+  {
+ArchSpec A("arm64-apple-ios");
+ArchSpec B("arm64-apple-ios-simulator");
+ASSERT_FALSE(A.IsExactMatch(B));
+ASSERT_FALSE(A.IsCompatibleMatch(B));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+  }
   {
 ArchSpec A("arm64-*-*");
 ArchSpec B("arm64-apple-ios");
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -987,6 +987,12 @@
   if (lhs == rhs)
 return true;
 
+  // Apple simulators are a different platform than what they simulate.
+  // As the environments are different at this point, if one of them is a
+  // simulator, then they are different.
+  if (lhs == llvm::Triple::Simulator || rhs == llvm::Triple::Simulator)
+return false;
+
   // If any of the environment is unknown then they are compatible
   if (lhs == llvm::Triple::UnknownEnvironment ||
   rhs == 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] D84716: [lldb/ArchSpec] Always match simulator environment in IsEqualTo

[Frederic Riss via Phabricator via lldb-commits]

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



Comment at: lldb/unittests/Utility/ArchSpecTest.cpp:329
 ASSERT_FALSE(A.IsExactMatch(B));
 // FIXME: See above, though the extra environment complicates things.
 ASSERT_FALSE(A.IsCompatibleMatch(B));

aprantl wrote:
> What's to fix here?
Aren't you the one who introduced this comment? I can move my new test one slot 
up so it makes this a little less confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84716



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

[Lldb-commits] [lldb] 64d99cc - [CMake] Move find_package(ZLIB) to LLVMConfig

[Petr Hosek via lldb-commits]

Author: Petr Hosek
Date: 2020-07-27T17:13:55-07:00
New Revision: 64d99cc6abed78c00a2a7863b02ce54911a5264f

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

LOG: [CMake] Move find_package(ZLIB) to LLVMConfig

This way, downstream projects don't have to invoke find_package(ZLIB)
reducing the amount of boilerplate.

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

Added: 


Modified: 
clang/CMakeLists.txt
lld/CMakeLists.txt
lldb/cmake/modules/LLDBStandalone.cmake
llvm/cmake/modules/LLVMConfig.cmake.in
mlir/examples/standalone/CMakeLists.txt

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 1c4c22b1aaad..1a6a20a271f3 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -114,10 +114,6 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
   option(CLANG_ENABLE_BOOTSTRAP "Generate the clang bootstrap target" OFF)
   option(LLVM_ENABLE_LIBXML2 "Use libxml2 if available." ON)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)

diff  --git a/lld/CMakeLists.txt b/lld/CMakeLists.txt
index bcfc2c6270b3..e9bd1bd29c5c 100644
--- a/lld/CMakeLists.txt
+++ b/lld/CMakeLists.txt
@@ -51,10 +51,6 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} 
NO_DEFAULT_PATH)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)

diff  --git a/lldb/cmake/modules/LLDBStandalone.cmake 
b/lldb/cmake/modules/LLDBStandalone.cmake
index edd2b34ec865..94781c358374 100644
--- a/lldb/cmake/modules/LLDBStandalone.cmake
+++ b/lldb/cmake/modules/LLDBStandalone.cmake
@@ -74,10 +74,6 @@ endif()
 # CMake modules to be in that directory as well.
 list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(AddLLVM)
 include(TableGen)
 include(HandleLLVMOptions)

diff  --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index e729a839f614..17cc5eacc57b 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -50,6 +50,9 @@ set(LLVM_ENABLE_THREADS @LLVM_ENABLE_THREADS@)
 set(LLVM_ENABLE_UNWIND_TABLES @LLVM_ENABLE_UNWIND_TABLES@)
 
 set(LLVM_ENABLE_ZLIB @LLVM_ENABLE_ZLIB@)
+if(LLVM_ENABLE_ZLIB)
+  find_package(ZLIB)
+endif()
 
 set(LLVM_LIBXML2_ENABLED @LLVM_LIBXML2_ENABLED@)
 

diff  --git a/mlir/examples/standalone/CMakeLists.txt 
b/mlir/examples/standalone/CMakeLists.txt
index 59d3c693546f..3f46dda4e4f6 100644
--- a/mlir/examples/standalone/CMakeLists.txt
+++ b/mlir/examples/standalone/CMakeLists.txt
@@ -29,10 +29,6 @@ set(MLIR_BINARY_DIR ${CMAKE_BINARY_DIR})
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)



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

[Lldb-commits] [PATCH] D84716: [lldb/ArchSpec] Always match simulator environment in IsEqualTo

[Adrian Prantl via Phabricator via lldb-commits]

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

Yes, this makes perfect sense. Thanks!




Comment at: lldb/unittests/Utility/ArchSpecTest.cpp:329
 ASSERT_FALSE(A.IsExactMatch(B));
 // FIXME: See above, though the extra environment complicates things.
 ASSERT_FALSE(A.IsCompatibleMatch(B));

What's to fix here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84716



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

[Lldb-commits] [PATCH] D84716: [lldb/ArchSpec] Always match simulator environment in IsEqualTo

[Frederic Riss via Phabricator via lldb-commits]

friss created this revision.
friss added a reviewer: aprantl.
Herald added subscribers: dexonsmith, inglorion.
Herald added a project: LLDB.

Initially, Apple simulator binarie triples didn't use a `-simulator`
environment and were just differentiated based on the architecture.
For example, `x86_64-apple-ios` would obviously be a simualtor as iOS
doesn't run on x86_64. With Catalyst, we made the disctinction
explicit and today, all simulator triples (even the legacy ones) are
constructed with an environment. This is especially important on Apple
Silicon were the architecture is not different from the one of the
simulated device.

This change makes the simulator part of the environment always part of
the criteria to detect whether 2 `ArchSpec`s are equal or compatible.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84716

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


Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -314,6 +314,14 @@
 // this is the desired behavior.
 ASSERT_FALSE(A.IsCompatibleMatch(B));
   }
+  {
+ArchSpec A("arm64-apple-ios");
+ArchSpec B("arm64-apple-ios-simulator");
+ASSERT_FALSE(A.IsExactMatch(B));
+ASSERT_FALSE(A.IsCompatibleMatch(B));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+  }
   {
 ArchSpec A("x86_64-*-*");
 ArchSpec B("x86_64-apple-ios-simulator");
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -987,6 +987,12 @@
   if (lhs == rhs)
 return true;
 
+  // Apple simulators are a different platform than what they simulate.
+  // As the environments are different at this point, if one of them is a
+  // simulator, then they are different.
+  if (lhs == llvm::Triple::Simulator || rhs == llvm::Triple::Simulator)
+return false;
+
   // If any of the environment is unknown then they are compatible
   if (lhs == llvm::Triple::UnknownEnvironment ||
   rhs == llvm::Triple::UnknownEnvironment)


Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -314,6 +314,14 @@
 // this is the desired behavior.
 ASSERT_FALSE(A.IsCompatibleMatch(B));
   }
+  {
+ArchSpec A("arm64-apple-ios");
+ArchSpec B("arm64-apple-ios-simulator");
+ASSERT_FALSE(A.IsExactMatch(B));
+ASSERT_FALSE(A.IsCompatibleMatch(B));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+ASSERT_FALSE(B.IsCompatibleMatch(A));
+  }
   {
 ArchSpec A("x86_64-*-*");
 ArchSpec B("x86_64-apple-ios-simulator");
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -987,6 +987,12 @@
   if (lhs == rhs)
 return true;
 
+  // Apple simulators are a different platform than what they simulate.
+  // As the environments are different at this point, if one of them is a
+  // simulator, then they are different.
+  if (lhs == llvm::Triple::Simulator || rhs == llvm::Triple::Simulator)
+return false;
+
   // If any of the environment is unknown then they are compatible
   if (lhs == llvm::Triple::UnknownEnvironment ||
   rhs == 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] D79219: [CMake] Simplify CMake handling for zlib

[Mehdi AMINI via Phabricator via lldb-commits]

mehdi_amini added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

I am a bit unsure that it is desirable that it is desirable to need this change?
This requires every single user of LLVM to do this. Is there a way this can be 
done in the call to `find_package(LLVM)` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Stephen Neuendorffer via Phabricator via lldb-commits]

stephenneuendorffer added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

mehdi_amini wrote:
> mehdi_amini wrote:
> > I am a bit unsure that it is desirable that it is desirable to need this 
> > change?
> > This requires every single user of LLVM to do this. Is there a way this can 
> > be done in the call to `find_package(LLVM)` instead?
> (Doc for the usual way to integrate LLVM: 
> https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project )
I tend to agree with Mehdi.  I think stuff like this which is *required* to use 
LLVM should get pushed into what LLVM exports.  (Actually there is a bunch of 
other configuration stuff which also needs to get pushed into the export, 
otherwise configuration defaults are all wrong and things like that.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

[Petr Hosek via Phabricator via lldb-commits]

phosek created this revision.
phosek added reviewers: smeenai, compnerd, mehdi_amini.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, 
lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, 
jpienaar, rriddle, mgorny.
Herald added projects: clang, LLDB, MLIR, LLVM.

This way, downstream projects don't have to invoke find_package(ZLIB)
reducing the amount of boilerplate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84691

Files:
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  mlir/examples/standalone/CMakeLists.txt


Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -29,10 +29,6 @@
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -50,6 +50,9 @@
 set(LLVM_ENABLE_UNWIND_TABLES @LLVM_ENABLE_UNWIND_TABLES@)
 
 set(LLVM_ENABLE_ZLIB @LLVM_ENABLE_ZLIB@)
+if(LLVM_ENABLE_ZLIB)
+  find_package(ZLIB)
+endif()
 
 set(LLVM_LIBXML2_ENABLED @LLVM_LIBXML2_ENABLED@)
 
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -74,10 +74,6 @@
 # CMake modules to be in that directory as well.
 list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(AddLLVM)
 include(TableGen)
 include(HandleLLVMOptions)
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -51,10 +51,6 @@
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} 
NO_DEFAULT_PATH)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -114,10 +114,6 @@
   option(CLANG_ENABLE_BOOTSTRAP "Generate the clang bootstrap target" OFF)
   option(LLVM_ENABLE_LIBXML2 "Use libxml2 if available." ON)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)


Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -29,10 +29,6 @@
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -50,6 +50,9 @@
 set(LLVM_ENABLE_UNWIND_TABLES @LLVM_ENABLE_UNWIND_TABLES@)
 
 set(LLVM_ENABLE_ZLIB @LLVM_ENABLE_ZLIB@)
+if(LLVM_ENABLE_ZLIB)
+  find_package(ZLIB)
+endif()
 
 set(LLVM_LIBXML2_ENABLED @LLVM_LIBXML2_ENABLED@)
 
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -74,10 +74,6 @@
 # CMake modules to be in that directory as well.
 list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
 
-if(LLVM_ENABLE_ZLIB)
-  find_package(ZLIB)
-endif()
-
 include(AddLLVM)
 include(TableGen)
 include(HandleLLVMOptions)
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -51,10 +51,6 @@
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -114,10 +114,6 @@
   option(CLANG_ENABLE_BOOTSTRAP "Generate the clang bootstrap target" OFF)
   

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

[Saleem Abdulrasool via Phabricator via lldb-commits]

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

Thanks, this is a great idea!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84691



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

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Mehdi AMINI via Phabricator via lldb-commits]

mehdi_amini added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

mehdi_amini wrote:
> I am a bit unsure that it is desirable that it is desirable to need this 
> change?
> This requires every single user of LLVM to do this. Is there a way this can 
> be done in the call to `find_package(LLVM)` instead?
(Doc for the usual way to integrate LLVM: 
https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via lldb-commits]

phosek marked 4 inline comments as done.
phosek added inline comments.



Comment at: mlir/examples/standalone/CMakeLists.txt:35
+endif()
+
 include(TableGen)

stephenneuendorffer wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > I am a bit unsure that it is desirable that it is desirable to need this 
> > > change?
> > > This requires every single user of LLVM to do this. Is there a way this 
> > > can be done in the call to `find_package(LLVM)` instead?
> > (Doc for the usual way to integrate LLVM: 
> > https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project )
> I tend to agree with Mehdi.  I think stuff like this which is *required* to 
> use LLVM should get pushed into what LLVM exports.  (Actually there is a 
> bunch of other configuration stuff which also needs to get pushed into the 
> export, otherwise configuration defaults are all wrong and things like that.)
Implemented in D84691.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [lldb] ef748b5 - [lldb] NFC: Use early exit in ArchSpec::IsEqualTo

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-07-27T14:12:02-07:00
New Revision: ef748b58d3b3edfaf0278d454cb30f7816c04aee

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

LOG: [lldb] NFC: Use early exit in ArchSpec::IsEqualTo

Added: 


Modified: 
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index a77ae8633070..cd382a322da7 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1010,77 +1010,70 @@ static bool 
IsCompatibleEnvironment(llvm::Triple::EnvironmentType lhs,
 bool ArchSpec::IsEqualTo(const ArchSpec , bool exact_match) const {
   // explicitly ignoring m_distribution_id in this method.
 
-  if (GetByteOrder() != rhs.GetByteOrder())
+  if (GetByteOrder() != rhs.GetByteOrder() ||
+  !cores_match(GetCore(), rhs.GetCore(), true, exact_match))
 return false;
 
-  const ArchSpec::Core lhs_core = GetCore();
-  const ArchSpec::Core rhs_core = rhs.GetCore();
+  const llvm::Triple _triple = GetTriple();
+  const llvm::Triple _triple = rhs.GetTriple();
+
+  const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
+  const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
+  if (lhs_triple_vendor != rhs_triple_vendor) {
+const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
+const bool lhs_vendor_specified = TripleVendorWasSpecified();
+// Both architectures had the vendor specified, so if they aren't equal
+// then we return false
+if (rhs_vendor_specified && lhs_vendor_specified)
+  return false;
 
-  const bool core_match = cores_match(lhs_core, rhs_core, true, exact_match);
-
-  if (core_match) {
-const llvm::Triple _triple = GetTriple();
-const llvm::Triple _triple = rhs.GetTriple();
-
-const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
-const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-if (lhs_triple_vendor != rhs_triple_vendor) {
-  const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
-  const bool lhs_vendor_specified = TripleVendorWasSpecified();
-  // Both architectures had the vendor specified, so if they aren't equal
-  // then we return false
-  if (rhs_vendor_specified && lhs_vendor_specified)
-return false;
-
-  // Only fail if both vendor types are not unknown
-  if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
-  rhs_triple_vendor != llvm::Triple::UnknownVendor)
-return false;
-}
+// Only fail if both vendor types are not unknown
+if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
+rhs_triple_vendor != llvm::Triple::UnknownVendor)
+  return false;
+  }
 
-const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
-const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
-const llvm::Triple::EnvironmentType lhs_triple_env =
+  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
+  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+  const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
-const llvm::Triple::EnvironmentType rhs_triple_env =
+  const llvm::Triple::EnvironmentType rhs_triple_env =
   rhs_triple.getEnvironment();
 
-if (!exact_match) {
-  // x86_64-apple-ios-macabi, x86_64-apple-macosx are compatible, no match.
-  if ((lhs_triple_os == llvm::Triple::IOS &&
-   lhs_triple_env == llvm::Triple::MacABI &&
-   rhs_triple_os == llvm::Triple::MacOSX) ||
-  (lhs_triple_os == llvm::Triple::MacOSX &&
-   rhs_triple_os == llvm::Triple::IOS &&
-   rhs_triple_env == llvm::Triple::MacABI))
-return true;
-}
-
-if (lhs_triple_os != rhs_triple_os) {
-  const bool rhs_os_specified = rhs.TripleOSWasSpecified();
-  const bool lhs_os_specified = TripleOSWasSpecified();
-  // Both architectures had the OS specified, so if they aren't equal then
-  // we return false
-  if (rhs_os_specified && lhs_os_specified)
-return false;
-
-  // Only fail if both os types are not unknown
-  if (lhs_triple_os != llvm::Triple::UnknownOS &&
-  rhs_triple_os != llvm::Triple::UnknownOS)
-return false;
-}
+  if (!exact_match) {
+// x86_64-apple-ios-macabi, x86_64-apple-macosx are compatible, no match.
+if ((lhs_triple_os == llvm::Triple::IOS &&
+ lhs_triple_env == llvm::Triple::MacABI &&
+ rhs_triple_os == llvm::Triple::MacOSX) ||
+(lhs_triple_os == llvm::Triple::MacOSX &&
+ rhs_triple_os == llvm::Triple::IOS &&
+ rhs_triple_env == llvm::Triple::MacABI))
+  return true;
+  }
 
-

[Lldb-commits] [lldb] 113f56f - Unify the return value of GetByteSize to an llvm::Optional (NFC-ish)

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-07-27T13:26:35-07:00
New Revision: 113f56fbb80e8d6f705be19f8ae169a3fee2e4f8

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

LOG: Unify the return value of GetByteSize to an llvm::Optional 
(NFC-ish)

This cleanup patch unifies all methods called GetByteSize() in the
ValueObject hierarchy to return an optional, like the methods in
CompilerType do. This means fewer magic 0 values, which could fix bugs
down the road in languages where types can have a size of zero, such
as Swift and C (but not C++).

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

This re-lands the patch with bogus :m_byte_size(0) initalizations removed.

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectCast.h
lldb/include/lldb/Core/ValueObjectChild.h
lldb/include/lldb/Core/ValueObjectConstResult.h
lldb/include/lldb/Core/ValueObjectDynamicValue.h
lldb/include/lldb/Core/ValueObjectMemory.h
lldb/include/lldb/Core/ValueObjectRegister.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/include/lldb/Core/ValueObjectVariable.h
lldb/include/lldb/Expression/ExpressionVariable.h
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/source/API/SBValue.cpp
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectCast.cpp
lldb/source/Core/ValueObjectConstResult.cpp
lldb/source/Core/ValueObjectDynamicValue.cpp
lldb/source/Core/ValueObjectMemory.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Expression/ExpressionVariable.cpp
lldb/source/Expression/Materializer.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 0080368fd996..a557d69f3ae3 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -358,7 +358,7 @@ class ValueObject : public UserID {
   virtual bool CanProvideValue();
 
   // Subclasses must implement the functions below.
-  virtual uint64_t GetByteSize() = 0;
+  virtual llvm::Optional GetByteSize() = 0;
 
   virtual lldb::ValueType GetValueType() const = 0;
 

diff  --git a/lldb/include/lldb/Core/ValueObjectCast.h 
b/lldb/include/lldb/Core/ValueObjectCast.h
index d91ca6a92be8..342803f8ca63 100644
--- a/lldb/include/lldb/Core/ValueObjectCast.h
+++ b/lldb/include/lldb/Core/ValueObjectCast.h
@@ -30,7 +30,7 @@ class ValueObjectCast : public ValueObject {
 ConstString name,
 const CompilerType _type);
 
-  uint64_t GetByteSize() override;
+  llvm::Optional GetByteSize() override;
 
   size_t CalculateNumChildren(uint32_t max) override;
 

diff  --git a/lldb/include/lldb/Core/ValueObjectChild.h 
b/lldb/include/lldb/Core/ValueObjectChild.h
index c6f44a29b059..9a9fd9294261 100644
--- a/lldb/include/lldb/Core/ValueObjectChild.h
+++ b/lldb/include/lldb/Core/ValueObjectChild.h
@@ -30,7 +30,7 @@ class ValueObjectChild : public ValueObject {
 public:
   ~ValueObjectChild() override;
 
-  uint64_t GetByteSize() override { return m_byte_size; }
+  llvm::Optional GetByteSize() override { return m_byte_size; }
 
   lldb::offset_t GetByteOffset() override { return m_byte_offset; }
 

diff  --git a/lldb/include/lldb/Core/ValueObjectConstResult.h 
b/lldb/include/lldb/Core/ValueObjectConstResult.h
index 0e868c687e93..8d823baa0b7b 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResult.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -62,7 +62,7 @@ class ValueObjectConstResult : public ValueObject {
   static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
 const Status );
 
-  uint64_t GetByteSize() override;
+  llvm::Optional GetByteSize() override;
 
   lldb::ValueType GetValueType() const override;
 
@@ -113,7 +113,7 @@ class ValueObjectConstResult : public ValueObject {
   CompilerType GetCompilerTypeImpl() override;
 
   ConstString m_type_name;
-  uint64_t m_byte_size;
+  llvm::Optional m_byte_size;
 
   ValueObjectConstResultImpl m_impl;
 

diff  --git a/lldb/include/lldb/Core/ValueObjectDynamicValue.h 
b/lldb/include/lldb/Core/ValueObjectDynamicValue.h
index 9f5304b55e93..2806857339ef 100644
--- a/lldb/include/lldb/Core/ValueObjectDynamicValue.h
+++ b/lldb/include/lldb/Core/ValueObjectDynamicValue.h
@@ -34,7 +34,7 @@ class ValueObjectDynamicValue : public ValueObject {
 public:
   ~ValueObjectDynamicValue() override;
 
-  uint64_t GetByteSize() override;
+  llvm::Optional GetByteSize() override;
 
   ConstString GetTypeName() 

[Lldb-commits] [PATCH] D84402: [lldb/DWARF] Add more line table validation

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I really like the idea of coming up with a low PC that is the first valid .text 
address. This will filter out many of the zeroed out dead stripped DWARF and 
will make a cheap way for us to check addresses when parsing debug info and 
line tables. Checking for -1 and -2 of course is great to do as well.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1044
   for (const llvm::DWARFDebugLine::Sequence  : line_table->Sequences) {
+if (!list.ContainsFileAddressRange(seq.LowPC, seq.HighPC - seq.LowPC))
+  continue;

labath wrote:
> MaskRay wrote:
> > clayborg wrote:
> > > labath wrote:
> > > > clayborg wrote:
> > > > > dblaikie wrote:
> > > > > > Could you specifically look for/propagate tombstones here, to 
> > > > > > reduce the risk of large functions overlapping with the valid 
> > > > > > address range, even when they're tombstoned to zero? (because 
> > > > > > zero+large function size could still end up overlapping with valid 
> > > > > > code)
> > > > > > 
> > > > > > To do that well, I guess it'd have to be implemented at a 
> > > > > > lower-layer, inside the line table state machine - essentially 
> > > > > > dropping all lines derived from a "set address" operation that 
> > > > > > specifies a tombstone.
> > > > > Just checking if the section lists contains an address doesn't help 
> > > > > weed out addresses that were tombstoned to zero since our PT_LOAD[0] 
> > > > > will almost always contain zero for shared libraries. It might be 
> > > > > nice to make a list of addresses that come from sections with read + 
> > > > > execute permissions and store that in SymbolFileDWARF one time at 
> > > > > startup. Then these searches will be faster as we are looking in less 
> > > > > ranges, and most likely will not contain address zero. This code will 
> > > > > catch the -1 and -2 tombstones, but most linkers I have run into use 
> > > > > zero and the tombstone. 
> > > > > 
> > > > > If our algorithm only checks sections with no subsections and then 
> > > > > makes a list of file addresses for and section ranges for those, we 
> > > > > should have a great list. The entire PT_LOAD[0] will usually be 
> > > > > mapped read + execute, so we can't just check top level sections for 
> > > > > ELF. Mach-o also has this issue __TEXT in mac is also mapped read + 
> > > > > execute and usually contains zero for shared libraries, but since the 
> > > > > sections must come after the mach-o header, the sections within the 
> > > > > __TEXT segment have correct permissions and would work, just like 
> > > > > they would for ELF.
> > > > You're right -- this would not handle shared libraries with base zero.
> > > > 
> > > > I am slightly uneasy about requiring executable permissions for all 
> > > > line tables. While it does not seem terribly useful to have line tables 
> > > > for non-executable code, if someone does have a line table for it for 
> > > > whatever reason (maybe he wants to make it executable at runtime?) it 
> > > > would be a shame not to display it. Also the choice of using section 
> > > > rather than segment permissions feels slightly arbitrary (although I 
> > > > could make a case for it), as it's the segment permissions which will 
> > > > actually define the runtime memory permissions.
> > > > 
> > > > Since this is really about filtering out (near) zero addresses, how 
> > > > about we make that explicit? Find the lowest executable (section) 
> > > > address and reject anything below that? Additionally, I'd also reject 
> > > > all addresses which are completely outside of the module range, as 
> > > > those not going to get used for anything, and they are generating bogus 
> > > > line-table dumps.
> > > > 
> > > > What do you think?
> > > > 
> > > > David: The -1 tombstones are already sort of handled in llvm (and in 
> > > > lldb since D83957). They are "handled" in the sense that the all 
> > > > sequences with and end PC lower than the start PC are rejected (and 
> > > > line sequences starting with (unsigned)-1 will definitely wrap). This 
> > > > is trying to do something about the zero tombstones.
> > > > Since this is really about filtering out (near) zero addresses, how 
> > > > about we make that explicit? Find the lowest executable (section) 
> > > > address and reject anything below that? Additionally, I'd also reject 
> > > > all addresses which are completely outside of the module range, as 
> > > > those not going to get used for anything, and they are generating bogus 
> > > > line-table dumps.
> > > > 
> > > > What do you think?
> > > 
> > > That will work for me. My main goal is to get anything that should have 
> > > been dead stripped out from appearing in results for line lookups or 
> > > function lookups. The quicker we can short circuit these cases the better 
> > > for performance. We can also use this when we try to lookup functions and 
> > > don't return any 

[Lldb-commits] [lldb] 4c6eebf - [lldb/AppleSimulator] Always provide a -simulator environment

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-07-27T12:50:50-07:00
New Revision: 4c6eebf86a0734779cd20473cfcaa9d7c8899298

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

LOG: [lldb/AppleSimulator] Always provide a -simulator environment

Summary:
This commit is somewhat NFC-ish today as the environment of triples
is not considered when comparing s if one of them is
not set (I plan to change that).

We have made simulator triples unambiguous these days, but the
simulator platforms still advertise triples without the
environment. This wasn't an issue when the sims ran only on
a very different architecure than the real device, but this
has changed with Apple Silicon.

This patch simplifies the way GetSupportedArchitectureAtIndex
is implemented for the sim platforms and adds the environment.
It also trivially adds support for Apple Silicon to those
platforms.

Reviewers: aprantl

Subscribers: lldb-commits

Added: 
lldb/unittests/Platform/PlatformAppleSimulatorTest.cpp

Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.h
lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.h
lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.h
lldb/unittests/Platform/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index bd0a231303bd..0160fb95c58a 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -253,3 +253,11 @@ CoreSimulatorSupport::Device 
PlatformAppleSimulator::GetSimulatorDevice() {
 return CoreSimulatorSupport::Device();
 }
 #endif
+
+bool PlatformAppleSimulator::GetSupportedArchitectureAtIndex(uint32_t idx,
+ ArchSpec ) {
+  if (idx >= m_supported_triples.size())
+return false;
+  arch = ArchSpec(m_supported_triples[idx]);
+  return true;
+}

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
index 8c0174f2946e..6182acaf229a 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
@@ -44,6 +44,9 @@ class PlatformAppleSimulator : public PlatformDarwin {
lldb_private::Target *target,
lldb_private::Status ) override;
 
+  bool GetSupportedArchitectureAtIndex(uint32_t idx,
+   lldb_private::ArchSpec ) override;
+
 protected:
   std::mutex m_core_sim_path_mutex;
   llvm::Optional m_core_simulator_framework_path;
@@ -52,6 +55,9 @@ class PlatformAppleSimulator : public PlatformDarwin {
 
   lldb_private::FileSpec GetCoreSimulatorPath();
 
+  llvm::Triple::OSType m_os_type = llvm::Triple::UnknownOS;
+  llvm::ArrayRef m_supported_triples = {};
+
   void LoadCoreSimulator();
 
 #if defined(__APPLE__)

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
index 461624a2adaa..27f798b00ebf 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
@@ -77,6 +77,7 @@ PlatformSP PlatformAppleTVSimulator::CreateInstance(bool 
force,
   bool create = force;
   if (!create && arch && arch->IsValid()) {
 switch (arch->GetMachine()) {
+case llvm::Triple::aarch64:
 case llvm::Triple::x86_64: {
   const llvm::Triple  = arch->GetTriple();
   switch (triple.getVendor()) {
@@ -144,7 +145,24 @@ const char 
*PlatformAppleTVSimulator::GetDescriptionStatic() {
 /// Default Constructor
 PlatformAppleTVSimulator::PlatformAppleTVSimulator()
 : PlatformAppleSimulator(
-  CoreSimulatorSupport::DeviceType::ProductFamilyID::appleTV) {}
+  CoreSimulatorSupport::DeviceType::ProductFamilyID::appleTV) {
+#ifdef __APPLE__
+#if __arm64__
+  static const llvm::StringRef supported_triples[] = {
+  "arm64e-apple-tvos-simulator",
+  "arm64-apple-tvos-simulator",
+  "x86_64h-apple-tvos-simulator",
+  "x86_64-apple-tvos-simulator",
+  };
+#else
+  static const llvm::StringRef supported_triples[] = {
+  "x86_64h-apple-tvos-simulator",
+  "x86_64-apple-tvos-simulator",
+  };

[Lldb-commits] [lldb] 536baa1 - [lldb] Remove CMAKE_VERSION checks now that the minimum version is 3.13.4

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-07-27T12:31:41-07:00
New Revision: 536baa11cfe12362ea646ad731a2274a07208cc0

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

LOG: [lldb] Remove CMAKE_VERSION checks now that the minimum version is 3.13.4

Added: 


Modified: 
lldb/cmake/modules/FindPythonInterpAndLibs.cmake
lldb/cmake/modules/LLDBConfig.cmake

Removed: 




diff  --git a/lldb/cmake/modules/FindPythonInterpAndLibs.cmake 
b/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
index 243e0463f48b..3a64ebbcf972 100644
--- a/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
+++ b/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
@@ -61,46 +61,22 @@ if(PYTHON_LIBRARIES AND PYTHON_INCLUDE_DIRS AND 
PYTHON_EXECUTABLE AND SWIG_EXECU
 else()
   find_package(SWIG 2.0)
   if (SWIG_FOUND)
-if(NOT CMAKE_VERSION VERSION_LESS 3.12)
-  if (LLDB_PYTHON_VERSION)
-if (LLDB_PYTHON_VERSION VERSION_EQUAL "2")
-  FindPython2()
-elseif(LLDB_PYTHON_VERSION VERSION_EQUAL "3")
-  FindPython3()
-endif()
-  else()
+if (LLDB_PYTHON_VERSION)
+  if (LLDB_PYTHON_VERSION VERSION_EQUAL "2")
+FindPython2()
+  elseif(LLDB_PYTHON_VERSION VERSION_EQUAL "3")
 FindPython3()
-if (NOT PYTHON3_FOUND AND NOT CMAKE_SYSTEM_NAME STREQUAL Windows)
-  FindPython2()
-endif()
   endif()
 else()
-  find_package(PythonInterp)
-  find_package(PythonLibs)
-  if(PYTHONINTERP_FOUND AND PYTHONLIBS_FOUND AND SWIG_FOUND)
-if (NOT CMAKE_CROSSCOMPILING)
-  string(REPLACE "." ";" pythonlibs_version_list 
${PYTHONLIBS_VERSION_STRING})
-  list(GET pythonlibs_version_list 0 pythonlibs_major)
-  list(GET pythonlibs_version_list 1 pythonlibs_minor)
-
-  # Ignore the patch version. Some versions of macOS report a 
diff erent
-  # patch version for the system provided interpreter and libraries.
-  if (CMAKE_CROSSCOMPILING OR (PYTHON_VERSION_MAJOR VERSION_EQUAL 
pythonlibs_major AND
-  PYTHON_VERSION_MINOR VERSION_EQUAL pythonlibs_minor))
-mark_as_advanced(
-  PYTHON_LIBRARIES
-  PYTHON_INCLUDE_DIRS
-  PYTHON_EXECUTABLE
-  SWIG_EXECUTABLE)
-  endif()
-endif()
+  FindPython3()
+  if (NOT PYTHON3_FOUND AND NOT CMAKE_SYSTEM_NAME STREQUAL Windows)
+FindPython2()
   endif()
 endif()
   else()
 message(STATUS "SWIG 2 or later is required for Python support in LLDB but 
could not be found")
   endif()
 
-
   include(FindPackageHandleStandardArgs)
   find_package_handle_standard_args(PythonInterpAndLibs
 FOUND_VAR

diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index 8465cfe3b7b7..7e5848c800f8 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -79,11 +79,6 @@ if(LLDB_BUILD_FRAMEWORK)
   if(NOT APPLE)
 message(FATAL_ERROR "LLDB.framework can only be generated when targeting 
Apple platforms")
   endif()
-  # CMake 3.6 did not correctly emit POST_BUILD commands for Apple Framework 
targets
-  # CMake < 3.8 did not have the BUILD_RPATH target property
-  if(CMAKE_VERSION VERSION_LESS 3.8)
-message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.8")
-  endif()
 
   set(LLDB_FRAMEWORK_VERSION A CACHE STRING "LLDB.framework version (default 
is A)")
   set(LLDB_FRAMEWORK_BUILD_DIR bin CACHE STRING "Output directory for 
LLDB.framework")



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

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Shu Anzai via Phabricator via lldb-commits]

gedatsu217 updated this revision to Diff 281017.
gedatsu217 added a comment.

Revise Editline::ApplyAutosuggestCommand. (Change the return value.)

Revise IOHandlerEditline::IOHandlerEditline (Disable to use autosuggestion if 
debugger.GetUseColor() is false.)

Fix the test.


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

https://reviews.llvm.org/D81001

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/IOHandler.h
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Host/common/Editline.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py

Index: lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
===
--- /dev/null
+++ lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
@@ -0,0 +1,83 @@
+"""
+Tests autosuggestion using pexpect.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestCase(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfEditlineSupportMissing
+def test_autosuggestion(self):
+self.launch(extra_args=["-o", "settings set show-autosuggestion true", "-o", "settings set use-color true"])
+
+# Common input codes and escape sequences. 
+ctrl_f = "\x06"
+faint_color = "\x1b[2m"
+reset = "\x1b[0m"
+delete = chr(127)
+
+frame_output_needle = "Syntax: frame "  
+# Run 'help frame' once to put it into the command history. 
+self.expect("help frame", substrs=[frame_output_needle])
+
+# Check that LLDB shows the autosuggestion in gray behind the text. 
+self.child.send("hel")
+self.child.expect_exact("\x1b["+ str(len("(lldb) he") + 1) + "G" + "l" + faint_color + "p frame" + reset)
+
+# Apply the autosuggestion and press enter. This should print the
+# 'help frame' output if everything went correctly.
+self.child.send(ctrl_f + "\n")
+self.child.expect_exact(frame_output_needle)
+
+# Check that pressing Ctrl+F directly after Ctrl+F again does nothing.
+self.child.send("hel" + ctrl_f + ctrl_f + "\n")
+self.child.expect_exact(frame_output_needle)
+
+# Try autosuggestion using tab and ^f. 
+# \t makes "help" and ^f makes "help frame". If everything went 
+# correct we should see the 'help frame' output again.
+self.child.send("hel\t" + ctrl_f + "\n")
+self.child.expect_exact(frame_output_needle)
+
+# Check that autosuggestion works after delete.
+self.child.send("a1234" + 5 * delete + "hel" + ctrl_f + "\n")
+self.child.expect_exact(frame_output_needle)
+
+# Check that autosuggestion works after delete.
+self.child.send("help x" + delete + ctrl_f + "\n")
+self.child.expect_exact(frame_output_needle)
+
+# Check that autosuggestion complete to the most recent one.
+self.child.send("help frame variable\n")
+self.child.send("help fr")
+self.child.expect_exact(faint_color + "ame variable" + reset)
+self.child.send("\n")
+
+# Try another command.
+apropos_output_needle = "Syntax: apropos "
+# Run 'help frame' once to put it into the command history.
+self.expect("help apropos", substrs=[apropos_output_needle])
+
+# Check that 'hel' should have an autosuggestion for 'help apropos' now.   
+self.child.send("hel")
+self.child.expect_exact("\x1b["+ str(len("(lldb) he") + 1) + "G" + "l" + faint_color + "p apropos" + reset)
+
+# Run the command and expect the 'help apropos' output.   
+self.child.send(ctrl_f + "\n")
+self.child.expect_exact(apropos_output_needle)
+
+# Check that pressing Ctrl+F in an empty prompt does nothing. 
+breakpoint_output_needle = "Syntax: breakpoint "
+self.child.send(ctrl_f + "help breakpoint" +"\n")
+self.child.expect_exact(breakpoint_output_needle)
+
+self.quit()
+
\ No newline at end of file
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1866,6 +1866,19 @@
   HandleCompletionMatches(request);
 }
 

Re: [Lldb-commits] [lldb] 1d9b860 - Unify the return value of GetByteSize to an llvm::Optional (NFC-ish)

[Eric Christopher via lldb-commits]

Thanks Pavel :)

On Mon, Jul 27, 2020, 11:35 AM Pavel Labath  wrote:

> On 27/07/2020 19:03, Adrian Prantl via lldb-commits wrote:
> > Do you happen to have a link to a bot with the failures? Or can you send
> > me the output in case it's an internal bot?
> >
> > thanks,
> > adrian
> >
>
> This should do it:
> http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/14598
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D81001#2176076 , @gedatsu217 wrote:

> > Could you create a patch to change the definition of ANSI_UNFAINT ? (might 
> > be worth taking a quick look at git history if there is no good reason for 
> > why it uses the color code that it uses)
>
> Sure. Should I create another patch? (In short, should I create it separately 
> from the current patch?)


Yes, please.


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

https://reviews.llvm.org/D81001



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

Re: [Lldb-commits] [lldb] 1d9b860 - Unify the return value of GetByteSize to an llvm::Optional (NFC-ish)

[Pavel Labath via lldb-commits]

On 27/07/2020 19:03, Adrian Prantl via lldb-commits wrote:
> Do you happen to have a link to a bot with the failures? Or can you send
> me the output in case it's an internal bot?
> 
> thanks,
> adrian
> 

This should do it:
http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/14598
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] 1d9b860 - Unify the return value of GetByteSize to an llvm::Optional (NFC-ish)

[Adrian Prantl via lldb-commits]

Do you happen to have a link to a bot with the failures? Or can you send me the 
output in case it's an internal bot?

thanks,
adrian

> On Jul 27, 2020, at 9:54 AM, Eric Christopher  wrote:
> 
> No problem, thanks! :)
> 
> On Mon, Jul 27, 2020, 9:26 AM Adrian Prantl  > wrote:
> Thanks, Eric! Sorry for not paying attention after landing this.
> 
> -- adrian
> 
>> On Jul 25, 2020, at 6:43 PM, Eric Christopher > > wrote:
>> 
>> Hi Adrian,
>> 
>> I'm really sorry, but I've just reverted this. I'm not sure what's up, but 
>> it's causing massive test failures in lldb on linux. Happy to help sync up 
>> with you.
>> 
>> echristo@athyra ~/s/llvm-project> git push
>> To github.com :llvm/llvm-project.git
>>18975762c19..4b14ef33e81  master -> master
>> 
>> Thanks!
>> 
>> -eric
>> 
>> On Sat, Jul 25, 2020 at 8:28 AM Adrian Prantl via lldb-commits 
>> mailto:lldb-commits@lists.llvm.org>> wrote:
>> 
>> Author: Adrian Prantl
>> Date: 2020-07-25T08:27:21-07:00
>> New Revision: 1d9b860fb6a85df33fd52fcacc6a5efb421621bd
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/1d9b860fb6a85df33fd52fcacc6a5efb421621bd
>>  
>> 
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/1d9b860fb6a85df33fd52fcacc6a5efb421621bd.diff
>>  
>> 
>> 
>> LOG: Unify the return value of GetByteSize to an llvm::Optional 
>> (NFC-ish)
>> 
>> This cleanup patch unifies all methods called GetByteSize() in the
>> ValueObject hierarchy to return an optional, like the methods in
>> CompilerType do. This means fewer magic 0 values, which could fix bugs
>> down the road in languages where types can have a size of zero, such
>> as Swift and C (but not C++).
>> 
>> Differential Revision: https://reviews.llvm.org/D84285 
>> 
>> 
>> Added: 
>> 
>> 
>> Modified: 
>> lldb/include/lldb/Core/ValueObject.h
>> lldb/include/lldb/Core/ValueObjectCast.h
>> lldb/include/lldb/Core/ValueObjectChild.h
>> lldb/include/lldb/Core/ValueObjectConstResult.h
>> lldb/include/lldb/Core/ValueObjectDynamicValue.h
>> lldb/include/lldb/Core/ValueObjectMemory.h
>> lldb/include/lldb/Core/ValueObjectRegister.h
>> lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
>> lldb/include/lldb/Core/ValueObjectVariable.h
>> lldb/include/lldb/Expression/ExpressionVariable.h
>> lldb/include/lldb/Target/StackFrameRecognizer.h
>> lldb/source/API/SBValue.cpp
>> lldb/source/Commands/CommandObjectWatchpoint.cpp
>> lldb/source/Core/ValueObject.cpp
>> lldb/source/Core/ValueObjectCast.cpp
>> lldb/source/Core/ValueObjectConstResult.cpp
>> lldb/source/Core/ValueObjectDynamicValue.cpp
>> lldb/source/Core/ValueObjectMemory.cpp
>> lldb/source/Core/ValueObjectRegister.cpp
>> lldb/source/Core/ValueObjectSyntheticFilter.cpp
>> lldb/source/Core/ValueObjectVariable.cpp
>> lldb/source/Expression/ExpressionVariable.cpp
>> lldb/source/Expression/Materializer.cpp
>> lldb/source/Target/StackFrame.cpp
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/include/lldb/Core/ValueObject.h 
>> b/lldb/include/lldb/Core/ValueObject.h
>> index 0080368fd996..a557d69f3ae3 100644
>> --- a/lldb/include/lldb/Core/ValueObject.h
>> +++ b/lldb/include/lldb/Core/ValueObject.h
>> @@ -358,7 +358,7 @@ class ValueObject : public UserID {
>>virtual bool CanProvideValue();
>> 
>>// Subclasses must implement the functions below.
>> -  virtual uint64_t GetByteSize() = 0;
>> +  virtual llvm::Optional GetByteSize() = 0;
>> 
>>virtual lldb::ValueType GetValueType() const = 0;
>> 
>> 
>> diff  --git a/lldb/include/lldb/Core/ValueObjectCast.h 
>> b/lldb/include/lldb/Core/ValueObjectCast.h
>> index d91ca6a92be8..342803f8ca63 100644
>> --- a/lldb/include/lldb/Core/ValueObjectCast.h
>> +++ b/lldb/include/lldb/Core/ValueObjectCast.h
>> @@ -30,7 +30,7 @@ class ValueObjectCast : public ValueObject {
>>  ConstString name,
>>  const CompilerType _type);
>> 
>> -  uint64_t GetByteSize() override;
>> +  llvm::Optional GetByteSize() override;
>> 
>>size_t CalculateNumChildren(uint32_t max) override;
>> 
>> 
>> diff  --git a/lldb/include/lldb/Core/ValueObjectChild.h 
>> b/lldb/include/lldb/Core/ValueObjectChild.h
>> index c6f44a29b059..9a9fd9294261 100644
>> --- a/lldb/include/lldb/Core/ValueObjectChild.h
>> +++ b/lldb/include/lldb/Core/ValueObjectChild.h
>> @@ -30,7 +30,7 @@ class ValueObjectChild : public ValueObject {
>>  public:
>>~ValueObjectChild() override;
>> 
>> -  uint64_t GetByteSize() override { return m_byte_size; }
>> +  llvm::Optional GetByteSize() override { return m_byte_size; }
>> 
>>

Re: [Lldb-commits] [lldb] 1d9b860 - Unify the return value of GetByteSize to an llvm::Optional (NFC-ish)

[Eric Christopher via lldb-commits]

No problem, thanks! :)

On Mon, Jul 27, 2020, 9:26 AM Adrian Prantl  wrote:

> Thanks, Eric! Sorry for not paying attention after landing this.
>
> -- adrian
>
> On Jul 25, 2020, at 6:43 PM, Eric Christopher  wrote:
>
> Hi Adrian,
>
> I'm really sorry, but I've just reverted this. I'm not sure what's up, but
> it's causing massive test failures in lldb on linux. Happy to help sync up
> with you.
>
> echristo@athyra ~/s/llvm-project> git push
> To github.com:llvm/llvm-project.git
>18975762c19..4b14ef33e81  master -> master
>
> Thanks!
>
> -eric
>
> On Sat, Jul 25, 2020 at 8:28 AM Adrian Prantl via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>>
>> Author: Adrian Prantl
>> Date: 2020-07-25T08:27:21-07:00
>> New Revision: 1d9b860fb6a85df33fd52fcacc6a5efb421621bd
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/1d9b860fb6a85df33fd52fcacc6a5efb421621bd
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/1d9b860fb6a85df33fd52fcacc6a5efb421621bd.diff
>>
>> LOG: Unify the return value of GetByteSize to an llvm::Optional
>> (NFC-ish)
>>
>> This cleanup patch unifies all methods called GetByteSize() in the
>> ValueObject hierarchy to return an optional, like the methods in
>> CompilerType do. This means fewer magic 0 values, which could fix bugs
>> down the road in languages where types can have a size of zero, such
>> as Swift and C (but not C++).
>>
>> Differential Revision: https://reviews.llvm.org/D84285
>>
>> Added:
>>
>>
>> Modified:
>> lldb/include/lldb/Core/ValueObject.h
>> lldb/include/lldb/Core/ValueObjectCast.h
>> lldb/include/lldb/Core/ValueObjectChild.h
>> lldb/include/lldb/Core/ValueObjectConstResult.h
>> lldb/include/lldb/Core/ValueObjectDynamicValue.h
>> lldb/include/lldb/Core/ValueObjectMemory.h
>> lldb/include/lldb/Core/ValueObjectRegister.h
>> lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
>> lldb/include/lldb/Core/ValueObjectVariable.h
>> lldb/include/lldb/Expression/ExpressionVariable.h
>> lldb/include/lldb/Target/StackFrameRecognizer.h
>> lldb/source/API/SBValue.cpp
>> lldb/source/Commands/CommandObjectWatchpoint.cpp
>> lldb/source/Core/ValueObject.cpp
>> lldb/source/Core/ValueObjectCast.cpp
>> lldb/source/Core/ValueObjectConstResult.cpp
>> lldb/source/Core/ValueObjectDynamicValue.cpp
>> lldb/source/Core/ValueObjectMemory.cpp
>> lldb/source/Core/ValueObjectRegister.cpp
>> lldb/source/Core/ValueObjectSyntheticFilter.cpp
>> lldb/source/Core/ValueObjectVariable.cpp
>> lldb/source/Expression/ExpressionVariable.cpp
>> lldb/source/Expression/Materializer.cpp
>> lldb/source/Target/StackFrame.cpp
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/lldb/include/lldb/Core/ValueObject.h
>> b/lldb/include/lldb/Core/ValueObject.h
>> index 0080368fd996..a557d69f3ae3 100644
>> --- a/lldb/include/lldb/Core/ValueObject.h
>> +++ b/lldb/include/lldb/Core/ValueObject.h
>> @@ -358,7 +358,7 @@ class ValueObject : public UserID {
>>virtual bool CanProvideValue();
>>
>>// Subclasses must implement the functions below.
>> -  virtual uint64_t GetByteSize() = 0;
>> +  virtual llvm::Optional GetByteSize() = 0;
>>
>>virtual lldb::ValueType GetValueType() const = 0;
>>
>>
>> diff  --git a/lldb/include/lldb/Core/ValueObjectCast.h
>> b/lldb/include/lldb/Core/ValueObjectCast.h
>> index d91ca6a92be8..342803f8ca63 100644
>> --- a/lldb/include/lldb/Core/ValueObjectCast.h
>> +++ b/lldb/include/lldb/Core/ValueObjectCast.h
>> @@ -30,7 +30,7 @@ class ValueObjectCast : public ValueObject {
>>  ConstString name,
>>  const CompilerType _type);
>>
>> -  uint64_t GetByteSize() override;
>> +  llvm::Optional GetByteSize() override;
>>
>>size_t CalculateNumChildren(uint32_t max) override;
>>
>>
>> diff  --git a/lldb/include/lldb/Core/ValueObjectChild.h
>> b/lldb/include/lldb/Core/ValueObjectChild.h
>> index c6f44a29b059..9a9fd9294261 100644
>> --- a/lldb/include/lldb/Core/ValueObjectChild.h
>> +++ b/lldb/include/lldb/Core/ValueObjectChild.h
>> @@ -30,7 +30,7 @@ class ValueObjectChild : public ValueObject {
>>  public:
>>~ValueObjectChild() override;
>>
>> -  uint64_t GetByteSize() override { return m_byte_size; }
>> +  llvm::Optional GetByteSize() override { return m_byte_size; }
>>
>>lldb::offset_t GetByteOffset() override { return m_byte_offset; }
>>
>>
>> diff  --git a/lldb/include/lldb/Core/ValueObjectConstResult.h
>> b/lldb/include/lldb/Core/ValueObjectConstResult.h
>> index 0e868c687e93..8d823baa0b7b 100644
>> --- a/lldb/include/lldb/Core/ValueObjectConstResult.h
>> +++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
>> @@ -62,7 +62,7 @@ class ValueObjectConstResult : public ValueObject {
>>static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
>>  const Status );
>>
>> -  uint64_t 

Re: [Lldb-commits] [lldb] 1d9b860 - Unify the return value of GetByteSize to an llvm::Optional (NFC-ish)

[Adrian Prantl via lldb-commits]

Thanks, Eric! Sorry for not paying attention after landing this.

-- adrian

> On Jul 25, 2020, at 6:43 PM, Eric Christopher  wrote:
> 
> Hi Adrian,
> 
> I'm really sorry, but I've just reverted this. I'm not sure what's up, but 
> it's causing massive test failures in lldb on linux. Happy to help sync up 
> with you.
> 
> echristo@athyra ~/s/llvm-project> git push
> To github.com:llvm/llvm-project.git
>18975762c19..4b14ef33e81  master -> master
> 
> Thanks!
> 
> -eric
> 
> On Sat, Jul 25, 2020 at 8:28 AM Adrian Prantl via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> 
> Author: Adrian Prantl
> Date: 2020-07-25T08:27:21-07:00
> New Revision: 1d9b860fb6a85df33fd52fcacc6a5efb421621bd
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/1d9b860fb6a85df33fd52fcacc6a5efb421621bd
>  
> 
> DIFF: 
> https://github.com/llvm/llvm-project/commit/1d9b860fb6a85df33fd52fcacc6a5efb421621bd.diff
>  
> 
> 
> LOG: Unify the return value of GetByteSize to an llvm::Optional 
> (NFC-ish)
> 
> This cleanup patch unifies all methods called GetByteSize() in the
> ValueObject hierarchy to return an optional, like the methods in
> CompilerType do. This means fewer magic 0 values, which could fix bugs
> down the road in languages where types can have a size of zero, such
> as Swift and C (but not C++).
> 
> Differential Revision: https://reviews.llvm.org/D84285 
> 
> 
> Added: 
> 
> 
> Modified: 
> lldb/include/lldb/Core/ValueObject.h
> lldb/include/lldb/Core/ValueObjectCast.h
> lldb/include/lldb/Core/ValueObjectChild.h
> lldb/include/lldb/Core/ValueObjectConstResult.h
> lldb/include/lldb/Core/ValueObjectDynamicValue.h
> lldb/include/lldb/Core/ValueObjectMemory.h
> lldb/include/lldb/Core/ValueObjectRegister.h
> lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
> lldb/include/lldb/Core/ValueObjectVariable.h
> lldb/include/lldb/Expression/ExpressionVariable.h
> lldb/include/lldb/Target/StackFrameRecognizer.h
> lldb/source/API/SBValue.cpp
> lldb/source/Commands/CommandObjectWatchpoint.cpp
> lldb/source/Core/ValueObject.cpp
> lldb/source/Core/ValueObjectCast.cpp
> lldb/source/Core/ValueObjectConstResult.cpp
> lldb/source/Core/ValueObjectDynamicValue.cpp
> lldb/source/Core/ValueObjectMemory.cpp
> lldb/source/Core/ValueObjectRegister.cpp
> lldb/source/Core/ValueObjectSyntheticFilter.cpp
> lldb/source/Core/ValueObjectVariable.cpp
> lldb/source/Expression/ExpressionVariable.cpp
> lldb/source/Expression/Materializer.cpp
> lldb/source/Target/StackFrame.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/include/lldb/Core/ValueObject.h 
> b/lldb/include/lldb/Core/ValueObject.h
> index 0080368fd996..a557d69f3ae3 100644
> --- a/lldb/include/lldb/Core/ValueObject.h
> +++ b/lldb/include/lldb/Core/ValueObject.h
> @@ -358,7 +358,7 @@ class ValueObject : public UserID {
>virtual bool CanProvideValue();
> 
>// Subclasses must implement the functions below.
> -  virtual uint64_t GetByteSize() = 0;
> +  virtual llvm::Optional GetByteSize() = 0;
> 
>virtual lldb::ValueType GetValueType() const = 0;
> 
> 
> diff  --git a/lldb/include/lldb/Core/ValueObjectCast.h 
> b/lldb/include/lldb/Core/ValueObjectCast.h
> index d91ca6a92be8..342803f8ca63 100644
> --- a/lldb/include/lldb/Core/ValueObjectCast.h
> +++ b/lldb/include/lldb/Core/ValueObjectCast.h
> @@ -30,7 +30,7 @@ class ValueObjectCast : public ValueObject {
>  ConstString name,
>  const CompilerType _type);
> 
> -  uint64_t GetByteSize() override;
> +  llvm::Optional GetByteSize() override;
> 
>size_t CalculateNumChildren(uint32_t max) override;
> 
> 
> diff  --git a/lldb/include/lldb/Core/ValueObjectChild.h 
> b/lldb/include/lldb/Core/ValueObjectChild.h
> index c6f44a29b059..9a9fd9294261 100644
> --- a/lldb/include/lldb/Core/ValueObjectChild.h
> +++ b/lldb/include/lldb/Core/ValueObjectChild.h
> @@ -30,7 +30,7 @@ class ValueObjectChild : public ValueObject {
>  public:
>~ValueObjectChild() override;
> 
> -  uint64_t GetByteSize() override { return m_byte_size; }
> +  llvm::Optional GetByteSize() override { return m_byte_size; }
> 
>lldb::offset_t GetByteOffset() override { return m_byte_offset; }
> 
> 
> diff  --git a/lldb/include/lldb/Core/ValueObjectConstResult.h 
> b/lldb/include/lldb/Core/ValueObjectConstResult.h
> index 0e868c687e93..8d823baa0b7b 100644
> --- a/lldb/include/lldb/Core/ValueObjectConstResult.h
> +++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
> @@ -62,7 +62,7 @@ class ValueObjectConstResult : public ValueObject {
>static lldb::ValueObjectSP Create(ExecutionContextScope 

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Shu Anzai via Phabricator via lldb-commits]

gedatsu217 added a comment.

> Could you create a patch to change the definition of ANSI_UNFAINT ? (might be 
> worth taking a quick look at git history if there is no good reason for why 
> it uses the color code that it uses)

Sure. Should I create another patch? (In short, should I create it separately 
from the current patch?)


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D83767: [lldb] Use the basename of the Python test for the log name instead of the class name

[Raphael Isemann via Phabricator via lldb-commits]

teemperor planned changes to this revision.
teemperor added a comment.

Got reverted, so back to my TODO queue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83767



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

[Lldb-commits] [PATCH] D84528: [lldb][NFC] Use a StringRef for AddRegexCommand::AddRegexCommand parameters

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG432241955e03: [lldb][NFC] Use a StringRef for 
AddRegexCommand::AddRegexCommand parameters (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84528

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


Index: lldb/source/Interpreter/CommandObjectRegexCommand.cpp
===
--- lldb/source/Interpreter/CommandObjectRegexCommand.cpp
+++ lldb/source/Interpreter/CommandObjectRegexCommand.cpp
@@ -69,14 +69,13 @@
   return false;
 }
 
-bool CommandObjectRegexCommand::AddRegexCommand(const char *re_cstr,
-const char *command_cstr) {
+bool CommandObjectRegexCommand::AddRegexCommand(llvm::StringRef re_cstr,
+llvm::StringRef command_cstr) {
   m_entries.resize(m_entries.size() + 1);
   // Only add the regular expression if it compiles
-  m_entries.back().regex =
-  RegularExpression(llvm::StringRef::withNullAsEmpty(re_cstr));
+  m_entries.back().regex = RegularExpression(re_cstr);
   if (m_entries.back().regex.IsValid()) {
-m_entries.back().command.assign(command_cstr);
+m_entries.back().command = command_cstr.str();
 return true;
   }
   // The regex didn't compile...
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -631,15 +631,10 @@
   if (tbreak_regex_cmd_up) {
 bool success = true;
 for (size_t i = 0; i < num_regexes; i++) {
-  // If you add a resultant command string longer than 1024 characters be
-  // sure to increase the size of this buffer.
-  char buffer[1024];
-  int num_printed =
-  snprintf(buffer, 1024, "%s %s", break_regexes[i][1], "-o 1");
-  lldbassert(num_printed < 1024);
-  UNUSED_IF_ASSERT_DISABLED(num_printed);
+  std::string command = break_regexes[i][1];
+  command += " -o 1";
   success =
-  tbreak_regex_cmd_up->AddRegexCommand(break_regexes[i][0], buffer);
+  tbreak_regex_cmd_up->AddRegexCommand(break_regexes[i][0], command);
   if (!success)
 break;
 }
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -970,7 +970,7 @@
   std::string subst(std::string(regex_sed.substr(
   second_separator_char_pos + 1,
   third_separator_char_pos - second_separator_char_pos - 1)));
-  m_regex_cmd_up->AddRegexCommand(regex.c_str(), subst.c_str());
+  m_regex_cmd_up->AddRegexCommand(regex, subst);
 }
 return error;
   }
Index: lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
===
--- lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
+++ lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
@@ -30,7 +30,7 @@
 
   bool IsRemovable() const override { return m_is_removable; }
 
-  bool AddRegexCommand(const char *re_cstr, const char *command_cstr);
+  bool AddRegexCommand(llvm::StringRef re_cstr, llvm::StringRef command_cstr);
 
   bool HasRegexEntries() const { return !m_entries.empty(); }
 


Index: lldb/source/Interpreter/CommandObjectRegexCommand.cpp
===
--- lldb/source/Interpreter/CommandObjectRegexCommand.cpp
+++ lldb/source/Interpreter/CommandObjectRegexCommand.cpp
@@ -69,14 +69,13 @@
   return false;
 }
 
-bool CommandObjectRegexCommand::AddRegexCommand(const char *re_cstr,
-const char *command_cstr) {
+bool CommandObjectRegexCommand::AddRegexCommand(llvm::StringRef re_cstr,
+llvm::StringRef command_cstr) {
   m_entries.resize(m_entries.size() + 1);
   // Only add the regular expression if it compiles
-  m_entries.back().regex =
-  RegularExpression(llvm::StringRef::withNullAsEmpty(re_cstr));
+  m_entries.back().regex = RegularExpression(re_cstr);
   if (m_entries.back().regex.IsValid()) {
-m_entries.back().command.assign(command_cstr);
+m_entries.back().command = command_cstr.str();
 return true;
   }
   // The regex didn't compile...
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- 

[Lldb-commits] [lldb] 4322419 - [lldb][NFC] Use a StringRef for AddRegexCommand::AddRegexCommand parameters

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-27T14:36:47+02:00
New Revision: 432241955e032fba3d8b584ee6388212909bee9b

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

LOG: [lldb][NFC] Use a StringRef for AddRegexCommand::AddRegexCommand parameters

Summary: This way we can get rid of this 1024 char buffer workaround.

Reviewers: #lldb, labath

Reviewed By: labath

Subscribers: JDevlieghere

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/CommandObjectRegexCommand.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h 
b/lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
index 01d7c6d118d4..cbd50511c483 100644
--- a/lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
+++ b/lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
@@ -30,7 +30,7 @@ class CommandObjectRegexCommand : public CommandObjectRaw {
 
   bool IsRemovable() const override { return m_is_removable; }
 
-  bool AddRegexCommand(const char *re_cstr, const char *command_cstr);
+  bool AddRegexCommand(llvm::StringRef re_cstr, llvm::StringRef command_cstr);
 
   bool HasRegexEntries() const { return !m_entries.empty(); }
 

diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index 255fbe53fb2e..eaf22344fafa 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -970,7 +970,7 @@ a number follows 'f':"
   std::string subst(std::string(regex_sed.substr(
   second_separator_char_pos + 1,
   third_separator_char_pos - second_separator_char_pos - 1)));
-  m_regex_cmd_up->AddRegexCommand(regex.c_str(), subst.c_str());
+  m_regex_cmd_up->AddRegexCommand(regex, subst);
 }
 return error;
   }

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index aca3654b0309..4786e4602e4b 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -631,15 +631,10 @@ void CommandInterpreter::LoadCommandDictionary() {
   if (tbreak_regex_cmd_up) {
 bool success = true;
 for (size_t i = 0; i < num_regexes; i++) {
-  // If you add a resultant command string longer than 1024 characters be
-  // sure to increase the size of this buffer.
-  char buffer[1024];
-  int num_printed =
-  snprintf(buffer, 1024, "%s %s", break_regexes[i][1], "-o 1");
-  lldbassert(num_printed < 1024);
-  UNUSED_IF_ASSERT_DISABLED(num_printed);
+  std::string command = break_regexes[i][1];
+  command += " -o 1";
   success =
-  tbreak_regex_cmd_up->AddRegexCommand(break_regexes[i][0], buffer);
+  tbreak_regex_cmd_up->AddRegexCommand(break_regexes[i][0], command);
   if (!success)
 break;
 }

diff  --git a/lldb/source/Interpreter/CommandObjectRegexCommand.cpp 
b/lldb/source/Interpreter/CommandObjectRegexCommand.cpp
index 5a0265e58c5c..7485fd76cc25 100644
--- a/lldb/source/Interpreter/CommandObjectRegexCommand.cpp
+++ b/lldb/source/Interpreter/CommandObjectRegexCommand.cpp
@@ -69,14 +69,13 @@ bool CommandObjectRegexCommand::DoExecute(llvm::StringRef 
command,
   return false;
 }
 
-bool CommandObjectRegexCommand::AddRegexCommand(const char *re_cstr,
-const char *command_cstr) {
+bool CommandObjectRegexCommand::AddRegexCommand(llvm::StringRef re_cstr,
+llvm::StringRef command_cstr) {
   m_entries.resize(m_entries.size() + 1);
   // Only add the regular expression if it compiles
-  m_entries.back().regex =
-  RegularExpression(llvm::StringRef::withNullAsEmpty(re_cstr));
+  m_entries.back().regex = RegularExpression(re_cstr);
   if (m_entries.back().regex.IsValid()) {
-m_entries.back().command.assign(command_cstr);
+m_entries.back().command = command_cstr.str();
 return true;
   }
   // The regex didn't compile...



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

[Lldb-commits] [lldb] db203e0 - [lldb] Modernize away some snprintf calls

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-27T14:27:54+02:00
New Revision: db203e0268479d16d36a318c726cce5a4a5f75a6

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

LOG: [lldb] Modernize away some snprintf calls

Reviewers: #lldb, JDevlieghere

Reviewed By: #lldb, JDevlieghere

Subscribers: JDevlieghere

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Core/Communication.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index f86779d85b5f..fd8d38e85637 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -48,19 +48,19 @@ class CommandObjectProcessLaunchOrAttach : public 
CommandObjectParsed {
   state = process->GetState();
 
   if (process->IsAlive() && state != eStateConnected) {
-char message[1024];
+std::string message;
 if (process->GetState() == eStateAttaching)
-  ::snprintf(message, sizeof(message),
- "There is a pending attach, abort it and %s?",
- m_new_process_action.c_str());
+  message =
+  llvm::formatv("There is a pending attach, abort it and {0}?",
+m_new_process_action);
 else if (process->GetShouldDetach())
-  ::snprintf(message, sizeof(message),
- "There is a running process, detach from it and %s?",
- m_new_process_action.c_str());
+  message = llvm::formatv(
+  "There is a running process, detach from it and {0}?",
+  m_new_process_action);
 else
-  ::snprintf(message, sizeof(message),
- "There is a running process, kill it and %s?",
- m_new_process_action.c_str());
+  message =
+  llvm::formatv("There is a running process, kill it and {0}?",
+m_new_process_action);
 
 if (!m_interpreter.Confirm(message, true)) {
   result.SetStatus(eReturnStatusFailed);

diff  --git a/lldb/source/Core/Communication.cpp 
b/lldb/source/Core/Communication.cpp
index b358e70b1a91..859f5be74b43 100644
--- a/lldb/source/Core/Communication.cpp
+++ b/lldb/source/Core/Communication.cpp
@@ -199,9 +199,8 @@ bool Communication::StartReadThread(Status *error_ptr) {
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
"{0} Communication::StartReadThread ()", this);
 
-  char thread_name[1024];
-  snprintf(thread_name, sizeof(thread_name), "",
-   GetBroadcasterName().AsCString());
+  const std::string thread_name =
+  llvm::formatv("", GetBroadcasterName());
 
   m_read_thread_enabled = true;
   m_read_thread_did_exit = false;

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 5f4f1e266d81..05cfac19915e 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -666,9 +666,7 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   m_event_handler_thread(), m_io_handler_thread(),
   m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
   m_forward_listener_sp(), m_clear_once() {
-  char instance_cstr[256];
-  snprintf(instance_cstr, sizeof(instance_cstr), "debugger_%d", (int)GetID());
-  m_instance_name.SetCString(instance_cstr);
+  m_instance_name.SetString(llvm::formatv("debugger_{0}", GetID()).str());
   if (log_callback)
 m_log_callback_stream_sp =
 std::make_shared(log_callback, baton);

diff  --git a/lldb/source/Core/SourceManager.cpp 
b/lldb/source/Core/SourceManager.cpp
index 7414dd281d43..e79fcb48742d 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -183,14 +183,14 @@ size_t 
SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
 break;
   }
 
-  char prefix[32] = "";
+  std::string prefix;
   if (bp_locs) {
 uint32_t bp_count = bp_locs->NumLineEntriesWithLine(line);
 
 if (bp_count > 0)
-  ::snprintf(prefix, sizeof(prefix), "[%u] ", bp_count);
+  prefix = llvm::formatv("[{0}]", bp_count);
 else
-  ::snprintf(prefix, sizeof(prefix), "");
+  prefix = "";
   }
 
   char buffer[3];
@@ -206,7 +206,8 @@ size_t 
SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
 .str());
   }
 
-  s->Printf("%s%s %-4u\t", 

[Lldb-commits] [PATCH] D84530: [lldb] Modernize away some snprintf calls

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdb203e026847: [lldb] Modernize away some snprintf calls 
(authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84530

Files:
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Core/Communication.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectChild.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1987,10 +1987,7 @@
 if (value_type != OptionParser::eOptionalArgument)
   new_args.AppendArgument(value);
 else {
-  char buffer[255];
-  ::snprintf(buffer, sizeof(buffer), "%s%s", option.c_str(),
- value.c_str());
-  new_args.AppendArgument(llvm::StringRef(buffer));
+  new_args.AppendArgument(option + value);
 }
 
   } else if (static_cast(index) >= cmd_args.GetArgumentCount()) {
@@ -2012,10 +2009,7 @@
 if (value_type != OptionParser::eOptionalArgument)
   new_args.AppendArgument(cmd_args.GetArgumentAtIndex(index));
 else {
-  char buffer[255];
-  ::snprintf(buffer, sizeof(buffer), "%s%s", option.c_str(),
- cmd_args.GetArgumentAtIndex(index));
-  new_args.AppendArgument(buffer);
+  new_args.AppendArgument(option + cmd_args.GetArgumentAtIndex(index));
 }
 used[index] = true;
   }
Index: lldb/source/Core/ValueObjectChild.cpp
===
--- lldb/source/Core/ValueObjectChild.cpp
+++ lldb/source/Core/ValueObjectChild.cpp
@@ -57,15 +57,8 @@
 
 static void AdjustForBitfieldness(ConstString ,
   uint8_t bitfield_bit_size) {
-  if (name && bitfield_bit_size) {
-const char *compiler_type_name = name.AsCString();
-if (compiler_type_name) {
-  std::vector bitfield_type_name(strlen(compiler_type_name) + 32, 0);
-  ::snprintf(_type_name.front(), bitfield_type_name.size(),
- "%s:%u", compiler_type_name, bitfield_bit_size);
-  name.SetCString(_type_name.front());
-}
-  }
+  if (name && bitfield_bit_size)
+name.SetString(llvm::formatv("{0}:{1}", name, bitfield_bit_size).str());
 }
 
 ConstString ValueObjectChild::GetTypeName() {
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1702,8 +1702,7 @@
bool can_create) {
   ValueObjectSP synthetic_child_sp;
   if (IsPointerType() || IsArrayType()) {
-char index_str[64];
-snprintf(index_str, sizeof(index_str), "[%" PRIu64 "]", (uint64_t)index);
+std::string index_str = llvm::formatv("[{0}]", index);
 ConstString index_const_str(index_str);
 // Check if we have already created a synthetic array member in this valid
 // object. If we have we will re-use it.
@@ -1730,8 +1729,7 @@
  bool can_create) {
   ValueObjectSP synthetic_child_sp;
   if (IsScalarType()) {
-char index_str[64];
-snprintf(index_str, sizeof(index_str), "[%i-%i]", from, to);
+std::string index_str = llvm::formatv("[{0}-{1}]", from, to);
 ConstString index_const_str(index_str);
 // Check if we have already created a synthetic array member in this valid
 // object. If we have we will re-use it.
@@ -1768,9 +1766,7 @@
   ValueObjectSP synthetic_child_sp;
 
   if (name_const_str.IsEmpty()) {
-char name_str[64];
-snprintf(name_str, sizeof(name_str), "@%i", offset);
-name_const_str.SetCString(name_str);
+name_const_str.SetString("@" + std::to_string(offset));
   }
 
   // Check if we have already created a synthetic array member in this valid
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -183,14 +183,14 @@
 break;
   }
 
-  char prefix[32] = "";
+  std::string prefix;
   if (bp_locs) {
 uint32_t bp_count = bp_locs->NumLineEntriesWithLine(line);
 
 if (bp_count > 0)
-  ::snprintf(prefix, sizeof(prefix), "[%u] ", bp_count);
+  prefix = llvm::formatv("[{0}]", bp_count);
 else
-  ::snprintf(prefix, sizeof(prefix), "");
+  prefix = "";
   }
 
   char buffer[3];
@@ -206,7 +206,8 @@
 

[Lldb-commits] [PATCH] D84402: [lldb/DWARF] Add more line table validation

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1044
   for (const llvm::DWARFDebugLine::Sequence  : line_table->Sequences) {
+if (!list.ContainsFileAddressRange(seq.LowPC, seq.HighPC - seq.LowPC))
+  continue;

MaskRay wrote:
> clayborg wrote:
> > labath wrote:
> > > clayborg wrote:
> > > > dblaikie wrote:
> > > > > Could you specifically look for/propagate tombstones here, to reduce 
> > > > > the risk of large functions overlapping with the valid address range, 
> > > > > even when they're tombstoned to zero? (because zero+large function 
> > > > > size could still end up overlapping with valid code)
> > > > > 
> > > > > To do that well, I guess it'd have to be implemented at a 
> > > > > lower-layer, inside the line table state machine - essentially 
> > > > > dropping all lines derived from a "set address" operation that 
> > > > > specifies a tombstone.
> > > > Just checking if the section lists contains an address doesn't help 
> > > > weed out addresses that were tombstoned to zero since our PT_LOAD[0] 
> > > > will almost always contain zero for shared libraries. It might be nice 
> > > > to make a list of addresses that come from sections with read + execute 
> > > > permissions and store that in SymbolFileDWARF one time at startup. Then 
> > > > these searches will be faster as we are looking in less ranges, and 
> > > > most likely will not contain address zero. This code will catch the -1 
> > > > and -2 tombstones, but most linkers I have run into use zero and the 
> > > > tombstone. 
> > > > 
> > > > If our algorithm only checks sections with no subsections and then 
> > > > makes a list of file addresses for and section ranges for those, we 
> > > > should have a great list. The entire PT_LOAD[0] will usually be mapped 
> > > > read + execute, so we can't just check top level sections for ELF. 
> > > > Mach-o also has this issue __TEXT in mac is also mapped read + execute 
> > > > and usually contains zero for shared libraries, but since the sections 
> > > > must come after the mach-o header, the sections within the __TEXT 
> > > > segment have correct permissions and would work, just like they would 
> > > > for ELF.
> > > You're right -- this would not handle shared libraries with base zero.
> > > 
> > > I am slightly uneasy about requiring executable permissions for all line 
> > > tables. While it does not seem terribly useful to have line tables for 
> > > non-executable code, if someone does have a line table for it for 
> > > whatever reason (maybe he wants to make it executable at runtime?) it 
> > > would be a shame not to display it. Also the choice of using section 
> > > rather than segment permissions feels slightly arbitrary (although I 
> > > could make a case for it), as it's the segment permissions which will 
> > > actually define the runtime memory permissions.
> > > 
> > > Since this is really about filtering out (near) zero addresses, how about 
> > > we make that explicit? Find the lowest executable (section) address and 
> > > reject anything below that? Additionally, I'd also reject all addresses 
> > > which are completely outside of the module range, as those not going to 
> > > get used for anything, and they are generating bogus line-table dumps.
> > > 
> > > What do you think?
> > > 
> > > David: The -1 tombstones are already sort of handled in llvm (and in lldb 
> > > since D83957). They are "handled" in the sense that the all sequences 
> > > with and end PC lower than the start PC are rejected (and line sequences 
> > > starting with (unsigned)-1 will definitely wrap). This is trying to do 
> > > something about the zero tombstones.
> > > Since this is really about filtering out (near) zero addresses, how about 
> > > we make that explicit? Find the lowest executable (section) address and 
> > > reject anything below that? Additionally, I'd also reject all addresses 
> > > which are completely outside of the module range, as those not going to 
> > > get used for anything, and they are generating bogus line-table dumps.
> > > 
> > > What do you think?
> > 
> > That will work for me. My main goal is to get anything that should have 
> > been dead stripped out from appearing in results for line lookups or 
> > function lookups. The quicker we can short circuit these cases the better 
> > for performance. We can also use this when we try to lookup functions and 
> > don't return any matches for functions whose start address falls below this 
> > value.
> > 
> > 
> There is a concern about ContainsFileAddressRange's performance.
> 
> FindSectionContainingFileAddress iterates all sections and can be slow when 
> the number of sections is large.
> 
> >  if someone does have a line table for it for whatever reason (maybe he 
> > wants to make it executable at runtime?) it would be a shame not to display 
> > it.
> 
> +1
> 
> 

[Lldb-commits] [PATCH] D83302: [lldb/DWARF] Don't treat class declarations with children as definitions

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1956cf1042d3: [lldb/DWARF] Dont treat class 
declarations with children as definitions (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D83302?vs=276048=280847#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83302

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
  lldb/test/API/functionalities/limit-debug-info/main.cpp
  lldb/test/API/functionalities/limit-debug-info/one.cpp
  lldb/test/API/functionalities/limit-debug-info/onetwo.h
  lldb/test/API/functionalities/limit-debug-info/two.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
@@ -0,0 +1,160 @@
+# Test that a forward-declared (DW_AT_declaration) structure is treated as a
+# forward-declaration even if it has children. These types can be produced due
+# to vtable-based type homing, or other -flimit-debug-info optimizations.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj > %t
+# RUN: %lldb %t -o "expr a" -o exit 2>&1 | FileCheck %s --check-prefix=EXPR
+# RUN: %lldb %t -o "target var a" -o exit 2>&1 | FileCheck %s --check-prefix=VAR
+
+# EXPR: incomplete type 'A' where a complete type is required
+
+# FIXME: This should also produce some kind of an error.
+# VAR: (A) a = {}
+
+.text
+_ZN1AC2Ev:
+retq
+.LZN1AC2Ev_end:
+
+.data
+a:
+.quad   $_ZTV1A+16
+.quad   $0xdeadbeef
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   52  # DW_TAG_variable
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   2   # DW_AT_location
+.byte   24  # DW_FORM_exprloc
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   3   # Abbreviation Code
+.byte   19  # DW_TAG_structure_type
+.byte   1   # DW_CHILDREN_yes
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   60  # DW_AT_declaration
+.byte   25  # DW_FORM_flag_present
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   4   # Abbreviation Code
+.byte   46  # DW_TAG_subprogram
+.byte   1   # DW_CHILDREN_yes
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   60  # DW_AT_declaration
+.byte   25  # DW_FORM_flag_present
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   5   # Abbreviation Code
+.byte   5   # DW_TAG_formal_parameter
+.byte   0   # DW_CHILDREN_no
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   52  # DW_AT_artificial
+.byte   25  # DW_FORM_flag_present

[Lldb-commits] [lldb] 1956cf1 - [lldb/DWARF] Don't treat class declarations with children as definitions

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-27T12:58:22+02:00
New Revision: 1956cf1042d3c406d9e9cefe47d3b43adf2bdbe1

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

LOG: [lldb/DWARF] Don't treat class declarations with children as definitions

Summary:
This effectively reverts r188124, which added code to handle
(DW_AT_)declarations of structures with some kinds of children as
definitions. The commit message claims this is a workaround for some
kind of debug info produced by gcc. However, it does not go into
specifics, so it's hard to reproduce or verify that this is indeed still a
problem.

Having this code is definitely a problem though, because it mistakenly
declares incomplete dwarf declarations to be complete. Both clang (with
-flimit-debug-info) and gcc (by default) generate DW_AT_declarations of
structs with children. This happens when full debug info for a class is
not emitted in a given compile unit (e.g. because of vtable homing), but
the class has inline methods which are used in the given compile unit.
In that case, the compilers emit a DW_AT_declaration of a class, but
add a DW_TAG_subprogram child to it to describe the inlined instance of
the method.

Even though the class tag has some children, it definitely does not
contain enough information to construct a full class definition (most
notably, it lacks any members). Keeping the class as incomplete allows
us to search for a real definition in other modules, helping the
-flimit-debug-info flow. And in case the definition is not found we can
display a error message saying that, instead of just showing an empty
struct.

Reviewers: clayborg, aprantl, JDevlieghere, shafik

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
lldb/test/API/functionalities/limit-debug-info/main.cpp
lldb/test/API/functionalities/limit-debug-info/one.cpp
lldb/test/API/functionalities/limit-debug-info/onetwo.h
lldb/test/API/functionalities/limit-debug-info/two.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 35e7c34734e2..7e3628504727 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1641,33 +1641,6 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
   dwarf->GetUniqueDWARFASTTypeMap().Insert(unique_typename,
*unique_ast_entry_up);
 
-  if (attrs.is_forward_declaration && die.HasChildren()) {
-// Check to see if the DIE actually has a definition, some version of
-// GCC will
-// emit DIEs with DW_AT_declaration set to true, but yet still have
-// subprogram, members, or inheritance, so we can't trust it
-DWARFDIE child_die = die.GetFirstChild();
-while (child_die) {
-  switch (child_die.Tag()) {
-  case DW_TAG_inheritance:
-  case DW_TAG_subprogram:
-  case DW_TAG_member:
-  case DW_TAG_APPLE_property:
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_enumeration_type:
-  case DW_TAG_typedef:
-  case DW_TAG_union_type:
-child_die.Clear();
-attrs.is_forward_declaration = false;
-break;
-  default:
-child_die = child_die.GetSibling();
-break;
-  }
-}
-  }
-
   if (!attrs.is_forward_declaration) {
 // Always start the definition for a class type so that if the class
 // has child classes or types that require the class to be created

diff  --git 
a/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py 
b/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
index 9408ad6eee1d..aa383d0005e4 100644
--- a/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
+++ b/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
@@ -38,7 +38,8 @@ def test_one_and_two_debug(self):
 
 self._check_debug_info_is_limited(target)
 
-self.registerSharedLibrariesWithTarget(target, ["one", "two"])
+lldbutil.run_to_name_breakpoint(self, "main",
+extra_images=["one", "two"])
 
 # But when other shared libraries are loaded, we should be able to see
 # all members.
@@ -58,6 +59,10 @@ def test_one_and_two_debug(self):
 self.expect_expr("array_of_two[2].one[2].member", result_value="174")
 self.expect_expr("array_of_two[2].member", result_value="274")
 
+

[Lldb-commits] [lldb] 2e828e7 - [lldb] Fix e89414f406 for msvc

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-27T11:49:46+02:00
New Revision: 2e828e7579928e8cc1c5e53c84ab99ffb5afca03

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

LOG: [lldb] Fix e89414f406 for msvc

MSVC finds the APInt construction ambiguous. Use a case to help it
choose the right constructor.

Added: 


Modified: 
lldb/include/lldb/Utility/Scalar.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/Scalar.h 
b/lldb/include/lldb/Utility/Scalar.h
index 1dbcf80bfd89..45ba7c012229 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -62,18 +62,23 @@ class Scalar {
   // Constructors and Destructors
   Scalar() : m_type(e_void), m_float(0.0f) {}
   Scalar(int v)
-  : m_type(e_sint), m_integer(sizeof(v) * 8, v, true), m_float(0.0f) {}
+  : m_type(e_sint), m_integer(sizeof(v) * 8, uint64_t(v), true),
+m_float(0.0f) {}
   Scalar(unsigned int v)
-  : m_type(e_uint), m_integer(sizeof(v) * 8, v, false), m_float(0.0f) {}
+  : m_type(e_uint), m_integer(sizeof(v) * 8, uint64_t(v), false),
+m_float(0.0f) {}
   Scalar(long v)
-  : m_type(e_slong), m_integer(sizeof(v) * 8, v, true), m_float(0.0f) {}
+  : m_type(e_slong), m_integer(sizeof(v) * 8, uint64_t(v), true),
+m_float(0.0f) {}
   Scalar(unsigned long v)
-  : m_type(e_ulong), m_integer(sizeof(v) * 8, v, false), m_float(0.0f) {}
+  : m_type(e_ulong), m_integer(sizeof(v) * 8, uint64_t(v), false),
+m_float(0.0f) {}
   Scalar(long long v)
-  : m_type(e_slonglong), m_integer(sizeof(v) * 8, v, true), m_float(0.0f) 
{}
+  : m_type(e_slonglong), m_integer(sizeof(v) * 8, uint64_t(v), true),
+m_float(0.0f) {}
   Scalar(unsigned long long v)
-  : m_type(e_ulonglong), m_integer(sizeof(v) * 8, v, false), m_float(0.0f) 
{
-  }
+  : m_type(e_ulonglong), m_integer(sizeof(v) * 8, uint64_t(v), false),
+m_float(0.0f) {}
   Scalar(float v) : m_type(e_float), m_float(v) {}
   Scalar(double v) : m_type(e_double), m_float(v) {}
   Scalar(long double v) : m_type(e_long_double), m_float(double(v)) {



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

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D81001#2172658 , @gedatsu217 wrote:

> I do not intend for this feature to work with colors disabled.


Right in that case, we should probably disable the feature if colors are 
disabled.

> I found that pexpect output the below sequence, and this passed the test.
> 
>   self.child.expect_exact("\x1b[" + str(len("(lldb) he") + 1) + "G" + "l" + 
> "\x1b[2m" + "p frame" + "\x1b[0m\x1b[1G" + "l" + "\x1b[1G\x1b[2m" + "(lldb) " 
> + "\x1b[22m\x1b[" + str(len("(lldb) hel") + 1) + "G")
> 
> 
> Probably, "(lldb)" is redisplayed every time a character is typed. On the 
> other hand, the character is placed in the designated position.
> 
> However, there are two strange points.
> 
> 1. When a character is typed, it is placed in the designated position, but 
> later, it is placed again in column one and overwritten by "(lldb)".

Yes, that is true. This is a problem and the prompt drawing is only covering it 
up.

That said, now that I understand this code better, I believe this is actually a 
bug on our part that we can fix. In your `TypedCharacter` function you call 
`MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingPrompt);` after 
printing the suggested text. This is the thing which moves the cursor to the 
beginning of the line, and judging by the editline behavior (printing the 
character at the start of line), this is not what it expects. It seems like the 
right solution here would be to move the cursor to be just before the character 
that was added. `CursorLocation::EditingCursor` moves it just *after* it, so it 
seems we may need a new constant (`BeforeEditingCursor`) to perform the desired 
move.

Alternatively, it may also be possible to move to 
`CursorLocation::EditingCursor` with a combination of `return CC_NORM`. That 
seemed to work fine in my experiments except for introducing some artefacts 
when backspacing. It's possible this is another bug that could also be fixed, 
but I did not look into it.

> 2. About "\x1b[22m". I think this is equal to "\x1b[0m", but the test failed 
> when I replace "\x1b[22m" with "\x1b[0m".

The test checks for string equivalence, not functional equivalence. That string 
is coming from Editline::GetCharacter (ANSI_UNFAINT). That said, since both of 
these functions are doing the same thing (restoring normal formatting after 
displaying some custom stuff) it would make sense for them to do it the same 
way. `0m` seems to be more explicit so maybe we should standardize on that? 
Could you create a patch to change the definition of ANSI_UNFAINT ? (might be 
worth taking a quick look at git history if there is no good reason for why it 
uses the color code that it uses)

Also, given the presence of ANSI_(UN)FAINT, I'm not sure what to make of the 
usage of `FormatAnsiTerminalCodes` in this patch. This file is already 
intimately familiar with ansi escape sequences needed move cursors and stuff, 
so I'm not sure that going through that function actually buys us anything.

> Do you think this is a valid test?

With the above caveats, yes. There's also one additional aspect -- even after 
the above is addressed, we may still need to allow for some variance in the 
sequence to allow for different libedit versions -- as our test have shown, 
these can differ sometime.


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D79699: Add ptrace register access for AArch64 SVE registers

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h:22
+#define INCLUDE_LINUX_PTRACE_DEFINITIONS_FOR_SVE_ARM64
+#include "Plugins/Process/Utility/LinuxPTraceDefines_arm64sve.h"
+#endif

omjavaid wrote:
> labath wrote:
> > omjavaid wrote:
> > > omjavaid wrote:
> > > > labath wrote:
> > > > > At this point, that header is sufficiently different from the 
> > > > > asm/ptrace.h implementation (names of all structs and fields are 
> > > > > different) that I don't think it makes sense to use it as an optional 
> > > > > replacement. It makes it very likely things will not build in one of 
> > > > > the modes.
> > > > > 
> > > > > This should either use the system version (and then only build when 
> > > > > the system has the appropriate headers), or we should use the 
> > > > > in-house version unconditionally (which may require 
> > > > > renaming/namespacing the rest of the file so that it does not 
> > > > > conflict with the real `asm/ptrace.h`).
> > > > Ack.
> > > asm/ptrace.h also describes legacy structs for fpsimd access, hardware 
> > > breakpoint structs and other related ptrace access. Newer versions of 
> > > ptrace only append SVE specific structs and macros. In house LLDB's SVE 
> > > macros file only include SVE specific register names. I dont see any 
> > > conflict since I have also added a guard for multiple definitions in 
> > > LinuxPTraceDefines_arm64sve.h as it protects against builds of 
> > > AArch64/Linux where ThreadElfCore also includes sigcontext and ptrace.h 
> > > for accessing elf-core related struct definitions.
> > I'm not saying we shouldn't include asm/ptrace.h at all. It obviously 
> > contains a lot of useful stuff, and we wouldn't want to duplicate all of 
> > that.
> > 
> > What I am saying is that for the stuff that we already have duplicated, we 
> > just be using that unconditionally. Doing otherwise just needlessly forks 
> > the build matrix and creates opportunities for things to not work in one 
> > mode or the other. We're already committed to ensuring things work with the 
> > in-house definitions so why bothering to ensure that things work both with 
> > them _and_ with the system ones.
> Right now SVE in-house macros and the ones in asm/ptrace.h are identical. If 
> we include asm/ptrace.h and it already has SVE macros we cannot avoid that 
> inclusion so in case we want to force use of in-house macros we ll have to 
> rename the in-house macros and headers with LLVM_ or LLDB_ prefix to make 
> sure we avoid duplication. If you have strong opinion in favor of renaming i 
> can go ahead and do that.
I do, actually. Given that this code is also used for the core files, which can 
be used from any host, and the fact that we have already started making changes 
to it because of that (`_uNN` vs `uintNN_t`, the msvc compatibility patch, 
etc), I think we just just treating this as a part of our codebase and not some 
asm/ptrace addon.

I wouldn't tack an additional LLDB_ onto the names though. What I'd do is 
replace all of these macros with regular C(++) symbols. Parameter-less macros 
can be constants and function-like macros can just be functions. They can keep 
the original all-caps spelling to make it clear where they are coming from, but 
in order to distinguish them from the system symbols we can make a tiny 
modification -- instead of naming everyting `SVE_FOO`, we name the thing just 
`FOO`, but put it inside a `lldb_private::SVE` namespace -- that way the 
symbols can be referred to as `SVE::FOO`.



Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c:2-4
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+  return 0; // Set a break point here.

omjavaid wrote:
> labath wrote:
> > omjavaid wrote:
> > > labath wrote:
> > > > In our newer register tests we use the pattern where the debugger sets 
> > > > the register values, which are then read back through the inferior (and 
> > > > vice versa). I thing this is a good thing as it avoids the possibility 
> > > > of making symmetrical bugs in the read/write code, which will then 
> > > > appear to modify the register successfully, but they won't actually 
> > > > change the value observed by the inferior process.
> > > In this test we are also doing the same please have a look at 
> > > TestSVERegisters.py:140-142.
> > > 
> > > The code in main.c is used to make sure we enable SVE mode. AArch64 SVE 
> > > mode is enabled by Linux kernel on first execution of SVE code.
> > I have a feeling we're misunderstanding each other. I am talking about the 
> > tests in `test/Shell/Register` (e.g. `x86-64-xmm16-read.test`, 
> > `x86-64-xmm16-write.test`). This test definitely doesn't to that. The code 
> > in 140-142 is just reading back what was written by lines 135-137, not 
> > values set by the 

[Lldb-commits] [PATCH] D84623: Remove HAVE_VCS_VERSION_INC, not needed

[Marcel Hlopko via Phabricator via lldb-commits]

hlopko created this revision.
hlopko added a reviewer: gribozavr2.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, mgorny.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLDB, LLVM.

This preprocessor define was meant to be used to conditionally include
headers with VCS information (for example VCSRevision.inc). However, the
define was always set, and it was the content of the header that was
conditionally generated. Therefore HAVE_VCS_VERSION_INC should be
cleaned up.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84623

Files:
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Version.cpp
  lld/Common/CMakeLists.txt
  lld/Common/Version.cpp
  lldb/source/CMakeLists.txt
  lldb/source/lldb.cpp
  llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
  llvm/utils/gn/secondary/lld/Common/BUILD.gn

Index: llvm/utils/gn/secondary/lld/Common/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/Common/BUILD.gn
+++ llvm/utils/gn/secondary/lld/Common/BUILD.gn
@@ -42,5 +42,4 @@
 "Timer.cpp",
 "Version.cpp",
   ]
-  defines = [ "HAVE_VCS_VERSION_INC" ]  # For Version.cpp
 }
Index: llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
@@ -102,5 +102,4 @@
 "XRayInstr.cpp",
 "XRayLists.cpp",
   ]
-  defines = [ "HAVE_VCS_VERSION_INC" ]  # For Version.cpp
 }
Index: lldb/source/lldb.cpp
===
--- lldb/source/lldb.cpp
+++ lldb/source/lldb.cpp
@@ -13,9 +13,7 @@
 
 #include "clang/Basic/Version.h"
 
-#ifdef HAVE_VCS_VERSION_INC
 #include "VCSVersion.inc"
-#endif
 
 static const char *GetLLDBRevision() {
 #ifdef LLDB_REVISION
Index: lldb/source/CMakeLists.txt
===
--- lldb/source/CMakeLists.txt
+++ lldb/source/CMakeLists.txt
@@ -34,9 +34,6 @@
   PROPERTIES GENERATED TRUE
  HEADER_FILE_ONLY TRUE)
 
-set_property(SOURCE lldb.cpp APPEND PROPERTY
- COMPILE_DEFINITIONS "HAVE_VCS_VERSION_INC")
-
 list(APPEND lldbBase_SOURCES ${version_inc})
 
 if(LLDB_VERSION_STRING)
Index: lld/Common/Version.cpp
===
--- lld/Common/Version.cpp
+++ lld/Common/Version.cpp
@@ -12,9 +12,7 @@
 
 #include "lld/Common/Version.h"
 
-#ifdef HAVE_VCS_VERSION_INC
 #include "VCSVersion.inc"
-#endif
 
 // Returns a version string, e.g.:
 // lld 9.0.0 (https://github.com/llvm/llvm-project.git 9efdd7ac5e914d3c9fa1ef)
Index: lld/Common/CMakeLists.txt
===
--- lld/Common/CMakeLists.txt
+++ lld/Common/CMakeLists.txt
@@ -20,9 +20,6 @@
   PROPERTIES GENERATED TRUE
   HEADER_FILE_ONLY TRUE)
 
-set_property(SOURCE Version.cpp APPEND PROPERTY
-  COMPILE_DEFINITIONS "HAVE_VCS_VERSION_INC")
-
 add_lld_library(lldCommon
   Args.cpp
   DWARF.cpp
Index: clang/lib/Basic/Version.cpp
===
--- clang/lib/Basic/Version.cpp
+++ clang/lib/Basic/Version.cpp
@@ -17,9 +17,7 @@
 #include 
 #include 
 
-#ifdef HAVE_VCS_VERSION_INC
 #include "VCSVersion.inc"
-#endif
 
 namespace clang {
 
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -33,9 +33,6 @@
   PROPERTIES GENERATED TRUE
  HEADER_FILE_ONLY TRUE)
 
-set_property(SOURCE Version.cpp APPEND PROPERTY
- COMPILE_DEFINITIONS "HAVE_VCS_VERSION_INC")
-
 add_clang_library(clangBasic
   Attributes.cpp
   Builtins.cpp
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e89414f - [lldb/Utility] Clean up Scalar constructors

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-27T10:06:56+02:00
New Revision: e89414f4060d3ff2afcd1c89fc028d61270c4d22

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

LOG: [lldb/Utility] Clean up Scalar constructors

- move initialization to initializer lists
- make desctructor non-virtual (nothing else is)
- fix long double constructor so that it actually works

Added: 


Modified: 
lldb/include/lldb/Utility/Scalar.h
lldb/source/Utility/Scalar.cpp
lldb/unittests/Utility/ScalarTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Scalar.h 
b/lldb/include/lldb/Utility/Scalar.h
index 524b71523074..1dbcf80bfd89 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -60,41 +60,30 @@ class Scalar {
   };
 
   // Constructors and Destructors
-  Scalar();
-  Scalar(int v) : m_type(e_sint), m_float(static_cast(0)) {
-m_integer = llvm::APInt(sizeof(int) * 8, v, true);
-  }
-  Scalar(unsigned int v) : m_type(e_uint), m_float(static_cast(0)) {
-m_integer = llvm::APInt(sizeof(int) * 8, v);
-  }
-  Scalar(long v) : m_type(e_slong), m_float(static_cast(0)) {
-m_integer = llvm::APInt(sizeof(long) * 8, v, true);
-  }
-  Scalar(unsigned long v) : m_type(e_ulong), m_float(static_cast(0)) {
-m_integer = llvm::APInt(sizeof(long) * 8, v);
-  }
-  Scalar(long long v) : m_type(e_slonglong), m_float(static_cast(0)) {
-m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
-  }
+  Scalar() : m_type(e_void), m_float(0.0f) {}
+  Scalar(int v)
+  : m_type(e_sint), m_integer(sizeof(v) * 8, v, true), m_float(0.0f) {}
+  Scalar(unsigned int v)
+  : m_type(e_uint), m_integer(sizeof(v) * 8, v, false), m_float(0.0f) {}
+  Scalar(long v)
+  : m_type(e_slong), m_integer(sizeof(v) * 8, v, true), m_float(0.0f) {}
+  Scalar(unsigned long v)
+  : m_type(e_ulong), m_integer(sizeof(v) * 8, v, false), m_float(0.0f) {}
+  Scalar(long long v)
+  : m_type(e_slonglong), m_integer(sizeof(v) * 8, v, true), m_float(0.0f) 
{}
   Scalar(unsigned long long v)
-  : m_type(e_ulonglong), m_float(static_cast(0)) {
-m_integer = llvm::APInt(sizeof(long long) * 8, v);
-  }
-  Scalar(float v) : m_type(e_float), m_float(v) { m_float = llvm::APFloat(v); }
-  Scalar(double v) : m_type(e_double), m_float(v) {
-m_float = llvm::APFloat(v);
+  : m_type(e_ulonglong), m_integer(sizeof(v) * 8, v, false), m_float(0.0f) 
{
   }
-  Scalar(long double v)
-  : m_type(e_long_double),
-m_float(llvm::APFloat::x87DoubleExtended(),
-llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128,
-(reinterpret_cast())->x)) {}
-  Scalar(llvm::APInt v) : m_type(), m_float(static_cast(0)) {
-m_integer = llvm::APInt(std::move(v));
-m_type = GetBestTypeForBitSize(m_integer.getBitWidth(), true);
+  Scalar(float v) : m_type(e_float), m_float(v) {}
+  Scalar(double v) : m_type(e_double), m_float(v) {}
+  Scalar(long double v) : m_type(e_long_double), m_float(double(v)) {
+bool ignore;
+m_float.convert(llvm::APFloat::x87DoubleExtended(),
+llvm::APFloat::rmNearestTiesToEven, );
   }
-  // Scalar(const RegisterValue& reg_value);
-  virtual ~Scalar();
+  Scalar(llvm::APInt v)
+  : m_type(GetBestTypeForBitSize(v.getBitWidth(), true)),
+m_integer(std::move(v)), m_float(0.0f) {}
 
   /// Return the most efficient Scalar::Type for the requested bit size.
   static Type GetBestTypeForBitSize(size_t bit_size, bool sign);

diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 27d5b3b88d33..9309f8d662da 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -126,8 +126,6 @@ static Scalar::Type PromoteToMaxType(
   return Scalar::e_void;
 }
 
-Scalar::Scalar() : m_type(e_void), m_float(static_cast(0)) {}
-
 bool Scalar::GetData(DataExtractor , size_t limit_byte_size) const {
   size_t byte_size = GetByteSize();
   if (byte_size == 0) {
@@ -232,8 +230,6 @@ void Scalar::GetValue(Stream *s, bool show_type) const {
   }
 }
 
-Scalar::~Scalar() = default;
-
 Scalar::Type Scalar::GetBestTypeForBitSize(size_t bit_size, bool sign) {
   // Scalar types are always host types, hence the sizeof().
   if (sign) {

diff  --git a/lldb/unittests/Utility/ScalarTest.cpp 
b/lldb/unittests/Utility/ScalarTest.cpp
index f6bc6a404c15..70ce0a81627d 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -92,6 +92,7 @@ TEST(ScalarTest, Getters) {
   CheckConversion(0x8765432112345678ull);
   CheckConversion(42.25f);
   CheckConversion(42.25);
+  CheckConversion(42.25L);
 
   EXPECT_EQ(APInt(128, 1) << 70, Scalar(std::pow(2.0f, 
70.0f)).SInt128(APInt()));
   EXPECT_EQ(APInt(128, -1, true) << 70,

[Lldb-commits] [lldb] 81d7eba - [lldb/Utility] Fix a bug in RangeMap::CombineConsecutiveRanges

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-27T10:06:56+02:00
New Revision: 81d7ebaf5c369d42b77f9e3e47e2ac22c306ec04

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

LOG: [lldb/Utility] Fix a bug in RangeMap::CombineConsecutiveRanges

The function didn't combine a large entry which overlapped several other
entries, if those other entries were not overlapping among each other.

E.g., (0,20),(5,6),(10,11) produced (0,20),(10,11)

Now it just produced (0,20).

Added: 


Modified: 
lldb/include/lldb/Utility/RangeMap.h
lldb/unittests/Utility/RangeMapTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/RangeMap.h 
b/lldb/include/lldb/Utility/RangeMap.h
index fb24c5a43479..118fdfd85fa9 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -194,41 +194,25 @@ template  class 
RangeVector {
 #ifdef ASSERT_RANGEMAP_ARE_SORTED
 assert(IsSorted());
 #endif
-// Can't combine if ranges if we have zero or one range
-if (m_entries.size() > 1) {
-  // The list should be sorted prior to calling this function
-  typename Collection::iterator pos;
-  typename Collection::iterator end;
-  typename Collection::iterator prev;
-  bool can_combine = false;
-  // First we determine if we can combine any of the Entry objects so we
-  // don't end up allocating and making a new collection for no reason
-  for (pos = m_entries.begin(), end = m_entries.end(), prev = end;
-   pos != end; prev = pos++) {
-if (prev != end && prev->DoesAdjoinOrIntersect(*pos)) {
-  can_combine = true;
-  break;
-}
-  }
+auto first_intersect = std::adjacent_find(
+m_entries.begin(), m_entries.end(), [](const Entry , const Entry ) 
{
+  return a.DoesAdjoinOrIntersect(b);
+});
+if (first_intersect == m_entries.end())
+  return;
 
-  // We we can combine at least one entry, then we make a new collection
-  // and populate it accordingly, and then swap it into place.
-  if (can_combine) {
-Collection minimal_ranges;
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end;
- pos != end; prev = pos++) {
-  if (prev != end && prev->DoesAdjoinOrIntersect(*pos))
-minimal_ranges.back().SetRangeEnd(
-std::max(prev->GetRangeEnd(), pos->GetRangeEnd()));
-  else
-minimal_ranges.push_back(*pos);
-}
-// Use the swap technique in case our new vector is much smaller. We
-// must swap when using the STL because std::vector objects never
-// release or reduce the memory once it has been allocated/reserved.
-m_entries.swap(minimal_ranges);
-  }
+// We we can combine at least one entry, then we make a new collection and
+// populate it accordingly, and then swap it into place.
+auto pos = std::next(first_intersect);
+Collection minimal_ranges(m_entries.begin(), pos);
+for (; pos != m_entries.end(); ++pos) {
+  Entry  = minimal_ranges.back();
+  if (back.DoesAdjoinOrIntersect(*pos))
+back.SetRangeEnd(std::max(back.GetRangeEnd(), pos->GetRangeEnd()));
+  else
+minimal_ranges.push_back(*pos);
 }
+m_entries.swap(minimal_ranges);
   }
 
   BaseType GetMinRangeBase(BaseType fail_value) const {
@@ -353,6 +337,10 @@ template  class 
RangeVector {
 return nullptr;
   }
 
+  using const_iterator = typename Collection::const_iterator;
+  const_iterator begin() const { return m_entries.begin(); }
+  const_iterator end() const { return m_entries.end(); }
+
 protected:
   void CombinePrevAndNext(typename Collection::iterator pos) {
 // Check if the prev or next entries in case they need to be unioned with

diff  --git a/lldb/unittests/Utility/RangeMapTest.cpp 
b/lldb/unittests/Utility/RangeMapTest.cpp
index 8a243b656218..97432dca983d 100644
--- a/lldb/unittests/Utility/RangeMapTest.cpp
+++ b/lldb/unittests/Utility/RangeMapTest.cpp
@@ -12,6 +12,32 @@
 
 using namespace lldb_private;
 
+TEST(RangeVector, CombineConsecutiveRanges) {
+  using RangeVector = RangeVector;
+  using Entry = RangeVector::Entry;
+
+  RangeVector V;
+  V.Append(0, 1);
+  V.Append(5, 1);
+  V.Append(6, 1);
+  V.Append(10, 9);
+  V.Append(15, 1);
+  V.Append(20, 9);
+  V.Append(21, 9);
+  V.Sort();
+  V.CombineConsecutiveRanges();
+  EXPECT_THAT(V, testing::ElementsAre(Entry(0, 1), Entry(5, 2), Entry(10, 9),
+  Entry(20, 10)));
+
+  V.Clear();
+  V.Append(0, 20);
+  V.Append(5, 1);
+  V.Append(10, 1);
+  V.Sort();
+  V.CombineConsecutiveRanges();
+  EXPECT_THAT(V, testing::ElementsAre(Entry(0, 20)));
+}
+
 using RangeDataVectorT = RangeDataVector;
 using EntryT =