[Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Nice cleanup :-) Comment at: source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1918 +.Cases("r12", "r13", "r14", "r15", + "rbp", "rbx", "ebp", "ebx", true)

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

compnerd created this revision. compnerd added reviewers: davide, labath, vsk, zturner. Herald added a subscriber: mgorny. Use proper cmake techniques to detect where the libedit package resides. This allows for the use of libedit from an alternative location which is needed for supporting

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

compnerd added inline comments. Comment at: cmake/modules/FindLibEdit.cmake:11-14 +# libedit_FOUND - true if libedit was found +# libedit_INCLUDE_DIRS - include search path +# libedit_LIBRARIES - libraries to link +# libedit_VERSION - version number

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

compnerd updated this revision to Diff 147874. compnerd added a comment. fix regex https://reviews.llvm.org/D46726 Files: CMakeLists.txt cmake/modules/FindLibEdit.cmake scripts/Python/modules/readline/CMakeLists.txt Index: scripts/Python/modules/readline/CMakeLists.txt

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

compnerd added inline comments. Comment at: cmake/modules/FindLibEdit.cmake:53-54 +set(libedit_VERSION_STRING "${libedit_major_version}.${libedit_minor_version}") +unset(libedit_major_version) +unset(libedit_minor_version) + endif() labath wrote: >

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

compnerd closed this revision. compnerd added a comment. SVN r333041 https://reviews.llvm.org/D46726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47897: Check for process_vm_readv using CheckSymbolExists

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. This should be functionally identical and is much cleaner. Thanks! https://reviews.llvm.org/D47897 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

compnerd added inline comments. Comment at: CMakeLists.txt:145 + add_dependencies(lldb-suite lldb-framework) +elseif() + if (LLDB_CAN_USE_LLDB_SERVER) Shouldn't this be `else()`? Comment at: CMakeLists.txt:176 +if (LLDB_BUILD_FRAMEWORK)

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

compnerd added inline comments. Comment at: scripts/Python/modules/readline/CMakeLists.txt:11 + PRIVATE + ${libedit_INCLUDE_DIRS}) if (NOT LLDB_DISABLE_LIBEDIT) lonico77 wrote: > Should this be under if

[Lldb-commits] [PATCH] D48993: [CMake] Give Python module install command a component name

compnerd added a comment. The `install-` target is created by `add_llvm_install_targets`, not by the component. The component is part of CMake itself, and is controlled by the `CMAKE_INSTALL_DEFAULT_COMPONENT_NAME`. https://reviews.llvm.org/D48993

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

compnerd added a comment. It definitely shouldn't be `PUBLIC`, `PRIVATE` makes sense for this inclusion. Is there a library that pulls in the dependency (that is, do the other libraries need it due to `lldbHost`? If so, just adding it as `INTERFACE` on that library should be sufficient).

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rLLDB LLDB https://reviews.llvm.org/D51999 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. I personally prefer the use of `target_link_libraries` rather than the custom stuff added in the `add_lldb_tool`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56741/new/

[Lldb-commits] [PATCH] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

compnerd requested changes to this revision. compnerd added inline comments. This revision now requires changes to proceed. Comment at: cmake/modules/LLDBStandalone.cmake:7 option(LLVM_INSTALL_TOOLCHAIN_ONLY "Only include toolchain files in the 'install' target." OFF) +

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. Can you use `target_include_directories` instead and do this only on `lldbHost` instead please? That restricts the inclusion to just that target, which would help prevent

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

compnerd added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:67 + relative_path.clear(); + llvm::sys::path::append(relative_path, "lib", "lldb", "clang"); + llvm::sys::path::append(clang_dir, relative_path); Does swift-lldb

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Please add a comment about `verify` being only for tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59708/new/ https://reviews.llvm.org/D59708

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd added a comment. @sgraenitz yeah, I passed `LLVM_DIR` and `Clang_DIR`, but, this is for a **standalone** build, so I think that it is pretty reasonable to ask that the user tell us where LLVM and Clang are built. Although, if you install LLVM and Clang to your root (like on Linux),

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd added a comment. Yes, both paths currently work since the `LLDB_PATH_TO_*` variables are just hints to where to look. It just seems unnecessary to have the custom variables when CMake has a mechanism for directing the build to look for packages in a certain location. Repository:

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd updated this revision to Diff 186780. compnerd added a comment. Add HINTS Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57402/new/ https://reviews.llvm.org/D57402 Files: cmake/modules/LLDBStandalone.cmake Index: cmake/modules/LLDBStandalone.cmake

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd added a comment. @labath - absolutely, that I don't have a problem with. I think that having the additional LLDB specific paths with `LLDB_PATH_TO_*` is better done by using the standard CMake mechanisms. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION

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

compnerd added a comment. @sgraenitz - not really ... the `find_package` will load the `LLVMConfig.cmake` that is generated. We only have what we keep in there. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58193/new/ https://reviews.llvm.org/D58193

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

compnerd added a comment. Probably just a replay of GDB protocol packets connected to a testing server would work. But, I don't know if there is infrastructure for that yet. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58405/new/ https://reviews.llvm.org/D58405

[Lldb-commits] [PATCH] D58398: Add Facebook Minidump directory streams and options to dump them.

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Would be nice to change the filecheck prefixes to `CHECK-...` to separate the CHECK and the item being checked. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58398/new/

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd created this revision. compnerd added reviewers: zturner, xiaobai, sgraenitz, labath. Herald added subscribers: teemperor, mgorny. Prefer the standard CMake behaviour of using `_DIR` variables to indicate where to find the CMake configurations. This allows implicit use of the system

[Lldb-commits] [PATCH] D57194: [CMake] Use llvm-tblgen from NATIVE LLVM build when cross-compiling

compnerd accepted this revision. compnerd added inline comments. This revision is now accepted and ready to land. Comment at: cmake/modules/LLDBStandalone.cmake:44 +else() + set(LLVM_TABLEGEN_EXE +

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::SetCompilerTypeFromPersistentDecl

compnerd added inline comments. Comment at: source/Commands/CommandObjectMemory.cpp:479 +if (persistent_vars->SetCompilerTypeFromPersistentDecl( +lookup_type_name, clang_ast_type)) + break; Why is the parameter

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. This should get the build working again, so lets get this fixed, we can improve it later Comment at:

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Thanks, this looks much better. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62702/new/ https://reviews.llvm.org/D62702

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

compnerd added inline comments. Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1563 +// The compiler will faltten the nested aggregate type into single +// layer and push the value to stack NIT: `faltten` -> `flatten`

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. Actually, I think that we should extend `CompilerType` and `TypeSystem` to expose Clang's knowledge of whether a type is passed in a register by means of using

[Lldb-commits] [PATCH] D62812: [llvm] [CodeView] Move Triple::ArchType → CPUType mapping from LLDB

compnerd added inline comments. Comment at: llvm/include/llvm/DebugInfo/CodeView/CodeView.h:145 + switch (ArchType) { + case Triple::ArchType::aarch64: +return CPUType::ARM64; I that `aarch64_be` and `aarch64_32` should be included in this.

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

compnerd added inline comments. Comment at: lldb/source/Symbol/ClangASTContext.cpp:3915 +bool ClangASTContext::CanPassInRegisters(const CompilerType ) { + if (clang::RecordDecl *record_decl = + ClangASTContext::GetAsRecordDecl(type)) { I think that using

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

compnerd accepted this revision. compnerd added a subscriber: clayborg. compnerd added a comment. This revision is now accepted and ready to land. Would be nice to get someone like @clayborg to chime in, but, I think that @labath also seems to think that this is fine. Comment

[Lldb-commits] [PATCH] D62771: [LLDBRegisterNum] Update function call llvm::codeview::getRegisterNames(CPUType) in lldb

compnerd accepted this revision. compnerd added a comment. Generally, `clang-format` the changes, it will catch the formatting things. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:28 + llvm::codeview::CPUType cpu; +

[Lldb-commits] [PATCH] D62772: [COFF, ARM64] Fix CodeView API change for getRegisterNames

compnerd requested changes to this revision. compnerd added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:29 + llvm::codeview::CPUType cpu_type; + if (arch_type ==

[Lldb-commits] [PATCH] D63052: [Target] Remove Process::GetObjCLanguageRuntime

compnerd added inline comments. Comment at: include/lldb/Target/ObjCLanguageRuntime.h:202 + static ObjCLanguageRuntime *GetObjCLanguageRuntime(Process ) { +return llvm::cast_or_null( I think it would be nice to just call this `Get` (and we could have

[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

compnerd added inline comments. Comment at: include/lldb/Target/LanguageRuntime.h:137 + virtual DeclVendor *GetDeclVendor() { return nullptr; } + Can this not be `const`? Seems like retrieving the vendor should not mutate the runtime.

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

compnerd added inline comments. Comment at: source/Plugins/Process/Utility/RegisterContextWindows_i386.cpp:40 + +// clang-format off +#define DEFINE_GPR(reg, alt, kind1, kind2, kind3, kind4) \ I believe that this bounds the range, and

[Lldb-commits] [PATCH] D61686: Enable lldb-server on Windows

compnerd added inline comments. Comment at: include/lldb/Host/windows/PosixApi.h:106-109 +inline pid_t waitpid(pid_t pid, int *status, int options) { + // To be implemented. + return pid_t(-1); +} labath wrote: > As discussed in the review where this was

[Lldb-commits] [PATCH] D62273: Expression: correct relocation model for Windows

compnerd created this revision. compnerd added reviewers: aprantl, JDevlieghere, labath, clayborg, xiaobai, davide. Herald added a project: LLDB. The Windows CG model cannot generate code with the PIC relocation model as all code is implicitly PIC. Invert the condition and inline the single

[Lldb-commits] [PATCH] D62213: [ABI] Implement Windows ABI for x86_64

compnerd accepted this revision. compnerd added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1094 + if (arch_type == llvm::Triple::x86_64 && + os_type != llvm::Triple::OSType::Win32) {

[Lldb-commits] [PATCH] D62273: Expression: correct relocation model for Windows

compnerd closed this revision. compnerd added a comment. SVN r361443 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62273/new/ https://reviews.llvm.org/D62273 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D61686: Enable lldb-server on Windows

compnerd added inline comments. Comment at: include/lldb/Host/windows/PosixApi.h:108 + // To be implemented. + return pid_t(-1); +} This should be out-of-lined. Furthermore, is there any place where the use requires process group handling? Otherwise, can't

[Lldb-commits] [PATCH] D62213: [ABI] Implement Windows ABI for x86_64

compnerd added inline comments. Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1094 + if (arch_type == llvm::Triple::x86_64 && + os_type != llvm::Triple::OSType::Win32) { return ABISP(new ABISysV_x86_64(process_sp)); This really

[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

compnerd added inline comments. Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18 +set(CMAKE_BUILD_TYPE RelWithDebInfo) +set(LLVM_ENABLE_MODULES ON CACHE BOOL "") sgraenitz wrote: > Can / Should we add this? Here? This is fine to add assuming that you are

[Lldb-commits] [PATCH] D62159: LLGS: support 32-bit on 64-bit hosts

compnerd updated this revision to Diff 200366. compnerd added a comment. Fix inclusion Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62159/new/ https://reviews.llvm.org/D62159 Files: tools/lldb-server/SystemInitializerLLGS.cpp Index:

[Lldb-commits] [PATCH] D62159: LLGS: support 32-bit on 64-bit hosts

compnerd created this revision. compnerd added reviewers: xiaobai, labath, clayborg. Herald added subscribers: abidh, atanasyan, kristof.beyls, arichardson, javed.absar, sdardis. Herald added a project: LLDB. Enable the ARM emulation support on AArch64 which can execute ARM32 code. Similarly,

[Lldb-commits] [PATCH] D61362: lldb-server: remove link against lldbInterpreter

compnerd created this revision. compnerd added reviewers: davide, aprantl, clayborg. Herald added a subscriber: mgorny. Herald added a project: LLDB. This dependency is unused. Remove the extraneous link. Repository: rLLDB LLDB https://reviews.llvm.org/D61362 Files:

[Lldb-commits] [PATCH] D61360: Initialization: remove ObjectContainer from Common

compnerd created this revision. compnerd added reviewers: davide, aprantl, clayborg. Herald added subscribers: abidh, mgorny. Herald added a project: LLDB. This restructures the initialization path to move the ObjectContainer initialization into the *full* initialization path. This is not needed

[Lldb-commits] [PATCH] D61361: PluginInstructionARM: avoid unnecessary link

compnerd created this revision. compnerd added reviewers: davide, aprantl, clayborg. Herald added subscribers: kristof.beyls, javed.absar, mgorny. Herald added a project: LLDB. lldbPluginInstructionARM does not depend on lldbProcessUtility at link time. The dependency is for defines and helper

[Lldb-commits] [PATCH] D61362: lldb-server: remove link against lldbInterpreter

compnerd closed this revision. compnerd added a comment. SVN r359738 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61362/new/ https://reviews.llvm.org/D61362 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D61543: Initialization: move InstructionEmulation to full initialization

compnerd closed this revision. compnerd added a comment. SVN r360067 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61543/new/ https://reviews.llvm.org/D61543 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D61543: Initialization: move InstructionEmulation to full initialization

compnerd created this revision. compnerd added reviewers: labath, xiaobai, clayborg. Herald added a subscriber: mgorny. Herald added a project: LLDB. The debug server does not need to use the instruction emulation. This helps reduce the size of the final lldb-server binary by another ~100K (~1%

[Lldb-commits] [PATCH] D61473: ExpressionParser: only force link MCJIT when needed

compnerd closed this revision. compnerd added a comment. SVN r359944 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61473/new/ https://reviews.llvm.org/D61473 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D61360: Initialization: remove ObjectContainer from Common

compnerd closed this revision. compnerd added a comment. SVN r359810 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61360/new/ https://reviews.llvm.org/D61360 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D61361: PluginInstructionARM: avoid unnecessary link

compnerd updated this revision to Diff 197793. compnerd added a comment. convert link to a proper dependency Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61361/new/ https://reviews.llvm.org/D61361 Files: source/Plugins/Instruction/ARM/CMakeLists.txt

[Lldb-commits] [PATCH] D61361: PluginInstructionARM: avoid unnecessary link

compnerd added a comment. @labath - I don't have a problem with the dependency - we can do a `add_dependency` for the plugin. My issue is that this should not be linked against. This mess of dependencies makes it difficult to track down the bloat in `lldb-server`. Currently, `lldb-server`

[Lldb-commits] [PATCH] D61360: Initialization: remove ObjectContainer from Common

compnerd added inline comments. Comment at: source/API/SystemInitializerFull.cpp:132-133 + ObjectContainerBSDArchive::Initialize(); + ObjectContainerUniversalMachO::Initialize(); + labath wrote: > xiaobai wrote: > > You need to include the headers for these

[Lldb-commits] [PATCH] D61360: Initialization: remove ObjectContainer from Common

compnerd updated this revision to Diff 197797. compnerd marked 6 inline comments as done. compnerd added a comment. address feedback Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61360/new/ https://reviews.llvm.org/D61360 Files:

[Lldb-commits] [PATCH] D61473: ExpressionParser: only force link MCJIT when needed

compnerd created this revision. compnerd added reviewers: labath, clayborg, emaste, davide, xiaobai. Herald added subscribers: teemperor, abidh, krytarowski, mgorny. Herald added a project: LLDB. This was added to support FreeBSD. The inclusion of this header increases the size of `lldb-server`

[Lldb-commits] [PATCH] D61473: ExpressionParser: only force link MCJIT when needed

compnerd added a comment. gold-2.27, clang 6.0.0, `-ffunction-sections`, `-fdata-sections`, `-DLLVM_ENABLE_UNWIND_TABLES=NO`, MinSizeRel: **BEFORE** bin/lldb-server : section size addr .interp 284194928 .note.ABI-tag

[Lldb-commits] [PATCH] D61473: ExpressionParser: only force link MCJIT when needed

compnerd updated this revision to Diff 198020. compnerd added a comment. Move the inclusion as suggested. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61473/new/ https://reviews.llvm.org/D61473 Files: source/API/SystemInitializerFull.cpp

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

compnerd added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" +#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h" xiaobai wrote: >

[Lldb-commits] [PATCH] D64159: [Core] Generalize ValueObject::MaybeCalculateCompleteType

compnerd accepted this revision. compnerd added a comment. Seems that all the comments have been addressed and this is purely code motion. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64159/new/ https://reviews.llvm.org/D64159 ___

[Lldb-commits] [PATCH] D66448: Include "windows" Instead of "Windows"

compnerd closed this revision. compnerd added a comment. SVN r369307 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66448/new/ https://reviews.llvm.org/D66448 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D66858: POSIX DYLD: add workaround for android L loader

compnerd created this revision. compnerd added reviewers: davide, xiaobai. Herald added subscribers: abidh, srhines. Herald added a project: LLDB. In certain cases, the loader does not report the base address of the DSO. Attempt to infer it from the loaded address of the object file. This was

[Lldb-commits] [PATCH] D66445: Explicitly Cast Constants to DWORD

compnerd closed this revision. compnerd added a comment. SVN r369788 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66445/new/ https://reviews.llvm.org/D66445 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D65409: [ProcessWindows] Choose a register context file by prepocessor

compnerd added inline comments. Comment at: Common/CMakeLists.txt:28 +target_sources(lldbPluginProcessWindowsCommon PRIVATE + x86/RegisterContextWindows_x86.cpp) At this point, I would say its better to just merge it into the main source list.

[Lldb-commits] [PATCH] D65798: [lldb][CMake] Infer `Clang_DIR` if not passed explicitly

compnerd added inline comments. Comment at: lldb/cmake/modules/LLDBStandalone.cmake:6 + # next to LLVM's module directory. + set(Clang_DIR ${LLVM_DIR}/../clang) + message(STATUS "Inferred Clang_DIR: ${Clang_DIR}") What happens in the standalone clang build

[Lldb-commits] [PATCH] D65798: [lldb][CMake] Infer `Clang_DIR` if not passed explicitly

compnerd added inline comments. Comment at: lldb/cmake/modules/LLDBStandalone.cmake:6 + # next to LLVM's module directory. + set(Clang_DIR ${LLVM_DIR}/../clang) + message(STATUS "Inferred Clang_DIR: ${Clang_DIR}") sgraenitz wrote: > compnerd wrote: > > What

[Lldb-commits] [PATCH] D64806: [CMake] Always build debugserver on Darwin and allow tests to use the system's one

compnerd added inline comments. Comment at: lldb/cmake/modules/AddLLDB.cmake:292 + else() +string(STRIP ${xcode_dev_dir} xcode_dev_dir) +set(subdir "LLDB.framework/Resources/debugserver") Can you add a comment explaining that you want to strip leading

[Lldb-commits] [PATCH] D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing

compnerd added inline comments. Comment at: lldb/source/Host/common/Socket.cpp:479 } - NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept4, + NativeSocket fd = llvm::sys::RetryAfterSignal((NativeSocket) -1, ::accept4, sockfd, addr, addrlen, flags);

[Lldb-commits] [PATCH] D67913: [LLDB] [Windows] Map COFF ARM machine ids to the right triplet architectures

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Nit: `triple`, not `triplet`. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67913/new/ https://reviews.llvm.org/D67913

[Lldb-commits] [PATCH] D67911: [LLDB] [Windows] Add missing ifdefs to fix building for non-x86 architectures

compnerd added a comment. What do you think of adding some sort of notification that hardware breakpoints are currently unsupported and that we are falling back to software breakpoints for the `#else` case? Comment at:

[Lldb-commits] [PATCH] D67911: [LLDB] [Windows] Add missing ifdefs to fix building for non-x86 architectures

compnerd added a comment. I think that `printf` is quite an amazing notification :-) Comment at: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp:87 case 4: #if defined(__x86_64__) || defined(_M_AMD64) case 8: mstorsjo wrote: >

[Lldb-commits] [PATCH] D67954: [LLDB] [Windows] Initial support for ARM64 debugging

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Honestly, this is just setting up the register context for ARM64. I dont think that there is much of a test for this. I mean, I suppose you could test this by instantiating the context

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

compnerd updated this revision to Diff 230658. compnerd added a comment. Use @labath's suggestion of bumping minimum required version for Windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69535/new/ https://reviews.llvm.org/D69535 Files:

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd added a comment. In D70764#1767395 , @JDevlieghere wrote: > Having one canonical variable controlling zlib support seems indeed desirable. > > In D70519#1754618 , @labath wrote: > > > With this patch,

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

compnerd added a comment. @stella.stamenova ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69535/new/ https://reviews.llvm.org/D69535 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

compnerd created this revision. compnerd added reviewers: xiaobai, stella.stamenova. Herald added subscribers: JDevlieghere, mgorny. Herald added a project: LLDB. If we have a new enough CMake, use `find_package(Python3)`. This version is able to check both 32-bit and 64-bit versions and will

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

compnerd added a comment. The reason for bringing this back up as a Windows specific thing is that currently, there is no good way to build LLDB with python support without having to specify additional details on *just* windows because the windows path is doing something special. This is

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

compnerd marked an inline comment as done. compnerd added a comment. Yeah, doing an incremental rollout makes sense. Comment at: lldb/cmake/modules/LLDBConfig.cmake:225 + if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.13 AND CMAKE_SYSTEM_NAME STREQUAL Windows) +

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd created this revision. compnerd added reviewers: beanz, smeenai. Herald added subscribers: lldb-commits, Sanitizers, hiraditya, mgorny. Herald added projects: clang, Sanitizers, LLDB, LLVM. Rather than handling zlib handling manually, use `find_package` from CMake to find zlib properly.

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd added a comment. @labath I think you are misunderstanding the patch. This is not autoselecting the dependencies. It is simply doing that based on an existing option that we have - `LLVM_ENABLE_ZLIB`. We could always search for zlib and override the results with `LLVM_ENABLE_ZLIB`

[Lldb-commits] [PATCH] D68614: [LLDB] Remove standalone build dep on llvm-strip

compnerd added inline comments. Comment at: lit/CMakeLists.txt:64 ) +if(NOT LLDB_BUILT_STANDALONE) + list(APPEND LLDB_TEST_DEPS llvm-strip) JDevlieghere wrote: > xiaobai wrote: > > why not `if(TARGET llvm-strip)`? I think that expresses the intent more > >

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

compnerd closed this revision. compnerd added a comment. GIT 2046d72e916 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69535/new/ https://reviews.llvm.org/D69535

[Lldb-commits] [PATCH] D73289: [lldb/Test] Disallow using substituted binaries in shell test.

compnerd accepted this revision. compnerd added inline comments. This revision is now accepted and ready to land. Comment at: lldb/test/Shell/helper/toolchain.py:24 + warning.format(execName))) + + Wow, that took a couple of reads

[Lldb-commits] [PATCH] D73067: [lldb/CMake] Auto-generate the Initialize and Terminate calls for plugins

compnerd added a comment. Do we need to worry about ordering of the plugins? Comment at: lldb/source/Plugins/Platform/CMakeLists.txt:9 add_subdirectory(MacOSX) -#elseif (CMAKE_SYSTEM_NAME MATCHES "Windows") +elseif (CMAKE_SYSTEM_NAME MATCHES "Windows")

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

compnerd closed this revision. compnerd added a comment. GIT 68a235d07f9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70764/new/ https://reviews.llvm.org/D70764

[Lldb-commits] [PATCH] D72290: [lldb/CMake] Use LLDB's autodetection logic for libxml2

compnerd added inline comments. Comment at: lldb/cmake/modules/FindLibXml28.cmake:14 +if (APPLE) + set(LIBXML2_LIBRARIES xml2) +endif() labath wrote: > JDevlieghere wrote: > > kwk wrote: > > > labath wrote: > > > > Why is this under `if(APPLE)` ? >

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd added a comment. In D77287#1963242 , @labath wrote: > In D77287#1960390 , @compnerd wrote: > > > I think that the basic test is sufficient for this. > > > That test does not seem to be exercising the

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd added a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77662: [lldb/test] Make TestLoadUnload compatible with windows

compnerd added a comment. @labath - can this get merged so that I can rebase and get D77287 merged? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77662/new/ https://reviews.llvm.org/D77662

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd updated this revision to Diff 255580. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 Files: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp lldb/source/Plugins/Platform/Windows/PlatformWindows.h Index:

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd updated this revision to Diff 255581. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 Files: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp lldb/source/Plugins/Platform/Windows/PlatformWindows.h

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd updated this revision to Diff 254631. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 Files: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp lldb/source/Plugins/Platform/Windows/PlatformWindows.h Index:

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd updated this revision to Diff 254883. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 Files: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp lldb/source/Plugins/Platform/Windows/PlatformWindows.h

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd added a comment. Okay, thanks to some help from @JDevlieghere I was able to get a test going for this. I think that the basic test is sufficient for this. I think that the path sanitizing and conversion should be a subsequent change. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

compnerd marked an inline comment as done. compnerd added inline comments. Comment at: lldb/test/Shell/Process/Windows/process_load.cpp:3 + +// REQUIRES: system-windows +// RUN: %build --compiler=clang-cl -o %t.exe -- %s JDevlieghere wrote: > We should probably

  1   2   >