[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:89 } llvm_unreachable("invalid UnitType."); } Not your code, but this application of `llvm_unreachable` seems suspicious unless we can guarantee that we a

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

aprantl added a comment. In D59235#1426716 , @JDevlieghere wrote: > Agreed, and we've been doing this for new patches for a while now. However, I > very strongly prefer having asserts over "returning a default value", which > only hides real bugs. I t

[Lldb-commits] [PATCH] D59276: Delete dead code

aprantl added a comment. One one hand this seems fine to remove, but on the other hand — won't these functions come in handy to compare and debug differences between the LLDB and LLVM DWARF parsers, while we are switching over? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59276/new/

[Lldb-commits] [PATCH] D59291: Add basic minidump support to obj2yaml

aprantl added inline comments. Comment at: include/llvm/BinaryFormat/Minidump.h:26 + +struct Header { + static constexpr uint32_t MagicSignature = 0x504d444d; // PMDM Can you please make sure to Doxygen-ify all the public headers? Repository: rL LLVM CHANGE

[Lldb-commits] [PATCH] D59314: Fix an invalid static cast in ClangExpressionParser.cpp

aprantl created this revision. aprantl added a reviewer: jingham. Herald added a project: LLDB. This was found by the green dragon sanitizer bot. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D59314 Files: lldb/include/lldb/Expression/Expression.h lldb/include/lldb/Express

[Lldb-commits] [PATCH] D59314: Fix an invalid static cast in ClangExpressionParser.cpp

aprantl updated this revision to Diff 190460. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59314/new/ https://reviews.llvm.org/D59314 Files: lldb/include/lldb/Expression/Expression.h lldb/include/lldb/Expression/FunctionCaller.h lldb/include/lldb/Expression/LLVMUserExpression.h

[Lldb-commits] [PATCH] D59314: Fix an invalid static cast in ClangExpressionParser.cpp

aprantl updated this revision to Diff 190470. aprantl added a comment. Excellent point! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59314/new/ https://reviews.llvm.org/D59314 Files: lldb/include/lldb/Expression/Expression.h lldb/include/lldb/Expression/FunctionCaller.h lldb/inc

[Lldb-commits] [PATCH] D59314: Fix an invalid static cast in ClangExpressionParser.cpp

aprantl added a comment. I was following http://www.llvm.org/docs/HowToSetUpLLVMStyleRTTI.html I will rename it to eKind... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59314/new/ https://reviews.llvm.org/D59314 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D59314: Fix an invalid static cast in ClangExpressionParser.cpp

aprantl updated this revision to Diff 190476. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59314/new/ https://reviews.llvm.org/D59314 Files: lldb/include/lldb/Expression/Expression.h lldb/include/lldb/Expression/FunctionCaller.h lldb/include/lldb/Expression/LLVMUserExpression.h

[Lldb-commits] [PATCH] D59314: Fix an invalid static cast in ClangExpressionParser.cpp

aprantl added a comment. Renamed enumerators. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59314/new/ https://reviews.llvm.org/D59314 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[Lldb-commits] [PATCH] D59370: Return llvm::Error and llvm::Expected from some DWARF parsing functions

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h:50 + + /// Extract one a + llvm::Expected half-finished comment? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59370/new/ https://reviews.

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:218 + assert(offset < cu->GetNextCompileUnitOffset() && + debug_info_data.ValidOffset(offset)); Should this be a lldb_assert() followed by `return m

[Lldb-commits] [PATCH] D59524: Improve error handling for Clang module imports

aprantl created this revision. aprantl added a reviewer: JDevlieghere. Herald added a reviewer: serge-sans-paille. Herald added a project: LLDB. aprantl edited the summary of this revision. Most notably this avoids nullptr dereferences when the import fails. rdar://problem/48883558 Repository:

[Lldb-commits] [PATCH] D59291: [Object] Add basic minidump support

aprantl added a comment. Looks like this uncovered some UB: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-sanitized/2050/testReport/junit/lldb-Suite/functionalities_postmortem_minidump-new/TestMiniDumpUUID_py/ Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5929

[Lldb-commits] [PATCH] D59667: Regression test to ensure that we handling importing of anonymous enums correctly

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/cast_int_to_anonymous_enum/main.cpp:1 +#include + You can speed up compilation time of this test significantly by removing the printf and the include of cstdio. ==

[Lldb-commits] [PATCH] D59681: Update the lldb driver to support the -O and -S options when passing --repl

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! LGTM with one tiny change to the test. Comment at: lldb/lit/Driver/TestRepl.test:2 +# RUN: echo ':quit' | %lldb -x --repl -O 'expr 42' -S %S/Inputs/Print2.in -o 'e

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

aprantl added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:181 +#else +#if defined(__APPLE__) && PY_MAJOR_VERSION == 2 +// For Darwin, the only Python version supported is the one shipped in the OS Ju

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl created this revision. aprantl added reviewers: jingham, clayborg, labath, zturner, jasonmolenda. Herald added a project: LLDB. See the changes to the website for the full rationale. Having a policy document that explains when to use what method of error handling in LLDB will make on-boa

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lldb/www/source.html:83 + Assertions. +Assertions (from assert.h) should be used liberally to assert internal consistency. +

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192548. Herald added a subscriber: arphaman. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp lldb/www/source

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192708. aprantl added a comment. Address review feedback! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. While writing this I came to the conclusion that `lldb_assert()` is really a lazy way of handling errors. If we can we should replace it with error handling and perhaps logging. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. Thanks for summarizing your thoughts, Davide. I think that what you wrote is a much better explanation of what I was trying to say with Use these sparingly and only if error handling is not otherwise feasible. I think that unless we can eliminate all uses of lldb_ass

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192730. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp lldb/www/source.html Index: lldb/www/source.html ==

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. In D59911#1446835 , @davide wrote: > > I do personally believe that your wording is reasonable. If it was me, I > would just be a little stronger and say that new code should never use > lldbassert, and if you're touching exi

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. Another problem with `lldb_assert()` is that in its current form it prints to stderr, which AFAIK is not even visible when LLDB is invoked from an IDE like Xcode. I like Pavel's suggestion of having a report_once function. It looks like everybody seems to agree that ll

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

aprantl added a comment. Since this commit, TestMiniDumpUUID_py is failing on green dragon. Could you please revert the change or commit a fix? http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastCompletedBuild/testReport/lldb-Suite/functionalities_postmortem_minidump-new/TestMiniDumpUU

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

aprantl added a comment. The bots were broken for more than six hours so I took the liberty to revert the change in r357534. Sorry for the inconvenience! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60001/new/ https://reviews.llvm.org/D60001 _

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/ObjCDataFormatterTestCase.py:21 +# Find the line number to break at. +self.line = line_number('main.m', '// Set break point at this l

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjNSException.py:30 +def nsexception_data_formatter_commands(self): +self.expect( +'frame variable except0

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/ObjCDataFormatterTestCase.py:33 +lldbutil.run_break_set_by_file_and_line( +self, "main.m", self.line, num_expected_locations=1,

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

aprantl added a comment. What's the motivation behind all the `@no_debug_info_test` decorators? Are those codepaths guaranteed to be tested elsewhere? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 ___

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

aprantl added a comment. > What's the motivation behind all the `@no_debug_info_test` decorators? Are > those codepaths guaranteed to be tested elsewhere? As a rule of thumb I'd say, let's only stick `@no_debug_info_test` on tests where a (future) coverage bot proves that it is safe. How does t

[Lldb-commits] [PATCH] D60340: Unify random timeouts throughout LLDB and make them configurable.

aprantl created this revision. aprantl added a reviewer: jingham. Herald added subscribers: kubamracek, emaste. Herald added a project: LLDB. Since these timeouts guard against catastrophic error in debugserver, I also increased all of them to the maximum value among them. The motivation for thi

[Lldb-commits] [PATCH] D60340: Unify random timeouts throughout LLDB and make them configurable.

aprantl added a comment. I should also note that I did check that process was either checked or dereferenced above the the locations where this patch dereferences it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60340/new/ https://reviews.llvm.or

[Lldb-commits] [PATCH] D60340: Unify random timeouts throughout LLDB and make them configurable.

aprantl updated this revision to Diff 193975. aprantl added a comment. Tweaked the name of the option after some off-line discussion with Jim. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60340/new/ https://reviews.llvm.org/D60340 Files: lldb/include/lldb/Target/Process.h lldb/sou

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:240 public: Minion(ClangASTImporter &master, clang::ASTContext *target_ctx, clang::ASTContext *source_ctx) Bonus points for coming up with a more descriptiv

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

aprantl added a comment. Can you send me a mail with instructions to reproduce the failure? I'd like to take a look. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59537/new/ https://reviews.llvm.org/D59537 ___ lldb-commits mailing list lld

[Lldb-commits] [PATCH] D155198: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

aprantl added a comment. As far as I'm concerned, this doesn't look too controversial to me and it is unblocking one of the bots. I think it would be okay to tentatively land this, but be on the lookout and promptly react to any post-commit feedback from @clayborg. Repository: rG LLVM Githu

[Lldb-commits] [PATCH] D155198: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

This revision was automatically updated to reflect the committed changes. Closed by commit rGf09f0a6b1076: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr (authored by fdeazeve, committed by aprantl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

aprantl added inline comments. Comment at: lldb/include/lldb/Target/Platform.h:479 +/// to an internal SDK +bool found_internal_sdk = false; + These flags really only make sense in the context of an XcodeSDK, so why not just return an XcodeSDK or XcodeSD

[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

aprantl added a comment. This LGTM, but I'm surprized you didn't have to delete any older code that tried to do the same thing. Did the thing I remember not survive the transition to tablegen or is this orthogonal? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156279/new/ https://revi

[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nevermind, you answered my question in the description already! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156279/new/ https://reviews.llvm.org/D156279 _

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1363 + XcodeSDK sdk; + for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) +if (auto

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

aprantl added inline comments. Comment at: lldb/source/Interpreter/OptionValue.cpp:55 const OptionValueBoolean *OptionValue::GetAsBoolean() const { + std::lock_guard lock(m_mutex); if (GetType() == OptionValue::eTypeBoolean) If you are following @kastiglione

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

aprantl added a comment. > If it's straightforward, I think would be nice to have a unit test with two > threads modifying the same OptionValue. That way a TSan run would catch this > issue. If that's more work than expected then this is fine as is. We might just want to set up a bot that runs

[Lldb-commits] [PATCH] D157512: [lldb][PlatformDarwin] Only parse SDK from debug-info if target language is Objective-C

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Apart from Jim's comment (does this still work?) this seems like a reasonable heuristic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157512/

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

aprantl added inline comments. Comment at: lldb/include/lldb/Core/Module.h:127 const FileSpec &file_spec, const ArchSpec &arch, - const ConstString *object_name = nullptr, - lldb::offset_t object_offset = 0, + const ConstString object_name, lldb::offset_t ob

[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

aprantl added inline comments. Comment at: lldb/source/Core/Mangled.cpp:61 + // Swift's older style of mangling used "_T" as a mangling prefix. This can + // lead to false positives with other symbols that just so happen to start Feel free to completely ignor

[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

aprantl added inline comments. Comment at: lldb/source/Core/Mangled.cpp:61 + // Swift's older style of mangling used "_T" as a mangling prefix. This can + // lead to false positives with other symbols that just so happen to start fdeazeve wrote: > JDevliegher

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

aprantl added inline comments. Comment at: lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py:54 +self.assertIn("main", + thread.GetFrameAtIndex(0).GetDisplayFunctionName()) +process.Continue() Y

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

aprantl added a comment. Test LGTM, I'll defer to others for the dynamic loader plugin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158583/new/ https://reviews.llvm.org/D158583 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D158958: [LLDB][REPL] Change the default tab size

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This should probably be a language-specific setting, but I see no problem in doing it this way right now, since there is practically only one language. Please update any REPL tests on the Swi

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

aprantl added a comment. Is this covered by any tests? Comment at: lldb/source/Core/IOHandler.cpp:512 +StringList IOHandlerEditline::GetCurrentLines() const { +#if LLDB_ENABLE_LIBEDIT + if (m_editline_up) I think this would benefit from a comment explaining th

[Lldb-commits] [PATCH] D158958: [LLDB][REPL] Change the default tab size

aprantl added a comment. > @aprantl, how should I proceed with the swift branch? Should I create a new > branch in https://github.com/apple/swift and share it here so it's available > for whoever does the merge? Actually, I can just cherry-pick it myself, let's not worry about this. Repositor

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

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:996 + TypeSummaryImplSP(new StringSummaryFormat( + eTypeOptionHideChildren | eTypeOptionHideValue, "${var.__rep_}"))); } Nice! ===

[Lldb-commits] [PATCH] D115313: [lldb/Target] Slide source display for artificial locations at function start

aprantl added a comment. Thanks! I have some late wording suggestions. Comment at: lldb/source/Target/StackFrame.cpp:1904 + strm.Printf("Warning: the current PC is an artificial location " + "in function %s.", + fn_

[Lldb-commits] [PATCH] D115461: [lldb/Target] Refine source display warning for artificial locations (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks, I think this is much easier to understand! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115461/new/ https://reviews.llvm.org/D115461

[Lldb-commits] [PATCH] D116697: [lldb] Create a property to store the REPL language

aprantl added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1777 if (auto single_lang = repl_languages.GetSingularLanguage()) { language = *single_lang; As you said in the description, this is effectively dead code. Is this to be remov

[Lldb-commits] [PATCH] D116461: [lldb] Remove ProcessStructReader from NSStringSummaryProvider (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This doesn't look any less readable than the code it replaces and is definitely faster. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116461/n

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

aprantl added inline comments. Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:76 + +self.runCmd("run", RUN_SUCCEEDED) + Doesn't `lldbutil.run_break_set_by_symbol()` do the "file" and "run" already and this run command runs it

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

aprantl added inline comments. Comment at: lldb/source/Commands/CommandObjectMemory.cpp:1695 + section_sp = section_sp->GetParent(); +section_name = section_sp->GetName(); + } Why did `section_sp->GetName();` return the Segment n

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

aprantl added a comment. > it can vary on some platforms it is signed char but on others it is bool. For those interested, Objective-C `BOOL` is a signed char on macOS and 32-bit iOS and `bool` on 64-bit iOS and derived platforms (watchOS & tvOS). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D116847: [lldb] Remove most of the reproducer instrumentation

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. With regret, I accept this patch. Too bad couldn't make it work! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116847/new/ https://reviews.llvm.org/D116847

[Lldb-commits] [PATCH] D117623: [lldb] Print an error message when we're reading libobjc.A.dylib from memory

aprantl added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2377 +stream->Printf( +"warning: libobjc.A.dylib is being read from memory instead of the %s" +"shared cache. This will likely reduce p

[Lldb-commits] [PATCH] D117623: [lldb] Print an error message when we're reading libobjc.A.dylib from memory

aprantl added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2379 +stream->Printf("warning: libobjc.A.dylib is being read from process " + "memory. This indicates that we could not %s. This will

[Lldb-commits] [PATCH] D117632: Instrument SBAPI with scoped timers

aprantl abandoned this revision. aprantl added a comment. In D117632#3255158 , @JDevlieghere wrote: > Do you actually care about the timers or is this really about getting these > methods instrumented with signposts on our platform? If it's the latter,

[Lldb-commits] [PATCH] D117632: Instrument SBAPI with scoped timers

aprantl added a comment. > I'm a little confused about what you want to achieve as well. With signposts you get a flamegraph of all nested signposted functions, which is really intuitive to look at. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117632/new/ https://reviews.llvm.org/D11

[Lldb-commits] [PATCH] D117632: [lldb] Instrument SB API with signposts

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Assuming UpdateBoundary is only called in the destructor this looks nice! Comment at: lldb/source/Utility/Instrumentation.cpp:21 +// Instrument SB API calls with singpost

[Lldb-commits] [PATCH] D118265: [lldb] Make ReadCStringFromMemory default to read from the file-cache.

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. force_live_memory=false means use the file cache iff: - the address has a Section - the section is read-only Otherwise it reads from memory. Based on that this looks safe to me. Repositor

[Lldb-commits] [PATCH] D118395: Disable TestLldbGdbServer on Dwarf2 and clang versions below 14

aprantl added a comment. In D118395#3278431 , @labath wrote: > Also, if you're running these tests on some kind of a matrix bot, maybe you > can just skip all lldb-server tests completely (dotest.py --skip-categories > llgs). Given that they don't use d

[Lldb-commits] [PATCH] D118494: [lldb] Observe SG_READ_ONLY flag in MachO binaries

aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1436 result |= ePermissionsReadable; - if (seg_cmd.initprot & VM_PROT_WRITE) + if ((seg_cmd.initprot & VM_PROT_WRITE) && !(seg_cmd.flags & SG_READ_ONLY)) result |= ePe

[Lldb-commits] [PATCH] D118395: Disable TestLldbGdbServer on Dwarf2 and clang versions below 14

aprantl added a comment. Done in zorg and reverted this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118395/new/ https://reviews.llvm.org/D118395 ___ lldb-commits mailing list lldb-commits@lists.

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl added a comment. I think this looks pretty good! I have a few questions inside. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4667 + + SmallVector Expr; + AppendAddressSpaceXDeref(AddressSpace, Expr); 13 seems to be unnecessarily high. Shouldn't 1 be

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4647 +const bool UsePointerValue) { + assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); + assert(!LexicalBlockStack.empty() && "Region stack mismatch,

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4647 +const bool UsePointerValue) { + assert(CGM.getCodeGenOpts().hasRedu

[Lldb-commits] [PATCH] D119167: [lldb/test] Remove sleeps from some lldb-server tests

aprantl added a comment. This looks amazing! Thanks, Pavel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119167/new/ https://reviews.llvm.org/D119167 ___ lldb-commits mailing list lldb-commits@lists.llv

[Lldb-commits] [PATCH] D120105: Remove recursive include of GDBRemoteCommunicationServerCommon.h

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

[Lldb-commits] [PATCH] D119915: Replace use of double underscore in identifiers

aprantl added a comment. Herald added a subscriber: JDevlieghere. Nice. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:261 if (m_pair_ptr) { - auto __i_(valobj_sp->GetChildMemberWithName(g___i_, true)); + auto __i_(valobj_sp->GetChildMemberWithName

[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/ObjectFile.h:676 + const char *GetCStrFromSection(Section *section, + lldb::offset_t section_offset) const; Can you add a doxygen comment for the method?

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

aprantl added a comment. This is pretty (and) awesome. Comment at: lldb/source/Core/Debugger.cpp:1670 +if (event_type & Debugger::eBroadcastBitProgress) + HandleProgressEvent(event_sp); } side note: this function could benefi

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

aprantl added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1757 + File &output = GetOutputFile(); + if (!output.GetIsTerminalWithColors() || !GetShowProgress()) +return; And withColors also implies that it's an interactive TTY? CHANGES SINCE

[Lldb-commits] [PATCH] D121131: [lldb] Support "bright" ANSI colors

aprantl added inline comments. Comment at: lldb/include/lldb/Utility/AnsiTerminal.h:12 + #define ANSI_FG_COLOR_BLACK 30 #define ANSI_FG_COLOR_RED 31 Nice! Should we convert this to enums? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[Lldb-commits] [PATCH] D121062: [lldb] Add a setting to change the progress ANSI escape codes

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Core/CoreProperties.td:141 +DefaultStringValue<"${ansi.faint}">, +Desc<"When displaying progress in a color-enabled (i.e. ANSI) terminal

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

aprantl added a comment. This is nice, I have a few mostly stylistic comments inline. Comment at: lldb/source/Expression/DWARFExpression.cpp:947 +llvm::Optional +ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp, Maybe add a Doxyge

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

aprantl added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s:1 +# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s +# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s Out of curiosity:

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! Comment at: lldb/source/Expression/DWARFExpression.cpp:962 +/// check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a +/// success if so_

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:637 if (location_sp->GetName() == g_size_name) - location_sp = short_sp->GetChildAtIndex(3, true); + location_sp = short_sp->GetChildAtIndex(2, true); if (using_bitmas

[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nice work! I only had superficial comments, maybe let's also wait for @jingham to accept the patch. Comment at: lldb/include/lldb/Expression/UserExpression.h:283 + stat

[Lldb-commits] [PATCH] D129364: [LLDB][DataFormatter] Add data formatter for libcxx std::unordered_map iterator

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nice! Few minor stylistic comments inside. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:403 + + // this must be a ValueObject* because it is a child of th

[Lldb-commits] [PATCH] D129367: [LLDB][ClangExpression] Remove unused StructVars::m_object_pointer_type

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nice cleanup, thanks! Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h:356 struct StructVars { -StructVars() : m_result_name(), m_obj

[Lldb-commits] [PATCH] D130213: [LLDB][ClangExpression] Fix initialization of static enum alias members

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. @werat did you build with assertions enabled? This change looks fine to me, but it might be interesting to see where the discrepancy between the platforms comes from. Com

[Lldb-commits] [PATCH] D129962: [LLDB][DataFormatter] Add support for std::__map_const_iterator

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/map/main.cpp:30 +intint_map::const_iterator const_it = ii.cbegin(); +std::pr

[Lldb-commits] [PATCH] D130561: [LLDB][ClangExpression] Prevent nullptr namespace map access during logging

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/D130561/new/ https://reviews.llvm.org/D130561

[Lldb-commits] [PATCH] D130540: [lldb] Read from the Rosetta shared cache with Xcode 14

aprantl added inline comments. Comment at: lldb/test/API/macosx/rosetta/TestRosetta.py:7-10 +def apple_silicon(test): +return platform.system() == 'Darwin' and test.getArchitecture() in [ +'arm64', 'arm64e' +] mib wrote: > Why not add this to `dec

[Lldb-commits] [PATCH] D130803: [lldb] Allow SymbolTable regex search functions to match mangled name

aprantl added a comment. It sounds like it should be possible to test this with `lldb-test`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130803/new/ https://reviews.llvm.org/D130803 ___ lldb-commits ma

[Lldb-commits] [PATCH] D131025: Document why test is disabled on macos ventura

aprantl created this revision. aprantl added reviewers: JDevlieghere, jasonmolenda. Herald added a project: All. aprantl requested review of this revision. Starting with macOS 13 CoreFoundation pulls in Foundation. rdar://96224141 https://reviews.llvm.org/D131025 Files: lldb/test/API/lang/ob

[Lldb-commits] [PATCH] D131033: [lldb/crashlog] Fix parsing issue with `procPath`

aprantl added a comment. Could we test this by removing the key from an existing testcase? Comment at: lldb/examples/python/crashlog.py:473 +if 'procPath' in json_data: +self.crashlog.process_path = json_data['procPath'] Is it now undefine

[Lldb-commits] [PATCH] D131025: Document why test is disabled on macos ventura

This revision was automatically updated to reflect the committed changes. Closed by commit rG11e5275cc266: Document why test is disabled on macOS Ventura (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[Lldb-commits] [PATCH] D131110: [lldb] Make LLDB resilient against failing dyld introspection APIs

aprantl added a comment. Nice. Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:533 if (dyld_process_create_for_current_task) { - auto dyld_process = dyld_process_create_for_current_task(); - auto snapshot = - dyld_process_snapshot_create_for

<    1   2   3   4   5   6   7   8   9   10   >