Page MenuHomePhabricator

amccarth (Adrian McCarthy)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2015, 9:26 AM (260 w, 6 d)

Recent Activity

Fri, Jan 24

amccarth added a comment to D71092: [VFS] More consistent support for Windows.

cc: +thakis, who expressed interest in seeing a fix for the text exempted on Windows.

Fri, Jan 24, 10:06 AM · Restricted Project
amccarth updated subscribers of D71092: [VFS] More consistent support for Windows.
Fri, Jan 24, 10:04 AM · Restricted Project
amccarth updated the diff for D71092: [VFS] More consistent support for Windows.
Fri, Jan 24, 10:04 AM · Restricted Project

Thu, Jan 2

amccarth added a comment to D71991: Fix external-names.c test when separator is \\.

It would surprise me if the different test config is cause here. The path is assembled entirely in the code, so it should be consistent. If you had a systemic problem introduced by test config, I'd expect more of the VFS tests to fail.

Thu, Jan 2, 11:14 AM · Restricted Project

Mon, Dec 30

amccarth added a comment to D71991: Fix external-names.c test when separator is \\.

I'm still not seeing how the backslashes got in there for you. They don't happen for me. It looks like these paths come (almost) directly from the temp .yaml files created by the sed commands at the beginning of the test, so--for these particular tests--I'd expect forward slashes regardless of platform.

Mon, Dec 30, 4:22 PM · Restricted Project
amccarth accepted D71991: Fix external-names.c test when separator is \\.

I've made a similar change in several other tests, but this test works for me without this change. I'm trying to understand why, but I don't see any harm in landing this.

Mon, Dec 30, 11:10 AM · Restricted Project

Dec 20 2019

amccarth added a comment to D71786: RFC: [Support] On Windows, add optional support for rpmalloc.

Just a nit on the patch description: This change replaces the CRT allocator (i.e., malloc and free). The "default Windows heap allocator" could be misunderstood to mean replacing the OS-level heap manager (i.e., https://docs.microsoft.com/en-us/windows/win32/memory/heap-functions).

Dec 20 2019, 5:01 PM · Restricted Project

Dec 19 2019

amccarth closed D34468: Make IPDBSession::getGlobalScope a non-const method.
Dec 19 2019, 9:43 AM · Restricted Project
amccarth closed D34542: Introduce symbol cache to PDB NativeSession.
Dec 19 2019, 9:43 AM · Restricted Project
amccarth closed D34479: Add IDs and clone methods to NativeRawSymbol.
Dec 19 2019, 9:43 AM · Restricted Project
amccarth closed D70458: [NFC] Refactor and improve comments in CommandObjectTarget.

This landed in 3b69f0c5550a229dd6d39e361182cdd7cecc36a4, but there was a typo in the patch description so the tools didn't automatically close this.

Dec 19 2019, 9:33 AM

Dec 18 2019

amccarth committed rG738b5c9639b4: Fix more VFS tests on Windows (authored by amccarth).
Fix more VFS tests on Windows
Dec 18 2019, 11:45 AM
amccarth committed rG9d38fd8d0be3: [NFC] Update FIXME for one VFS test (authored by amccarth).
[NFC] Update FIXME for one VFS test
Dec 18 2019, 11:45 AM
amccarth closed D70701: Fix more VFS tests on Windows.
Dec 18 2019, 11:45 AM · Restricted Project, Restricted Project

Dec 9 2019

amccarth added inline comments to D70701: Fix more VFS tests on Windows.
Dec 9 2019, 4:20 PM · Restricted Project, Restricted Project

Dec 5 2019

amccarth planned changes to D71092: [VFS] More consistent support for Windows.

Hold off on this one. It needs an explicit test for canonicalization.

Dec 5 2019, 2:57 PM · Restricted Project
amccarth created D71092: [VFS] More consistent support for Windows.
Dec 5 2019, 2:39 PM · Restricted Project

Dec 3 2019

amccarth added a comment to D70701: Fix more VFS tests on Windows.

A (hopefully) more cogent response than my last round. I'm still hoping to hear from VFS owners.

Dec 3 2019, 12:53 PM · Restricted Project, Restricted Project

Dec 2 2019

amccarth added inline comments to D70701: Fix more VFS tests on Windows.
Dec 2 2019, 1:57 PM · Restricted Project, Restricted Project
amccarth resigned from D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM.

I'm following along, but I don't think I have enough domain knowledge to qualify as a reviewer.

Dec 2 2019, 11:38 AM · Restricted Project

Nov 27 2019

amccarth added inline comments to D70778: [LLDB] [PECOFF] Factorize mapping section names to types using StringSwitch. NFCI..
Nov 27 2019, 11:13 AM · Restricted Project
amccarth accepted D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name.

Nice. LGTM.

Nov 27 2019, 7:31 AM · Restricted Project

Nov 26 2019

amccarth added a comment to D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name.

Is .eh_frame the only one that matters? Should this be more general and compare const_sect_name to the full name and the truncated name for any known section names?

Nov 26 2019, 4:14 PM · Restricted Project
amccarth accepted D70742: [LLDB] [Windows] Avoid using InitializeContext for allocating a CONTEXT. NFC..

That's fair. LGTM.

Nov 26 2019, 4:00 PM · Restricted Project
amccarth added a comment to D70742: [LLDB] [Windows] Avoid using InitializeContext for allocating a CONTEXT. NFC..

I'm good with the change, but have a couple small requests. I hope to hear from others, too, as this area is outside my wheelhouse.

Nov 26 2019, 2:41 PM · Restricted Project

Nov 25 2019

amccarth created D70701: Fix more VFS tests on Windows.
Nov 25 2019, 4:10 PM · Restricted Project, Restricted Project

Nov 21 2019

amccarth committed rG3b69f0c5550a: [NFC] Refactor and improve comments in CommandObjectTarget (authored by amccarth).
[NFC] Refactor and improve comments in CommandObjectTarget
Nov 21 2019, 9:06 AM

Nov 20 2019

amccarth updated the diff for D70458: [NFC] Refactor and improve comments in CommandObjectTarget.

Reverted unintended change caught by reviewer.

Nov 20 2019, 11:46 AM
amccarth added inline comments to D70458: [NFC] Refactor and improve comments in CommandObjectTarget.
Nov 20 2019, 11:46 AM

Nov 19 2019

amccarth created D70458: [NFC] Refactor and improve comments in CommandObjectTarget.
Nov 19 2019, 1:01 PM

Nov 18 2019

amccarth added a comment to D70387: [LLDB] [Windows] Allow making subprocesses use the debugger's console with LLDB_INHERIT_CONSOLE=true.

labath has added great context here, and I generally agree with clayborg's ideal of optimal behavior. That said, if memory serves, getting that behavior on Windows can be quite challenging, so I'm not sure if it's worth the effort.

Nov 18 2019, 4:54 PM · Restricted Project

Nov 15 2019

amccarth accepted D69968: [COFF] Don't error if the only inputs are from /wholearchive:.
Nov 15 2019, 3:25 PM · Restricted Project

Nov 14 2019

amccarth accepted D70281: Fix -Wunused-result warnings in LLDB.

It's too bad that pattern is repeated three times, including the explanatory comment.

Nov 14 2019, 3:50 PM · Restricted Project
amccarth committed rG1275ab1620b6: Improve VFS compatibility on Windows (authored by amccarth).
Improve VFS compatibility on Windows
Nov 14 2019, 8:53 AM
amccarth closed D69958: Improve VFS compatibility on Windows.
Nov 14 2019, 8:53 AM · Restricted Project, Restricted Project

Nov 13 2019

amccarth updated the diff for D69958: Improve VFS compatibility on Windows.

Corrected comment about default for case sensitivity.

Nov 13 2019, 4:13 PM · Restricted Project, Restricted Project
amccarth added a comment to D69958: Improve VFS compatibility on Windows.

Thanks for the review.

Nov 13 2019, 4:04 PM · Restricted Project, Restricted Project
amccarth updated the diff for D69958: Improve VFS compatibility on Windows.

Modified comment per feedback.

Nov 13 2019, 11:27 AM · Restricted Project, Restricted Project
amccarth added a comment to D69958: Improve VFS compatibility on Windows.

Friendly ping for any VFS experts to comment.

Nov 13 2019, 11:25 AM · Restricted Project, Restricted Project

Nov 12 2019

amccarth requested changes to D69968: [COFF] Don't error if the only inputs are from /wholearchive:.

This looks like the right approach, but I have a concern or two about the ramifications as noted inline. Let me know if my concerns are unwarranted.

Nov 12 2019, 9:50 AM · Restricted Project

Nov 7 2019

amccarth accepted D69969: [SEH] Defer checking filter expression types until instantiaton.

LGTM.

Nov 7 2019, 2:45 PM · Restricted Project
amccarth created D69958: Improve VFS compatibility on Windows.
Nov 7 2019, 11:43 AM · Restricted Project, Restricted Project
amccarth abandoned D68953: Enable most VFS tests on Windows.

I create a new review thread for my improved patch shortly. These changes were too wide-ranging.

Nov 7 2019, 11:07 AM · Restricted Project

Nov 4 2019

amccarth added a comment to D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up.

Thanks!

Nov 4 2019, 8:44 AM · Restricted Project

Oct 30 2019

amccarth accepted D69646: [LLDB] [PECOFF] Fix error handling for executables that object::createBinary error out on.
Oct 30 2019, 4:23 PM · Restricted Project
amccarth requested changes to D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up.

Thanks for identifying this race condition.

Oct 30 2019, 4:16 PM · Restricted Project
amccarth added a comment to D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up.

(Ideally, I'd just delete ProcessWindows entirely, and move everything to lldb-server based model -- it's much easier to reason about threads there)

Oct 30 2019, 11:41 AM · Restricted Project
amccarth added a comment to D69612: [lldb-vscod] fix build with NDEBUG on windows.

LGTM, modulo the (void) result stuff. Do you need someone to commit this for you?

Yes I need.

Oct 30 2019, 11:12 AM · Restricted Project
amccarth added a comment to D69535: build: improve python check for Windows.

.. Given that this code is python3 specific and other platforms still support python2, maybe we can make this bump windows-specific at first, and then extend it to other platforms once we officially stop supporting python2 ?

Oct 30 2019, 8:06 AM · Restricted Project

Oct 25 2019

amccarth accepted D69455: Correct size_t format specifier.
Oct 25 2019, 5:37 PM · Restricted Project
amccarth committed rG5a3c657f3e8f: Fix after 738af7a6241c98164625b9cd1ba9f8af4e36f197 (authored by amccarth).
Fix after 738af7a6241c98164625b9cd1ba9f8af4e36f197
Oct 25 2019, 4:02 PM
amccarth added a comment to D68671: Add the ability to pass extra args to a Python breakpoint command function.

FYI: This broke the build for me.

Oct 25 2019, 3:52 PM · Restricted Project
amccarth accepted D69285: minidump: Rename some architecture constants.
Oct 25 2019, 3:24 PM · Restricted Project, Restricted Project
amccarth added a comment to D68953: Enable most VFS tests on Windows.

Somehow Phabricator failed to notify me that you'd already left comments. I even searched my inbox to see if I'd just missed the notification. Nope. Nothing. I came here today to ping you for the review and discovered you'd already done your part.

Oct 25 2019, 9:48 AM · Restricted Project

Oct 24 2019

amccarth added inline comments to D69366: [LLDB] [PECOFF] Use FindSectionByID to associate symbols to sections.
Oct 24 2019, 10:56 AM · Restricted Project

Oct 21 2019

amccarth accepted D69100: COFF: Create a separate "section" for the file header.

I'm OK with this. I'm just wondering whether it would be a good idea to make it clear that these header sections are "not considered to be a section in the strictest sense." Is the distinction going to be important to future code readers? Do we already have "sections" that aren't truly "sections"?

Oct 21 2019, 1:45 PM

Oct 15 2019

amccarth added a comment to D68980: [LLDB] [test] Allow specifying the full arch for msvc/clang-cl in build.py.
I know there are some complexities with configuring msvc/clang-cl, where one needs to fetch registry keys and whatnot in order to get the right build environment.
Oct 15 2019, 11:17 AM · Restricted Project

Oct 14 2019

amccarth created D68953: Enable most VFS tests on Windows.
Oct 14 2019, 11:58 AM · Restricted Project

Oct 10 2019

amccarth added a comment to D68770: [LLDB] [Driver] Use llvm::InitLLVM to do unicode argument conversion on Windows.

On Windows, it _does_ rewrite argv[0], but it looks like it tries to not change whether it was relative/absolute, so I think this is fine.

Oct 10 2019, 1:56 PM · Restricted Project
amccarth accepted D68770: [LLDB] [Driver] Use llvm::InitLLVM to do unicode argument conversion on Windows.

Cool. I didn't know about InitLLVM. That makes things much cleaner.

Oct 10 2019, 1:07 PM · Restricted Project

Oct 9 2019

amccarth added a comment to D68736: [MSVC] Automatically add atlmfc include and lib directories as system paths..

This matches the behavior of cl.

Oct 9 2019, 3:23 PM · Restricted Project

Oct 8 2019

amccarth accepted D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api.

LGTM after one question.

Oct 8 2019, 2:24 PM · Restricted Project, Restricted Project
amccarth accepted D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows.

Given Pavel's comment, this LGTM.

Oct 8 2019, 2:15 PM · Restricted Project
amccarth added a comment to D61886: Support member functions construction and lookup in PdbAstBuilder.

Is this still an active review or has this been abandoned?

Oct 8 2019, 2:15 PM · Restricted Project

Oct 2 2019

amccarth added a comment to D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows.

I've made it in the way similar to Zachary have made for the SymbolFileNativePDB plugin. An environment variable could be more convenient e.g. to run a bunch of tests using the lldb-test option.

Oct 2 2019, 8:39 AM · Restricted Project

Oct 1 2019

amccarth added a comment to D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows.

Why an environment variable rather than a command line option?

Oct 1 2019, 2:47 PM · Restricted Project

Sep 30 2019

amccarth added a comment to D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans.

I'm just back from vacation. I agree with Pavel that we need to hear more from Jason at this point. I'm still very interested in helping this land in some form.

Sep 30 2019, 8:51 AM · Restricted Project, Restricted Project

Sep 13 2019

amccarth committed rG646a893f1583: Fix error in ProcessLauncherWindows.cpp (authored by amccarth).
Fix error in ProcessLauncherWindows.cpp
Sep 13 2019, 11:53 AM
amccarth committed rL371882: Fix error in ProcessLauncherWindows.cpp.
Fix error in ProcessLauncherWindows.cpp
Sep 13 2019, 11:52 AM

Sep 12 2019

amccarth added a comment to D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans.

I have a couple more questions and some renaming requests.

Sep 12 2019, 5:13 PM · Restricted Project, Restricted Project
amccarth accepted D66638: Unwind: Add a stack scanning mechanism to support win32 unwinding.

My concerns are satisfied.

Sep 12 2019, 1:07 PM · Restricted Project, Restricted Project

Sep 11 2019

amccarth accepted D67454: Start porting ivfsoverlay tests to Windows.

LGTM

Sep 11 2019, 1:17 PM · Restricted Project, Restricted Project
amccarth added a comment to D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans.

I've been trying to figure out how to implement the same functionality you've got here, so I'm very interested in helping you land this in some form.

Sep 11 2019, 10:47 AM · Restricted Project, Restricted Project

Sep 5 2019

amccarth accepted D67168: [Windows] Add support of watchpoints to `ProcessWindows`.

Thanks for the changes! I think this looks good now.

Sep 5 2019, 3:32 PM · Restricted Project, Restricted Project
amccarth added a comment to D66638: Unwind: Add a stack scanning mechanism to support win32 unwinding.

I don't have any specific code comments, but I do have a couple general questions and points to consider.

Sep 5 2019, 2:46 PM · Restricted Project, Restricted Project
amccarth committed rGf141de5bc928: Fix windows-x86-debug compilation with python enabled using multi-target… (authored by amccarth).
Fix windows-x86-debug compilation with python enabled using multi-target…
Sep 5 2019, 10:23 AM
amccarth committed rL371090: Fix windows-x86-debug compilation with python enabled using multi-target….
Fix windows-x86-debug compilation with python enabled using multi-target…
Sep 5 2019, 10:23 AM
amccarth closed D66994: [lldb][CMake] Fix windows-x86-debug compilation with python enabled using multi-target generator.
Sep 5 2019, 10:23 AM · Restricted Project, Restricted Project
amccarth added a comment to D66994: [lldb][CMake] Fix windows-x86-debug compilation with python enabled using multi-target generator.

Yes, I can commit it for you soon.

Sep 5 2019, 9:57 AM · Restricted Project, Restricted Project
amccarth accepted D66994: [lldb][CMake] Fix windows-x86-debug compilation with python enabled using multi-target generator.

Thanks for factoring out the duplication.

Sep 5 2019, 8:11 AM · Restricted Project, Restricted Project

Sep 4 2019

amccarth added inline comments to D67168: [Windows] Add support of watchpoints to `ProcessWindows`.
Sep 4 2019, 3:29 PM · Restricted Project, Restricted Project
amccarth accepted D67166: Win: handle \\?\UNC\ prefix in realPathFromHandle (PR43204).

The \\?\ prefix tells the Windows API layer not to parse the path strings and just pass it along to the file system. Ramifications:

Sep 4 2019, 1:22 PM · Restricted Project
amccarth added a comment to D67123: [lldb] Early exit in RangeDataVector:FindEntryIndexesThatContain.

Disregard my last comment. Phabricator wasn't showing me that latest, nor that the patch had already been submitted, which seems to be happening with increasing frequency.

Sep 4 2019, 9:31 AM · Restricted Project, Restricted Project
amccarth added inline comments to D67123: [lldb] Early exit in RangeDataVector:FindEntryIndexesThatContain.
Sep 4 2019, 9:27 AM · Restricted Project, Restricted Project
amccarth accepted D67067: Breakpad: Basic support for STACK WIN unwinding.

LGTM. Please consider adding a comment to the assert that summarizes what you explained in the thread.

Sep 4 2019, 9:20 AM · Restricted Project

Sep 3 2019

amccarth added a comment to D67067: Breakpad: Basic support for STACK WIN unwinding.

This is looking pretty good.

Sep 3 2019, 1:41 PM · Restricted Project
amccarth accepted D67083: [dotest] Avoid the need for LEVEL= makefile boilerplate.

Nice!

Sep 3 2019, 1:10 PM · Restricted Project
amccarth added a comment to D67123: [lldb] Early exit in RangeDataVector:FindEntryIndexesThatContain.

This feels very familiar. I think I've reviewed a similar change back when we first implemented minidumps.

Sep 3 2019, 1:02 PM · Restricted Project, Restricted Project
amccarth added a comment to D66994: [lldb][CMake] Fix windows-x86-debug compilation with python enabled using multi-target generator.

I'm open to this if we can reduce the code duplication.

Sep 3 2019, 11:59 AM · Restricted Project, Restricted Project

Aug 30 2019

amccarth requested changes to D66994: [lldb][CMake] Fix windows-x86-debug compilation with python enabled using multi-target generator.
Aug 30 2019, 3:04 PM · Restricted Project, Restricted Project
amccarth committed rL370537: Request commit access for amccarth.
Request commit access for amccarth
Aug 30 2019, 2:45 PM
amccarth added a comment to D66994: [lldb][CMake] Fix windows-x86-debug compilation with python enabled using multi-target generator.

I'll look at this in detail soon, but a few caveats:

Aug 30 2019, 10:23 AM · Restricted Project, Restricted Project

Aug 28 2019

amccarth abandoned D66841: Skip test_target_create_invalid_core_file on Windows.

Somebody else already did this while I was waiting for review.

Aug 28 2019, 1:55 PM
amccarth added a comment to D66863: [lldb] Unify target checking in CommandObject.

I support anything that reduces the code path differences between user-entered commands and their SBAPI counterparts. Thanks for doing this!

Aug 28 2019, 9:15 AM · Restricted Project, Restricted Project

Aug 27 2019

amccarth added a comment to D66634: Postfix: move more code out of the PDB plugin.

When I added my comments, Phabricator showed this patch as still open. Now it looks like it landed four hours before that. :-(

Aug 27 2019, 4:47 PM · Restricted Project
amccarth created D66841: Skip test_target_create_invalid_core_file on Windows.
Aug 27 2019, 3:57 PM

Aug 26 2019

amccarth added a comment to D66634: Postfix: move more code out of the PDB plugin.

I have a couple inline questions. After that, it looks fine.

Aug 26 2019, 8:27 AM · Restricted Project

Aug 23 2019

amccarth accepted D66633: Breakpad: Add support for parsing STACK WIN records.

LGTM.

Aug 23 2019, 11:38 AM · Restricted Project

Aug 22 2019

amccarth added a comment to D66447: Add char8_t support (C++20).

A couple inline comments. I think this is looking pretty good.

Aug 22 2019, 2:34 PM · Restricted Project, Restricted Project
amccarth added a comment to D66445: Explicitly Cast Constants to DWORD.

Is this necessary? I see

Aug 22 2019, 8:47 AM · Restricted Project