[Lldb-commits] [PATCH] D100212: [lldb] Delete dead StackFrameList::Merge

vsk accepted this revision as: vsk. vsk added a comment. This revision is now accepted and ready to land. +1 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100212/new/ https://reviews.llvm.org/D100212 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

vsk created this revision. vsk added reviewers: JDevlieghere, jingham, jasonmolenda. Herald added subscribers: omjavaid, kristof.beyls. vsk requested review of this revision. Herald added a project: LLDB. Upstream lldb support for summarizing BLRAx and LDRAx auth failures. rdar://41615322 Repos

[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

vsk updated this revision to Diff 345513. vsk added a comment. Address code review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102428/new/ https://reviews.llvm.org/D102428 Files: lldb/include/lldb/Core/Address.h lldb/include/lldb/C

[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

vsk added a comment. Thanks for the review! Comment at: lldb/packages/Python/lldbsuite/test/functionalities/ptrauth_diagnostics/BLRAA_error/blraa.c:19 + +// Before: +#if 0 DavidSpickett wrote: > What is the purpose of the `// Before:` blocks here? Simply to gi

[Lldb-commits] [PATCH] D102624: [lldb] Optimize expressions

vsk added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:409 + // static MyClass *__lldb_expr_result_ptr = &c; // Points to stack. + // __lldb_expr_result_ptr(__lldb_expr_result_ptr); + // } // End of expressio

[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

vsk updated this revision to Diff 345940. vsk added a comment. Address review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102428/new/ https://reviews.llvm.org/D102428 Files: lldb/include/lldb/Core/Address.h lldb/include/lldb/Core/D

[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

vsk added inline comments. Comment at: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp:126 +Address brk_address; +if (!target.ResolveLoadAddress(fixed_bad_address, brk_address)) + return false; DavidSpickett wrote: > vsk wrote: > > Davi

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

vsk added a comment. @phosek I suspect this is causing a cmake error on the lldb standalone bot, would you mind taking a look? http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake-standalone/1858/ CMake Error at /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/clang-build/lib/cma

[Lldb-commits] [PATCH] D85243: [WIP] Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

vsk added inline comments. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp:313 +#if defined(__APPLE__) + // Only accept "unknown" for the vendor if the host is Apple and it + // "unknown" wasn't specified (it was just returned because it was N

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

vsk added a comment. I gave this another look-over, and while I didn't spot anything troubling, I'm not terribly familiar with this code. I'll give this a third pass later today if review is still needed. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85243/new/ https://reviews.llvm.org/D85243 ___ lldb-commits mailing list lldb-co

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

vsk added a comment. This looks very cool, thanks @clayborg! I think using JSON to describe the trace data (what kind of trace is this, what's in it, etc.) sounds reasonable. > For "trace load", I get the plugin for the JSON file by matching it up with > the "name" field in the JSON, but I don'

[Lldb-commits] [PATCH] D85770: Build a flat LLDB.framework for embedded Darwin targets

vsk created this revision. vsk added reviewers: JDevlieghere, davide, friss. Herald added a subscriber: mgorny. Herald added a project: LLDB. vsk requested review of this revision. This patch configures LLDB.framework to build as a flat unversioned framework on non-macOS Darwin targets, which have

[Lldb-commits] [PATCH] D85770: Build a flat LLDB.framework for embedded Darwin targets

vsk added inline comments. Comment at: lldb/cmake/modules/LLDBFramework.cmake:6-37 +if(NOT APPLE_EMBEDDED) + set_target_properties(liblldb PROPERTIES +FRAMEWORK ON +FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION} - OUTPUT_NAME LLDB - VERSION ${LLDB_VERSION} - LIBRARY_OU

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

vsk added a comment. In D85705#2211607 , @clayborg wrote: > In D85705#2211073 , @vsk wrote: > >> This looks very cool, thanks @clayborg! I think using JSON to describe the >> trace data (what kind of trace is this,

[Lldb-commits] [PATCH] D85770: Build a flat LLDB.framework for embedded Darwin targets

This revision was automatically updated to reflect the committed changes. Closed by commit rGd49aedd315e3: Build a flat LLDB.framework for embedded Darwin targets (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D85770?vs=284849&id=285174#toc Repository: rG LLVM Github Mo

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

vsk added a comment. It seems like calling any 'mmap' definition should work. Is the interposed mmap implementation failing and correctly returning -1, or is it succeeding and incorrectly returning -1? In either case, it seems like it's worth digging into why it's failing / returning the wrong

[Lldb-commits] [PATCH] D89215: [lldb] Add a page to the documentation with (external) links on how to use LLDB

vsk added a comment. + 1, this looks good to me. I think there might be room to include a short (~1 sentence) summary next to each extension entry, so readers know what to expect before clicking on one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89215/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D89408: [trace] rename ThreadIntelPT to ThreadTrace

vsk added inline comments. Comment at: lldb/include/lldb/Target/ThreadTrace.h:34 + /// this thread was being executed. + ThreadTrace(Process &process, lldb::tid_t tid, const FileSpec trace_file) + : Thread(process, tid), m_trace_file(trace_file) {} `co

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

vsk added a comment. I'm not sure if the current revision of the patch reflects the long-term testing strategy. But if so, I'm quite concerned about the proliferation of large binary files in the repo (like ld.so, or the raw trace binary itself). These are opaque blobs that are hard to understa

[Lldb-commits] [PATCH] D89408: [trace] rename ThreadIntelPT to ThreadTrace

vsk added inline comments. Comment at: lldb/include/lldb/Target/ThreadTrace.h:34 + /// this thread was being executed. + ThreadTrace(Process &process, lldb::tid_t tid, const FileSpec trace_file) + : Thread(process, tid), m_trace_file(trace_file) {} wal

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

vsk added a comment. In D89283#2336120 , @wallace wrote: > @vsk, I agree with you regarding the files. At the moment our implementation > of intel-pt tracing doesn't support collecting a trace, but soon we'll do so. > Then, we'll be able to generate thes

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

vsk added a subscriber: labath. vsk added a comment. Great find. The changes in this patch makes sense to me locally, but I'm having trouble picking up on the context necessary to confidently 'lgtm'. + @JDevlieghere & @labath to get some more experienced people. I'd love to see the big switch i

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Sweet! Does this 'automatically' fix the 'llvm-argdumper has issues escaping JSON-ified input' issue we discussed in person? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

vsk created this revision. vsk added reviewers: labath, shafik, teemperor. Herald added a subscriber: aprantl. Herald added a reviewer: jdoerfert. Split out the logic to parse structure-like types into a separate function, in an attempt to reduce the complexity of ParseTypeFromDWARF. Note: Phab s

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

vsk added a comment. Thank you. I've just realized that Phab's "Raw Diff" mode (https://reviews.llvm.org/F10156020) renders the diff in a much clearer way. I suspect reviewing the raw diff will be much easier than reviewing what Phab has rendered here. CHANGES SINCE LAST ACTION https://revi

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

vsk added a comment. In D68422#1694147 , @labath wrote: > Looks good to me. Thanks for doing this. Personally, I'd just remove the > `Log` argument from the function argument list, and let the function re-fetch > it if needed. I know we sometimes pass ar

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

vsk updated this revision to Diff 223357. vsk added a comment. - Remove the `log` parameter from `ParseTypeFromDWARF`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68422/new/ https://reviews.llvm.org/D68422 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h lldb/source/P

[Lldb-commits] [PATCH] D68661: StopInfo/Mach: Delete PPC support

vsk created this revision. vsk added reviewers: jingham, jasonmolenda, JDevlieghere. LLDB appears to have at least partial support for PPC, but PPC on Mach isn't a thing AFAIK. https://reviews.llvm.org/D68661 Files: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp Index: lldb/s

[Lldb-commits] [PATCH] D68661: StopInfo/Mach: Delete PPC support

This revision was automatically updated to reflect the committed changes. vsk marked an inline comment as done. Closed by commit rG4805c817c3fa: StopInfo/Mach: Delete PPC support (authored by vsk). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D68661?vs=223924&

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk created this revision. vsk added reviewers: jfb, nickdesaulniers, friss, JDevlieghere. Herald added subscribers: dexonsmith, hiraditya, mgorny. Herald added a project: LLVM. Occasionally, during test teardown, there is a write to a closed pipe in of LLDB's handful of IPC channels. Sometimes th

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk planned changes to this revision. vsk marked an inline comment as done. vsk added inline comments. Comment at: lldb/tools/driver/Driver.cpp:864 + // in LLDB leaves behind temporary objects). + llvm::sys::SetExitOnFailedPipeWrite(/*ExitOnFailedWrite=*/false); + -

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk updated this revision to Diff 225546. vsk edited the summary of this revision. vsk added a comment. - Allow setting a SIGPIPE handler. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69148/new/ https://reviews.llvm.org/D69148 Files: lldb/tools/driver/Driver.cpp llvm/include/llvm/

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk marked an inline comment as done. vsk added inline comments. Comment at: llvm/lib/Support/Unix/Signals.inc:372 if (Sig == SIGPIPE) -exit(EX_IOERR); +if (SignalHandlerFunctionType CurrentPipeFunction = PipeSignalFunction) + CurrentPipeFunction()

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk planned changes to this revision. vsk added a comment. The death tests are flaky. I've noticed two issues: 1. When run under lit, the DisableExitOnSIGPIPE doesn't actually exit when it receives SIGPIPE. Dtrace suggests that the unit test process inherits an "ignore" sigaction from its paren

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk updated this revision to Diff 225661. vsk added a comment. This revision is now accepted and ready to land. Replace the death tests with a non-death test. I plan to commit this later in the day if there aren't any objections. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69148/new/

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

This revision was automatically updated to reflect the committed changes. Closed by commit rG32ce14e55e5a: Disable exit-on-SIGPIPE in lldb (authored by vsk). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69148/new/ http

[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

vsk created this revision. vsk added reviewers: jasonmolenda, ab. DisassemblerLLVMC exposes a few MCInst predicates (e.g. `HasDelaySlot`). Group the logic for evaluating the existing predicates together, to prep for adding new ones. This is NFC-ish: with this change, the existing predicates will

[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

vsk added a comment. Hm, this patch is bugging me. It looks a bit like instructions are still decoded multiple times in different ways (e.g. in the `Decode` and `CalculateMnemonicOperandsAndComment` methods, which both modify `m_opcode`). Any ideas on whether/how to consolidate these? CHANGES

[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

vsk marked an inline comment as done. vsk added a comment. In D69210#1716861 , @clayborg wrote: > In D69210#1715679 , @vsk wrote: > > > Hm, this patch is bugging me. > > > > It looks a bit like instructions are stil

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk marked an inline comment as done. vsk added a comment. In D69148#1715532 , @labath wrote: > In D69148#1714785 , @vsk wrote: > > > The death tests are flaky. I've noticed two issues: > > > > 1. When run under lit

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk marked an inline comment as done. vsk added a comment. I admit I hadn't paid enough attention to the order in which signal handlers were registered in lldb. Comment at: lldb/tools/driver/Driver.cpp:850 #if !defined(_MSC_VER) signal(SIGPIPE, SIG_IGN); signal(SIGWINCH

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

vsk added a comment. Reverted in d0bd3fc88be . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69148/new/ https://reviews.llvm.org/D69148 ___

[Lldb-commits] [PATCH] D69403: [Driver] Force llvm to install its handlers before lldb's

vsk created this revision. vsk added reviewers: labath, JDevlieghere. Herald added a reviewer: jfb. Install llvm's signal handlers up front to prevent lldb's handlers from being ignored. This is (hopefully) a stopgap workaround. When lldb invokes an llvm API that installs signal handlers (e.g. ll

[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

vsk added a comment. I don't think it should be necessary to read the class in its entirety to understand when `m_opcode` is safe to use. However, as I'm not sure how the disassembler is called in to, I don't think it's a good idea to refactor the whole thing right away. Let's start with this

[Lldb-commits] [PATCH] D69403: [Driver] Force llvm to install its handlers before lldb's

This revision was automatically updated to reflect the committed changes. Closed by commit rG0877dd14e4e8: [Driver] Force llvm to install its handlers before lldb's (authored by vsk). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [PATCH] D69523: [debugserver] Detect arch from LLVM_DEFAULT_TARGET_TRIPLE

vsk created this revision. vsk added reviewers: davide, JDevlieghere, xiaobai. Herald added a subscriber: mgorny. The debugserver build needs to conditionally include files depending on the target architecture. Switch on the architecture specified by LLVM_DEFAULT_TARGET_TRIPLE, as the llvm and sw

[Lldb-commits] [PATCH] D69524: [debugserver] Delete macOS/PPC debug server implementation

vsk created this revision. vsk added reviewers: clayborg, jasonmolenda. Herald added subscribers: kbarton, mgorny, nemanjai. macOS/PPC support was dropped in 10.6 (Snow Leopard). https://reviews.llvm.org/D69524 Files: lldb/tools/debugserver/source/MacOSX/CMakeLists.txt lldb/tools/debugserve

[Lldb-commits] [PATCH] D69523: [debugserver] Detect arch from LLVM_DEFAULT_TARGET_TRIPLE

vsk closed this revision. vsk added a comment. Landed in 82d3ba. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69523/new/ https://reviews.llvm.org/D69523 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[Lldb-commits] [PATCH] D70100: [DWARF] Handle call sites with indirect call targets

vsk created this revision. vsk added reviewers: aprantl, friss, jingham. Split CallEdge into DirectCallEdge and IndirectCallEdge. Teach DWARFExpression how to evaluate entry values in cases where the current activation was created by an indirect call. Writing tests for this is challenging, at the

[Lldb-commits] [PATCH] D70100: [DWARF] Handle call sites with indirect call targets

vsk updated this revision to Diff 228920. vsk marked 3 inline comments as done. vsk added a comment. - Add doxygen comments describing {Direct,Indirect}CallEdge & address the rest of Adrian's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70100/new/ https://reviews.llvm.org/D7

[Lldb-commits] [PATCH] D70100: [DWARF] Handle call sites with indirect call targets

vsk updated this revision to Diff 228921. vsk added a comment. - Remove a few more redundant Optional initializations. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70100/new/ https://reviews.llvm.org/D70100 Files: lldb/include/lldb/Symbol/Function.h lldb/include/lldb/Symbol/Symbol

[Lldb-commits] [PATCH] D70216: [LLDB] Don't install the pretty backtrace handlers twice.

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Looks good to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70216/new/ https://reviews.llvm.org/D70216 ___ lldb-commits mailing list lldb-

[Lldb-commits] [PATCH] D70277: [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling

vsk created this revision. vsk added reviewers: JDevlieghere, jfb, labath. Herald added subscribers: dexonsmith, hiraditya. Herald added a project: LLVM. Allow clients of the llvm library to opt-in to one-shot SIGPIPE handling, instead of exiting with an IO error by default. This prevents "IO err

[Lldb-commits] [PATCH] D70277: [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling

vsk updated this revision to Diff 229601. vsk edited the summary of this revision. vsk added a comment. - Preserve the current behavior w.r.t SIGPIPE for all llvm-derived tools except lldb. - Document the trickiness re: registering a SIGPIPE handler before llvm sets up its signal handlers. CHA

[Lldb-commits] [PATCH] D70277: [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling

This revision was automatically updated to reflect the committed changes. Closed by commit rG4624e83ce7b1: [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling (authored by vsk). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D70409: [Docs] Add reproducer documentation.

vsk added a comment. Herald added a subscriber: dexonsmith. I like it! Comment at: lldb/docs/resources/reproducers.rst:9 + +Reproducers in LLDB are an attempt to improve the experience for both the users +encountering these bugs and the developers working on resolving them. The

[Lldb-commits] [PATCH] D65594: Fix `skipIfSanitized` decorator on macOS

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65594/new/ https://reviews.llvm.org/D65594 ___ lldb-comm

[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error

vsk abandoned this revision. vsk added a subscriber: shafik. vsk added a comment. I don't think this got a lot of buy-in. Let's revisit this if the need to revamp CompilerType becomes more pressing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43912/new/ https://reviews.llvm.org/D4391

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk created this revision. vsk added reviewers: aprantl, friss, jasonmolenda, jingham. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. Add support for evaluating DW_OP_entry_value. This involves: - Teaching clang to emit debug entry values when the debugger tuning

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk updated this revision to Diff 219442. vsk added a comment. - Clean up the test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67376/new/ https://reviews.llvm.org/D67376 Files: lldb/include/lldb/Symbol/Function.h lldb/packages/Python/lldbsuite/test/decorators.py lldb/packages

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks a lot! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67378/new/ https://reviews.llvm.org/D67378 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

vsk added a comment. Looks good overall, especially the testing. Comment at: lldb/include/lldb/Utility/Scalar.h:107 + /// Return the most efficient Scalar::Type for the requested size. + static Type GetBestType(size_t bit_size, bool sign); + JDevlieghere wrot

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk updated this revision to Diff 219462. vsk marked 7 inline comments as done. vsk added a comment. - Partially address review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67376/new/ https://reviews.llvm.org/D67376 Files: lldb/include/lldb/Symbol/Function.h lldb/include

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk planned changes to this revision. vsk added a comment. TODO: - Split out llvm change. - Add a test to validate that lldb skips inline frames when evaluating DW_OP_entry_value. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_va

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk updated this revision to Diff 219573. vsk marked an inline comment as done. vsk edited the summary of this revision. vsk added a comment. - Addressed review feedback, split out unrelated changes, and improved test coverage. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67376/new/ h

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk marked an inline comment as done. vsk added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3752 +LocationInCaller = parse_simple_location(i); +break; + } aprantl wrote: > vsk wrote: > > aprantl wrote

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk planned changes to this revision. vsk marked an inline comment as done. vsk added a comment. While tightening up the test case I think I found an issue with the way inlined frames are handled. I need to take a closer look. Comment at: lldb/include/lldb/Symbol/Function.h:25

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk updated this revision to Diff 219646. vsk marked 4 inline comments as done. vsk added a comment. - Fix return address lookup when the immediate parent frame is inlined. - Tighten the test so that it actually verifies that tail calls, inlining, etc. occur, instead of assuming :). - Add/move va

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:258 + +using CallSiteParameterArray = std::unique_ptr>; + aprantl wrote: > vsk wrote: > > grandinj wrote: > > > the way this is being used seems to indicate it can be > > >std::vector

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:258 + +using CallSiteParameterArray = std::unique_ptr>; + vsk wrote: > aprantl wrote: > > vsk wrote: > > > grandinj wrote: > > > > the way this is being used seems to indicate it can be >

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk updated this revision to Diff 219648. vsk marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67376/new/ https://reviews.llvm.org/D67376 Files: lldb/include/lldb/Symbol/Function.h lldb/packages/Python/lldbsuite/test/decorators.py lldb/packages/Pytho

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:258 + +using CallSiteParameterArray = std::unique_ptr>; + aprantl wrote: > vsk wrote: > > vsk wrote: > > > aprantl wrote: > > > > vsk wrote: > > > > > grandinj wrote: > > > > > > the way t

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk updated this revision to Diff 219779. vsk marked 3 inline comments as done. vsk added a comment. - Add a working example and some additional comments in Evaluate_DW_OP_entry_value. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67376/new/ https://reviews.llvm.org/D67376 Files: ll

[Lldb-commits] [PATCH] D67774: [Mangle] Check ExternalASTSource before adding prefix to asm label names

vsk created this revision. vsk added reviewers: teemperor, jasonmolenda, rjmccall. LLDB may synthesize decls using asm labels. These decls cannot have a mangle different than the one specified in the label name. I.e., the mangle-suppression prefix should not be added. Fixes an expression evaluati

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

vsk updated this revision to Diff 221066. vsk retitled this revision from "[Mangle] Check ExternalASTSource before adding prefix to asm label names" to "[Mangle] Add flag to asm labels to disable global prefixing". vsk edited the summary of this revision. vsk added a comment. Thanks for your fee

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

vsk updated this revision to Diff 221391. vsk marked 3 inline comments as done. vsk added a comment. - Address latest review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67774/new/ https://reviews.llvm.org/D67774 Files: clang/include/clang/Basic/Attr.td clang/include/cla

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

vsk added inline comments. Comment at: clang/lib/AST/Mangle.cpp:130 + return; +} + rjmccall wrote: > This is actually backwards, right? A literal label is one that doesn't get > the global prefix and therefore potentially needs the `\01` prefix to > s

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable '\01' prefixing

vsk updated this revision to Diff 221645. vsk retitled this revision from "[Mangle] Add flag to asm labels to disable global prefixing" to "[Mangle] Add flag to asm labels to disable '\01' prefixing". vsk edited the summary of this revision. vsk added a comment. - Address latest round of comment

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable '\01' prefixing

vsk updated this revision to Diff 221801. vsk added a comment. - Add a comment describing where non-literal labels are used. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67774/new/ https://reviews.llvm.org/D67774 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/A

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

vsk added a comment. @wallace thanks for winnowing the test objects. I left an inline suggestion about simplifying the error-handling in IntelPTInstruction. Other than that, mechanically this is looking good. Thanks! Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThrea

[Lldb-commits] [PATCH] D90276: [lldb/utils] Add the lldb-env tool

vsk created this revision. vsk added reviewers: JDevlieghere, teemperor, mgorny. Herald added a project: LLDB. vsk requested review of this revision. Introduce lldb-env, a tool that prints out the environment variables needed to launch lldb. Usually, no environment variables need to be set to lau

[Lldb-commits] [PATCH] D90276: [lldb/utils] Add the lldb-env tool

vsk added a comment. In D90276#2357978 , @JDevlieghere wrote: > Can you explain the added value of the tool a bit more. I can see how it's > nice to be able to run `export $(lldb-env); path/to/lldb` but I wonder how > often you'd end up doing that. When

[Lldb-commits] [PATCH] D90276: [lldb/utils] Add the lldb-env tool

vsk added a comment. >> Re: making the lit.site.cfg self-contained, IIUC the situation is that there >> are a couple places where we shell out via `subprocess`, but that you'd like >> to get rid of these? I wonder whether we could bundle up the necessary >> scripts along with lit.site.cfg inste

[Lldb-commits] [PATCH] D90276: [lldb/utils] Add the lldb-env tool

vsk abandoned this revision. vsk added a comment. Jonas walked me through some of the issues here offline: - Looks like this makes testing depend on having a configured lldb-env tool, this will make it hard to run testing against an lldb root. - If we need to launch ./bin/lldb in some other work

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

vsk added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:26 +public: + IntelPTError() = default; + Do we need a default constructor for this pure-virtual class? Comment at: lldb/source/Plugins/Trace/intel-pt/De

[Lldb-commits] [PATCH] D90706: [crashlog] Pass the debugger around instead of relying on lldb.debugger

vsk added inline comments. Comment at: lldb/examples/python/crashlog.py:853 +# can rely on lldb.debugger being set. +SymbolicateCrashLogs(lldb.debugger, shlex.split(command)) except Exception as e: Why does crashlog.Symbolicate accept a debug

[Lldb-commits] [PATCH] D90706: [crashlog] Pass the debugger around instead of relying on lldb.debugger

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90706/new/ https://reviews.llvm.org/D90706 ___ lldb-commits mailing list lldb-co

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

vsk created this revision. vsk added reviewers: teemperor, JDevlieghere, jingham. Herald added a project: LLDB. vsk requested review of this revision. Factor out dummy target creation from CreateTargetInternal. This makes it impossible for dummy target creation to accidentally fail due to too-str

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

vsk updated this revision to Diff 303223. vsk added a comment. Delete some dead code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90872/new/ https://reviews.llvm.org/D90872 Files: lldb/include/lldb/Target/TargetList.h lldb/source/Target/Targ

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

vsk marked an inline comment as done. vsk added inline comments. Comment at: lldb/source/Target/TargetList.cpp:293 // FIXME: Maybe the dummy target should be per-Debugger - if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) { + if (!m_dummy_target_sp) { ArchSpec a

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

vsk updated this revision to Diff 303252. vsk marked an inline comment as done. vsk added a comment. Make the dummy target per-debugger. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90872/new/ https://reviews.llvm.org/D90872 Files: lldb/include

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

vsk added inline comments. Comment at: lldb/source/Target/TargetList.cpp:293 // FIXME: Maybe the dummy target should be per-Debugger - if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) { + if (!m_dummy_target_sp) { ArchSpec arch(Target::GetDefaultArchitecture());

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

vsk added inline comments. Comment at: lldb/source/Core/Debugger.cpp:685 - m_dummy_target_sp = m_target_list.GetDummyTarget(*this); + { +ArchSpec arch(Target::GetDefaultArchitecture()); teemperor wrote: > Maybe have a small comment here that summarizes th

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG16e5a347e70b: [TargetList] Simplify dummy target creation (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D90872?vs=303252&id

[Lldb-commits] [PATCH] D90895: [TargetList] Delete the destructor

vsk created this revision. vsk added reviewers: teemperor, JDevlieghere, jingham. Herald added a project: LLDB. vsk requested review of this revision. AFAICT, ~TargetList simply implements the default destructor, plus some locking. The history is murky, so I'm not sure why we do this locking. Per

[Lldb-commits] [PATCH] D90895: [TargetList] Delete the destructor

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG65d15fefe339: [TargetList] Delete the destructor (authored by vsk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[Lldb-commits] [PATCH] D91220: [ThreadPlan] Add a test for `thread step-in -r`, NFC

vsk created this revision. vsk added a reviewer: jingham. Herald added a project: LLDB. vsk requested review of this revision. Herald added a subscriber: JDevlieghere. Adds test coverage for ThreadPlanStepInRange::SetAvoidRegexp. See: http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Us

[Lldb-commits] [PATCH] D91220: [ThreadPlan] Add a test for `thread step-in -r`, NFC

vsk updated this revision to Diff 304355. vsk added a comment. Simplify, using run_to_source_breakpoint. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91220/new/ https://reviews.llvm.org/D91220 Files: lldb/packages/Python/lldbsuite/test/lldbtest

[Lldb-commits] [PATCH] D91220: [ThreadPlan] Add a test for `thread step-in -r`, NFC

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGae3640e386cc: [ThreadPlan] Add a test for `thread step-in -r`, NFC

<    1   2   3   4   5   >