[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization

2021-08-03 Thread Jay Foad via Phabricator via lldb-commits
foad added inline comments.



Comment at: lldb/docs/htr.rst:13
+
+ **Instruction Layer:** Composed of the oad addresses of the instructions in 
the trace. In an effort to save space, 
+ metadata is only stored for instructions that are of interest, not every 
instruction in the trace. HTR contains a 

Typo "oad".



Comment at: lldb/docs/htr.rst:22
+**Pass:** A transformation applied to a *layer* that generates a new *layer* 
that is a more summarized, consolidated representation of the trace data.
+A pass merges instructions/blocks based on its specific purpose - for example, 
a pass designed to summarize a processor trace by function calls would merge 
all the blocks of a function into a single block representing the entire 
function.l
+

Stray "l" at end of line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105741

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


[Lldb-commits] [PATCH] D107338: [lldb] Get rid of HAVE_SYS_TYPES_H

2021-08-03 Thread Nico Weber via Phabricator via lldb-commits
thakis created this revision.
thakis added a reviewer: JDevlieghere.
thakis added a project: LLDB.
Herald added a subscriber: mgorny.
thakis requested review of this revision.

LLVM includes this header unconditionally on all platforms
(including Windows), so this define should no longer be necessary.

No behavior change.


https://reviews.llvm.org/D107338

Files:
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/windows/PosixApi.h
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Expression/UtilityFunction.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/tools/driver/Platform.h

Index: lldb/tools/driver/Platform.h
===
--- lldb/tools/driver/Platform.h
+++ lldb/tools/driver/Platform.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_TOOLS_DRIVER_PLATFORM_H
 #define LLDB_TOOLS_DRIVER_PLATFORM_H
 
+// FIXME: This include is now unused. Make sure all files that got it
+// via Platform.h that need it include it directly, then remove this.
 #include "lldb/Host/Config.h"
 
 #if defined(_WIN32)
@@ -17,11 +19,10 @@
 #if defined(_MSC_VER)
 #include 
 #endif
-#if HAVE_SYS_TYPES_H
-#include 
-#endif
+
 #include "lldb/Host/windows/windows.h"
 #include 
+#include 
 
 struct winsize {
   long ws_col;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
@@ -6,8 +6,6 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include "ClangUtilityFunction.h"
 #include "ClangExpressionDeclMap.h"
 #include "ClangExpressionParser.h"
@@ -15,9 +13,7 @@
 #include "ClangPersistentVariables.h"
 
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 
 #include "lldb/Core/Module.h"
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -6,12 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 
Index: lldb/source/Expression/UtilityFunction.cpp
===
--- lldb/source/Expression/UtilityFunction.cpp
+++ lldb/source/Expression/UtilityFunction.cpp
@@ -6,13 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
-
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -6,12 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 
Index: lldb/include/lldb/Host/windows/PosixApi.h
===
--- lldb/include/lldb/Host/windows/PosixApi.h
+++ lldb/include/lldb/Host/windows/PosixApi.h
@@ -56,14 +56,12 @@
 #define S_IRWXO 0
 #endif
 
-#if HAVE_SYS_TYPES_H
 // pyconfig.h typedefs this.  We require python headers to be included before
 // any LLDB headers, but there's no way to prevent python's pid_t definition
 // from leaking, so this is the best option.
 #ifndef NO_PID_T
 #include 
 #endif
-#endif // HAVE_SYS_TYPES_H
 
 #ifdef _MSC_VER
 
Index: lldb/include/lldb/Host/Config.h.cmake
===
--- lldb/include/lldb/Host/Config.h.cmake
+++ lldb/include/lldb/Host/Config.h.cmake
@@ -13,9 +13,6 @@
 
 #cmakedefine01 LLDB_HAVE_EL_RFUNC_T
 
-
-#cmakedefine01 HAVE_SYS_TYPES_H
-
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL
Index: lldb/cmake/modules/LLDBGenerateConfig.cmake
===
--- lldb/cmake/modules/LLDBGenerateConfig.cmake
+++ lldb/cmake/modules/LLDBGenerateConfig.cmake
@@ -12,7 +12,6 @@
 check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
 
 check_include_file(termios.h HAVE_TERMIOS_H)
-check_include_file("sys/types.h" HAVE_SYS_TYPES_H)
 check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)
 
 check_cxx_symbol_exists(process_vm_readv "sys/uio.h" HAVE_PROCESS_VM_READV)
_

[Lldb-commits] [PATCH] D107338: [lldb] Get rid of HAVE_SYS_TYPES_H

2021-08-03 Thread Nico Weber via Phabricator via lldb-commits
thakis updated this revision to Diff 363685.
thakis added a comment.

Remove Config.h include from driver/Platform.h. Platform.h only gets included 
in 5 files, and I checked that none of those need Config.h.


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

https://reviews.llvm.org/D107338

Files:
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/windows/PosixApi.h
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Expression/UtilityFunction.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/tools/driver/Platform.h

Index: lldb/tools/driver/Platform.h
===
--- lldb/tools/driver/Platform.h
+++ lldb/tools/driver/Platform.h
@@ -9,19 +9,16 @@
 #ifndef LLDB_TOOLS_DRIVER_PLATFORM_H
 #define LLDB_TOOLS_DRIVER_PLATFORM_H
 
-#include "lldb/Host/Config.h"
-
 #if defined(_WIN32)
 
 #include 
 #if defined(_MSC_VER)
 #include 
 #endif
-#if HAVE_SYS_TYPES_H
-#include 
-#endif
+
 #include "lldb/Host/windows/windows.h"
 #include 
+#include 
 
 struct winsize {
   long ws_col;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
@@ -6,8 +6,6 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include "ClangUtilityFunction.h"
 #include "ClangExpressionDeclMap.h"
 #include "ClangExpressionParser.h"
@@ -15,9 +13,7 @@
 #include "ClangPersistentVariables.h"
 
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 
 #include "lldb/Core/Module.h"
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -6,12 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 
Index: lldb/source/Expression/UtilityFunction.cpp
===
--- lldb/source/Expression/UtilityFunction.cpp
+++ lldb/source/Expression/UtilityFunction.cpp
@@ -6,13 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
-
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -6,12 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 
Index: lldb/include/lldb/Host/windows/PosixApi.h
===
--- lldb/include/lldb/Host/windows/PosixApi.h
+++ lldb/include/lldb/Host/windows/PosixApi.h
@@ -56,14 +56,12 @@
 #define S_IRWXO 0
 #endif
 
-#if HAVE_SYS_TYPES_H
 // pyconfig.h typedefs this.  We require python headers to be included before
 // any LLDB headers, but there's no way to prevent python's pid_t definition
 // from leaking, so this is the best option.
 #ifndef NO_PID_T
 #include 
 #endif
-#endif // HAVE_SYS_TYPES_H
 
 #ifdef _MSC_VER
 
Index: lldb/include/lldb/Host/Config.h.cmake
===
--- lldb/include/lldb/Host/Config.h.cmake
+++ lldb/include/lldb/Host/Config.h.cmake
@@ -13,9 +13,6 @@
 
 #cmakedefine01 LLDB_HAVE_EL_RFUNC_T
 
-
-#cmakedefine01 HAVE_SYS_TYPES_H
-
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL
Index: lldb/cmake/modules/LLDBGenerateConfig.cmake
===
--- lldb/cmake/modules/LLDBGenerateConfig.cmake
+++ lldb/cmake/modules/LLDBGenerateConfig.cmake
@@ -12,7 +12,6 @@
 check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
 
 check_include_file(termios.h HAVE_TERMIOS_H)
-check_include_file("sys/types.h" HAVE_SYS_TYPES_H)
 check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)
 
 check_cxx_symbol_exists(process_vm_readv "sys/uio.h" HAVE_PROCESS_VM_READV)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D107338: [lldb] Get rid of HAVE_SYS_TYPES_H

2021-08-03 Thread Nico Weber via Phabricator via lldb-commits
thakis added a comment.

In LLVM, this was removed in 37f69de11b8c1810e29851430da317db5c1350a9.


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

https://reviews.llvm.org/D107338

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


[Lldb-commits] [PATCH] D107338: [lldb] Get rid of HAVE_SYS_TYPES_H

2021-08-03 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

Thanks! This LGTM itself, but I would prefer if we could remove that define in 
this commit and remove the redundant Config.h includes in another (so, the 
first version of this patch LGTM).

Removing Config.h is always a bit icky as it will just silently disable all the 
`#if feature` code in case it's actually needed. IIUC you checked all the 
including files, but there are also downstream folks who will have probably an 
easier time bisecting potential fallout from this when it's a dedicated commit 
with a clear '[lldb] Remove redundant Config.h includes' as a title.

(Also unrelated: I think quite a few of the files here don't actually need 
anything from sys/types.h? We could probably just remove a bunch of this stuff 
all-together as a followup)


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

https://reviews.llvm.org/D107338

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


[Lldb-commits] [PATCH] D107341: [lldb] Move comment about noindex next to line it refers to

2021-08-03 Thread Nico Weber via Phabricator via lldb-commits
thakis created this revision.
thakis added a reviewer: JDevlieghere.
thakis added a project: LLDB.
Herald added subscribers: arphaman, mgorny.
thakis requested review of this revision.

The comment was originally added in 34769d80d. Then D44526 

removed the flag added there (but kept the comment), and then
D66966  reintroduced a .noindex dir (which 
D68606  and then 33fca97880
moved around a bit).

No behavior change.


https://reviews.llvm.org/D107341

Files:
  lldb/test/API/CMakeLists.txt
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -12,6 +12,11 @@
 endif()
 
 # Configure the build directory.
+# The .noindex suffix is a marker for Spotlight to never index the
+# build directory.  LLDB queries Spotlight to locate .dSYM bundles
+# based on the UUID embedded in a binary, and because the UUID is a
+# hash of filename and .text section, there *will* be conflicts inside
+# the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" 
CACHE PATH "The build root for building tests.")
 
 # Configure and create module cache directories.
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -37,11 +37,6 @@
   ""
   CACHE STRING "Specify additional arguments to pass to test runner. For 
example: '-C gcc -C clang -A i386 -A x86_64'")
 
-# The .noindex suffix is a marker for Spotlight to never index the
-# build directory.  LLDB queries Spotlight to locate .dSYM bundles
-# based on the UUID embedded in a binary, and because the UUID is a
-# hash of filename and .text section, there *will* be conflicts inside
-# the build directory.
 set(LLDB_TEST_COMMON_ARGS
   -u CXXFLAGS
   -u CFLAGS


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -12,6 +12,11 @@
 endif()
 
 # Configure the build directory.
+# The .noindex suffix is a marker for Spotlight to never index the
+# build directory.  LLDB queries Spotlight to locate .dSYM bundles
+# based on the UUID embedded in a binary, and because the UUID is a
+# hash of filename and .text section, there *will* be conflicts inside
+# the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" CACHE PATH "The build root for building tests.")
 
 # Configure and create module cache directories.
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -37,11 +37,6 @@
   ""
   CACHE STRING "Specify additional arguments to pass to test runner. For example: '-C gcc -C clang -A i386 -A x86_64'")
 
-# The .noindex suffix is a marker for Spotlight to never index the
-# build directory.  LLDB queries Spotlight to locate .dSYM bundles
-# based on the UUID embedded in a binary, and because the UUID is a
-# hash of filename and .text section, there *will* be conflicts inside
-# the build directory.
 set(LLDB_TEST_COMMON_ARGS
   -u CXXFLAGS
   -u CFLAGS
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D107341: [lldb] Move comment about noindex next to line it refers to

2021-08-03 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

Thanks, lgtm.


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

https://reviews.llvm.org/D107341

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


[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread Amy Huang via Phabricator via lldb-commits
akhuang added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1636
+if (Opts.getDebugInfo() == codegenoptions::DebugInfoConstructor)
+  Opts.setDebugInfo(codegenoptions::LimitedDebugInfo);
 

probinson wrote:
> No... you want to check both options in one call, otherwise 
> `-fno-use-ctor-homing -fuse-ctor-homing` will prefer the `no` version instead 
> of last-one-wins.
> 
> I suggest fiddling the options should be done separately.
> Also if you want to make it a clang option, the option name should have 
> `debug` in it; pretty sure all the -f options related to debug info do that, 
> and in any case it's a good idea.
ah, right. 

I'll move this to a separate patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

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


[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread Paul Robinson via Phabricator via lldb-commits
probinson added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1636
+if (Opts.getDebugInfo() == codegenoptions::DebugInfoConstructor)
+  Opts.setDebugInfo(codegenoptions::LimitedDebugInfo);
 

No... you want to check both options in one call, otherwise 
`-fno-use-ctor-homing -fuse-ctor-homing` will prefer the `no` version instead 
of last-one-wins.

I suggest fiddling the options should be done separately.
Also if you want to make it a clang option, the option name should have `debug` 
in it; pretty sure all the -f options related to debug info do that, and in any 
case it's a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

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


[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread Amy Huang via Phabricator via lldb-commits
akhuang updated this revision to Diff 360918.
akhuang added a comment.

Remove fno-use-ctor-homing flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

Files:
  clang/include/clang/Basic/DebugInfoOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCXX/debug-info-template-deduction-guide.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-g-opts.c
  clang/test/Driver/cuda-dwarf-2.cu
  clang/test/Driver/debug-options-as.c
  clang/test/Driver/debug-options.c
  clang/test/Driver/integrated-as.s
  clang/test/Driver/myriad-toolchain.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/split-debug.c
  lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp

Index: lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
===
--- lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
+++ lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
@@ -106,6 +106,9 @@
 int main() {
   MemberTest::Base B1;
   B1.Get();
+  // Create instance of C1 so that it has debug info (due to constructor
+  // homing).
+  MemberTest::Class C1;
   MemberTest::Class::StaticMemberFunc(1, 10, 2);
   return 0;
 }
Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -9,7 +9,7 @@
 
 // INLINE: "-fsplit-dwarf-inlining"
 // NOINLINE-NOT: "-fsplit-dwarf-inlining"
-// SPLIT:  "-debug-info-kind=limited"
+// SPLIT:  "-debug-info-kind=constructor"
 // SPLIT-SAME: "-ggnu-pubnames"
 // SPLIT-SAME: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
 
@@ -38,14 +38,14 @@
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gno-split-dwarf -g -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefixes=NOINLINE,SPLIT
 
-// NOSPLIT: "-debug-info-kind=limited"
+// NOSPLIT: "-debug-info-kind=constructor"
 // NOSPLIT-NOT: "-ggnu-pubnames"
 // NOSPLIT-NOT: "-split-dwarf
 
 /// Test -gsplit-dwarf=single.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g %s 2>&1 | FileCheck %s --check-prefix=SINGLE
 
-// SINGLE: "-debug-info-kind=limited"
+// SINGLE: "-debug-info-kind=constructor"
 // SINGLE: "-split-dwarf-file" "split-debug.o"
 // SINGLE-NOT: "-split-dwarf-output"
 
@@ -62,7 +62,7 @@
 
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=split -g -gno-pubnames %s 2>&1 | FileCheck %s --check-prefixes=NOPUBNAMES
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=split -g -gno-gnu-pubnames %s 2>&1 | FileCheck %s --check-prefixes=NOPUBNAMES
-// NOPUBNAMES:  "-debug-info-kind=limited"
+// NOPUBNAMES:  "-debug-info-kind=constructor"
 // NOPUBNAMES-NOT:  "-ggnu-pubnames"
 // NOPUBNAMES-SAME: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
 
Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -248,7 +248,7 @@
 
 // HAS_DEBUG-NOT: warning: debug
 // HAS_DEBUG: "-triple" "nvptx64-nvidia-cuda"
-// HAS_DEBUG-SAME: "-debug-info-kind={{limited|line-tables-only}}"
+// HAS_DEBUG-SAME: "-debug-info-kind={{constructor|line-tables-only}}"
 // HAS_DEBUG-SAME: "-dwarf-version=2"
 // HAS_DEBUG-SAME: "-fopenmp-is-device"
 // HAS_DEBUG: ptxas
Index: clang/test/Driver/myriad-toolchain.c
===
--- clang/test/Driver/myriad-toolchain.c
+++ clang/test/Driver/myriad-toolchain.c
@@ -83,7 +83,7 @@
 // NOSTDLIB-NOT: "-lc"
 
 // RUN: %clang -### -c -g %s -target sparc-myriad 2>&1 | FileCheck -check-prefix=G_SPARC %s
-// G_SPARC: "-debug-info-kind=limited" "-dwarf-version=2"
+// G_SPARC: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -c %s -target sparc-myriad-rtems -fuse-init-array 2>&1 \
 // RUN: | FileCheck -check-prefix=USE-INIT-ARRAY %s
Index: clang/test/Driver/integrated-as.s
===
--- clang/test/Driver/integrated-as.s
+++ clang/test/Driver/integrated-as.s
@@ -27,19 +27,19 @@
 // XA_INCLUDE2: "-Ifoo_dir"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-4 -gdwarf-2 2>&1 | FileCheck --check-prefix=DWARF2 %s
-// DWARF2: "-debug-info-kind=limited" "-dwarf-version=2"
+// DWARF2: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-3 2>&1 | FileCheck --check-prefix=DWARF3 %s
-// DWARF3: "-debug-info-kind=limited" "-dwarf-version=3"
+// DWARF3: "-debug-info-kind=constructor" "-dwarf-version=3"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-4 2>&1 | FileCheck --

[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread Jeremy Morse via Phabricator via lldb-commits
jmorse added a comment.

David wrote:

> think what I'm missing here: If -fno-standalone-debug is already in use/the 
> default and is causing missing types because parts of the program are bulit 
> without debug info, then I'm not sure what the rationale is for slicing 
> -fstandalone-debug into a "has ctor homing" and a "doesn't have ctor homing" 
> strategy. The same general design philosophy applies - that in both cases 
> (about 4 cases in total now: types that aren't required to be complete, 
> explicitly instantiated templates, vtables, and now ctor homing) the whole 
> program must be compiled with debug info enabled for these DWARF size 
> optimizations to be sound.

It's a matter of degree:

- Some types go missing, but due to the dllimport mechanism mentioned, all the 
important ones are retained,
- Switching to -fstandalone-debug increases the amount of DWARF by ~50% in 
experiments I've run, which for a {single-file-recompile, link, 
load-into-debugger} round trip will translate to an almost 50% increase in 
round-trip-time. (What with DWARF being the major part of linking and 
debugger-loading).

Thus the status quo (-fno-standalone-debug and closed-source libraries) hasn't 
been conceptually sounds, but it's given a "good enough" debugging experience 
without major round-trip-time penalty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

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


[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.

In D106084#2897515 , @jmorse wrote:

> David wrote:
>
>> think what I'm missing here: If -fno-standalone-debug is already in use/the 
>> default and is causing missing types because parts of the program are bulit 
>> without debug info, then I'm not sure what the rationale is for slicing 
>> -fstandalone-debug into a "has ctor homing" and a "doesn't have ctor homing" 
>> strategy. The same general design philosophy applies - that in both cases 
>> (about 4 cases in total now: types that aren't required to be complete, 
>> explicitly instantiated templates, vtables, and now ctor homing) the whole 
>> program must be compiled with debug info enabled for these DWARF size 
>> optimizations to be sound.
>
> It's a matter of degree:
>
> - Some types go missing, but due to the dllimport mechanism mentioned, all 
> the important ones are retained,
> - Switching to -fstandalone-debug increases the amount of DWARF by ~50% in 
> experiments I've run, which for a {single-file-recompile, link, 
> load-into-debugger} round trip will translate to an almost 50% increase in 
> round-trip-time. (What with DWARF being the major part of linking and 
> debugger-loading).
>
> Thus the status quo (-fno-standalone-debug and closed-source libraries) 
> hasn't been conceptually sounds, but it's given a "good enough" debugging 
> experience without major round-trip-time penalty.

I think it'd be unfortunate to split this hair in LLVM/Clang proper & feel like 
that sort of value tradeoff might be better suited in a downstream patch. 
(mostly because eventually it'd be good to get rid of the distinction between 
other type homing and ctor type homing entirely - there's already 3 categories 
of type homing under the existing category (type completeness, explicit 
template instantiations, and vtable based homing) & I don't think there's a 
good line to draw between those and ctor type homing - and module type homing 
(which I think I've already implemented &the  is under -fno-standalone-debug, 
but no one's using modular code generation right now so it's not super 
interesting to anyone), maybe ThinLTO type homing (which would be a bit more 
robust than the others, since it has whole program-ish view)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

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


[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread Amy Huang via Phabricator via lldb-commits
akhuang added a comment.

In D106084#2897515 , @jmorse wrote:

> David wrote:
>
>> think what I'm missing here: If -fno-standalone-debug is already in use/the 
>> default and is causing missing types because parts of the program are bulit 
>> without debug info, then I'm not sure what the rationale is for slicing 
>> -fstandalone-debug into a "has ctor homing" and a "doesn't have ctor homing" 
>> strategy. The same general design philosophy applies - that in both cases 
>> (about 4 cases in total now: types that aren't required to be complete, 
>> explicitly instantiated templates, vtables, and now ctor homing) the whole 
>> program must be compiled with debug info enabled for these DWARF size 
>> optimizations to be sound.
>
> It's a matter of degree:
>
> - Some types go missing, but due to the dllimport mechanism mentioned, all 
> the important ones are retained,
> - Switching to -fstandalone-debug increases the amount of DWARF by ~50% in 
> experiments I've run, which for a {single-file-recompile, link, 
> load-into-debugger} round trip will translate to an almost 50% increase in 
> round-trip-time. (What with DWARF being the major part of linking and 
> debugger-loading).
>
> Thus the status quo (-fno-standalone-debug and closed-source libraries) 
> hasn't been conceptually sounds, but it's given a "good enough" debugging 
> experience without major round-trip-time penalty.

Right, I think chromium on mac does the same thing (build with 
-fno-standalone-debug even though it drops some types, because 
-fstandalone-debug is too large).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

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


[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

2021-08-03 Thread Tony Tye via Phabricator via lldb-commits
t-tye updated this revision to Diff 361402.
t-tye added a comment.

Split change for clang makefile to elimnate Sphinx warnings of missing .rst 
fies when building man pages into D106734 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106339

Files:
  clang-tools-extra/docs/CMakeLists.txt
  clang-tools-extra/docs/conf.py
  clang-tools-extra/docs/make.bat
  clang/docs/CMakeLists.txt
  clang/docs/Makefile.sphinx
  clang/docs/analyzer/conf.py
  clang/docs/analyzer/make.bat
  clang/docs/conf.py
  clang/docs/make.bat
  flang/docs/CMakeLists.txt
  flang/docs/conf.py
  libcxx/docs/CMakeLists.txt
  libcxx/docs/Makefile.sphinx
  libcxx/docs/conf.py
  libunwind/docs/CMakeLists.txt
  libunwind/docs/conf.py
  lld/docs/CMakeLists.txt
  lld/docs/conf.py
  lld/docs/make.bat
  lld/docs/sphinx_intro.rst
  lldb/docs/CMakeLists.txt
  lldb/docs/conf.py
  lldb/docs/resources/build.rst
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/FindSphinx.cmake
  llvm/docs/CMake.rst
  llvm/docs/CMakeLists.txt
  llvm/docs/Makefile.sphinx
  llvm/docs/README.txt
  llvm/docs/conf.py
  llvm/docs/make.bat
  openmp/docs/CMakeLists.txt
  openmp/docs/README.txt
  openmp/docs/conf.py
  polly/docs/CMakeLists.txt
  polly/docs/conf.py

Index: polly/docs/conf.py
===
--- polly/docs/conf.py
+++ polly/docs/conf.py
@@ -28,6 +28,9 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax']
 
+if tags.has('builder-docx'):
+extensions.append('docxbuilder')
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
 
Index: polly/docs/CMakeLists.txt
===
--- polly/docs/CMakeLists.txt
+++ polly/docs/CMakeLists.txt
@@ -98,6 +98,9 @@
 if (${SPHINX_OUTPUT_MAN})
   add_sphinx_target(man polly)
 endif()
+if (${SPHINX_OUTPUT_DOCX})
+  add_sphinx_target(docx polly)
+endif()
   endif()
 endif()
 
Index: openmp/docs/conf.py
===
--- openmp/docs/conf.py
+++ openmp/docs/conf.py
@@ -28,6 +28,9 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax', 'sphinx.ext.intersphinx']
 
+if tags.has('builder-docx'):
+extensions.append('docxbuilder')
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
 
Index: openmp/docs/README.txt
===
--- openmp/docs/README.txt
+++ openmp/docs/README.txt
@@ -1,5 +1,5 @@
 OpenMP LLVM Documentation
-==
+=
 
 OpenMP LLVM's documentation is written in reStructuredText, a lightweight
 plaintext markup language (file extension `.rst`). While the
@@ -14,11 +14,15 @@
 cd 
 cmake -DLLVM_ENABLE_SPHINX=true -DSPHINX_OUTPUT_HTML=true 
 make
-$BROWSER /projects/openmp/docs//html/index.html
+$BROWSER /projects/openmp/docs/html/index.html
 
-The mapping between reStructuredText files and generated documentation is
-`docs/Foo.rst` <-> `/projects/openmp/docs//html/Foo.html` <->
-`https://openmp.llvm.org/docs/Foo.html`.
+The correspondence between reStructuredText files and generated HTML pages is:
+
+LLVM project
+`llvm/docs/Foo.rst` <-> `/docs/html/Foo.html` <-> `/share/doc/llvm/html/Foo.html` <-> `https://llvm.org/docs/Foo.html`
+
+Other projects
+`/docs/Foo.rst` <-> `/tools//docs/html/Foo.html` <-> `/share/doc//html/Foo.html`<-> `https://.llvm.org/docs/Foo.html`
 
 If you are interested in writing new documentation, you will want to read
 `llvm/docs/SphinxQuickstartTemplate.rst` which will get you writing
@@ -26,7 +30,7 @@
 reStructuredText markup syntax.
 
 Manpage Output
-===
+==
 
 Building the manpages is similar to building the HTML documentation. The
 primary difference is to use the `man` makefile target, instead of the
@@ -36,10 +40,41 @@
 cd 
 cmake -DLLVM_ENABLE_SPHINX=true -DSPHINX_OUTPUT_MAN=true 
 make
-man -l >build-dir>/docs/man/FileCheck.1
+man -l /docs/man/FileCheck.1
+
+The correspondence between reStructuredText files and generated man pages is:
+
+LLVM project
+`llvm/docs/CommandGuide/Foo.rst` <-> `/docs/man/Foo.1` <-> `/share/man/man1/Foo.1`
+
+Other projects
+`/docs/CommandGuide/Foo.rst` <-> `/tools//docs/man/Foo.1` <-> `/share/man/man1/Foo.1`
 
-The correspondence between .rst files and man pages is
-`docs/CommandGuide/Foo.rst` <-> `/projects/openmp/docs//man/Foo.1`.
 These .rst files are also included during HTML generation so they are also
-viewable online (as noted above) at e.g.
-`https://openmp.llvm.org/docs/CommandGuide/Foo.html`.
+viewable online:
+
+LLVM project
+

[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

2021-08-03 Thread Tony Tye via Phabricator via lldb-commits
t-tye added a comment.

In D106339#2890258 , @ldionne wrote:

> What's the benefit of having docx documentation? We generate HTML 
> documentation, which ends up in the website, and that seems strictly superior 
> to generating docx. What do you need it for?
>
> The libc++ changes are almost trivial so I would not object to the change on 
> that basis, however in general I think it's better to avoid adding support 
> for things we won't be using on a regular basis.

We do have a project that requires docx documentation that includes parts of 
the LLVM documentation, so being able to generate it from the build is helpful. 
However, if adding docx support is not useful to anyone else then the changes 
can be kept out of tree.

A few observations are that the makefile.bat and Makefile.sphinx files already 
appear to support many of the build targets supported by Sphinx, so adding docx 
did not seem out of place. Building docx as part of the LLVM build is disabled 
by default so has no impact unless explicitly enabled. The changes to support 
it appeared fairly minor.

The changes not directly related to adding docx support have been split out to 
D106338 , D106734 
 and D106736 
 which may be worth considering independent 
of whether this review is useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106339

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


[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

2021-08-03 Thread Tony Tye via Phabricator via lldb-commits
t-tye updated this revision to Diff 361407.
t-tye added a comment.

Factor out documentation and CMake file changes unrelated to adding DOCX 
support to D106736 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106339

Files:
  clang-tools-extra/docs/CMakeLists.txt
  clang-tools-extra/docs/conf.py
  clang-tools-extra/docs/make.bat
  clang/docs/CMakeLists.txt
  clang/docs/Makefile.sphinx
  clang/docs/analyzer/conf.py
  clang/docs/analyzer/make.bat
  clang/docs/conf.py
  clang/docs/make.bat
  flang/docs/CMakeLists.txt
  flang/docs/conf.py
  libcxx/docs/CMakeLists.txt
  libcxx/docs/Makefile.sphinx
  libcxx/docs/conf.py
  libunwind/docs/CMakeLists.txt
  libunwind/docs/conf.py
  lld/docs/CMakeLists.txt
  lld/docs/conf.py
  lld/docs/make.bat
  lld/docs/sphinx_intro.rst
  lldb/docs/CMakeLists.txt
  lldb/docs/conf.py
  lldb/docs/resources/build.rst
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/FindSphinx.cmake
  llvm/docs/CMake.rst
  llvm/docs/CMakeLists.txt
  llvm/docs/Makefile.sphinx
  llvm/docs/README.txt
  llvm/docs/conf.py
  llvm/docs/make.bat
  openmp/docs/CMakeLists.txt
  openmp/docs/README.txt
  openmp/docs/conf.py
  polly/docs/CMakeLists.txt
  polly/docs/conf.py

Index: polly/docs/conf.py
===
--- polly/docs/conf.py
+++ polly/docs/conf.py
@@ -28,6 +28,9 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax']
 
+if tags.has('builder-docx'):
+extensions.append('docxbuilder')
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
 
Index: polly/docs/CMakeLists.txt
===
--- polly/docs/CMakeLists.txt
+++ polly/docs/CMakeLists.txt
@@ -98,6 +98,9 @@
 if (${SPHINX_OUTPUT_MAN})
   add_sphinx_target(man polly)
 endif()
+if (${SPHINX_OUTPUT_DOCX})
+  add_sphinx_target(docx polly)
+endif()
   endif()
 endif()
 
Index: openmp/docs/conf.py
===
--- openmp/docs/conf.py
+++ openmp/docs/conf.py
@@ -28,6 +28,9 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax', 'sphinx.ext.intersphinx']
 
+if tags.has('builder-docx'):
+extensions.append('docxbuilder')
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
 
Index: openmp/docs/README.txt
===
--- openmp/docs/README.txt
+++ openmp/docs/README.txt
@@ -58,3 +58,23 @@
 
 Other projects (such as openmp)
 `https://.llvm.org/docs/CommandGuide/Foo.html`
+
+DOCX Output
+===
+
+Building the DOCX files is similar to building the HTML documentation. The
+primary difference is to use the `docx` makefile target, instead of the
+default (which is `html`). Sphinx then produces the DOCX files in the
+directory `/docs/docx/`.
+
+cd 
+cmake -DLLVM_ENABLE_SPHINX=true -DSPHINX_OUTPUT_DOCX=true 
+make
+
+The correspondence between reStructuredText files and generated DOCX files is:
+
+LLVM project
+`llvm/docs/index.rst` <-> `/docs/docx/LLVM.docx` <-> `/share/doc/llvm/LLVM.docx`
+
+Other projects
+`/docs/index.rst` <-> `/tools//docs/docx/.docx` <-> `/share/doc//.docx`
Index: openmp/docs/CMakeLists.txt
===
--- openmp/docs/CMakeLists.txt
+++ openmp/docs/CMakeLists.txt
@@ -99,5 +99,8 @@
 if (${SPHINX_OUTPUT_MAN})
   add_sphinx_target(man openmp)
 endif()
+if (${SPHINX_OUTPUT_DOCX})
+  add_sphinx_target(docx openmp)
+endif()
   endif()
 endif()
Index: llvm/docs/make.bat
===
--- llvm/docs/make.bat
+++ llvm/docs/make.bat
@@ -31,6 +31,7 @@
 	echo.  text   to make text files
 	echo.  manto make manual pages
 	echo.  texinfoto make Texinfo files
+	echo.  docx   to make DOCX files
 	echo.  gettextto make PO message catalogs
 	echo.  changesto make an overview over all changed/added/deprecated items
 	echo.  linkcheck  to check all external links for integrity
@@ -153,6 +154,14 @@
 	goto end
 )
 
+if "%1" == "docx" (
+	%SPHINXBUILD% -b docx %ALLSPHINXOPTS% %BUILDDIR%/docx
+	if errorlevel 1 exit /b 1
+	echo.
+	echo.Build finished. The DOCX files are in %BUILDDIR%/docx.
+	goto end
+)
+
 if "%1" == "gettext" (
 	%SPHINXBUILD% -b gettext %I18NSPHINXOPTS% %BUILDDIR%/locale
 	if errorlevel 1 exit /b 1
Index: llvm/docs/conf.py
===
--- llvm/docs/conf.py
+++ llvm/docs/conf.py
@@ -28,6 +28,9 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.i

[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

2021-08-03 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

I remember that we even converted some `*.md` to `*.rst` to remove the number 
of supported documentation formats, so I agree that adding support the even 
less used `*.docx` may not be the proper direction.

Thanks for splitting this off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106339

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


[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread David Blaikie via Phabricator via lldb-commits
dblaikie accepted this revision.
dblaikie added a comment.

Sounds like this is good for Google and Sony, and Apple doesn't use 
`-fno-standalone-debug`/`-flimit-debug-info` anyway, so probably about ready to 
move forward, then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

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


[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread Amy Huang via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a3bf2953a92: [DebugInfo] Switch to using constructor homing 
(-debug-info-kind=constructor)… (authored by akhuang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

Files:
  clang/include/clang/Basic/DebugInfoOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCXX/debug-info-template-deduction-guide.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-g-opts.c
  clang/test/Driver/cuda-dwarf-2.cu
  clang/test/Driver/debug-options-as.c
  clang/test/Driver/debug-options.c
  clang/test/Driver/integrated-as.s
  clang/test/Driver/myriad-toolchain.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/split-debug.c
  lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp

Index: lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
===
--- lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
+++ lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
@@ -106,6 +106,9 @@
 int main() {
   MemberTest::Base B1;
   B1.Get();
+  // Create instance of C1 so that it has debug info (due to constructor
+  // homing).
+  MemberTest::Class C1;
   MemberTest::Class::StaticMemberFunc(1, 10, 2);
   return 0;
 }
Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -9,7 +9,7 @@
 
 // INLINE: "-fsplit-dwarf-inlining"
 // NOINLINE-NOT: "-fsplit-dwarf-inlining"
-// SPLIT:  "-debug-info-kind=limited"
+// SPLIT:  "-debug-info-kind=constructor"
 // SPLIT-SAME: "-ggnu-pubnames"
 // SPLIT-SAME: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
 
@@ -38,14 +38,14 @@
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gno-split-dwarf -g -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefixes=NOINLINE,SPLIT
 
-// NOSPLIT: "-debug-info-kind=limited"
+// NOSPLIT: "-debug-info-kind=constructor"
 // NOSPLIT-NOT: "-ggnu-pubnames"
 // NOSPLIT-NOT: "-split-dwarf
 
 /// Test -gsplit-dwarf=single.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g %s 2>&1 | FileCheck %s --check-prefix=SINGLE
 
-// SINGLE: "-debug-info-kind=limited"
+// SINGLE: "-debug-info-kind=constructor"
 // SINGLE: "-split-dwarf-file" "split-debug.o"
 // SINGLE-NOT: "-split-dwarf-output"
 
@@ -62,7 +62,7 @@
 
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=split -g -gno-pubnames %s 2>&1 | FileCheck %s --check-prefixes=NOPUBNAMES
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=split -g -gno-gnu-pubnames %s 2>&1 | FileCheck %s --check-prefixes=NOPUBNAMES
-// NOPUBNAMES:  "-debug-info-kind=limited"
+// NOPUBNAMES:  "-debug-info-kind=constructor"
 // NOPUBNAMES-NOT:  "-ggnu-pubnames"
 // NOPUBNAMES-SAME: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
 
Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -254,7 +254,7 @@
 
 // HAS_DEBUG-NOT: warning: debug
 // HAS_DEBUG: "-triple" "nvptx64-nvidia-cuda"
-// HAS_DEBUG-SAME: "-debug-info-kind={{limited|line-tables-only}}"
+// HAS_DEBUG-SAME: "-debug-info-kind={{constructor|line-tables-only}}"
 // HAS_DEBUG-SAME: "-dwarf-version=2"
 // HAS_DEBUG-SAME: "-fopenmp-is-device"
 // HAS_DEBUG: ptxas
Index: clang/test/Driver/myriad-toolchain.c
===
--- clang/test/Driver/myriad-toolchain.c
+++ clang/test/Driver/myriad-toolchain.c
@@ -83,7 +83,7 @@
 // NOSTDLIB-NOT: "-lc"
 
 // RUN: %clang -### -c -g %s -target sparc-myriad 2>&1 | FileCheck -check-prefix=G_SPARC %s
-// G_SPARC: "-debug-info-kind=limited" "-dwarf-version=2"
+// G_SPARC: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -c %s -target sparc-myriad-rtems -fuse-init-array 2>&1 \
 // RUN: | FileCheck -check-prefix=USE-INIT-ARRAY %s
Index: clang/test/Driver/integrated-as.s
===
--- clang/test/Driver/integrated-as.s
+++ clang/test/Driver/integrated-as.s
@@ -27,19 +27,19 @@
 // XA_INCLUDE2: "-Ifoo_dir"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-4 -gdwarf-2 2>&1 | FileCheck --check-prefix=DWARF2 %s
-// DWARF2: "-debug-info-kind=limited" "-dwarf-version=2"
+// DWARF2: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-3 2>&1 | FileCheck --check-prefix=DWARF3 %s
-// DWARF3: "-debug-info-kind=limited" "-dwarf-ver

[Lldb-commits] [PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

2021-08-03 Thread Amy Huang via Phabricator via lldb-commits
akhuang added a comment.

yep, should be good now - I'll commit it soon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106084

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


[Lldb-commits] [PATCH] D100810: Use `GNUInstallDirs` to support custom installation dirs. -- LLVM

2021-08-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd added inline comments.



Comment at: llvm/CMakeLists.txt:589
 CACHE STRING "Doxygen-generated HTML documentation install directory")
-set(LLVM_INSTALL_OCAMLDOC_HTML_DIR "share/doc/llvm/ocaml-html"
+set(LLVM_INSTALL_OCAMLDOC_HTML_DIR 
"${CMAKE_INSTALL_DOCDIR}/${project}/ocaml-html"
 CACHE STRING "OCamldoc-generated HTML documentation install directory")

Why the change from `llvm` -> `${project}`?  (not that it really makes a 
difference)



Comment at: llvm/cmake/modules/AddSphinxTarget.cmake:77
 if (CMAKE_INSTALL_MANDIR)
-  set(INSTALL_MANDIR ${CMAKE_INSTALL_MANDIR}/)
+  set(INSTALL_MANDIR "${CMAKE_INSTALL_MANDIR}/")
 else()

Nit: trailing slash is unnecessary, `CMAKE_INSTALL_MANDIR` should be defined, 
and if not, you do not want installation into `/` anyway.



Comment at: llvm/cmake/modules/CMakeLists.txt:3
 
-set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
+set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm CACHE STRING 
"Path for CMake subdirectory (defaults to 'cmake/llvm')")
 set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")

Why is this variable being put into the cache now?



Comment at: llvm/cmake/modules/LLVMInstallSymlink.cmake:7
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}/")
+  set(bindir "${DESTDIR}${outdir}/")
 

Nit: trailing slash shouldn't be there.



Comment at: llvm/tools/llvm-config/llvm-config.cpp:361
+{
+  SmallString<256> Path(StringRef(LLVM_INSTALL_INCLUDEDIR));
+  sys::fs::make_absolute(ActivePrefix, Path);

Why the temporary `StringRef`?  Can you not just initialize `Path` with the 
literal?



Comment at: llvm/tools/llvm-config/llvm-config.cpp:366
+{
+  SmallString<256> Path(StringRef(LLVM_INSTALL_BINDIR));
+  sys::fs::make_absolute(ActivePrefix, Path);

Similar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

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


[Lldb-commits] [PATCH] D106466: [llvm+lldb] 2/2: Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

2021-08-03 Thread Igor Kudrin via Phabricator via lldb-commits
ikudrin added a comment.

As far as I understand it, you need a specially constructed 
`llvm::DWARFDebugRnglistTable` object so that 
`DWARFUnit::FindRnglistFromOffset()` can call its `findList()` method and get a 
list of records located at a specific offset. It looks like a newly created 
`llvm::DWARFDebugRnglistTable` object has its `Header.Length` set to `0`, so it 
already works as required for the usage, and there is no need to fill its 
fields with artificial values that do not represent real content of the 
section. Am I right?




Comment at: llvm/lib/DebugInfo/DWARF/DWARFListTable.cpp:54
+
+  Data.setAddressSize(HeaderData.AddrSize);
+  return Error::success();

The line looks suspicious because `Data` is a local variable that is destroyed 
right after the statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106466

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


[Lldb-commits] [lldb] d2b2ab4 - [lldb] Further constrain a test that fails without python enabled

2021-08-03 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2021-08-03T11:50:36-07:00
New Revision: d2b2ab4e1c347161b622b9dc2553de04f9f82f04

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

LOG: [lldb] Further constrain a test that fails without python enabled

The test relies on the python embedded interpreter being available and
fails otherwise.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/split-optimized.c

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/split-optimized.c 
b/lldb/test/Shell/SymbolFile/DWARF/split-optimized.c
index f3a0990615fe..b024c22f88b8 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/split-optimized.c
+++ b/lldb/test/Shell/SymbolFile/DWARF/split-optimized.c
@@ -3,6 +3,9 @@
 // -gsplit-dwarf is supported only on Linux.
 // REQUIRES: system-linux
 
+// This test uses lldb's embedded python interpreter
+// REQUIRES: python
+
 // ObjectFileELF::ApplyRelocations does not implement arm32.
 // XFAIL: target-arm && linux-gnu
 



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


[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

2021-08-03 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev created this revision.
OmarEmaraDev added a reviewer: clayborg.
Herald added a reviewer: teemperor.
OmarEmaraDev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a breakpoints window that lists all breakpoints and
breakpoints locations. The window is implemented as a tree, where the
first level is the breakpoints and the second level is breakpoints
locations.

The tree delegate was hardcoded to only draw when there is a process,
which is not necessary for breakpoints, so the relevant logic was
abstracted in the TreeDelegateShouldDraw method.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107386

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -3772,9 +3772,14 @@
TreeItem *&selected_item) {
 return;
   }
-  virtual bool TreeDelegateItemSelected(
-  TreeItem &item) = 0; // Return true if we need to update views
+  // This is invoked when a tree item is selected. If true is returned, the
+  // views are updated.
+  virtual bool TreeDelegateItemSelected(TreeItem &item) = 0;
   virtual bool TreeDelegateExpandRootByDefault() { return false; }
+  // This is mostly useful for root tree delegates. If false is returned,
+  // drawing will be skipped completely. This is needed, for instance, in
+  // skipping drawing of the threads tree if there is no running process.
+  virtual bool TreeDelegateShouldDraw() { return true; }
 };
 
 typedef std::shared_ptr TreeDelegateSP;
@@ -3989,21 +3994,6 @@
   int NumVisibleRows() const { return m_max_y - m_min_y; }
 
   bool WindowDelegateDraw(Window &window, bool force) override {
-ExecutionContext exe_ctx(
-m_debugger.GetCommandInterpreter().GetExecutionContext());
-Process *process = exe_ctx.GetProcessPtr();
-
-bool display_content = false;
-if (process) {
-  StateType state = process->GetState();
-  if (StateIsStoppedState(state, true)) {
-// We are stopped, so it is ok to
-display_content = true;
-  } else if (StateIsRunningState(state)) {
-return true; // Don't do any updating when we are running
-  }
-}
-
 m_min_x = 2;
 m_min_y = 1;
 m_max_x = window.GetWidth() - 1;
@@ -4012,35 +4002,36 @@
 window.Erase();
 window.DrawTitleBox(window.GetName());
 
-if (display_content) {
-  const int num_visible_rows = NumVisibleRows();
-  m_num_rows = 0;
-  m_root.CalculateRowIndexes(m_num_rows);
-  m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
- m_selected_item);
-
-  // If we unexpanded while having something selected our total number of
-  // rows is less than the num visible rows, then make sure we show all the
-  // rows by setting the first visible row accordingly.
-  if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
-m_first_visible_row = 0;
-
-  // Make sure the selected row is always visible
-  if (m_selected_row_idx < m_first_visible_row)
-m_first_visible_row = m_selected_row_idx;
-  else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
-m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
-
-  int row_idx = 0;
-  int num_rows_left = num_visible_rows;
-  m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
-  num_rows_left);
-  // Get the selected row
-  m_selected_item = m_root.GetItemForRowIndex(m_selected_row_idx);
-} else {
+if (!m_delegate_sp->TreeDelegateShouldDraw()) {
   m_selected_item = nullptr;
+  return true;
 }
 
+const int num_visible_rows = NumVisibleRows();
+m_num_rows = 0;
+m_root.CalculateRowIndexes(m_num_rows);
+m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
+   m_selected_item);
+
+// If we unexpanded while having something selected our total number of
+// rows is less than the num visible rows, then make sure we show all the
+// rows by setting the first visible row accordingly.
+if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
+  m_first_visible_row = 0;
+
+// Make sure the selected row is always visible
+if (m_selected_row_idx < m_first_visible_row)
+  m_first_visible_row = m_selected_row_idx;
+else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
+  m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
+
+int row_idx = 0;
+int num_rows_left = num_visible_rows;
+m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
+num_rows_left);
+// Get the sel

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

2021-08-03 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added a comment.
Herald added a subscriber: JDevlieghere.

I faced a bit of difficulty in this patch, so it is not exactly complete. One 
issue is that breakpoint descriptions can be very long, I tried to implemented 
multiline items to workaround this, but that didn't work out and was a waste of 
time. For now, I just tried to keep descriptions as small as possible. One 
thing I want to try now is to simply add another tree level and add all 
necessary information there, similar to verbose breakpoint description.
This is also missing the breakpoint operators, like enabling and disabling 
breakpoints. Since the window doesn't delegate key handing to the items, this 
will have to be implemented first.

F18327498: 20210803-28.png <https://reviews.llvm.org/F18327498>

F18327497: 20210803-211059.png <https://reviews.llvm.org/F18327497>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107386

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


[Lldb-commits] [lldb] 4367cba - [lldb] Move comment about noindex next to line it refers to

2021-08-03 Thread Nico Weber via lldb-commits

Author: Nico Weber
Date: 2021-08-03T22:14:12+02:00
New Revision: 4367cbab4cf2cbf8f81a24bdcc2dc0612b932aa5

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

LOG: [lldb] Move comment about noindex next to line it refers to

The comment was originally added in 34769d80d. Then D44526
removed the flag added there (but kept the comment), and then
D66966 reintroduced a .noindex dir (which D68606 and then 33fca97880
moved around a bit).

No behavior change.

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

Added: 


Modified: 
lldb/test/API/CMakeLists.txt
lldb/test/CMakeLists.txt

Removed: 




diff  --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index 7625a4e5e29c..3124ca602e8e 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -37,11 +37,6 @@ set(LLDB_TEST_USER_ARGS
   ""
   CACHE STRING "Specify additional arguments to pass to test runner. For 
example: '-C gcc -C clang -A i386 -A x86_64'")
 
-# The .noindex suffix is a marker for Spotlight to never index the
-# build directory.  LLDB queries Spotlight to locate .dSYM bundles
-# based on the UUID embedded in a binary, and because the UUID is a
-# hash of filename and .text section, there *will* be conflicts inside
-# the build directory.
 set(LLDB_TEST_COMMON_ARGS
   -u CXXFLAGS
   -u CFLAGS

diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 584cacbb99c1..ca60295a0054 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -12,6 +12,11 @@ if(LLDB_BUILT_STANDALONE)
 endif()
 
 # Configure the build directory.
+# The .noindex suffix is a marker for Spotlight to never index the
+# build directory.  LLDB queries Spotlight to locate .dSYM bundles
+# based on the UUID embedded in a binary, and because the UUID is a
+# hash of filename and .text section, there *will* be conflicts inside
+# the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" 
CACHE PATH "The build root for building tests.")
 
 # Configure and create module cache directories.



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


[Lldb-commits] [PATCH] D107341: [lldb] Move comment about noindex next to line it refers to

2021-08-03 Thread Nico Weber via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4367cbab4cf2: [lldb] Move comment about noindex next to line 
it refers to (authored by thakis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107341

Files:
  lldb/test/API/CMakeLists.txt
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -12,6 +12,11 @@
 endif()
 
 # Configure the build directory.
+# The .noindex suffix is a marker for Spotlight to never index the
+# build directory.  LLDB queries Spotlight to locate .dSYM bundles
+# based on the UUID embedded in a binary, and because the UUID is a
+# hash of filename and .text section, there *will* be conflicts inside
+# the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" 
CACHE PATH "The build root for building tests.")
 
 # Configure and create module cache directories.
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -37,11 +37,6 @@
   ""
   CACHE STRING "Specify additional arguments to pass to test runner. For 
example: '-C gcc -C clang -A i386 -A x86_64'")
 
-# The .noindex suffix is a marker for Spotlight to never index the
-# build directory.  LLDB queries Spotlight to locate .dSYM bundles
-# based on the UUID embedded in a binary, and because the UUID is a
-# hash of filename and .text section, there *will* be conflicts inside
-# the build directory.
 set(LLDB_TEST_COMMON_ARGS
   -u CXXFLAGS
   -u CFLAGS


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -12,6 +12,11 @@
 endif()
 
 # Configure the build directory.
+# The .noindex suffix is a marker for Spotlight to never index the
+# build directory.  LLDB queries Spotlight to locate .dSYM bundles
+# based on the UUID embedded in a binary, and because the UUID is a
+# hash of filename and .text section, there *will* be conflicts inside
+# the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" CACHE PATH "The build root for building tests.")
 
 # Configure and create module cache directories.
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -37,11 +37,6 @@
   ""
   CACHE STRING "Specify additional arguments to pass to test runner. For example: '-C gcc -C clang -A i386 -A x86_64'")
 
-# The .noindex suffix is a marker for Spotlight to never index the
-# build directory.  LLDB queries Spotlight to locate .dSYM bundles
-# based on the UUID embedded in a binary, and because the UUID is a
-# hash of filename and .text section, there *will* be conflicts inside
-# the build directory.
 set(LLDB_TEST_COMMON_ARGS
   -u CXXFLAGS
   -u CFLAGS
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] bf33835 - [lldb] Get rid of HAVE_SYS_TYPES_H

2021-08-03 Thread Nico Weber via lldb-commits

Author: Nico Weber
Date: 2021-08-03T22:14:56+02:00
New Revision: bf3383501fefc6cfe0d8f313a2814f7fa34d4492

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

LOG: [lldb] Get rid of HAVE_SYS_TYPES_H

LLVM includes this header unconditionally on all platforms
(including Windows), so this define should no longer be necessary.

No behavior change.

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

Added: 


Modified: 
lldb/cmake/modules/LLDBGenerateConfig.cmake
lldb/include/lldb/Host/Config.h.cmake
lldb/include/lldb/Host/windows/PosixApi.h
lldb/source/Expression/UserExpression.cpp
lldb/source/Expression/UtilityFunction.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
lldb/tools/driver/Platform.h

Removed: 




diff  --git a/lldb/cmake/modules/LLDBGenerateConfig.cmake 
b/lldb/cmake/modules/LLDBGenerateConfig.cmake
index 646720a74098..94332656b28c 100644
--- a/lldb/cmake/modules/LLDBGenerateConfig.cmake
+++ b/lldb/cmake/modules/LLDBGenerateConfig.cmake
@@ -12,7 +12,6 @@ set(CMAKE_REQUIRED_DEFINITIONS)
 check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
 
 check_include_file(termios.h HAVE_TERMIOS_H)
-check_include_file("sys/types.h" HAVE_SYS_TYPES_H)
 check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)
 
 check_cxx_symbol_exists(process_vm_readv "sys/uio.h" HAVE_PROCESS_VM_READV)

diff  --git a/lldb/include/lldb/Host/Config.h.cmake 
b/lldb/include/lldb/Host/Config.h.cmake
index f691ef3abb8f..777a6d1be541 100644
--- a/lldb/include/lldb/Host/Config.h.cmake
+++ b/lldb/include/lldb/Host/Config.h.cmake
@@ -13,9 +13,6 @@
 
 #cmakedefine01 LLDB_HAVE_EL_RFUNC_T
 
-
-#cmakedefine01 HAVE_SYS_TYPES_H
-
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL

diff  --git a/lldb/include/lldb/Host/windows/PosixApi.h 
b/lldb/include/lldb/Host/windows/PosixApi.h
index 26398ac5eb11..87908209d716 100644
--- a/lldb/include/lldb/Host/windows/PosixApi.h
+++ b/lldb/include/lldb/Host/windows/PosixApi.h
@@ -56,14 +56,12 @@
 #define S_IRWXO 0
 #endif
 
-#if HAVE_SYS_TYPES_H
 // pyconfig.h typedefs this.  We require python headers to be included before
 // any LLDB headers, but there's no way to prevent python's pid_t definition
 // from leaking, so this is the best option.
 #ifndef NO_PID_T
 #include 
 #endif
-#endif // HAVE_SYS_TYPES_H
 
 #ifdef _MSC_VER
 

diff  --git a/lldb/source/Expression/UserExpression.cpp 
b/lldb/source/Expression/UserExpression.cpp
index eac89c24bc1e..b61781c0b82b 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -6,12 +6,8 @@
 //
 
//===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 

diff  --git a/lldb/source/Expression/UtilityFunction.cpp 
b/lldb/source/Expression/UtilityFunction.cpp
index d7a89a8e1446..1a4df9722706 100644
--- a/lldb/source/Expression/UtilityFunction.cpp
+++ b/lldb/source/Expression/UtilityFunction.cpp
@@ -6,13 +6,8 @@
 //
 
//===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
-
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 1b205b13113b..41e86a1f897e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -6,12 +6,8 @@
 //
 
//===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
index a78116352c2e..363ca2ec54cc 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
@@ -6,8 +6,6 @@
 //
 
//===--===//
 
-#include "lldb/Host/Config.h"
-
 #include "ClangUtilityFunction.h"
 #include "ClangExpressionDeclMap.h"
 #include "ClangExpressionParser.h"
@@ -15,9 +13,7 @@
 #include "ClangPersistentVariables.h"
 
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 
 #include "lldb/Core/Module.h"

diff  --git a/lldb/tools/driver/Platform.h b/lldb/tools/driver/Platform.h
index d7573b75bf32..ff017c44

[Lldb-commits] [PATCH] D107338: [lldb] Get rid of HAVE_SYS_TYPES_H

2021-08-03 Thread Nico Weber via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbf3383501fef: [lldb] Get rid of HAVE_SYS_TYPES_H (authored 
by thakis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107338

Files:
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/windows/PosixApi.h
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Expression/UtilityFunction.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/tools/driver/Platform.h

Index: lldb/tools/driver/Platform.h
===
--- lldb/tools/driver/Platform.h
+++ lldb/tools/driver/Platform.h
@@ -9,19 +9,16 @@
 #ifndef LLDB_TOOLS_DRIVER_PLATFORM_H
 #define LLDB_TOOLS_DRIVER_PLATFORM_H
 
-#include "lldb/Host/Config.h"
-
 #if defined(_WIN32)
 
 #include 
 #if defined(_MSC_VER)
 #include 
 #endif
-#if HAVE_SYS_TYPES_H
-#include 
-#endif
+
 #include "lldb/Host/windows/windows.h"
 #include 
+#include 
 
 struct winsize {
   long ws_col;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
@@ -6,8 +6,6 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include "ClangUtilityFunction.h"
 #include "ClangExpressionDeclMap.h"
 #include "ClangExpressionParser.h"
@@ -15,9 +13,7 @@
 #include "ClangPersistentVariables.h"
 
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 
 #include "lldb/Core/Module.h"
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -6,12 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 
Index: lldb/source/Expression/UtilityFunction.cpp
===
--- lldb/source/Expression/UtilityFunction.cpp
+++ lldb/source/Expression/UtilityFunction.cpp
@@ -6,13 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
-
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -6,12 +6,8 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
-
 #include 
-#if HAVE_SYS_TYPES_H
 #include 
-#endif
 
 #include 
 #include 
Index: lldb/include/lldb/Host/windows/PosixApi.h
===
--- lldb/include/lldb/Host/windows/PosixApi.h
+++ lldb/include/lldb/Host/windows/PosixApi.h
@@ -56,14 +56,12 @@
 #define S_IRWXO 0
 #endif
 
-#if HAVE_SYS_TYPES_H
 // pyconfig.h typedefs this.  We require python headers to be included before
 // any LLDB headers, but there's no way to prevent python's pid_t definition
 // from leaking, so this is the best option.
 #ifndef NO_PID_T
 #include 
 #endif
-#endif // HAVE_SYS_TYPES_H
 
 #ifdef _MSC_VER
 
Index: lldb/include/lldb/Host/Config.h.cmake
===
--- lldb/include/lldb/Host/Config.h.cmake
+++ lldb/include/lldb/Host/Config.h.cmake
@@ -13,9 +13,6 @@
 
 #cmakedefine01 LLDB_HAVE_EL_RFUNC_T
 
-
-#cmakedefine01 HAVE_SYS_TYPES_H
-
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL
Index: lldb/cmake/modules/LLDBGenerateConfig.cmake
===
--- lldb/cmake/modules/LLDBGenerateConfig.cmake
+++ lldb/cmake/modules/LLDBGenerateConfig.cmake
@@ -12,7 +12,6 @@
 check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
 
 check_include_file(termios.h HAVE_TERMIOS_H)
-check_include_file("sys/types.h" HAVE_SYS_TYPES_H)
 check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)
 
 check_cxx_symbol_exists(process_vm_readv "sys/uio.h" HAVE_PROCESS_VM_READV)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D107295: [lldb] Use a struct to pass function search options to Module::FindFunction

2021-08-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 363887.

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

https://reviews.llvm.org/D107295

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp

Index: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
===
--- lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
+++ lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
@@ -8,12 +8,13 @@
 
 #include "InferiorCallPOSIX.h"
 #include "lldb/Core/Address.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Host/Config.h"
-#include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
@@ -41,12 +42,13 @@
   if (thread == nullptr)
 return false;
 
-  const bool include_symbols = true;
-  const bool include_inlines = false;
+  ModuleFunctionSearchOptions function_options;
+  function_options.include_symbols = true;
+  function_options.include_inlines = false;
+
   SymbolContextList sc_list;
   process->GetTarget().GetImages().FindFunctions(
-  ConstString("mmap"), eFunctionNameTypeFull, include_symbols,
-  include_inlines, sc_list);
+  ConstString("mmap"), eFunctionNameTypeFull, function_options, sc_list);
   const uint32_t count = sc_list.GetSize();
   if (count > 0) {
 SymbolContext sc;
@@ -135,12 +137,13 @@
   if (thread == nullptr)
 return false;
 
-  const bool include_symbols = true;
-  const bool include_inlines = false;
+  ModuleFunctionSearchOptions function_options;
+  function_options.include_symbols = true;
+  function_options.include_inlines = false;
+
   SymbolContextList sc_list;
   process->GetTarget().GetImages().FindFunctions(
-  ConstString("munmap"), eFunctionNameTypeFull, include_symbols,
-  include_inlines, sc_list);
+  ConstString("munmap"), eFunctionNameTypeFull, function_options, sc_list);
   const uint32_t count = sc_list.GetSize();
   if (count > 0) {
 SymbolContext sc;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1220,22 +1220,25 @@
 }
   }
 
-  const bool include_inlines = false;
   SymbolContextList sc_list;
   if (namespace_decl && module_sp) {
-const bool include_symbols = false;
+ModuleFunctionSearchOptions function_options;
+function_options.include_inlines = false;
+function_options.include_symbols = false;
 
 module_sp->FindFunctions(name, namespace_decl, eFunctionNameTypeBase,
- include_symbols, include_inlines, sc_list);
+ function_options, sc_list);
   } else if (target && !namespace_decl) {
-const bool include_symbols = true;
+ModuleFunctionSearchOptions function_options;
+function_options.include_inlines = false;
+function_options.include_symbols = true;
 
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
 target->GetImages().FindFunctions(
-name, eFunctionNameTypeFull | eFunctionNameTypeBase, include_symbols,
-include_inlines, sc_list);
+name, eFunctionNameTypeFull | eFunctionNameTypeBase, function_options,
+sc_list);
   }
 
   // If we found more than one function, see if we can use the frame's decl
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -974,8 +974,9 @@
interface_decl->getName(), selector_name);
   SymbolContextList sc_list;
 
-  const bool include_symbols = false;
-  const bool include_inlines = false;
+  ModuleFunctionSearchOptions function_options;
+  function_options.include_symbols = false;
+  function_options.include_inlines

[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization

2021-08-03 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added a comment.

In D105741#2920588 , @vsk wrote:

> Hey @jj10306, thanks for working on this.
>
> @wallace @clayborg -- It seems like there are a ton of logic changes 
> introduced in this patch that are tested at too coarse a level. I'm not 
> confident in the changes being made here. For example, there's a bunch of 
> subtle work being done in `BasicSuperBlockMerge`, but only ~one opaque 
> high-level check that attempts to validate any of the logic: 
> `self.assertTrue(num_units_by_layer[1] == 383)`. Where does 383 come from? 
> How do we know that that's the right result? If there's a bug in 
> `BasicSuperBlockMerge`, would the regression test just look like changing the 
> magic 383 number?
>
> I'm not really comfortable with this being checked in, and would suggest a 
> revert until some more granular unit tests can be added to the patch. (Also 
> paging @JDevlieghere in case he has thoughts on the testing.)

Apologies for this - will submit a patch with more comprehensive tests and 
other minor fixes this week!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105741

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


[Lldb-commits] [lldb] b9139ac - Fix expression evaluation result expansion in lldb-vscode

2021-08-03 Thread Jeffrey Tan via lldb-commits

Author: Jeffrey Tan
Date: 2021-08-03T15:24:44-07:00
New Revision: b9139acb85a4e5203364838e47b2e1ee90055ad3

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

LOG: Fix expression evaluation result expansion in lldb-vscode

VScode now sends a "scopes" DAP request immediately after any expression 
evaluation.
This scopes request would clear and invalidate any non-scoped expandable 
variables in g_vsc.variables, causing later "variables" request to return empty 
result.
The symptom is that any expandable variables in VScode watch window/debug 
console UI to return empty content.

This diff fixes this issue by only clearing the expandable variables at process 
continue time. To achieve this, we have to repopulate all scoped variables
during context switch for each "scopes" request without clearing global 
expandable variables.
So the PR puts the scoped variables into its own locals/globals/registers; and 
all expandable variables into separate "expandableVariables" list.
Also, instead of using the variable index for "variableReference", it generates 
a new variableReference id each time as the key of "expandableVariables".

As a further new feature, this PR adds a new "expandablePermanentVariables" 
which has the lifetime of debug session. Any expandable variables from debug 
console
are added into this list. This enables users to snapshot expanable old variable 
in debug console and compare with new variables if desire.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py 
b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index 765cfbe6ed5a..092d96edf4eb 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -23,7 +23,7 @@ class 
TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-def verify_values(self, verify_dict, actual, varref_dict=None):
+def verify_values(self, verify_dict, actual, varref_dict=None, 
expression=None):
 if 'equals' in verify_dict:
 verify = verify_dict['equals']
 for key in verify:
@@ -48,9 +48,13 @@ def verify_values(self, verify_dict, actual, 
varref_dict=None):
 if hasVariablesReference:
 # Remember variable references in case we want to test further
 # by using the evaluate name.
-varRef = actual['variablesReference']
+varRef = actual["variablesReference"]
 if varRef != 0 and varref_dict is not None:
-varref_dict[actual['evaluateName']] = varRef
+if expression is None:
+evaluateName = actual["evaluateName"]
+else:
+evaluateName = expression
+varref_dict[evaluateName] = varRef
 if ('hasVariablesReference' in verify_dict and
 verify_dict['hasVariablesReference']):
 self.assertTrue(hasVariablesReference,
@@ -282,3 +286,111 @@ def test_scopes_variables_setVariable_evaluate(self):
 self.assertNotIn('x @ main.cpp:21', names)
 
 self.verify_variables(verify_locals, locals)
+
+@skipIfWindows
+@skipIfRemote
+def test_scopes_and_evaluate_expansion(self):
+"""
+Tests the evaluated expression expands successfully after "scopes" 
packets
+and permanent expressions persist.
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+breakpoint1_line = line_number(source, "// breakpoint 1")
+lines = [breakpoint1_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(
+len(breakpoint_ids), len(lines), "expect correct number of 
breakpoints"
+)
+self.continue_to_breakpoints(breakpoint_ids)
+
+# Verify locals
+locals = self.vscode.get_local_variables()
+buffer_children = make_buffer_verify_dict(0, 32)
+verify_locals = {
+"argc": {"equals": {"type": "int", "value": "1"}},
+"argv": {
+"equals": {"type": "const char **"},
+"startswith": {"value": "0x"},
+"hasVariablesReference": True,
+},
+"pt": {
+"equals": {"t

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-08-03 Thread jeffrey tan via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9139acb85a4: Fix expression evaluation result expansion in 
lldb-vscode (authored by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105166

Files:
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -107,6 +107,19 @@
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
+lldb::SBValueList *GetTopLevelScope(int64_t variablesReference) {
+  switch (variablesReference) {
+  case VARREF_LOCALS:
+return &g_vsc.variables.locals;
+  case VARREF_GLOBALS:
+return &g_vsc.variables.globals;
+  case VARREF_REGS:
+return &g_vsc.variables.registers;
+  default:
+return nullptr;
+  }
+}
+
 SOCKET AcceptConnection(int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
@@ -439,6 +452,7 @@
 }
 break;
   case lldb::eStateRunning:
+g_vsc.WillContinue();
 break;
   case lldb::eStateExited: {
 // Run any exit LLDB commands the user specified in the
@@ -1196,6 +1210,10 @@
 lldb::SBValue value = frame.GetValueForVariablePath(
 expression.data(), lldb::eDynamicDontRunTarget);
 
+// Freeze dry the value in case users expand it later in the debug console
+if (value.GetError().Success() && context == "repl")
+  value = value.Persist();
+
 if (value.GetError().Fail() && context != "hover")
   value = frame.EvaluateExpression(expression.data());
 
@@ -1215,9 +1233,9 @@
   EmplaceSafeString(body, "type",
 value_typename ? value_typename : NO_TYPENAME);
   if (value.MightHaveChildren()) {
-auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
-g_vsc.variables.Append(value);
-body.try_emplace("variablesReference", variablesReference);
+auto variableReference = g_vsc.variables.InsertExpandableVariable(
+value, /*is_permanent=*/context == "repl");
+body.try_emplace("variablesReference", variableReference);
   } else {
 body.try_emplace("variablesReference", (int64_t)0);
   }
@@ -1895,20 +1913,15 @@
 frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
 frame.GetThread().SetSelectedFrame(frame.GetFrameID());
   }
-  g_vsc.variables.Clear();
-  g_vsc.variables.Append(frame.GetVariables(true,   // arguments
-true,   // locals
-false,  // statics
-true)); // in_scope_only
-  g_vsc.num_locals = g_vsc.variables.GetSize();
-  g_vsc.variables.Append(frame.GetVariables(false,  // arguments
-false,  // locals
-true,   // statics
-true)); // in_scope_only
-  g_vsc.num_globals = g_vsc.variables.GetSize() - (g_vsc.num_locals);
-  g_vsc.variables.Append(frame.GetRegisters());
-  g_vsc.num_regs =
-  g_vsc.variables.GetSize() - (g_vsc.num_locals + g_vsc.num_globals);
+  g_vsc.variables.locals = frame.GetVariables(/*arguments=*/true,
+  /*locals=*/true,
+  /*statics=*/false,
+  /*in_scope_only=*/true);
+  g_vsc.variables.globals = frame.GetVariables(/*arguments=*/false,
+   /*locals=*/false,
+   /*statics=*/true,
+   /*in_scope_only=*/true);
+  g_vsc.variables.registers = frame.GetRegisters();
   body.try_emplace("scopes", g_vsc.CreateTopLevelScopes());
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2750,37 +2763,20 @@
   // of the variable within the g_vsc.variables list.
   const auto id_value = GetUnsigned(arguments, "id", UINT64_MAX);
   if (id_value != UINT64_MAX) {
-variable = g_vsc.variables.GetValueAtIndex(id_value);
-  } else if (VARREF_IS_SCOPE(variablesReference)) {
+variable = g_vsc.variables.GetVariable(id_value);
+  } else if (lldb::SBValueList *top_scope =
+ GetTopLevelScope(variablesReference)) {
 // variablesReference is one of our scopes, not an actual vari

[Lldb-commits] [PATCH] D107407: [rfc] target stats

2021-08-03 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added reviewers: clayborg, jingham.
Herald added subscribers: dang, arphaman, mgorny.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

D45547  added a while ago the skeleton for 
some target stats, along with
a command to enable, disable, and dump them. However, it seems it wasn't
further developed.

My team is in need of a bunch of other target stats so that we can fix some 
bottlenecks. Some of these stats are related to expressions (e.g. # of expr 
eval that trigger global lookups, time per expr eval), and some others are are 
related to modules (e.g. amount of debug info parsed, time spent indexing 
dwarf, etc.).

In order to collect this metrics, I'm proposing improving the existing
code and create a TargetStats class, that can keep track of them.

Some things to consider:

- I think it's useful to have colletion enabled by default. You might

encounter some perf issues and you might want to dump the stats right
away, instead of rerunning the debug session but this time with
collection enabled.

- The information that is very cheap to collect, should be collected on

the fly, e.g. # of failed frame vars.

- The information that is expensive to collect, should be collected at

retrieval time (e.g. when invoking ToJSON or Dump). That way the
collection can be enabled by default without paying any noticeable
penalty.

As an interesting case, I added a metric for the amount of time spent indexing 
dwarf per module, which is not as trivial as the other existing metrics because 
the Target is not available there. However, it was easy to implement and can be 
extended to all symbol files. This metric is collected at retrieval time. This 
doesn't account for cases in which a dynamic library is unloaded at runtime, 
but I'm just making the current changes just for demostration purposes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107407

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/TargetStats.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectStats.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetStats.cpp

Index: lldb/source/Target/TargetStats.cpp
===
--- /dev/null
+++ lldb/source/Target/TargetStats.cpp
@@ -0,0 +1,101 @@
+//===-- Target.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Target/TargetStats.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/SymbolFile.h"
+#include "lldb/Target/Target.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace llvm;
+
+void TargetStats::Operation::Notify(bool success) {
+  if (success)
+successes++;
+  else
+failures++;
+}
+
+void TargetStats::NotifyExprEval(bool success) { m_expr_eval.Notify(success); }
+
+void TargetStats::NotifyFrameVar(bool success) { m_frame_var.Notify(success); }
+
+void TargetStats::CollectModuleStats(Target &target) {
+  for (ModuleSP module_sp : target.GetImages().Modules()) {
+m_module_stats[module_sp->GetFileSpec().GetPath()] = {
+module_sp->GetSymbolFile()->GetSymbolLoadingTime()};
+  }
+}
+
+void TargetStats::Dump(Stream &s, Target &target) {
+  CollectModuleStats(target);
+  s << llvm::formatv("Number of expr evaluation successes: {0}\n",
+ m_expr_eval.successes)
+   .str();
+  s << llvm::formatv("Number of expr evaluation failures: {0}\n",
+ m_expr_eval.failures)
+   .str();
+  s << llvm::formatv("Number of frame var successes: {0}\n",
+ m_frame_var.successes)
+   .str();
+  s << llvm::formatv("Number of frame var failures: {0}\n",
+ m_frame_var.failures)
+   .str();
+
+  s << "Modules: \n";
+
+  std::for_each(m_module_stats.begin(), m_module_stats.end(),
+[&](const std::pair &module_entry) {
+  s << "  " << module_entry.first << ":\n";
+  s << "Symbol loading time in seconds: ";
+  if (module_e

[Lldb-commits] [PATCH] D107206: [lldb] Refactor IRExecutionUnit::FindInSymbols (NFC)

2021-08-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 363908.
JDevlieghere added a comment.

Convert lambda into a stateful class.


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

https://reviews.llvm.org/D107206

Files:
  lldb/source/Expression/IRExecutionUnit.cpp

Index: lldb/source/Expression/IRExecutionUnit.cpp
===
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -769,133 +769,125 @@
   }
 }
 
-lldb::addr_t IRExecutionUnit::FindInSymbols(
-const std::vector &specs,
-const lldb_private::SymbolContext &sc,
-bool &symbol_was_missing_weak) {
-  symbol_was_missing_weak = false;
-  Target *target = sc.target_sp.get();
-
-  if (!target) {
-// we shouldn't be doing any symbol lookup at all without a target
-return LLDB_INVALID_ADDRESS;
-  }
+class LoadAddressResolver {
+public:
+  LoadAddressResolver(Target *target, bool &symbol_was_missing_weak)
+  : m_target(target), m_symbol_was_missing_weak(symbol_was_missing_weak) {}
 
-  for (const SearchSpec &spec : specs) {
-SymbolContextList sc_list;
-
-lldb::addr_t best_internal_load_address = LLDB_INVALID_ADDRESS;
-
-std::function
-get_external_load_address = [&best_internal_load_address, target,
- &symbol_was_missing_weak](
-lldb::addr_t &load_address, SymbolContextList &sc_list,
-const lldb_private::SymbolContext &sc) -> lldb::addr_t {
-  load_address = LLDB_INVALID_ADDRESS;
-
-  if (sc_list.GetSize() == 0)
-return false;
-
-  // missing_weak_symbol will be true only if we found only weak undefined 
-  // references to this symbol.
-  symbol_was_missing_weak = true;  
-  for (auto candidate_sc : sc_list.SymbolContexts()) {
-// Only symbols can be weak undefined:
-if (!candidate_sc.symbol)
-  symbol_was_missing_weak = false;
-else if (candidate_sc.symbol->GetType() != lldb::eSymbolTypeUndefined
-  || !candidate_sc.symbol->IsWeak())
-  symbol_was_missing_weak = false;
-
-const bool is_external =
-(candidate_sc.function) ||
-(candidate_sc.symbol && candidate_sc.symbol->IsExternal());
-if (candidate_sc.symbol) {
-  load_address = candidate_sc.symbol->ResolveCallableAddress(*target);
-
-  if (load_address == LLDB_INVALID_ADDRESS) {
-if (target->GetProcessSP())
-  load_address =
-  candidate_sc.symbol->GetAddress().GetLoadAddress(target);
-else
-  load_address = candidate_sc.symbol->GetAddress().GetFileAddress();
-  }
-}
+  llvm::Optional Resolve(SymbolContextList &sc_list) {
+if (sc_list.IsEmpty())
+  return llvm::None;
 
-if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) {
-  if (target->GetProcessSP())
-load_address = candidate_sc.function->GetAddressRange()
-   .GetBaseAddress()
-   .GetLoadAddress(target);
-  else
-load_address = candidate_sc.function->GetAddressRange()
-   .GetBaseAddress()
-   .GetFileAddress();
-}
+lldb::addr_t load_address = LLDB_INVALID_ADDRESS;
 
-if (load_address != LLDB_INVALID_ADDRESS) {
-  if (is_external) {
-return true;
-  } else if (best_internal_load_address == LLDB_INVALID_ADDRESS) {
-best_internal_load_address = load_address;
-load_address = LLDB_INVALID_ADDRESS;
-  }
+// Missing_weak_symbol will be true only if we found only weak undefined
+// references to this symbol.
+m_symbol_was_missing_weak = true;
+
+for (auto candidate_sc : sc_list.SymbolContexts()) {
+  // Only symbols can be weak undefined.
+  if (!candidate_sc.symbol ||
+  candidate_sc.symbol->GetType() != lldb::eSymbolTypeUndefined ||
+  !candidate_sc.symbol->IsWeak())
+m_symbol_was_missing_weak = false;
+
+  // First try the symbol.
+  if (candidate_sc.symbol) {
+load_address = candidate_sc.symbol->ResolveCallableAddress(*m_target);
+if (load_address == LLDB_INVALID_ADDRESS) {
+  Address addr = candidate_sc.symbol->GetAddress();
+  load_address = m_target->GetProcessSP()
+ ? addr.GetLoadAddress(m_target)
+ : addr.GetFileAddress();
 }
   }
 
-  // You test the address of a weak symbol against NULL to see if it is
-  // present.  So we should return 0 for a missing weak symbol.
-  if (symbol_was_missing_weak) {
-load_address = 0;
-return true;
+  // If that didn't work, try the function.
+  if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) {
+   

[Lldb-commits] [PATCH] D107206: [lldb] Refactor IRExecutionUnit::FindInSymbols (NFC)

2021-08-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Expression/IRExecutionUnit.cpp:785
+  auto get_external_load_address =
+  [target, &symbol_was_missing_weak](
+  lldb::addr_t &load_address, lldb::addr_t &best_internal_load_address,

shafik wrote:
> It feels like the lambda should really be a small `class` that tracks the 
> state of `load_address` and `best_internal_load_address` over multiple calls 
> and then the user can access the state. 
> 
> The lambda is doing a lot of logic and there is state being carried across 
> calls and that feels like we have crossed the boundary and a class would be 
> better.
Thanks, I really like that idea. Updated the diff accordingly. 


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

https://reviews.llvm.org/D107206

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


[Lldb-commits] [PATCH] D107407: [rfc] target stats

2021-08-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/include/lldb/Symbol/SymbolFile.h:302-305
+  virtual llvm::Optional> GetSymbolLoadingTime() 
{
+return llvm::None;
+  }
+

I would add an instance variable to this base class and then return it here. 
Then subclasses can just access the ivar, and clients can use this accessor to 
access it and no need to be virtual



Comment at: lldb/include/lldb/Target/TargetStats.h:43
+
+  void NotifyModuleSymbolParsing(std::chrono::duration duration);
+

Does this belong here? It seems like this is something we should gather at the 
time the stats are asked to be returned. If it does belong here, what is this 
saying? Total symbol parsing time? 



Comment at: lldb/source/API/SBTarget.cpp:213-226
 SBStructuredData SBTarget::GetStatistics() {
   LLDB_RECORD_METHOD_NO_ARGS(lldb::SBStructuredData, SBTarget, GetStatistics);
 
   SBStructuredData data;
   TargetSP target_sp(GetSP());
   if (!target_sp)
 return LLDB_RECORD_RESULT(data);

We can't replace this if this was doing something before. Was this doing 
nothing previously? Or are we doing everything this used to do and more?



Comment at: lldb/source/API/SBTarget.cpp:228-230
 void SBTarget::SetCollectingStats(bool v) {
   LLDB_RECORD_METHOD(void, SBTarget, SetCollectingStats, (bool), v);
 }

We can't change this if it did something valid prior to this diff? Or are we 
doing everything this used to do and more?



Comment at: lldb/source/API/SBTarget.cpp:234
   LLDB_RECORD_METHOD_NO_ARGS(bool, SBTarget, GetCollectingStats);
-
-  TargetSP target_sp(GetSP());
-  if (!target_sp)
-return false;
-  return target_sp->GetCollectingStats();
+  return true;
 }

We can't change this if it did something valid prior to this diff? Or are we 
doing everything this used to do and more?



Comment at: lldb/source/Commands/CommandObjectStats.cpp:16
 
-class CommandObjectStatsEnable : public CommandObjectParsed {
-public:

We should leave this command in place. Not a good idea to remote a command, if 
we do, it might make some lldb command files fail in the middle or parsing them 
and change people's script behavior.



Comment at: lldb/source/Commands/CommandObjectStats.cpp:40
-
-class CommandObjectStatsDisable : public CommandObjectParsed {
 public:

We should leave this command in place. Not a good idea to remote a command, if 
we do, it might make some lldb command files fail in the middle or parsing them 
and change people's script behavior.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:26-38
 void ManualDWARFIndex::Index() {
+  std::chrono::duration t1 =
+  std::chrono::steady_clock::now().time_since_epoch();
+  DoIndex();
+  m_loading_time = std::chrono::steady_clock::now().time_since_epoch() - t1;
+}
+

Maybe use a scope C++ class to help get the time of the index here and avoid 
making the Index() and DoIndex() calls? The "ScopedTimer" class would be 
something like:

```
class ScopedTimer {
  llvm::Optional> &m_opt_time;
  std::chrono::durationm_start;
public:
  ScopedTimer (llvm::Optional> &opt_time) : 
m_opt_time(opt_time) {
m_start = std::chrono::steady_clock::now().time_since_epoch();
  }
  ~ScopedTimer() {
m_opt_time = std::chrono::steady_clock::now().time_since_epoch() - m_start;
  }
}
```

And we should probably put this in a header file so it can be reused elsewhere.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h:58
 
+  llvm::Optional> GetSymbolLoadingTime() 
override;
+

Remove, see comment in DWARFIndex.h



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h:72
   void Index();
+  void DoIndex();
   void IndexUnit(DWARFUnit &unit, SymbolFileDWARFDwo *dwp, IndexSet &set);

Remove and see "ScopedTimer" comment



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h:86
   IndexSet m_set;
+  llvm::Optional> m_loading_time;
 };

This should be in base class.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:517-522
+llvm::Optional>
+SymbolFileDWARF::GetSymbolLoadingTime() {
+  if (!m_index)
+return llvm::None;
+  return m_index->GetSymbolLoadingTime();
+}

Remove this and just set the SymbolFile::m_index_time directly after indexing



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:323-324
 
+  llvm::Optional> GetSymbolLoadingTime() 
override;
+
 protected:

Remove, see comment in SymbolFile.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://re