[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. LGTM with @kastiglione's comments addressed. Comment at: lldb/source/Core/Section.cpp:153 + case eSectionTypeSwiftModules: +return "swift-modules"; }

[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. LGTM outside of the PE header check. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:1035 + // llvm::object::COFFObjectFile::getSectionSize(). +

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

This revision was automatically updated to reflect the committed changes. Closed by commit rG5014830ede78: ObjectFile: introduce a COFF object file plugin (authored by compnerd). Changed prior to commit: https://reviews.llvm.org/D149987?vs=520102&id=520207#toc Repository: rG LLVM Github Mono

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

compnerd updated this revision to Diff 520102. compnerd marked an inline comment as done. compnerd added a comment. Address feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149987/new/ https://reviews.llvm.org/D149987 Files: lldb/source/Plugins/ObjectFile/CMakeLists.txt lldb/

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

compnerd marked 4 inline comments as done. compnerd added inline comments. Comment at: lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp:203 +.Case(".debug_pubnames", eSectionTypeDWARFDebugPubNames) +.Case(".debug_pubtypes", eSectionTypeDWARFDebugPubT

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

compnerd updated this revision to Diff 519984. compnerd added a comment. Address feedback, add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149987/new/ https://reviews.llvm.org/D149987 Files: lldb/source/Plugins/ObjectFile/CMakeLists.txt

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

compnerd added inline comments. Comment at: lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp:103-104 + + return new ObjectFileCOFF(module_sp, data_sp, data_offset, file, file_offset, +length); +} bulbazord wrote: > compnerd wrot

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

compnerd added inline comments. Comment at: lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp:103-104 + + return new ObjectFileCOFF(module_sp, data_sp, data_offset, file, file_offset, +length); +} bulbazord wrote: > Reading the i

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

compnerd created this revision. compnerd added reviewers: aprantl, kastiglione. compnerd added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. compnerd requested review of this revision. Windows uses COFF as an object file format and PE/COFF as an executable

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

This revision was automatically updated to reflect the committed changes. Closed by commit rGade3c6a6a88e: Host: generalise `GetXcodeSDKPath` (authored by compnerd). Changed prior to commit: https://reviews.llvm.org/D149397?vs=517764&id=517959#toc Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

compnerd created this revision. compnerd added reviewers: JDevlieghere, kastiglione, aprantl. compnerd added a project: LLDB. Herald added a project: All. compnerd requested review of this revision. This generalises the `GetXcodeSDKPath` hook to a `GetSDKRoot` path which will be re-used for the Wi

[Lldb-commits] [PATCH] D146297: [lldb][gnustep][PDB] Parse ObjC base classes and recognize NSObject type

compnerd added inline comments. Comment at: lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:399 // Such symbols will be handled here. // Ignore unnamed-tag UDTs. Does it make sense to drop a note here that the 0-length UDT is used as a marker for

[Lldb-commits] [PATCH] D147669: PECOFF: consume errors properly

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

[Lldb-commits] [PATCH] D147669: PECOFF: consume errors properly

compnerd updated this revision to Diff 513614. compnerd added a comment. Address feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147669/new/ https://reviews.llvm.org/D147669 Files: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Index: lldb/source/Plugins/ObjectFi

[Lldb-commits] [PATCH] D147669: PECOFF: consume errors properly

compnerd updated this revision to Diff 511706. compnerd retitled this revision from "PECOFF: enforce move semantics and consume errors properly" to "PECOFF: consume errors properly". compnerd edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147669/new/ h

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

compnerd added a comment. In D147669#4251441 , @compnerd wrote: > In D147669#4249968 , @sgraenitz > wrote: > >> First of all, yes the existing code is incorrect. Thanks for looking into >> this. However, I am af

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

compnerd added a comment. In D147669#4249968 , @sgraenitz wrote: > First of all, yes the existing code is incorrect. Thanks for looking into > this. However, I am afraid this isn't the right solution. > You probably mean `LLVM_ENABLE_ABI_BREAKING_CHEC

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

compnerd added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873 LLDB_LOG(log, "ObjectFilePECOFF::AppendFromExportTable - failed to get export " "table entry name: {0}", llvm::fm

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

compnerd added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873 LLDB_LOG(log, "ObjectFilePECOFF::AppendFromExportTable - failed to get export " "table entry name: {0}", llvm::fm

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

compnerd added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873 LLDB_LOG(log, "ObjectFilePECOFF::AppendFromExportTable - failed to get export " "table entry name: {0}", llvm::fm

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

compnerd created this revision. compnerd added reviewers: bulbazord, sgraenitz, labath. compnerd added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. compnerd requested review of this revision. Ensure that we explicitly indicate that we would like the move s

[Lldb-commits] [PATCH] D146536: SymbolFile: ensure that we have a value before invoking `getBitWidth`

This revision was automatically updated to reflect the committed changes. Closed by commit rG16b7cf245ec0: SymbolFile: ensure that we have a value before invoking `getBitWidth` (authored by compnerd). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146

[Lldb-commits] [PATCH] D146536: SymbolFile: ensure that we have a value before invoking `getBitWidth`

compnerd updated this revision to Diff 507330. compnerd added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: jplehr, sstefan1. Address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146536/new/ https://reviews.llvm

[Lldb-commits] [PATCH] D146536: SymbolFile: ensure that we have a value before invoking `getBitWidth`

compnerd added a comment. In D146536#4210062 , @sgraenitz wrote: > Thanks for tackling this! Yes, I hit it as well during development and > wondered what is the right thing to do. The effect of the proposed change > will be log messages like this right?

[Lldb-commits] [PATCH] D146536: SymbolFile: ensure that we have a value before invoking `getBitWidth`

compnerd created this revision. compnerd added reviewers: rnk, sgraenitz. Herald added a project: All. compnerd requested review of this revision. Herald added a project: LLDB. Ensure that the variant returned by `member->getValue()` has a value and is not `Empty`. Failure to do so will trigger a

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

compnerd added inline comments. Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32 SmallString<128> LibPath = llvm::sys::path::parent_path(Dir); - llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX); + llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR); if

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

compnerd added inline comments. Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32 SmallString<128> LibPath = llvm::sys::path::parent_path(Dir); - llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX); + llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR); if

[Lldb-commits] [PATCH] D133890: [CMake] Do these replacements to make use of D132608

compnerd added a comment. I don't see anything wrong with this change per se, but I'm conflicted on the name of the variable. These are not standard variables but are encroaching on the CMake namespace. What happens if upstream decides to use these names? I think that we should keep the name

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

compnerd added inline comments. Comment at: cmake/Modules/GNUBinaryDirs.cmake:3 + get_filename_component(CMAKE_LIBDIR_BASENAME "${CMAKE_INSTALL_LIBDIR}" NAME) +endif() + Should this perhaps be moved further down near the usage? Comment at: cma

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

compnerd added inline comments. Comment at: clang/include/clang/Config/config.h.cmake:57 +/* Multilib basename for libdir. */ +#define CLANG_INSTALL_LIBDIR_BASENAME "${CLANG_INSTALL_LIBDIR_BASENAME}" Does this not potentially break downstreams? Repository:

[Lldb-commits] [PATCH] D77287: Windows: support `DoLoadImage`

compnerd added a comment. Thanks @thakis - 906e60b9f923464cba0f71a9205846550752162f should have addressed that from a few days ago (sorry about not mentioning that here) Repository: rG LLVM Github Monorepo CHANGES SINCE L

[Lldb-commits] [PATCH] D77287: Windows: support `DoLoadImage`

This revision was automatically updated to reflect the committed changes. compnerd marked an inline comment as done. Closed by commit rGf1585a4b47cc: Windows: support `DoLoadImage` (authored by compnerd). Changed prior to commit: https://reviews.llvm.org/D77287?vs=390249&id=391847#toc Reposito

[Lldb-commits] [PATCH] D77287: Windows: support `DoLoadImage`

compnerd marked an inline comment as done. compnerd added inline comments. Comment at: lldb/source/Plugins/Platform/Windows/PlatformWindows.h:47-51 + uint32_t DoLoadImage(lldb_private::Process *process, + const lldb_private::FileSpec &remote_file, +

[Lldb-commits] [PATCH] D77287: Windows: support `DoLoadImage`

compnerd updated this revision to Diff 390249. compnerd added a comment. Add missing null-terminators. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 Files: lldb/source/Plugins/Platform/Windows/CMakeLists.txt lldb/source/Plugins/Platform/Wi

[Lldb-commits] [PATCH] D77287: Windows: support `DoLoadImage`

compnerd updated this revision to Diff 390246. compnerd added a comment. Herald added a subscriber: mgorny. Fix build rules Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 Files: lldb/source/Plugins/Platf

[Lldb-commits] [PATCH] D77287: Windows: support `DoLoadImage`

compnerd updated this revision to Diff 390245. compnerd retitled this revision from "Windows: add very basic support for `DoLoadImage`" to "Windows: support `DoLoadImage`". compnerd edited the summary of this revision. compnerd added a comment. This is a more complete implementation that allows f

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

compnerd accepted this revision. compnerd added inline comments. This revision is now accepted and ready to land. Comment at: llvm/CMakeLists.txt:289 +set(LLVM_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE STRING +"Path for binary subdirectory (defaults to 'bin')") mark_

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

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

[Lldb-commits] [PATCH] D104176: [libunwind] Define and use portable macro for checking for presence of ASAN

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe03be2efe564: unwind: allow building with GCC (authored by compnerd). Changed prior to commit: https://reviews.llvm.org/D104176?vs=351711&id=35174

[Lldb-commits] [PATCH] D104176: [libunwind] Define and use portable macro for checking for presence of ASAN

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. LGTM; do you need someone to commit this on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104176/new/ https://reviews.llvm.org/D104176 __

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

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. This is a pretty straightforward cleanup now, which adds additional functionality by deferring work to CMake. There are a couple of minor points about inconsistent quoting but this seems

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

compnerd added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:127 + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + RUNTIME DESTINATION ${CMAKE_INSTAL

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

compnerd added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:127 + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + RUNTIME DESTINATION ${CMAKE_INSTAL

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

compnerd added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:127 + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + RUNTIME DESTINATION ${CMAKE_INSTAL

[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] D88181: Utility: ignore OS version on non-Darwin targets in `ArchSpec`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG92d42b32a9b7: Utility: ignore OS version on non-Darwin targets in `ArchSpec` (authored by compnerd). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D88181: Utility: ignore OS version on non-Darwin targets in `ArchSpec`

compnerd added a comment. Correct, this is just upstreaming the original ArchSpec change that we found on Swift. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88181/new/ https://reviews.llvm.org/D88181

[Lldb-commits] [PATCH] D88181: Utility: ignore OS version on non-Darwin targets in `ArchSpec`

compnerd created this revision. compnerd added reviewers: JDevlieghere, kastiglione. compnerd added a project: LLDB. compnerd requested review of this revision. The OS version field is generally not very helpful for non-Darwin targets. On Linux, it identifies the kernel version which moves out-of

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Thanks, this is a great idea! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84691/new/ https://reviews.llvm.org/D84691 __

[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] 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] 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 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/test/Shell/Process/Win

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

[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] 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 "unloa

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

[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/test/Shell/Process/Win

[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 https://reviews.llvm.

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

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

compnerd added a comment. Thanks for the hint about the string conversion, however, I think that it's going to complicate things as the string is going to be a mixture of UTF-8 and UTF-16 content. As to the testing, `functionalities/load_using_paths/TestLoadUsingPaths.py` is not applicable to

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

compnerd created this revision. compnerd added reviewers: JDevlieghere, xiaobai. compnerd added a project: LLDB. Add some very basic support for `DoLoadImage` and `UnloadImage` for Windows. This was previously not implemented and would result in a failure at runtime that is hard to detect. Thi

[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

[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] 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] 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] 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] 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] 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] 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 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] 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] 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) +find_package(Pyt

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

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

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

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

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

[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] D67912: [LLDB] [PECOFF] Recognize arm64 executables

compnerd accepted this revision. compnerd added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:202 + case MachineArm64: +spec.SetTriple("aarch64-pc-windows"); +specs.Append(module_s

[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/source/Plugins/Process/Windows/Common/RegisterConte

[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] 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 ori

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

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

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

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

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

  1   2   >