Page MenuHomePhabricator

jingham (Jim Ingham)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 23 2014, 5:24 PM (260 w, 5 d)

Recent Activity

Fri, Sep 20

jingham requested changes to D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects.

This is all about the decision in "batch" mode for what constitutes a stop by the program that should cause batch mode to return the control to the user.

Fri, Sep 20, 11:18 AM · Restricted Project

Tue, Sep 17

jingham committed rGd6cad3931635: Clean up this test. (authored by jingham).
Clean up this test.
Tue, Sep 17, 6:55 PM
jingham committed rL372196: Clean up this test..
Clean up this test.
Tue, Sep 17, 6:52 PM
jingham committed rGf547cf12ee57: TestFoundationDisassembly.py is not dependent on debug information. (authored by jingham).
TestFoundationDisassembly.py is not dependent on debug information.
Tue, Sep 17, 5:42 PM
jingham committed rL372193: TestFoundationDisassembly.py is not dependent on debug information..
TestFoundationDisassembly.py is not dependent on debug information.
Tue, Sep 17, 5:42 PM
jingham accepted D67685: [ScriptInterpreter] Make sure LLDB's global variables are only available in interactive mode..

This looks right.

Tue, Sep 17, 5:10 PM · Restricted Project

Mon, Sep 16

jingham committed rG66e9f239b5b3: Revert "[lldb][NFC] Make ApplyObjcCastHack less scary" (authored by jingham).
Revert "[lldb][NFC] Make ApplyObjcCastHack less scary"
Mon, Sep 16, 5:45 PM
jingham added a reverting change for rG21641a2f6dba: [lldb][NFC] Make ApplyObjcCastHack less scary: rG66e9f239b5b3: Revert "[lldb][NFC] Make ApplyObjcCastHack less scary".
Mon, Sep 16, 5:45 PM
jingham committed rL372057: Revert "[lldb][NFC] Make ApplyObjcCastHack less scary".
Revert "[lldb][NFC] Make ApplyObjcCastHack less scary"
Mon, Sep 16, 5:45 PM
jingham added a comment to rL372016: Implement std::condition_variable via pthread_cond_clockwait() where available.

This is failing to build on Darwin:

`
 /Volumes/Jonas/llvm/llvm-project/libcxx/include/__config:1091:58: error: function-like macro '_LIBCPP_GLIBC_PREREQ' is not defined
 #  if (defined(__ANDROID__) && __ANDROID_API__ >= 30) || _LIBCPP_GLIBC_PREREQ(2, 30)

Our CMake bot was offline for a while, but I'm pretty sure it'll start showing up here: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/

Mon, Sep 16, 2:17 PM
jingham added inline comments to rL372016: Implement std::condition_variable via pthread_cond_clockwait() where available.
Mon, Sep 16, 2:17 PM

Fri, Sep 13

jingham accepted D67474: [Reproducer] Add `reproducer dump` command.

Approved with that addition.

Fri, Sep 13, 4:08 PM · Restricted Project, Restricted Project
jingham added a comment to D67474: [Reproducer] Add `reproducer dump` command.

The help for -f should say what happens if this option isn't provided. Otherwise this looks fine.

Fri, Sep 13, 4:08 PM · Restricted Project, Restricted Project
jingham accepted D67523: [Reproducer] Move GDB Remote Packet into Utility. (NFC).

Looks right.

Fri, Sep 13, 4:02 PM · Restricted Project, Restricted Project
jingham accepted D67538: [Reproducer] Include the this pointer in the API log..

LGTM

Fri, Sep 13, 10:26 AM · Restricted Project, Restricted Project

Thu, Sep 12

jingham accepted D67472: [Target] Move InferiorCall to Process.

Excellent!

Thu, Sep 12, 4:11 PM · Restricted Project, Restricted Project
jingham added a comment to D67472: [Target] Move InferiorCall to Process.

I don't think CallNoArgNoReturnFunc is the right name. This routine calls a function that takes no arguments but returns a void *.

Thu, Sep 12, 3:40 PM · Restricted Project, Restricted Project
jingham added a comment to D67523: [Reproducer] Move GDB Remote Packet into Utility. (NFC).

There's already Utils/StreamGDBRemote.cpp... It might be natural to stick the class in there?

Thu, Sep 12, 3:32 PM · Restricted Project, Restricted Project
jingham added a comment to D67523: [Reproducer] Move GDB Remote Packet into Utility. (NFC).

It seems weird to me to have the GDBRemotePacket class live in the repro namespace. There's nothing particularly Reproducer specific about it, and it clearly gets used outside the reproducer as well. Wouldn't it make more sense to have this in Utils?

Thu, Sep 12, 3:08 PM · Restricted Project, Restricted Project
jingham added a comment to D67520: Add pretty printing of Clang "bitfield" enums.

The code looks fine to me.

Thu, Sep 12, 2:53 PM
jingham requested changes to D67472: [Target] Move InferiorCall to Process.
Thu, Sep 12, 2:47 PM · Restricted Project, Restricted Project
jingham added a comment to D67472: [Target] Move InferiorCall to Process.

lgtm. Jim?

Thu, Sep 12, 2:46 PM · Restricted Project, Restricted Project
jingham added a comment to D67520: Add pretty printing of Clang "bitfield" enums.

Also, since in C++ you can't do:

Thu, Sep 12, 2:21 PM
jingham added a comment to D67520: Add pretty printing of Clang "bitfield" enums.

Your description says that if there are bits not belonging to the enum, you will print them after the enum values are listed. But you don't have a test for that. I thought you were going to use the "nonsense" var for that, but then you didn't...

Thu, Sep 12, 2:16 PM

Wed, Sep 11

jingham added a comment to D67474: [Reproducer] Add `reproducer dump` command.

If I've run lldb like:

Wed, Sep 11, 5:58 PM · Restricted Project, Restricted Project
jingham added a comment to D67472: [Target] Move InferiorCall to Process.

This seems like the right place to put it, it's clearly something you would do with a process.

Wed, Sep 11, 4:15 PM · Restricted Project, Restricted Project

Tue, Sep 10

jingham committed rG6ca76ceb63bf: Fix a thinko in handling the QSetLogging packet. (authored by jingham).
Fix a thinko in handling the QSetLogging packet.
Tue, Sep 10, 2:58 PM
jingham committed rL371560: Fix a thinko in handling the QSetLogging packet..
Fix a thinko in handling the QSetLogging packet.
Tue, Sep 10, 2:57 PM

Mon, Sep 9

jingham added a comment to D67378: [Utility] Replace Cleanup by llvm::scope_exit.

This pattern repeated a couple of times north or the one you fixed. Can you get those too?

Mon, Sep 9, 4:47 PM · Restricted Project, Restricted Project
jingham added inline comments to D67378: [Utility] Replace Cleanup by llvm::scope_exit.
Mon, Sep 9, 4:29 PM · Restricted Project, Restricted Project

Fri, Sep 6

jingham committed rL371270: Request commit access for jingham.
Request commit access for jingham
Fri, Sep 6, 4:30 PM

Thu, Aug 29

jingham added a comment to D66962: [lldb][NFC] Remove TestFormats.py as is tests nothing.

That was the only test that tested the "command regex" functionality. It was probably called TestFormats.py because somebody copied over a test without changing the file name since that name makes no sense.

Thu, Aug 29, 11:12 AM · Restricted Project
jingham added a comment to D66962: [lldb][NFC] Remove TestFormats.py as is tests nothing.

Does anybody know what this was supposed to test?

Thu, Aug 29, 10:22 AM · Restricted Project

Wed, Aug 28

jingham requested changes to D66908: [dotest] Remove the -# option.

I use -# for running single tests multiple times. This is useful particularly if you have a test that only fails sometimes, you can do:

Wed, Aug 28, 4:03 PM · Restricted Project
jingham added a comment to D66863: [lldb] Unify target checking in CommandObject.

The whole command flags was a late addition, but we were using it for new commands and "when you touch it" kind of changes. That seems to have stalled the conversion, so thanks for completing this!

Wed, Aug 28, 10:06 AM · Restricted Project, Restricted Project

Aug 22 2019

jingham accepted D66628: [Symbol] Decouple clang from DeclVendor.

LGTM

Aug 22 2019, 6:29 PM · Restricted Project, Restricted Project
jingham accepted D66581: [lldb] Construct the dummy target when the first Dummy object is constructed.

You can go a long time without actually using the Dummy target, which is why I made it lazily.

Aug 22 2019, 10:16 AM · Restricted Project, Restricted Project

Aug 19 2019

jingham added a comment to D66345: [lldb][NFC] Allow for-range iterating over StringList.
Aug 19 2019, 10:27 AM · Restricted Project

Aug 16 2019

jingham accepted D66174: [Utility] Reimplement RegularExpression on top of llvm::Regex.

Pavel said "we'll just have to bite the bullet and say that our expressions are now (more or less) POSIX conformant"... We should say this somewhere users are likely to find.

Aug 16 2019, 1:10 PM · Restricted Project, Restricted Project
jingham added a comment to D66354: Add LLDB dataformatters for llvm::StringRef and lldb_private::ConstString.

I don't think you need to go back and cover all the ones already added, but could we start adding tests for new lldb/llvm data formatters as we add them? That way we can keep them useful as people change llvm.

Aug 16 2019, 10:04 AM · Restricted Project

Aug 15 2019

jingham committed rG7049b0ad4d61: Stop-hooks weren't getting called on step-out. Fix that. (authored by jingham).
Stop-hooks weren't getting called on step-out. Fix that.
Aug 15 2019, 2:39 PM
jingham committed rL369052: Stop-hooks weren't getting called on step-out. Fix that..
Stop-hooks weren't getting called on step-out. Fix that.
Aug 15 2019, 2:37 PM
jingham closed D66241: stop-hooks don't fire on "step-out".
Aug 15 2019, 2:37 PM · Restricted Project, Restricted Project
jingham added a comment to D66241: stop-hooks don't fire on "step-out".

Better. Might be good to put a comment inside CalculatePublicStopInfo() regarding why the "SetStopInfo(GetStopInfo());" is needed. Seems like you could just remove the code from the sight of it. I will leave that up to you though.

Aug 15 2019, 2:20 PM · Restricted Project, Restricted Project

Aug 14 2019

jingham updated the diff for D66241: stop-hooks don't fire on "step-out".

Does this seem clearer? We use the language of Private and Public StopInfo's in a bunch of places, even though at present that's more about "How" you get them than actual separate entities. But still, this seems like it follows the language we are using elsewhere.

Aug 14 2019, 4:29 PM · Restricted Project, Restricted Project
jingham added a comment to D66241: stop-hooks don't fire on "step-out".

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?

Aug 14 2019, 3:13 PM · Restricted Project, Restricted Project
jingham added inline comments to D66241: stop-hooks don't fire on "step-out".
Aug 14 2019, 2:23 PM · Restricted Project, Restricted Project
jingham created D66241: stop-hooks don't fire on "step-out".
Aug 14 2019, 12:38 PM · Restricted Project, Restricted Project
jingham added a comment to D66174: [Utility] Reimplement RegularExpression on top of llvm::Regex.

I think we need to fix the behavior for "", other than that, this looks fine to me.

Aug 14 2019, 11:39 AM · Restricted Project, Restricted Project

Aug 13 2019

jingham added a comment to D65682: Give a little more info when "run_to_x_breakpoint" fails.

Thanks for pointing that out.

Aug 13 2019, 10:07 AM · Restricted Project
jingham updated the diff for D65682: Give a little more info when "run_to_x_breakpoint" fails.

Fixed the inverted call order.

Aug 13 2019, 10:07 AM · Restricted Project

Aug 9 2019

jingham added a comment to D56010: [NativePDB] Fix setting breakpoint by file and line.

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.

Aug 9 2019, 3:22 PM

Aug 8 2019

jingham committed rG8240b0d7fe31: Fix a comment which was incorrect. (authored by jingham).
Fix a comment which was incorrect.
Aug 8 2019, 1:49 PM
jingham committed rL368340: Fix a comment which was incorrect..
Fix a comment which was incorrect.
Aug 8 2019, 1:49 PM

Aug 6 2019

jingham added a comment to D65611: [Driver] Expand the executable path in the target create output.

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.

Aug 6 2019, 11:06 AM · Restricted Project, Restricted Project
jingham added inline comments to D65691: Various build fixes for lldb on MinGW.
Aug 6 2019, 10:10 AM · Restricted Project, Restricted Project
jingham added a comment to D65797: [lldb][CMake] Generating Xcode projects.

Thanks for doing this. Looks okay to me just one typo... But I don't know CMake well enough to comment on substance...

Aug 6 2019, 10:01 AM · Restricted Project, Restricted Project

Aug 2 2019

jingham created D65682: Give a little more info when "run_to_x_breakpoint" fails.
Aug 2 2019, 5:08 PM · Restricted Project
jingham added a comment to D65646: [lldb] Print better diagnostics for user expressions and modules.

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.

Aug 2 2019, 1:36 PM · Restricted Project, Restricted Project
jingham added a comment to D65646: [lldb] Print better diagnostics for user expressions and modules.

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 orthogonal or do we now have to ways of pretending there is a source file backing up the expression?

Aug 2 2019, 10:34 AM · Restricted Project, Restricted Project
jingham added inline comments to D65555: [lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP].
Aug 2 2019, 10:10 AM

Aug 1 2019

jingham accepted D65489: Format OptionEnumValueElement.

Okay, if clang-format won't undo this, then I'm fine with writing them this way.

Aug 1 2019, 5:03 PM · Restricted Project, Restricted Project
jingham added a comment to D65611: [Driver] Expand the executable path in the target create output.

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.

Aug 1 2019, 3:46 PM · Restricted Project, Restricted Project
jingham added a comment to D65611: [Driver] Expand the executable path in the target create output.

If you just want the path and don't need the FileSpec, you can call the static SBFileSpec::ResolvePath.

Aug 1 2019, 3:39 PM · Restricted Project, Restricted Project
jingham added a comment to D65489: Format OptionEnumValueElement.

What happens if you clang-format your reformatted version of the setter? Do we need clang-format off for them?

Aug 1 2019, 3:07 PM · Restricted Project, Restricted Project

Jul 31 2019

jingham added a comment to D65469: Remove `bugreport` command.

If we don't want to forget that there was specific useful info, we could gate this patch on implementing the "reproducer generate <FEATURE>" 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 unwind" was all that much used, so it might be better to design this at leisure.

Jul 31 2019, 10:32 AM · Restricted Project
jingham added a comment to D65469: Remove `bugreport` command.

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.

Jul 31 2019, 10:27 AM · Restricted Project

Jul 29 2019

jingham added a comment to D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

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 prologue extents so it can more accurately push the breakpoint past the prologue. But that's not really desirable, it is a lot of work just for setting a breakpoint, and if I ever figure out how not to do that, I will. In which case, this test will no longer test what you think it tests.

Jul 29 2019, 4:01 PM · Restricted Project
jingham added a comment to D65386: [lldb][NFC] Use an enum instead of chars when handling options [WIP].

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::Force or whatever.

Jul 29 2019, 11:39 AM · Restricted Project

Jul 26 2019

jingham added a comment to D65311: [dotest] Remove multiprocessing.

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.

Jul 26 2019, 2:29 PM · Restricted Project, Restricted Project
jingham added a comment to D65122: [Symbol] Use llvm::Expected when getting TypeSystems.

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 future, it is never an error to not have a given type system. And even if lldb has builtin code to support a given type system, I may not want to pay the cost to construct it. If I'm debugging ObjC code in a program that also supports swift, I might want to tell lldb not to bother with swift types.

Jul 26 2019, 10:39 AM · Restricted Project
jingham committed rGbe4a78af465a: Document that LLDB_LOG macros use the format_providers. (authored by jingham).
Document that LLDB_LOG macros use the format_providers.
Jul 26 2019, 10:26 AM
jingham committed rL367132: Document that LLDB_LOG macros use the format_providers..
Document that LLDB_LOG macros use the format_providers.
Jul 26 2019, 10:25 AM
jingham closed D65293: Document the fact that LLDB_LOG uses llvm::format_providers.
Jul 26 2019, 10:25 AM · Restricted Project, Restricted Project
jingham added a comment to D65293: Document the fact that LLDB_LOG uses llvm::format_providers.

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.

Jul 26 2019, 10:25 AM · Restricted Project, Restricted Project

Jul 25 2019

jingham committed rG2b6afdf71046: Mention adding predicates to settings in the projects page. (authored by jingham).
Mention adding predicates to settings in the projects page.
Jul 25 2019, 2:38 PM
jingham committed rL367059: Mention adding predicates to settings in the projects page..
Mention adding predicates to settings in the projects page.
Jul 25 2019, 2:38 PM
jingham committed rGd16a034c7cd3: Remove a project that was completed. (authored by jingham).
Remove a project that was completed.
Jul 25 2019, 2:30 PM
jingham committed rL367057: Remove a project that was completed..
Remove a project that was completed.
Jul 25 2019, 2:30 PM
jingham added a comment to D65271: Increase testsuite packet-timeout 5secs -> 1min.

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:

Jul 25 2019, 11:01 AM · Restricted Project, Restricted Project
jingham added inline comments to D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC).
Jul 25 2019, 10:51 AM · Restricted Project, Restricted Project
jingham created D65293: Document the fact that LLDB_LOG uses llvm::format_providers.
Jul 25 2019, 10:46 AM · Restricted Project, Restricted Project

Jul 24 2019

jingham added a comment to D65109: [LLDB] Remove the Xcode project.

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 internal team meetings sounds concerning.

As far as we know, Greg was the only user of the Xcode project outside of Apple, and he gave the thumbs up.

I was a user of the Xcode project before Apple.

I don't think removing it was especially controversial

That's right. And still, it would be appreciated to give folks in other time zones at least a chance to take part. Thks

Jul 24 2019, 10:12 AM · Restricted Project, Restricted Project
jingham accepted D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC).

Sure, this is okay.

Jul 24 2019, 10:05 AM · Restricted Project, Restricted Project
jingham added a comment to D65165: [Symbol] Fix some botched logic in Variable::GetLanguage.

Thanks for responding quickly. LGTM, with one comment..

Jul 24 2019, 10:02 AM · Restricted Project

Jul 23 2019

jingham added a comment to D65152: Fix issues with inferior stdout coming out of order.

I agree with Greg, having one function that can do any of the combinations of stdout & stderr seems more convenient.

Jul 23 2019, 1:59 PM · Restricted Project

Jul 22 2019

jingham requested changes to D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC).

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 will make them much harder to read.

Jul 22 2019, 6:54 PM · Restricted Project, Restricted Project
jingham added a comment to D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC).

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.

Jul 22 2019, 6:46 PM · Restricted Project, Restricted Project
jingham accepted D64042: [Symbol] Improve Variable::GetLanguage.

Sounds reasonable.

Jul 22 2019, 10:24 AM · Restricted Project

Jul 18 2019

jingham accepted D64964: [API] Remove use of ClangASTContext from SBTarget.

This looks fine to me. Makes it really clear that we need SBTarget::FindFirstTypeForLanguage, etc. But FindFirstType was always a crapshoot anyway...

Jul 18 2019, 5:26 PM · Restricted Project
jingham committed rGdb6cfe1337c0: Remember to sort the Xcode project!!! (authored by jingham).
Remember to sort the Xcode project!!!
Jul 18 2019, 3:28 PM
jingham committed rL366508: Remember to sort the Xcode project!!!.
Remember to sort the Xcode project!!!
Jul 18 2019, 3:28 PM
jingham committed rGfa6199bc5d37: Add an expectedFailure test for type finding. (authored by jingham).
Add an expectedFailure test for type finding.
Jul 18 2019, 3:22 PM
jingham committed rL366507: Add an expectedFailure test for type finding..
Add an expectedFailure test for type finding.
Jul 18 2019, 3:22 PM
jingham committed rGee515d3d03e9: The switch to table-genning command options broke the xcode project. This gets… (authored by jingham).
The switch to table-genning command options broke the xcode project. This gets…
Jul 18 2019, 3:22 PM
jingham committed rL366506: The switch to table-genning command options broke.
The switch to table-genning command options broke
Jul 18 2019, 3:22 PM

Jul 17 2019

jingham added a comment to D64853: Fix CommandInterpreter for _regex-break with options.

This looks good in general. I also have a few nits, and you need to fix (and probably test) the behavior for files with spaces in their names.

Jul 17 2019, 5:29 PM · Restricted Project
jingham added a comment to D64897: Move start-address finding to Target, implement fallback if main executable does not have a start address.

I wonder if we ought to allow target properties (target as an example) that are only for testing, so they don't print when you do settings list, etc. But the we could have some settings like a "target.testing.dont-read-LC_MAIN" and that would make it easy to automate your hand testing. Kind of like the "experimental" decorator, except you have to know they exist to use them...

Jul 17 2019, 5:19 PM · Restricted Project, Restricted Project
jingham added a comment to D64844: [Target] Remove Target::GetScratchClangASTContext.

The nice thing about the way the ObjCLanguageRuntime::Get was that is was only useable where we decided it was legit to use it, in the actual ObjCLanguageRuntime plugin or its direct users. If you want to keep the convenience cast function in a header in Plugins/ExpressionParser/Clang, that would be fine.

Jul 17 2019, 2:57 PM
jingham added a comment to D64844: [Target] Remove Target::GetScratchClangASTContext.

Seems like in most places we're just pulling out basic types or their sizes, which we should certainly be able to do with a TypeSystem.

Jul 17 2019, 12:27 PM