[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl created this revision. aprantl added reviewers: JDevlieghere, kastiglione, jingham. aprantl requested review of this revision. This patch adds support for Swift compiler producer strings to DWARFUnit. https://reviews.llvm.org/D111278 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUn

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl updated this revision to Diff 377702. aprantl added a comment. Sort variables in a more logical order. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111278/new/ https://reviews.llvm.org/D111278 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/Sy

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:684 + llvm::SmallVector matches; + if (g_swiftlang_version_regex.Execute(producer, &matches)) { + m_producer_version.tryParse(matches[1]); JDevlieghere wrote: > F

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:684 + llvm::SmallVector matches; + if (g_swiftlang_version_regex.Execute(producer, &matches)) { + m_producer_version.tryParse(matches[1]); aprantl wrote: > JDevli

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl updated this revision to Diff 377977. aprantl added a comment. Const. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111278/new/ https://reviews.llvm.org/D111278 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h lldb

[Lldb-commits] [PATCH] D111339: [lldb] Parse and display reporting errors from JSON crashlogs

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

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

This revision was automatically updated to reflect the committed changes. Closed by commit rG4651576edd09: Recognize the Swift compiler in DW_AT_producer (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D111278?vs=377977&id=378001#toc Repos

[Lldb-commits] [PATCH] D112662: [lldb] Fixup code addresses in the Objective-C language runtime

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This is all good of course, but I wonder if we should lift this into the type system and have functions returned a UnstrippedAddress type that you can only turn into an addr_t by calling Fix

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Yup that's wrong. Personally I would only append env to the list if non-empty. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112676/new/ https://reviews.llvm.org/D112676 _

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

aprantl added a comment. thanks! Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:62 +component = [arch, vendor, os + version] +if env: +components.append(env) So this tests for the empty string? CHANGES SINCE LAST ACTION h

[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112165/new/ https://reviews.llvm.org/D112165

[Lldb-commits] [PATCH] D112439: normalize file path when searching the source map

aprantl added inline comments. Comment at: lldb/source/Target/PathMappingList.cpp:33 // nomalized path pairs to ensure things match up. ConstString NormalizePath(ConstString path) { // If we use "path" to construct a FileSpec, it will normalize the path for ---

[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

aprantl added a comment. This is great, thanks! (one question inline) Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:1008 } - result.SetStatus(lldb::eReturnStatusSuccessFinishResult); - return true; + success

[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

aprantl added inline comments. Comment at: lldb/test/Shell/ObjC/tagged_pointer_info.test:3 + +RUN: %clang_host -g -framework Foundation -o %t.out %S/Inputs/test.m + personal opinion without through reasoning behind it: This feels more like an API test to me. C

[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

aprantl accepted this revision. aprantl added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112863/new/ https://reviews.llvm.org/D112863 ___ lldb-commits mailing list lldb-commits@l

[Lldb-commits] [PATCH] D112439: normalize file path when searching the source map

aprantl accepted this revision. aprantl added a comment. In D112439#3098307 , @xujuntwt95329 wrote: > Hi, @aprantl > > I've submitted a NFC patch here: > https://reviews.llvm.org/D112863 > > Seems that patch can't build by CI because it is based on this

[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Excellent, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112945/new/ https://reviews.llvm.org/D112945 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D113445: Support looking up absolute symbols

aprantl created this revision. aprantl added reviewers: jasonmolenda, jingham, JDevlieghere. aprantl requested review of this revision. The Swift stdlib uses absolute symbols in the dylib to communicate feature flags to the process. LLDB's expression evaluator needs to be able to find them. This

[Lldb-commits] [PATCH] D113445: Support looking up absolute symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rGc9881c7d99c6: Support looking up absolute symbols (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113445/n

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

aprantl added a comment. This seems to have broken the green dragon incremental bot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/38387/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113650/new/ https://reviews.llvm.org/D113650 _

[Lldb-commits] [PATCH] D111899: LLDB tests modification for hardware breakpoints

aprantl added a comment. FYI, this broke all the TestObjCNewSyntax* tests. I *think* I've fixed them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111899/new/ https://reviews.llvm.org/D111899 ___ lldb-c

[Lldb-commits] [PATCH] D114742: [lldb] Search PrivateFrameworks when using an internal SDK

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:86 +private_frameworks = os.path.join(sdk_root, 'System','Library','PrivateFramework

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

aprantl added a comment. @Mordante @Michael137 This seems to fail on older versions of compiler/libcxx https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-matrix/7247/ You can probably fix this by just requiring a minimum clang version in the tests Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D133069: Fix inconsistent target arch when attaching to arm64 binaries on arm64e platforms.

aprantl created this revision. aprantl added reviewers: JDevlieghere, jasonmolenda. Herald added subscribers: omjavaid, kristof.beyls. Herald added a project: All. aprantl requested review of this revision. On arm64e-capable Apple platforms, the system libraries are always arm64e, but application

[Lldb-commits] [PATCH] D133075: Fix lldb-dotest when dotest_args_str is empty

aprantl added a comment. Interesting, I would have expected `array.extend([])` be a noop? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133075/new/ https://reviews.llvm.org/D133075 ___ lldb-commits maili

[Lldb-commits] [PATCH] D133075: Fix lldb-dotest when dotest_args_str is empty

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks, that makes more sense! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133075/new/ https://reviews.llvm.org/D133075 ___

[Lldb-commits] [PATCH] D133069: Fix inconsistent target arch when attaching to arm64 binaries on arm64e platforms.

aprantl added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp:584 +triple.setArchName("arm64"); +target_arch.SetTriple(triple); +target.SetArchitecture(target_arch, /*set_platform=*/false, ---

[Lldb-commits] [PATCH] D133069: Fix inconsistent target arch when attaching to arm64 binaries on arm64e platforms.

aprantl updated this revision to Diff 457416. aprantl added a comment. Address feedback! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133069/new/ https://reviews.llvm.org/D133069 Files: lldb/include/lldb/Target/Target.h lldb/packages/Python/lldbsuite/test/gdbclientutils.py lldb/

[Lldb-commits] [PATCH] D133069: Fix inconsistent target arch when attaching to arm64 binaries on arm64e platforms.

This revision was automatically updated to reflect the committed changes. Closed by commit rGced4e0006fab: Fix inconsistent target arch when attaching to arm64 binaries on (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Mo

[Lldb-commits] [PATCH] D133069: Fix inconsistent target arch when attaching to arm64 binaries on arm64e platforms.

aprantl added a comment. In D133069#3768088 , @rsmith wrote: > Hi, the newly-added test, `TestDynamicLoaderDarwin.py`, fails under asan: > https://gist.github.com/zygoloid/bf7b21f03d7db966e456b6c365c4635d Thanks! Should be fixed in commit 603d5a8d632

[Lldb-commits] [PATCH] D132940: [lldb] Use just-built libcxx for tests when available

aprantl added a comment. Could we land this today so we can get the bots green again? Please still make sure to respond to any post-commit feedback from @labath in case there's anything hasn't been addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D133519: Document some of the clang-specific DWARF extensions supported by LLDB

aprantl created this revision. aprantl added reviewers: JDevlieghere, Michael137, jingham, labath. Herald added a project: All. aprantl requested review of this revision. This patch adds a new page to the LLDB documentation that documents, among other things the `-gmodules` debug info format. h

[Lldb-commits] [PATCH] D133519: Document some of the clang-specific DWARF extensions supported by LLDB

aprantl updated this revision to Diff 458839. aprantl added a comment. Herald added a subscriber: arphaman. Wire up the ToC CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133519/new/ https://reviews.llvm.org/D133519 Files: lldb/docs/index.rst lldb/docs/use/extensions.rst Index: lld

[Lldb-commits] [PATCH] D133618: Adapt LLDB dataformatters for libcxx change D129386

aprantl created this revision. aprantl added reviewers: Michael137, fdeazeve, ldionne, philnik. Herald added a project: All. aprantl requested review of this revision. We at first misattributed this to a CMake configuration change that happened on green dragon at the same time, hence the delay.

[Lldb-commits] [PATCH] D133618: Adapt LLDB dataformatters for libcxx change D129386

aprantl updated this revision to Diff 459214. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133618/new/ https://reviews.llvm.org/D133618 Files: lldb/include/lldb/DataFormatters/VectorIterator.h lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp lldb/source/Plugins/Language/CPlusPlus

[Lldb-commits] [PATCH] D133618: Adapt LLDB dataformatters for libcxx change D129386

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:1004 + auto dataobj = GetChildMemberWithName( + valobj, {ConstString("__data_"), ConstString("__data")}); + auto sizeobj = GetChildMemberWithName( ldionne wrote: >

[Lldb-commits] [PATCH] D133618: Adapt LLDB dataformatters for libcxx change D129386

This revision was automatically updated to reflect the committed changes. Closed by commit rG2f4a66eed688: Adapt LLDB dataformatters for libcxx change D129386 (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D133519: Document some of the clang-specific DWARF extensions supported by LLDB

This revision was automatically updated to reflect the committed changes. Closed by commit rG1f47352ce7e8: Document some of the clang-specific DWARF extensions supported by LLDB (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D133623: Fix DW_OP_convert to resolve the CU relative offset correctly.

aprantl added a comment. You would need to modify or clone the existing test https://github.com/llvm/llvm-project/blob/38ffa2bb963714cd117b6d4534d328fa6a0fb875/lldb/unittests/Expression/DWARFExpressionTest.cpp#L153 and add a second CU to it. The inlined text is just the output of obj2yaml. Rep

[Lldb-commits] [PATCH] D133623: Fix DW_OP_convert to resolve the CU relative offset correctly.

aprantl added a comment. In D133623#3784604 , @clayborg wrote: > In D133623#378 , @aprantl wrote: > >> You would need to modify or clone the existing test >> https://github.com/llvm/llvm-project/blob/38ffa2bb

[Lldb-commits] [PATCH] D133623: Fix DW_OP_convert to resolve the CU relative offset correctly.

aprantl added a comment. unless yaml2obj doesn't support multi-CU `.debug_info` sections. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133623/new/ https://reviews.llvm.org/D133623 ___ lldb-commits maili

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

aprantl added a comment. This test fails to compile on Darwin https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46816/testReport/junit/lldb-api/commands_expression_macros/TestMacros_py/ https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46811/ Can you please add a @skipIfDarwin de

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

aprantl added a comment. Out of curiosity — did you get an email notification from the bot? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130062/new/ https://reviews.llvm.org/D130062 ___ lldb-commits mai

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

aprantl added a comment. In D130062#3789858 , @kpdev42 wrote: > In D130062#3789671 , @aprantl wrote: > >> Out of curiosity — did you get an email notification from the bot? > > Hi, yes, I will add a priority to th

[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Definitely better than the previous contents! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133906/new/ https://reviews.llvm.org/D133906 ___

[Lldb-commits] [PATCH] D133670: [LLDB][RISCV] Add RVM and RVA instruction support for EmulateInstructionRISCV

aprantl added a comment. Hey @Emmmer the unit test is throwing a UBSAN failure, could you please take a look? https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/lastFailedBuild/testReport/lldb-unit/Instruction___EmulatorTests_3/26/ Stacktrace Script(shard): -- GTEST_OU

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

aprantl added a comment. Thanks! As the one who introduced this feature back in the day (and against Pavel's resistance) I've come around to seeing more value in targeted tests than a spray-paint approach of running the entire testsuite. It doesn't catch many interesting issues, since most test

[Lldb-commits] [PATCH] D133670: [LLDB][RISCV] Add RVM and RVA instruction support for EmulateInstructionRISCV

aprantl added a comment. Awesome, bot is green again! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133670/new/ https://reviews.llvm.org/D133670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

aprantl added a comment. In D134344#3805953 , @Michael137 wrote: >> teach the debug info replication to ignore tests with the gmodules category >> (just like it does for @no_debug_info_test_case tests). This step wouldn't >> be necessary if we made deb

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Seems fine if Pavel is happy with it. Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:249 def __init__(self, name=None, value=None, type=None, summary=None

[Lldb-commits] [PATCH] D134574: [lldb][test] 2 - Add gmodules test category explicitly where previously done implicitly

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134574/new/ https://reviews.llvm.org/D134574 __

[Lldb-commits] [PATCH] D134922: [lldb] Remove scoped timer from high firing and fast running SymbolFileDWARF::FindFunctions

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Probably because it just queries `m_index`. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134922/new/ https://reviews.llvm.org/D13492

[Lldb-commits] [PATCH] D134920: [lldb] Remove scoped timer from high firing and fast running ExtractUnitDIENoDwoIfNeeded

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Yeah, it does not look like there would be a lot of time spent in here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134920/new/ https://revi

[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

aprantl created this revision. aprantl added reviewers: jingham, kastiglione, labath. Herald added a project: All. aprantl requested review of this revision. When `UserExpression::Evaluate()` fails and doesn't return a ValueObject there is no vehicle for returning the error in the return value.

[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

aprantl added a comment. In case anyone is curious about the motivation — In swift-lldb `GetUserExpressionForLanguage()` can actually fail and then you get into this very situation. Even though the expression still fails, a command line user has no feedback about this. CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

aprantl added a comment. > It's a bit wrong that UserExpression::Evaluate takes an error and a > result_valobj_sp. I agree. I might be fixing that in a follow-up commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135998/new/ https://reviews.llvm.org/D135998 ___

[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

This revision was automatically updated to reflect the committed changes. Closed by commit rGa31a5da3c7d7: Make sure Target::EvaluateExpression() passes up an error instead of silently… (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG

[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

aprantl added a comment. Turns out this did have an effect on the test suite: There was one test that expected a failing expression to have an invalid SBValue and an Error: diff --git a/lldb/test/API/commands/expression/context-object/TestContextObject.py b/lldb/test/API/c ommands/expressi

[Lldb-commits] [PATCH] D136171: [lldb-tests] Remove dubious standard library flag

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This looks obviously correct. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136171/new/ https://reviews.llvm.org/D136171

[Lldb-commits] [PATCH] D136114: [lldb] Allow SymbolFileDWARFDebugMap to register multiple compile units

aprantl added a comment. Could we add a test for this that either uses full-LTO in an API test or some assembler code and and lldb-test test? Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:55 - if (debug_aranges->GetNumRanges() == num_debug_aranges) {

[Lldb-commits] [PATCH] D136114: [lldb] Allow SymbolFileDWARFDebugMap to register multiple compile units

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1270 + +for (auto &cu : m_compile_unit_infos[cu_idx].compile_units_sps) + if (cu->GetID() == dwarf_cu.GetID()) This is technically

[Lldb-commits] [PATCH] D136114: [lldb] Allow SymbolFileDWARFDebugMap to register multiple compile units

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM, with two tiny new suggestions. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h:179 +/// The compile units that an object file contains.

[Lldb-commits] [PATCH] D136306: [lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. As much as I dislike expanding our hand-rolled parser, this does seem small and important enough to warrant doing it! Comment at: lldb/source/Plugins/Language/CPlusPlus/CP

[Lldb-commits] [PATCH] D136650: Add a check for TypeSystem use-after-free problems

aprantl created this revision. aprantl added reviewers: kastiglione, jingham, labath. Herald added a project: All. aprantl requested review of this revision. When a process gets restarted TypeSystem objects associated with it may get deleted, and any CompilerType objects holding on to a reference

[Lldb-commits] [PATCH] D136650: Add a check for TypeSystem use-after-free problems

aprantl added inline comments. Comment at: lldb/source/Symbol/CompilerType.cpp:35 + bool unused; + if (GetTypeSystemGraveyard().Lookup(m_type_system, unused)) { +lldbassert(false && "CompilerType belongs to deleted Typesystem"); hawkinsw wrote: > I am sorry

[Lldb-commits] [PATCH] D136934: [lldb][FormatEntity][NFC] Move function argument parsing code into separate functions

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Core/FormatEntity.cpp:1057 + // the arguments list + if (generic && open_paren && generic < open_paren) { +int generic_depth = 1;

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Mechanically this looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132624/new/ https://reviews.llvm.org/D132624 __

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

aprantl added a comment. Ping @JDevlieghere @labath CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136650/new/ https://reviews.llvm.org/D136650 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

aprantl added a comment. Thanks for looking into this. I'm not sure I like how `TypeMatch` is both input and output of the type search. What do you think about making an immutable "TypeQuery" object that describes the search characteristics and have the API return a TypeMap? Apart from making i

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

aprantl created this revision. aprantl added reviewers: JDevlieghere, kastiglione. Herald added a project: All. aprantl requested review of this revision. Because Host::RunShellCommand runs commands through `$SHELL` there is an opportunity for this to fail spectacularly on systems that use custom

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

aprantl updated this revision to Diff 475630. aprantl edited the summary of this revision. aprantl added a comment. Implemented error handling of sorts. Because the computation result is cached and most uses are nested deep in places that have no proper error handling themselves I'm logging to t

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/CompilerType.h:58 +template TypeSystemType *dyn_cast_or_null() { + return llvm::dyn_cast_or_null(m_typesystem_sp.get()); +} labath wrote: > Maybe do something like this instead: > `

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

aprantl updated this revision to Diff 475857. aprantl added a comment. Address feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138060/new/ https://reviews.llvm.org/D138060 Files: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm lldb/unittests/Host/CMakeLists.txt lldb/unitt

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:410 + LLDB_LOG(log, "xcrun returned exit code %d", status); + return ""; +} JDevlieghere wrote: > Why not return an Error? For the caller of this lambda "the

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:13 #include "lldb/Host/macosx/HostInfoMacOSX.h" +#include "lldb/Core/Debugger.h" #include "lldb/Utility/Args.h" labath wrote: > We shouldn't have Host->Core dependencies

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/Type.h:300 - TypeSystem *GetTypeSystem(bool prefer_dynamic); + CompilerType::TypeSystemSPWrapper GetTypeSystem(bool prefer_dynamic); labath wrote: > Maybe the wrapper type should just be it

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

aprantl added a comment. In D136650#3932481 , @stella.stamenova wrote: > Looks like this broke the windows lldb bot: > https://lab.llvm.org/buildbot/#/builders/83/builds/26042 I'm trying to fix this blindly. Should hopefully work after a few iterations

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

aprantl added a comment. I think it's fixed now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136650/new/ https://reviews.llvm.org/D136650 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

aprantl added a comment. I still have a few more questions about the interface design here. May main goal is to eliminate as much mutable state and objects being passed by reference as makes sense (and no more :-). In that vain I'm trying to get rid of all Set...() methods and all places where

[Lldb-commits] [PATCH] D138259: Add the ability to see when a type in incomplete.

aprantl added inline comments. Comment at: lldb/include/lldb/API/SBType.h:212 + /// Returns true for types that were incomplete in the debug information but + /// should have been a complete. When the debugger constructs types, we must + /// have enough information to reconstr

[Lldb-commits] [PATCH] D138259: Add the ability to see when a type in incomplete.

aprantl added a comment. Generally, I think this can be useful information. I don't have any better suggestion, but I'd like to ask the room if we think that `` is a good message for the end users. (`Forward-declared type`, `type missing from debug info`, ...?) Comment at: l

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

aprantl added a comment. I think the ` void FindTypes(const TypeQuery &query, TypeResults &results);` API looks like a reasonable compromise. I have a bunch of smaller questions inside, but otherwise I think this can work. Comment at: lldb/include/lldb/Core/Module.h:435 + //

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Small nitpicks, otherwise good! Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp:41 +private: + lldb::ValueObjectSP m_range_sp = nullptr; ///< Po

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

aprantl added inline comments. Comment at: lldb/source/Target/Target.cpp:207 +// E.g., this could happen on rebuild & relaunch +// of the debugee. +m_scratch_type_system_map.Clear(); Not opposed to this, but this is leaking an implementation detail of

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

aprantl updated this revision to Diff 478762. aprantl added a comment. Herald added a subscriber: fedor.sergeev. Don't introduce a Host->Core dependency. Cache the errors and remove Progress reports (since they also depend on Core). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138060/n

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

aprantl added a comment. In D132735#3959530 , @avogelsgesang wrote: > As discussed via email, the reason for the test failures on green dragon is, > that green dragon seems to be using some compiler which passes the test > > if not (is_clang and self.

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

aprantl added inline comments. Comment at: lldb/source/Target/Target.cpp:208 +// of the debugee. +m_scratch_type_system_map.Clear(); m_process_sp.reset(); Michael137 wrote: > Michael137 wrote: > > kastiglione wrote: > > > Do we have some place in the

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp:36 + lldb::ValueObjectSP GetChildAtIndex(size_t idx) override { +// Since we only have a single child, return it +assert(idx == 0); To be very nitpi

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

aprantl added a comment. I think I'm fine with this variant modulo outstanding comments! Comment at: lldb/source/Core/ModuleList.cpp:1080 + bool ret = true; + ForEach([&](const ModuleSP &module_sp) { +ret &= callback(module_sp); kastiglione wrote: > I won

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I *think* this is what the code intended, yes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139066/new/ https://reviews.llvm.org/D139066 ___

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

aprantl added inline comments. Comment at: lldb/include/lldb/Core/Module.h:435 + /// match: "b::a", "c::b::a", "d::b::a", "e::f::b::a". + lldb::TypeSP FindFirstType(ConstString type_name, bool exact_match); clayborg wrote: > aprantl wrote: > > Why is this n

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

aprantl requested changes to this revision. aprantl added a comment. This revision now requires changes to proceed. I'll defer to @jasonmolenda Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139066/new/ https://reviews.llvm.org/D139066

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM with a slightly more relaxed test Comment at: lldb/source/Target/Target.cpp:1707 + +if (should_flush_type_systems) { + m_scratch_type_system_map.Clear();

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

aprantl added a comment. This is definitely useful — I just have one question: Isn't ForEach a special case of AnyOf? Could we implement on in terms of the other? Comment at: lldb/include/lldb/Core/ModuleList.h:480 + /// This function is thread-safe. + bool AnyOf(std::functi

[Lldb-commits] [PATCH] D139083: [lldb][Module][NFC] Add ModuleList::AnyOf

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Core/ModuleList.cpp:1079 +bool ModuleList::AnyOf( +std::function const &callback) +const { module Repository: rG LL

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

This revision was automatically updated to reflect the committed changes. Closed by commit rG33cbda4cacb8: Improve error logging when xcrun fails to execute successfully (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Mono

[Lldb-commits] [PATCH] D139082: [lldb][Module] Document ModuleList::ForEach and assert nullness

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. If we assert non-null, why still pass a shared pointer? Could we have the lambda take a `Module &`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

aprantl added a comment. I have only two concerns left: 1. IMHO it would be better if `find_one` (which I think of as a search *input*) would live in TypeQuery instead of TypeResult. It feels wrong to use TypeResult to pass something *in*. 2. It's inconsistent that Module has a FindTypes(TypeQu

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

aprantl added a comment. (or skip the test on Windows) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138724/new/ https://reviews.llvm.org/D138724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D139226: Don't mark DW_OP_addr addresses as file addresses while converting them to load addresses sometimes

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Yeah, the previous code was a hack that worked under *some* circumstances. That's not good enough to keep it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

<    3   4   5   6   7   8   9   10   11   12   >