[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

vsk added a comment. The general direction lgtm, I'd be happy if we could replace interactive autocomplete tests with this. https://reviews.llvm.org/D43048 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[Lldb-commits] [PATCH] D43432: [cmake] Fix LLDB_CODESIGN_IDENTITY logic.

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm! https://reviews.llvm.org/D43432 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[Lldb-commits] [PATCH] D43577: Fix TestUbsanBasic

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. I'd be fine with this version of the patch, or one which uses path.basename to be more stringent. This test is skipped on green dragon because I have not upstreamed the debugger integration for ubs

[Lldb-commits] [PATCH] D43591: [testsuite] Run lit tests as part of `check-lldb`

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D43591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-comm

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

vsk created this revision. vsk added reviewers: zturner, labath, davide, JDevlieghere. Removing the template arguments and most of the mutating methods from CleanUp makes it easier to understand and reuse. In its present state, CleanUp would be too cumbersome to adapt to cases where multiple obje

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

vsk added a comment. > Couldn't we just add a creator helper so that you can say: > > auto cleanup = makeCleanupRAII([&] { > > process_sp->Destroy(/*force_kill=*/false); > debugger.StopEventHandlerThread(); > }); @zturner In this version of the patch, CleanUp accepts a std::function,

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

vsk added a comment. In https://reviews.llvm.org/D43662#1016950, @labath wrote: > In https://reviews.llvm.org/D43662#1016939, @vsk wrote: > > > > What do you think about a syntax like: > > > > > > lldb_utility::CleanUp cleanup_our(::close, our_socket); // The CleanUp > > > constructor creates t

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

vsk updated this revision to Diff 135671. vsk added a comment. Herald added a subscriber: mgorny. Thanks for the feedback. I've added support for forwarding arguments to the cleanup function though the constructor. It uses std::bind instead of a lambda because I couldn't figure out a simpler wa

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

vsk accepted this revision. vsk added a comment. I like that we can customize the command substitutions and the way we dump internal state in this test mode. This offers some added flexibility over the batch mode, IMO. LGTM, thanks! https://reviews.llvm.org/D43686 __

[Lldb-commits] [PATCH] D43662: [Utility] Simplify and generalize the CleanUp helper, NFC

vsk added a comment. Thanks! I'll clean up the issues you pointed out before committing. https://reviews.llvm.org/D43662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

vsk added a comment. I'm concerned about the time it takes to run the test suite. If we can get decent test coverage of this patch without adding a new debug info flavor, that'd be ideal. Comment at: packages/Python/lldbsuite/test/lldbtest.py:718

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

vsk created this revision. vsk added reviewers: labath, zturner, davide, aprantl, lhames. LLDB CompilerTypes may be invalid, i.e they should be checked for validity before use. Compared to a more typical model in which only valid types exist [1], this has a few disadvantages: - The debugger is g

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

vsk added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:335-340 +if (llvm::Error E = user_type_or_err.takeError()) { + std::string Reason = llvm::toString(std::move(E)); + if (log) +log->Printf("%s", Reason.c_

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

vsk updated this revision to Diff 136433. vsk marked an inline comment as done. vsk edited the summary of this revision. vsk added a comment. - Address some review feedback. https://reviews.llvm.org/D43912 Files: include/lldb/Symbol/CompilerType.h include/lldb/Utility/Log.h source/Plugins

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

vsk added a comment. In https://reviews.llvm.org/D43912#1023078, @jingham wrote: > In https://reviews.llvm.org/D43912#1022916, @aprantl wrote: > > > > Going forward, we should transition to a model in which CompilerTypes are > > > either valid or do not exist. > > > > I don't understand very wel

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

vsk updated this revision to Diff 136450. vsk added a comment. > The problem with your RETURN_IF_UNEXPECTED macro is that it make it > impossible to put a breakpoint only on the "type was invalid case." To catch > that happening while debugging you'd have to write a conditional breakpoint > tha

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

vsk added inline comments. Comment at: include/lldb/Utility/Log.h:249-254 + ::lldb_private::Log *log_private = (log); \ + if (log_private) \ +log_private->FormatError(::std:

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

vsk updated this revision to Diff 136640. vsk retitled this revision from "[Symbol] Add InvalidType, a force-checked recoverable error" to "[Symbol] Add InvalidTypeError, a force-checked recoverable error". vsk added a comment. - While playing around with InvalidTypeError, I found that it's usef

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

vsk updated this revision to Diff 136656. vsk added a comment. - Clarify that the log_categories macro argument is for logging to the "lldb" channel. https://reviews.llvm.org/D43912 Files: include/lldb/Symbol/CompilerType.h include/lldb/Utility/Log.h source/Plugins/ExpressionParser/Clang

[Lldb-commits] [PATCH] D44342: Introduce a setting to disable Spotlight while running the test suite

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks @aprantl! https://reviews.llvm.org/D44342 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[Lldb-commits] [PATCH] D44507: [cmake] Copy system debugserver from the right place when only CommandLineTools are installed

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thank you, lgtm. https://reviews.llvm.org/D44507 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[Lldb-commits] [PATCH] D34553: Shorten sanitizer plugin names

vsk added a comment. Thanks, this lgtm. Our tools don't depend on these plugins having a particular file name. https://reviews.llvm.org/D34553 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG66902a32c838: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures (authored by vsk). Repository: rG LLVM Github Monorepo CHANGES S

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk created this revision. vsk added reviewers: jasonmolenda, jingham, JDevlieghere. Herald added subscribers: zzheng, omjavaid, pengfei, kristof.beyls. vsk requested review of this revision. Herald added a project: LLDB. A MachO userspace corefile may contain LC_THREAD commands which specify thre

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk updated this revision to Diff 372591. vsk added a comment. - Add license header to the .def file. - (I'm not sure whether/how this change can be tested, any pointers appreciated.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109795/new/ https

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk updated this revision to Diff 372809. vsk marked 3 inline comments as done. vsk added a comment. Address review feedback: - Include far register in output, which now looks like e.g.: (lldb) bt all * thread #1, stop reason = ESR_EC_DABORT_EL0 (fault address: 0x6261747563657860) * fra

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk added inline comments. Comment at: lldb/include/lldb/Target/AppleArm64ExceptionClass.h:14 + +enum class AppleArm64ExceptionClass : unsigned { +#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code) Name = Code, shafik wrote: > We should use fixed sized integer types

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk added a comment. FTR, I was able to write a test using `process save-core`, but even with `-s dirty` this is generating gigabytes of data and would presumably be super-flaky on the bots. I also tried `-s stack`, but for the test program I passed in (just some 2-line .c file that dereference

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk updated this revision to Diff 372836. vsk added a comment. - Add a test (thanks @jasonmolenda!). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109795/new/ https://reviews.llvm.org/D109795 Files: lldb/include/lldb/Target/AppleArm64ExceptionCl

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk added inline comments. Comment at: lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py:19 +@skipUnlessDarwin +@skipIf(archs=['i386','x86_64']) # exception codes not yet supported for Intel macs +def test(self): jasonmol

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk updated this revision to Diff 373014. vsk added a comment. - Limit testing to arm64/arm64e only. - Set a stop reason for crashing threads only (and not for threads waiting on a syscall to finish). - Use @jasonmolenda's multi-threaded test case; make sure we only select the crashing thread.

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

vsk updated this revision to Diff 373016. vsk added a comment. - Add missing skipIf(arch=no_match(['arm64', 'arm64e'])) change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109795/new/ https://reviews.llvm.org/D109795 Files: lldb/include/lldb/T

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7eb67748f9d7: [MachCore] Report arm64 thread exception state (authored by vsk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[Lldb-commits] [PATCH] D110013: [lldb][crashlog] Avoid specifying arch for image when a UUID is present

vsk created this revision. vsk added reviewers: jasonmolenda, jingham, JDevlieghere. Herald added subscribers: pengfei, kristof.beyls. vsk requested review of this revision. Herald added a project: LLDB. When adding an image to a target for crashlog purposes, avoid specifying the architecture of t

[Lldb-commits] [PATCH] D110013: [lldb][crashlog] Avoid specifying arch for image when a UUID is present

This revision was automatically updated to reflect the committed changes. Closed by commit rGe31b2d7d7be9: [lldb][crashlog] Avoid specifying arch for image when a UUID is present (authored by vsk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110013

[Lldb-commits] [PATCH] D110013: [lldb][crashlog] Avoid specifying arch for image when a UUID is present

vsk added a comment. In D110013#3013733 , @aprantl wrote: > Without having dug into this deeper, do you think not specifying the arch > could cause any problems with universal binaries? I guess not, because we > still have the UUID, and the UUID is per

[Lldb-commits] [PATCH] D112034: [lldb/test] Update test/API/functionalities/load_lazy to macOS 12

vsk created this revision. vsk added reviewers: JDevlieghere, kastiglione, kledzik. vsk requested review of this revision. In macOS 12, dyld switched to using chained fixups. As a result, all symbols are bound at launch and there are no lazy pointers any more. Since we wish to import/dlopen() a dy

[Lldb-commits] [PATCH] D112034: [lldb/test] Update test/API/functionalities/load_lazy to macOS 12

This revision was automatically updated to reflect the committed changes. Closed by commit rG5e004b03f72a: [lldb/test] Update test/API/functionalities/load_lazy to macOS 12 (authored by vsk). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D112325: [lldb] Pass the target triple to the compiler when determining the DWARF version

vsk added a comment. Thanks! Lgtm as well, but with the same nits as Raphael. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112325/new/ https://reviews.llvm.org/D112325 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D103020: [lldb] Add missing mutex guards to TargetList::CreateTarget

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

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

vsk added a comment. Friendly ping. 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 wrot

[Lldb-commits] [PATCH] D103500: [trace][intel-pt] Create basic SB API

vsk requested changes to this revision. vsk added a subscriber: mib. vsk added a comment. This revision now requires changes to proceed. Thanks, excited to see this work progress! Comment at: lldb/include/lldb/Target/Target.h:1129 /// The trace object. It might be undefine

[Lldb-commits] [PATCH] D103588: [trace] Create a top-level instruction class

vsk requested changes to this revision. vsk added a comment. This revision now requires changes to proceed. I'm quite concerned about the design decision to represent a trace as a vector of TraceInstruction. This bakes considerations that appear to be specific to Facebook's usage of IntelPT way

[Lldb-commits] [PATCH] D103588: [trace] Create a top-level instruction class

vsk added a comment. In D103588#2806512 , @wallace wrote: > Right now we haven't implemented lazy decoding; we are decoding the entire > trace. But that's just because so far we have been working with small traces. > As we progress in this work, we'll s

[Lldb-commits] [PATCH] D103588: [trace] Create a top-level instruction class

vsk added a comment. In D103588#2806611 , @wallace wrote: >> This approach - of prototyping trace analyses on a vector >> representation and then rewriting them later when scaling issues arise - >> doesn't sound good to me. Even for something simple, li

[Lldb-commits] [PATCH] D103588: [trace] Create a top-level instruction class

vsk added a comment. In D103588#2806894 , @wallace wrote: > I've been thinking about what you said and I'm having second thoughts on my > implementation. I'll share more context: > > - I want to work in the short term on reverse debugging and reconstruct

[Lldb-commits] [PATCH] D103588: [trace] Create a top level ThreadTrace class

vsk added a comment. I think this is moving in the right direction! However, the way ThreadTrace is specified still raises the same scalability concerns (see inline). The "instruction index" concept doesn't seem sufficient. Decoding needs to be done in two stages: a separate "trace index" conce

[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

vsk added a comment. FTR, I'm quite happy with the new direction. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104422/new/ https://reviews.llvm.org/D104422 ___ lldb-commits mailing list lldb-com

[Lldb-commits] [PATCH] D105030: [lldb/Interpreter] Add setting to set session transcript save directory

vsk added a comment. I love the save-session feature. Just checking, has it already been release-noted on llvm.org (and for that matter Xcode)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105030/new/ https://reviews.llvm.org/D105030 ___

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

vsk updated this revision to Diff 360180. vsk added a comment. Drop unneeded braces in switch cases. 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/includ

[Lldb-commits] [PATCH] D106584: [lldb] Improve checking of file cache read eligibility for mach-O

vsk added a comment. Hey Augusto, thanks for tackling this, I'm just now slowly paging things in. Is this a correct statement of the problem: LLDB is failing to disable its file cache optimization when reading writable segments (say, __DATA) from a MachO sourced from the shared cache? If that'

[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization

vsk added a comment. Hey @jj10306, thanks for working on this. @wallace @clayborg -- It seems like there are a ton of logic changes introduced in this patch that are tested at too coarse a level. I'm not confident in the changes being made here. For example, there's a bunch of subtle work being

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

vsk updated this revision to Diff 243608. vsk retitled this revision from "[Host.mm] Check for the right macro instead of inlining it." to "[Host.mm] Check for the right macro instead of inlining it". vsk added a comment. - Check TARGET_OS_EMBEDDED. CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

vsk commandeered this revision. vsk edited reviewers, added: davide; removed: vsk. vsk added a comment. This just came up again as the watchOS/tvOS build breaks without the TARGET_OS_EMBEDDED check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73938/new/ https://reviews.llvm.org/D7393

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

This revision was automatically updated to reflect the committed changes. Closed by commit rG936d1427da14: [Host.mm] Check for the right macro instead of inlining it (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D73938?vs=243608&id=243613#toc Repository: rG LLVM Github

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

vsk added a comment. Fixed in d23c15a687ff15327b88fa64da3184395012c2dc . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73938/new/ https://reviews.llvm.org/D73938 _

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

vsk reopened this revision. vsk added a comment. This revision is now accepted and ready to land. Reverted in bf65f19bce88fd9f1a74154d92afe37193ecd7a5 . Jason pointed out this breaks macOS, as TARGET_OS_EMBEDDED is always defin

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

vsk updated this revision to Diff 243679. vsk added a reviewer: jasonmolenda. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73938/new/ https://reviews.llvm.org/D73938 Files: lldb/source/Host/macosx/objcxx/Host.mm Index: lldb/source/Host/macosx/objcxx/Host.mm =

[Lldb-commits] [PATCH] D73860: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

vsk added a comment. Ping, @teemperor do you have any concerns about this one? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73860/new/ https://reviews.llvm.org/D73860 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.

[Lldb-commits] [PATCH] D74018: [lldb/LibCxx] Have ExtractLibcxxStringInfo return an Optional result, NFC

vsk marked 2 inline comments as done. vsk added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:507 size_mode_value = (size_mode->GetValueAsUnsigned(0)); short_mode = ((size_mode_value & 0x80) == 0); JDevlieghere wrote:

[Lldb-commits] [PATCH] D73860: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

vsk marked an inline comment as done. vsk added a comment. Thanks everyone for the reviews! Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:29 +if (sizeof(std::string) == sizeof(garbage_string_sso)) +

[Lldb-commits] [PATCH] D74018: [lldb/LibCxx] Have ExtractLibcxxStringInfo return an Optional result, NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rG90a94c02fb24: [lldb/LibCxx] Have ExtractLibcxxStringInfo return an Optional result, NFC (authored by vsk). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D74018?vs=24249

[Lldb-commits] [PATCH] D73860: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings

This revision was automatically updated to reflect the committed changes. Closed by commit rG7aabad131288: [lldb/StringPrinter] Avoid reading garbage in uninitialized strings (authored by vsk). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D73860?vs=242661&id=2

[Lldb-commits] [PATCH] D73808: [lldb/TypeSystemClang] Supply trivial TypeSourceInfo to NonTypeTemplateParmDecl::Create

This revision was automatically updated to reflect the committed changes. Closed by commit rGd6e47a405a34: [lldb/TypeSystemClang] Supply trivial TypeSourceInfo to NonTypeTemplateParmDecl… (authored by vsk). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D73808?v

[Lldb-commits] [PATCH] D74551: [lldb/dotest] Remove the "exclusive test subdir" concept

vsk accepted this revision. vsk added a comment. Thanks. 'exclusive_test_dir' used to be set in dotest.py, but now no longer is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74551/new/ https://reviews.llvm.org/D74551 ___

[Lldb-commits] [PATCH] D74659: [lldb/FileSystem] Rename CreateDataBuffer -> CreateWritableDataBuffer, NFC

vsk created this revision. vsk added reviewers: JDevlieghere, jingham. vsk added a project: LLDB. Some clients of lldb::FileSystem look like they are unintentionally heap-allocating file contents. This patch renames the CreateDataBuffer API to allow clients to express intention more clearly. As a

[Lldb-commits] [PATCH] D74660: WIP: [lldb/FileSystem] Add & use CreateReadonlyDataBuffer where possible

vsk created this revision. vsk added reviewers: JDevlieghere, jingham. vsk added a project: LLDB. Herald added a subscriber: aprantl. vsk added subscribers: rupprecht, labath. vsk added a comment. + @labath @rupprecht, my tentative plan is to fix this by having ObjectFileELF make a copy into a he

[Lldb-commits] [PATCH] D74660: WIP: [lldb/FileSystem] Add & use CreateReadonlyDataBuffer where possible

vsk added subscribers: rupprecht, labath. vsk added a comment. + @labath @rupprecht, my tentative plan is to fix this by having ObjectFileELF make a copy into a heap-allocated buffer when applying relocations. Please lmk if that's problematic (although, that seems like a slightly lazier version

[Lldb-commits] [PATCH] D74660: WIP: [lldb/FileSystem] Add & use CreateReadonlyDataBuffer where possible

vsk planned changes to this revision. vsk added a comment. @labath thank you for your comments. I'm not yet sure what explains the average size of the WritableMemoryBuffer's allocated on our users' machines (~ 187,918 bytes, comfortably larger than the 16K threshold in `shouldUseMmap`). I'll nee

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

vsk added a comment. Hi @paolosev, the lldb sanitized bot is flagging a container-overflow error here. I know that this /can/ have FPs when sanitized and unsanitized code is mixed, but we should be in purely sanitized code here, and this looks like a valid report. PTAL. http://green.lab.llvm.o

[Lldb-commits] [PATCH] D76337: [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address

vsk created this revision. vsk added reviewers: aprantl, jasonmolenda, friss. lldb currently guesses the address to use when creating an artificial frame (i.e., a frame constructed by determining the sequence of (tail) calls which must have happened). Guessing the address creates problems -- use

[Lldb-commits] [PATCH] D76449: [lldb/Dwarf] Change DW_OP_piece to use an llvm::BitVector

vsk added a reviewer: labath. vsk added a comment. Excited to see this take shape :) Comment at: lldb/source/Expression/DWARFExpression.cpp:935 /// Insertion point for evaluating multi-piece expression. + llvm::BitVector bit_pieces; Stale comment here, wo

[Lldb-commits] [PATCH] D76476: Add formatter for libc++ std::unique_ptr

vsk added a comment. Awesome :) Comment at: lldb/include/lldb/DataFormatters/FormattersHelpers.h:59 +lldb::ValueObjectSP GetValueOfCompressedPair(ValueObject &pair); + Mind adding ‘libcxx’ somewhere in the function name, to avoid confusion?

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

vsk added a comment. @manojgupta I apologize for not catching this earlier. The issue should really be fixed by 636665331bbd4c369a9f33c4d35fb9a863c94646 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ http

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

vsk added a comment. In D73534#1933988 , @djtodoro wrote: > In D73534#1933985 , @djtodoro wrote: > > > Oh sorry, I thought it all has been fixed, since all the tests pass. > > > > We should revert the D75036

[Lldb-commits] [PATCH] D76476: Add formatter for libc++ std::unique_ptr

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM with some added test coverage. Jim? Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/main.cpp:9 + std::unique_ptr up_int = std::

[Lldb-commits] [PATCH] D76337: [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address

vsk added a comment. I'll land this shortly if there aren't any objections. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76337/new/ https://reviews.llvm.org/D76337 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llv

[Lldb-commits] [PATCH] D76337: [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG6905394d1539: [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address (authored by vsk). Herald added a proje

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

vsk created this revision. vsk added reviewers: JDevlieghere, davide. Herald added a subscriber: mgorny. Herald added a project: LLDB. Languages can have different ways of formatting special characters. E.g. when debugging C++ code a string might look like "\b", but when debugging Swift code the s

[Lldb-commits] [PATCH] D96549: Support dereferencing a DWARF scalar stack value

vsk added a comment. The logic change seems reasonable to me. Any chance we can provide a mock memory in the DwarfExpression unit test, or check in assembly for a program that uses such an entry value expression? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96549/new/ https://reviews

[Lldb-commits] [PATCH] D96549: Support dereferencing a DWARF scalar stack value

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Comment at: lldb/unittests/Expression/DWARFExpressionTest.cpp:347 + ExecutionContext exe_ctx(process_sp); + EXPECT_THAT_EXPECTED(Evaluate({DW_OP_lit4, DW_OP_deref},

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

vsk added inline comments. Comment at: lldb/cmake/modules/LLDBFramework.cmake:142-147 + add_custom_command(TARGET liblldb POST_BUILD +COMMAND ${CMAKE_COMMAND} -E create_symlink +Versions/Current/XPCServices +${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.f

[Lldb-commits] [PATCH] D97701: [lldb] Remove XPCServices symlinking

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97701/new/ https://reviews.llvm.org/D97701 ___ lldb

[Lldb-commits] [PATCH] D98272: [lldb/Platform] Skip very slow xcrun queries for simulator platforms, NFC

vsk created this revision. vsk added reviewers: aprantl, JDevlieghere. vsk requested review of this revision. Herald added a project: LLDB. GetXcodeSDK() consistently takes over 1 second to complete if the queried SDK is missing, because `xcrun` doesn't cache negative lookups. Because there are m

[Lldb-commits] [PATCH] D98272: [lldb/Platform] Skip very slow xcrun queries for simulator platforms, NFC

vsk added inline comments. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp:534 +if (arch && arch->IsValid() && !arch->TripleEnvironmentWasSpecified()) + return nullptr; // Avoid very slow xcrun query for non-simulator archs. llvm::StringRe

[Lldb-commits] [PATCH] D98272: [lldb/Platform] Skip very slow xcrun queries for simulator platforms, NFC

vsk updated this revision to Diff 329733. vsk added a comment. Use a helper function, respect the force argument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98272/new/ https://reviews.llvm.org/D98272 Files: lldb/source/Plugins/Platform/MacOSX

[Lldb-commits] [PATCH] D98272: [lldb/Platform] Skip very slow xcrun queries for simulator platforms, NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGac29c35207a5: [lldb/Platform] Skip very slow xcrun queries for simulator platforms, NFC (authored by vsk). Changed prior to commit: https://review

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk created this revision. vsk added a reviewer: jingham. Herald added a subscriber: emaste. vsk requested review of this revision. Herald added a project: LLDB. In general, it seems like the debugger should allow programs to load & run with libraries as far as possible, instead of defaulting to b

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk planned changes to this revision. vsk added a comment. Herald added a subscriber: JDevlieghere. This needs a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98879/new/ https://reviews.llvm.org/D98879 ___

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk added a comment. The best way I can think of to test this is to: - Make a library, T1, out of: % cat t1.c extern void use(); void f1() {} void f2() { use(); } - Make a library, T2, out of: % cat t2.c void use() {} - Link T1.dylib against T2.dylib, so that the reference to `use`

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk updated this revision to Diff 331694. vsk added a comment. - Add a test. It's Darwin-specific as I couldn't work out how to pass the .dylib path to the linker in a platform-agnostic way. - Trim the comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk updated this revision to Diff 331695. vsk added a comment. - Make the test generic (not Darwin-specific) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98879/new/ https://reviews.llvm.org/D98879 Files: lldb/source/Plugins/Platform/POSIX/Platf

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk updated this revision to Diff 331998. vsk added a comment. - Define RTLD_LAZY in the expression as suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98879/new/ https://reviews.llvm.org/D98879 Files: lldb/source/Plugins/Platform/POSIX/P

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk updated this revision to Diff 331999. vsk added a comment. - Delete an unused '#undef DLOPEN_OPTIONS' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98879/new/ https://reviews.llvm.org/D98879 Files: lldb/source/Plugins/Platform/POSIX/Platform

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcb8c1ee269da: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY (authored by vsk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk updated this revision to Diff 332024. vsk added a comment. Add asserts checking that the library paths exist, and restrict the test to Darwin. See https://bugs.llvm.org/show_bug.cgi?id=49656 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98879/new/ https://reviews.llvm.org/D98879 F

[Lldb-commits] [PATCH] D98879: [lldb/PlatformPOSIX] Change LoadImage default to RTLD_LAZY

vsk added a comment. Sorry about that. The file was removed in 8248dd91d7f042893d4a605e98d19cb1b89a44d4 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98879/new/ https://reviews.llvm.org/D98879

[Lldb-commits] [PATCH] D99746: [lldb/test] Respect --apple-sdk path when querying SDK info

vsk created this revision. vsk added a reviewer: JDevlieghere. vsk requested review of this revision. Herald added a project: LLDB. Respect --apple-sdk if it's specified. If the SDK simply mounted from some disk image, and not actually installed, this is the only way to use it. Repository: rG

[Lldb-commits] [PATCH] D99746: [lldb/test] Respect --apple-sdk path when querying SDK info

vsk added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:69 +# use it. +if configuration.apple_sdk is not None: +return configuration.apple_sdk JDevlieghere wrote: > This will also exclude the empty string. Thanks, I

[Lldb-commits] [PATCH] D99746: [lldb/test] Respect --apple-sdk path when querying SDK info

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7d15fb577945: [lldb/test] Respect --apple-sdk path when querying SDK info (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D99

<    1   2   3   4   5   >