[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 fu

[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: tools/lldb-server/CMak

[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@lists.llvm.or

[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 Index

[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` we

[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 c

[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: source/API/SystemInitializerF

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

compnerd added a comment. @labath - but that is the reality of this: there *are* two classes of dependencies. I think that if we try to split up all the modules into linked libraries and header-only libraries, we are going to make an even bigger mess than what we have. Repository: rLLDB LL

[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@lists.llvm.or

[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` d

[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 source/Plugins/Ex

[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@lists.llvm.or

[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% s

[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@lists.llvm.or

[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 w

[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 forked

[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 b

[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, h

[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: tools/lldb-server/Syst

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

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

[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 isn

[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 use

[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@lists.llvm.or

[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] 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` Comm

[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 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 `clang::RecordDecl

[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 &type) { + if (clang::RecordDecl *record_decl = + ClangASTContext::GetAsRecordDecl(type)) { I think that us

[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; + switch(arch_type

[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 == llvm::Triple

[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 `clang_

[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: source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.

[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] 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] D63052: [Target] Remove Process::GetObjCLanguageRuntime

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

[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 needs

[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. Comm

[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: > JDevli

[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 ___ l

[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 wh

[Lldb-commits] [PATCH] D95185: lldb: repair the standalone build for Windows

compnerd created this revision. compnerd added a reviewer: JDevlieghere. compnerd added a project: LLDB. Herald added a subscriber: mgorny. compnerd requested review of this revision. Herald added a subscriber: lldb-commits. The previous code path only happened to work incidentally. The `file(MAK

[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] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

compnerd added a comment. WeakODR requires that the symbol actually be discarded if not referenced. This will preserve the symbol even if unreferenced will it not? One approach might be to just create a `DenseMap` and check for any references and mark is as preserved otherwise just drop it.

[Lldb-commits] [PATCH] D81501: [lldb/CMake] Make it possible to build against Python 2 with CMake > 3.12

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Would be nice to remove this entirely in favour of CMake's builtin support for Python Interpeter and Libraries. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.

[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: lld

[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` as

[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, what

[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 https

[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] 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] 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") add_subdirectory(Wi

[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 to

<    1   2