[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

vsk added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:366 -if args.server: +if args.server and not args.out_of_tree_debugserver: os.environ['LLDB_DEBUGSERVER_PATH'] = args.server aprantl wrote: > davide wrote: >

[Lldb-commits] [PATCH] D81980: Repair support for launching iphone/tv/watch simulator binaries through platform

vsk accepted this revision. vsk added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:135 + ifeq "$(TRIPLE_ENV)" "" + CODESIGN := codesign +

[Lldb-commits] [PATCH] D81980: Repair support for launching iphone/tv/watch simulator binaries through platform

vsk added a comment. Generally this looks really nice! Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:135 + ifeq "$(TRIPLE_ENV)" "" + CODESIGN := codesign + endif If

[Lldb-commits] [PATCH] D81612: [lldb/Test] Assert that no targets or global modules remain after a test completes.

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

[Lldb-commits] [PATCH] D81696: [lldb/Test] Fix ASan/TSan workaround for Xcode Python 3

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Lgtm, thanks. Comment at: lldb/test/API/lit.cfg.py:102 +if 'DYLD_INSERT_LIBRARIES' in config.environment and platform.system() == 'Darwin': + config.python_executable =

[Lldb-commits] [PATCH] D81696: [lldb/Test] Fix ASan/TSan workaround for Xcode Python 3

vsk added inline comments. Comment at: lldb/test/API/lit.cfg.py:56 +# copy of the "real" python to work with. +def find_python_interpreter(): + # Avoid doing any work if we already copied the binary. This serves as This isn't relevant for the unit or shell

[Lldb-commits] [PATCH] D81612: [lldb/Test] Assert that no targets or global modules remain after a test completes.

vsk added inline comments. Comment at: lldb/bindings/interface/SBModule.i:347 +static uint32_t +GetNumberAllocatedModules(); Can we add a %feature("docstring", ...) blurb about this, advising script authors that it's probably not an API they're

[Lldb-commits] [PATCH] D81516: [lldb/Test] Ensure inline tests have a unique build directory

vsk added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1750 +# (unique) test name is used instead. +inline_name = None +for attrname, attrvalue in attrs.items(): Is `inline_suffix =

[Lldb-commits] [PATCH] D81010: [lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Lgtm. Comment at: lldb/source/Symbol/Function.cpp:321 llvm::ArrayRef> Function::GetTailCallingEdges() { // Call edges are sorted by return PC, and tail calling edges have

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

vsk added inline comments. Comment at: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:99 + if (!overflow) +overflow |= !llvm::checkedAdd(*signed_sum, SInt(carry_in)); uint64_t result = unsigned_sum; aprantl wrote: > vsk wrote: > > The

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

This revision was automatically updated to reflect the committed changes. Closed by commit rG6e39379bbbe1: [DwarfExpression] Support entry values for indirect parameters (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D80345?vs=265815=266337#toc Repository: rG LLVM

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

vsk added inline comments. Comment at: lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8 + // FROM-FUNC1-NEXT: func1 + // FROM-FUNC1-SAME: [artificial] + // FROM-FUNC1-NEXT: main dblaikie wrote: > vsk wrote: > >

[Lldb-commits] [PATCH] D81010: [lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case

vsk added a comment. Thanks, this ended up being a lot cleaner than I expected! Comment at: lldb/include/lldb/Symbol/Function.h:332 - /// The address of the call instruction. Usually an invalid address, unless - /// this is a tail call. - lldb::addr_t call_inst_pc; +

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

vsk added inline comments. Comment at: lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8 + // FROM-FUNC1-NEXT: func1 + // FROM-FUNC1-SAME: [artificial] + // FROM-FUNC1-NEXT: main labath wrote: > labath wrote: > >

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

vsk added a comment. I should add: otherwise, this looks good! Comment at: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:99 + if (!overflow) +overflow |= !llvm::checkedAdd(*signed_sum, SInt(carry_in)); uint64_t result = unsigned_sum;

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

vsk added a comment. Any chance we can export AddWithCarry via EmulateInstructionARM64.h, then add a unit test in TestArm64InstEmulation.cpp? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80955/new/ https://reviews.llvm.org/D80955 ___

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm! Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3740 + if (tail_call) +call_inst_pc = low_pc; + else labath

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

vsk added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3740 + if (tail_call) +call_inst_pc = low_pc; + else labath wrote: > vsk wrote: > > I think this needs to be `call_inst_pc = low_pc - 1`, see > >

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

vsk added a comment. Also, I think it'd be good to enable most/all of the tests under test/API/functionalities/tail_call_frames under -ggdb along with this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80519/new/

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

vsk added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3740 + if (tail_call) +call_inst_pc = low_pc; + else I think this needs to be `call_inst_pc = low_pc - 1`, see

[Lldb-commits] [PATCH] D80518: [lldb] Make "inline" tests more configurable

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

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

vsk updated this revision to Diff 265830. vsk added a comment. Fix an ASSERT_LE that was actually meant to be an ASSERT_GE. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80150/new/ https://reviews.llvm.org/D80150 Files:

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

vsk marked 2 inline comments as done and an inline comment as not done. vsk added inline comments. Comment at: lldb/unittests/DataFormatter/MockTests.cpp:30 + // Can't convert the date_value to a time_t. + EXPECT_EQ(formatDateValue(std::numeric_limits::max() + 1), +

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

vsk updated this revision to Diff 265829. vsk added a comment. Apologies for all the breakage. I think I've identified the issues in the initial patch which caused breakage on the bots, and which caused the tests to not target what they meant to test. I've reworked this patch to use std::llrint,

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk added a comment. Apparently, we always describe the value of an indirect parameter as being "whatever is in the temporary slot", even if the callee modifies it: https://godbolt.org/z/ZgWr_n. So treating the indirect parameter DBG_VALUE as pointing to a (modifiable) location sounds

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk marked 2 inline comments as done. vsk added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:151 +IndirectValue = 1 << 1, +CallSiteParamValue = 1 << 2 + }; aprantl wrote: > I'm going to be pedantic now: Should this be

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk marked an inline comment as done. vsk added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:790 +// FIXME: This produces unusable descriptions when the register contains +// a pointer to a temporary copy of a struct passed by value.

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk updated this revision to Diff 265815. vsk added a comment. Address review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80345/new/ https://reviews.llvm.org/D80345 Files:

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk updated this revision to Diff 265594. vsk marked an inline comment as done. vsk added a comment. Address review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80345/new/ https://reviews.llvm.org/D80345 Files:

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk updated this revision to Diff 265629. vsk added a comment. Remove incorrect fixme. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80345/new/ https://reviews.llvm.org/D80345 Files:

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk marked 2 inline comments as done. vsk added inline comments. Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18 +# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0 +# CHECK-NEXT: [0x0010, 0x001c):

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk updated this revision to Diff 265805. vsk added a comment. Add test coverage for indirect parameter location with offset; fix typo; fix LangRef entry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80345/new/ https://reviews.llvm.org/D80345

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk marked 11 inline comments as done. vsk added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1288 DwarfExpr.addFragmentOffset(DIExpr); - if (Location.isIndirect()) + if (Location.isIndirect() && !DIExpr->isEntryValue())

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

vsk created this revision. vsk added reviewers: djtodoro, aprantl, dstenb. Herald added subscribers: lldb-commits, hiraditya. Herald added projects: LLDB, LLVM. A struct argument can be passed-by-value to a callee via a pointer to a temporary stack copy. Add support for emitting an entry value

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

vsk marked 3 inline comments as done. vsk added a comment. @labath Agreed on all points, I've addressed the feedback in 82dbf4aca84 by moving "DataFormatters/Mock.h" to "Plugins/Language/ObjC/Utilities.h", and adding a

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

This revision was automatically updated to reflect the committed changes. vsk marked an inline comment as done. Closed by commit rGb783f70a4257: [lldb/DataFormatter] Check for overflow when finding NSDate epoch (authored by vsk). Changed prior to commit:

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

vsk marked 2 inline comments as done. vsk added a comment. Thanks! Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:801-804 + // this snippet of code assumes that time_t == seconds since Jan-1-1970 this + // is generally true and POSIXly happy, but might break if a

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

vsk created this revision. vsk added reviewers: JDevlieghere, mib, teemperor. Herald added a subscriber: mgorny. Herald added a project: LLDB. Fixes UBSan-reported issues where the date value inside of an uninitialized NSDate overflows the 64-bit epoch. rdar://61774575 Repository: rG LLVM

[Lldb-commits] [PATCH] D79645: [lldb/test] Fix for flakiness in TestNSDictionarySynthetic

This revision was automatically updated to reflect the committed changes. vsk marked an inline comment as done. Closed by commit rGf807d0b4acdb: [lldb/test] Fix for flakiness in TestNSDictionarySynthetic (authored by vsk). Changed prior to commit:

[Lldb-commits] [PATCH] D79645: [lldb/test] Fix for flakiness in TestNSDictionarySynthetic

vsk created this revision. vsk added reviewers: JDevlieghere, jingham, shafik. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. TestNSDictionarySynthetic sets up an NSURL which does not initialize its _baseURL member. When the test runs and we print out the NSURL, we print

[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

This revision was automatically updated to reflect the committed changes. Closed by commit rG8cb86ead7741: [lldb/test][Darwin] Ask dyld where the real python is (authored by vsk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79607/new/

[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

vsk updated this revision to Diff 262799. vsk added a comment. - Reinstated comment about SIP to explain why we copy python - Added a platform check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79607/new/ https://reviews.llvm.org/D79607 Files:

[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

vsk updated this revision to Diff 262792. vsk added a comment. Query dyld while running within the right python process. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79607/new/ https://reviews.llvm.org/D79607 Files:

[Lldb-commits] [PATCH] D79603: Add an API to construct an XcodeSDK from an SDK type.

vsk added inline comments. Comment at: lldb/source/Utility/XcodeSDK.cpp:44 + } + static_assert(XcodeSDK::Linux == XcodeSDK::numSDKTypes - 1, +"New SDK type was added, update this list!"); I'd expect -Wcovered-switch to break the build if a new

[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

vsk created this revision. vsk added a reviewer: JDevlieghere. Herald added a project: LLDB. On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim python binary as the ASan interceptors get loaded too late. Find the "real" python binary, copy it, and invoke it. Hopefully this makes

[Lldb-commits] [PATCH] D79535: Add a function to detect whether an Xcode SDK supports Swift

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

[Lldb-commits] [PATCH] D79538: Add an XcodeSDK::GetSDKTypeForTriple function

vsk accepted this revision. vsk added a comment. lgtm as well CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79538/new/ https://reviews.llvm.org/D79538 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM, thanks. It'd be nice to have a skipUnlessEntryValuesSupportedArch decorator, but I don't think that has to be folded into this change. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

vsk added a comment. Thanks! LGTM, modulo any comments Raphael may have about the test change. Comment at: lldb/packages/Python/lldbsuite/test/lldbinline.py:155 process, lldb.eStopReasonBreakpoint) -breakpoint_id =

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

vsk added a comment. Thanks for working on this! Comment at: lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:21-22 + ++global; //% self.filecheck("image lookup -va $pc", "main.cpp", "-check-prefix=FUNC1-DESC") // FUNC1-DESC: name =

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGa37caebc2d2d: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D77843?vs=261558=261934#toc Repository: rG LLVM Github

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

vsk marked an inline comment as done. vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:42-43 + llvm_unreachable("unsupported length"); +memcpy(reinterpret_cast(m_data), + reinterpret_cast(bytes), size); + }

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

vsk marked 4 inline comments as done. vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:268 case GetPrintableElementType::UTF8: -return [](uint8_t *buffer, uint8_t *buffer_end, - uint8_t *) ->

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

vsk updated this revision to Diff 261558. vsk marked an inline comment as done. vsk added a comment. Address review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77843/new/ https://reviews.llvm.org/D77843 Files:

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

vsk updated this revision to Diff 261387. vsk added a comment. - Avoid heap memory management in "StringPrinterBufferPointer" entirely. - Rename "StringPrinterBufferPointer" to "DecodedCharBuffer", as that seems to more accurately reflect what the class is for. Repository: rG LLVM Github

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

vsk added a comment. Friendly ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77843/new/ https://reviews.llvm.org/D77843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D78825: [lldb/Driver] Exit with a non-zero exit code in batch mode when stopping because of an error.

vsk added a comment. Can we delete an override of SBDebugger::RunCommandInterpreter, or are they all part of the stable API? If we can, it'd be nice to get rid of the one with 6 args. Otherwise this lgtm. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION

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

vsk updated this revision to Diff 258825. vsk added a comment. Address review feedback: - Make 'escape_style' a regular parameter, use raw string literals Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77843/new/ https://reviews.llvm.org/D77843

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

vsk updated this revision to Diff 258419. vsk marked an inline comment as done. vsk added a comment. Address review feedback: - Use standard gtest operators (EXPECT_EQ) - constexpr/std::move cleanup, comment cleanups, sprintf length fix Also, I took the opportunity to further unify the ASCII

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

vsk marked an inline comment as done. vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:405 -template <> -bool StringPrinter::ReadStringAndDumpToStream< -StringPrinter::StringElementType::ASCII>( This is what I mean. The

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

vsk marked 8 inline comments as done. vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:173 + unsigned escaped_len; + const unsigned max_buffer_size = 7; + uint8_t *data = new uint8_t[max_buffer_size]; shafik wrote: >

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

vsk planned changes to this revision. vsk added a comment. Sorry for the delay, I plan to have an update for this soon. In D77843#1978964 , @JDevlieghere wrote: > Have you asked the foundation people about the ASCII support in NSString? No, I'm not

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

vsk marked an inline comment as done. vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:126 case 0: -retval = {"\\0", 2}; -break; +return {"\\0", 2}; case '\a': aprantl wrote: > We have a lot of implementations

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

vsk added reviewers: teemperor, labath. vsk added a comment. + Raphael, I suppose this is in a similar direction as D68691 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77843/new/ https://reviews.llvm.org/D77843

[Lldb-commits] [PATCH] D76697: [lldb] Replace debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType with lldbassert

vsk added a comment. The change looks reasonable to me. @davide? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76697/new/ https://reviews.llvm.org/D76697 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[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

[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

[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

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

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

[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 added a comment. Awesome :) Comment at: lldb/include/lldb/DataFormatters/FormattersHelpers.h:59 +lldb::ValueObjectSP GetValueOfCompressedPair(ValueObject ); + Mind adding ‘libcxx’ somewhere in the function name, to avoid confusion?

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

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

[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

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

[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

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

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

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

[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

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

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

[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

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=243613#toc Repository: rG LLVM Github

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

[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

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

vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:144 +return retval; + if (!llvm::checkedAdd(reinterpret_cast(buffer), +static_cast(utf8_encoded_len))) shafik wrote: > Wouldn't we want

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

vsk updated this revision to Diff 242661. vsk marked 4 inline comments as done. vsk added a comment. - `s/cap/capacity/` - Remove the `checkedAdd` pointer overflow check as it's not necessary. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73860/new/ https://reviews.llvm.org/D73860

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

vsk updated this revision to Diff 242495. vsk marked an inline comment as done. vsk added a comment. - Make a bool const. - Split off other refactoring into https://reviews.llvm.org/D74018. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73860/new/ https://reviews.llvm.org/D73860 Files:

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

vsk created this revision. vsk added a reviewer: JDevlieghere. ... and a few other minor simplifications. https://reviews.llvm.org/D74018 Files: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

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

vsk updated this revision to Diff 242479. vsk added a comment. Per offline feedback from Jonas, label variables `const` where applicable and get rid of the ScopeExits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73860/new/ https://reviews.llvm.org/D73860 Files:

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

vsk updated this revision to Diff 242461. vsk added a reviewer: LLDB. vsk added a comment. - Enhance test coverage with several more examples of garbage long-mode std::strings. - In several cases, when we detected an invalid string, we would fall back to spewing the "raw mode" representation of

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

vsk added a comment. The original sanitizer report /does/ show UB in the NonTypeTemplateParmDecl static constructor: * thread #1, queue = 'com.apple.main-thread', stop reason = Null pointer use *

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

vsk added a comment. It probably makes sense to land this and D73946 as a defensive measure, without any test change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73808/new/ https://reviews.llvm.org/D73808

<    1   2   3   4   5   >