[Lldb-commits] [PATCH] D84083: [lldb/ObjectFileMachO] Correctly account for resolver symbols

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84083



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

[Lldb-commits] [PATCH] D84083: [lldb/ObjectFileMachO] Correctly account for resolver symbols

[Frederic Riss via Phabricator via lldb-commits]

friss created this revision.
friss added reviewers: jasonmolenda, jingham.
Herald added a project: LLDB.

The resolver addresses stored in the dyld trie are relative to the base
of the __TEXT segment. This is usually 0 in a dylib, so this was never
noticed, but it is not 0 for most dylibs integrated in the shared cache.
As we started using the shared cache images recently as symbol source,
this causes LLDB to fail to resolve symbols which go through a runtime
resolver.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84083

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/test/API/macosx/indirect_symbol/Makefile


Index: lldb/test/API/macosx/indirect_symbol/Makefile
===
--- lldb/test/API/macosx/indirect_symbol/Makefile
+++ lldb/test/API/macosx/indirect_symbol/Makefile
@@ -8,7 +8,8 @@
 
 build-libindirect: indirect.c
$(MAKE) -f $(MAKEFILE_RULES) \
-   DYLIB_C_SOURCES=indirect.c DYLIB_NAME=indirect DYLIB_ONLY=YES
+   DYLIB_C_SOURCES=indirect.c DYLIB_NAME=indirect DYLIB_ONLY=YES \
+   LD_EXTRAS="-Wl,-image_base,0x2"
 
 build-libreepxoprt: reexport.c
$(MAKE) -f $(MAKEFILE_RULES) \
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1990,6 +1990,8 @@
   if (e.entry.flags & EXPORT_SYMBOL_FLAGS_STUB_AND_RESOLVER) {
 e.entry.other = data.GetULEB128();
 uint64_t resolver_addr = e.entry.other;
+if (text_seg_base_addr != LLDB_INVALID_ADDRESS)
+  resolver_addr += text_seg_base_addr;
 if (is_arm)
   resolver_addr &= THUMB_ADDRESS_BIT_MASK;
 resolver_addresses.insert(resolver_addr);


Index: lldb/test/API/macosx/indirect_symbol/Makefile
===
--- lldb/test/API/macosx/indirect_symbol/Makefile
+++ lldb/test/API/macosx/indirect_symbol/Makefile
@@ -8,7 +8,8 @@
 
 build-libindirect: indirect.c
 	$(MAKE) -f $(MAKEFILE_RULES) \
-		DYLIB_C_SOURCES=indirect.c DYLIB_NAME=indirect DYLIB_ONLY=YES
+		DYLIB_C_SOURCES=indirect.c DYLIB_NAME=indirect DYLIB_ONLY=YES \
+		LD_EXTRAS="-Wl,-image_base,0x2"
 
 build-libreepxoprt: reexport.c
 	$(MAKE) -f $(MAKEFILE_RULES) \
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1990,6 +1990,8 @@
   if (e.entry.flags & EXPORT_SYMBOL_FLAGS_STUB_AND_RESOLVER) {
 e.entry.other = data.GetULEB128();
 uint64_t resolver_addr = e.entry.other;
+if (text_seg_base_addr != LLDB_INVALID_ADDRESS)
+  resolver_addr += text_seg_base_addr;
 if (is_arm)
   resolver_addr &= THUMB_ADDRESS_BIT_MASK;
 resolver_addresses.insert(resolver_addr);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[walter erquinigo via Phabricator via lldb-commits]

wallace accepted this revision.
wallace added inline comments.



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:65
+@skipIfWindows
+@ skipUnlessDarwin
+@skipIfRemote  

remove this whitespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731



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

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

[Shoaib Meenai via Phabricator via lldb-commits]

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

Oops, missed that I'd become a blocking reviewer for this (cos of requesting 
changes before).


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

https://reviews.llvm.org/D79219



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

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

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

In D79219#2152747 , @labath wrote:

> I wouldn't mind separate (internal) cache variable, though I am somewhat 
> surprised by this problem. A (non-cache) variable set in one of the parent 
> scopes should still take precedence over a cache variable with the same name. 
> And since config-ix.cmake is included from the top CMakeLists.txt, the value 
> it defines should be available everywhere. Was this a problem for the regular 
> build, or only for some of the more exotic build setups that don't start with 
> llvm/CMakeLists.txt ?


Never mind, it's working as expected. The problem is that we disable zlib 
detection on Windows which I missed before. I'm not sure why that's the case, I 
tested this on Windows and it seems to be working fine, but for now I've kept 
the existing behavior, we can consider enabling zlib on Windows in a follow up 
change.


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

https://reviews.llvm.org/D79219



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

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

[Petr Hosek via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c1a79dc12f3: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND 

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

[Petr Hosek via Phabricator via lldb-commits]

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

I had to revert this change because it broke bots that don't have zlib 
installed. What I haven't realized is that the shadowed variable will only be 
accessible from the same file. I could move the logic from 
`llvm/lib/Support/CMakeLists.txt` into `llvm/cmake/config-ix.cmake`, but that 
doesn't handle all the other files that need to access that variable, such as 
all the `*/test/CMakeLists.txt` files. So I don't know if shadowing is the way 
to go here. Maybe we should set a different internal cached variable 
(`HAVE_LIBZ`?) and use that to check if zlib is available. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [PATCH] D83857: Harmonize python shebang

[serge via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG515bc8c1554f: Harmonize Python shebang (authored by 
serge-sans-paille).
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc-project.
Herald added subscribers: libc-commits, openmp-commits, libcxx-commits, 
lldb-commits, Sanitizers, cfe-commits.
Herald added a reviewer: libc++.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83857

Files:
  clang/utils/clangdiag.py
  clang/utils/modfuzz.py
  compiler-rt/lib/sanitizer_common/scripts/litlint_test.py
  compiler-rt/test/sanitizer_common/android_commands/android_compile.py
  compiler-rt/test/sanitizer_common/android_commands/android_run.py
  compiler-rt/test/sanitizer_common/ios_commands/iossim_compile.py
  compiler-rt/test/sanitizer_common/ios_commands/iossim_env.py
  compiler-rt/test/sanitizer_common/ios_commands/iossim_prepare.py
  compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py
  debuginfo-tests/dexter/dexter.py
  debuginfo-tests/llgdb-tests/llgdb.py
  libc/AOR_v20.02/math/tools/plot.py
  libcxx/utils/google-benchmark/mingw.py
  lldb/examples/darwin/heap_find/heap.py
  lldb/examples/python/armv7_cortex_m_target_defintion.py
  lldb/examples/python/bsd.py
  lldb/examples/python/cmdtemplate.py
  lldb/examples/python/crashlog.py
  lldb/examples/python/delta.py
  lldb/examples/python/disasm-stress-test.py
  lldb/examples/python/disasm.py
  lldb/examples/python/file_extract.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/globals.py
  lldb/examples/python/lldb_module_utils.py
  lldb/examples/python/lldbtk.py
  lldb/examples/python/mach_o.py
  lldb/examples/python/memory.py
  lldb/examples/python/operating_system.py
  lldb/examples/python/performance.py
  lldb/examples/python/process_events.py
  lldb/examples/python/sbvalue.py
  lldb/examples/python/shadow.py
  lldb/examples/python/sources.py
  lldb/examples/python/stacks.py
  lldb/examples/python/symbolication.py
  lldb/examples/python/types.py
  lldb/examples/python/x86_64_linux_target_definition.py
  lldb/examples/python/x86_64_qemu_target_definition.py
  lldb/examples/python/x86_64_target_definition.py
  lldb/scripts/analyze-project-deps.py
  lldb/scripts/reproducer-replay.py
  lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py
  lldb/test/API/functionalities/plugins/python_os_plugin/operating_system2.py
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/operating_system.py
  lldb/test/Shell/helper/build.py
  lldb/third_party/Python/module/progress/progress.py
  llvm/utils/DSAclean.py
  llvm/utils/DSAextract.py
  llvm/utils/benchmark/mingw.py
  llvm/utils/docker/scripts/llvm_checksum/llvm_checksum.py
  llvm/utils/lint/common_lint.py
  llvm/utils/lint/cpp_lint.py
  llvm/utils/lint/generic_lint.py
  llvm/utils/schedcover.py
  llvm/utils/testgen/mc-bundling-x86-gen.py
  openmp/runtime/tools/summarizeStats.py
  polly/test/update_check.py
  polly/utils/jscop2cloog.py
  polly/utils/pyscop/jscop2iscc.py

Index: polly/utils/pyscop/jscop2iscc.py
===
--- polly/utils/pyscop/jscop2iscc.py
+++ polly/utils/pyscop/jscop2iscc.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 import argparse, isl, os
 import json
 
Index: polly/utils/jscop2cloog.py
===
--- polly/utils/jscop2cloog.py
+++ polly/utils/jscop2cloog.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 import argparse, os
 import json
 
Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -1,4 +1,4 @@
-#! /usr/bin/env python3
+#!/usr/bin/env python3
 # -*- coding: UTF-8 -*-
 
 # Polly/LLVM update_check.py
Index: openmp/runtime/tools/summarizeStats.py
===
--- openmp/runtime/tools/summarizeStats.py
+++ openmp/runtime/tools/summarizeStats.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 import pandas as pd
 import numpy as np
Index: llvm/utils/testgen/mc-bundling-x86-gen.py
===
--- llvm/utils/testgen/mc-bundling-x86-gen.py
+++ llvm/utils/testgen/mc-bundling-x86-gen.py
@@ -1,5 +1,5 @@
 
-#!/usr/bin/python
+#!/usr/bin/env python
 
 # Auto-generates an exhaustive and repetitive test for correct bundle-locked
 # alignment on x86.
Index: llvm/utils/schedcover.py
===
--- llvm/utils/schedcover.py
+++ llvm/utils/schedcover.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 # This creates a CSV file from the output of the debug output of subtarget:
 #   llvm-tblgen --gen-subtarget --debug-only=subtarget-emitter
Index: 

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

[Petr Hosek via Phabricator via lldb-commits]

phosek updated this revision to Diff 278216.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I wouldn't mind separate (internal) cache variable, though I am somewhat 
surprised by this problem. A (non-cache) variable set in one of the parent 
scopes should still take precedence over a cache variable with the same name. 
And since config-ix.cmake is included from the top CMakeLists.txt, the value it 
defines should be available everywhere. Was this a problem for the regular 
build, or only for some of the more exotic build setups that don't start with 
llvm/CMakeLists.txt ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

[Fangrui Song via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG53880b8cb9c6: [CMake] Make `intrinsics_gen` dependency 
unconditional. (authored by michele.scandale, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83454

Files:
  clang/CMakeLists.txt
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/Frontend/CMakeLists.txt
  clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
  clang/tools/clang-import-test/CMakeLists.txt
  clang/tools/clang-offload-bundler/CMakeLists.txt
  clang/tools/clang-offload-wrapper/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  lld/COFF/CMakeLists.txt
  lld/Common/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/MinGW/CMakeLists.txt
  lld/lib/Core/CMakeLists.txt
  lld/wasm/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt

Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt
@@ -1,8 +1,3 @@
-if(NOT LLDB_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
-
 add_lldb_library(lldbPluginRenderScriptRuntime PLUGIN
   RenderScriptRuntime.cpp
   RenderScriptExpressionOpts.cpp
@@ -10,7 +5,7 @@
   RenderScriptScriptGroup.cpp
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
 
   LINK_LIBS
 lldbBreakpoint
Index: lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
===
--- lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
+++ lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
@@ -1,7 +1,3 @@
-if(NOT LLDB_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lldb_library(lldbPluginExpressionParserClang
   ASTResultSynthesizer.cpp
   ASTStructExtractor.cpp
@@ -29,7 +25,7 @@
   NameSearchContext.cpp
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
 
   LINK_LIBS
 lldbCore
Index: lldb/source/Expression/CMakeLists.txt
===
--- lldb/source/Expression/CMakeLists.txt
+++ lldb/source/Expression/CMakeLists.txt
@@ -1,7 +1,3 @@
-if(NOT LLDB_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lldb_library(lldbExpression
   DiagnosticManager.cpp
   DWARFExpression.cpp
@@ -18,7 +14,7 @@
   UtilityFunction.cpp
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
 
   LINK_LIBS
 lldbCore
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -64,7 +64,7 @@
 # some of these generated headers. This approach is copied from Clang's main
 # CMakeLists.txt, so it should kept in sync the code in Clang which was added
 # in llvm-svn 308844.
-if(LLVM_ENABLE_MODULES AND NOT LLDB_BUILT_STANDALONE)
+if(LLVM_ENABLE_MODULES)
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 
Index: lld/wasm/CMakeLists.txt
===
--- lld/wasm/CMakeLists.txt
+++ lld/wasm/CMakeLists.txt
@@ -2,10 +2,6 @@
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
 add_public_tablegen_target(WasmOptionsTableGen)
 
-if(NOT LLD_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lld_library(lldWasm
   Driver.cpp
   InputChunks.cpp
@@ -37,5 +33,5 @@
 
   DEPENDS
   WasmOptionsTableGen
-  ${tablegen_deps}
+  intrinsics_gen
   )
Index: lld/lib/Core/CMakeLists.txt
===
--- lld/lib/Core/CMakeLists.txt
+++ lld/lib/Core/CMakeLists.txt
@@ -1,7 +1,3 @@
-if(NOT LLD_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lld_library(lldCore
   DefinedAtom.cpp
   Error.cpp
@@ -24,5 +20,5 @@
   ${LLVM_PTHREAD_LIB}
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
   )
Index: lld/MinGW/CMakeLists.txt
===
--- lld/MinGW/CMakeLists.txt
+++ lld/MinGW/CMakeLists.txt
@@ -2,10 +2,6 @@
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
 add_public_tablegen_target(MinGWOptionsTableGen)
 
-if(NOT LLD_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lld_library(lldMinGW
   Driver.cpp
 
@@ -19,5 +15,5 @@
 
   DEPENDS
   MinGWOptionsTableGen
-  ${tablegen_deps}
+  intrinsics_gen
 )
Index: lld/ELF/CMakeLists.txt
===
--- lld/ELF/CMakeLists.txt
+++ lld/ELF/CMakeLists.txt
@@ -2,10 +2,6 @@
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
 add_public_tablegen_target(ELFOptionsTableGen)
 
-if(NOT 

[Lldb-commits] [lldb] 53880b8 - [CMake] Make `intrinsics_gen` dependency unconditional.

[Fangrui Song via lldb-commits]

Author: Michele Scandale
Date: 2020-07-17T16:43:17-07:00
New Revision: 53880b8cb9c61e81457d13c0adefe51ff41664fa

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

LOG: [CMake] Make `intrinsics_gen` dependency unconditional.

The `intrinsics_gen` target exists in the CMake exports since r309389
(see LLVMConfig.cmake.in), hence projects can depend on `intrinsics_gen`
even it they are built separately from LLVM.

Reviewed By: MaskRay, JDevlieghere

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/lib/CodeGen/CMakeLists.txt
clang/lib/Frontend/CMakeLists.txt
clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
clang/tools/clang-import-test/CMakeLists.txt
clang/tools/clang-offload-bundler/CMakeLists.txt
clang/tools/clang-offload-wrapper/CMakeLists.txt
clang/tools/driver/CMakeLists.txt
lld/COFF/CMakeLists.txt
lld/Common/CMakeLists.txt
lld/ELF/CMakeLists.txt
lld/MinGW/CMakeLists.txt
lld/lib/Core/CMakeLists.txt
lld/wasm/CMakeLists.txt
lldb/CMakeLists.txt
lldb/source/Expression/CMakeLists.txt
lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 7f8e0718c2eb..948452661a32 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -531,7 +531,7 @@ list(APPEND LLVM_COMMON_DEPENDS clang-tablegen-targets)
 # Force target to be built as soon as possible. Clang modules builds depend
 # header-wise on it as they ship all headers from the umbrella folders. 
Building
 # an entire module might include header, which depends on intrinsics_gen.
-if(LLVM_ENABLE_MODULES AND NOT CLANG_BUILT_STANDALONE)
+if(LLVM_ENABLE_MODULES)
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 

diff  --git a/clang/lib/CodeGen/CMakeLists.txt 
b/clang/lib/CodeGen/CMakeLists.txt
index c4bedf34921c..8afd7219fbe1 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -26,15 +26,6 @@ set(LLVM_LINK_COMPONENTS
   TransformUtils
   )
 
-# In a standard Clang+LLVM build, we need to generate intrinsics before
-# building codegen. In a standalone build, LLVM is already built and we don't
-# need this dependency. Furthermore, LLVM doesn't export it so we can't have
-# this dependency.
-set(codegen_deps intrinsics_gen)
-if (CLANG_BUILT_STANDALONE)
-  set(codegen_deps)
-endif()
-
 if (MSVC)
   set_source_files_properties(CodeGenModule.cpp PROPERTIES COMPILE_FLAGS 
/bigobj)
 endif()
@@ -99,7 +90,7 @@ add_clang_library(clangCodeGen
   VarBypassDetector.cpp
 
   DEPENDS
-  ${codegen_deps}
+  intrinsics_gen
 
   LINK_LIBS
   clangAnalysis

diff  --git a/clang/lib/Frontend/CMakeLists.txt 
b/clang/lib/Frontend/CMakeLists.txt
index 0e23b92e2dea..af5446618b03 100644
--- a/clang/lib/Frontend/CMakeLists.txt
+++ b/clang/lib/Frontend/CMakeLists.txt
@@ -8,11 +8,6 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
-set(optional_deps intrinsics_gen)
-if (CLANG_BUILT_STANDALONE)
-  set(optional_deps)
-endif()
-
 add_clang_library(clangFrontend
   ASTConsumers.cpp
   ASTMerge.cpp
@@ -49,7 +44,7 @@ add_clang_library(clangFrontend
 
   DEPENDS
   ClangDriverOptions
-  ${optional_deps}
+  intrinsics_gen
 
   LINK_LIBS
   clangAST

diff  --git a/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt 
b/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
index 47f9fdf68f40..9ceb1d331828 100644
--- a/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
+++ b/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
@@ -16,15 +16,9 @@ set(LLVM_LINK_COMPONENTS
   native
 )
 
-# Depend on LLVM IR intrinsic generation.
-set(handle_llvm_deps intrinsics_gen)
-if (CLANG_BUILT_STANDALONE)
-  set(handle_llvm_deps)
-endif()
-
 add_clang_library(clangHandleLLVM
   handle_llvm.cpp
 
   DEPENDS
-  ${handle_llvm_deps}
+  intrinsics_gen
   )

diff  --git a/clang/tools/clang-import-test/CMakeLists.txt 
b/clang/tools/clang-import-test/CMakeLists.txt
index 4ccc2d752aac..e459de8f635f 100644
--- a/clang/tools/clang-import-test/CMakeLists.txt
+++ b/clang/tools/clang-import-test/CMakeLists.txt
@@ -3,14 +3,10 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
-if(NOT CLANG_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_clang_executable(clang-import-test
   clang-import-test.cpp
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
   )
 
 set(CLANG_IMPORT_TEST_LIB_DEPS

diff  --git a/clang/tools/clang-offload-bundler/CMakeLists.txt 
b/clang/tools/clang-offload-bundler/CMakeLists.txt
index 4ef099493364..2738bf02e729 100644
--- a/clang/tools/clang-offload-bundler/CMakeLists.txt
+++ b/clang/tools/clang-offload-bundler/CMakeLists.txt
@@ -1,14 +1,10 @@
 

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

[Jim Ingham via Phabricator via lldb-commits]

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



Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:120-127
+  // The setting value may have whitespace, double-quotes, or single-quotes
+  // around the file path to indicate that internal spaces are not word
+  // breaks.  Strip off any ws & quotes from the start and end of the file
+  // path - we aren't doing any word // breaking here so the quoting is
+  // unnecessary.  NB this will cause a problem if someone tries to specify
+  // a file path that legitimately begins or ends with a " or ' character,
+  // or whitespace.

labath wrote:
> This seems excessive. The command interpreter should already handle the word 
> splitting and quotes, which means that things like `br set -y "f i l 
> e.cpp":12:23` will work without this. And it can handle escaping properly, so 
> `br set -y "\"file.cpp:12:23"` would work, if this code wouldn't get in the 
> way.
I wondered about that too.  This is what OptionValueFileSpec did - I started 
this one from there.  So I removed it from both places, and the testsuite is 
still okay with it.



Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:74
+  
+  // First separate the file and the line specification:
+  llvm::StringRef right_of_last_colon;

labath wrote:
> jingham wrote:
> > JDevlieghere wrote:
> > > I think parsing this would be a lot easier with a regex: 
> > > `([^:]+):(\d+)(:(\d+))?`. What do you think?
> > A regex seems overpowered for what I'm doing here, plus that might tempt 
> > people to add more stuff to the specifier and I don't want to back into -y 
> > being the gdb breakpoint specifier mini-language...
> I am not a fan of regex parsing, but I still can't escape the feeling that 
> this should be easier. Maybe a utility function would help:
> ```
> bool chop_number(StringRef , uint32_t ) {
>   auto parts = str.rsplit(':');
>   if (to_integer(parts.second, num)) {
> str = parts.first;
> return true;
>   }
>   return false;
> }
> ...
> if (!input.contains(':')
>   one_colon_expected();
> if (!chop_number(input, col))
>   bad_line_number(); // This complains about line numbers because col will be 
> promoted to a line number if the second chop_number fails.
> if (!chop_number(input, line)) {
>   line = col;
>   col = 0;
> }
> file = input;
> ```
The only tricky bit was making sure that you got the right error, and were able 
to show the string that was wrong.  I tried your approach but also passing 
around the found string bit, but it really didn't make anything clearer.

But I reworked it a little to make the logic clearer and added some comments.  
This version seems pretty straightforward to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83975



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

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 278928.
jingham added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83975

Files:
  lldb/include/lldb/Interpreter/OptionValue.h
  lldb/include/lldb/Interpreter/OptionValueFileColonLine.h
  lldb/include/lldb/Interpreter/OptionValues.h
  lldb/include/lldb/lldb-defines.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/CMakeLists.txt
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/OptionValue.cpp
  lldb/source/Interpreter/OptionValueArray.cpp
  lldb/source/Interpreter/OptionValueDictionary.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Interpreter/Property.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/Makefile
  
lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/TestBreakpointByFileColonLine.py
  lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c
  lldb/test/API/source-manager/TestSourceManager.py
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp

Index: lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestOptionValueFileColonLine.cpp
@@ -0,0 +1,58 @@
+//===-- ArgsTest.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/Interpreter/OptionValueFileColonLine.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Status.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+void CheckSetting(const char *input, bool success, const char *path = nullptr,
+  uint32_t line_number = LLDB_INVALID_ADDRESS,
+  uint32_t column_number = 0) {
+
+  OptionValueFileColonLine value;
+  Status error;
+  llvm::StringRef s_ref(input);
+  error = value.SetValueFromString(s_ref);
+  ASSERT_EQ(error.Success(), success);
+
+  // If we were meant to fail, we don't need to do more checks:
+  if (!success)
+return;
+
+  ASSERT_EQ(value.GetLineNumber(), line_number);
+  ASSERT_EQ(value.GetColumnNumber(), column_number);
+  std::string value_path = value.GetFileSpec().GetPath();
+  ASSERT_STREQ(value_path.c_str(), path);
+}
+
+TEST(OptionValueFileColonLine, setFromString) {
+  bool success = false;
+  OptionValueFileColonLine value;
+  Status error;
+
+  // Make sure a default constructed value is invalid:
+  ASSERT_EQ(value.GetLineNumber(), LLDB_INVALID_LINE_NUMBER);
+  ASSERT_EQ(value.GetColumnNumber(), 0);
+  ASSERT_FALSE(value.GetFileSpec());
+
+  // Make sure it is an error to pass a specifier with no line number:
+  CheckSetting("foo.c", false);
+
+  // Now try with just a file & line:
+  CheckSetting("foo.c:12", true, "foo.c", 12);
+  CheckSetting("foo.c:12:20", true, "foo.c", 12, 20);
+  // Make sure a colon doesn't mess us up:
+  CheckSetting("foo:bar.c:12", true, "foo:bar.c", 12);
+  CheckSetting("foo:bar.c:12:20", true, "foo:bar.c", 12, 20);
+  // Try errors in the line number:
+  CheckSetting("foo.c:12c", false);
+  CheckSetting("foo.c:12:20c", false);
+}
Index: lldb/unittests/Interpreter/CMakeLists.txt
===
--- lldb/unittests/Interpreter/CMakeLists.txt
+++ lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestOptionValueFileColonLine.cpp
 
   LINK_LIBS
 lldbInterpreter
Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -197,6 +197,14 @@
 SOURCE_DISPLAYED_CORRECTLY,
 substrs=['Hello world'])
 
+# Do the same thing with a file & line spec:
+self.expect(
+"source list -y main-copy.c:%d" %
+self.line,
+SOURCE_DISPLAYED_CORRECTLY,
+substrs=['Hello world'])
+
+
 # The '-b' option shows the line table locations from the debug information
 # that indicates valid places to set source level breakpoints.
 
Index: lldb/test/API/functionalities/breakpoint/breakpoint_by_file_colon_line/main.c

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 278922.
aelitashen added a comment.

Add TODO comment for Linux, Use ostringstream for Debug Info Size message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731

Files:
  clang/tools/clang-format/git-clang-format
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/JSONUtils.cpp

Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include 
+#include 
+#include 
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FormatAdapters.h"
@@ -327,6 +329,50 @@
   return llvm::json::Value(std::move(object));
 }
 
+static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
+  uint64_t debug_info_size = 0;
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug") ||
+  section_name.startswith(".apple") || section_name.startswith("__apple"))
+debug_info_size += section.GetFileByteSize();
+  size_t num_sub_sections = section.GetNumSubSections();
+  for (size_t i = 0; i < num_sub_sections; i++) {
+debug_info_size +=
+GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static uint64_t GetDebugInfoSize(lldb::SBModule module) {
+  uint64_t debug_info_size = 0;
+  size_t num_sections = module.GetNumSections();
+  for (size_t i = 0; i < num_sections; i++) {
+debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
+  std::ostringstream oss;
+  oss << "(";
+  oss << std::fixed << std::setprecision(1);
+
+  if (debug_info < 1024) {
+oss << debug_info << "B";
+  } else if (debug_info < 1024 * 1024) {
+double kb = double(debug_info) / 1024.0;
+oss << kb << "KB";
+  } else if (debug_info < 1024 * 1024 * 1024) {
+double mb = double(debug_info) / (1024.0 * 1024.0);
+oss << mb << "MB";
+  } else {
+double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
+oss << gb << "GB";
+;
+  }
+  oss << ")";
+  return oss.str();
+}
 llvm::json::Value CreateModule(lldb::SBModule ) {
   llvm::json::Object object;
   if (!module.IsValid())
@@ -339,9 +385,15 @@
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-object.try_emplace("symbolStatus", "Symbols loaded.");
+std::string symbol_str = "Symbols loaded.";
+uint64_t debug_info = GetDebugInfoSize(module);
+if (debug_info > 0) {
+  symbol_str += ConvertDebugInfoSizeToString(debug_info);
+}
+object.try_emplace("symbolStatus", symbol_str);
 char symbol_path_arr[PATH_MAX];
-module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbol_path_arr));
+module.GetSymbolFileSpec().GetPath(symbol_path_arr,
+   sizeof(symbol_path_arr));
 std::string symbol_path(symbol_path_arr);
 object.try_emplace("symbolFilePath", symbol_path);
   } else {
@@ -352,8 +404,9 @@
   object.try_emplace("addressRange", loaded_addr);
   std::string version_str;
   uint32_t version_nums[3];
-  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
-  for (uint32_t i=0; ihttps://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-#
-#======#
-
-r"""
-clang-format git integration
-
-
-This file provides a clang-format integration for git. Put it somewhere in your
-path and ensure that it is executable. Then, "git clang-format" will invoke
-clang-format on the changes in current files or a specific commit.
-
-For further details, run:
-git clang-format -h
-
-Requires Python 2.7 or Python 3
-"""
-
-from __future__ import absolute_import, division, print_function
-import argparse
-import collections
-import contextlib
-import errno
-import os
-import re
-import subprocess
-import sys
-
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
-
-desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
-
-If two commits are given (requires --diff), run clang-format on all lines in the
-second  that differ from the first .
-
-The following git-config settings set the default of the corresponding option:
-  clangFormat.binary
-  clangFormat.commit
-  

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[walter erquinigo via Phabricator via lldb-commits]

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

The logic looks very good, just some final comments and the code will be high 
quality




Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:66
+@skipIfRemote
+def test_module_event(self):
+# Mac or linux.

With you current Makefile, this test will fail on Linux, as the Makefile will 
expect a .dsym file to be created. Simply put @ skipUnlessDarwin back here, and 
add this comment
  #TODO: Update the Makefile so that this test runs on Linux

Once this is committed, I'll work on make this test pass on Linux



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:67-73
+# Mac or linux.
+
+# On mac, if we load a.out as our symbol file, we will use DWARF with 
.o files and we will
+# have debug symbols, but we won't see any debug info size because all 
of the DWARF
+# sections are in .o files.
+
+# On other platforms, we expect a.out to have debug info, so we will 
expect a size.

The common way to write function comments is with ''', like this
  '''
Mac or linux.

On mac, if we load a.out as our symbol file, we will use DWARF with .o 
files and we will
have debug symbols, but we won't see any debug info size because all of the 
DWARF
sections are in .o files.

On other platforms, we expect a.out to have debug info, so we will expect a 
size.
  '''

That way you don't need to type that many #



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:81-84
+# Darwin only test with dSYM file.
+
+# On mac, if we load a.out.dSYM as our symbol file, we will have debug 
symbols and we
+# will have DWARF sections added to the module, so we will expect a 
size.

same here about the comment



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:333
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug")
+  || section_name.startswith(".apple") || 
section_name.startswith("__apple"))

apply the format change it suggests, i.e start the second line with ||



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:353
+
+static std::string ConvertDebugInfoSize(uint64_t debug_info) {
+  char debug_info_size[32];

ConvertDebugInfoSizeToString is a better name



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:354-368
+  char debug_info_size[32];
+  if (debug_info < 1024) {
+snprintf(debug_info_size, sizeof(debug_info_size), " (%" PRIu64 "B)",
+ debug_info);
+  } else if (debug_info < 1024 * 1024) {
+double kb = double(debug_info) / 1024.0;
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fKB)", kb);

a more modern way to implement this is

  #include 
  #include 
   ...
  std::ostringstream oss;
  oss << "(";
  oss << std::fixed << std::setprecision(1);

  if (debug_info < 1024) {
oss << debug_info << "B";
  } else if (debug_info < 1024 * 1024) {
double kb = double(debug_info) / 1024.0;
oss << kb << "KB";
  } else if (debug_info < 1024 * 1024 * 1024) {
double mb = double(debug_info) / (1024.0 * 1024.0);
oss << mb << "MB";
  } else {
double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
oss << gb << "GB";;
  }
  oss << ")";
  return oss.str();

It's actually safer, as you don't need to specify the array size of your 
debug_info_size_buffer



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:385-386
+if (debug_info > 0) {
+  std::string debug_info_size = ConvertDebugInfoSize(debug_info);
+  symbol_str = symbol_str + debug_info_size;
+}

  symbol_str += ConvertDebugInfoSizeToString(debug_info);
is more concise


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731



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

[Lldb-commits] [PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

[Michele Scandale via Phabricator via lldb-commits]

michele.scandale added a comment.

In D83454#2145086 , @michele.scandale 
wrote:

> In D83454#2144386 , @JDevlieghere 
> wrote:
>
> > In D83454#2142719 , 
> > @michele.scandale wrote:
> >
> > > I tested locally the standalone build of Clang with D83426 
> > > .
> > >  I just want to make sure that we all agree this is right way moving 
> > > forward.
> >
> >
> > Yep, with the target exported this is the right thing to do.
>
>
> Great.
>
> I don't have commit rights. Could someone land this on my behalf?


Anybody?

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83454



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

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 278900.
aelitashen added a comment.

Merge if statements in GetDebugInfoSizeInSection()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731

Files:
  clang/tools/clang-format/git-clang-format
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/JSONUtils.cpp

Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -327,6 +327,46 @@
   return llvm::json::Value(std::move(object));
 }
 
+static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
+  uint64_t debug_info_size = 0;
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug")
+  || section_name.startswith(".apple") || section_name.startswith("__apple"))
+debug_info_size += section.GetFileByteSize();
+  size_t num_sub_sections = section.GetNumSubSections();
+  for (size_t i = 0; i < num_sub_sections; i++) {
+debug_info_size +=
+GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static uint64_t GetDebugInfoSize(lldb::SBModule module) {
+  uint64_t debug_info_size = 0;
+  size_t num_sections = module.GetNumSections();
+  for (size_t i = 0; i < num_sections; i++) {
+debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static std::string ConvertDebugInfoSize(uint64_t debug_info) {
+  char debug_info_size[32];
+  if (debug_info < 1024) {
+snprintf(debug_info_size, sizeof(debug_info_size), " (%" PRIu64 "B)",
+ debug_info);
+  } else if (debug_info < 1024 * 1024) {
+double kb = double(debug_info) / 1024.0;
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fKB)", kb);
+  } else if (debug_info < 1024 * 1024 * 1024) {
+double mb = double(debug_info) / (1024.0 * 1024.0);
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fMB)", mb);
+  } else {
+double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fGB)", gb);
+  }
+  return std::string(debug_info_size);
+}
 llvm::json::Value CreateModule(lldb::SBModule ) {
   llvm::json::Object object;
   if (!module.IsValid())
@@ -339,9 +379,16 @@
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-object.try_emplace("symbolStatus", "Symbols loaded.");
+std::string symbol_str = "Symbols loaded.";
+uint64_t debug_info = GetDebugInfoSize(module);
+if (debug_info > 0) {
+  std::string debug_info_size = ConvertDebugInfoSize(debug_info);
+  symbol_str = symbol_str + debug_info_size;
+}
+object.try_emplace("symbolStatus", symbol_str);
 char symbol_path_arr[PATH_MAX];
-module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbol_path_arr));
+module.GetSymbolFileSpec().GetPath(symbol_path_arr,
+   sizeof(symbol_path_arr));
 std::string symbol_path(symbol_path_arr);
 object.try_emplace("symbolFilePath", symbol_path);
   } else {
@@ -352,8 +399,9 @@
   object.try_emplace("addressRange", loaded_addr);
   std::string version_str;
   uint32_t version_nums[3];
-  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
-  for (uint32_t i=0; ihttps://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-#
-#======#
-
-r"""
-clang-format git integration
-
-
-This file provides a clang-format integration for git. Put it somewhere in your
-path and ensure that it is executable. Then, "git clang-format" will invoke
-clang-format on the changes in current files or a specific commit.
-
-For further details, run:
-git clang-format -h
-
-Requires Python 2.7 or Python 3
-"""
-
-from __future__ import absolute_import, division, print_function
-import argparse
-import collections
-import contextlib
-import errno
-import os
-import re
-import subprocess
-import sys
-
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
-
-desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
-
-If two commits are given (requires --diff), run clang-format on all lines in the
-second  that differ from the first .
-
-The following git-config settings set the default of the corresponding option:
-  clangFormat.binary
-  clangFormat.commit
-  clangFormat.extensions
-  

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 278898.
aelitashen added a comment.

Merge Codes and Improve Readability


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731

Files:
  clang/tools/clang-format/git-clang-format
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/JSONUtils.cpp

Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -327,6 +327,46 @@
   return llvm::json::Value(std::move(object));
 }
 
+static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
+  uint64_t debug_info_size = 0;
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug")
+  ||section_name.startswith(".apple") || section_name.startswith("__apple"))
+debug_info_size += section.GetFileByteSize();
+  size_t num_sub_sections = section.GetNumSubSections();
+  for (size_t i = 0; i < num_sub_sections; i++) {
+debug_info_size +=
+GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static uint64_t GetDebugInfoSize(lldb::SBModule module) {
+  uint64_t debug_info_size = 0;
+  size_t num_sections = module.GetNumSections();
+  for (size_t i = 0; i < num_sections; i++) {
+debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static std::string ConvertDebugInfoSize(uint64_t debug_info) {
+  char debug_info_size[32];
+  if (debug_info < 1024) {
+snprintf(debug_info_size, sizeof(debug_info_size), " (%" PRIu64 "B)",
+ debug_info);
+  } else if (debug_info < 1024 * 1024) {
+double kb = double(debug_info) / 1024.0;
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fKB)", kb);
+  } else if (debug_info < 1024 * 1024 * 1024) {
+double mb = double(debug_info) / (1024.0 * 1024.0);
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fMB)", mb);
+  } else {
+double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fGB)", gb);
+  }
+  return std::string(debug_info_size);
+}
 llvm::json::Value CreateModule(lldb::SBModule ) {
   llvm::json::Object object;
   if (!module.IsValid())
@@ -339,9 +379,16 @@
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-object.try_emplace("symbolStatus", "Symbols loaded.");
+std::string symbol_str = "Symbols loaded.";
+uint64_t debug_info = GetDebugInfoSize(module);
+if (debug_info > 0) {
+  std::string debug_info_size = ConvertDebugInfoSize(debug_info);
+  symbol_str = symbol_str + debug_info_size;
+}
+object.try_emplace("symbolStatus", symbol_str);
 char symbol_path_arr[PATH_MAX];
-module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbol_path_arr));
+module.GetSymbolFileSpec().GetPath(symbol_path_arr,
+   sizeof(symbol_path_arr));
 std::string symbol_path(symbol_path_arr);
 object.try_emplace("symbolFilePath", symbol_path);
   } else {
@@ -352,8 +399,9 @@
   object.try_emplace("addressRange", loaded_addr);
   std::string version_str;
   uint32_t version_nums[3];
-  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
-  for (uint32_t i=0; ihttps://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-#
-#======#
-
-r"""
-clang-format git integration
-
-
-This file provides a clang-format integration for git. Put it somewhere in your
-path and ensure that it is executable. Then, "git clang-format" will invoke
-clang-format on the changes in current files or a specific commit.
-
-For further details, run:
-git clang-format -h
-
-Requires Python 2.7 or Python 3
-"""
-
-from __future__ import absolute_import, division, print_function
-import argparse
-import collections
-import contextlib
-import errno
-import os
-import re
-import subprocess
-import sys
-
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
-
-desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
-
-If two commits are given (requires --diff), run clang-format on all lines in the
-second  that differ from the first .
-
-The following git-config settings set the default of the corresponding option:
-  clangFormat.binary
-  clangFormat.commit
-  clangFormat.extensions
-  

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

[Shu Anzai via Phabricator via lldb-commits]

gedatsu217 marked an inline comment as done.
gedatsu217 added a comment.

> help help frame should not have an autosuggestion to help frame. You can just 
> try to get the autosuggestion for help help frame and check for the error for 
> an invalid command.

Sorry, I do not know what this means. Now, if I execute "help help frame" once, 
it is suggested. Is this wrong?




Comment at: lldb/source/Core/IOHandler.cpp:204
+ .GetAutoSuggestionForCommand(line))
+result = res.getValue();
+}

labath wrote:
> Why does this switch Optional result to a by-ref string argument. We 
> have both code which uses an empty string to signify failure, and code which 
> does that with `None`. Both have their advantages and disadvantages, and I 
> don't have any strong objections to either one, but I certainly don't see an 
> advantage in using both in the same patch.
Sorry, I do not what you mean. Would you explain it more in detail?

(If None is returned, it is not assigned to "result". So, only is string 
assigned to "result". Does this answer your question?)


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D84070: [LLDB] [COFF] Fix handling of symbols with more than one aux symbol

[Adrian McCarthy via Phabricator via lldb-commits]

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

LGTM.  That was a nice catch on the other review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84070



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

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 278894.
aelitashen added a comment.

Create help function in tests for code re-use. Take care of both macOS and 
Linux system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731

Files:
  clang/tools/clang-format/git-clang-format
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/JSONUtils.cpp

Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -327,6 +327,47 @@
   return llvm::json::Value(std::move(object));
 }
 
+static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
+  uint64_t debug_info_size = 0;
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug"))
+debug_info_size += section.GetFileByteSize();
+  if (section_name.startswith(".apple") || section_name.startswith("__apple"))
+debug_info_size += section.GetFileByteSize();
+  size_t num_sub_sections = section.GetNumSubSections();
+  for (size_t i = 0; i < num_sub_sections; i++) {
+debug_info_size +=
+GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static uint64_t GetDebugInfoSize(lldb::SBModule module) {
+  uint64_t debug_info_size = 0;
+  size_t num_sections = module.GetNumSections();
+  for (size_t i = 0; i < num_sections; i++) {
+debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static std::string ConvertDebugInfoSize(uint64_t debug_info) {
+  char debug_info_size[32];
+  if (debug_info < 1024) {
+snprintf(debug_info_size, sizeof(debug_info_size), " (%" PRIu64 "B)",
+ debug_info);
+  } else if (debug_info < 1024 * 1024) {
+double kb = double(debug_info) / 1024.0;
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fKB)", kb);
+  } else if (debug_info < 1024 * 1024 * 1024) {
+double mb = double(debug_info) / (1024.0 * 1024.0);
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fMB)", mb);
+  } else {
+double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
+snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fGB)", gb);
+  }
+  return std::string(debug_info_size);
+}
 llvm::json::Value CreateModule(lldb::SBModule ) {
   llvm::json::Object object;
   if (!module.IsValid())
@@ -339,9 +380,16 @@
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-object.try_emplace("symbolStatus", "Symbols loaded.");
+std::string symbol_str = "Symbols loaded.";
+uint64_t debug_info = GetDebugInfoSize(module);
+if (debug_info > 0) {
+  std::string debug_info_size = ConvertDebugInfoSize(debug_info);
+  symbol_str = symbol_str + debug_info_size;
+}
+object.try_emplace("symbolStatus", symbol_str);
 char symbol_path_arr[PATH_MAX];
-module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbol_path_arr));
+module.GetSymbolFileSpec().GetPath(symbol_path_arr,
+   sizeof(symbol_path_arr));
 std::string symbol_path(symbol_path_arr);
 object.try_emplace("symbolFilePath", symbol_path);
   } else {
@@ -352,8 +400,9 @@
   object.try_emplace("addressRange", loaded_addr);
   std::string version_str;
   uint32_t version_nums[3];
-  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
-  for (uint32_t i=0; ihttps://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-#
-#======#
-
-r"""
-clang-format git integration
-
-
-This file provides a clang-format integration for git. Put it somewhere in your
-path and ensure that it is executable. Then, "git clang-format" will invoke
-clang-format on the changes in current files or a specific commit.
-
-For further details, run:
-git clang-format -h
-
-Requires Python 2.7 or Python 3
-"""
-
-from __future__ import absolute_import, division, print_function
-import argparse
-import collections
-import contextlib
-import errno
-import os
-import re
-import subprocess
-import sys
-
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
-
-desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
-
-If two commits are given (requires --diff), run clang-format on all lines in the
-second  that differ from the first .
-
-The following git-config settings set the default of the 

[Lldb-commits] [PATCH] D84070: [LLDB] [COFF] Fix handling of symbols with more than one aux symbol

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth.
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84070

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml


Index: lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
@@ -101,6 +101,13 @@
 SimpleType:  IMAGE_SYM_TYPE_NULL
 ComplexType: IMAGE_SYM_DTYPE_NULL
 StorageClass:IMAGE_SYM_CLASS_STATIC
+  - Name:.file
+Value:   0
+SectionNumber:   -2
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_FILE
+File:longfilenameusingtwoauxsymbols
   - Name:entry
 Value:   0
 SectionNumber:   1
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -698,7 +698,7 @@
 
 if (symbol.naux > 0) {
   i += symbol.naux;
-  offset += symbol_size;
+  offset += symbol.naux * symbol_size;
 }
   }
 }


Index: lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
@@ -101,6 +101,13 @@
 SimpleType:  IMAGE_SYM_TYPE_NULL
 ComplexType: IMAGE_SYM_DTYPE_NULL
 StorageClass:IMAGE_SYM_CLASS_STATIC
+  - Name:.file
+Value:   0
+SectionNumber:   -2
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_FILE
+File:longfilenameusingtwoauxsymbols
   - Name:entry
 Value:   0
 SectionNumber:   1
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -698,7 +698,7 @@
 
 if (symbol.naux > 0) {
   i += symbol.naux;
-  offset += symbol_size;
+  offset += symbol.naux * symbol_size;
 }
   }
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83904: [lldb] Unify sleep and time outs in GDB remote testcases

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0fbbf3a98ca6: [lldb] Unify sleep and time outs in GDB remote 
testcases (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D83904?vs=278297=278851#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83904

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteKill.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteProcessInfo.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
  lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/test/API/tools/lldb-server/commandline/TestStubReverseConnect.py

Index: lldb/test/API/tools/lldb-server/commandline/TestStubReverseConnect.py
===
--- lldb/test/API/tools/lldb-server/commandline/TestStubReverseConnect.py
+++ lldb/test/API/tools/lldb-server/commandline/TestStubReverseConnect.py
@@ -14,8 +14,6 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-_DEFAULT_TIMEOUT = 20 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
-
 def setUp(self):
 # Set up the test.
 gdbremote_testcase.GdbRemoteTestCaseBase.setUp(self)
@@ -25,11 +23,11 @@
 self.assertIsNotNone(self.listener_socket)
 self.listener_port = self.listener_socket.getsockname()[1]
 
-def create_listener_socket(self, timeout_seconds=_DEFAULT_TIMEOUT):
+def create_listener_socket(self):
 sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
 self.assertIsNotNone(sock)
 
-sock.settimeout(timeout_seconds)
+sock.settimeout(self.DEFAULT_TIMEOUT)
 sock.bind(("127.0.0.1", 0))
 sock.listen(1)
 
@@ -77,7 +75,7 @@
 address, stub_socket.getsockname()))
 
 # Verify we can do the handshake.  If that works, we'll call it good.
-self.do_handshake(stub_socket, timeout_seconds=self._DEFAULT_TIMEOUT)
+self.do_handshake(stub_socket)
 
 # Clean up.
 stub_socket.shutdown(socket.SHUT_RDWR)
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -642,7 +642,7 @@
 self.run_process_then_stop(run_seconds=1)
 
 # Wait at most x seconds for 3 threads to be present.
-threads = self.wait_for_thread_count(3, timeout_seconds=self._WAIT_TIMEOUT)
+threads = self.wait_for_thread_count(3)
 self.assertEqual(len(threads), 3)
 
 # verify we can $H to each thead, and $qC matches the thread we set.
@@ -723,7 +723,7 @@
 # context = self.run_process_then_stop(run_seconds=1)
 
 # Wait at most x seconds for all threads to be present.
-# threads = self.wait_for_thread_count(NUM_THREADS, timeout_seconds=5)
+# threads = self.wait_for_thread_count(NUM_THREADS)
 # self.assertEquals(len(threads), NUM_THREADS)
 
 signaled_tids = {}
@@ -739,7 +739,7 @@
 2: "thread_id"}}],
  True)
 
-context = self.expect_gdbremote_sequence(timeout_seconds=self._DEFAULT_TIMEOUT)
+context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)
 signo = context.get("signo")
 self.assertEqual(int(signo, 16), segfault_signo)
@@ -775,8 +775,7 @@
 True)
 
 # Run the sequence.
-context = self.expect_gdbremote_sequence(
-timeout_seconds=self._DEFAULT_TIMEOUT)
+context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)
 
 # Ensure the stop signal is the signal we delivered.
@@ -1491,7 +1490,7 @@
 self.assertIsNotNone(context)
 
 # Wait for 3 threads to be present.
-threads = self.wait_for_thread_count(3, timeout_seconds=self._WAIT_TIMEOUT)
+threads = self.wait_for_thread_count(3)
 self.assertEqual(len(threads), 3)
 
 expected_reg_values = []
Index: lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
@@ -33,7 +33,7 @@
 self.assertIsNotNone(context)
 
 # Give threads time to start up, then break.
-time.sleep(self._WAIT_TIMEOUT)
+time.sleep(self.DEFAULT_SLEEP)
 self.reset_test_sequence()
 self.test_sequence.add_log_lines(
  

[Lldb-commits] [lldb] 0fbbf3a - [lldb] Unify sleep and time outs in GDB remote testcases

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-07-17T11:03:16-07:00
New Revision: 0fbbf3a98ca6fc75aafb6405943ee00e7c4d5594

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

LOG: [lldb] Unify sleep and time outs in GDB remote testcases

Reduce sleep and time outs in GDB remote testcases to one default value
for each. Stop passing these values around and always use the default
instead.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/test/API/tools/lldb-server/TestGdbRemoteKill.py
lldb/test/API/tools/lldb-server/TestGdbRemoteProcessInfo.py
lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/test/API/tools/lldb-server/commandline/TestStubReverseConnect.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index 0b81912e3d3f..d702d8ee6820 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -31,10 +31,10 @@ class GdbRemoteTestCaseBase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
-_TIMEOUT_SECONDS = 120 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
-_DEFAULT_TIMEOUT =  10 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
-_READ_TIMEOUT=   5 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
-_WAIT_TIMEOUT=   5 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
+# Default time out in seconds. The timeout is increased tenfold under Asan.
+DEFAULT_TIMEOUT =  10 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
+# Default sleep time in seconds. The sleep time is doubled under Asan.
+DEFAULT_SLEEP   =  5  * (2  if ('ASAN_OPTIONS' in os.environ) else 1)
 
 _GDBREMOTE_KILL_PACKET = "$k#6b"
 
@@ -204,10 +204,10 @@ def shutdown_named_pipe():
 
 return (named_pipe_path, named_pipe, named_pipe_fd)
 
-def get_stub_port_from_named_socket(self, read_timeout_seconds):
+def get_stub_port_from_named_socket(self):
 # Wait for something to read with a max timeout.
 (ready_readers, _, _) = select.select(
-[self.named_pipe_fd], [], [], read_timeout_seconds)
+[self.named_pipe_fd], [], [], self.DEFAULT_TIMEOUT)
 self.assertIsNotNone(
 ready_readers,
 "write side of pipe has not written anything - stub isn't writing 
to pipe.")
@@ -407,7 +407,7 @@ def launch_debug_monitor(self, attach_pid=None, 
logfile=None):
 # If we're receiving the stub's listening port from the named pipe, do
 # that here.
 if self.named_pipe:
-self.port = 
self.get_stub_port_from_named_socket(self._READ_TIMEOUT)
+self.port = self.get_stub_port_from_named_socket()
 
 return server
 
@@ -568,14 +568,14 @@ def prep_debug_monitor_and_inferior(
 def expect_socket_recv(
 self,
 sock,
-expected_content_regex,
-timeout_seconds):
+expected_content_regex
+):
 response = ""
-timeout_time = time.time() + timeout_seconds
+timeout_time = time.time() + self.DEFAULT_TIMEOUT
 
 while not expected_content_regex.match(
 response) and time.time() < timeout_time:
-can_read, _, _ = select.select([sock], [], [], timeout_seconds)
+can_read, _, _ = select.select([sock], [], [], 
self.DEFAULT_TIMEOUT)
 if can_read and sock in can_read:
 recv_bytes = sock.recv(4096)
 if recv_bytes:
@@ -583,24 +583,21 @@ def expect_socket_recv(
 
 self.assertTrue(expected_content_regex.match(response))
 
-def expect_socket_send(self, sock, content, timeout_seconds):
+def expect_socket_send(self, sock, content):
 request_bytes_remaining = content
-timeout_time = time.time() + timeout_seconds
+timeout_time = time.time() + self.DEFAULT_TIMEOUT
 
 while len(request_bytes_remaining) > 0 and time.time() < timeout_time:
-_, can_write, _ = select.select([], [sock], [], timeout_seconds)
+_, can_write, _ = select.select([], [sock], [], 
self.DEFAULT_TIMEOUT)
 if can_write and sock in can_write:
 written_byte_count = 
sock.send(request_bytes_remaining.encode())
 request_bytes_remaining = request_bytes_remaining[
 written_byte_count:]
 

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers added a comment.

Sounds like another approach was landed in https://reviews.llvm.org/D69403, and 
a follow up in https://reviews.llvm.org/D70277, and 
9a3f892d018238dce5181e458905311db8e682f5 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69148



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

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

[Raphael Isemann via Phabricator via lldb-commits]

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

> The test looks wrong to me. If the feature is supposed to be off by default, 
> then how can it test anything without first turning the feature on? Either 
> the feature isn't really off, or the test is broken. I would expect it to be 
> the latter, because the test sequence seems incorrect to me. I would expect 
> to see some newlines in the strings being sent, otherwise all of this will 
> just get concatenated into a single command.
>  Also, this only seems to test the ^F functionality. It does not check that 
> the command is printed in gray _before_ the user hits ^F.

Yeah, and the test obviously doesn't pass.

Alright, let's try to fix this.

@gedatsu217 Not sure if you uploaded the wrong diff, but that test doesn't 
pass. It would be a nice test if if worked though. But as we anyway need 
another iteration, let's also extend the test while we're at it.

First, you need to get the test to actually enable autosuggestions, so you need 
to launch via `self.launch(extra_args=["-o", "settings set show-autosuggestion 
true"])` (the `extra_args` are just passed to LLDB's command line).

Also as Pavel said, we should also test that we display the autosuggestion in 
faint colour. Not sure if there is a good way to test that the text is directly 
behind the cursor, but we can test that it's there. And I guess we should test 
that we actually pick the most recent command for the autosuggestion (this is 
currently wrong and we pick the least recent command).

I extend your test a bit to something like this (which is currently failing as 
it always completes `help frame` but not the `help apropos`).

  self.launch(extra_args=["-o", "settings set show-autosuggestion true"]) 
  # Common input codes and escape sequences.

  ctrl_f = "\x06" 
  faint_color = "\x1b[2m" 
  reset = "\x1b[0m"   
  
  frame_output_needle = "Syntax: frame "  
  # Run 'help frame' once to put it into the command history. 
  self.expect("help frame", substrs=[frame_output_needle])
  
  # Check that LLDB shows the autosuggestion in gray behind the text. 
  self.child.send("hel")  
  self.child.expect_exact(faint_color + "p frame" + reset)
  
  # Apply the autosuggestion and press enter. This should print the   
  # the 'help frame' output if everything went correctly. 
  self.child.send(ctrl_f + "\n")  
  self.child.expect_exact(frame_output_needle)
  
  # Try autosuggestion using tab and ^f.  
  # \t makes "help" and ^f makes "help frame". If everything went 
  # correct we should see the 'help frame' output again.  
  self.child.send("hel\t" + ctrl_f + "\n")
  self.child.expect_exact(frame_output_needle)
  
  # Try another command.  
  apropos_output_needle = "Syntax: apropos " 
  # Run 'help frame' once to put it into the command history. 
  self.expect("help apropos", substrs=[apropos_output_needle])
  
  # 'hel' should have an autosuggestion for 'help apropos' now.   
  self.child.send("hel")  
  # FIXME: This is showing 'frame' instead.   
  self.child.expect_exact(faint_color + "p apropos" + reset)
  
  # Run the command and expect the 'help apropos' output. 
  self.child.send(ctrl_f + "\n")  
  self.child.expect_exact(apropos_output_needle)

A few more test cases that come to my mind and you should add:

- `help help frame` should not have an autosuggestion to `help frame`. You can 
just try to get the autosuggestion for `help help frame` and check for the 
error for an invalid command.
- Pressing Ctrl+F in an empty prompt should do nothing (you can just check for 
the `(lldb) ` prompt I would say).
- Pressing Ctrl+F directly after Ctrl+F 

[Lldb-commits] [PATCH] D83904: [lldb] Unify sleep and time outs in GDB remote testcases

[Pavel Labath via Phabricator via lldb-commits]

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

Sounds like a good idea.




Comment at: 
lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:63-64
 # Wait until all threads have started.
-threads = self.wait_for_thread_count(thread_count,
- 
timeout_seconds=self._WAIT_TIMEOUT)
+threads = self.wait_for_thread_count(thread_count
+ )
 self.assertIsNotNone(threads)

unwrap this


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D83904



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

[Lldb-commits] [PATCH] D83840: [lldb][test] Prevent infinite loop while looking for use_lldb_suite_root.py.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D83840#2156040 , @JDevlieghere 
wrote:

> In D83840#2155308 , @labath wrote:
>
> > In D83840#2154263 , @shafik wrote:
> >
> > > @labath why do we need two copies of `use_lldb_suite.py`?
> >
> >
> > This script is responsible for setting up an appropriate python import 
> > path. Before we can import any code in lldb/third_party/Python/module or 
> > lldb/packages/Python, we need to add those paths to `sys.path`. And we 
> > cannot completely put this code into a central place because then we 
> > wouldn't know how to import that.
> >
> > So, the way get around that is by placing this file into the same folder as 
> > the script that needs it. Then, the script can load this file using a 
> > relative import, and afterwards, it can import anything it wants. And since 
> > we have scripts needing this functionality in multiple places, we have 
> > multiple copies of the script.
> >
> > At least that's the current state of the art. It's possible that there are 
> > better solutions, but we just don't know about them.
>
>
> We could have CMake configure this at the cost of always having to use the 
> corresponding scripts from the build directory? It seems like only 
> `analyze-project-deps.py` and `host_art_bt.py` are importing lldb right now 
> so it might be worth considering.


The important part is who is importing `use_lldb_suite`, not `lldb`. By the 
looks of things host_art_bt.py is meant to be included from within lldb, so the 
path will be correct there.

Not being able to run analyze-project-deps from the source tree would be a 
minor inconvenience, so I'm not sure if it's worth it. But I am not opposed to 
the idea either. All that script really needs is the location of the repo root. 
Since it already has a lot of knowledge about the repo layout, we could just 
hardcode that too...

I don't know if it's the only script that needs that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83840



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

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

[Pavel Labath via Phabricator via lldb-commits]

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

I think this is as good as we can get right now. Thanks for sticking through. 
Two quick comments inline.




Comment at: 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp:152-153
+
+  if (sve_reg_num != LLDB_INVALID_REGNUM &&
+  offset != LLDB_INVALID_INDEX32) {
+assert(offset < m_sveregset.GetByteSize());

Are there any FPR registers which we should not be able to retrieve from the 
SVE state? I'm wondering if this should be turned into an assert.



Comment at: 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp:167-169
+uint64_t byte_size = 1;
+uint8_t zeros = 0;
+const uint8_t *src = 

It looks like this is only used in the `IsSVEZ(reg)` block below. You could 
move the declarations there and avoid initializations with bogus values.


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

https://reviews.llvm.org/D77047



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The test looks wrong to me. If the feature is supposed to be off by default, 
then how can it test anything without first turning the feature on? Either the 
feature isn't really off, or the test is broken. I would expect it to be the 
latter, because the test sequence seems incorrect to me. I would expect to see 
some newlines in the strings being sent, otherwise all of this will just get 
concatenated into a single command.

Also, this only seems to test the ^F functionality. It does not check that the 
command is printed in gray _before_ the user hits ^F.




Comment at: lldb/source/Core/IOHandler.cpp:204
+ .GetAutoSuggestionForCommand(line))
+result = res.getValue();
+}

Why does this switch Optional result to a by-ref string argument. We 
have both code which uses an empty string to signify failure, and code which 
does that with `None`. Both have their advantages and disadvantages, and I 
don't have any strong objections to either one, but I certainly don't see an 
advantage in using both in the same patch.



Comment at: lldb/source/Host/common/Editline.cpp:1213
+  char bind_key[2] = {0, 0};
+  llvm::StringRef indent_chars =
+  
"abcdefghijklmnopqrstuvwxzyABCDEFGHIJKLMNOPQRSTUVWXZY1234567890!\"#$%"

These don't have anything to do with indenting. Looks like a leftover from the 
copy paste from line 1255


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:120-127
+  // The setting value may have whitespace, double-quotes, or single-quotes
+  // around the file path to indicate that internal spaces are not word
+  // breaks.  Strip off any ws & quotes from the start and end of the file
+  // path - we aren't doing any word // breaking here so the quoting is
+  // unnecessary.  NB this will cause a problem if someone tries to specify
+  // a file path that legitimately begins or ends with a " or ' character,
+  // or whitespace.

This seems excessive. The command interpreter should already handle the word 
splitting and quotes, which means that things like `br set -y "f i l 
e.cpp":12:23` will work without this. And it can handle escaping properly, so 
`br set -y "\"file.cpp:12:23"` would work, if this code wouldn't get in the way.



Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:129
+  m_value_was_set = true;
+  m_file_spec.SetFile(file_name.str(), FileSpec::Style::native);
+  NotifyValueChanged();

`SetFile` already accepts a StringRef.



Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:74
+  
+  // First separate the file and the line specification:
+  llvm::StringRef right_of_last_colon;

jingham wrote:
> JDevlieghere wrote:
> > I think parsing this would be a lot easier with a regex: 
> > `([^:]+):(\d+)(:(\d+))?`. What do you think?
> A regex seems overpowered for what I'm doing here, plus that might tempt 
> people to add more stuff to the specifier and I don't want to back into -y 
> being the gdb breakpoint specifier mini-language...
I am not a fan of regex parsing, but I still can't escape the feeling that this 
should be easier. Maybe a utility function would help:
```
bool chop_number(StringRef , uint32_t ) {
  auto parts = str.rsplit(':');
  if (to_integer(parts.second, num)) {
str = parts.first;
return true;
  }
  return false;
}
...
if (!input.contains(':')
  one_colon_expected();
if (!chop_number(input, col))
  bad_line_number(); // This complains about line numbers because col will be 
promoted to a line number if the second chop_number fails.
if (!chop_number(input, line)) {
  line = col;
  col = 0;
}
file = input;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83975



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

[Lldb-commits] [lldb] ede7c02 - [lldb/COFF] Remove strtab zeroing hack

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-17T13:24:59+02:00
New Revision: ede7c02b38c0c3adf7fb9ee308bd0f6d92a0eb4e

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

LOG: [lldb/COFF] Remove strtab zeroing hack

Summary:
This code (recently responsible for a unaligned access sanitizer
failure) claims that the string table offset zero should result in an
empty string.

I cannot find any mention of this detail in the Microsoft COFF
documentation, and the llvm COFF parser also does not handle offset zero
specially. This code was introduced in 0076e7159, which also does not go
into specifics, citing "various bugfixes".

Given that this is obviously a hack, and does not cause tests to fail, I
think we should just delete it.

Reviewers: amccarth, markmentovai

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index d606b49130c4..5feec8167186 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -543,12 +543,6 @@ DataExtractor ObjectFilePECOFF::ReadImageData(uint32_t 
offset, size_t size) {
   if (m_data.ValidOffsetForDataOfSize(offset, size))
 return DataExtractor(m_data, offset, size);
 
-  if (m_file) {
-// A bit of a hack, but we intend to write to this buffer, so we can't
-// mmap it.
-auto buffer_sp = MapFileData(m_file, size, offset);
-return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
-  }
   ProcessSP process_sp(m_process_wp.lock());
   DataExtractor data;
   if (process_sp) {
@@ -652,12 +646,6 @@ Symtab *ObjectFilePECOFF::GetSymtab() {
   DataExtractor strtab_data = ReadImageData(
   m_coff_header.symoff + symbol_data_size, strtab_size);
 
-  // First 4 bytes should be zeroed after strtab_size has been read,
-  // because it is used as offset 0 to encode a NULL string.
-  uint32_t *strtab_data_start = const_cast(
-  reinterpret_cast(strtab_data.GetDataStart()));
-  ::memset(_data_start[0], 0, sizeof(uint32_t));
-
   offset = 0;
   std::string symbol_name;
   Symbol *symbols = m_symtab_up->Resize(num_syms);



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

[Lldb-commits] [PATCH] D83881: [lldb/COFF] Remove strtab zeroing hack

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGede7c02b38c0: [lldb/COFF] Remove strtab zeroing hack 
(authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83881

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -543,12 +543,6 @@
   if (m_data.ValidOffsetForDataOfSize(offset, size))
 return DataExtractor(m_data, offset, size);
 
-  if (m_file) {
-// A bit of a hack, but we intend to write to this buffer, so we can't
-// mmap it.
-auto buffer_sp = MapFileData(m_file, size, offset);
-return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
-  }
   ProcessSP process_sp(m_process_wp.lock());
   DataExtractor data;
   if (process_sp) {
@@ -652,12 +646,6 @@
   DataExtractor strtab_data = ReadImageData(
   m_coff_header.symoff + symbol_data_size, strtab_size);
 
-  // First 4 bytes should be zeroed after strtab_size has been read,
-  // because it is used as offset 0 to encode a NULL string.
-  uint32_t *strtab_data_start = const_cast(
-  reinterpret_cast(strtab_data.GetDataStart()));
-  ::memset(_data_start[0], 0, sizeof(uint32_t));
-
   offset = 0;
   std::string symbol_name;
   Symbol *symbols = m_symtab_up->Resize(num_syms);


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -543,12 +543,6 @@
   if (m_data.ValidOffsetForDataOfSize(offset, size))
 return DataExtractor(m_data, offset, size);
 
-  if (m_file) {
-// A bit of a hack, but we intend to write to this buffer, so we can't
-// mmap it.
-auto buffer_sp = MapFileData(m_file, size, offset);
-return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
-  }
   ProcessSP process_sp(m_process_wp.lock());
   DataExtractor data;
   if (process_sp) {
@@ -652,12 +646,6 @@
   DataExtractor strtab_data = ReadImageData(
   m_coff_header.symoff + symbol_data_size, strtab_size);
 
-  // First 4 bytes should be zeroed after strtab_size has been read,
-  // because it is used as offset 0 to encode a NULL string.
-  uint32_t *strtab_data_start = const_cast(
-  reinterpret_cast(strtab_data.GetDataStart()));
-  ::memset(_data_start[0], 0, sizeof(uint32_t));
-
   offset = 0;
   std::string symbol_name;
   Symbol *symbols = m_symtab_up->Resize(num_syms);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 278718.
omjavaid added a comment.

This revision updates SVE ptrace support to reflect changes we made in SVE core 
file support specially around handling of FPSR and FPCR.


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

https://reviews.llvm.org/D79699

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
@@ -0,0 +1,5 @@
+int main() {
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+  return 0; // Set a break point here.
+}
\ No newline at end of file
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -0,0 +1,142 @@
+"""
+Test the AArch64 SVE registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Verify we have a register named "%s"' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Verify "%s" == %i' % (name, expected))
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf
+def test_sve_registers_configuration(self):
+"""Test AArch64 SVE registers size configuration."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+currentFrame = thread.GetFrameAtIndex(0)
+
+has_sve = False
+for registerSet in currentFrame.GetRegisters():
+if 'Scalable Vector Extension Registers' in registerSet.GetName():
+has_sve = True
+
+if not has_sve:
+self.skipTest('SVE registers must be supported.')
+
+registerSets = process.GetThreadAtIndex(
+0).GetFrameAtIndex(0).GetRegisters()
+
+sve_registers = registerSets.GetValueAtIndex(2)
+
+vg_reg = sve_registers.GetChildMemberWithName("vg")
+
+vg_reg_value = sve_registers.GetChildMemberWithName(
+"vg").GetValueAsUnsigned()
+
+z_reg_size = vg_reg_value * 8
+
+p_reg_size = z_reg_size / 8
+
+for i in range(32):
+self.check_sve_register_size(
+sve_registers, 'z%i' % (i), z_reg_size)
+
+for i in range(16):
+self.check_sve_register_size(
+sve_registers, 'p%i' % (i), p_reg_size)
+
+self.check_sve_register_size(sve_registers, 'ffr', p_reg_size)
+
+mydir = TestBase.compute_mydir(__file__)
+@no_debug_info_test
+@skipIf
+def test_sve_registers_read_write(self):
+"""Test AArch64 SVE registers read and write."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = 

[Lldb-commits] [PATCH] D83881: [lldb/COFF] Remove strtab zeroing hack

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for checking this out. Sorry for forgetting you Martin, I'll have to 
remember to add you to my future coff patches.




Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:672
 break;
   symbol_name.assign(symbol_name_cstr, sizeof(symbol.name));
 }

mstorsjo wrote:
> Kind of unrelated question: Does this treat the assigned symbol as always 8 
> bytes long, or does it scan the provided 8 bytes for potential trailing 
> terminators? Object/COFFObjectFile has got a bit of extra logic to 
> distinguish between those cases: 
> https://github.com/llvm/llvm-project/blob/master/llvm/lib/Object/COFFObjectFile.cpp#L999-L1004
It always treats is as 8 bytes, but then the string is accessed via `.c_str()` 
on line 679. which picks up the first nul character in the string, which is 
either one of the embedded ones, or the one that std::string appends internally.

I don't know if that was intentional -- it's kinda neat, but also quite scary.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:689
   i += symbol.naux;
   offset += symbol_size;
 }

mstorsjo wrote:
> Unrelated to the patch at hand: I believe this should be `offset += 
> symbol.naux * symbol_size;`. I could try to make a patch to exercise that 
> case at some later time...
Yeah, that looks like a bug. If it is, we could still manage to cherry-pick 
that for 11.0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83881



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

[Lldb-commits] [PATCH] D83957: [lldb/DWARF] Don't get confused by line sequences with tombstone values

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked 3 inline comments as done.
Closed by commit rGf3fab392f574: [lldb/DWARF] Dont get confused by line 
sequences with tombstone values (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D83957?vs=278503=278691#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83957

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s

Index: lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s
@@ -0,0 +1,106 @@
+# This test that we don't get confused by line tables containing a tombstone
+# (-1) value, as produced by recent lld's. Line sequences with the tombstone
+# value should be completely ignored. The tombstone sequence is deliberately
+# longer so that any attempt at an address binary search will likely land inside
+# the sequence.
+
+# RUN: llvm-mc --filetype=obj --triple=x86_64-pc-linux %s -o %t
+# RUN: %lldb -o "image lookup -n main -v" -o "image dump line-table main.cpp" \
+# RUN:   -o exit %t | FileCheck %s
+
+# CHECK-LABEL: image lookup -n main -v
+# CHECK: LineEntry: [0x1000-0x1001): main.cpp:1
+# CHECK-LABEL: image dump line-table main.cpp
+# CHECK-NEXT: Line table for main.cpp
+# CHECK-NEXT: 0x1000: main.cpp:1
+# CHECK-NEXT: 0x1001: main.cpp:1
+# CHECK-EMPTY:
+# CHECK-NEXT: exit
+
+.text
+.space 0x1000
+main:
+  nop
+.Lmain_end:
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   0   # DW_CHILDREN_no
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   16  # DW_AT_stmt_list
+.byte   23  # DW_FORM_sec_offset
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+
+.section.debug_info,"",@progbits
+.Lcu_begin0:
+.long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+.short  4   # DWARF version number
+.long   0   # Offset Into Abbrev. Section
+.byte   8   # Address Size (in bytes)
+.byte   1   # Abbrev [1] 0xb:0xc4 DW_TAG_compile_unit
+.asciz  "Hand-written DWARF"# DW_AT_producer
+.asciz  "main.cpp"  # DW_AT_name
+.long   0   # DW_AT_stmt_list
+.quad   main-.text  # DW_AT_low_pc
+.long   .Lmain_end-main # DW_AT_high_pc
+.Ldebug_info_end0:
+
+.section .debug_line,"",@progbits
+.long   .Llt1_end - .Llt1_start # Length of Unit (DWARF-32 format)
+.Llt1_start:
+.short  4   # DWARF version number
+.long   .Lprologue1_end-.Lprologue1_start # Length of Prologue
+.Lprologue1_start:
+.byte   1   # Minimum Instruction Length
+.byte   1   # Maximum Operations per Instruction
+.byte   1   # Default is_stmt
+.byte   -5  # Line Base
+.byte   14  # Line Range
+.byte   13  # Opcode Base
+.byte   0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 # Standard Opcode Lengths
+.byte   0
+.asciz "main.cpp"  # File table
+.byte   0, 0, 0
+.byte   0
+.Lprologue1_end:
+.byte   0, 9, 2 # DW_LNE_set_address
+.quad   -1
+.byte   1   # DW_LNS_copy
+.byte   33  # address += 1,  line += 1
+.byte   33  # address += 1,  line += 1
+.byte   33  # address += 1,  line += 1
+.byte   33  # address += 1,  line += 1
+.byte   33  # address += 1,  line += 1
+.byte   33  # address += 1,  line += 1
+.byte   33  # address += 1,  line += 1
+.byte   33  

[Lldb-commits] [lldb] f3fab39 - [lldb/DWARF] Don't get confused by line sequences with tombstone values

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-17T11:51:52+02:00
New Revision: f3fab392f57421a5bdabfb7e40820257d8f637b2

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

LOG: [lldb/DWARF] Don't get confused by line sequences with tombstone values

Summary:
With D81784, lld has started debug info resolving relocations to
garbage-collected symbols as -1 (instead of relocation addend). For an
unaware consumer this generated sequences which seemingly wrap the
address space -- their first entry was 0xf, but all other entries
were low numbers.

Lldb stores line sequences concatenated into one large vector, sorted by
the first entry, and searched with std::lower_bound. This resulted in
the low-value entries being placed at the end of the vector, which
utterly confused the lower_bound algorithm, and caused it to not find a
match. (Previously, these sequences would be at the start of the vector,
and normally would contain addresses that are far smaller than any real
address we want to look up, so std::lower_bound was fine.)

This patch makes lldb ignore these kinds of sequences completely. It
does that by changing the construction algorithm from iterating over the
rows (as parsed by llvm), to iterating over the sequences. This is
important because the llvm parsed performs validity checks when
constructing the sequence array, whereas the row array contains raw
data.

Reviewers: JDevlieghere, MaskRay

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s

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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 9f64e5255fd5..0b7e31ae2d1d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1036,18 +1036,20 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit 
_unit) {
   // FIXME: Rather than parsing the whole line table and then copying it over
   // into LLDB, we should explore using a callback to populate the line table
   // while we parse to reduce memory usage.
-  std::unique_ptr sequence =
-  LineTable::CreateLineSequenceContainer();
   std::vector> sequences;
-  for (auto  : line_table->Rows) {
-LineTable::AppendLineEntryToSequence(
-sequence.get(), row.Address.Address, row.Line, row.Column, row.File,
-row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
-row.EndSequence);
-if (row.EndSequence) {
-  sequences.push_back(std::move(sequence));
-  sequence = LineTable::CreateLineSequenceContainer();
+  // The Sequences view contains only valid line sequences. Don't iterate over
+  // the Rows directly.
+  for (const llvm::DWARFDebugLine::Sequence  : line_table->Sequences) {
+std::unique_ptr sequence =
+LineTable::CreateLineSequenceContainer();
+for (unsigned idx = seq.FirstRowIndex; idx < seq.LastRowIndex; ++idx) {
+  const llvm::DWARFDebugLine::Row  = line_table->Rows[idx];
+  LineTable::AppendLineEntryToSequence(
+  sequence.get(), row.Address.Address, row.Line, row.Column, row.File,
+  row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
+  row.EndSequence);
 }
+sequences.push_back(std::move(sequence));
   }
 
   std::unique_ptr line_table_up =

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s 
b/lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s
new file mode 100644
index ..53600ac5f4b1
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s
@@ -0,0 +1,106 @@
+# This test that we don't get confused by line tables containing a tombstone
+# (-1) value, as produced by recent lld's. Line sequences with the tombstone
+# value should be completely ignored. The tombstone sequence is deliberately
+# longer so that any attempt at an address binary search will likely land 
inside
+# the sequence.
+
+# RUN: llvm-mc --filetype=obj --triple=x86_64-pc-linux %s -o %t
+# RUN: %lldb -o "image lookup -n main -v" -o "image dump line-table main.cpp" \
+# RUN:   -o exit %t | FileCheck %s
+
+# CHECK-LABEL: image lookup -n main -v
+# CHECK: LineEntry: [0x1000-0x1001): main.cpp:1
+# CHECK-LABEL: image dump line-table main.cpp
+# CHECK-NEXT: Line table for main.cpp
+# CHECK-NEXT: 0x1000: main.cpp:1
+# CHECK-NEXT: 0x1001: main.cpp:1
+# CHECK-EMPTY:
+# CHECK-NEXT: exit
+
+.text
+.space 0x1000
+main:
+  nop
+.Lmain_end:
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17   

[Lldb-commits] [PATCH] D84008: [DWARFYAML] Refactor emitDebugInfo() to make the length be inferred.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing added a comment.

I'll add some tests later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84008



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

[Lldb-commits] [PATCH] D84008: [DWARFYAML] Refactor emitDebugInfo() to make the length be inferred.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing created this revision.
Higuoxing added reviewers: jhenderson, grimar, MaskRay.
Herald added subscribers: llvm-commits, lldb-commits, hiraditya, mgorny.
Herald added projects: LLDB, LLVM.

This patch refactors `emitDebugInfo()` to make the length field be
inferred from its content. Besides, the `Visitor` class is removed in
this patch. The original `Visitor` class helps us determine an
appropriate length and emit the .debug_info section. These two
processes can be merged into one process. Besides, the length field
should be inferred when it's missing rather than when it's zero.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84008

Files:
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp
  llvm/include/llvm/ObjectYAML/DWARFEmitter.h
  llvm/include/llvm/ObjectYAML/DWARFYAML.h
  llvm/lib/ObjectYAML/CMakeLists.txt
  llvm/lib/ObjectYAML/DWARFEmitter.cpp
  llvm/lib/ObjectYAML/DWARFVisitor.cpp
  llvm/lib/ObjectYAML/DWARFVisitor.h
  llvm/lib/ObjectYAML/DWARFYAML.cpp
  llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
  llvm/test/ObjectYAML/MachO/DWARF2-AddrSize8-FormValues.yaml
  llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
  llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp
@@ -34,8 +34,7 @@
   - Attribute:   DW_AT_call_data_location
 Form:DW_FORM_sec_offset
 debug_info:
-  - Length:  0
-Version: 5
+  - Version: 5
 UnitType:DW_UT_compile
 AbbrOffset:  0
 AddrSize:4
@@ -49,7 +48,7 @@
   - Value:   25
   )";
   Expected>> Sections =
-  DWARFYAML::emitDebugSections(StringRef(yamldata), /*ApplyFixups=*/true,
+  DWARFYAML::emitDebugSections(StringRef(yamldata),
/*IsLittleEndian=*/true);
   ASSERT_THAT_EXPECTED(Sections, Succeeded());
   std::vector Loclists{
Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -1368,8 +1368,7 @@
  "Children:DW_CHILDREN_yes\n"
  "Attributes:\n"
  "debug_info:\n"
- "  - Length:  0\n"
- "Version: 4\n"
+ "  - Version: 4\n"
  "AbbrOffset:  0\n"
  "AddrSize:8\n"
  "Entries:\n"
@@ -1378,7 +1377,7 @@
  "  - AbbrCode:0x\n"
  "Values:\n";
 
-  auto ErrOrSections = DWARFYAML::emitDebugSections(StringRef(yamldata), true);
+  auto ErrOrSections = DWARFYAML::emitDebugSections(StringRef(yamldata));
   ASSERT_TRUE((bool)ErrOrSections);
   std::unique_ptr DwarfContext =
   DWARFContext::create(*ErrOrSections, 8);
Index: llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
===
--- llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
+++ llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
@@ -513,7 +513,7 @@
 
 #  DWARF32: DWARF:
 #  DWARF32:   debug_info:
-# DWARF32-NEXT: - Length: 118
+# DWARF32-NEXT: - Length: 0x0076
 # DWARF32-NEXT:   Version:5
 # DWARF32-NEXT:   UnitType:   DW_UT_compile
 # DWARF32-NEXT:   AbbrOffset: 0
@@ -585,7 +585,7 @@
 # RUN:   obj2yaml | FileCheck %s --check-prefix=DWARF32-YAML
 
 #  DWARF32-YAML: debug_info:
-# DWARF32-YAML-NEXT:   - Length: 12
+# DWARF32-YAML-NEXT:   - Length: 0x000C
 # DWARF32-YAML-NEXT: Version:5
 # DWARF32-YAML-NEXT: UnitType:   DW_UT_compile
 # DWARF32-YAML-NEXT: AbbrOffset: 0x
@@ -669,7 +669,7 @@
 
 #  DWARF64-YAML: debug_info:
 # DWARF64-YAML-NEXT:   - Format: DWARF64
-# DWARF64-YAML-NEXT: Length: 20
+# DWARF64-YAML-NEXT: Length: 0x0014
 # DWARF64-YAML-NEXT: Version:5
 # DWARF64-YAML-NEXT: UnitType:   DW_UT_compile
 # DWARF64-YAML-NEXT: AbbrOffset: 0x
Index: llvm/test/ObjectYAML/MachO/DWARF2-AddrSize8-FormValues.yaml
===
--- llvm/test/ObjectYAML/MachO/DWARF2-AddrSize8-FormValues.yaml
+++ llvm/test/ObjectYAML/MachO/DWARF2-AddrSize8-FormValues.yaml
@@ -415,7 +415,7 @@
 ...
 
 #CHECK:   debug_info:  

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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

This now looks good to me. I'll give the others a chance to take a look too but 
beside that I think this is ready to go IMHO.


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [lldb] 1b7c9ea - [lldb] Store StackFrameRecognizers in the target instead of a global list

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-17T09:26:27+02:00
New Revision: 1b7c9eae6dcad09c264e29e41a69bed0d34c2f5f

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

LOG: [lldb] Store StackFrameRecognizers in the target instead of a global list

Summary:

Currently the frame recognizers are stored in a global list (the list in the
StackFrameRecognizersManagerImpl singleton to be precise). All commands and
plugins that modify the list are just modifying that global list of recognizers
which is shared by all Target and Debugger instances.

This is clearly against the idea of LLDB being usable as a library and it also
leads to some very obscure errors as now multiple tests are sharing the used
frame recognizers. For example D83400 is currently failing as it reorders some
test_ functions which permanently changes the frame recognizers of all
debuggers/targets. As all frame recognizers are also initialized in a 'once'
guard, it's also impossible to every restore back the original frame recognizers
once they are deleted in a process.

This patch just moves the frame recognizers into the current target. This seems
the way everyone assumes the system works as for example the assert frame
recognizers is using the current target to find the function/so-name to look for
(which only works if the recognizers are stored in the target).

Reviewers: jingham, mib

Reviewed By: jingham, mib

Subscribers: MrHate, JDevlieghere

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

Added: 


Modified: 
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/lldb-forward.h
lldb/source/Commands/CommandObjectFrame.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Target/AssertFrameRecognizer.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameRecognizer.cpp
lldb/source/Target/Target.cpp
lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
lldb/unittests/Target/StackFrameRecognizerTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/StackFrameRecognizer.h 
b/lldb/include/lldb/Target/StackFrameRecognizer.h
index 9c9105ac04e4..38b21e7c9856 100644
--- a/lldb/include/lldb/Target/StackFrameRecognizer.h
+++ b/lldb/include/lldb/Target/StackFrameRecognizer.h
@@ -17,6 +17,8 @@
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-public.h"
 
+#include 
+
 namespace lldb_private {
 
 /// \class RecognizedStackFrame
@@ -95,37 +97,45 @@ class ScriptedStackFrameRecognizer : public 
StackFrameRecognizer {
   operator=(const ScriptedStackFrameRecognizer &) = delete;
 };
 
-/// \class StackFrameRecognizerManager
-///
-/// Static class that provides a registry of known stack frame recognizers.
-/// Has static methods to add, enumerate, remove, query and invoke recognizers.
-
+/// Class that provides a registry of known stack frame recognizers.
 class StackFrameRecognizerManager {
 public:
-  static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
-ConstString module,
-llvm::ArrayRef symbols,
-bool first_instruction_only = true);
+  void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
+ ConstString module, llvm::ArrayRef symbols,
+ bool first_instruction_only = true);
+
+  void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
+ lldb::RegularExpressionSP module,
+ lldb::RegularExpressionSP symbol,
+ bool first_instruction_only = true);
 
-  static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
-lldb::RegularExpressionSP module,
-lldb::RegularExpressionSP symbol,
-bool first_instruction_only = true);
+  void ForEach(std::function<
+   void(uint32_t recognizer_id, std::string recognizer_name,
+std::string module, llvm::ArrayRef symbols,
+bool regexp)> const );
 
-  static void
-  ForEach(std::function symbols,
- bool regexp)> const );
+  bool RemoveRecognizerWithID(uint32_t recognizer_id);
 
-  static bool RemoveRecognizerWithID(uint32_t recognizer_id);
+  void RemoveAllRecognizers();
 
-  static void RemoveAllRecognizers();
+  lldb::StackFrameRecognizerSP GetRecognizerForFrame(lldb::StackFrameSP frame);
 
-  static lldb::StackFrameRecognizerSP GetRecognizerForFrame(
-  lldb::StackFrameSP frame);
+  lldb::RecognizedStackFrameSP RecognizeFrame(lldb::StackFrameSP frame);
+
+private:
+  struct RegisteredEntry {
+uint32_t recognizer_id;
+bool deleted;
+

[Lldb-commits] [PATCH] D83757: [lldb] Store StackFrameRecognizers in the target instead of a global list

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1b7c9eae6dca: [lldb] Store StackFrameRecognizers in the 
target instead of a global list (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83757

Files:
  lldb/include/lldb/Target/StackFrameRecognizer.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Commands/CommandObjectFrame.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
  lldb/unittests/Target/StackFrameRecognizerTest.cpp

Index: lldb/unittests/Target/StackFrameRecognizerTest.cpp
===
--- lldb/unittests/Target/StackFrameRecognizerTest.cpp
+++ lldb/unittests/Target/StackFrameRecognizerTest.cpp
@@ -51,18 +51,14 @@
   std::string GetName() override { return "Dummy StackFrame Recognizer"; }
 };
 
-void RegisterDummyStackFrameRecognizer() {
-  static llvm::once_flag g_once_flag;
+void RegisterDummyStackFrameRecognizer(StackFrameRecognizerManager ) {
+  RegularExpressionSP module_regex_sp = nullptr;
+  RegularExpressionSP symbol_regex_sp(new RegularExpression("boom"));
 
-  llvm::call_once(g_once_flag, []() {
-RegularExpressionSP module_regex_sp = nullptr;
-RegularExpressionSP symbol_regex_sp(new RegularExpression("boom"));
+  StackFrameRecognizerSP dummy_recognizer_sp(new DummyStackFrameRecognizer());
 
-StackFrameRecognizerSP dummy_recognizer_sp(new DummyStackFrameRecognizer());
-
-StackFrameRecognizerManager::AddRecognizer(
-dummy_recognizer_sp, module_regex_sp, symbol_regex_sp, false);
-  });
+  manager.AddRecognizer(dummy_recognizer_sp, module_regex_sp, symbol_regex_sp,
+false);
 }
 
 } // namespace
@@ -71,13 +67,15 @@
   DebuggerSP debugger_sp = Debugger::CreateInstance();
   ASSERT_TRUE(debugger_sp);
 
-  RegisterDummyStackFrameRecognizer();
+  StackFrameRecognizerManager manager;
+
+  RegisterDummyStackFrameRecognizer(manager);
 
   bool any_printed = false;
-  StackFrameRecognizerManager::ForEach(
-  [_printed](uint32_t recognizer_id, std::string name,
- std::string function, llvm::ArrayRef symbols,
- bool regexp) { any_printed = true; });
+  manager.ForEach([_printed](uint32_t recognizer_id, std::string name,
+ std::string function,
+ llvm::ArrayRef symbols,
+ bool regexp) { any_printed = true; });
 
   EXPECT_TRUE(any_printed);
 }
Index: lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
===
--- lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -145,6 +145,50 @@
 self.expect("frame recognizer info 0",
 substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])
 
+@skipUnlessDarwin
+def test_frame_recognizer_target_specific(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Clear internal & plugins recognizers that get initialized at launch
+self.runCmd("frame recognizer clear")
+
+# Create a target.
+target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo",
+ exe_name = exe)
+
+self.runCmd("command script import " + os.path.join(self.getSourceDir(), "recognizer.py"))
+
+# Check that this doesn't contain our own FrameRecognizer somehow.
+self.expect("frame recognizer list",
+matching=False, substrs=['MyFrameRecognizer'])
+
+# Add a frame recognizer in that target.
+self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar")
+
+self.expect("frame recognizer list",
+substrs=['recognizer.MyFrameRecognizer, module a.out, symbol foo, symbol bar'])
+
+self.expect("frame recognizer info 0",
+substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])
+
+# Create a second target. That one shouldn't have the frame recognizer.
+target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "bar",
+ exe_name = exe)
+
+self.expect("frame recognizer info 0",
+substrs=['frame 0 not recognized by any recognizer'])
+
+# Add a frame recognizer to 

[Lldb-commits] [PATCH] D80724: [lldb] Only set the executable module for a target once

[Raphael Isemann via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG16926115ed28: [lldb] Only set the executable module for a 
target once (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80724

Files:
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/test/API/functionalities/dlopen_other_executable/Makefile
  
lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
  lldb/test/API/functionalities/dlopen_other_executable/main.c
  lldb/test/API/functionalities/dlopen_other_executable/other.c


Index: lldb/test/API/functionalities/dlopen_other_executable/other.c
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen_other_executable/other.c
@@ -0,0 +1 @@
+int main() {}
Index: lldb/test/API/functionalities/dlopen_other_executable/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen_other_executable/main.c
@@ -0,0 +1,10 @@
+#include 
+#include 
+
+int main() {
+  int i = 0; // break here
+  // dlopen the 'other' test executable.
+  int h = dlopen("other", RTLD_LAZY);
+  assert(h && "dlopen failed?");
+  return i; // break after dlopen
+}
Index: 
lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
@@ -0,0 +1,42 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfRemote
+@skipIfWindows
+# glibc's dlopen doesn't support opening executables.
+# https://sourceware.org/bugzilla/show_bug.cgi?id=11754
+@skipIfLinux
+@no_debug_info_test
+def test(self):
+self.build()
+# Launch and stop before the dlopen call.
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.c"))
+
+# Delete the breakpoint we no longer need.
+self.target().DeleteAllBreakpoints()
+
+# Check that the executable is the test binary.
+self.assertEqual(self.target().GetExecutable().GetFilename(), "a.out")
+
+# Continue so that dlopen is called.
+breakpoint = self.target().BreakpointCreateBySourceRegex(
+"// break after dlopen", lldb.SBFileSpec("main.c"))
+self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), 
breakpoint)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that the executable is still the test binary and not "other".
+self.assertEqual(self.target().GetExecutable().GetFilename(), "a.out")
+
+# Kill the process and run the program again.
+err = self.process().Kill()
+self.assertTrue(err.Success(), str(err))
+
+# Test that we hit the breakpoint after dlopen.
+lldbutil.run_to_breakpoint_do_run(self, self.target(), breakpoint)
Index: lldb/test/API/functionalities/dlopen_other_executable/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen_other_executable/Makefile
@@ -0,0 +1,8 @@
+C_SOURCES := main.c
+USE_LIBDL := 1
+
+other:
+   $(MAKE) -f $(MAKEFILE_RULES) C_SOURCES=other.c EXE=other
+all: other
+
+include Makefile.rules
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -577,7 +577,8 @@
 }
   }
 
-  if (exe_idx != UINT32_MAX) {
+  // Set the target executable if we haven't found one so far.
+  if (exe_idx != UINT32_MAX && !target.GetExecutableModule()) {
 const bool can_create = true;
 ModuleSP exe_module_sp(FindTargetModuleForImageInfo(image_infos[exe_idx],
 can_create, nullptr));


Index: lldb/test/API/functionalities/dlopen_other_executable/other.c
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen_other_executable/other.c
@@ -0,0 +1 @@
+int main() {}
Index: lldb/test/API/functionalities/dlopen_other_executable/main.c
===
--- /dev/null
+++ 

[Lldb-commits] [lldb] 1692611 - [lldb] Only set the executable module for a target once

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-17T08:35:38+02:00
New Revision: 16926115ed28d1370bca1085dfb1b20c842b0ffb

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

LOG: [lldb] Only set the executable module for a target once

Summary:

When we try to find the executable module for our target we don't check
if we already have an executable module set. This causes that when debugging
a program that dlopens another executable, LLDB will take that other executable
as the new executable of the target (which causes that future launches of the
target will launch the dlopen'd executable instead of the original executable).

This just adds a check that we only set the executable when we haven't already
found one.

Fixes rdar://63443099

Reviewers: jasonmolenda, jingham, teemperor

Reviewed By: jasonmolenda, teemperor

Subscribers: jingham, JDevlieghere

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

Added: 
lldb/test/API/functionalities/dlopen_other_executable/Makefile

lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
lldb/test/API/functionalities/dlopen_other_executable/main.c
lldb/test/API/functionalities/dlopen_other_executable/other.c

Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 731004340434..569d84d39c80 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -577,7 +577,8 @@ void 
DynamicLoaderDarwin::UpdateSpecialBinariesFromNewImageInfos(
 }
   }
 
-  if (exe_idx != UINT32_MAX) {
+  // Set the target executable if we haven't found one so far.
+  if (exe_idx != UINT32_MAX && !target.GetExecutableModule()) {
 const bool can_create = true;
 ModuleSP exe_module_sp(FindTargetModuleForImageInfo(image_infos[exe_idx],
 can_create, nullptr));

diff  --git a/lldb/test/API/functionalities/dlopen_other_executable/Makefile 
b/lldb/test/API/functionalities/dlopen_other_executable/Makefile
new file mode 100644
index ..113b9fd7d3f1
--- /dev/null
+++ b/lldb/test/API/functionalities/dlopen_other_executable/Makefile
@@ -0,0 +1,8 @@
+C_SOURCES := main.c
+USE_LIBDL := 1
+
+other:
+   $(MAKE) -f $(MAKEFILE_RULES) C_SOURCES=other.c EXE=other
+all: other
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
 
b/lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
new file mode 100644
index ..2ccfaeaea41a
--- /dev/null
+++ 
b/lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
@@ -0,0 +1,42 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfRemote
+@skipIfWindows
+# glibc's dlopen doesn't support opening executables.
+# https://sourceware.org/bugzilla/show_bug.cgi?id=11754
+@skipIfLinux
+@no_debug_info_test
+def test(self):
+self.build()
+# Launch and stop before the dlopen call.
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.c"))
+
+# Delete the breakpoint we no longer need.
+self.target().DeleteAllBreakpoints()
+
+# Check that the executable is the test binary.
+self.assertEqual(self.target().GetExecutable().GetFilename(), "a.out")
+
+# Continue so that dlopen is called.
+breakpoint = self.target().BreakpointCreateBySourceRegex(
+"// break after dlopen", lldb.SBFileSpec("main.c"))
+self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), 
breakpoint)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that the executable is still the test binary and not "other".
+self.assertEqual(self.target().GetExecutable().GetFilename(), "a.out")
+
+# Kill the process and run the program again.
+err = self.process().Kill()
+self.assertTrue(err.Success(), str(err))
+
+# Test that we hit the breakpoint after dlopen.
+lldbutil.run_to_breakpoint_do_run(self, self.target(), breakpoint)

diff  --git a/lldb/test/API/functionalities/dlopen_other_executable/main.c 
b/lldb/test/API/functionalities/dlopen_other_executable/main.c
new file mode 100644
index 

[Lldb-commits] [PATCH] D83957: [lldb/DWARF] Don't get confused by line sequences with tombstone values

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1040
   std::unique_ptr sequence =
   LineTable::CreateLineSequenceContainer();
   std::vector> sequences;

While here, delete the initialization and move `sequence = 
LineTable::CreateLineSequenceContainer()` to the top of the loop body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83957



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