Page MenuHomePhabricator

amccarth (Adrian McCarthy)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2015, 9:26 AM (246 w, 2 d)

Recent Activity

Yesterday

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.
Tue, Oct 15, 11:17 AM · Restricted Project

Mon, Oct 14

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

Thu, Oct 10

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.

Thu, Oct 10, 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.

Thu, Oct 10, 1:07 PM · Restricted Project

Wed, Oct 9

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

This matches the behavior of cl.

Wed, Oct 9, 3:23 PM · Restricted Project

Tue, Oct 8

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

LGTM after one question.

Tue, Oct 8, 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.

Tue, Oct 8, 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?

Tue, Oct 8, 2:15 PM · Restricted Project

Wed, Oct 2

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.

Wed, Oct 2, 8:39 AM · Restricted Project

Tue, Oct 1

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?

Tue, Oct 1, 2:47 PM · Restricted Project

Mon, Sep 30

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.

Mon, Sep 30, 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

Aug 16 2019

amccarth added inline comments to D66345: [lldb][NFC] Allow for-range iterating over StringList.
Aug 16 2019, 3:37 PM · Restricted Project

Aug 14 2019

amccarth resigned from D56010: [NativePDB] Fix setting breakpoint by file and line.
Aug 14 2019, 4:02 PM
amccarth added a comment to D56010: [NativePDB] Fix setting breakpoint by file and line.

It turns out these tests all seem to exist, so I guess Zachary submitted them as part of other CLs.

Aug 14 2019, 4:02 PM

Aug 9 2019

amccarth added a comment to D65230: [CMake] Loosen Python version check and ignore patch version.

Just closing the loop. Regarding the "Python memory allocator called without holding the GIL" bugs: this is not a "just me" problem. It's for anyone who enables debug mode. This appears to be a test infrastructure problem, but it might be harmless. If I run in "release" mode, I get the same behavior as the Windows build bot.

Aug 9 2019, 4:58 PM · Restricted Project, Restricted Project
amccarth 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:00 PM

Aug 8 2019

amccarth accepted D65955: Minidump/Windows: Fix module lookup.

This looks fine, and thanks for the tests for a one-line fix.

Aug 8 2019, 10:14 AM · Restricted Project

Aug 5 2019

amccarth added inline comments to D65691: Various build fixes for lldb on MinGW.
Aug 5 2019, 3:20 PM · Restricted Project, Restricted Project
amccarth added a comment to D65691: Various build fixes for lldb on MinGW.

Yes, zturner isn't as active here as he was before.

Aug 5 2019, 8:59 AM · Restricted Project, Restricted Project

Aug 1 2019

amccarth committed rG5f5379d0767d: Fix TestThreadSpecificBreakpoint on Windows (authored by amccarth).
Fix TestThreadSpecificBreakpoint on Windows
Aug 1 2019, 7:50 AM
amccarth committed rL367573: Fix TestThreadSpecificBreakpoint on Windows.
Fix TestThreadSpecificBreakpoint on Windows
Aug 1 2019, 7:50 AM
amccarth closed D65546: Fix TestThreadSpecificBreakpoint on Windows.
Aug 1 2019, 7:50 AM · Restricted Project

Jul 31 2019

amccarth created D65546: Fix TestThreadSpecificBreakpoint on Windows.
Jul 31 2019, 3:46 PM · Restricted Project

Jul 30 2019

amccarth accepted D65437: [lldb][docs] Update landing page for monorepo.
Jul 30 2019, 8:15 AM · Restricted Project, Restricted Project

Jul 26 2019

amccarth added inline comments to D65330: [lldb][docs] Update documentation for monorepo and CMake caches.
Jul 26 2019, 11:09 AM · Restricted Project, Restricted Project
amccarth added a comment to D65330: [lldb][docs] Update documentation for monorepo and CMake caches.

Looks good. Thanks for doing these updates!

Jul 26 2019, 9:54 AM · Restricted Project, Restricted Project
amccarth added a comment to D65230: [CMake] Loosen Python version check and ignore patch version.

Thanks.

BTW, I tried this on windows, and it didn't blow up in my face, I think it should be relatively safe. I couldn't reproduce the problems that @amccarth was experiencing, so I don't know if it fixes that problem. It would be good to know whether it does, as we could cherry-pick this to the release branch in that case.

Jul 26 2019, 9:42 AM · Restricted Project, Restricted Project

Jul 22 2019

amccarth committed rG3f0621029505: [Windows] Fix race condition between state changes (authored by amccarth).
[Windows] Fix race condition between state changes
Jul 22 2019, 10:08 AM
amccarth committed rL366703: [Windows] Fix race condition between state changes.
[Windows] Fix race condition between state changes
Jul 22 2019, 10:03 AM
amccarth closed D62183: [Windows] Fix race condition between state changes.
Jul 22 2019, 10:02 AM · Restricted Project, Restricted Project
amccarth added a comment to D62183: [Windows] Fix race condition between state changes.

Yes, I can submit it for you, probably in the next hour or two. Thanks for the patch!

Jul 22 2019, 9:11 AM · Restricted Project, Restricted Project

Jul 18 2019

amccarth accepted D64689: [NativePDB] Make GetOrCreateDeclForUid return an lldb CompilerDecl.

Thanks for the change. I was thinking Expected based on the name of the function: GetOrCreateDeclForUid. I was thinking that it would be odd to have a UID if you don't have debug info. Anyway, Optional is fine. Thanks again.

Jul 18 2019, 8:33 AM
amccarth requested changes to D63165: Initial support for native debugging of x86/x64 Windows processes.
In D63165#1540606, @Hui wrote:

Sorry for the stupid question, but ...

What exactly is meant here by "Native"? How is a NativeProcessWindows different from ProcessWindows?

The Native*** classes are meant to be used from lldb-server. They look somewhat similar to their non-native counterpart because they still do debugging, but they're a lot dumber, because they only deal with basic process control, and none of the fancy symbolic stuff that you'd need debug info for.

They differ in APIs but most of them have common implementations. The APIs from native process classes are more easy to apply process/thread control.
Hope the native and non-native ones can be merged. The similar thing to the RegisterContext and NativeRegisterContext classes.

The other thing is that using "native" classes can avoid linking a lot of unnecessary lldb libs (LLDB plugins or whatever comes with the plugins) to lldb-server.
The nativeprocesswindows could just be a pass-through to processwindows plugin, but the usage is a sort of strange since the
lldb-server needs to initialize the plugin, create a target, and create a instance just like what lldb does. This means literally
there will be two lldb debuggers, one on host and the other one on remote. It is doable, but not that applicable.

So the idea is ProcessWindows will be deleted once lldb-server supports windows debugging. A bit of history here: when we first started we started making ProcessXXXX for each platform (mac, linux, windows). Then we thought about remote debugging and decided to have everyone just make a lldb-server for their platform and even when we are locally debugging, we launch a lldb-server. This is how linux, macOS, the BSDs and other targets do it. Why? Because if you do it the other way you have more code to support: one native plug-in and one remote plug-in. This means the remote debugging usually has more issues since it is the less tested debugging scenario. It also means that if you had a native process implementation only, like ProcessWindows is currently, you can't remotely debug to a windows machine.

So yes there is duplicated code for now, but ProcessWindows.cpp and ThreadWindows.cpp should go away in the near future once lldb-server takes over so the temporary code duplication is just to keep things working until lldb-server takes over.

Jul 18 2019, 8:18 AM · Restricted Project, Restricted Project

Jul 17 2019

amccarth accepted D62183: [Windows] Fix race condition between state changes.
Jul 17 2019, 4:16 PM · Restricted Project, Restricted Project
amccarth committed rG3628a8fae9f3: [NFC] Clarify a Cmake status message regarding Python on LLDBConfig (authored by amccarth).
[NFC] Clarify a Cmake status message regarding Python on LLDBConfig
Jul 17 2019, 3:37 PM
amccarth committed rL366383: [NFC] Clarify a Cmake status message regarding Python on LLDBConfig.
[NFC] Clarify a Cmake status message regarding Python on LLDBConfig
Jul 17 2019, 3:36 PM
amccarth added a comment to D64881: [Cmake] Use the modern way to find Python when possible.

An aside ...

I'm still trying to get back to a buildable state the earlier changes, like the one that tries to enforce version consistency between the libs and the interpreter. I'm currently bisecting to figure out what I hope is the final blocker.

Maybe we should just revert D64443? This is what broke my setup, and led me to introduce to the consistency check in the first place, which merely diagnoses a real problem.

Jul 17 2019, 2:46 PM · Restricted Project
amccarth added a comment to D64881: [Cmake] Use the modern way to find Python when possible.

An aside ...

Jul 17 2019, 2:05 PM · Restricted Project
amccarth added a comment to D62183: [Windows] Fix race condition between state changes.

This change looks fine to me, but I wish there was a reliable way to test it. Is it a true race condition (e.g., because the unpredictability of thread scheduling), or is there a way to write a test that would always fail without this fix?

Jul 17 2019, 11:33 AM · Restricted Project, Restricted Project
amccarth accepted D64851: [NativePDB] Add a FromCompilerDecl for going from lldb -> clang.
Jul 17 2019, 9:09 AM · Restricted Project
amccarth added a comment to D64689: [NativePDB] Make GetOrCreateDeclForUid return an lldb CompilerDecl.

Oh, and thanks for the background and context on the motivation for this change!

Jul 17 2019, 8:12 AM
amccarth accepted D64689: [NativePDB] Make GetOrCreateDeclForUid return an lldb CompilerDecl.

LGTM, but please reconsider whether the return type of GetOrCreateDeclForUid should be changed in this patch rather than in a future one.

Jul 17 2019, 8:11 AM
amccarth added a comment to D64688: [NativePDB] Make GetTranslationUnitDecl return an lldb CompilerDeclCtx.

LGTM.

Jul 17 2019, 8:04 AM

Jul 16 2019

amccarth added a comment to D64812: [CMake] Fail when Python interpreter doesn't match Python libraries version .

OK, the only way I was able to make this work was to remove all traces of Python 2.x from my machine. As long as the older Python existed on my machine CMake would find that one, regardless of which one was in my PATH or indicated by PYTHON_HOME_DIR. Of course, it required killing the cmake cache a couple times, too.

Jul 16 2019, 4:12 PM · Restricted Project, Restricted Project
amccarth added a comment to D64812: [CMake] Fail when Python interpreter doesn't match Python libraries version .
In D64812#1588061, @rnk wrote:
In D64812#1588055, @rnk wrote:

You broke my build. =/ I got this output:

CMake Error at C:/src/llvm-project/lldb/cmake/modules/LLDBConfig.cmake:201 (message):
  Found incompatible Python interpreter (3.7.3) and Python libraries ()

I'll mess with it a bit I guess.

Fixed that in r366247. Apparently Windows has totally custom Python version detection logic.

Jul 16 2019, 3:19 PM · Restricted Project, Restricted Project
amccarth added inline comments to D64824: [CMake] Move standalone check so we don't have to reconfigure LLDB .
Jul 16 2019, 3:07 PM · Restricted Project, Restricted Project
amccarth added a comment to D64821: [CMake] Remove duplicated logic to find Python when doing a standalone build.

Actually, right now I'm trying to figure out where the interpreter is found because cmake is finding different, incompatible versions of the interpreter (2.7) and the libs (3.6).

Jul 16 2019, 3:04 PM · Restricted Project, Restricted Project
amccarth added inline comments to D64688: [NativePDB] Make GetTranslationUnitDecl return an lldb CompilerDeclCtx.
Jul 16 2019, 11:01 AM
amccarth added a comment to D64689: [NativePDB] Make GetOrCreateDeclForUid return an lldb CompilerDecl.

Was there a proposal or discussion (e.g., a thread on lldb-dev) about making PdbAstBuilder abstract in order to handle other languages? It sounds like a good idea, but I'd like to catch up on any context I may have missed.

Jul 16 2019, 10:38 AM

Jul 9 2019

amccarth added a comment to D64444: Add Python 3.6 and 3.7 to the version list.

Be aware that Swig 3.0.12 has a compatibility problem with Python 3.7 that causes hundreds of LLDB tests to fail an assertion (at least on Windows).

Jul 9 2019, 1:52 PM · Restricted Project, Restricted Project

Jun 25 2019

amccarth committed rG17c18a9e8161: Fix a typo in help text. (authored by amccarth).
Fix a typo in help text.
Jun 25 2019, 4:14 PM
amccarth committed rL364361: Fix a typo in help text..
Fix a typo in help text.
Jun 25 2019, 4:14 PM
amccarth added a comment to D63790: [dotest] Add the ability to set environment variables for the inferior..

The LGTM, but I wasn't nominated as a reviewer, and I was mostly looking at it from the point of Windows compatibility.

Jun 25 2019, 4:04 PM · Restricted Project, Restricted Project
amccarth added inline comments to D63790: [dotest] Add the ability to set environment variables for the inferior..
Jun 25 2019, 3:29 PM · Restricted Project, Restricted Project

Jun 24 2019

amccarth added a comment to D62183: [Windows] Fix race condition between state changes.

zturner is not a regular here anymore. I think he pops in from time-to-time, but I wouldn't depend on just him for a review.

Jun 24 2019, 4:45 PM · Restricted Project, Restricted Project

Jun 18 2019

amccarth committed rG46e6e1329875: Fix some lit test ResourceWarnings on Windows (authored by amccarth).
Fix some lit test ResourceWarnings on Windows
Jun 18 2019, 9:36 AM
amccarth committed rL363700: Fix some lit test ResourceWarnings on Windows.
Fix some lit test ResourceWarnings on Windows
Jun 18 2019, 9:34 AM
amccarth closed D63102: Fix some lit test ResourceWarnings on Windows.
Jun 18 2019, 9:34 AM · Restricted Project

Jun 17 2019

amccarth added inline comments to D63102: Fix some lit test ResourceWarnings on Windows.
Jun 17 2019, 4:17 PM · Restricted Project

Jun 14 2019

amccarth added a comment to D63166: Move common functionality from processwindows into processdebugger.

I'm OK with moving common stuff out for now, and I like the separation of ProcessWindows and ProcessDebugger.

Jun 14 2019, 12:03 PM · Restricted Project

Jun 13 2019

amccarth added a comment to D63166: Move common functionality from processwindows into processdebugger.

It's been a while for me, so I'm not super-familiar with the code being changed, but I'm okay with factoring out common code. I agree with labath's open points and will try to look at it in more detail tomorrow.

Jun 13 2019, 4:31 PM · Restricted Project

Jun 11 2019

amccarth added a comment to D63165: Initial support for native debugging of x86/x64 Windows processes.

Sorry for the stupid question, but ...

Jun 11 2019, 4:36 PM · Restricted Project, Restricted Project

Jun 10 2019

amccarth created D63102: Fix some lit test ResourceWarnings on Windows.
Jun 10 2019, 3:48 PM · Restricted Project

Jun 7 2019

amccarth committed rG4ca8435528ca: Fix string literals to avoid deprecation warnings in regexp patterns (authored by amccarth).
Fix string literals to avoid deprecation warnings in regexp patterns
Jun 7 2019, 2:13 PM
amccarth committed rGa4198c22dc1d: NFC: Fix typo in a cmake message (authored by amccarth).
NFC: Fix typo in a cmake message
Jun 7 2019, 2:13 PM
amccarth committed rL362846: Fix string literals to avoid deprecation warnings in regexp patterns.
Fix string literals to avoid deprecation warnings in regexp patterns
Jun 7 2019, 2:12 PM
amccarth closed D62882: Use raw strings to avoid deprecation warnings in regexp patterns.
Jun 7 2019, 2:12 PM · Restricted Project