Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

jingham (Jim Ingham)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Today

jingham committed rGb9e08cbf3037: I can't make this test fail locally, but it is failing on the macOS (authored by jingham).
I can't make this test fail locally, but it is failing on the macOS
Thu, Sep 21, 1:31 PM · Restricted Project

Yesterday

jingham committed rG3c61e4bf2853: Copy and paste error in a file header. (authored by jingham).
Copy and paste error in a file header.
Wed, Sep 20, 5:44 PM · Restricted Project
jingham committed rGde94c109b64d: The test: test_run_then_attach_wait_interrupt was flakey on Linux & Windows. (authored by jingham).
The test: test_run_then_attach_wait_interrupt was flakey on Linux & Windows.
Wed, Sep 20, 5:41 PM · Restricted Project
jingham committed rGdf93c4ffdf22: Remove some raciness from the TestProcessAttach. (authored by jingham).
Remove some raciness from the TestProcessAttach.
Wed, Sep 20, 1:10 PM · Restricted Project

Tue, Sep 19

jingham committed rG74338bfe0cfe: A test was changing directory and then incorrectly restoring the directory (authored by jingham).
A test was changing directory and then incorrectly restoring the directory
Tue, Sep 19, 4:46 PM · Restricted Project

Wed, Aug 30

jingham added a comment to D158010: [lldb] Allow synthetic providers in C++ and fix linking problems.

Sorry, I got distracted, but after the fact I also agree this is reasonable.

Wed, Aug 30, 12:24 PM · Restricted Project, Restricted Project

Aug 17 2023

jingham accepted D158237: Change LLGSTest.cpp to run environment_check inferior with stdin/stdout disabled, to work around ASAN CI bot issue.

This seems good to me. Tests that aren't using stdio for a purpose shouldn't have to worry about random output.

Aug 17 2023, 5:45 PM · Restricted Project, Restricted Project
jingham accepted D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime.
Aug 17 2023, 3:16 PM · Restricted Project, Restricted Project

Aug 16 2023

jingham committed rGd268ba38083c: Test follow-up to 2e7aa2ee34eb53347396731dc8a3b2dbc6a3df45 (authored by jingham).
Test follow-up to 2e7aa2ee34eb53347396731dc8a3b2dbc6a3df45
Aug 16 2023, 12:19 PM · Restricted Project
jingham committed rG2e7aa2ee34eb: Replace the singleton "ShadowListener" with a primary and N secondary Listeners (authored by jingham).
Replace the singleton "ShadowListener" with a primary and N secondary Listeners
Aug 16 2023, 10:36 AM · Restricted Project
jingham closed D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners.
Aug 16 2023, 10:36 AM · Restricted Project, Restricted Project

Aug 15 2023

jingham added a comment to D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners.

I implemented Alex's suggestion for a GetListeners filter, but doing that triggered some bugs due to the fact that we weren't really pruning the m_listeners correctly as listeners come and go. I fixed some bugs there along with implementing this suggestion.

Aug 15 2023, 3:09 PM · Restricted Project, Restricted Project
jingham updated the diff for D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners.

Respond to review comments.

Aug 15 2023, 3:05 PM · Restricted Project, Restricted Project
jingham added inline comments to D158010: [lldb] Allow synthetic providers in C++ and fix linking problems.
Aug 15 2023, 1:16 PM · Restricted Project, Restricted Project

Aug 14 2023

jingham accepted D157756: Support corefiles that put the uuid of their binary in a fixed address in low memory; load that binary.

LGTM

Aug 14 2023, 2:29 PM · Restricted Project, Restricted Project

Aug 10 2023

jingham added a comment to D157640: [lldb] Improve error message when trying to debug a non-debuggable process.

Not sure if codesign exists on iOS devices, but at least we could do this on macOS.

Aug 10 2023, 2:09 PM · Restricted Project, Restricted Project
jingham added a comment to D157640: [lldb] Improve error message when trying to debug a non-debuggable process.

In this function we have the path to the binary. We could spawn codesign -d -entitlements - and then we would know whether it had that entitlement.

Aug 10 2023, 2:08 PM · Restricted Project, Restricted Project

Aug 9 2023

jingham requested review of D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners.
Aug 9 2023, 3:40 PM · Restricted Project, Restricted Project
jingham added a comment to D157512: [lldb][PlatformDarwin] Only parse SDK from debug-info if target language is Objective-C.

It's fairly common for AppKit developers to use access to static methods under NSApplication to see the state of the application. Since you aren't actually accessing the frame variables for this purpose, it's also pretty common for those folks to do:

Aug 9 2023, 10:26 AM · Restricted Project, Restricted Project

Aug 4 2023

jingham committed rG4185656625a2: Fix the NSIndexSet data formatter for changes in macOS Sonoma. (authored by jingham).
Fix the NSIndexSet data formatter for changes in macOS Sonoma.
Aug 4 2023, 9:56 AM · Restricted Project

Aug 3 2023

jingham added inline comments to D157022: Fix the NSIndexSet formatter for macOS Sonoma.
Aug 3 2023, 4:40 PM · Restricted Project, Restricted Project
jingham updated the diff for D157022: Fix the NSIndexSet formatter for macOS Sonoma.

Remove redundant code

Aug 3 2023, 4:39 PM · Restricted Project, Restricted Project
jingham added inline comments to D157022: Fix the NSIndexSet formatter for macOS Sonoma.
Aug 3 2023, 12:48 PM · Restricted Project, Restricted Project
jingham updated the diff for D157022: Fix the NSIndexSet formatter for macOS Sonoma.

Use llvm::popcount instead of using the builtin directly.

Aug 3 2023, 12:47 PM · Restricted Project, Restricted Project
jingham requested review of D157022: Fix the NSIndexSet formatter for macOS Sonoma.
Aug 3 2023, 11:26 AM · Restricted Project, Restricted Project

Aug 2 2023

jingham added a comment to D156822: [lldb] Make IR interpretation interruptible.

LGTM. Thanks for adding this.

Aug 2 2023, 11:25 AM · Restricted Project, Restricted Project

Jul 19 2023

jingham accepted D155334: [lldb] Make frame var --regex always search globals.

Excellent!

Jul 19 2023, 11:23 AM · Restricted Project, Restricted Project
jingham requested changes to D155334: [lldb] Make frame var --regex always search globals.

LGTM, with a couple suggestions.

Jul 19 2023, 10:40 AM · Restricted Project, Restricted Project

Jul 17 2023

jingham committed rGb9541b670771: Fix the root directory completion test. (authored by jingham).
Fix the root directory completion test.
Jul 17 2023, 10:54 AM · Restricted Project

Jul 11 2023

jingham committed rG8402ad23104b: Add a generic Process method to dump plugin history. (authored by jingham).
Add a generic Process method to dump plugin history.
Jul 11 2023, 12:33 PM · Restricted Project
jingham closed D154992: Add a generic Process method to dump plugin history.
Jul 11 2023, 12:33 PM · Restricted Project, Restricted Project
jingham requested review of D154992: Add a generic Process method to dump plugin history.
Jul 11 2023, 11:02 AM · Restricted Project, Restricted Project

Jul 10 2023

jingham committed rGc1885d2dfa95: "settings set -g target.load-script-from-symbol-file" shouldn't crash. (authored by jingham).
"settings set -g target.load-script-from-symbol-file" shouldn't crash.
Jul 10 2023, 2:40 PM · Restricted Project
jingham accepted D154643: [lldb] Prevent crash when completing ambiguous subcommands.

This looks like it might be the place where we also say "set s could either be set or show". But that's actually done elsewhere so this change is okay. We should maybe make sure we have a test for setting s<TAB> -> {set or show} options.

Jul 10 2023, 1:56 PM · Restricted Project, Restricted Project

Jul 7 2023

jingham committed rG01e3393b94d1: Split up the runCmd trace printing to print the command before running. (authored by jingham).
Split up the runCmd trace printing to print the command before running.
Jul 7 2023, 7:09 PM · Restricted Project
jingham closed D154752: runCmd should print the command before running it in case of crashes.
Jul 7 2023, 7:09 PM · Restricted Project, Restricted Project
jingham requested review of D154752: runCmd should print the command before running it in case of crashes.
Jul 7 2023, 3:36 PM · Restricted Project, Restricted Project

Jul 6 2023

jingham committed rG2b0c88654212: Refine the reporting mechanism for interruption. (authored by jingham).
Refine the reporting mechanism for interruption.
Jul 6 2023, 4:19 PM · Restricted Project
jingham closed D154542: Further refinements on reporting interruption in lldb.
Jul 6 2023, 4:19 PM · Restricted Project, Restricted Project
jingham updated the diff for D154542: Further refinements on reporting interruption in lldb.

Add some std::move's, Jonas agrees all the cool kids are doing that these days.

Jul 6 2023, 3:35 PM · Restricted Project, Restricted Project
jingham added inline comments to D154542: Further refinements on reporting interruption in lldb.
Jul 6 2023, 2:42 PM · Restricted Project, Restricted Project
jingham accepted D154649: [lldb] Fix dead lock issue when loading modules in Scripted Process.

Yes, that makes sense. lldb always updates its shared library state in reaction to a stop, so it's much safer to have the scripted process emulate this behavior. Plus, refreshing the state for a given stop makes more sense in the place where you generate the stop, than it does in the place where you tell yourself you've resumed.

Jul 6 2023, 1:23 PM · Restricted Project, Restricted Project

Jul 5 2023

jingham added inline comments to D154542: Further refinements on reporting interruption in lldb.
Jul 5 2023, 5:09 PM · Restricted Project, Restricted Project
jingham updated the diff for D154542: Further refinements on reporting interruption in lldb.

Protect InterruptRequested from null function & format strings.

Jul 5 2023, 5:09 PM · Restricted Project, Restricted Project
jingham added inline comments to D154542: Further refinements on reporting interruption in lldb.
Jul 5 2023, 5:03 PM · Restricted Project, Restricted Project
jingham updated the diff for D154542: Further refinements on reporting interruption in lldb.

Address review comments:

Jul 5 2023, 4:40 PM · Restricted Project, Restricted Project
jingham added inline comments to D154542: Further refinements on reporting interruption in lldb.
Jul 5 2023, 3:26 PM · Restricted Project, Restricted Project
jingham requested review of D154542: Further refinements on reporting interruption in lldb.
Jul 5 2023, 1:10 PM · Restricted Project, Restricted Project

Jun 27 2023

jingham accepted D153922: [lldb] Duplicate Target::Launch resuming logic into CommandObjectPlatformProcessLaunch.

The fact that you had to duplicate code from Target::Launch means the various paths to Launching a process aren't properly factored out. However, that seems like a lot just to get Dave's fix for "platform process launch" working, so this LGTM as a short-term patch.

Jun 27 2023, 2:40 PM · Restricted Project, Restricted Project

Jun 26 2023

jingham committed rGf05e2fb013f0: Don't allow SBValue::Cast to cast from a smaller type to a larger, (authored by jingham).
Don't allow SBValue::Cast to cast from a smaller type to a larger,
Jun 26 2023, 4:02 PM · Restricted Project
jingham closed D153657: Don't allow ValueObject::Cast from a smaller type to a larger.
Jun 26 2023, 4:02 PM · Restricted Project, Restricted Project
jingham added a comment to D153657: Don't allow ValueObject::Cast from a smaller type to a larger.

I think this patch is probably okay to do, but it does break a supported use case that I'm aware of: One way you can do "object oriented programming" in C is to do exactly what you're trying to prevent. Using the test, you could say that "MyStruct" is the base class and "MyBiggerStruct" would be a derived class. You might have a pointer to a "MyStruct" object, even though you called malloc(sizeof(MyBiggerStruct)), and maybe you can perform that cast if you're sure that you actually have a MyBiggerStruct object.

I know there's not necessarily a good way to support that without also supporting the bug you're trying to fix though. :(

Jun 26 2023, 10:36 AM · Restricted Project, Restricted Project

Jun 23 2023

jingham requested review of D153657: Don't allow ValueObject::Cast from a smaller type to a larger.
Jun 23 2023, 11:54 AM · Restricted Project, Restricted Project
jingham added a comment to D153648: [lldb] Remove broken shared cache duplicate class handling.

There's similar code for the runtime versions 12-15 which actually looks correct. It seems to claim that the duplicate classes are all stuffed after the capacity in the classOffsets array and that the one that won will indeed be in that list but will not be marked as a duplicate. Did it not work to fix what look like transcription errors going from the 12-15 version of this code to the 16 version?

Jun 23 2023, 11:31 AM · Restricted Project, Restricted Project

Jun 15 2023

jingham accepted D153079: Add an llvm::report_fatal_error for when the darwin kernel says we've finished an insn-step but the thread doesn't think it was insn-stepping.

LGTM. Let's see if we can get this to actually trap the concurrent test failures.

Jun 15 2023, 3:26 PM · Restricted Project, Restricted Project

Jun 13 2023

jingham accepted D151951: [lldb][NFCI] Change return type of Properties::GetExperimentalSettingsName.

LGTM

Jun 13 2023, 10:40 AM · Restricted Project, Restricted Project
jingham accepted D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC).

It's OK to retain this as the default, and as you say, taking it out would be a trivial patch after this work. The control does allow you to do "Have I already made this child" before setting about to make it, though I can't currently see anywhere where we could use this information.

Jun 13 2023, 10:40 AM · Restricted Project, Restricted Project
jingham accepted D152031: [lldb] Default can_create to true in GetChildAtIndex (NFC).

I can dream up a few speculative uses of can_create => false but they are all a little far-fetched. Certainly passing true is the dominant mode, so making it a default seems fine to me and reduces a bunch of noise.

Jun 13 2023, 10:37 AM · Restricted Project, Restricted Project
jingham accepted D152579: [lldb] Symtab::SectionFileAddressesChanged should clear the file address map instead of name map.

That does look like a thinko. Nothing about a file's section addresses changing will change the name to symbol map as that isn't dependent on load addresses.

Jun 13 2023, 10:33 AM · Restricted Project, Restricted Project

Jun 12 2023

jingham accepted D151597: [lldb][NFCI] Remove use of ConstString from IOHandler.
Jun 12 2023, 2:07 PM · Restricted Project, Restricted Project
jingham added a comment to D151597: [lldb][NFCI] Remove use of ConstString from IOHandler.

I don't think we need to support the case where the "end line token" doesn't have to stand on its own line. It's reasonable for lldb to impose this policy, and if we are going to do that then requiring the "\n" after the end line token also seems odd. The way this is done seems okay to me.

Jun 12 2023, 2:07 PM · Restricted Project, Restricted Project
jingham added a comment to D152324: [lldb][NFCI] Change return type of PersistentExpressionState::GetNextPersistentVariableName.

I wonder about this one. In every instance where the API is used, its result is turned into a ConstString first. That's because this variable name lives in the same slot as normal variable names, which come from the debug information and so tend to be in the ConstString pool for better reasons. Do you project being able to get rid of that latter requirement? If not, it seems a bit odd to go to the trouble to avoid this value starting life as a ConstString when the first thing everybody does with it is to turn it into a ConstString.

Jun 12 2023, 1:57 PM · Restricted Project, Restricted Project
jingham accepted D152573: [lldb][NFCI] Remove use of ConstString from Listener.

LGTM, if no one has found a use for this way of filtering by this point, there probably isn't a good one.

Jun 12 2023, 1:49 PM · Restricted Project, Restricted Project

Jun 1 2023

jingham committed rGdf1bb2e65bf4: Restrict the test from 22667e3220de5ead353a2148265d841644b63824 (authored by jingham).
Restrict the test from 22667e3220de5ead353a2148265d841644b63824
Jun 1 2023, 6:46 PM · Restricted Project
jingham committed rG22667e3220de: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables (authored by jingham).
Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables
Jun 1 2023, 4:15 PM · Restricted Project
jingham closed D151940: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables.
Jun 1 2023, 4:15 PM · Restricted Project, Restricted Project
jingham added a reviewer for D151940: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables: bulbazord.
Jun 1 2023, 2:25 PM · Restricted Project, Restricted Project
jingham requested review of D151940: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables.
Jun 1 2023, 2:21 PM · Restricted Project, Restricted Project
jingham committed rG267a4cda8248: Prevent some spurious error messages in the debugserver logs. (authored by jingham).
Prevent some spurious error messages in the debugserver logs.
Jun 1 2023, 10:24 AM · Restricted Project
jingham closed D151861: Don't emit a bunch of spurious "Unknown platform 0" warnings from debugserver.
Jun 1 2023, 10:24 AM · Restricted Project, Restricted Project
jingham committed rG620dc1224ff9: Add EXC_SYSCALL to the set of ignorable mach exceptions. (authored by jingham).
Add EXC_SYSCALL to the set of ignorable mach exceptions.
Jun 1 2023, 10:21 AM · Restricted Project
jingham closed D151843: Add EXC_SYSCALL to the allowable ignored exceptions for Darwin.
Jun 1 2023, 10:21 AM · Restricted Project, Restricted Project

May 31 2023

jingham added a comment to D151859: [lldb/Target] Add ability to set a label to targets.

I'd also maybe call this the Target "Label" not the Name. We have a fairly strong use of Name for breakpoint names, and this doesn't have that character at all. Also, if they are Labels, I think it's legit for us to keep them unique, which I think is more sane than trying to handle two targets with the same label...

May 31 2023, 5:47 PM · Restricted Project, Restricted Project
jingham added a comment to D151859: [lldb/Target] Add ability to set a label to targets.

Make sure we do something sensible with "target select <NAME>" if the user has given the same name to two targets (or disallow doing that, one or the other).

May 31 2023, 5:44 PM · Restricted Project, Restricted Project
jingham requested review of D151861: Don't emit a bunch of spurious "Unknown platform 0" warnings from debugserver.
May 31 2023, 5:40 PM · Restricted Project, Restricted Project
jingham added a comment to D151765: [lldb] Introduce the FileSpecBuilder abstraction.

Why did you choose to have a separate FileSpecBuilder class, rather than making FileSpec smarter about the structure of the path (e.g. have an array of StringRef's into the paths for each component.) We could do the parse once the first time a path element was requested, and then operations like "RemovePathComponent" would be trivial. We could maybe even get rid of the distinction between "path" and "filename" since "filename" is really just "last path component".

May 31 2023, 3:48 PM · Restricted Project, Restricted Project
jingham requested review of D151843: Add EXC_SYSCALL to the allowable ignored exceptions for Darwin.
May 31 2023, 3:28 PM · Restricted Project, Restricted Project

May 30 2023

jingham committed rG14186773e79b: Fix SBValue::FindValue for file static variables (authored by jingham).
Fix SBValue::FindValue for file static variables
May 30 2023, 5:14 PM · Restricted Project
jingham closed D151392: Fix SBValue::FindValue for file static variables.
May 30 2023, 5:13 PM · Restricted Project, Restricted Project

May 24 2023

jingham requested review of D151392: Fix SBValue::FindValue for file static variables.
May 24 2023, 6:19 PM · Restricted Project, Restricted Project
jingham accepted D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses.

The help string for the setting seems clear. There's also some logic to handle the setting vrs. the values we find from the stub which you describe in the comment to the review, but it would be nice to see that in a comment in the code somewhere to help out future generations.

May 24 2023, 4:52 PM · Restricted Project, Restricted Project

May 22 2023

jingham added a comment to D151043: [lldb] Add "Trace" stop reason in Scripted Thread.

If a Scripted process is able to provide private events and enough info to lldb's lower level calls (read memory, registers) to work the thread-plan machinery then it won't need to generate the inferred stop reasons like PlanComplete and Instrumentation. But the scripted threads might just be producing public stops w/o the underlying model that would allow lldb to generate them. In that case, the scripted thread will also need to be able to produce the inferred stop reasons.

May 22 2023, 5:06 PM · Restricted Project, Restricted Project

May 16 2023

jingham added a comment to D150619: [lldb] Delay removal of persistent results.

This seems fine in general, with one quibble:

May 16 2023, 5:24 PM · Restricted Project, Restricted Project

May 15 2023

jingham added a comment to D149914: [lldb] Refactor ObjCLanguage::MethodName.

LGTM, Adrian's comment is still outstanding however.

May 15 2023, 3:48 PM · Restricted Project, Restricted Project
jingham added a comment to D149379: [lldb] Add tests for command removal.

The alias still works because it still holds a reference to it. I could add that as a test

May 15 2023, 2:51 PM · Restricted Project, Restricted Project
jingham added a comment to D149914: [lldb] Refactor ObjCLanguage::MethodName.

Your test measured setting a found simple breakpoint. That should measure filling all the names caches - we do that the first time you try to set a breakpoint of any sort. But doesn't measure the effects on lookup. I am guessing you will find the same "not much difference" here as well, but it would be good to test that. So it would be good to also ensure you aren't slowing down looking for a selector by name, and looking for a selector you aren't going to find by name, and looking by full ObjC name. But if that's also true, then I'm fine with this.

May 15 2023, 2:41 PM · Restricted Project, Restricted Project

May 11 2023

jingham committed rGe19387e6936c: We can't let GetStackFrameCount get interrupted or it will give the (authored by jingham).
We can't let GetStackFrameCount get interrupted or it will give the
May 11 2023, 2:49 PM · Restricted Project
jingham closed D150236: Thread::GetStackFrameCount should forestall interruption.
May 11 2023, 2:49 PM · Restricted Project, Restricted Project
jingham added a comment to D150236: Thread::GetStackFrameCount should forestall interruption.

I added the enum, that's a good idea.

May 11 2023, 11:22 AM · Restricted Project, Restricted Project
jingham updated the diff for D150236: Thread::GetStackFrameCount should forestall interruption.

Address Review Comments

May 11 2023, 11:15 AM · Restricted Project, Restricted Project

May 10 2023

jingham updated the diff for D150236: Thread::GetStackFrameCount should forestall interruption.

Address review comments

May 10 2023, 5:53 PM · Restricted Project, Restricted Project
jingham added inline comments to D150236: Thread::GetStackFrameCount should forestall interruption.
May 10 2023, 5:50 PM · Restricted Project, Restricted Project
jingham committed rG7b5dc63fc446: When the Debugger runs HandleProcessEvent it should allow (authored by jingham).
When the Debugger runs HandleProcessEvent it should allow
May 10 2023, 3:42 PM · Restricted Project
jingham closed D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing.
May 10 2023, 3:41 PM · Restricted Project, Restricted Project
jingham requested review of D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing.
May 10 2023, 3:11 PM · Restricted Project, Restricted Project
jingham accepted D150239: ObjectFileMachO: Prioritize the TEXT segment as the mach header segment, regardless of the order the segments appear in the file.

LGTM for support of something that really should hurt a little more than you are making it...

May 10 2023, 3:09 PM · Restricted Project, Restricted Project

May 9 2023

jingham requested review of D150236: Thread::GetStackFrameCount should forestall interruption.
May 9 2023, 5:08 PM · Restricted Project, Restricted Project
jingham resigned from D140630: [lldb-vscode] Add data breakpoint support.
May 9 2023, 4:55 PM · Restricted Project, Restricted Project
jingham added a comment to D140630: [lldb-vscode] Add data breakpoint support.

Ping.

May 9 2023, 4:54 PM · Restricted Project, Restricted Project
jingham added a comment to D149914: [lldb] Refactor ObjCLanguage::MethodName.

Most of this is fine. I wonder about avoiding caching the full name and name w/o category & selector name. One of the main uses of this class is to take incoming ObjC names from the ConstString pool, chop them up into full name, name w/o category, and selectorName, which we then insert into the lookup names tables for the modules they are found in. All those lookup table names will also exist in the ConstString pool.

May 9 2023, 4:49 PM · Restricted Project, Restricted Project