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

2018-09-21 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 http://list

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

2018-05-10 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 cross-c

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

2018-05-11 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2018-05-21 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2018-05-21 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2018-05-22 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D46726: build: use cmake to find the libedit content

2018-06-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 (NOT

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

2018-06-07 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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-commit

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

2018-06-12 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D48993: [CMake] Give Python module install command a component name

2018-07-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2018-09-18 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 accide

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

2018-09-20 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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). Re

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

2017-09-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

2019-01-10 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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) + fi

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

2019-01-15 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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/ https:/

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

2019-01-25 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 + "${LLVM_NATIVE_BUILD}/Release/bin/llvm-tblgen${HOST_EXECUTABLE_SU

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

2019-01-29 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 pr

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

2019-02-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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), yo

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

2019-02-12 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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: r

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

2019-02-13 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 https://reviews.llvm.org/

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

2019-02-13 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D58193: Do not explicitly depend on llvm tools during standalone build

2019-02-14 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D58398: Add Facebook Minidump directory streams and options to dump them.

2019-02-19 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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/ https://r

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

2019-02-19 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

2019-03-22 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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 n

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

2019-03-25 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

2023-08-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D159142: [lldb] Add support for recognizing swift ast sections in object files

2023-08-29 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D77287: Windows: support `DoLoadImage`

2021-12-04 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2021-12-13 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

2022-12-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D132608: [CMake] Clean up CMake binary dir handling

2022-12-07 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D133890: [CMake] Do these replacements to make use of D132608

2022-12-07 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

2022-12-08 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2022-12-09 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D146536: SymbolFile: ensure that we have a value before invoking `getBitWidth`

2023-03-21 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D146536: SymbolFile: ensure that we have a value before invoking `getBitWidth`

2023-03-22 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2023-03-22 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2023-03-22 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D147669: PECOFF: enforce move semantics and consume errors properly

2023-04-05 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D147669: PECOFF: enforce move semantics and consume errors properly

2023-04-05 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-04-05 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-04-07 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-04-07 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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: consume errors properly

2023-04-07 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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: consume errors properly

2023-04-14 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-04-14 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D146297: [lldb][gnustep][PDB] Parse ObjC base classes and recognize NSObject type

2023-04-25 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D149397: Host: generalise `GetXcodeSDKPath`

2023-04-27 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D149397: Host: generalise `GetXcodeSDKPath`

2023-04-28 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D149987: ObjectFile: introduce a COFF object file plugin

2023-05-05 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D149987: ObjectFile: introduce a COFF object file plugin

2023-05-05 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-05-05 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-05-05 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-05-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-05-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2023-05-07 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D100810: Use `GNUInstallDirs` to support custom installation dirs. -- LLVM

2021-11-02 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D77287: Windows: support `DoLoadImage`

2021-11-28 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D77287: Windows: support `DoLoadImage`

2021-11-28 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2021-11-28 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2021-11-29 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D104176: [libunwind] Define and use portable macro for checking for presence of ASAN

2021-06-13 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D104176: [libunwind] Define and use portable macro for checking for presence of ASAN

2021-06-13 Thread Saleem Abdulrasool 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 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] D100810: Use `GNUInstallDirs` to support custom installation dirs. -- LLVM

2021-08-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D77287: Windows: add very basic support for `DoLoadImage`

2020-04-01 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D77287: Windows: add very basic support for `DoLoadImage`

2020-04-02 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-02 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-03 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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.

2021-04-01 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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.

2021-04-01 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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.

2021-04-29 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

2020-07-27 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D88181: Utility: ignore OS version on non-Darwin targets in `ArchSpec`

2020-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D88181: Utility: ignore OS version on non-Darwin targets in `ArchSpec`

2020-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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`

2020-09-23 Thread Saleem Abdulrasool 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 rG92d42b32a9b7: Utility: ignore OS version on non-Darwin targets in `ArchSpec` (authored by compnerd). Repository: rG LLVM Github Monorepo CHANGES

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

2019-10-08 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D69535: build: improve python check for Windows

2019-10-28 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D69535: build: improve python check for Windows

2019-10-28 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2019-10-29 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D65409: [ProcessWindows] Choose a register context file by prepocessor

2019-07-29 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2019-08-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D65798: [lldb][CMake] Infer `Clang_DIR` if not passed explicitly

2019-08-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D66448: Include "windows" Instead of "Windows"

2019-08-19 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D66445: Explicitly Cast Constants to DWORD

2019-08-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D66858: POSIX DYLD: add workaround for android L loader

2019-08-27 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing

2019-09-20 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D67912: [LLDB] [PECOFF] Recognize arm64 executables

2019-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2019-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D67913: [LLDB] [Windows] Map COFF ARM machine ids to the right triplet architectures

2019-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

2019-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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] D67954: [LLDB] [Windows] Initial support for ARM64 debugging

2019-09-24 Thread Saleem Abdulrasool via Phabricator via lldb-commits
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

  1   2   >