Page MenuHomePhabricator

amccarth (Adrian McCarthy)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2015, 9:26 AM (230 w, 1 d)

Recent Activity

Today

amccarth committed rG17c18a9e8161: Fix a typo in help text. (authored by amccarth).
Fix a typo in help text.
Tue, Jun 25, 4:14 PM
amccarth committed rL364361: Fix a typo in help text..
Fix a typo in help text.
Tue, Jun 25, 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.

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

Yesterday

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.

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

Tue, Jun 18

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

Mon, Jun 17

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

Fri, Jun 14

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.

Fri, Jun 14, 12:03 PM · Restricted Project

Thu, Jun 13

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.

Thu, Jun 13, 4:31 PM · Restricted Project

Tue, Jun 11

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

Sorry for the stupid question, but ...

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

Mon, Jun 10

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

Fri, Jun 7

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
Fri, Jun 7, 2:13 PM
amccarth committed rGa4198c22dc1d: NFC: Fix typo in a cmake message (authored by amccarth).
NFC: Fix typo in a cmake message
Fri, Jun 7, 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
Fri, Jun 7, 2:12 PM
amccarth closed D62882: Use raw strings to avoid deprecation warnings in regexp patterns.
Fri, Jun 7, 2:12 PM · Restricted Project
amccarth committed rG4447d15aef08: Fix lit tests on Windows related to CR+LF (authored by amccarth).
Fix lit tests on Windows related to CR+LF
Fri, Jun 7, 2:11 PM
amccarth committed rL362845: NFC: Fix typo in a cmake message.
NFC: Fix typo in a cmake message
Fri, Jun 7, 2:11 PM
amccarth committed rL362844: Fix lit tests on Windows related to CR+LF.
Fix lit tests on Windows related to CR+LF
Fri, Jun 7, 2:10 PM
amccarth closed D62759: Fix lit tests on Windows related to CR.
Fri, Jun 7, 2:10 PM · Restricted Project
amccarth added inline comments to D62882: Use raw strings to avoid deprecation warnings in regexp patterns.
Fri, Jun 7, 2:09 PM · Restricted Project
amccarth updated the diff for D62882: Use raw strings to avoid deprecation warnings in regexp patterns.

Changed the approach in the first fix to use explicit escaping after Joel Denny's comment got me to re-think it.

Fri, Jun 7, 1:36 PM · Restricted Project
amccarth added a comment to D62882: Use raw strings to avoid deprecation warnings in regexp patterns.

But how do I see the bad behavior or deprecation warnings? Python 3.6.7 with -ddd didn't reveal anything on a simple example I just tried.

Fri, Jun 7, 10:49 AM · Restricted Project

Tue, Jun 4

amccarth created D62882: Use raw strings to avoid deprecation warnings in regexp patterns.
Tue, Jun 4, 3:53 PM · Restricted Project
amccarth added a reviewer for D62759: Fix lit tests on Windows related to CR: stella.stamenova.
Tue, Jun 4, 3:38 PM · Restricted Project

Fri, May 31

amccarth created D62759: Fix lit tests on Windows related to CR.
Fri, May 31, 4:19 PM · Restricted Project

May 22 2019

amccarth accepted D60962: [NativePDB] Extend .pdb files search folders.
May 22 2019, 3:30 PM · Restricted Project

May 9 2019

amccarth accepted D61733: Breakpad: Generate unwind plans from STACK CFI records.

LGTM once you double-check the return value in the error case at the end of SymbolFileBreakpad::ParseUnwindRow.

May 9 2019, 1:05 PM · Restricted Project

May 8 2019

amccarth added inline comments to D61686: Enable lldb-server on Windows.
May 8 2019, 12:48 PM · Restricted Project
amccarth added inline comments to D61686: Enable lldb-server on Windows.
May 8 2019, 11:33 AM · Restricted Project

Apr 30 2019

amccarth accepted D61311: PostfixExpression: Use signed integers in IntegerNode.

This looks fine to me, and the rationale sounds right. I'll be curious to see if any other reviewers see a potential problem with this.

Apr 30 2019, 7:47 AM · Restricted Project

Apr 29 2019

amccarth accepted D61183: PostfixExpression: Introduce CFANode.

LGTM, but I found one comment a bit confusing for me.

Apr 29 2019, 1:01 PM · Restricted Project
amccarth accepted D61128: Support member function types in PdbAstBuilder.

Thanks for the improved commit message. Again, sorry about the delay.

Apr 29 2019, 11:26 AM · Restricted Project

Apr 25 2019

amccarth added a comment to D61128: Support member function types in PdbAstBuilder.

Please add some detail to the commit message and consider re-titling it. It looks like this patch is actually adding missing functionality rather than "fixing" a bug in how structs are handled. If that's correct, then please make the commit message reflect that.

Apr 25 2019, 5:18 PM · Restricted Project
amccarth accepted D61056: PostfixExpression: move DWARF generator out of NativePDB internals.

Clever (hopefully not too clever)! Not having to derive a special class from Visitor is really nice.

Apr 25 2019, 11:04 AM · Restricted Project

Apr 24 2019

amccarth added a comment to D61056: PostfixExpression: move DWARF generator out of NativePDB internals.

Can the test deletions be a separate patch, or will they fail because of the other changes in this patch? They feel like a good but separable step.

Apr 24 2019, 4:38 PM · Restricted Project
amccarth added a comment to D60962: [NativePDB] Extend .pdb files search folders.

Thanks Pavel!

Apr 24 2019, 9:15 AM · Restricted Project

Apr 23 2019

amccarth accepted D61003: PostfixExpression: move parser out of NativePDB internals.

Nice. Thanks for adding the test, too!

Apr 23 2019, 3:48 PM · Restricted Project
amccarth added a comment to D60962: [NativePDB] Extend .pdb files search folders.

cc: Pavel Labath because I know he's been involved in conversations about how to find symbol files in general (PDB, split DWARF, Breakpad, etc.), especially when doing post-mortem debugging.

Apr 23 2019, 3:39 PM · Restricted Project
amccarth updated subscribers of D60962: [NativePDB] Extend .pdb files search folders.
Apr 23 2019, 3:30 PM · Restricted Project
amccarth accepted D61000: Kill modify-python-lldb.py.
Apr 23 2019, 3:26 PM · Restricted Project

Apr 19 2019

amccarth accepted D60817: [NativePDB] Add anonymous namespaces support.

Thanks for the changes. If the tests pass, then this LGTM. Just check the one last question I added about the AC1D test.

Apr 19 2019, 8:18 AM · Restricted Project, Restricted Project

Apr 18 2019

amccarth added inline comments to D60817: [NativePDB] Add anonymous namespaces support.
Apr 18 2019, 1:39 PM · Restricted Project, Restricted Project
amccarth added a comment to D60817: [NativePDB] Add anonymous namespaces support.

Sorry for the slow response; I'm still learning about this code.

Apr 18 2019, 1:35 PM · Restricted Project, Restricted Project

Apr 17 2019

amccarth added a comment to D60519: [Windows] Dump more information about access violation exception.

Thanks for the new test and the test fix. It's unfortunate that the tests are so sensitive to an arbitrary buffer size.

Apr 17 2019, 3:31 PM · Restricted Project, Restricted Project
amccarth accepted D60405: MinidumpYAML: Add support for ModuleList stream.

Nice!

Apr 17 2019, 3:23 PM · Restricted Project
amccarth accepted D60667: Allow direct comparison of ConstString against StringRef.

I, too, have some concern that this could have unintended side effects. To make the temporary StringRefs from the zero-terminated strings requires a strlen call each time. So we're making two passes over a string each time (once to measure and once to compare). Granted, these are mostly very short.

I think I'm not understanding something here. There is one strlen call to make the StringRef from the literal, but I don't see why we need a second one to compare (as StringRef and ConstString both store the string length and don't recompute it when comparing). Simple godbolt test case here: https://godbolt.org/z/9X1RRt So both the old and new version both require one strlen call from what I can see (which is anyway optimized out for comparing against literals).

Apr 17 2019, 8:36 AM · Restricted Project, Restricted Project

Apr 15 2019

amccarth added a comment to D60667: Allow direct comparison of ConstString against StringRef.

I, too, have some concern that this could have unintended side effects. To make the temporary StringRefs from the zero-terminated strings requires a strlen call each time. So we're making two passes over a string each time (once to measure and once to compare). Granted, these are mostly very short.

Apr 15 2019, 1:24 PM · Restricted Project, Restricted Project

Apr 11 2019

amccarth accepted D60410: PDBFPO: Improvements to the AST visitor.

I'm even happier. Thanks for giving the params more specific names.

Apr 11 2019, 1:18 PM · Restricted Project

Apr 10 2019

amccarth accepted D60501: Minidump: extend UUID byte-swapping to windows platform.
Apr 10 2019, 4:54 PM · Restricted Project
amccarth added a comment to D60519: [Windows] Dump more information about access violation exception.

Does this affect any existing tests? Is there a good way to test it?

Apr 10 2019, 10:53 AM · Restricted Project, Restricted Project
amccarth accepted D60498: Clean up docstrings in swig interface files.
Apr 10 2019, 10:29 AM · Restricted Project
amccarth added a comment to D60498: Clean up docstrings in swig interface files.

LGTM, though I'm not clear why this extra visual noise was there in the first place. Does doxygen depend on this style when reading the swigged API?

Apr 10 2019, 9:51 AM · Restricted Project

Apr 8 2019

amccarth accepted D60410: PDBFPO: Improvements to the AST visitor.

Overall, I'm ambivalent.

Apr 8 2019, 4:25 PM · Restricted Project

Apr 5 2019

amccarth added a comment to D60268: Breakpad: Parse Stack CFI records.

My concerns were address, so LGTM. I'll leave the rest to you and clayborg.

Apr 5 2019, 7:49 AM · Restricted Project

Apr 4 2019

amccarth accepted D60271: PDBFPO: Use references instead of pointers, where possible.

I noticed this also deleted two overloads of Visit from FPOProgramASTVisitorDWARFCodegen, but that appears to be harmless (the base class overloads were also no-ops).

Apr 4 2019, 9:42 AM · Restricted Project
amccarth added a comment to D60268: Breakpad: Parse Stack CFI records.

This looks good. I have a few questions inline, but none of those are major concerns.

Apr 4 2019, 9:23 AM · Restricted Project

Apr 3 2019

amccarth committed rGffa857c7a655: Fix and simplify PrepareCommandsForSourcing (authored by amccarth).
Fix and simplify PrepareCommandsForSourcing
Apr 3 2019, 12:48 PM
amccarth committed rLLDB357626: Fix and simplify PrepareCommandsForSourcing.
Fix and simplify PrepareCommandsForSourcing
Apr 3 2019, 12:48 PM
amccarth committed rL357626: Fix and simplify PrepareCommandsForSourcing.
Fix and simplify PrepareCommandsForSourcing
Apr 3 2019, 12:48 PM
amccarth closed D60152: Fix and simplify PrepareCommandsForSourcing.
Apr 3 2019, 12:48 PM · Restricted Project
amccarth accepted D60195: modify-python-lldb.py: (Re)move __len__ and __iter__ support.

Nice! Thanks for cleaning up the affected comments as well.

Apr 3 2019, 9:11 AM · Restricted Project
amccarth updated the diff for D60152: Fix and simplify PrepareCommandsForSourcing.

Further simplification per labath's feedback.

Apr 3 2019, 9:08 AM · Restricted Project
amccarth added a comment to D60152: Fix and simplify PrepareCommandsForSourcing.

Thanks for the review.

Apr 3 2019, 8:49 AM · Restricted Project

Apr 2 2019

amccarth accepted D60119: modify-python-lldb.py: clean up __iter__ and __len__ support.
Apr 2 2019, 4:50 PM · Restricted Project
amccarth added reviewers for D60152: Fix and simplify PrepareCommandsForSourcing: zturner, labath, rnk.
Apr 2 2019, 2:15 PM · Restricted Project
amccarth created D60152: Fix and simplify PrepareCommandsForSourcing.
Apr 2 2019, 2:15 PM · Restricted Project

Apr 1 2019

amccarth accepted D60068: PDBFPO: Refactor register reference resolution.
Apr 1 2019, 2:51 PM · Restricted Project
amccarth added inline comments to D60094: [MSVC] If unable to find link.exe from a MSVC installation, look for link.exe next to cl.exe.
Apr 1 2019, 2:24 PM · Restricted Project

Mar 26 2019

amccarth added a comment to D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary.

The Windows code looks correct to me, but I've added a couple inline questions/comments.

Mar 26 2019, 5:01 PM · Restricted Project

Mar 12 2019

amccarth committed rG38d4a6c496e3: Correcting some comments in PdbIndex.cpp [NFC] (authored by amccarth).
Correcting some comments in PdbIndex.cpp [NFC]
Mar 12 2019, 10:40 AM
amccarth committed rL355943: Correcting some comments in PdbIndex.cpp [NFC].
Correcting some comments in PdbIndex.cpp [NFC]
Mar 12 2019, 10:40 AM
amccarth committed rLLDB355943: Correcting some comments in PdbIndex.cpp [NFC].
Correcting some comments in PdbIndex.cpp [NFC]
Mar 12 2019, 10:40 AM

Mar 6 2019

amccarth accepted D58975: Introduce MinidumpEnums.def textual header.
Mar 6 2019, 11:44 AM

Feb 28 2019

amccarth closed D58648: Improve process launch comments for Windows.

Closed with r355121.

Feb 28 2019, 11:16 AM
amccarth committed rG34f2bee0fb04: Improve process launch comments for Windows (authored by amccarth).
Improve process launch comments for Windows
Feb 28 2019, 11:16 AM
amccarth committed rL355121: Improve process launch comments for Windows.
Improve process launch comments for Windows
Feb 28 2019, 11:15 AM
amccarth committed rLLDB355121: Improve process launch comments for Windows.
Improve process launch comments for Windows
Feb 28 2019, 11:15 AM
amccarth added a comment to D58648: Improve process launch comments for Windows.

Thanks. I really don't know what other types of conditions may trigger that "request is not supported" message, so I'm going to shy away from trying to make it more specific.

Feb 28 2019, 10:37 AM

Feb 27 2019

amccarth added reviewers for D58648: Improve process launch comments for Windows: labath, lemo.
Feb 27 2019, 3:35 PM

Feb 25 2019

amccarth updated the diff for D58648: Improve process launch comments for Windows.

Forgot to include context in original diff.

Feb 25 2019, 3:21 PM
amccarth created D58648: Improve process launch comments for Windows.
Feb 25 2019, 3:15 PM

Feb 11 2019

amccarth added a comment to D56537: ObjectFilePECOFF: Create a "container" section spanning the entire module image.

My knowledge of PECOFF is even more limited, but it's my understanding that the ImageBase is the _preferred_ address for the module. That doesn't guarantee that's the actual address it would be loaded at. Not just because of ASLR but also because there may be conflicts with modules that have already been loaded. Is GetBaseAddress supposed to return the actual base address or the preferred one?

Feb 11 2019, 9:21 AM · Restricted Project, Restricted Project

Feb 8 2019

amccarth accepted D57895: Breakpad: auto-detect path style of file entries.

I think absolute_path is great. Thanks for checking on the native/None. I didn't want there to be any latent surprises.

Feb 8 2019, 8:06 AM · Restricted Project

Feb 7 2019

amccarth added inline comments to D57895: Breakpad: auto-detect path style of file entries.
Feb 7 2019, 2:52 PM · Restricted Project

Feb 5 2019

amccarth accepted D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules.

Nice.

Feb 5 2019, 11:09 AM · Restricted Project

Jan 10 2019

amccarth added inline comments to D56461: [NativePDB] Add support for parsing typedefs and make lldb-test work with the native reader..
Jan 10 2019, 4:59 PM

Jan 9 2019

amccarth added a comment to D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit.

So this Patch is effectively NFC, since no caller (not even a test) was using the functionality you've removed. Seems like a nice refactor.

Jan 9 2019, 9:10 AM

Jan 8 2019

amccarth added inline comments to D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext.
Jan 8 2019, 2:32 PM

Dec 6 2018

amccarth added a comment to D55344: Support skewed Stream Arrays.

If there is a magic value, should we be checking it?

Dec 6 2018, 11:38 AM
amccarth accepted D55356: Add a method to get the "base" file address of an object file.

LGTM.

Dec 6 2018, 11:05 AM

Nov 30 2018

amccarth added a comment to D55142: Minidump debugging using the native PDB reader.

This looks good to me, save for a couple comment nits.

Nov 30 2018, 4:31 PM · Restricted Project

Jul 19 2018

amccarth added inline comments to D49552: Add a Microsoft Demangler library and utility..
Jul 19 2018, 10:22 AM

Jul 12 2018

amccarth accepted D49202: Restructure the minidump loading path and add early & explicit consistency checks.

LGTM if you don't want to consider my remaining suggestion for this patch.

Jul 12 2018, 8:37 AM · Restricted Project

Jul 11 2018

amccarth added inline comments to D49202: Restructure the minidump loading path and add early & explicit consistency checks.
Jul 11 2018, 3:23 PM · Restricted Project
amccarth added a comment to D49202: Restructure the minidump loading path and add early & explicit consistency checks.

Also, I'm not seeing tests for the other consistency checks you're adding (like whether there are any streams or whether the streams overlap). Is it difficult to create such malformed minidumps?

Jul 11 2018, 3:22 PM · Restricted Project
amccarth added inline comments to D49202: Restructure the minidump loading path and add early & explicit consistency checks.
Jul 11 2018, 2:18 PM · Restricted Project
amccarth added inline comments to D48271: [llvm-readobj] Fix printing format.
Jul 11 2018, 8:41 AM

Jul 10 2018

amccarth added inline comments to D48271: [llvm-readobj] Fix printing format.
Jul 10 2018, 11:00 AM