[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: > > 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 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 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 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 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 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 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] 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] 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] 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] 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] 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] 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] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

vsk added a comment. Looks like a nice/reasonable cleanup, thanks! Based on the coverage report (https://teemperor.de/lldb-coverage/coverage/Users/vsk/src/llvm.org-lldbsan/llvm/tools/lldb/source/Breakpoint/BreakpointList.cpp.html#L217) and benchmarks (https://teemperor.de/lldb-bench/static.html

[Lldb-commits] [PATCH] D55761: lldb-test ir-memory-map: Use IntervalMap::contains

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

[Lldb-commits] [PATCH] D55574: Remove else statements after returns

vsk added a comment. + 1 to this. If there's a tidy plugin for misleading indention, that might address some of Adrian's concerns. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55574/new/ https://reviews.llvm.org/D55574 _

[Lldb-commits] [PATCH] D54731: [lit] Enable the use of custom user-defined lit commands

vsk added a subscriber: filcab. vsk added a comment. For compiling/linking, I think we can get by using lit substitutions to fill in platform-specific options? iOS testing for Swift is done this way (both on-device and simulator), as is testing for the profiling runtime. Dan and @filcab are mor

[Lldb-commits] [PATCH] D54385: Remove comments after header includes.

vsk accepted this revision as: vsk. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rLLDB LLDB https://reviews.llvm.org/D54385 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.

[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables

vsk added a comment. In https://reviews.llvm.org/D53731#1276732, @zturner wrote: > In https://reviews.llvm.org/D53731#1276660, @jingham wrote: > > > Could you also use Vedant's new FileCheck dotest test class? That should > > allow you to write the tests exactly as you are, but use the dotest

[Lldb-commits] [PATCH] D53415: [lldbsuite, windows] Disable two tail call frames tests that fail on Windows

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rLLDB LLDB https://reviews.llvm.org/D53415 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

vsk updated this revision to Diff 169471. vsk added a comment. - Address comments from @stella.stamenova https://reviews.llvm.org/D53175 Files: lldb/packages/Python/lldbsuite/test/configuration.py lldb/packages/Python/lldbsuite/test/dotest.py lldb/packages/Python/lldbsuite/test/lldbtest.p

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

vsk created this revision. vsk added reviewers: stella.stamenova, zturner. This allows bots which haven't updated to pass in --filecheck to dotest.py to run more tests. FileCheck-dependent tests will continue to fail. https://reviews.llvm.org/D53175 Files: lldb/packages/Python/lldbsuite/test

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk added a comment. In https://reviews.llvm.org/D50478#1262710, @stella.stamenova wrote: > Unfortunately, the bots are broken because of the FileCheck issue, so I can't > confirm with them, but I see a number of these tests fail in our local > testing. Some fail on both Windows and Linux and s

[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)

vsk added a comment. Ping? https://reviews.llvm.org/D50155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module

vsk added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/target_var/globals.ll:1 +source_filename = "globals.c" +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" davide wrote: > vsk wrote: > > Should we check in bitcode in

[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module

vsk added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/target_var/globals.ll:1 +source_filename = "globals.c" +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" Should we check in bitcode instead? That might make it easie

[Lldb-commits] [PATCH] D51874: Fix buildbot regression by rL339929: NameError: global name 'test_directory' is not defined

vsk added inline comments. Comment at: packages/Python/lldbsuite/test/dosep.py:1693 for core in cores: dst = core.replace(test_directory, "")[1:] dst = dst.replace(os.path.sep, "-") Instead of redefining test_directory, please use 'test_sub

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:304 +public: + CallEdge(const char *mangled_name, lldb::addr_t return_pc); + aprantl wrote: > vsk wrote: > > vsk wrote: > > > aprantl wrote: > > > > Does this also work for C functions? I

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:304 +public: + CallEdge(const char *mangled_name, lldb::addr_t return_pc); + aprantl wrote: > Does this also work for C functions? If yes, would `symbol_name` be a more > accurate descri

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 168155. vsk marked an inline comment as done. vsk added a comment. - Address feedback from Adrian. https://reviews.llvm.org/D50478 Files: lldb/include/lldb/API/SBFrame.h lldb/include/lldb/Core/FormatEntity.h lldb/include/lldb/Symbol/Block.h lldb/include

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

vsk added a comment. Could you describe how the test exercises DW_FORM_implicit_const support? It's not immediately clear to me. https://reviews.llvm.org/D52689 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk added a comment. Friendly ping. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

vsk added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1531 + if (m_compile_unit_infos.size() > 1) +return 0; + sgraenitz wrote: > sgraenitz wrote: > > Skipping AddOSOARanges() here. > > Could you leave an in-sourc

[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

vsk added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:431 const size_t num_ranges = -die->GetAttributeAddressRanges(dwarf, this, ranges, false); +die->GetAttributeAddressRanges(dwarf, this, ranges, check_hi_lo_pc); if (num_

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk added inline comments. Comment at: lldb/source/Target/StackFrameList.cpp:331 +dfs(next_callee); +if (ambiguous) + return; aprantl wrote: > On what path can this happen? Aren't all paths that set `ambiguous=true` > returning immediate

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 167148. vsk marked an inline comment as done. vsk added a comment. - Use std::make_shared(...), instead of StackFrameSP(new ...). https://reviews.llvm.org/D50478 Files: lldb/include/lldb/API/SBFrame.h lldb/include/lldb/Core/FormatEntity.h lldb/include/lld

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 167038. vsk added a comment. The bug I thought I saw in CallEdge::GetReturnPCAddress turned out to be an issue I introduced in dsymutil. It's not correct for dsymutil to relocate the return PC value in TAG_call_site entries, because the correct section slide of

[Lldb-commits] [PATCH] D52516: [lldbinline] Set directory attribute on test-specific classes

vsk created this revision. vsk added reviewers: labath, jingham, JDevlieghere. Herald added subscribers: eraman, aprantl. Set the "mydir" attribute of an inline test on the test-specific class, instead of on the base InlineTest class. This makes it possible to run dotest.py on a directory contain

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk planned changes to this revision. vsk added a comment. While testing this out, I found an issue with CallEdge::GetReturnPCAddress. Getting the load address there adds an unnecessary slide to the PC. I'll try to have that worked out next week. https://reviews.llvm.org/D50478

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 166534. vsk added a comment. As discussed offline, print out a note when stepping out of a frame with artificial ancestors explaining that they were skipped while stepping out. See the added test: functionalities/tail_call_frames/thread_step_out_message. https

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 166394. vsk added a comment. Teach SBThread::StepOut and SBThread::ReturnFromFrame to behave as-if artificial frames were not present. This preserves the current behavior of "finish" and "thread return". The alternatives -- stepping out into an artificial frame

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk planned changes to this revision. vsk added a comment. In https://reviews.llvm.org/D50478#1241264, @jingham wrote: > Can you add a test that makes sure that when you stop in a frame that has > artificial frames above it, and then you do "finish", or "step out" past the > end of frame 0, the

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 166378. vsk added a comment. I've added SB API support (SBFrame::IsArtificial), a SB API test, fleshed out the remaining tests, and rebased. PTAL, thanks! https://reviews.llvm.org/D50478 Files: lldb/include/lldb/API/SBFrame.h lldb/include/lldb/Core/FormatE

[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)

vsk updated this revision to Diff 165613. vsk marked 6 inline comments as done. vsk added a comment. Herald added a subscriber: mgorny. Sorry for the delay, I was busy with other work. I think I've addressed the review feedback. PTAL. https://reviews.llvm.org/D50751 Files: lldb/CMakeLists.tx

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

vsk added a comment. Please clang-format your diffs. https://reviews.llvm.org/D51520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

vsk added a comment. This LGTM. Davide? Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

vsk added a comment. I think it'd be useful to test the driver output specifically. The kind of testing lldb-test facilitates might not be a good fit here (too low-level). Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits m

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

vsk added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py:70 +self.expect("frame variable v_no_value", +substrs=['v_no_value = No Value'])

[Lldb-commits] [PATCH] D51319: Use a RAII guard to control access to DisassemblerLLVMC.

vsk accepted this revision. vsk added a comment. Looks great, thanks! Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:176 bool got_op = false; -std::shared_ptr disasm_sp(GetDisassembler()); -if (disasm_sp) { - const ArchSpec &arch = disasm_s

[Lldb-commits] [PATCH] D51319: Use a RAII guard to lock/unlock DisassemblerLLVMC [NFC]

vsk added inline comments. Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:81 + struct Guard { +DisassemblerLLVMC *m_instance; +Guard(DisassemblerLLVMC *instance, InstructionLLVMC *inst, This is nice. Do you think it might be even safer

[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)

vsk updated this revision to Diff 162491. vsk retitled this revision from "WIP: Expose FileCheck-style testing within lldb inline tests" to "Allow use of self.filecheck in LLDB tests (c.f self.expect)". vsk edited the summary of this revision. vsk added reviewers: teemperor, aprantl, zturner. vsk

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 162416. vsk marked 4 inline comments as done. vsk added a comment. Thanks for the feedback! https://reviews.llvm.org/D50478 Files: lldb/include/lldb/Symbol/Block.h lldb/include/lldb/Symbol/Function.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/ll

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:331 + /// \ref resolved. + union { +const char *mangled_name; aprantl wrote: > `llvm::PointerUnion` ? It's not possible to use PointerUnion here because `const char *` has 1-byte al

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk added a comment. Ping. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51185: Reuse the SelectorTable from Clang's Preprocessor

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM. This is NFC, it seems. There's a FIXME in Preprocessor.h about the lifetime of SelectorTable eventually not being tied to Preprocessor, but this is correct for now. Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D50481: Fix broken builtin functions in the expression command

vsk accepted this revision as: vsk. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Repository: rLLDB LLDB https://reviews.llvm.org/D50481 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.

[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. (LGTM with the second comment addressed.) https://reviews.llvm.org/D50997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.o

[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.

vsk added a comment. Thanks so much for doing this! Comment at: lit/Suite/lit.cfg:28 + resource_dir = subprocess.check_output(config.cmake_cxx_compiler + + ' -print-resource-dir', shell=True) + runtime = os.path.join(resource_dir[:-1],

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

vsk added inline comments. Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:46 + // Member __f_ has type __base*, the contents of which will either directly + // hold a pointer to the callable object or vtable entry which will hold the + // type information need to dis

[Lldb-commits] [PATCH] D50751: WIP: Expose FileCheck-style testing within lldb inline tests

vsk created this revision. This patch isn't quite ready for review. It's a simple proof-of-concept which shows what it would take to make FileCheck available within lldb inline tests. I'll kick off a discussion about this on lldb-dev. https://reviews.llvm.org/D50751 Files: lldb/packages/Pyt

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

vsk updated this revision to Diff 160666. vsk retitled this revision from "WIP: Basic tail call frame support" to "Add support for artificial tail call frames". vsk edited the summary of this revision. vsk added reviewers: aprantl, probinson, JDevlieghere, jingham, friss, zturner. vsk removed subs

[Lldb-commits] [PATCH] D50620: Added test for Core/Range class.

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, looks good with nitpicks. Comment at: unittests/Core/RangeTest.cpp:139 + RangeT r; + // FIXME: This is probably not intended. + EXPECT_TRUE(r.ContainsEndInclusive(0)); -

[Lldb-commits] [PATCH] D50478: WIP: Basic tail call frame support

vsk updated this revision to Diff 160224. vsk added a comment. Herald added a subscriber: mgrang. Rebase, and update the patch to use DW_AT_call_return_pc information. https://reviews.llvm.org/D50478 Files: lldb/include/lldb/Symbol/Block.h lldb/include/lldb/Symbol/Function.h lldb/include/

[Lldb-commits] [PATCH] D50225: Use a DenseMap for looking up functions by UID in CompileUnit::FindFunctionByUID

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

[Lldb-commits] [PATCH] D50478: WIP: Basic tail call frame support

vsk created this revision. Herald added a subscriber: JDevlieghere. This is a prototype of tail call frame support for lldb based on compiler support from https://reviews.llvm.org/D49887. It isn't ready for review. I'm sharing the proof-of-concept to give a heads-up about this coming down the pip

[Lldb-commits] [PATCH] D50271: [IRMemoryMap] Shrink Allocation make it move-only (NFC)

vsk updated this revision to Diff 159339. vsk retitled this revision from "[IRMemoryMap] Shrink Allocation by sizeof(addr_t) (NFC)" to "[IRMemoryMap] Shrink Allocation make it move-only (NFC)". vsk edited the summary of this revision. vsk added a comment. - Make Allocation move-only. This should

[Lldb-commits] [PATCH] D50271: [IRMemoryMap] Shrink Allocation by sizeof(addr_t) (NFC)

vsk created this revision. vsk added reviewers: teemperor, lhames. Profiling data show that Allocation::operator= is hot, see: https://teemperor.de/lldb-bench/data/arithmetic.svg Reorder a few fields within Allocation to avoid implicit structure padding and shrink the structure. This should ma

[Lldb-commits] [PATCH] D50225: Use a DenseMap for looking up functions by UID in CompileUnit::FindFunctionByUID

vsk added a comment. Thanks for doing this :)! Comment at: source/Symbol/CompileUnit.cpp:111 // TODO: order these by address m_functions.push_back(funcSP); } Is m_functions used to do anything crucial? I see a use in Dump, but it seems like you could re

[Lldb-commits] [PATCH] D50149: Fix out-of-bounds read in Stream::PutCStringAsRawHex8

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

[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)

vsk created this revision. vsk added reviewers: aprantl, jasonmolenda. Herald added subscribers: chrib, krytarowski, mgorny, srhines. This code looks like a good reference for building a new unwinder, but is currently unused, so there's no need to keep it. https://reviews.llvm.org/D50155 Files:

[Lldb-commits] [PATCH] D50087: Add doxygen comments to the StackFrameList API (NFC)

vsk added a comment. In https://reviews.llvm.org/D50087#1183982, @labath wrote: > I am not too familiar with this code, but the descriptions seem to make sense. > > However, since you have kind of opened up the `Optional` discussion, I'll use > this opportunity to give my take on it: > > I've wa

[Lldb-commits] [PATCH] D50087: Add doxygen comments to the StackFrameList API (NFC)

vsk created this revision. vsk added reviewers: labath, jasonmolenda, tberghammer. Clarify how StackFrameList works by documenting its methods. Also, delete some dead code and insert some TODOs. https://reviews.llvm.org/D50087 Files: lldb/include/lldb/Target/StackFrameList.h lldb/source/Tar

[Lldb-commits] [PATCH] D48303: Don't take the address of an xvalue when printing an expr result

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM. This looks pretty cut and dry. The evaluator shouldn't try to take the address of an rvalue. https://reviews.llvm.org/D48303 ___ lldb-co

[Lldb-commits] [PATCH] D48450: [DataFormatter] Add CFDictionary data formatter

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rL LLVM https://reviews.llvm.org/D48450 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cg

[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands

vsk added a comment. In https://reviews.llvm.org/D47646#1119512, @stella.stamenova wrote: > I can look into the failure - but can you XFAIL the test rather than skipping > it and log a bug, so that we can track the failure rather than potentially > assuming down the line that the test is not me

[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands

vsk added a comment. In https://reviews.llvm.org/D47646#1119487, @vsk wrote: > In https://reviews.llvm.org/D47646#1119479, @stella.stamenova wrote: > > > In https://reviews.llvm.org/D47646#1119471, @vsk wrote: > > > > > In https://reviews.llvm.org/D47646#1119396, @stella.stamenova wrote: > > > >

[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands

vsk added a comment. In https://reviews.llvm.org/D47646#1119479, @stella.stamenova wrote: > In https://reviews.llvm.org/D47646#1119471, @vsk wrote: > > > In https://reviews.llvm.org/D47646#1119396, @stella.stamenova wrote: > > > > > While you are at it, can you make sure this works on Windows? Th

[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands

vsk added a comment. In https://reviews.llvm.org/D47646#1119396, @stella.stamenova wrote: > While you are at it, can you make sure this works on Windows? The current > version of the test that is checked in fails. Sorry about that. Could you point me to the error message? https://reviews.llv

[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands

vsk created this revision. vsk added a reviewer: labath. Change the syntax of the malloc and free commands in lldb-test's ir-memory-map subcommand to: ::= = malloc ::= free This should make it easier to read and extend tests in the future, e.g to test IRMemoryMap::WriteMemory or dou

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

vsk updated this revision to Diff 149355. vsk marked 2 inline comments as done. vsk added a comment. - Address Pavel's feedback, remove a questionable FIXME. https://reviews.llvm.org/D47551 Files: lit/Expr/Inputs/ir-memory-map-basic.test lit/Expr/Inputs/ir-memory-map-overlap1.test lit/Exp

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

vsk added a comment. In https://reviews.llvm.org/D47551#1117086, @lhames wrote: > LGTM. > > I haven't looked at process memory management. How hard would your FIXME be > to implement? After looking at this more carefully, I think the FIXME makes a bad prescription. It's based on the assumptio

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

vsk updated this revision to Diff 149198. vsk added a reviewer: lhames. vsk added a comment. - Don't insert extra padding bytes when `alignment` = 1. - + Lang https://reviews.llvm.org/D47551 Files: lit/Expr/Inputs/ir-memory-map-basic.test lit/Expr/Inputs/ir-memory-map-overlap1.test lit/Ex

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

vsk created this revision. vsk added reviewers: labath, zturner, jingham, aprantl. vsk edited the summary of this revision. This prevents Malloc from allocating the same chunk of memory twice, as a byproduct of an alignment adjustment which gave the client access to unallocated memory. Prior to t

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

vsk updated this revision to Diff 149173. vsk edited the summary of this revision. vsk added a comment. - Really fix the allocation overlap test. The previous version of this patch would not detect overlaps in which the end of the new allocation is contained within an existing allocation. > The

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

vsk added inline comments. Comment at: tools/lldb-test/lldb-test.cpp:503 + uint8_t Alignment; + int Matches = sscanf(Line.data(), "malloc %lu %hhu", &Size, &Alignment); + if (Matches != 2) labath wrote: > is `Line` null-terminated here? Also a size_t arg shoul

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

vsk updated this revision to Diff 149159. vsk edited the summary of this revision. vsk added a comment. - Use %zu, and improve detection of overlapping allocations. https://reviews.llvm.org/D47508 Files: lit/Expr/TestIRMemoryMap.test source/Target/Process.cpp tools/lldb-test/lldb-test.cpp

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

vsk created this revision. vsk added reviewers: jingham, davide, labath, zturner. This teaches lldb-test how to launch a process, set up an IRMemoryMap, and issue memory allocations in the target process through the map. This makes it possible to test IRMemoryMap in a targeted way. The main motiv

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

<    1   2   3   4   5   >