[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited, part 2

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added a comment.

I think I fixed the issue: I was confusing `/lib` stemming from lib subdirs in 
the source and `/lib` stemming from building things in a lib directory that 
will later be installed also to a lib directory. It is just the latter case 
that uses the suffix, and so just the latter case will be so `sed`ed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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

[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited, part 2

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 459975.
Ericson2314 added a comment.

Require `LLVM_LIBDIR_SUFFIX` in the sed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

Files:
  clang/cmake/modules/CMakeLists.txt
  flang/cmake/modules/CMakeLists.txt
  lld/cmake/modules/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  mlir/cmake/modules/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/test/CMakeLists.txt

Index: polly/test/CMakeLists.txt
===
--- polly/test/CMakeLists.txt
+++ polly/test/CMakeLists.txt
@@ -46,7 +46,7 @@
 
 set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
-set(LLVM_LIBS_DIR "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
+set(LLVM_LIBS_DIR "${LLVM_LIBRARY_DIR}")
 if (CMAKE_LIBRARY_OUTPUT_DIRECTORY)
   set(POLLY_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
 else()
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -12,7 +12,7 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 if (CMAKE_CONFIGURATION_TYPES)
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-$>.cmake")
Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -15,7 +15,7 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 get_property(MLIR_EXPORTS GLOBAL PROPERTY MLIR_EXPORTS)
 export(TARGETS ${MLIR_EXPORTS} FILE ${mlir_cmake_builddir}/MLIRTargets.cmake)
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -3,7 +3,7 @@
 include(FindPrefixFromConfig)
 
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 # First for users who use an installed LLVM, create the LLVMExports.cmake file.
 set(LLVM_EXPORTS_FILE ${llvm_cmake_builddir}/LLVMExports.cmake)
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1093,7 +1093,7 @@
   message(FATAL_ERROR "LLVM_INSTALL_PACKAGE_DIR must be defined and writable. GEN_CONFIG should only be passe when building LLVM proper.")
   endif()
   # LLVM_INSTALL_PACKAGE_DIR might be absolute, so don't reuse below.
-  set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+  set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
   file(WRITE
   "${llvm_cmake_builddir}/LLVMConfigExtensions.cmake"
   "set(LLVM_STATIC_EXTENSIONS ${LLVM_STATIC_EXTENSIONS})")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -940,9 +940,9 @@
   )
 
 # They are not referenced. See set_output_directory().
-set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/bin )
-set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
-set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
+set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_TOOLS_BINARY_DIR} )
+set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
+set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
 
 if(LLVM_INCLUDE_TESTS)
   include(GetErrcMessages)
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -276,7 +276,7 @@
   # could be and pick the first that exists.
   foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
 "${LLVM_BUILD_LIBRARY_DIR}"
-"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
+"${LLVM_LIBRARY_DIR}")
 # Build 

[Lldb-commits] [PATCH] D133828: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited, part 1

[John Ericson via Phabricator via lldb-commits]

Ericson2314 created this revision.
Herald added a subscriber: mgorny.
Herald added a project: All.
Ericson2314 requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

A simple sed doing these substitutions:

- `${LLVM_BINARY_DIR}/\$\{CMAKE_CFG_INTDIR}/lib(${LLVM_LIBDIR_SUFFIX})?\>` -> 
`${LLVM_LIBRARY_DIR}`
- `${LLVM_BINARY_DIR}/\$\{CMAKE_CFG_INTDIR}/bin\>` -> `${LLVM_TOOLS_BINARY_DIR}`

where `\>` means "word boundary".

The only manual modifications were reverting changes in

- `compiler-rt/cmake/Modules/CompilerRTUtils.cmake`

because these were "entry points" where we wanted to tread carefully not not 
introduce a "loop" which would end with an undefined variable being expanded to 
nothing.

There are many more occurrences without `CMAKE_CFG_INTDIR`, but those
are left for D132316  as they have proved 
somewhat tricky to fix.

This hopefully increases readability overall, and also decreases the usages of 
`LLVM_LIBDIR_SUFFIX`, preparing us for D130586 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133828

Files:
  lldb/test/API/CMakeLists.txt
  llvm/tools/llvm-shlib/CMakeLists.txt


Index: llvm/tools/llvm-shlib/CMakeLists.txt
===
--- llvm/tools/llvm-shlib/CMakeLists.txt
+++ llvm/tools/llvm-shlib/CMakeLists.txt
@@ -88,7 +88,7 @@
 
   set(LLVM_EXPORTED_SYMBOL_FILE ${LLVM_BINARY_DIR}/libllvm-c.exports)
 
-  set(LIB_DIR ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
+  set(LIB_DIR ${LLVM_LIBRARY_DIR})
   set(LIB_NAME ${LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}LLVM)
   set(LIB_PATH ${LIB_NAME}${CMAKE_SHARED_LIBRARY_SUFFIX})
   set(LIB_EXPORTS_PATH ${LIB_NAME}.exports)
@@ -136,7 +136,7 @@
 
   # Get the full name to the libs so the python script understands them.
   foreach(lib ${LIB_NAMES})
-list(APPEND FULL_LIB_NAMES 
${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${lib}.lib)
+list(APPEND FULL_LIB_NAMES ${LLVM_LIBRARY_DIR}/${lib}.lib)
   endforeach()
 
   # Need to separate lib names with newlines.
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -45,10 +45,10 @@
 # Set the path to the default lldb test executable.
 set(LLDB_DEFAULT_TEST_EXECUTABLE 
"${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
 
-set(LLDB_DEFAULT_TEST_DSYMUTIL 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
+set(LLDB_DEFAULT_TEST_DSYMUTIL 
"${LLVM_TOOLS_BINARY_DIR}/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
 
 if (TARGET clang)
-  set(LLDB_DEFAULT_TEST_COMPILER 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
+  set(LLDB_DEFAULT_TEST_COMPILER 
"${LLVM_TOOLS_BINARY_DIR}/clang${CMAKE_EXECUTABLE_SUFFIX}")
 else()
   set(LLDB_DEFAULT_TEST_COMPILER "")
 endif()


Index: llvm/tools/llvm-shlib/CMakeLists.txt
===
--- llvm/tools/llvm-shlib/CMakeLists.txt
+++ llvm/tools/llvm-shlib/CMakeLists.txt
@@ -88,7 +88,7 @@
 
   set(LLVM_EXPORTED_SYMBOL_FILE ${LLVM_BINARY_DIR}/libllvm-c.exports)
 
-  set(LIB_DIR ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
+  set(LIB_DIR ${LLVM_LIBRARY_DIR})
   set(LIB_NAME ${LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}LLVM)
   set(LIB_PATH ${LIB_NAME}${CMAKE_SHARED_LIBRARY_SUFFIX})
   set(LIB_EXPORTS_PATH ${LIB_NAME}.exports)
@@ -136,7 +136,7 @@
 
   # Get the full name to the libs so the python script understands them.
   foreach(lib ${LIB_NAMES})
-list(APPEND FULL_LIB_NAMES ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${lib}.lib)
+list(APPEND FULL_LIB_NAMES ${LLVM_LIBRARY_DIR}/${lib}.lib)
   endforeach()
 
   # Need to separate lib names with newlines.
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -45,10 +45,10 @@
 # Set the path to the default lldb test executable.
 set(LLDB_DEFAULT_TEST_EXECUTABLE "${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
 
-set(LLDB_DEFAULT_TEST_DSYMUTIL "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
+set(LLDB_DEFAULT_TEST_DSYMUTIL "${LLVM_TOOLS_BINARY_DIR}/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
 
 if (TARGET clang)
-  set(LLDB_DEFAULT_TEST_COMPILER "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
+  set(LLDB_DEFAULT_TEST_COMPILER "${LLVM_TOOLS_BINARY_DIR}/clang${CMAKE_EXECUTABLE_SUFFIX}")
 else()
   set(LLDB_DEFAULT_TEST_COMPILER "")
 endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133530: [lldb] Add zstd support

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

Ping for unresolved issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133530

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

[Lldb-commits] [PATCH] D133680: Add support for mach-o corefile loading of a platform binary

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG92bd2e443e8f: Add mach-o corefile support for platform 
binaries (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133680

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -85,7 +85,14 @@
 lldb_private::MemoryRegionInfo _info) override;
 
 private:
-  bool GetDynamicLoaderAddress(lldb::addr_t addr);
+  void CreateMemoryRegions();
+  void LoadBinariesViaMetadata();
+  void LoadBinariesViaExhaustiveSearch();
+  void LoadBinariesAndSetDYLD();
+  void CleanupMemoryRegionPermissions();
+
+  bool CheckAddressForDyldOrKernel(lldb::addr_t addr, lldb::addr_t ,
+   lldb::addr_t );
 
   enum CorefilePreference { eUserProcessCorefile, eKernelCorefile };
 
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -40,6 +40,7 @@
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwinKernel.h"
 
 #include 
 #include 
@@ -124,10 +125,13 @@
   Finalize();
 }
 
-bool ProcessMachCore::GetDynamicLoaderAddress(lldb::addr_t addr) {
+bool ProcessMachCore::CheckAddressForDyldOrKernel(lldb::addr_t addr,
+  addr_t ,
+  addr_t ) {
   Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
   llvm::MachO::mach_header header;
   Status error;
+  dyld = kernel = LLDB_INVALID_ADDRESS;
   if (DoReadMemory(addr, , sizeof(header), error) != sizeof(header))
 return false;
   if (header.magic == llvm::MachO::MH_CIGAM ||
@@ -141,9 +145,6 @@
 header.flags = llvm::ByteSwap_32(header.flags);
   }
 
-  // TODO: swap header if needed...
-  // printf("0x%16.16" PRIx64 ": magic = 0x%8.8x, file_type= %u\n", vaddr,
-  // header.magic, header.filetype);
   if (header.magic == llvm::MachO::MH_MAGIC ||
   header.magic == llvm::MachO::MH_MAGIC_64) {
 // Check MH_EXECUTABLE to see if we can find the mach image that contains
@@ -152,26 +153,23 @@
 // has the list of kexts to load
 switch (header.filetype) {
 case llvm::MachO::MH_DYLINKER:
-  // printf("0x%16.16" PRIx64 ": file_type = MH_DYLINKER\n", vaddr);
-  // Address of dyld "struct mach_header" in the core file
   LLDB_LOGF(log,
-"ProcessMachCore::GetDynamicLoaderAddress found a user "
+"ProcessMachCore::%s found a user "
 "process dyld binary image at 0x%" PRIx64,
-addr);
-  m_dyld_addr = addr;
+__FUNCTION__, addr);
+  dyld = addr;
   return true;
 
 case llvm::MachO::MH_EXECUTE:
-  // printf("0x%16.16" PRIx64 ": file_type = MH_EXECUTE\n", vaddr);
   // Check MH_EXECUTABLE file types to see if the dynamic link object flag
   // is NOT set. If it isn't, then we have a mach_kernel.
   if ((header.flags & llvm::MachO::MH_DYLDLINK) == 0) {
 LLDB_LOGF(log,
-  "ProcessMachCore::GetDynamicLoaderAddress found a mach "
+  "ProcessMachCore::%s found a mach "
   "kernel binary image at 0x%" PRIx64,
-  addr);
+  __FUNCTION__, addr);
 // Address of the mach kernel "struct mach_header" in the core file.
-m_mach_kernel_addr = addr;
+kernel = addr;
 return true;
   }
   break;
@@ -180,46 +178,10 @@
   return false;
 }
 
-// Process Control
-Status ProcessMachCore::DoLoadCore() {
-  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
-  Status error;
-  if (!m_core_module_sp) {
-error.SetErrorString("invalid core module");
-return error;
-  }
-
+void ProcessMachCore::CreateMemoryRegions() {
   ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
-  if (core_objfile == nullptr) {
-error.SetErrorString("invalid core object file");
-return error;
-  }
-
-  if (core_objfile->GetNumThreadContexts() == 0) {
-error.SetErrorString("core file doesn't contain 

[Lldb-commits] [lldb] 92bd2e4 - Add mach-o corefile support for platform binaries

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-09-13T15:46:18-07:00
New Revision: 92bd2e443e8f0337ef5cc55c9b4a7ea6e3f450b6

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

LOG: Add mach-o corefile support for platform binaries

Add support for recognizing a platform binary in the ObjectFileMachO
method that parses the "load binary" LC_NOTEs in a corefile.

A bit of reorganization to ProcessMachCore::DoLoadCore to separate
all of the unrelated things being done in that method into their own
separate methods, as well as small fixes to improve the handling of
a corefile with multiple kernel images in the corefile.

Differential Revision: https://reviews.llvm.org/D133680
rdar://98754861

Added: 


Modified: 
lldb/include/lldb/Target/DynamicLoader.h
lldb/source/Core/DynamicLoader.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

Removed: 




diff  --git a/lldb/include/lldb/Target/DynamicLoader.h 
b/lldb/include/lldb/Target/DynamicLoader.h
index 7e7a4218810b5..22d047ab4b616 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -226,6 +226,13 @@ class DynamicLoader : public PluginInterface {
   /// \param[in] process
   /// The process to add this binary to.
   ///
+  /// \param[in] name
+  /// Name of the binary, if available.  If this method cannot find a
+  /// matching binary on the debug host, it may create a memory module
+  /// out of live memory, and the provided name will be used.  If an
+  /// empty StringRef is provided, a name will be constructed for the 
module
+  /// based on the address it is loaded at.
+  ///
   /// \param[in] uuid
   /// UUID of the binary to be loaded.  UUID may be empty, and if a
   /// load address is supplied, will read the binary from memory, get
@@ -251,10 +258,9 @@ class DynamicLoader : public PluginInterface {
   ///
   /// \return
   /// Returns a shared pointer for the Module that has been added.
-  static lldb::ModuleSP
-  LoadBinaryWithUUIDAndAddress(Process *process, UUID uuid, lldb::addr_t value,
-   bool value_is_offset, bool force_symbol_search,
-   bool notify);
+  static lldb::ModuleSP LoadBinaryWithUUIDAndAddress(
+  Process *process, llvm::StringRef name, UUID uuid, lldb::addr_t value,
+  bool value_is_offset, bool force_symbol_search, bool notify);
 
   /// Get information about the shared cache for a process, if possible.
   ///

diff  --git a/lldb/source/Core/DynamicLoader.cpp 
b/lldb/source/Core/DynamicLoader.cpp
index 911101c0a7740..8849ccedbd481 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -175,17 +175,19 @@ ModuleSP DynamicLoader::LoadModuleAtAddress(const 
FileSpec ,
   return nullptr;
 }
 
-static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr) {
+static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr,
+llvm::StringRef name) {
   char namebuf[80];
-  snprintf(namebuf, sizeof(namebuf), "memory-image-0x%" PRIx64, addr);
-  return process->ReadModuleFromMemory(FileSpec(namebuf), addr);
+  if (name.empty()) {
+snprintf(namebuf, sizeof(namebuf), "memory-image-0x%" PRIx64, addr);
+name = namebuf;
+  }
+  return process->ReadModuleFromMemory(FileSpec(name), addr);
 }
 
-ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(Process *process,
- UUID uuid, addr_t value,
- bool value_is_offset,
- bool force_symbol_search,
- bool notify) {
+ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
+Process *process, llvm::StringRef name, UUID uuid, addr_t value,
+bool value_is_offset, bool force_symbol_search, bool notify) {
   ModuleSP memory_module_sp;
   ModuleSP module_sp;
   PlatformSP platform_sp = process->GetTarget().GetPlatform();
@@ -195,7 +197,7 @@ ModuleSP 
DynamicLoader::LoadBinaryWithUUIDAndAddress(Process *process,
   module_spec.GetUUID() = uuid;
 
   if (!uuid.IsValid() && !value_is_offset) {
-memory_module_sp = ReadUnnamedMemoryModule(process, value);
+memory_module_sp = ReadUnnamedMemoryModule(process, value, name);
 
 if (memory_module_sp)
   uuid = memory_module_sp->GetUUID();
@@ -223,7 +225,7 @@ ModuleSP 
DynamicLoader::LoadBinaryWithUUIDAndAddress(Process *process,
   // read it out of memory.
   if (!module_sp.get() && value 

[Lldb-commits] [lldb] 4fdda4f - [lldb][fuzz] Allow expression fuzzer to be passed as a flag.

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2022-09-13T15:39:15-07:00
New Revision: 4fdda4f82d787c988d5bbdf365df95fd0c8ca14b

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

LOG: [lldb][fuzz] Allow expression fuzzer to be passed as a flag.

The expression fuzzer checks an environment variable, `LLDB_FUZZER_TARGET`, to 
get the fuzzer target binary. This is fine, but internally our tooling for 
running fuzz tests only has proper handling for flag values. It's surprisingly 
complicated to add support for that, and allowing it to be passed via flag 
seems reasonable anyway.

Reviewed By: cassanova

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

Added: 


Modified: 
lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp

Removed: 




diff  --git 
a/lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp 
b/lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
index d8b8ff3326903..2832651eb9d9d 100644
--- a/lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
+++ b/lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
@@ -35,7 +35,7 @@ using namespace lldb;
 using namespace llvm;
 using namespace clang_fuzzer;
 
-char *target_path;
+const char *target_path = nullptr;
 
 void ReportError(llvm::StringRef message) {
   WithColor::error() << message << '\n';
@@ -47,10 +47,24 @@ extern "C" int LLVMFuzzerInitialize(int *argc, char 
***argv) {
   signal(SIGPIPE, SIG_IGN);
 #endif
 
-  target_path = ::getenv("LLDB_FUZZER_TARGET");
+  // `target_path` can be set by either the "--lldb_fuzzer_target" commandline
+  // flag or the "LLDB_FUZZER_TARGET" environment variable. Arbitrarily, we
+  // always do flag parsing and only check the environment variable if the
+  // commandline flag is not set.
+  for (int i = 1; i < *argc; ++i) {
+auto this_arg = llvm::StringRef((*argv)[i]);
+WithColor::note() << "argv[" << i << "] = " << this_arg << "\n";
+if (this_arg.consume_front("--lldb_fuzzer_target="))
+  target_path = this_arg.data();
+  }
+
+  if (!target_path)
+target_path = ::getenv("LLDB_FUZZER_TARGET");
+
   if (!target_path)
-ReportError(
-"no target path specified in with the LLDB_FUZZER_TARGET variable");
+ReportError("No target path specified. Set one either as an environment "
+"variable (i.e. LLDB_FUZZER_TARGET=target_path) or pass as a "
+"command line flag (i.e. --lldb_fuzzer_target=target_path).");
 
   if (!sys::fs::exists(target_path))
 ReportError(formatv("target path '{0}' does not exist", 
target_path).str());



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

[Lldb-commits] [PATCH] D133546: [lldb][fuzz] Allow expression fuzzer to be passed as a flag.

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
rupprecht marked an inline comment as done.
Closed by commit rG4fdda4f82d78: [lldb][fuzz] Allow expression fuzzer to be 
passed as a flag. (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133546

Files:
  lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp


Index: lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
===
--- lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
+++ lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
@@ -35,7 +35,7 @@
 using namespace llvm;
 using namespace clang_fuzzer;
 
-char *target_path;
+const char *target_path = nullptr;
 
 void ReportError(llvm::StringRef message) {
   WithColor::error() << message << '\n';
@@ -47,10 +47,24 @@
   signal(SIGPIPE, SIG_IGN);
 #endif
 
-  target_path = ::getenv("LLDB_FUZZER_TARGET");
+  // `target_path` can be set by either the "--lldb_fuzzer_target" commandline
+  // flag or the "LLDB_FUZZER_TARGET" environment variable. Arbitrarily, we
+  // always do flag parsing and only check the environment variable if the
+  // commandline flag is not set.
+  for (int i = 1; i < *argc; ++i) {
+auto this_arg = llvm::StringRef((*argv)[i]);
+WithColor::note() << "argv[" << i << "] = " << this_arg << "\n";
+if (this_arg.consume_front("--lldb_fuzzer_target="))
+  target_path = this_arg.data();
+  }
+
+  if (!target_path)
+target_path = ::getenv("LLDB_FUZZER_TARGET");
+
   if (!target_path)
-ReportError(
-"no target path specified in with the LLDB_FUZZER_TARGET variable");
+ReportError("No target path specified. Set one either as an environment "
+"variable (i.e. LLDB_FUZZER_TARGET=target_path) or pass as a "
+"command line flag (i.e. --lldb_fuzzer_target=target_path).");
 
   if (!sys::fs::exists(target_path))
 ReportError(formatv("target path '{0}' does not exist", 
target_path).str());


Index: lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
===
--- lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
+++ lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
@@ -35,7 +35,7 @@
 using namespace llvm;
 using namespace clang_fuzzer;
 
-char *target_path;
+const char *target_path = nullptr;
 
 void ReportError(llvm::StringRef message) {
   WithColor::error() << message << '\n';
@@ -47,10 +47,24 @@
   signal(SIGPIPE, SIG_IGN);
 #endif
 
-  target_path = ::getenv("LLDB_FUZZER_TARGET");
+  // `target_path` can be set by either the "--lldb_fuzzer_target" commandline
+  // flag or the "LLDB_FUZZER_TARGET" environment variable. Arbitrarily, we
+  // always do flag parsing and only check the environment variable if the
+  // commandline flag is not set.
+  for (int i = 1; i < *argc; ++i) {
+auto this_arg = llvm::StringRef((*argv)[i]);
+WithColor::note() << "argv[" << i << "] = " << this_arg << "\n";
+if (this_arg.consume_front("--lldb_fuzzer_target="))
+  target_path = this_arg.data();
+  }
+
+  if (!target_path)
+target_path = ::getenv("LLDB_FUZZER_TARGET");
+
   if (!target_path)
-ReportError(
-"no target path specified in with the LLDB_FUZZER_TARGET variable");
+ReportError("No target path specified. Set one either as an environment "
+"variable (i.e. LLDB_FUZZER_TARGET=target_path) or pass as a "
+"command line flag (i.e. --lldb_fuzzer_target=target_path).");
 
   if (!sys::fs::exists(target_path))
 ReportError(formatv("target path '{0}' does not exist", target_path).str());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133680: Add support for mach-o corefile loading of a platform binary

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 459889.
jasonmolenda added a comment.

Update to address David's feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133680

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -85,7 +85,14 @@
 lldb_private::MemoryRegionInfo _info) override;
 
 private:
-  bool GetDynamicLoaderAddress(lldb::addr_t addr);
+  void CreateMemoryRegions();
+  void LoadBinariesViaMetadata();
+  void LoadBinariesViaExhaustiveSearch();
+  void LoadBinariesAndSetDYLD();
+  void CleanupMemoryRegionPermissions();
+
+  bool CheckAddressForDyldOrKernel(lldb::addr_t addr, lldb::addr_t ,
+   lldb::addr_t );
 
   enum CorefilePreference { eUserProcessCorefile, eKernelCorefile };
 
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -40,6 +40,7 @@
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwinKernel.h"
 
 #include 
 #include 
@@ -124,10 +125,13 @@
   Finalize();
 }
 
-bool ProcessMachCore::GetDynamicLoaderAddress(lldb::addr_t addr) {
+bool ProcessMachCore::CheckAddressForDyldOrKernel(lldb::addr_t addr,
+  addr_t ,
+  addr_t ) {
   Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
   llvm::MachO::mach_header header;
   Status error;
+  dyld = kernel = LLDB_INVALID_ADDRESS;
   if (DoReadMemory(addr, , sizeof(header), error) != sizeof(header))
 return false;
   if (header.magic == llvm::MachO::MH_CIGAM ||
@@ -141,9 +145,6 @@
 header.flags = llvm::ByteSwap_32(header.flags);
   }
 
-  // TODO: swap header if needed...
-  // printf("0x%16.16" PRIx64 ": magic = 0x%8.8x, file_type= %u\n", vaddr,
-  // header.magic, header.filetype);
   if (header.magic == llvm::MachO::MH_MAGIC ||
   header.magic == llvm::MachO::MH_MAGIC_64) {
 // Check MH_EXECUTABLE to see if we can find the mach image that contains
@@ -152,26 +153,23 @@
 // has the list of kexts to load
 switch (header.filetype) {
 case llvm::MachO::MH_DYLINKER:
-  // printf("0x%16.16" PRIx64 ": file_type = MH_DYLINKER\n", vaddr);
-  // Address of dyld "struct mach_header" in the core file
   LLDB_LOGF(log,
-"ProcessMachCore::GetDynamicLoaderAddress found a user "
+"ProcessMachCore::%s found a user "
 "process dyld binary image at 0x%" PRIx64,
-addr);
-  m_dyld_addr = addr;
+__FUNCTION__, addr);
+  dyld = addr;
   return true;
 
 case llvm::MachO::MH_EXECUTE:
-  // printf("0x%16.16" PRIx64 ": file_type = MH_EXECUTE\n", vaddr);
   // Check MH_EXECUTABLE file types to see if the dynamic link object flag
   // is NOT set. If it isn't, then we have a mach_kernel.
   if ((header.flags & llvm::MachO::MH_DYLDLINK) == 0) {
 LLDB_LOGF(log,
-  "ProcessMachCore::GetDynamicLoaderAddress found a mach "
+  "ProcessMachCore::%s found a mach "
   "kernel binary image at 0x%" PRIx64,
-  addr);
+  __FUNCTION__, addr);
 // Address of the mach kernel "struct mach_header" in the core file.
-m_mach_kernel_addr = addr;
+kernel = addr;
 return true;
   }
   break;
@@ -180,46 +178,10 @@
   return false;
 }
 
-// Process Control
-Status ProcessMachCore::DoLoadCore() {
-  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
-  Status error;
-  if (!m_core_module_sp) {
-error.SetErrorString("invalid core module");
-return error;
-  }
-
+void ProcessMachCore::CreateMemoryRegions() {
   ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
-  if (core_objfile == nullptr) {
-error.SetErrorString("invalid core object file");
-return error;
-  }
-
-  if (core_objfile->GetNumThreadContexts() == 0) {
-error.SetErrorString("core file doesn't contain any LC_THREAD load "
- "commands, or the 

[Lldb-commits] [lldb] ac05bc0 - Revert "Be more careful to maintain quoting information when parsing commands."

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-09-13T14:59:21-07:00
New Revision: ac05bc0524c66c74278b26742896a4c634c034cf

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

LOG: Revert "Be more careful to maintain quoting information when parsing 
commands."

This reverts commit 6c089b2af5d8d98f66b27b67f70958f520820a76.

This was causing the test test_help_run_hides_options from TestHelp.py to
fail on Linux and Windows (but the test succeeds on macOS).  The decision
to print option information is determined by 
CommandObjectAlias::IsDashDashCommand
which was changed, but only by replacing an inline string constant with a const 
char *
CommandInterpreter::g_argument which has the same string value.  I can't see 
why this
would fail, I'll have to spin up a vm to see if I can repo there.

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Interpreter/CommandAlias.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Interpreter/Options.cpp
lldb/source/Utility/Args.cpp

Removed: 
lldb/test/API/commands/command/backticks/Makefile
lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
lldb/test/API/commands/command/backticks/main.c



diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 71587eed68c9..0f137a7b3c49 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -239,14 +239,6 @@ class CommandInterpreter : public Broadcaster,
 eCommandTypesAllThem = 0x  //< all commands
   };
 
-  // The CommandAlias and CommandInterpreter both have a hand in 
-  // substituting for alias commands.  They work by writing special tokens
-  // in the template form of the Alias command, and then detecting them when 
the
-  // command is executed.  These are the special tokens:
-  static const char * const g_no_argument;
-  static const char * const g_need_argument;
-  static const char * const g_argument;
-
   CommandInterpreter(Debugger , bool synchronous_execution);
 
   ~CommandInterpreter() override = default;

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 5c058769ff3f..101921ffc768 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2472,13 +2472,6 @@ def assertSuccess(self, obj, msg=None):
 self.fail(self._formatMessage(msg,
 "'{}' is not success".format(error)))
 
-"""Assert that a command return object is successful"""
-def assertCommandReturn(self, obj, msg=None):
-if not obj.Succeeded():
-error = obj.GetError()
-self.fail(self._formatMessage(msg,
-"'{}' is not success".format(error)))
-
 """Assert two states are equal"""
 def assertState(self, first, second, msg=None):
 if first != second:

diff  --git a/lldb/source/Interpreter/CommandAlias.cpp 
b/lldb/source/Interpreter/CommandAlias.cpp
index e36edcac15a5..d55b3fdd44fa 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -60,12 +60,11 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP 
_obj_sp,
 
   if (!options_string.empty()) {
 if (cmd_obj_sp->WantsRawCommandString())
-  option_arg_vector->emplace_back(CommandInterpreter::g_argument, 
-  -1, options_string);
+  option_arg_vector->emplace_back("", -1, options_string);
 else {
   for (auto  : args.entries()) {
 if (!entry.ref().empty())
-  
option_arg_vector->emplace_back(std::string(CommandInterpreter::g_argument), -1,
+  option_arg_vector->emplace_back(std::string(""), -1,
   std::string(entry.ref()));
   }
 }
@@ -154,12 +153,11 @@ void CommandAlias::GetAliasExpansion(StreamString 
_string) const {
 
   for (const auto _entry : *options) {
 std::tie(opt, std::ignore, value) = opt_entry;
-if (opt == CommandInterpreter::g_argument) {
+if (opt == "") {
   help_string.Printf(" %s", value.c_str());
 } else {
   help_string.Printf(" %s", opt.c_str());
-  if ((value != CommandInterpreter::g_no_argument) 
-   && (value != CommandInterpreter::g_need_argument)) {
+  if ((value != "") && (value != " 
CommandAlias::Desugar() {
 return {nullptr, nullptr};
 
   if (underlying->IsAlias()) {
-// FIXME: This doesn't work if the original alias fills a slot in the
-// underlying alias, since this just appends the two 

[Lldb-commits] [lldb] a2d0a01 - Revert "constexpr isn't right here."

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-09-13T14:58:53-07:00
New Revision: a2d0a01e3f9941d0d5370890105f70da142f493f

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

LOG: Revert "constexpr isn't right here."

This didn't help either.

This reverts commit 8433b210839ed655852428ba8b34bb67b191957a.

Added: 


Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 89878713161b..2218d54e3d97 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -105,9 +105,9 @@ static constexpr const char *InitFileWarning =
 "and\n"
 "accept the security risk.";
 
-const char * const CommandInterpreter::g_no_argument = "";
-const char * const CommandInterpreter::g_need_argument = "";
-const char * const CommandInterpreter::g_argument = "";
+constexpr const char *CommandInterpreter::g_no_argument = "";
+constexpr const char *CommandInterpreter::g_need_argument = "";
+constexpr const char *CommandInterpreter::g_argument = "";
 
 
 #define LLDB_PROPERTIES_interpreter



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

[Lldb-commits] [lldb] 3f23eb6 - Revert "Trying to understand the TestHelp.py failure from 6c089b2."

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-09-13T14:58:27-07:00
New Revision: 3f23eb6591bb1eecae38019e8ab1fdf5e2951ae9

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

LOG: Revert "Trying to understand the TestHelp.py failure from 6c089b2."

It didn't help.

This reverts commit 81f8788528886ee611041e1f4ee54eea8bbfa277.

Added: 


Modified: 
lldb/source/Interpreter/CommandAlias.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandAlias.cpp 
b/lldb/source/Interpreter/CommandAlias.cpp
index 80a926ef4d6a..e36edcac15a5 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -180,7 +180,7 @@ bool CommandAlias::IsDashDashCommand() {
 
   for (const auto _entry : *GetOptionArguments()) {
 std::tie(opt, std::ignore, value) = opt_entry;
-if (opt == "" && !value.empty() &&
+if (opt == CommandInterpreter::g_argument && !value.empty() &&
 llvm::StringRef(value).endswith("--")) {
   m_is_dashdash_alias = eLazyBoolYes;
   break;



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

[Lldb-commits] [PATCH] D133680: Add support for mach-o corefile loading of a platform binary

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/include/lldb/Target/DynamicLoader.h:231
+  /// Name of the binary, if available.  If an empty StringRef is
+  /// passed here, a name will be constructed for the module.
+  ///

DavidSpickett wrote:
> Can you list what it might be constructed from? My guess:
> * the filename if loading from a path
> * @ if loading from something in memory
Thanks, this should have been clearer.  Yeah this name is only used when a file 
cannot be found on the debug host; it may construct a module out of live 
memory.  And in that case, it needs to make up a name for it.



Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:420
 LLDB_LOGF(log,
-  "ProcessMachCore::DoLoadCore: Using kernel corefile image "
+  "ProcessMachCore::LoadBinariesAndSetDYLD: Using kernel "
+  "corefile image "

DavidSpickett wrote:
> I think the various log messages could use `__FUNCTION__` instead but hey 
> let's not make this diff any bigger :) .
> (the one time I know you need to write out the name is inside a lambda)
LOL I even updated the names of all the log messages to reflect their new 
methods, but of course you're right.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133680

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

[Lldb-commits] [PATCH] D133680: Add support for mach-o corefile loading of a platform binary

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere 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/D133680/new/

https://reviews.llvm.org/D133680

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

[Lldb-commits] [lldb] 81f8788 - Trying to understand the TestHelp.py failure from 6c089b2.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-09-13T14:09:44-07:00
New Revision: 81f8788528886ee611041e1f4ee54eea8bbfa277

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

LOG: Trying to understand the TestHelp.py failure from 6c089b2.

Sadly, the test passes on macOS, but fails on Ubuntu & Win.  The
extra option printing is supposed to be suppressed by the return
from CommandObjectAlias::IsDashDashCommand.  That was changed, but just
by replacing an inline string compare with a const string from
CommandInterpreter.  Putting the old version back temporarily to
see if that is really the problem.

Added: 


Modified: 
lldb/source/Interpreter/CommandAlias.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandAlias.cpp 
b/lldb/source/Interpreter/CommandAlias.cpp
index e36edcac15a5..80a926ef4d6a 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -180,7 +180,7 @@ bool CommandAlias::IsDashDashCommand() {
 
   for (const auto _entry : *GetOptionArguments()) {
 std::tie(opt, std::ignore, value) = opt_entry;
-if (opt == CommandInterpreter::g_argument && !value.empty() &&
+if (opt == "" && !value.empty() &&
 llvm::StringRef(value).endswith("--")) {
   m_is_dashdash_alias = eLazyBoolYes;
   break;



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

[Lldb-commits] [PATCH] D133546: [lldb][fuzz] Allow expression fuzzer to be passed as a flag.

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova added a comment.

The wording is good, LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133546

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

[Lldb-commits] [lldb] 8433b21 - constexpr isn't right here.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-09-13T13:01:56-07:00
New Revision: 8433b210839ed655852428ba8b34bb67b191957a

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

LOG: constexpr isn't right here.

Added: 


Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 2218d54e3d97..89878713161b 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -105,9 +105,9 @@ static constexpr const char *InitFileWarning =
 "and\n"
 "accept the security risk.";
 
-constexpr const char *CommandInterpreter::g_no_argument = "";
-constexpr const char *CommandInterpreter::g_need_argument = "";
-constexpr const char *CommandInterpreter::g_argument = "";
+const char * const CommandInterpreter::g_no_argument = "";
+const char * const CommandInterpreter::g_need_argument = "";
+const char * const CommandInterpreter::g_argument = "";
 
 
 #define LLDB_PROPERTIES_interpreter



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

[Lldb-commits] [lldb] ef3fa23 - [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

[Jorge Gorbe Moya via lldb-commits]

Author: Jorge Gorbe Moya
Date: 2022-09-13T12:50:55-07:00
New Revision: ef3fa232b338fd73475656ed61847295b0beef24

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

LOG: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

Modify `SBTypeNameSpecifier` and `lldb_private::TypeMatcher` so they
have an enum value for the type of matching to perform instead of an
`m_is_regex` boolean value.

This change paves the way for introducing formatter matching based on
the result of a python callback in addition to the existing name-based
matching. See the RFC thread at
https://discourse.llvm.org/t/rfc-python-callback-for-data-formatters-type-matching/64204
for more details.

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

Added: 


Modified: 
lldb/bindings/interface/SBTypeNameSpecifier.i
lldb/include/lldb/API/SBTypeNameSpecifier.h
lldb/include/lldb/DataFormatters/FormatClasses.h
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/API/SBTypeNameSpecifier.cpp
lldb/source/Core/FormatEntity.cpp

Removed: 




diff  --git a/lldb/bindings/interface/SBTypeNameSpecifier.i 
b/lldb/bindings/interface/SBTypeNameSpecifier.i
index 772f7c174093f..4bd5f205cfbc6 100644
--- a/lldb/bindings/interface/SBTypeNameSpecifier.i
+++ b/lldb/bindings/interface/SBTypeNameSpecifier.i
@@ -20,6 +20,9 @@ namespace lldb {
 SBTypeNameSpecifier (const char* name,
  bool is_regex = false);
 
+SBTypeNameSpecifier (const char* name,
+ lldb::FormatterMatchType match_type);
+
 SBTypeNameSpecifier (SBType type);
 
 SBTypeNameSpecifier (const lldb::SBTypeNameSpecifier );
@@ -40,6 +43,9 @@ namespace lldb {
 lldb::SBType
 GetType ();
 
+lldb::FormatterMatchType
+GetMatchType();
+
 bool
 IsRegex();
 

diff  --git a/lldb/include/lldb/API/SBTypeNameSpecifier.h 
b/lldb/include/lldb/API/SBTypeNameSpecifier.h
index eee424c1710a4..56a85d72c723b 100644
--- a/lldb/include/lldb/API/SBTypeNameSpecifier.h
+++ b/lldb/include/lldb/API/SBTypeNameSpecifier.h
@@ -20,6 +20,9 @@ class LLDB_API SBTypeNameSpecifier {
 
   SBTypeNameSpecifier(const char *name, bool is_regex = false);
 
+  SBTypeNameSpecifier(const char *name,
+  lldb::FormatterMatchType match_type);
+
   SBTypeNameSpecifier(SBType type);
 
   SBTypeNameSpecifier(const lldb::SBTypeNameSpecifier );
@@ -34,6 +37,8 @@ class LLDB_API SBTypeNameSpecifier {
 
   SBType GetType();
 
+  lldb::FormatterMatchType GetMatchType();
+
   bool IsRegex();
 
   bool GetDescription(lldb::SBStream ,

diff  --git a/lldb/include/lldb/DataFormatters/FormatClasses.h 
b/lldb/include/lldb/DataFormatters/FormatClasses.h
index f8f17b6d5d8ae..ac2b070a55cdc 100644
--- a/lldb/include/lldb/DataFormatters/FormatClasses.h
+++ b/lldb/include/lldb/DataFormatters/FormatClasses.h
@@ -133,21 +133,23 @@ class TypeNameSpecifierImpl {
 public:
   TypeNameSpecifierImpl() = default;
 
-  TypeNameSpecifierImpl(llvm::StringRef name, bool is_regex)
-  : m_is_regex(is_regex) {
+  TypeNameSpecifierImpl(llvm::StringRef name,
+lldb::FormatterMatchType match_type)
+  : m_match_type(match_type) {
 m_type.m_type_name = std::string(name);
   }
 
-  // if constructing with a given type, is_regex cannot be true since we are
-  // giving an exact type to match
-  TypeNameSpecifierImpl(lldb::TypeSP type) : m_is_regex(false) {
+  // if constructing with a given type, we consider that a case of exact match.
+  TypeNameSpecifierImpl(lldb::TypeSP type)
+  : m_match_type(lldb::eFormatterMatchExact) {
 if (type) {
   m_type.m_type_name = std::string(type->GetName().GetStringRef());
   m_type.m_compiler_type = type->GetForwardCompilerType();
 }
   }
 
-  TypeNameSpecifierImpl(CompilerType type) : m_is_regex(false) {
+  TypeNameSpecifierImpl(CompilerType type)
+  : m_match_type(lldb::eFormatterMatchExact) {
 if (type.IsValid()) {
   m_type.m_type_name.assign(type.GetTypeName().GetCString());
   m_type.m_compiler_type = type;
@@ -166,10 +168,12 @@ class TypeNameSpecifierImpl {
 return CompilerType();
   }
 
-  bool IsRegex() { return m_is_regex; }
+  lldb::FormatterMatchType GetMatchType() { return m_match_type; }
+
+  bool IsRegex() { return m_match_type == lldb::eFormatterMatchRegex; }
 
 private:
-  bool m_is_regex = false;
+  lldb::FormatterMatchType m_match_type = lldb::eFormatterMatchExact;
   // TODO: Replace this with TypeAndOrName.
   struct TypeOrName {
 std::string m_type_name;

diff  --git a/lldb/include/lldb/DataFormatters/FormattersContainer.h 
b/lldb/include/lldb/DataFormatters/FormattersContainer.h
index 

[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGef3fa232b338: [Formatters][NFCI] Replace 
is_regex arguments with an enum. (authored by jgorbe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133240

Files:
  lldb/bindings/interface/SBTypeNameSpecifier.i
  lldb/include/lldb/API/SBTypeNameSpecifier.h
  lldb/include/lldb/DataFormatters/FormatClasses.h
  lldb/include/lldb/DataFormatters/FormattersContainer.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBTypeNameSpecifier.cpp
  lldb/source/Core/FormatEntity.cpp

Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -825,7 +825,7 @@
 bitfield_name.Printf("%s:%d", target->GetTypeName().AsCString(),
  target->GetBitfieldBitSize());
 auto type_sp = std::make_shared(
-bitfield_name.GetString(), false);
+bitfield_name.GetString(), lldb::eFormatterMatchExact);
 if (val_obj_display ==
 ValueObject::eValueObjectRepresentationStyleSummary &&
 !DataVisualization::GetSummaryForType(type_sp))
Index: lldb/source/API/SBTypeNameSpecifier.cpp
===
--- lldb/source/API/SBTypeNameSpecifier.cpp
+++ lldb/source/API/SBTypeNameSpecifier.cpp
@@ -20,8 +20,15 @@
 SBTypeNameSpecifier::SBTypeNameSpecifier() { LLDB_INSTRUMENT_VA(this); }
 
 SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name, bool is_regex)
-: m_opaque_sp(new TypeNameSpecifierImpl(name, is_regex)) {
+: SBTypeNameSpecifier(name, is_regex ? eFormatterMatchRegex
+ : eFormatterMatchExact) {
   LLDB_INSTRUMENT_VA(this, name, is_regex);
+}
+
+SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name,
+ FormatterMatchType match_type)
+: m_opaque_sp(new TypeNameSpecifierImpl(name, match_type)) {
+  LLDB_INSTRUMENT_VA(this, name, match_type);
 
   if (name == nullptr || (*name) == 0)
 m_opaque_sp.reset();
@@ -72,13 +79,20 @@
   return SBType();
 }
 
+FormatterMatchType SBTypeNameSpecifier::GetMatchType() {
+  LLDB_INSTRUMENT_VA(this);
+  if (!IsValid())
+return eFormatterMatchExact;
+  return m_opaque_sp->GetMatchType();
+}
+
 bool SBTypeNameSpecifier::IsRegex() {
   LLDB_INSTRUMENT_VA(this);
 
   if (!IsValid())
 return false;
 
-  return m_opaque_sp->IsRegex();
+  return m_opaque_sp->GetMatchType() == eFormatterMatchRegex;
 }
 
 bool SBTypeNameSpecifier::GetDescription(
@@ -116,7 +130,7 @@
   if (!IsValid())
 return !rhs.IsValid();
 
-  if (IsRegex() != rhs.IsRegex())
+  if (GetMatchType() != rhs.GetMatchType())
 return false;
   if (GetName() == nullptr || rhs.GetName() == nullptr)
 return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -832,6 +832,15 @@
   eTemplateArgumentKindNullPtr,
 };
 
+/// Type of match to be performed when looking for a formatter for a data type.
+/// Used by classes like SBTypeNameSpecifier or lldb_private::TypeMatcher.
+enum FormatterMatchType {
+  eFormatterMatchExact,
+  eFormatterMatchRegex,
+
+  eLastFormatterMatchType = eFormatterMatchRegex,
+};
+
 /// Options that can be set for a formatter to alter its behavior. Not
 /// all of these are applicable to all formatter types.
 FLAGS_ENUM(TypeOptions){eTypeOptionNone = (0u),
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -41,9 +41,11 @@
 class TypeMatcher {
   RegularExpression m_type_name_regex;
   ConstString m_type_name;
-  /// False if m_type_name_regex should be used for matching. False if this is
-  /// just matching by comparing with m_type_name string.
-  bool m_is_regex;
+  /// Indicates what kind of matching strategy should be used:
+  /// - eFormatterMatchExact: match the exact type name in m_type_name.
+  /// - eFormatterMatchRegex: match using the RegularExpression object
+  ///   `m_type_name_regex` instead.
+  lldb::FormatterMatchType m_match_type;
 
   // if the user tries to add formatters for, say, "struct Foo" those will not
   // match any type because of the way we strip qualifiers from typenames this
@@ -71,22 +73,25 @@
   TypeMatcher() = delete;
   /// Creates a matcher that accepts any type with exactly the given type name.
   TypeMatcher(ConstString type_name)
-  : m_type_name(type_name), m_is_regex(false) {}
+  : m_type_name(type_name), m_match_type(lldb::eFormatterMatchExact) 

[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe updated this revision to Diff 459845.
jgorbe added a comment.

Replaced `kNumFormatterMatchTypes` with `eLastFormatterMatchType` to avoid 
"unhandled enum value" warnings, as suggested by Pavel.


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

https://reviews.llvm.org/D133240

Files:
  lldb/bindings/interface/SBTypeNameSpecifier.i
  lldb/include/lldb/API/SBTypeNameSpecifier.h
  lldb/include/lldb/DataFormatters/FormatClasses.h
  lldb/include/lldb/DataFormatters/FormattersContainer.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBTypeNameSpecifier.cpp
  lldb/source/Core/FormatEntity.cpp

Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -825,7 +825,7 @@
 bitfield_name.Printf("%s:%d", target->GetTypeName().AsCString(),
  target->GetBitfieldBitSize());
 auto type_sp = std::make_shared(
-bitfield_name.GetString(), false);
+bitfield_name.GetString(), lldb::eFormatterMatchExact);
 if (val_obj_display ==
 ValueObject::eValueObjectRepresentationStyleSummary &&
 !DataVisualization::GetSummaryForType(type_sp))
Index: lldb/source/API/SBTypeNameSpecifier.cpp
===
--- lldb/source/API/SBTypeNameSpecifier.cpp
+++ lldb/source/API/SBTypeNameSpecifier.cpp
@@ -20,8 +20,15 @@
 SBTypeNameSpecifier::SBTypeNameSpecifier() { LLDB_INSTRUMENT_VA(this); }
 
 SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name, bool is_regex)
-: m_opaque_sp(new TypeNameSpecifierImpl(name, is_regex)) {
+: SBTypeNameSpecifier(name, is_regex ? eFormatterMatchRegex
+ : eFormatterMatchExact) {
   LLDB_INSTRUMENT_VA(this, name, is_regex);
+}
+
+SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name,
+ FormatterMatchType match_type)
+: m_opaque_sp(new TypeNameSpecifierImpl(name, match_type)) {
+  LLDB_INSTRUMENT_VA(this, name, match_type);
 
   if (name == nullptr || (*name) == 0)
 m_opaque_sp.reset();
@@ -72,13 +79,20 @@
   return SBType();
 }
 
+FormatterMatchType SBTypeNameSpecifier::GetMatchType() {
+  LLDB_INSTRUMENT_VA(this);
+  if (!IsValid())
+return eFormatterMatchExact;
+  return m_opaque_sp->GetMatchType();
+}
+
 bool SBTypeNameSpecifier::IsRegex() {
   LLDB_INSTRUMENT_VA(this);
 
   if (!IsValid())
 return false;
 
-  return m_opaque_sp->IsRegex();
+  return m_opaque_sp->GetMatchType() == eFormatterMatchRegex;
 }
 
 bool SBTypeNameSpecifier::GetDescription(
@@ -116,7 +130,7 @@
   if (!IsValid())
 return !rhs.IsValid();
 
-  if (IsRegex() != rhs.IsRegex())
+  if (GetMatchType() != rhs.GetMatchType())
 return false;
   if (GetName() == nullptr || rhs.GetName() == nullptr)
 return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -832,6 +832,15 @@
   eTemplateArgumentKindNullPtr,
 };
 
+/// Type of match to be performed when looking for a formatter for a data type.
+/// Used by classes like SBTypeNameSpecifier or lldb_private::TypeMatcher.
+enum FormatterMatchType {
+  eFormatterMatchExact,
+  eFormatterMatchRegex,
+
+  eLastFormatterMatchType = eFormatterMatchRegex,
+};
+
 /// Options that can be set for a formatter to alter its behavior. Not
 /// all of these are applicable to all formatter types.
 FLAGS_ENUM(TypeOptions){eTypeOptionNone = (0u),
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -41,9 +41,11 @@
 class TypeMatcher {
   RegularExpression m_type_name_regex;
   ConstString m_type_name;
-  /// False if m_type_name_regex should be used for matching. False if this is
-  /// just matching by comparing with m_type_name string.
-  bool m_is_regex;
+  /// Indicates what kind of matching strategy should be used:
+  /// - eFormatterMatchExact: match the exact type name in m_type_name.
+  /// - eFormatterMatchRegex: match using the RegularExpression object
+  ///   `m_type_name_regex` instead.
+  lldb::FormatterMatchType m_match_type;
 
   // if the user tries to add formatters for, say, "struct Foo" those will not
   // match any type because of the way we strip qualifiers from typenames this
@@ -71,22 +73,25 @@
   TypeMatcher() = delete;
   /// Creates a matcher that accepts any type with exactly the given type name.
   TypeMatcher(ConstString type_name)
-  : m_type_name(type_name), m_is_regex(false) {}
+  : m_type_name(type_name), m_match_type(lldb::eFormatterMatchExact) {}
   /// Creates a matcher that accepts any type matching the given regex.
   

[Lldb-commits] [PATCH] D133546: [lldb][fuzz] Allow expression fuzzer to be passed as a flag.

[Jordan Rupprecht via Phabricator via lldb-commits]

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



Comment at: 
lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp:66
 ReportError(
 "no target path specified in with the LLDB_FUZZER_TARGET variable");
 

cassanova wrote:
> I think having a flag for the fuzzer target alongside the env variable is a 
> good addition. My only wonder is if there should be an error message if no 
> flag is specified so that anyone that uses this knows that they can use a 
> flag as well.
That makes sense -- let me know if the updated wording looks OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133546

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

[Lldb-commits] [PATCH] D133546: [lldb][fuzz] Allow expression fuzzer to be passed as a flag.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht updated this revision to Diff 459844.
rupprecht added a comment.

- Update error message to reference the new command line flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133546

Files:
  lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp


Index: lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
===
--- lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
+++ lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
@@ -35,7 +35,7 @@
 using namespace llvm;
 using namespace clang_fuzzer;
 
-char *target_path;
+const char *target_path = nullptr;
 
 void ReportError(llvm::StringRef message) {
   WithColor::error() << message << '\n';
@@ -47,10 +47,24 @@
   signal(SIGPIPE, SIG_IGN);
 #endif
 
-  target_path = ::getenv("LLDB_FUZZER_TARGET");
+  // `target_path` can be set by either the "--lldb_fuzzer_target" commandline
+  // flag or the "LLDB_FUZZER_TARGET" environment variable. Arbitrarily, we
+  // always do flag parsing and only check the environment variable if the
+  // commandline flag is not set.
+  for (int i = 1; i < *argc; ++i) {
+auto this_arg = llvm::StringRef((*argv)[i]);
+WithColor::note() << "argv[" << i << "] = " << this_arg << "\n";
+if (this_arg.consume_front("--lldb_fuzzer_target="))
+  target_path = this_arg.data();
+  }
+
+  if (!target_path)
+target_path = ::getenv("LLDB_FUZZER_TARGET");
+
   if (!target_path)
-ReportError(
-"no target path specified in with the LLDB_FUZZER_TARGET variable");
+ReportError("No target path specified. Set one either as an environment "
+"variable (i.e. LLDB_FUZZER_TARGET=target_path) or pass as a "
+"command line flag (i.e. --lldb_fuzzer_target=target_path).");
 
   if (!sys::fs::exists(target_path))
 ReportError(formatv("target path '{0}' does not exist", 
target_path).str());


Index: lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
===
--- lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
+++ lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp
@@ -35,7 +35,7 @@
 using namespace llvm;
 using namespace clang_fuzzer;
 
-char *target_path;
+const char *target_path = nullptr;
 
 void ReportError(llvm::StringRef message) {
   WithColor::error() << message << '\n';
@@ -47,10 +47,24 @@
   signal(SIGPIPE, SIG_IGN);
 #endif
 
-  target_path = ::getenv("LLDB_FUZZER_TARGET");
+  // `target_path` can be set by either the "--lldb_fuzzer_target" commandline
+  // flag or the "LLDB_FUZZER_TARGET" environment variable. Arbitrarily, we
+  // always do flag parsing and only check the environment variable if the
+  // commandline flag is not set.
+  for (int i = 1; i < *argc; ++i) {
+auto this_arg = llvm::StringRef((*argv)[i]);
+WithColor::note() << "argv[" << i << "] = " << this_arg << "\n";
+if (this_arg.consume_front("--lldb_fuzzer_target="))
+  target_path = this_arg.data();
+  }
+
+  if (!target_path)
+target_path = ::getenv("LLDB_FUZZER_TARGET");
+
   if (!target_path)
-ReportError(
-"no target path specified in with the LLDB_FUZZER_TARGET variable");
+ReportError("No target path specified. Set one either as an environment "
+"variable (i.e. LLDB_FUZZER_TARGET=target_path) or pass as a "
+"command line flag (i.e. --lldb_fuzzer_target=target_path).");
 
   if (!sys::fs::exists(target_path))
 ReportError(formatv("target path '{0}' does not exist", target_path).str());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133446: [LLDB][NativePDB] Global ctor and dtor should be global decls.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

In D133446#3786561 , @labath wrote:

> In D133446#3780961 , @zequanwu 
> wrote:
>
>> In D133446#3779600 , @labath wrote:
>>
>>> I believe that this fixes the crash, but the names of generated functions 
>>> still look fairly weird. Could we create them using their mangled name 
>>> instead? That way, someone might actually call them, if he was so inclined.
>>
>> It looks like they don't have mangled name stored in pdb.
>
> Hmm.. That's unfortunate. In that case, I'm wondering if this shouldn't be 
> handled somehow directly inside `MSVCUndecoratedNameParser`. What does the 
> function return right now? Do you think that result is going to be useful for 
> other users of that class?

`MSVCUndecoratedNameParser` expects a undercoated name of a entity(class, 
function, variable, etc) and returns the a pair of {pointer to parent decl 
context, base name of the entity}. This is the only way we use to get enclosing 
context for a given entity.

> Also, I'm still continuing to be amazed (and scared) by the amount of name 
> parsing that is going on here. Have you checked that there's no better way to 
> associate an entity with its enclosing context?

If the entity is a global variable, we don't have any other ways to know if its 
enclosing context is record or namespace.  If the entity is record or function, 
then we can know if its enclosing context is a record by looking at its type 
info but need to fall back to parse name again if it's inside a namespace. The 
issue is there isn't a way to represent namespaces in PDB, and they are just 
embedded into the name strings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133446

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

[Lldb-commits] [PATCH] D132954: lldb: Add support for R_386_32 relocations to ObjectFileELF

[David M. Lary via Phabricator via lldb-commits]

dmlary added a comment.

I'm not sure what to do about the test cases that now unexpectedly pass.  The 
removal of the `assert()` in `ObjectFileELF::ApplyRelocations` means each of 
these cases is now passing on what looks like 32-bit arm, but I don't know 
enough about the three tests to say that it's right to remove the `XPASS`.  I 
think the buildbot is reporting that they do pass, so is it safe?

  Unexpectedly Passed Tests (3):
lldb-shell :: SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll
lldb-shell :: 
SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
lldb-shell :: SymbolFile/DWARF/split-optimized.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132954

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

[Lldb-commits] [PATCH] D132954: lldb: Add support for R_386_32 relocations to ObjectFileELF

[David M. Lary via Phabricator via lldb-commits]

dmlary added a comment.

I saw the build bot failures; looking into them now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132954

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

[Lldb-commits] [PATCH] D133045: Partial fix for handling backticks in commands and aliases

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

This seems to break the build with GCC (tested with the system compiler GCC 9 
on Ubuntu 20.04):

  ld.lld: error: undefined symbol: lldb_private::CommandInterpreter::g_argument
  >>> referenced by CommandAlias.cpp
  >>>   
CommandAlias.cpp.o:(lldb_private::CommandAlias::GetAliasExpansion(lldb_private::StreamString&)
 const (.localalias)) in archive lib/liblldbInterpreter.a

(with lots more similar errors)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133045

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

[Lldb-commits] [PATCH] D133790: Fix heap-use-after-free when clearing DIEs in fission compile units.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

LGTM, but I will let Pavel comment since he did the fission support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133790

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

[Lldb-commits] [lldb] 6c089b2 - Be more careful to maintain quoting information when parsing commands.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-09-13T11:02:47-07:00
New Revision: 6c089b2af5d8d98f66b27b67f70958f520820a76

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

LOG: Be more careful to maintain quoting information when parsing commands.

This is particularly a problem for alias construction, where you might
want to have a backtick surrounded option in the alias.  Before this
patch:

command alias expression -Z \`argc\` -- argv

for instance would be rendered as:

expression -Z argc -- argv

and would fail to work.

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

Added: 
lldb/test/API/commands/command/backticks/Makefile
lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
lldb/test/API/commands/command/backticks/main.c

Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Interpreter/CommandAlias.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Interpreter/Options.cpp
lldb/source/Utility/Args.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 0f137a7b3c496..71587eed68c95 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -239,6 +239,14 @@ class CommandInterpreter : public Broadcaster,
 eCommandTypesAllThem = 0x  //< all commands
   };
 
+  // The CommandAlias and CommandInterpreter both have a hand in 
+  // substituting for alias commands.  They work by writing special tokens
+  // in the template form of the Alias command, and then detecting them when 
the
+  // command is executed.  These are the special tokens:
+  static const char * const g_no_argument;
+  static const char * const g_need_argument;
+  static const char * const g_argument;
+
   CommandInterpreter(Debugger , bool synchronous_execution);
 
   ~CommandInterpreter() override = default;

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 101921ffc768b..5c058769ff3f9 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2472,6 +2472,13 @@ def assertSuccess(self, obj, msg=None):
 self.fail(self._formatMessage(msg,
 "'{}' is not success".format(error)))
 
+"""Assert that a command return object is successful"""
+def assertCommandReturn(self, obj, msg=None):
+if not obj.Succeeded():
+error = obj.GetError()
+self.fail(self._formatMessage(msg,
+"'{}' is not success".format(error)))
+
 """Assert two states are equal"""
 def assertState(self, first, second, msg=None):
 if first != second:

diff  --git a/lldb/source/Interpreter/CommandAlias.cpp 
b/lldb/source/Interpreter/CommandAlias.cpp
index d55b3fdd44faf..e36edcac15a59 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -60,11 +60,12 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP 
_obj_sp,
 
   if (!options_string.empty()) {
 if (cmd_obj_sp->WantsRawCommandString())
-  option_arg_vector->emplace_back("", -1, options_string);
+  option_arg_vector->emplace_back(CommandInterpreter::g_argument, 
+  -1, options_string);
 else {
   for (auto  : args.entries()) {
 if (!entry.ref().empty())
-  option_arg_vector->emplace_back(std::string(""), -1,
+  
option_arg_vector->emplace_back(std::string(CommandInterpreter::g_argument), -1,
   std::string(entry.ref()));
   }
 }
@@ -153,11 +154,12 @@ void CommandAlias::GetAliasExpansion(StreamString 
_string) const {
 
   for (const auto _entry : *options) {
 std::tie(opt, std::ignore, value) = opt_entry;
-if (opt == "") {
+if (opt == CommandInterpreter::g_argument) {
   help_string.Printf(" %s", value.c_str());
 } else {
   help_string.Printf(" %s", opt.c_str());
-  if ((value != "") && (value != " 
CommandAlias::Desugar() {
 return {nullptr, nullptr};
 
   if (underlying->IsAlias()) {
+// FIXME: This doesn't work if the original alias fills a slot in the
+// underlying alias, since this just appends the two lists.
 auto desugared = ((CommandAlias *)underlying.get())->Desugar();
 auto options = GetOptionArguments();
 options->insert(options->begin(), desugared.second->begin(),

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 

[Lldb-commits] [PATCH] D133045: Partial fix for handling backticks in commands and aliases

[Jim Ingham 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.
jingham marked an inline comment as done.
Closed by commit rG6c089b2af5d8: Be more careful to maintain quoting 
information when parsing commands. (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D133045?vs=457063=459812#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133045

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Interpreter/CommandAlias.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/Options.cpp
  lldb/source/Utility/Args.cpp
  lldb/test/API/commands/command/backticks/Makefile
  lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
  lldb/test/API/commands/command/backticks/main.c

Index: lldb/test/API/commands/command/backticks/main.c
===
--- /dev/null
+++ lldb/test/API/commands/command/backticks/main.c
@@ -0,0 +1,5 @@
+int main (int argc, char const *argv[])
+{
+  return argc; // break here
+}
+
Index: lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
===
--- /dev/null
+++ lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
@@ -0,0 +1,32 @@
+"""
+Test that an alias can contain active backticks
+"""
+
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestBackticksInAlias(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_backticks_in_alias(self):
+"""Test that an alias can contain active backticks."""
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+interp = self.dbg.GetCommandInterpreter();
+result = lldb.SBCommandReturnObject()
+interp.HandleCommand('command alias _test-argv-cmd expression -Z \`argc\` -- argv', result)
+self.assertCommandReturn(result, "Made the alias")
+interp.HandleCommand("_test-argv-cmd", result)
+self.assertCommandReturn(result, "The alias worked")
+
+# Now try a harder case where we create this using an alias:
+interp.HandleCommand('command alias _test-argv-parray-cmd parray \`argc\` argv', result)
+self.assertCommandReturn(result, "Made the alias")
+interp.HandleCommand("_test-argv-parray-cmd", result)
+self.assertFalse(result.Succeeded(), "CommandAlias::Desugar currently fails if a alias substitutes %N arguments in another alias")
+
+
Index: lldb/test/API/commands/command/backticks/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/command/backticks/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Utility/Args.cpp
===
--- lldb/source/Utility/Args.cpp
+++ lldb/source/Utility/Args.cpp
@@ -215,7 +215,12 @@
   for (size_t i = 0; i < m_entries.size(); ++i) {
 if (i > 0)
   command += ' ';
+char quote = m_entries[i].quote;
+if (quote != '\0')
+ command += quote;
 command += m_entries[i].ref();
+if (quote != '\0')
+  command += quote;
   }
 
   return !m_entries.empty();
Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -1002,37 +1002,47 @@
   .str(),
   llvm::inconvertibleErrorCode());
 }
-if (!option_arg)
-  option_arg = "";
-option_arg_vector->emplace_back(std::string(option_str.GetString()),
-has_arg, std::string(option_arg));
-
 // Find option in the argument list; also see if it was supposed to take an
 // argument and if one was supplied.  Remove option (and argument, if
 // given) from the argument list.  Also remove them from the
 // raw_input_string, if one was passed in.
+// Note: We also need to preserve any option argument values that were
+// surrounded by backticks, as we lose track of them in the 
+// option_args_vector.
 size_t idx =
 FindArgumentIndexForOption(args_copy, long_options[long_options_index]);
+std::string option_to_insert;
+if (option_arg) {
+  if (idx != size_t(-1) && has_arg) {
+bool arg_has_backtick = args_copy[idx + 1].GetQuoteChar() == '`';
+if (arg_has_backtick)
+  option_to_insert = "`";
+option_to_insert += option_arg;
+if (arg_has_backtick)
+  option_to_insert += "`";
+  

[Lldb-commits] [PATCH] D133045: Partial fix for handling backticks in commands and aliases

[Jim Ingham via Phabricator via lldb-commits]

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



Comment at: lldb/source/Interpreter/Options.cpp:1026
+} else
+  option_to_insert = CommandInterpreter::g_no_argument;
+

hawkinsw wrote:
> Question: Could we drop the final `else` if we initialized `option_to_insert` 
> to `CommandInterpreter::g_no_argument`? Just curious. 
No, the correct default when there is an option_arg is the empty string.  So 
I'd also have to set the string back to empty after the `if (option_arg) {` 
which would look weird.



Comment at: lldb/test/API/commands/command/backticks/TestBackticksInAlias.py:16
+def test_backticks_in_alias(self):
+"""Test that an alias can contain active backtraces."""
+self.build()

hawkinsw wrote:
> Nit: Typo. backtraces => backticks.
Thanks for spotting that, I'll fix on commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133045

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

[Lldb-commits] [PATCH] D131437: Don't index the skeleton CU when we have a fission compile unit.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D131437#3751366 , @bkramer wrote:

> This seems to trigger a use after free in `lldb-api :: 
> functionalities/thread/create_after_attach/TestCreateAfterAttach.py`
>
> asan log:
>
>   ==4741==ERROR: AddressSanitizer: heap-use-after-free on address 
> 0x62f00023bf58 at pc 0x563639db88f1 bp 0x7ffd942412f0 sp 0x7ffd942412e8
>   READ of size 4 at 0x62f00023bf58 thread T0
>   #0 0x563639db88f0 in HasChildren 
> lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:124:37
>   #1 0x563639db88f0 in GetFirstChild 
> lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:148:12
>   #2 0x563639db88f0 in GetFirstChild 
> lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:101:34
>   #3 0x563639db88f0 in child_iterator 
> lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h:107:57
>   #4 0x563639db88f0 in DWARFDIE::children() const 
> lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:466:27
>   #5 0x563639d9f4e1 in 
> DWARFASTParserClang::EnsureAllDIEsInDeclContextHaveBeenParsed(lldb_private::CompilerDeclContext)
>  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2203:37
>   #6 0x563639f1ab62 in 
> lldb_private::TypeSystemClang::DeclContextFindDeclByName(void*, 
> lldb_private::ConstString, bool) 
> lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:9494:22
>   #7 0x563639f489fd in 
> lldb_private::CompilerDeclContext::FindDeclByName(lldb_private::ConstString, 
> bool) lldb/source/Symbol/CompilerDeclContext.cpp:20:27
>   #8 0x563639b6113c in 
> lldb_private::ClangExpressionDeclMap::LookupLocalVariable(lldb_private::NameSearchContext&,
>  lldb_private::ConstString, lldb_private::SymbolContext&, 
> lldb_private::CompilerDeclContext const&) 
> lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1084:20
>   #9 0x563639b5d7cf in 
> lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls(lldb_private::NameSearchContext&,
>  std::__u::shared_ptr, 
> lldb_private::CompilerDeclContext const&) 
> lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1434:9
>   #10 0x563639b5c9df in 
> lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls(lldb_private::NameSearchContext&)
>  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:728:5
>   #11 0x563639b3df83 in 
> lldb_private::ClangASTSource::FindExternalVisibleDeclsByName(clang::DeclContext
>  const*, clang::DeclarationName) 
> lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:180:3
>   #12 0x56363d02aa30 in 
> clang::DeclContext::lookup(clang::DeclarationName) const 
> clang/lib/AST/DeclBase.cpp:1706:17
>   #13 0x56363c2bca5b in LookupDirect(clang::Sema&, clang::LookupResult&, 
> clang::DeclContext const*) clang/lib/Sema/SemaLookup.cpp:1108:39
>   #14 0x56363c2b67f5 in CppNamespaceLookup(clang::Sema&, 
> clang::LookupResult&, clang::ASTContext&, clang::DeclContext*, (anonymous 
> namespace)::UnqualUsingDirectiveSet&) clang/lib/Sema/SemaLookup.cpp:1207:16
>   #15 0x56363c2b5a1e in clang::Sema::CppLookupName(clang::LookupResult&, 
> clang::Scope*) clang/lib/Sema/SemaLookup.cpp:1495:15
>   #16 0x56363c2bc0f2 in clang::Sema::LookupName(clang::LookupResult&, 
> clang::Scope*, bool, bool) clang/lib/Sema/SemaLookup.cpp:2259:9
>   #17 0x56363bdb50b8 in clang::Sema::BuildUsingDeclaration(clang::Scope*, 
> clang::AccessSpecifier, clang::SourceLocation, bool, clang::SourceLocation, 
> clang::CXXScopeSpec&, clang::DeclarationNameInfo, clang::SourceLocation, 
> clang::ParsedAttributesView const&, bool, bool) 
> clang/lib/Sema/SemaDeclCXX.cpp:12329:5
>   #18 0x56363bdb49f3 in clang::Sema::ActOnUsingDeclaration(clang::Scope*, 
> clang::AccessSpecifier, clang::SourceLocation, clang::SourceLocation, 
> clang::CXXScopeSpec&, clang::UnqualifiedId&, clang::SourceLocation, 
> clang::ParsedAttributesView const&) clang/lib/Sema/SemaDeclCXX.cpp:11833:7
>   #19 0x56363b49df12 in 
> clang::Parser::ParseUsingDeclaration(clang::DeclaratorContext, 
> clang::Parser::ParsedTemplateInfo const&, clang::SourceLocation, 
> clang::SourceLocation&, clang::ParsedAttributes&, clang::AccessSpecifier) 
> clang/lib/Parse/ParseDeclCXX.cpp:803:26
>   #20 0x56363b49c27d in 
> clang::Parser::ParseUsingDirectiveOrDeclaration(clang::DeclaratorContext, 
> clang::Parser::ParsedTemplateInfo const&, clang::SourceLocation&, 
> clang::ParsedAttributes&) clang/lib/Parse/ParseDeclCXX.cpp:512:10
>   #21 0x56363b46c161 in 
> clang::Parser::ParseDeclaration(clang::DeclaratorContext, 
> clang::SourceLocation&, clang::ParsedAttributes&, clang::ParsedAttributes&, 
> clang::SourceLocation*) clang/lib/Parse/ParseDecl.cpp:1797:12
>   #22 0x56363b55fb99 in 
> clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector  32u>&, clang::Parser::ParsedStmtContext, clang::SourceLocation*, 
> clang::ParsedAttributes&, clang::ParsedAttributes&) 
> 

[Lldb-commits] [PATCH] D133790: Fix heap-use-after-free when clearing DIEs in fission compile units.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
rupprecht added reviewers: labath, clayborg.
Herald added a project: All.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

D131437  caused heap-use-after-free failures 
when testing TestCreateAfterAttach.py in asan mode, and "regular" crashes 
outside of asan.

This appears to be due to a mismatch in a couple places where we choose to 
clear the DIEs. When we clear the DIE of a skeleton unit, we unconditionally 
clear the DIE of the DWO unit if it exists. However, `~ScopedExtractDIEs()` 
only looks at the skeleton unit when deciding to clear. If we decide to clear 
the skeleton unit because it is now unused, we end up clearing the DWO unit 
that _is_ used. This change adds a guard by checking `m_cancel_scopes` to 
prevent clearing the DWO unit.

This is 100% reproducible by running TestCreateAfterAttach.py in asan mode, 
although it only seems to reproduce in our internal build, so no test case is 
added here. If someone has suggestions on how to write one, I can add it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133790

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -598,7 +598,7 @@
   m_die_array.clear();
   m_die_array.shrink_to_fit();
 
-  if (m_dwo)
+  if (m_dwo && !m_dwo->m_cancel_scopes)
 m_dwo->ClearDIEsRWLocked();
 }
 


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -598,7 +598,7 @@
   m_die_array.clear();
   m_die_array.shrink_to_fit();
 
-  if (m_dwo)
+  if (m_dwo && !m_dwo->m_cancel_scopes)
 m_dwo->ClearDIEsRWLocked();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133546: [lldb][fuzz] Allow expression fuzzer to be passed as a flag.

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp:66
 ReportError(
 "no target path specified in with the LLDB_FUZZER_TARGET variable");
 

I think having a flag for the fuzzer target alongside the env variable is a 
good addition. My only wonder is if there should be an error message if no flag 
is specified so that anyone that uses this knows that they can use a flag as 
well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133546

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

[Lldb-commits] [PATCH] D131122: [lldb] Fixed a number of typos

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7240436c94bd: [lldb] Fixed a number of typos (authored by 
GabrielRavier, committed by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D131122?vs=449806=459801#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131122

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/bindings/interface/SBType.i
  lldb/docs/use/python-reference.rst
  lldb/examples/python/armv7_cortex_m_target_defintion.py
  lldb/examples/python/lldb_module_utils.py
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/Mangled.h
  lldb/include/lldb/Symbol/Type.h
  lldb/packages/Python/lldbsuite/test/lldbbench.py
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/Type.cpp
  
lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp
  lldb/test/API/functionalities/breakpoint/debugbreak/TestDebugBreak.py
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
  lldb/test/API/functionalities/module_cache/bsd/TestModuleCacheBSD.py
  lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
  
lldb/test/API/functionalities/module_cache/universal/TestModuleCacheUniversal.py
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/minidump-new/makefile.txt
  lldb/tools/debugserver/source/DNBTimer.h
  lldb/tools/debugserver/source/JSON.h
  lldb/tools/debugserver/source/MacOSX/Genealogy.cpp

Index: lldb/tools/debugserver/source/MacOSX/Genealogy.cpp
===
--- lldb/tools/debugserver/source/MacOSX/Genealogy.cpp
+++ lldb/tools/debugserver/source/MacOSX/Genealogy.cpp
@@ -129,7 +129,7 @@
   return true;
 });
 
-// Collect all the Activites
+// Collect all the Activities
 m_os_activity_iterate_activities(
 process_info->activities, process_info,
 ^bool(os_activity_entry_t activity) {
Index: lldb/tools/debugserver/source/JSON.h
===
--- lldb/tools/debugserver/source/JSON.h
+++ lldb/tools/debugserver/source/JSON.h
@@ -71,7 +71,7 @@
 public:
   typedef std::shared_ptr SP;
 
-  // We cretae a constructor for all integer and floating point type with using
+  // We create a constructor for all integer and floating point type with using
   // templates and
   // SFINAE to avoid having ambiguous overloads because of the implicit type
   // promotion. If we
Index: lldb/tools/debugserver/source/DNBTimer.h
===
--- lldb/tools/debugserver/source/DNBTimer.h
+++ lldb/tools/debugserver/source/DNBTimer.h
@@ -53,7 +53,7 @@
 PTHREAD_MUTEX_LOCKER(locker, m_mutexAP.get());
 gettimeofday(_timeval, NULL);
   }
-  // Get the total mircoseconds since Jan 1, 1970
+  // Get the total microseconds since Jan 1, 1970
   uint64_t TotalMicroSeconds() const {
 PTHREAD_MUTEX_LOCKER(locker, m_mutexAP.get());
 return (uint64_t)(m_timeval.tv_sec) * 100ull +
Index: lldb/test/API/functionalities/postmortem/minidump-new/makefile.txt
===
--- lldb/test/API/functionalities/postmortem/minidump-new/makefile.txt
+++ lldb/test/API/functionalities/postmortem/minidump-new/makefile.txt
@@ -3,7 +3,7 @@
 # The binary should have debug symbols because stack unwinding doesn't work
 # correctly using the information in the Minidump only. Also we want to evaluate
 # local variables, etc.
-# Breakpad compiles as a static library, so statically linking againts it
+# Breakpad compiles as a static library, so statically linking against it
 # makes the binary huge.
 # Dynamically linking to it does improve things, but we are still #include-ing
 # breakpad headers (which is a lot of source code for which we generate debug
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- 

[Lldb-commits] [lldb] 7240436 - [lldb] Fixed a number of typos

[Jonas Devlieghere via lldb-commits]

Author: Gabriel Ravier
Date: 2022-09-13T10:38:38-07:00
New Revision: 7240436c94bd02762a723a2e3551528d16c8efdb

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

LOG: [lldb] Fixed a number of typos

I went over the output of the following mess of a command:

  (ulimit -m 200; ulimit -v 200; git ls-files -z | parallel
  --xargs -0 cat | aspell list --mode=none --ignore-case | grep -E
  '^[A-Za-z][a-z]*$' | sort | uniq -c | sort -n | grep -vE '.{25}' |
  aspell pipe -W3 | grep : | cut -d' ' -f2 | less)

and proceeded to spend a few days looking at it to find probable typos
and fixed a few hundred of them in all of the llvm project (note, the
ones I found are not anywhere near all of them, but it seems like a
good start).

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

Added: 


Modified: 
lldb/bindings/interface/SBProcess.i
lldb/bindings/interface/SBType.i
lldb/docs/use/python-reference.rst
lldb/examples/python/armv7_cortex_m_target_defintion.py
lldb/examples/python/lldb_module_utils.py
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/Mangled.h
lldb/include/lldb/Symbol/Type.h
lldb/packages/Python/lldbsuite/test/lldbbench.py
lldb/source/Core/DynamicLoader.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/Type.cpp

lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp
lldb/test/API/functionalities/breakpoint/debugbreak/TestDebugBreak.py
lldb/test/API/functionalities/load_unload/TestLoadUnload.py
lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
lldb/test/API/functionalities/module_cache/bsd/TestModuleCacheBSD.py

lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py

lldb/test/API/functionalities/module_cache/universal/TestModuleCacheUniversal.py
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
lldb/test/API/functionalities/postmortem/minidump-new/makefile.txt
lldb/tools/debugserver/source/DNBTimer.h
lldb/tools/debugserver/source/JSON.h
lldb/tools/debugserver/source/MacOSX/Genealogy.cpp

Removed: 




diff  --git a/lldb/bindings/interface/SBProcess.i 
b/lldb/bindings/interface/SBProcess.i
index 7a9f8fc5757c1..52f9fc7fa204b 100644
--- a/lldb/bindings/interface/SBProcess.i
+++ b/lldb/bindings/interface/SBProcess.i
@@ -422,7 +422,7 @@ public:
 
 %feature("autodoc", "
 Allocates a block of memory within the process, with size and
-access permissions specified in the arguments. The permisssions
+access permissions specified in the arguments. The permissions
 argument is an or-combination of zero or more of
 lldb.ePermissionsWritable, lldb.ePermissionsReadable, and
 lldb.ePermissionsExecutable. Returns the address

diff  --git a/lldb/bindings/interface/SBType.i 
b/lldb/bindings/interface/SBType.i
index 8db2fa7496340..ba33f9231f0e8 100644
--- a/lldb/bindings/interface/SBType.i
+++ b/lldb/bindings/interface/SBType.i
@@ -537,8 +537,8 @@ public:
 "Returns the `BasicType` value that is most appropriate to this type.
 
 Returns `eBasicTypeInvalid` if no appropriate `BasicType` was found or this
-type is invalid. See the `BasicType` documentation for the 
language-specific m
-aning of each `BasicType` value.
+type is invalid. See the `BasicType` documentation for the 
language-specific
+meaning of each `BasicType` value.
 
 **Overload behaviour:** When called with a `BasicType` parameter, the
 following behaviour applies:
@@ -731,8 +731,8 @@ public:
 
 * C: Always returns ``0``.
 * C++: If this type is a class template instantiation then this returns the
-  number of template parameters that were used in this instantiation. This 
i
-  cludes both explicit and implicit template parameters.
+  number of template parameters that were used in this instantiation. This
+  includes both explicit and implicit template parameters.
 * Objective-C: Always returns ``0``.
 ") GetNumberOfTemplateArguments;
 uint32_t

diff  --git a/lldb/docs/use/python-reference.rst 

[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added inline comments.



Comment at: lldb/include/lldb/lldb-enumerations.h:841
+
+  kNumFormatterMatchTypes,
+};

labath wrote:
> I see the enums in this file use wildly inconsistent styles for the "largest 
> value" enumerator. However, you've picked the one I like the least, because 
> this tends to produce `unhandled switch case "kNumFormatterMatchTypes"` 
> warnings. If you feel like you need to have a sentinel, I'd recommend going 
> with the `eLastFormatterMatchType = ` pattern (used e.g. in 
> the StateType enum).
I //do// want a sentinel because the plan is to replace some hardcoded 
instances of "check for an exact match and if nothing is found then check for a 
regex match" with a class that has an array of size `kNumFormatterMatchTypes` 
formatter containers, and knows how to do a lookup in priority order. So then I 
can add another "priority tier" for callback matchers and not add extra logic 
in a bunch of places.

But I have no strong preference between having a `std::array` and a `std::array`, and I agree the `unhandled switch case` warnings are annoying. I'll 
change it shortly.


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

https://reviews.llvm.org/D133240

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

[Lldb-commits] [PATCH] D132954: lldb: Add support for R_386_32 relocations to ObjectFileELF

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf4fc4056319d: lldb: Add support for R_386_32 relocations to 
ObjectFileELF (authored by dmlary, committed by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132954

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/test/Shell/ObjectFile/ELF/i386-relocations.yaml

Index: lldb/test/Shell/ObjectFile/ELF/i386-relocations.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/i386-relocations.yaml
@@ -0,0 +1,57 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test object-file --contents %t | FileCheck %s
+#
+# CHECK-LABEL:  Name: .debug_info
+# CHECK:Data: (
+# CHECK-NEXT: : 
+#
+# CHECK-LABEL:  Name: .debug_lines
+# CHECK:Data: (
+# CHECK-NEXT: : 
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+AddressAlign:0x20
+  - Name:.debug_info
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: 
+  - Name:.debug_lines
+Type:SHT_PROGBITS
+AddressAlign:0x1
+Content: 
+  - Name:.rel.debug_info
+Type:SHT_REL
+Flags:   [ SHF_INFO_LINK ]
+Link:.symtab
+AddressAlign:0x0
+Info:.debug_info
+Relocations:
+  - Offset:  0x0
+Symbol:  var
+Type:R_386_32
+  - Name:.rela.debug_lines
+Type:SHT_RELA
+Link:.symtab
+AddressAlign:0x4
+Info:.debug_lines
+Relocations:
+  - Offset:  0x0
+Addend:  0x
+Symbol:  var
+Type:R_386_32
+Symbols:
+  - Name:var
+Type:STT_OBJECT
+Section: .data
+Binding: STB_GLOBAL
+Value:   0x
+Size:0x5
+...
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -121,6 +121,8 @@
 
   static unsigned RelocAddend64(const ELFRelocation );
 
+  bool IsRela() { return (reloc.is()); }
+
 private:
   typedef llvm::PointerUnion RelocUnion;
 
@@ -2597,25 +2599,46 @@
   }
 
   for (unsigned i = 0; i < num_relocations; ++i) {
-if (!rel.Parse(rel_data, ))
+if (!rel.Parse(rel_data, )) {
+  GetModule()->ReportError(".rel%s[%d] failed to parse relocation",
+   rel_section->GetName().AsCString(), i);
   break;
-
+}
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
   switch (reloc_type(rel)) {
   case R_386_32:
+symbol = symtab->FindSymbolByID(reloc_symbol(rel));
+if (symbol) {
+  addr_t f_offset =
+  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
+  DataBufferSP _buffer_sp = debug_data.GetSharedDataBuffer();
+  // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+  WritableDataBuffer *data_buffer =
+  llvm::cast(data_buffer_sp.get());
+  uint32_t *dst = reinterpret_cast(
+  data_buffer->GetBytes() + f_offset);
+
+  addr_t value = symbol->GetAddressRef().GetFileAddress();
+  if (rel.IsRela()) {
+value += ELFRelocation::RelocAddend32(rel);
+  } else {
+value += *dst;
+  }
+  *dst = value;
+} else {
+  GetModule()->ReportError(".rel%s[%u] unknown symbol id: %d",
+   rel_section->GetName().AsCString(), i,
+   reloc_symbol(rel));
+}
+break;
   case R_386_PC32:
   default:
-// FIXME: This asserts with this input:
-//
-// foo.cpp
-// int main(int argc, char **argv) { return 0; }
-//
-// clang++.exe --target=i686-unknown-linux-gnu -g -c foo.cpp -o foo.o
-//
-// and running this on the foo.o module.
-assert(false && "unexpected relocation type");
+GetModule()->ReportError("unsupported 32-bit relocation:"
+ " .rel%s[%u], type %u",
+ rel_section->GetName().AsCString(), i,
+ reloc_type(rel));
   }
 } else {
   switch (reloc_type(rel)) {
___
lldb-commits mailing list

[Lldb-commits] [lldb] f4fc405 - lldb: Add support for R_386_32 relocations to ObjectFileELF

[Pavel Labath via lldb-commits]

Author: David M. Lary
Date: 2022-09-13T18:38:48+02:00
New Revision: f4fc4056319dd8ddfb8759f71c7aec86882c9110

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

LOG: lldb: Add support for R_386_32 relocations to ObjectFileELF

I encountered an issue where `p ` was finding an incorrect address for
32-bit PIC ELF files loaded into a running process.  The problem was that the
R_386_32 ELF relocations were not being applied to the DWARF section, so all
variables in that file were reporting as being at the start of their respective
section.  There is an assert that catches this on debug builds, but silently
ignores the issue on non-debug builds.

In this changeset, I added handling for the R_386_32 relocation type to
ObjectFileELF, and a supporting function to ELFRelocation to differentiate
between DT_REL & DT_RELA in ObjectFileELF::ApplyRelocations().

Demonstration of issue:
```
[dmlary@host work]$ cat rel.c
volatile char padding[32] = "make sure var isnt at .data+0";
volatile char var[] = "test";
[dmlary@host work]$ gcc -c rel.c -FPIC -fpic -g -m32

[dmlary@host work]$ lldb ./exec
(lldb) target create "./exec"
Current executable set to '/home/dmlary/src/work/exec' (i386).
(lldb) process launch --stop-at-entry
Process 21278 stopped
* thread #1, name = 'exec', stop reason = signal SIGSTOP
frame #0: 0xf7fdb150 ld-2.17.so`_start
ld-2.17.so`_start:
->  0xf7fdb150 <+0>: movl   %esp, %eax
0xf7fdb152 <+2>: calll  0xf7fdb990; _dl_start

ld-2.17.so`_dl_start_user:
0xf7fdb157 <+0>: movl   %eax, %edi
0xf7fdb159 <+2>: calll  0xf7fdb140
Process 21278 launched: '/home/dmlary/src/work/exec' (i386)

(lldb) image add ./rel.o
(lldb) image load --file rel.o .text 0x4000 .data 0x5000
section '.text' loaded at 0x4000
section '.data' loaded at 0x5000

(lldb) image dump symtab rel.o
Symtab, file = rel.o, num_symbols = 13:
   Debug symbol
   |Synthetic symbol
   ||Externally Visible
   |||
Index   UserID DSX TypeFile Address/Value Load Address   Size   
Flags  Name
--- -- --- --- -- -- 
-- -- --
[0]  1 SourceFile  0x
0x 0x0004 rel.c
[1]  2 Invalid 0x
0x0020 0x0003
[2]  3 Invalid 0x 0x5000 
0x0020 0x0003
[3]  4 Invalid 0x0025
0x 0x0003
[4]  5 Invalid 0x
0x0020 0x0003
[5]  6 Invalid 0x
0x0020 0x0003
[6]  7 Invalid 0x
0x0020 0x0003
[7]  8 Invalid 0x
0x0020 0x0003
[8]  9 Invalid 0x
0x0020 0x0003
[9] 10 Invalid 0x
0x0020 0x0003
[   10] 11 Invalid 0x
0x0020 0x0003
[   11] 12   X Data0x 0x5000 
0x0020 0x0011 padding
[   12] 13   X Data0x0020 0x5020 
0x0005 0x0011 var

(lldb) p 
(volatile char (*)[5]) $1 = 0x5000
```

Reviewed By: labath

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

Added: 
lldb/test/Shell/ObjectFile/ELF/i386-relocations.yaml

Modified: 
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index ba4a1f62f342..af1b0930c1b7 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -121,6 +121,8 @@ class ELFRelocation {
 
   static unsigned RelocAddend64(const ELFRelocation );
 
+  bool IsRela() { return (reloc.is()); }
+
 private:
   typedef llvm::PointerUnion RelocUnion;
 
@@ -2597,25 +2599,46 @@ unsigned ObjectFileELF::ApplyRelocations(
   }
 
   for (unsigned i = 0; i < num_relocations; ++i) {
-if (!rel.Parse(rel_data, ))
+if (!rel.Parse(rel_data, )) {
+  GetModule()->ReportError(".rel%s[%d] failed to parse relocation",
+   rel_section->GetName().AsCString(), i);
   break;
-
+}
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
  

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels (pre-4.1)

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Thanks!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133778

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

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels (pre-4.1)

[walter erquinigo via Phabricator via lldb-commits]

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

Lgtm, but please include the kernel version for the record


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133778

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

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> old Linux kernels

Also, could you include the kernel versions (that you know about at least) in 
the commit message. For the next person who hits this and wonders if your 
change is what they need to include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133778

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

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels

[Caleb Zulawski via Phabricator via lldb-commits]

calebzulawski added a comment.

In terms of who calls it and might error, it looks like it's all used in 
IntelPTMultiCoreTrace.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133778

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

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

llvm_unreachable is equivalent to __builtin_unreachable so yes it aborts with a 
stack trace. I misread and thought you were returning values still.

Worth trying one of the trace commands in this build and see if lldb crashes or 
we get a nice "not supported" message. unreachable is fine if we're reasonably 
sure you won't get there by accident.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133778

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

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels

[Caleb Zulawski via Phabricator via lldb-commits]

calebzulawski added a comment.

I'm not exactly sure what llvm_unreachable does, but I don't believe it will 
return 0 (maybe it aborts?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133778

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

[Lldb-commits] [PATCH] D133763: WIP strawman building perf.cpp on an older kernel

[David Spickett via Phabricator via lldb-commits]

DavidSpickett abandoned this revision.
DavidSpickett added a comment.

For https://github.com/llvm/llvm-project/issues/57594, succeeded by 
https://reviews.llvm.org/D133778.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133763

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

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added reviewers: wallace, jj10306.
DavidSpickett added a comment.

Adding reviewers who know the trace feature.

Seems fine at a glance for the functions that return error types already but 
for example `GetEffectiveDataBufferSize` check who calls it and might crash if 
it returns 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133778

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

[Lldb-commits] [PATCH] D133446: [LLDB][NativePDB] Global ctor and dtor should be global decls.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D133446#3780961 , @zequanwu wrote:

> In D133446#3779600 , @labath wrote:
>
>> I believe that this fixes the crash, but the names of generated functions 
>> still look fairly weird. Could we create them using their mangled name 
>> instead? That way, someone might actually call them, if he was so inclined.
>
> It looks like they don't have mangled name stored in pdb.

Hmm.. That's unfortunate. In that case, I'm wondering if this shouldn't be 
handled somehow directly inside `MSVCUndecoratedNameParser`. What does the 
function return right now? Do you think that result is going to be useful for 
other users of that class?

Also, I'm still continuing to be amazed (and scared) by the amount of name 
parsing that is going on here. Have you checked that there's no better way to 
associate an entity with its enclosing context?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133446

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

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[David Rector via Phabricator via lldb-commits]

davrec added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1113-1114
+// "double _Complex" is promoted to "long double _Complex".
+static QualType handleComplexFloatConversion(Sema , ExprResult ,
+ QualType LHSType, QualType 
RHSType,
+ bool PromotePrecision) {

davrec wrote:
> Rename params for clarity, e.g.
> LHS->Longer
> LHSType->LongerType
> RHSType->ShorterType
> 
Actually I probably have that backwards, I think LHS is the Shorter 
expression...in any case you see why renaming would be helpful


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

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

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[David Rector via Phabricator via lldb-commits]

davrec added a comment.

The lines you changed (clang/lib/Sema/SemaExpr.cpp:1091-1150) look good (IIUC 
you just change cast to cast_as and dyn_cast to getAs, and reorganize for 
clarity).  I suggested some renaming and a few more comments to further clarify.




Comment at: clang/lib/Sema/SemaExpr.cpp:1113-1114
+// "double _Complex" is promoted to "long double _Complex".
+static QualType handleComplexFloatConversion(Sema , ExprResult ,
+ QualType LHSType, QualType 
RHSType,
+ bool PromotePrecision) {

Rename params for clarity, e.g.
LHS->Longer
LHSType->LongerType
RHSType->ShorterType




Comment at: clang/lib/Sema/SemaExpr.cpp:1143
 return RHSType;
 
   int Order = S.Context.getFloatingTypeOrder(LHSType, RHSType);

```
// Compute the rank of the two types, regardless of whether they are complex.
```



Comment at: clang/lib/Sema/SemaExpr.cpp:1145
   int Order = S.Context.getFloatingTypeOrder(LHSType, RHSType);
-
-  auto *LHSComplexType = dyn_cast(LHSType);
-  auto *RHSComplexType = dyn_cast(RHSType);
-  QualType LHSElementType =
-  LHSComplexType ? LHSComplexType->getElementType() : LHSType;
-  QualType RHSElementType =
-  RHSComplexType ? RHSComplexType->getElementType() : RHSType;
-
-  QualType ResultType = S.Context.getComplexType(LHSElementType);
-  if (Order < 0) {
-// Promote the precision of the LHS if not an assignment.
-ResultType = S.Context.getComplexType(RHSElementType);
-if (!IsCompAssign) {
-  if (LHSComplexType)
-LHS =
-S.ImpCastExprToType(LHS.get(), ResultType, CK_FloatingComplexCast);
-  else
-LHS = S.ImpCastExprToType(LHS.get(), RHSElementType, CK_FloatingCast);
-}
-  } else if (Order > 0) {
-// Promote the precision of the RHS.
-if (RHSComplexType)
-  RHS = S.ImpCastExprToType(RHS.get(), ResultType, CK_FloatingComplexCast);
-else
-  RHS = S.ImpCastExprToType(RHS.get(), LHSElementType, CK_FloatingCast);
-  }
-  return ResultType;
+  if (Order < 0)
+return handleComplexFloatConversion(S, LHS, LHSType, RHSType,

```
// Promote the precision of the LHS if not an assignment.
```



Comment at: clang/lib/Sema/SemaExpr.cpp:1147
+return handleComplexFloatConversion(S, LHS, LHSType, RHSType,
+/*PromotePrecision=*/!IsCompAssign);
+  return handleComplexFloatConversion(S, RHS, RHSType, LHSType,

```
// Promote the precision of the RHS unless it is already the same as the LHS.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

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

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels

[Caleb Zulawski via Phabricator via lldb-commits]

calebzulawski created this revision.
calebzulawski added a reviewer: DavidSpickett.
calebzulawski added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
calebzulawski requested review of this revision.
Herald added a subscriber: lldb-commits.

These fields are guarded elsewhere, but were missing here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133778

Files:
  lldb/source/Plugins/Process/Linux/Perf.cpp

Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -125,6 +125,10 @@
 }
 
 llvm::Error PerfEvent::MmapAuxBuffer(size_t num_aux_pages) {
+#ifndef PERF_ATTR_SIZE_VER5
+  return createStringError(inconvertibleErrorCode(),
+   "Intel PT Linux perf event not supported");
+#else
   if (num_aux_pages == 0)
 return Error::success();
 
@@ -141,6 +145,7 @@
 return Error::success();
   } else
 return mmap_aux.takeError();
+#endif
 }
 
 llvm::Error PerfEvent::MmapMetadataAndBuffers(size_t num_data_pages,
@@ -170,16 +175,24 @@
 }
 
 ArrayRef PerfEvent::GetDataBuffer() const {
+#ifndef PERF_ATTR_SIZE_VER5
+  llvm_unreachable("Intel PT Linux perf event not supported");
+#else
   perf_event_mmap_page _metadata = GetMetadataPage();
   return {reinterpret_cast(m_metadata_data_base.get()) +
   mmap_metadata.data_offset,
   static_cast(mmap_metadata.data_size)};
+#endif
 }
 
 ArrayRef PerfEvent::GetAuxBuffer() const {
+#ifndef PERF_ATTR_SIZE_VER5
+  llvm_unreachable("Intel PT Linux perf event not supported");
+#else
   perf_event_mmap_page _metadata = GetMetadataPage();
   return {reinterpret_cast(m_aux_base.get()),
   static_cast(mmap_metadata.aux_size)};
+#endif
 }
 
 Expected> PerfEvent::GetReadOnlyDataBuffer() {
@@ -188,6 +201,10 @@
   // this piece of code updates some pointers. See more about data_tail
   // in https://man7.org/linux/man-pages/man2/perf_event_open.2.html.
 
+#ifndef PERF_ATTR_SIZE_VER5
+  return createStringError(inconvertibleErrorCode(),
+   "Intel PT Linux perf event not supported");
+#else
   bool was_enabled = m_enabled;
   if (Error err = DisableWithIoctl())
 return std::move(err);
@@ -224,6 +241,7 @@
   }
 
   return output;
+#endif
 }
 
 Expected> PerfEvent::GetReadOnlyAuxBuffer() {
@@ -232,6 +250,10 @@
   // this piece of code updates some pointers. See more about aux_tail
   // in https://man7.org/linux/man-pages/man2/perf_event_open.2.html.
 
+#ifndef PERF_ATTR_SIZE_VER5
+  return createStringError(inconvertibleErrorCode(),
+   "Intel PT Linux perf event not supported");
+#else
   bool was_enabled = m_enabled;
   if (Error err = DisableWithIoctl())
 return std::move(err);
@@ -264,6 +286,7 @@
   }
 
   return output;
+#endif
 }
 
 Error PerfEvent::DisableWithIoctl() {
@@ -295,11 +318,15 @@
 }
 
 size_t PerfEvent::GetEffectiveDataBufferSize() const {
+#ifndef PERF_ATTR_SIZE_VER5
+  llvm_unreachable("Intel PT Linux perf event not supported");
+#else
   perf_event_mmap_page _metadata = GetMetadataPage();
   if (mmap_metadata.data_head < mmap_metadata.data_size)
 return mmap_metadata.data_head;
   else
 return mmap_metadata.data_size; // The buffer has wrapped.
+#endif
 }
 
 Expected
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133763: WIP strawman building perf.cpp on an older kernel

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

(This file is only used for the processor trace feature, and one can debug just 
fine without it. So, I'd just conditionally compile out the processor trace 
support on older kernels -- it's highly questionable whether this works there 
anyway).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133763

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

[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

[Pavel Labath via Phabricator via lldb-commits]

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

This seems in line with what was being discussed. Jim, do you have any thoughts 
on this?




Comment at: lldb/include/lldb/lldb-enumerations.h:841
+
+  kNumFormatterMatchTypes,
+};

I see the enums in this file use wildly inconsistent styles for the "largest 
value" enumerator. However, you've picked the one I like the least, because 
this tends to produce `unhandled switch case "kNumFormatterMatchTypes"` 
warnings. If you feel like you need to have a sentinel, I'd recommend going 
with the `eLastFormatterMatchType = ` pattern (used e.g. in 
the StateType enum).


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

https://reviews.llvm.org/D133240

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

[Lldb-commits] [PATCH] D133534: Complete support of loading a darwin kernel over a live gdb-remote connection given the address of a mach-o fileset

[Nico Weber via Phabricator via lldb-commits]

thakis added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt:47
 lldbUtility
+lldbPluginDynamicLoaderDarwinKernel
+lldbPluginObjectContainerMachOFileset

jasonmolenda wrote:
> jasonmolenda wrote:
> > thakis wrote:
> > > This causes a dependency cycle:
> > > 
> > >   //lldb/source/Plugins/Platform/MacOSX:MacOSX ->
> > >   //lldb/source/Plugins/DynamicLoader/Darwin-Kernel:Darwin-Kernel ->
> > >   //lldb/source/Plugins/Platform/MacOSX:MacOSX
> > Ach, naturally.  DynamicLoaderDarwinKernel has to create a 
> > PlatformDarwinKernel to set in the Target when it is initializing itself.  
> > :/  Maybe I'll just add DynamicLoaderDarwinKernel to the unit tests that 
> > have PlatformMacOSX in them.
> I removed the dependency in DynamicLoaderDarwinKernel, a very specialized 
> plugin, and left the dependency in PlatformMacOSX which includes all of the 
> darwin platforms and is a common one to import.  I believe any target that is 
> linking against DynamicLoaderDarwinKernel will also have a dependency on 
> PlatformMacOSX already.  I landed this as 
> 30578c08568bc8de79dea72e41f49899ba10ea55 to make sure this causes no 
> problems, we can fix it better if someone has a suggestion.
Thanks!

> I believe anything linking the darwin kernel DynamicLoader plugin
> will already have lldbPluginPlatformMacOSX in its dependency list,
> so not explicitly expressing this dependency is safe.

That sounds like it doesn't really remove the dependency, it just lies about 
the build system about it and things still happen to work :)

Normally, if you have a cycle between libraries A and B, you'd either:

- decide which of the two should depend on the other
- make the "lower" library expose some virtual delegate
- implement that in the "higher" library

…or move the needed shared bits to an even lower library that both A and B 
naturally depend on already.

https://chromium.googlesource.com/chromium/src/+/lkgr/docs/patterns/inversion-of-control.md
 has some (slightly, but not overly so) chromium-specific notes on this.

But layering in lldb is generally a mess, so I think your workaround is ok for 
now. It'd be nice to clean up lldb's layering at some point!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133534

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

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov marked an inline comment as not done.
mizvekov added a subscriber: davrec.
mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1091-1150
+QualType fpTy = ComplexTy->castAs()->getElementType();
 IntExpr = S.ImpCastExprToType(IntExpr.get(), fpTy, CK_IntegralToFloating);
 IntExpr = S.ImpCastExprToType(IntExpr.get(), ComplexTy,
   CK_FloatingRealToComplex);
   } else {
 assert(IntTy->isComplexIntegerType());
 IntExpr = S.ImpCastExprToType(IntExpr.get(), ComplexTy,

mizvekov wrote:
> @shafik let's continue revision of D133522 in this patch, since that one was 
> a quick fix that I attempted in order to avoid reverting it. But now that it 
> was reverted, I will abandon the other one.
> 
> I have highlighted with this comment the areas affected by D133522 so we can 
> continue here.
> 
> But now I have refactored the whole thing to be simpler / more readable.
Also @davrec if you can please take a look at this, this is the only part of 
the patch that changed, in order to fix the reason why it was reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

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

[Lldb-commits] [PATCH] D133763: WIP strawman building perf.cpp on an older kernel

[David Spickett via Phabricator via lldb-commits]

DavidSpickett planned changes to this revision.
DavidSpickett added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:28
 lldb_private::process_linux::LoadPerfTscConversionParameters() {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 12, 0)
   lldb::pid_t pid = getpid();

Use this sort of thing, or an ifdef set by cmake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133763

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

[Lldb-commits] [PATCH] D133763: WIP strawman building perf.cpp on an older kernel

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133763

Files:
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h

Index: lldb/source/Plugins/Process/Linux/Perf.h
===
--- lldb/source/Plugins/Process/Linux/Perf.h
+++ lldb/source/Plugins/Process/Linux/Perf.h
@@ -22,6 +22,21 @@
 #include 
 #include 
 
+struct perf_event_mmap_page_custom {
+  int cap_user_time;
+  int cap_user_time_zero;
+  uint32_t time_mult;
+  uint16_t time_shift;
+  uint64_t time_zero;
+
+  //uint64_t aux_head;
+  //int aux_offset;
+  //unsigned long aux_size;
+  //uint64_t data_head;
+  //int data_offset;
+  //unsigned long data_size;
+};
+
 namespace lldb_private {
 namespace process_linux {
 namespace resource_handle {
@@ -142,7 +157,7 @@
   /// \param[in] num_data_pages
   /// Number of pages in the data buffer to mmap, must be a power of 2.
   /// A value of 0 is useful for "dummy" events that only want to access
-  /// the metadata, \a perf_event_mmap_page, or the aux buffer.
+  /// the metadata, \a perf_event_mmap_page_custom, or the aux buffer.
   ///
   /// \param[in] num_aux_pages
   /// Number of pages in the aux buffer to mmap, must be a power of 2.
@@ -171,8 +186,8 @@
   /// otherwise a failure might happen.
   ///
   /// \return
-  ///   The data section's \a perf_event_mmap_page.
-  perf_event_mmap_page () const;
+  ///   The data section's \a perf_event_mmap_page_custom.
+  perf_event_mmap_page_custom () const;
 
   /// Get the data buffer from the data section's mmap buffer.
   ///
@@ -278,7 +293,7 @@
   /// \param[in] num_data_pages
   /// Number of pages in the data buffer to mmap, must be a power of 2.
   /// A value of 0 is useful for "dummy" events that only want to access
-  /// the metadata, \a perf_event_mmap_page, or the aux buffer.
+  /// the metadata, \a perf_event_mmap_page_custom, or the aux buffer.
   ///
   /// \param[in] data_buffer_write
   /// Whether to mmap the data buffer with WRITE permissions. This changes
@@ -317,7 +332,7 @@
 CreateContextSwitchTracePerfEvent(lldb::cpu_id_t cpu_id,
   const PerfEvent *parent_perf_event = nullptr);
 
-/// Load \a PerfTscConversionParameters from \a perf_event_mmap_page, if
+/// Load \a PerfTscConversionParameters from \a perf_event_mmap_page_custom, if
 /// available.
 llvm::Expected LoadPerfTscConversionParameters();
 
Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -42,7 +42,7 @@
  /*data_buffer_write=*/false))
 return std::move(mmap_err);
 
-  perf_event_mmap_page _metada = perf_event->GetMetadataPage();
+  perf_event_mmap_page_custom _metada = perf_event->GetMetadataPage();
   if (mmap_metada.cap_user_time && mmap_metada.cap_user_time_zero) {
 return LinuxPerfZeroTscConversion{
 mmap_metada.time_mult, mmap_metada.time_shift, {mmap_metada.time_zero}};
@@ -128,19 +128,22 @@
   if (num_aux_pages == 0)
 return Error::success();
 
-  perf_event_mmap_page _page = GetMetadataPage();
+  return llvm::createStringError(
+llvm::inconvertibleErrorCode(), "foo");
 
-  metadata_page.aux_offset =
-  metadata_page.data_offset + metadata_page.data_size;
-  metadata_page.aux_size = num_aux_pages * getpagesize();
-
-  if (Expected mmap_aux =
-  DoMmap(nullptr, metadata_page.aux_size, PROT_READ, MAP_SHARED,
- metadata_page.aux_offset, "aux buffer")) {
-m_aux_base = std::move(mmap_aux.get());
-return Error::success();
-  } else
-return mmap_aux.takeError();
+//  perf_event_mmap_page_custom _page = GetMetadataPage();
+//
+//  metadata_page.aux_offset =
+//  metadata_page.data_offset + metadata_page.data_size;
+//  metadata_page.aux_size = num_aux_pages * getpagesize();
+//
+//  if (Expected mmap_aux =
+//  DoMmap(nullptr, metadata_page.aux_size, PROT_READ, MAP_SHARED,
+// metadata_page.aux_offset, "aux buffer")) {
+//m_aux_base = std::move(mmap_aux.get());
+//return Error::success();
+//  } else
+//return mmap_aux.takeError();
 }
 
 llvm::Error PerfEvent::MmapMetadataAndBuffers(size_t num_data_pages,
@@ -165,21 +168,24 @@
 
 long PerfEvent::GetFd() const { return *(m_fd.get()); }
 
-perf_event_mmap_page ::GetMetadataPage() const {
-  return *reinterpret_cast(m_metadata_data_base.get());
+perf_event_mmap_page_custom ::GetMetadataPage() const {
+  return *reinterpret_cast(m_metadata_data_base.get());
 }
 
 ArrayRef PerfEvent::GetDataBuffer() const {
-  perf_event_mmap_page _metadata = GetMetadataPage();