[Lldb-commits] [PATCH] D159315: [lldb] Add OperatingSystem base class to the lldb python module

hawkinsw added a comment. I hope that some comments are helpful! The documentation that you added is tremendously helpful -- I know how documentation is sometimes a thankless task, so I will say what everyone is thinking: "THANK YOU". Comment at: lldb/examples/python/template

[Lldb-commits] [PATCH] D129338: Tell the user which pathname was invalid...

hawkinsw added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2717 if (module_file.GetDirectory().IsEmpty()) { error.SetErrorString("invalid directory name"); return false; Sorry for

[Lldb-commits] [PATCH] D131294: [LLDB][NFC] Reliability fixes to TCPSocket code

hawkinsw added a comment. Not that my opinion means much, but the changes look good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131294/new/ https://reviews.llvm.org/D131294 ___ lldb-commits mail

[Lldb-commits] [PATCH] D131312: [LLDB][NFC] Fix suspicious bitwise expression in PrintBTEntry()

hawkinsw added inline comments. Comment at: lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp:66 - if ((lbound == one_cmpl64 || one_cmpl32) && ubound == 0) { result.Printf("Null bounds on map: pointer value = 0x%" PRIu64 "\n", value); Accordin

[Lldb-commits] [PATCH] D131312: [LLDB][NFC] Fix suspicious bitwise expression in PrintBTEntry()

hawkinsw added inline comments. Comment at: lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp:66 - if ((lbound == one_cmpl64 || one_cmpl32) && ubound == 0) { result.Printf("Null bounds on map: pointer value = 0x%" PRIu64 "\n", value); DavidSpi

[Lldb-commits] [PATCH] D131983: [LLDB][NFC] Fix optons parsing and misc. reliability in CommandObjectThread

hawkinsw added inline comments. Comment at: lldb/source/Commands/CommandObjectThread.cpp:66 + case 'c': +if (option_arg.getAsInteger(0, m_count) || (m_count < 0)) { m_count = UINT32_MAX; fixathon wrote: > If getAsInteger() fails we trigger

[Lldb-commits] [PATCH] D131983: [LLDB][NFC] Fix optons parsing and misc. reliability in CommandObjectThread

hawkinsw added inline comments. Comment at: lldb/source/Commands/CommandObjectThread.cpp:66 + case 'c': +if (option_arg.getAsInteger(0, m_count) || (m_count < 0)) { m_count = UINT32_MAX; Hope this doesn't screw up Phabricator, but I just w

[Lldb-commits] [PATCH] D121800: [LLDB] Change enumaration to enumeration

hawkinsw created this revision. hawkinsw added reviewers: clayborg, zequanwu, labath. Herald added a project: All. hawkinsw requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Change enumaration to enumeration in code handling LLDB help output.

[Lldb-commits] [PATCH] D121802: [LLDB] Fix typos in LLDB help output.

hawkinsw created this revision. hawkinsw added reviewers: clayborg, zequanwu, DavidSpickett. Herald added a project: All. hawkinsw requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Correct a few spelling errors and typos in the LLDB help outp

[Lldb-commits] [PATCH] D121802: [LLDB] Fix typos in LLDB help output.

hawkinsw added a comment. Herald added a subscriber: JDevlieghere. I hope I didn't spam you all with asking for a review. It seems you are recently committers to this file. I am a long-time user, first-time contributor. I hope I am following best practices. Repository: rG LLVM Github Monorep

[Lldb-commits] [PATCH] D121800: [LLDB] Change enumaration to enumeration

hawkinsw added a comment. In D121800#3385982 , @labath wrote: > Looks good. I presume you need someone to commit this for you (?) What's the > name&email I can use for commit attribution? Thanks for the quick review!! So glad to be able to help. I do ne

[Lldb-commits] [PATCH] D121802: [LLDB] Fix typos in LLDB help output.

hawkinsw added a comment. In D121802#3386012 , @labath wrote: > I think you're doing everything right. Thanks for the fixes. I can commit > this together with the other patch. Thanks again for being so welcoming. Definitely encourages me to continue of

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

hawkinsw created this revision. hawkinsw added reviewers: JDevlieghere, teemperor. Herald added a project: All. hawkinsw requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Update the online help text for `breakpoint set` to be consistent with

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

hawkinsw added a comment. I hope that I didn't SPAM you, @teemperor and @JDevlieghere ! Thanks for all that you do for the LLVM community! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124338/new/ https://reviews.llvm.org/D124338

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

hawkinsw updated this revision to Diff 425032. hawkinsw added a comment. Updating thanks to @jdevlieghere feedback! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124338/new/ https://reviews.llvm.org/D124338 Files: lldb/source/Commands/Options.td

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

hawkinsw added a comment. Added that extra space, as requested! And, yes, I will need someone to land this one for me. I am going to ask for commit access, but so far have not! Thanks for your help, @JDevlieghere ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw created this revision. hawkinsw added reviewers: tatyana-krasnukha, jingham. Herald added a project: All. hawkinsw requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. When setting an address breakpoint using a non-section address in ll

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. First, a thank you from a long-time `gdb` user who has finally seen the light and working to switch to `lldb`! The work that you all do to build an awesome, modern debugger is amazing. As for this patch, I have committed a few typo fixes in the past, but this is my fi

[Lldb-commits] [PATCH] D124922: [lldb] Inject commands into log output when directed to file

hawkinsw created this revision. hawkinsw added reviewers: teemperor, JDevlieghere, labath. Herald added a project: All. hawkinsw requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. When logging for the lldb channel is enabled for any category a

[Lldb-commits] [PATCH] D124922: [lldb] Inject commands into log output when directed to file

hawkinsw added a comment. As always, thank you for the work that you all do on lldb! An amazing tool! It is so much fun to be able to help and I //hope// that this is a helpful addition. Please let me know whether this is something that you find useful and whether or not there are changes that

[Lldb-commits] [PATCH] D124922: [lldb] Inject commands into log output when directed to file

hawkinsw added a comment. In D124922#3491190 , @labath wrote: > Just my opinion, but I wouldn't like to complicate the logging architecture > with file-only log statements and cross-channel dependencies. If you want to > cross-reference some other loggi

[Lldb-commits] [PATCH] D124922: [lldb] Inject commands into log output when directed to file

hawkinsw added a comment. In D124922#3491214 , @labath wrote: > Yeah, I guess that what I am saying. > > For me the `commands` channel gives 6 lines of output for a command. Compared > to what some of our other logging channels do, that's nothing. Total

[Lldb-commits] [PATCH] D124922: [lldb] Inject commands into log output when directed to file

hawkinsw added a comment. In D124922#3491267 , @labath wrote: > Heh, well.. there definitely are, though a lot of things that come to mind > right now are not really suitable for a first-time contributor. Is there any > specific are that you would like

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. In D124731#3494609 , @jingham wrote: > This seems like a reasonable fallback, and the implementation looks fine. > You need to add a test case setting an address breakpoint in the executable > and making sure that works. Should

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw updated this revision to Diff 427791. hawkinsw added a comment. Updated patch including responses to helpful feedback from @jingham. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://reviews.llvm.org/D124731 Files: lldb/

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw updated this revision to Diff 427794. hawkinsw added a comment. (for real) Updating the patch based on helpful feedback from @jingham. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://reviews.llvm.org/D124731 Files: lld

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. In D124731#3498195 , @hawkinsw wrote: > (for real) Updating the patch based on helpful feedback from @jingham. I used to know better how to make sure that phabricator notified people that the commit was updated based on their f

[Lldb-commits] [PATCH] D125148: Add an example command that runs to one of a set of breakpoints

hawkinsw added a comment. Just trying to be helpful! Comment at: lldb/examples/python/cont_to_bkpt.py:26 +target = exe_ctx.target +if not exe_ctx.target.IsValid(): +result.SetError("Need a valid target") Reuse `target` here rather t

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. Sorry to bother you all (@jingham, @JDevlieghere and @labath), but I just wondered if there was anything else that I could do to spruce this up! I want to make sure that it meets everyone's expectations! It's been awesome to work on this submission! Will Repository

[Lldb-commits] [PATCH] D125434: Make a more convenient way to allow Darwin users to ignore certain Mach Exceptions

hawkinsw added a comment. I hope that those minor edits help at least a little! Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td:14 + "(e.g. 'EXC_BAD_ACCESS|EXC_BAD_INSTRUCTION'). " + "lldb will instead stop on the BSD signal the excep

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. Hello @labath and @JDevlieghere -- I hope that your Friday is going well! I'd love to make any other changes to this if necessary! Thanks for everything! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://r

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw marked 5 inline comments as done. hawkinsw added inline comments. Comment at: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:11 +def get_address_from_symbol(self, symbol): +target = lldbutil.run_to_breakpoint_make_tar

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw updated this revision to Diff 429411. hawkinsw added a comment. Updating the patchset according to the helpful feedback from @jdevlieghere. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://reviews.llvm.org/D124731 Files:

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. @JDevlieghere Thanks again for the feedback. I updated the patchset according to your comments. I left open one of your suggestions to get your feedback. Thanks again! Will Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. Just wanted to see if there was anything else I could do to help make this patch acceptable, @JDevlieghere and @jingham . Thanks again for all your help! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://r

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw updated this revision to Diff 430781. hawkinsw added a comment. Stupidly missed a few of the places where bare assertions were used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://reviews.llvm.org/D124731 Files: lldb/

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw marked 2 inline comments as done. hawkinsw added a comment. @JDevlieghere So sorry! I cannot believe I missed those. Thanks for putting up with me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://reviews.llvm.org/D124731

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. In D124731#3528269 , @JDevlieghere wrote: > Please let me know if you need someone to land this on your behalf. Unfortunately, I do! Sorry! I have not yet completed the committer process. I still feel too new. Thanks for the m

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. I am going to investigate why the buildbot failed. When I ran the tests locally everything ran fine. I am terribly, terribly sorry! @JDevlieghere -- If you could help me debug the failure, or point me to the documentation on how to read buildbot output, that would be g

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw added a comment. I realize the reason for the failure. It would appear that there are tests that are otherwise affected by the change in the behavior introduced here. I will take care of updating these tests as quickly as possible. I am sorry for the trouble, @JDevlieghere ! Reposito

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw created this revision. hawkinsw added reviewers: teemperor, JDevlieghere. Herald added a project: All. hawkinsw requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. After changing the "fallback" behavior when a user sets a breakpoint wi

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw added inline comments. Comment at: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py:43 +if region.GetRegionEnd() > illegal_address: +illegal_address = region.GetRegionEnd() DavidSpic

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw added inline comments. Comment at: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py:43 +if region.GetRegionEnd() > illegal_address: +illegal_address = region.GetRegionEnd() hawkinsw

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw updated this revision to Diff 431427. hawkinsw added a comment. Updating based on DavidSpickett helpful feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126109/new/ https://reviews.llvm.org/D126109 Files: lldb/test/API/functiona

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw added a comment. @DavidSpickett Thank you for the review! I hope that this 2nd version of the patch addresses your helpful comments! Will Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126109/new/ https://reviews.llvm.org/D126109 ___

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw added a comment. @DavidSpickett Thanks (again) for the feedback. Once I fix the nit, is the proper protocol for you to review again? I am sorry to ask but I don't know the right procedure and don't want to do the wrong thing! Comment at: lldb/test/API/functionalitie

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw updated this revision to Diff 432192. hawkinsw added a comment. Updating the revision to consider feedback (!= None vs is not None) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126109/new/ https://reviews.llvm.org/D126109 Files: lldb/

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

hawkinsw added a comment. In D126109#3535931 , @JDevlieghere wrote: > In D126109#3534522 , @hawkinsw > wrote: > >> @DavidSpickett Thanks (again) for the feedback. Once I fix the nit, is the >> proper protocol f

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

hawkinsw reopened this revision. hawkinsw added a comment. In D124731#3530692 , @omjavaid wrote: > I have reverted this temporarily as It broke LLDB API test > TestBadAddressBreakpoints.py @omjavaid We have updated that test so that this should no longe

[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.

hawkinsw added inline comments. Comment at: lldb/source/DataFormatters/FormatManager.cpp:595 + { +std::lock_guard guard(m_language_categories_mutex); +m_language_categories_map[lang_type] = Forgive me if I am speaking out of turn, but do we need to check

[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.

hawkinsw added inline comments. Comment at: lldb/source/DataFormatters/FormatManager.cpp:595 + { +std::lock_guard guard(m_language_categories_mutex); +m_language_categories_map[lang_type] = jgorbe wrote: > hawkinsw wrote: > > Forgive me if I am speaking

[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.

hawkinsw added a comment. In D126240#3564913 , @jgorbe wrote: > I've been experimenting a little bit and it seems that I can avoid the > deadlocks I was seeing by applying only the modification to > `TypeCategoryMap::Add`. This would avoid the problem t

[Lldb-commits] [PATCH] D133045: Partial fix for handling backticks in commands and aliases

hawkinsw added a comment. Sorry if these comments are not helpful! Everything looks great! Comment at: lldb/source/Interpreter/Options.cpp:1026 +} else + option_to_insert = CommandInterpreter::g_no_argument; + Question: Could we drop the final `else` i

[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

hawkinsw added inline comments. Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:46 /// just matching by comparing with m_type_name string. - bool m_is_regex; + lldb::FormatterMatchType m_match_type; I am really intrigued by this entire p

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

hawkinsw added a comment. I hope that these comments are helpful. If they are not, please feel free to tell me to stop! I appreciated learning from reading through your discussion with @labath ! Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:265 +:param d

[Lldb-commits] [PATCH] D133038: Add SBDebugger::GetSetting()/GetSettings() public APIs

hawkinsw added inline comments. Comment at: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py:25 +src_dir = self.getSourceDir() +self.runCmd('settings set target.source-map . "%s"' % src_dir) + Sorry if this comment is not helpful, b

[Lldb-commits] [PATCH] D133038: Add SBDebugger::GetSetting() public API

hawkinsw added inline comments. Comment at: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py:25 +src_dir = self.getSourceDir() +self.runCmd('settings set target.source-map . "%s"' % src_dir) + yinghuitan wrote: > hawkinsw wrote: > >

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

hawkinsw added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:1947 - // name ([%d]) to the expression path - if (m_flags.m_is_array_item_for_pointer && - epformat == eGetExpressionPathFormatHonorPointers) I am *absolutely* not an expert here

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

hawkinsw reopened this revision. hawkinsw added a comment. This revision is now accepted and ready to land. I updated this revision with the following change and I *think* that things are happy again: diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index 226

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

hawkinsw added a comment. There are *so* many reasons that I love this patch. First and foremost: I am a professor of computer science at University of Cincinnati and teaching Programming Languages this semester and I am about to teach *this very technique* for protecting against use-after-free

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

hawkinsw added a comment. If you would like, I'd be happy to reopen this patch as a separate issue if that's the better way to handle it! Sorry for the SPAM! Will Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132734/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

hawkinsw added a comment. Mr. Sabolčec, Thank you for the kind response -- I hope my work was helpful! It was lots of fun to dig in to this part of the codebase! Sincerely, Will Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132734/new/ https://r

[Lldb-commits] [PATCH] D136935: [lldb][CPlusPlus] Introduce CPlusPlusLanguage::MethodName::GetReturnType

hawkinsw added inline comments. Comment at: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp:85 // Function pointers - {"string (*f(vector&&))(float)", "", "f", "(vector&&)", "", - "f"}, - {"void (*&std::_Any_data::_M_access())()", "std::_Any_dat

[Lldb-commits] [PATCH] D137645: [trace] Add `SBTraceCursor::GetWallClockTime` API

hawkinsw added inline comments. Comment at: lldb/source/API/SBTraceCursor.cpp:131 + const auto &maybe_wall_clock_time = m_opaque_sp->GetWallClockTime(); + return maybe_wall_clock_time ? *maybe_wall_clock_time : -1.0; +} jj10306 wrote: > open to suggestions on t

[Lldb-commits] [PATCH] D137645: [trace] Add `SBTraceCursor::GetWallClockTime` API

hawkinsw added inline comments. Comment at: lldb/source/API/SBTraceCursor.cpp:127-131 +double SBTraceCursor::GetWallClockTime() const { + LLDB_INSTRUMENT_VA(this); + + const auto &maybe_wall_clock_time = m_opaque_sp->GetWallClockTime(); + return maybe_wall_clock_time ? *maybe_