[Lldb-commits] [PATCH] D89315: [debugserver] Add option to propagate SIGSEGV to target process

jingham added a comment. Thanks, that's a little better. It would be even nicer if the DNB API's you are adding "unmask_signals" to would take the Context instead. At this point they are pulling the launch flavor and the unmask signals from the context, so just getting the RNBContext would be

[Lldb-commits] [PATCH] D89358: Add an API to get an SBBreakpoint's owning SBTarget

jingham updated this revision to Diff 298210. jingham added a comment. Use lldb-instr instead of hand-generating the reproducer wrappers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89358/new/ https://reviews.llvm.org/D89358 Files: lldb/bindin

[Lldb-commits] [PATCH] D89315: [debugserver] Add option to propagate SIGSEGV to target process

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That's nicer, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89315/new/ https://reviews.llvm.org/D89315 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D89358: Add an API to get an SBBreakpoint's owning SBTarget

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6754caa9bf21: Add an SB API to get the SBTarget from an SBBreakpoint (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[Lldb-commits] [PATCH] D89589: [lldb] Implement ObjCExceptionThrowFrameRecognizer::GetName()

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89589/new/ https://reviews.llvm.org/D89589 ___

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

jingham added a comment. In D68130#1686530 , @shafik wrote: > So if I look at `ClangASTContext::AddMethodToCXXRecordType(...)` it has the > following: > > if (is_artificial) > return nullptr; // skip everything artificial > > > but why? if I look

[Lldb-commits] [PATCH] D68173: Report a useful error when a the user mistypes a class name in making a script ThreadPlan

jingham created this revision. Herald added subscribers: lldb-commits, JDevlieghere, abidh. Herald added a project: LLDB. Before this patch if you used SBThread.StepUsingScriptedThreadPlan but mistyped the Python ThreadPlan class name, you wouldn't get any error, and a bogus ThreadPlan would get

[Lldb-commits] [PATCH] D68173: Report a useful error when a the user mistypes a class name in making a script ThreadPlan

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL373135: Give an error when StepUsingScriptedThreadPlan is passed a bad classname. (authored by jingham, committed by ). He

[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex

jingham created this revision. jingham added a reviewer: clayborg. Herald added subscribers: lldb-commits, JDevlieghere. Herald added a project: LLDB. When we do synchronous process execution in lldb, the call that restarted the target will generally have acquired first the TargetAPI lock, and th

[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex

jingham added a comment. Note, a better way to do this would be to have some kind of "API/Runlock baton" that could get handed off from the thread that initiated the continue to the private state thread, then handed back when the process stops and returns control to the SB API that initiated th

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

jingham added a comment. It would be good as Pavel and Davide suggest to write a test directly for the JSON parser. Doing so in the C++ Unit test seems the most convenient, but that's up to you. But it would also be good to add a test for the particular "breakpoint write" -> "breakpoint read"

[Lldb-commits] [PATCH] D67793: new api class: SBFile

jingham added a comment. This looks fine to me, it doesn't look like you are leaving out anything useful. Note, I think just to enforce the "default constructor" discipline, there's a test in the test suite (python_api/default-constructor/TestDefaultConstructorForAPIObjects.py) that has a file

[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL373280: Allow the internal-state-thread free access to the TargetAPI mutex. (authored by jingham, committed by ). Herald a

[Lldb-commits] [PATCH] D67831: [lldb] Get the TargetAPI lock in SBProcess::IsInstrumentationRuntimePresen

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67831/new/ https://reviews.llvm.org/D67831 ___ lldb-commi

[Lldb-commits] [PATCH] D68363: Segregate the Python class + key/value dictionary into a separate OptionGroup

jingham created this revision. jingham added reviewers: clayborg, teemperor. Herald added subscribers: lldb-commits, JDevlieghere, abidh, mgorny. Herald added a project: LLDB. I want to reuse these options (break set's -P -k -v) in another command (thread step-scripted) in a future commit, so I b

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

jingham created this revision. jingham added reviewers: labath, JDevlieghere, aprantl. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. jingham added a parent revision: D68363: Segregate the Python class + key/value dictionary into a separate OptionGroup. It is trivial to wr

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

jingham updated this revision to Diff 222952. jingham added a comment. Missed a bit that removed m_class_name from the old command options (and didn't use the new option.) Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68366/new/ https://reviews.llvm.org/D68366

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

jingham updated this revision to Diff 223096. jingham added a comment. Added the missing REGISTER macros Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68366/new/ https://reviews.llvm.org/D68366 Files: lldb/include/lldb/API/SBStructuredData.h lldb/include/l

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

jingham marked 4 inline comments as done. jingham added inline comments. Comment at: lldb/include/lldb/Target/ThreadPlanPython.h:35 + ThreadPlanPython(Thread &thread, const char *class_name, + StructuredDataImpl *args_data); ~ThreadPlanPython() override; --

[Lldb-commits] [PATCH] D68363: Segregate the Python class + key/value dictionary into a separate OptionGroup

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL373673: Break out the Python class & key/value options into a separate OptionGroup. (authored by jingham, committed by ).

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

This revision was automatically updated to reflect the committed changes. Closed by commit rL373675: Pass an SBStructuredData to scripted ThreadPlans on use. (authored by jingham, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: htt

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

jingham added a comment. When one or the other side of the if/else test is trivial, then this rewrite is fine. When both the if and the else have a decent bit of code in them, however, removing the "} else {" and the concluding "}" obscures the fact that these are two coequal branches in the c

[Lldb-commits] [PATCH] D68545: DWIMy filterspecs for dotest.py

jingham added a comment. This is great, and will make the -f option much easier to use. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68545/new/ https://reviews.llvm.org/D68545 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D68546: remove FILE* usage from ReportEventState() and HandleProcessEvent()

jingham added a comment. SBProcess::ReportEventState was introduced in r112221, and SBStream was added in r114188, so Pavel's speculation seems like a reasonable one, though that was 9 years ago... But in the SB API's we use SBStream in a bunch of places to be more like an SBString, something

[Lldb-commits] [PATCH] D68606: [test] Split LLDB tests into filecheck, unittest and dotest.

jingham added a comment. This seems much clearer to me. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68606/new/ https://reviews.llvm.org/D68606 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lis

[Lldb-commits] [PATCH] D68546: remove FILE* usage from ReportEventState() and HandleProcessEvent()

jingham added a comment. In D68546#1699390 , @labath wrote: > Thanks for weighing in, Jim. My responses are inline. > > In D68546#1698430 , @jingham wrote: > > > SBProcess::ReportEventState was introduced in r112221

[Lldb-commits] [PATCH] D68661: StopInfo/Mach: Delete PPC support

jingham added a comment. Yes, I don't think we need to support this anymore. Comment at: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp:387 case 1: // EXC_BAD_ACCESS break; Might as well remove this break too. CHANGES SINCE LAST ACTIO

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham created this revision. jingham added reviewers: JDevlieghere, clayborg. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. For example, it is pretty easy to write a breakpoint command that implements "stop when my caller is Foo", and it is pretty easy to write a breakp

[Lldb-commits] [PATCH] D68696: [lldb][NFC] Remove strange bool parameter from Searcher::SearchCallback

jingham accepted this revision. jingham added a comment. One of the oddities of the Search -> Search callback stuff is that the Searcher's Filter might be narrower than the level at which the Search Callback wants to be invoked. The former is whatever the user actually wants limit the search t

[Lldb-commits] [PATCH] D68696: [lldb][NFC] Remove strange bool parameter from Searcher::SearchCallback

jingham added a comment. I have no memory of adding the 'addr' parameter. If you had a search that was guaranteed to only return one address per invocation, this would allow you to have the searcher coordinate gathering the results, rather than having the callback record them on its own. That

[Lldb-commits] [PATCH] D68750: Implement serialization and deserialization of scripted points

jingham created this revision. jingham added a reviewer: teemperor. Herald added subscribers: lldb-commits, JDevlieghere. Herald added a project: LLDB. This allows you to use "break read" and "break write" for scripted breakpoints. This is the test Raphael was offering to have Jan write in, plus

[Lldb-commits] [PATCH] D68750: Implement serialization and deserialization of scripted points

This revision was automatically updated to reflect the committed changes. Closed by commit rG47b33dcc0df8: Implement serializing scripted breakpoints and their extra args. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68750/new

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham added a comment. In D68671#1704803 , @clayborg wrote: > Looks good overall. A few questions: > > - should we do a global "s/extra_args/args/" edit in this patch? Not sure > what "extra_" is adding? Can we remove? Since we still support the form

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham updated this revision to Diff 224719. jingham added a comment. I added ScriptInterpreter::GetNumArgumentsForCallable, so I can base all the decisions on how to call the function on whether I was passed a function with 3 or 4 arguments. That's cleaner. I also plumbed an error through th

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham updated this revision to Diff 224929. jingham added a comment. Addressed Pavel's comments. I explicitly want the number of fixed arguments, so I changed the function name to GetNumFixedArguments. The docs say explicitly that you have to make the function take either three or four fixed

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham marked 2 inline comments as done. jingham added inline comments. Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469 + + virtual int GetNumArgumentsForCallable(const char *callable_name) { +return -1; lawrence_danna wrote: > labath w

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469 + + virtual int GetNumArgumentsForCallable(const char *callable_name) { +return -1; labath wrote: > labath wrote: >

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469 + + virtual int GetNumArgumentsForCallable(const char *callable_name) { +return -1; jingham wrote: > labath wrote: >

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469 + + virtual int GetNumArgumentsForCallable(const char *callable_name) { +return -1; labath wrote: > jingham wrote: >

[Lldb-commits] [PATCH] D69014: [LLDB] bugfix: command script add -f doesn't work for some callables

jingham added a comment. IIUC, the only external change in this patch is that before if you implemented your Python command using a class with an `__call__` method, it would have to be the signature that took exe_ctx. Since this is now switching off of the number of arguments (less self) you

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469 + + virtual int GetNumArgumentsForCallable(const char *callable_name) { +return -1; labath wrote: > jingham wrote: >

[Lldb-commits] [PATCH] D68968: [android/process info] Introduce bundle id

jingham added a comment. As Greg said, iOS (and macOS as well, though less directly) have the notion of bundleID. At present, lldb doesn't directly use/figure out the bundle ID, though it could either from the binary itself or from debugserver, which does have to know that. As far as I know w

[Lldb-commits] [PATCH] D69273: ValueObject: Fix a crash related to children address type computation

jingham added a comment. Except for the comment comment this looks fine. I think the Host -> Load address code isn't quite right, though it looks like that's not your doing. The main reason why you would copy a ValueObject into Host memory it is to freeze-dry it as a ConstResult. Since that i

[Lldb-commits] [PATCH] D69422: [lldb][Docs] Add extra lldb aliases to gdb->lldb map

jingham added a comment. I have no objection to documenting the aliases we have. Having the ability to define shortcuts for common operations was one of the key pieces that allowed the straight command set to be laid out in an orderly fashion, which I think is great for discoverability. It's

[Lldb-commits] [PATCH] D69425: [lldb] Fix broken -D option for breakpoint set command

jingham added a comment. Ack, yes. That's definitely wrong. Could you write a test for this to make sure we don't break it in the future. This should fail if you make one target, set a breakpoint with the -D option, then make a second target and check that it has the breakpoint, so it should

[Lldb-commits] [PATCH] D69453: Modernize TestThreadStepOut.py

jingham created this revision. Herald added subscribers: lldb-commits, JDevlieghere, abidh. Herald added a project: LLDB. This test was timing out on the bots occasionally, so I was poking around at it. I didn't see any obvious reason why it would stall, but it had a slightly odd way of rendezv

[Lldb-commits] [PATCH] D69453: Modernize TestThreadStepOut.py

jingham updated this revision to Diff 226772. jingham added a comment. Addressed review comments. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69453/new/ https://reviews.llvm.org/D69453 Files: lldb/packages/Python/lldbsuite/test/functionalities/thread/step

[Lldb-commits] [PATCH] D69453: Modernize TestThreadStepOut.py

jingham marked 4 inline comments as done. jingham added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py:136 +(self.inferior_target, self.inferior_process, thread, bkpt) = lldbutil.run_to_source_breakp

[Lldb-commits] [PATCH] D69453: Modernize TestThreadStepOut.py

This revision was automatically updated to reflect the committed changes. Closed by commit rG651b5e725ee6: Modernize TestThreadStepOut.py (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69453/new/ https://reviews.llvm.org/D69453

[Lldb-commits] [PATCH] D69425: [lldb] Fix broken -D option for breakpoint set command

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

[Lldb-commits] [PATCH] D69738: Fix handling for the clang name mangling extension for block invocations

jingham added inline comments. Comment at: lldb/source/Core/Mangled.cpp:99 + + if (s.size() >= 2 && (s[0] == '_' && s[1] == 'Z')) +return Mangled::eManglingSchemeItanium; StringRef has a startswith. That might be easier to read. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D69273: ValueObject: Fix a crash related to children address type computation

jingham added a comment. In Swift you have to ask the runtime for most of the layout details of objects so getting a const result object, stepping, then asking it to reevaluate itself would lead to passing the runtime incorrect data and potentially undoing a correct type decision that you had m

[Lldb-commits] [PATCH] D69425: [lldb] Fix broken -D option for breakpoint set command

jingham added a comment. In D69425#1731155 , @poya wrote: > No commit access, so would need some assistance getting this change into the > repo. I pushed this (f1539b9db39a59a5e50c065c39e491ba4f890377

[Lldb-commits] [PATCH] D69425: [lldb] Fix broken -D option for breakpoint set command

This revision was automatically updated to reflect the committed changes. Closed by commit rGf1539b9db39a: BreakpointDummyOptionGroup was using g_breakpoint_modify_options rather than… (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [PATCH] D70002: [lldb] Make Target* a Target& in CommandObjectExpression::DoExecute REPL logic

jingham added a comment. As long as target.GetREPL (..., true) returns a sensible error for the Dummy target, this is fine. Actually having the dummy target run the REPL seems like a bad idea, since that's a pretty heavy-weight activity and the dummy target is supposed to be just for priming s

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

jingham added a comment. Why not have the FindFunctions lambda take a FunctionSP? It would be easy to get the Function name out of the function in the lambda, and that would make the function more general (and also match what the Foreach does... CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D70002: [lldb] Make Target* a Target& in CommandObjectExpression::DoExecute REPL logic

jingham added a comment. I don't think either version is right. We really shouldn't be using "GetSelectedTarget" (with or without the Dummy) for getting the Target context for a command. That's how it was originally done, but you aren't guaranteed that the selected target is the right target

[Lldb-commits] [PATCH] D69704: [lldb] Add IsTypeSystemCompatible method to SBModule to allow checking compatibility between language versions

jingham added a comment. In D69704#1732484 , @labath wrote: > The part that bothers me here is that this assumes that the entirety of a > module (or an least the portion of it written in the same language) uses the > same version of the language. Is that

[Lldb-commits] [PATCH] D69704: [lldb] Add IsDebugInfoCompatible method to SBModule to allow checking compatibility between language versions

jingham added a comment. In D69704#1742494 , @labath wrote: > I think this is fine. > > In D69704#1741588 , @jingham wrote: > > > If, for instance, we decided to use .pcm files as a serialization form for > > type

[Lldb-commits] [PATCH] D70134: dotest: Add a way for the run_to_* helpers to register dylibs

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This is fine. I wondered a bit about whether it would be generally useful to add the 'dylibs that have to be copied' to the SBLaunchInfo? It has some other platform'y like things. I'm not

[Lldb-commits] [PATCH] D70127: [lldb-vscode] Fix a race in test_extra_launch_commands

jingham added a comment. It looks like all the continue routines in lldbvscode_testcase.py use self.vscode.wait_for_stopped() after setting the target going. Would that fix the problem here? Without knowing anything about the protocol VSCode is using, I can't say that's right. But if launch

[Lldb-commits] [PATCH] D70134: dotest: Add a way for the run_to_* helpers to register dylibs

jingham added a comment. In D70134#1743075 , @clayborg wrote: > In D70134#1742647 , @jingham wrote: > > > This is fine. > > > > I wondered a bit about whether it would be generally useful to add the > > 'dylibs tha

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

jingham marked an inline comment as done. jingham added inline comments. Comment at: source/Core/SearchFilter.cpp:757 + } + if (m_cu_spec_list.FindFileIndex(0, sym_ctx.comp_unit, false) == UINT32_MAX) +return false; // Fails the file check kwk wrote: >

[Lldb-commits] [PATCH] D70281: Fix -Wunused-result warnings in LLDB

jingham added a comment. There's a PP above each instance of this code explaining why it's okay if we don't get the API lock, since that's a bit of a tricky point. So if you want to add a comment, it would be better to say "see explanation above". Repository: rG LLVM Github Monorepo CHANGE

[Lldb-commits] [PATCH] D70272: [-gmodules] Let LLDB log a warning if the Clang module hash mismatches.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Host/common/Host.cpp:300 va_end(args); + + // Log to log channel. This allows testcases to grep for log output. On macOS,

[Lldb-commits] [PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)

jingham added a comment. IIUC, LLDB_LOG already adds the file and function. A bunch of these logs are also adding __FUNCTION__, so that's probably going to come out twice now. You should probably remove the duplicates. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Actually, I don't want this change as is. Some logs - like the expression and step logs - are laid out for readability, and LLDB_LOG automatically adds the file & function which w

[Lldb-commits] [PATCH] D65152: Fix issues with inferior stdout coming out of order

jingham added a comment. I agree with Greg, having one function that can do any of the combinations of stdout & stderr seems more convenient. Other than that, this is fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65152/new/ https://reviews.llvm.org/D65152

[Lldb-commits] [PATCH] D65165: [Symbol] Fix some botched logic in Variable::GetLanguage

jingham added a comment. In D65165#1598553 , @labath wrote: > Thanks for responding quickly. LGTM, with one comment.. Don't use checks that assume eLanguageTypeUnknown == 0, that seems really confusing and we don't use that practice anywhere else. Pave

[Lldb-commits] [PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Sure, this is okay. Comment at: lldb/source/Expression/IRExecutionUnit.cpp:601 if (log) { +LLDB_LOGF(log, labath wrote: > looks like there are som

[Lldb-commits] [PATCH] D65109: [LLDB] Remove the Xcode project

jingham added a comment. In D65109#1598695 , @sgraenitz wrote: > > We discussed this and came to an agreement only a few hours before in the > > team meeting > > After all, LLVM is open-source and has a community. Making preemptive > decisions in interna

[Lldb-commits] [PATCH] D65293: Document the fact that LLDB_LOG uses llvm::format_providers

jingham created this revision. jingham added a reviewer: labath. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is a handy and powerful addition to the logging system, but not at all discoverable. This is a trivial patch, but since I don't actually know much about th

[Lldb-commits] [PATCH] D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC)

jingham added inline comments. Comment at: lldb/source/Expression/IRExecutionUnit.cpp:601 if (log) { +LLDB_LOGF(log, labath wrote: > jingham wrote: > > labath wrote: > > > looks like there are some `if(log)`s still remaining. Maybe the `{}` > > > around

[Lldb-commits] [PATCH] D65271: Increase testsuite packet-timeout 5secs -> 5mins

jingham added a comment. There isn't an SB API for "settings set" yet. I've put off doing that because the "settings" subsystem is currently unfinished. In the original design you would be specify qualifiers in the setting hierarchy, like: settings set target[arch=x86_64].whatever That would

[Lldb-commits] [PATCH] D65293: Document the fact that LLDB_LOG uses llvm::format_providers

jingham added a comment. Yeah, I didn't want to bother with formatting in case there were suggestions on content. I checked in a version that's correctly truncated. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65293/new/ https://reviews.llvm.org/D65293 __

[Lldb-commits] [PATCH] D65293: Document the fact that LLDB_LOG uses llvm::format_providers

This revision was automatically updated to reflect the committed changes. Closed by commit rL367132: Document that LLDB_LOG macros use the format_providers. (authored by jingham, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: http

[Lldb-commits] [PATCH] D65122: [Symbol] Use llvm::Expected when getting TypeSystems

jingham added a comment. It seems to me part of the goal of Alex's efforts is to make it possible for a given lldb to have or not have support for any particular type system. In some future day they might even be live pluggable, so that which ones get loaded would be a user choice. In that fu

[Lldb-commits] [PATCH] D65311: [dotest] Remove multiprocessing

jingham added a comment. This seems okay to me. I would also check with Jason. I don't know who coordinates running tests remotely - or even if they can run in parallel now. But that's the one bit I don't understand well enough to say yea or nay on. CHANGES SINCE LAST ACTION https://revie

[Lldb-commits] [PATCH] D65386: [lldb][NFC] Use an enum instead of chars when handling options [WIP]

jingham added a comment. It worries me a little bit that we are making it harder and harder to figure out "where does the option for "-t" get stored once this CommandObject's options have been parsed. Can you show the steps I would have to go through to get from "-f" to OptionEnumSettingsSet::

[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

jingham added a comment. It might be good to see if you can trigger the bug more directly (for instance by getting the SBType for the method and pulling out its arguments?) The reason that a breakpoint triggers this at present is that lldb reads the debug info for the function to find the prol

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

jingham added a comment. The thing the "bugreport unwind" adds is that it runs a handful of commands that gather data useful in diagnosing unwind problems. That's useful when the information you need might not be output by the session in which the bug appeared. The same thing could be achieve

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

jingham added a comment. If we don't want to forget that there was specific useful info, we could gate this patch on implementing the "reproducer generate " feature. That should be easy to add if we're clear this this is the right design for reproducers. But I don't think that "bugreport unwi

[Lldb-commits] [PATCH] D65489: Format OptionEnumValueElement

jingham added a comment. What happens if you clang-format your reformatted version of the setter? Do we need clang-format off for them? Otherwise this looks fine to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65489/new/ https://reviews.llvm.org/D65489 ___

[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

jingham added a comment. If you just want the path and don't need the FileSpec, you can call the static SBFileSpec::ResolvePath. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65611/new/ https://reviews.llvm.org/D65611 ___ lldb-commits mail

[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

jingham added a comment. IIRC both GetPath and ResolvePath return the number of characters it would take to actually resolve the path. 128 is actually pretty short, so you should check the result and malloc a buffer of the return + 1 if 128 ends up not being enough. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D65489: Format OptionEnumValueElement

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Okay, if clang-format won't undo this, then I'm fine with writing them this way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65489/new/ https://reviews.llvm.org/D65489

[Lldb-commits] [PATCH] D65555: [lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP]

jingham added inline comments. Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:737 + for (auto it = m_threads.begin(); it != m_threads.end(); ++it) { +if (*it && ((*it)->GetID() == thread_id)) { + m_threads.erase(it); mgorny wrote

[Lldb-commits] [PATCH] D65646: [lldb] Print better diagnostics for user expressions.

jingham added a comment. In D65646#1612436 , @aprantl wrote: > I have a question based on my half-knowledge about the expression evaluator: > I thought that we already wrote out a temporary file for the expression in > order to support `expr -g`? Is this

[Lldb-commits] [PATCH] D65646: [lldb] Print better diagnostics for user expressions.

jingham added a comment. That would be fine. In the swift REPL, we increment the line number so that it appears that all the expressions are one contiguous source file. But I think for the expression parser treating each expression as a separate file is a better fiction. Repository: rLLDB

[Lldb-commits] [PATCH] D65682: Give a little more info when "run_to_x_breakpoint" fails

jingham created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. We failure was a little bit terse. Say whether we exited, hit another breakpoint/crashed, etc. Repository: rLLDB LLDB https://reviews.llvm.org/D65682 Files: lldb/packages/Python/lldbsuit

[Lldb-commits] [PATCH] D65797: [lldb][CMake] Generating Xcode projects

jingham added a comment. Thanks for doing this. Looks okay to me just one typo... But I don't know CMake well enough to comment on substance... Comment at: lldb/cmake/modules/LLDBConfig.cmake:78 + "When building with Xcode, we recommend using the corresponding cache sc

[Lldb-commits] [PATCH] D65691: Various build fixes for lldb on MinGW

jingham added inline comments. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:55 #define PATH_MAX MAX_PATH +#endif typedef int socklen_t; labath wrote: > amccarth wrote: > > hhb wrote: > > > amccarth wrote: > > > > Nothing in the rest of this .cpp file uses

[Lldb-commits] [PATCH] D65611: [Driver] Expand the executable path in the target create output

jingham added a comment. Given that we do some work to find executables (like search along the path for "ls" -> /bin/ls") which work might not end up with the result you expected, printing the full path seems useful. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65611/new/ https://rev

[Lldb-commits] [PATCH] D56010: [NativePDB] Fix setting breakpoint by file and line

jingham added a comment. In D56010#1623762 , @amccarth wrote: > It looks like the code changes landed (probably as part of another patch) but > not the tests. I'll look and see if the tests should to be revived. I can't see that any of these tests test

[Lldb-commits] [PATCH] D65682: Give a little more info when "run_to_x_breakpoint" fails

jingham updated this revision to Diff 214865. jingham added a comment. Fixed the inverted call order. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65682/new/ https://reviews.llvm.org/D65682 Files: packages/Python/lldbsuite/test/lldbutil.py Index: packages

[Lldb-commits] [PATCH] D65682: Give a little more info when "run_to_x_breakpoint" fails

jingham added a comment. Thanks for pointing that out. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65682/new/ https://reviews.llvm.org/D65682 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://list

[Lldb-commits] [PATCH] D66174: [Utility] Reimplement RegularExpression on top of llvm::Regex

jingham added a comment. I think we need to fix the behavior for "", other than that, this looks fine to me. Comment at: lldb/source/Host/common/Socket.cpp:287 + if (g_regex.Execute(host_and_port, &matches)) { +if (matches.size() >= 3) { + host_str = matches[1].str()

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

jingham created this revision. jingham added a reviewer: clayborg. Herald added subscribers: lldb-commits, abidh. Herald added a project: LLDB. Step hooks were not firing when you did a "step out". This was because, when a breakpoint's PerformAction observes that the breakpoint has completed a p

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

jingham marked an inline comment as done. jingham added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/stop-hooks/Makefile:4 +C_SOURCES := main.c +CFLAGS_EXTRAS += -std=c99 + davide wrote: > Forgive my ignorance, but why you need this

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

jingham marked an inline comment as done. jingham added a comment. Do you think I should put more verbiage in the comment, or would this have been clear if you were actually working on the code? Comment at: source/Target/StopInfo.cpp:550 thread_sp->ResetStopInfo(); +

<    10   11   12   13   14   15   16   17   18   19   >