Page MenuHomePhabricator

dcoughlin (Devin Coughlin)
User

Projects

User does not belong to any projects.

User Details

User Since
May 26 2015, 5:42 PM (229 w, 6 d)

Recent Activity

Yesterday

dcoughlin committed rL375385: Update to use the correct svn username for dcoughlin.
Update to use the correct svn username for dcoughlin
Sun, Oct 20, 6:32 PM
dcoughlin committed rL375383: Request commit access for coughlid.
Request commit access for coughlid
Sun, Oct 20, 6:23 PM

Aug 9 2019

dcoughlin added a reviewer for D65453: [analyzer] Improve the accuracy of the Clang call graph analysis: dergachev.a.
Aug 9 2019, 11:35 AM · Restricted Project, Restricted Project

May 15 2019

dcoughlin accepted D61958: [analyzer] RetainCount: Fix a crash when os_returns_retained_on_zero (_nonzero) is applied to functions that return weird stuff..

Looks good do me. Thanks!

May 15 2019, 11:34 AM · Restricted Project

May 14 2019

dcoughlin accepted D61925: [analyzer] MIGChecker: Add support for os_ref_retain()..

LGTM. Thanks!

May 14 2019, 5:21 PM · Restricted Project

May 7 2019

dcoughlin accepted D61545: [analyzer] Fix a crash in RVO from within blocks..

Looks good to me.

May 7 2019, 3:01 PM · Restricted Project

Apr 25 2019

dcoughlin accepted D61161: [analyzer] RetainCount: Add a suppression for functions that follow "the Matching rule"..

Looks good to me!

Apr 25 2019, 5:03 PM · Restricted Project

Apr 23 2019

dcoughlin accepted D60991: [analyzer] RetainCount: Allow offsets in return values..
Apr 23 2019, 9:34 PM · Restricted Project
dcoughlin accepted D60988: [analyzer] Fix crash when returning C++ objects from ObjC messages-to-nil..
Apr 23 2019, 9:29 PM · Restricted Project
dcoughlin added a comment to D60988: [analyzer] Fix crash when returning C++ objects from ObjC messages-to-nil..

Looks good to me. This is a really interesting corner of Objective-C++!

Apr 23 2019, 9:29 PM · Restricted Project

Apr 15 2019

dcoughlin accepted D60742: [analyzer] RegionStore: Enable loading default bindings from variables..

LGTM.

Apr 15 2019, 5:57 PM · Restricted Project

Mar 17 2019

dcoughlin requested changes to D59465: [analyzer] Add a test plugin for checker option handling.

I'm not sure we really want to add any more examples of plugins.

Mar 17 2019, 8:47 PM · Restricted Project, Restricted Project
dcoughlin added a reviewer for D59465: [analyzer] Add a test plugin for checker option handling: dcoughlin.
Mar 17 2019, 8:44 PM · Restricted Project, Restricted Project
dcoughlin added a reviewer for D57855: [analyzer][NFC] Reimplement checker options: dcoughlin.
Mar 17 2019, 8:41 PM · Restricted Project
dcoughlin added a comment to D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable.

This looks good to me. It is great to see this tested!

Mar 17 2019, 8:38 PM · Restricted Project, Restricted Project
dcoughlin added a reviewer for D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable: dcoughlin.
Mar 17 2019, 8:38 PM · Restricted Project, Restricted Project
dcoughlin requested changes to D57860: [analyzer] Validate checker option names and values.

I think it would be better if the default for compatibility mode were 'true'. That way this change will be backwards compatible and clients who want to enforce stricter checking could enable it. Setting compatibility mode to be true in the driver is not sufficient since many build systems call the frontend directly.

Mar 17 2019, 8:29 PM · Restricted Project, Restricted Project
dcoughlin added a reviewer for D57860: [analyzer] Validate checker option names and values: dcoughlin.
Mar 17 2019, 8:23 PM · Restricted Project, Restricted Project
dcoughlin requested changes to D57858: [analyzer] Add a new frontend flag to display all checker options.

I'm pretty worried about exposing this flag to end users.

Mar 17 2019, 8:06 PM · Restricted Project, Restricted Project
dcoughlin added a reviewer for D57858: [analyzer] Add a new frontend flag to display all checker options: dcoughlin.
Mar 17 2019, 7:30 PM · Restricted Project, Restricted Project

Mar 15 2019

dcoughlin committed rGa61641ef4009: [analyzer] Teach scan-build to find clang when installed in /usr/local/bin/ (authored by dcoughlin).
[analyzer] Teach scan-build to find clang when installed in /usr/local/bin/
Mar 15 2019, 6:01 PM
dcoughlin committed rC356308: [analyzer] Teach scan-build to find clang when installed in /usr/local/bin/.
[analyzer] Teach scan-build to find clang when installed in /usr/local/bin/
Mar 15 2019, 6:00 PM
dcoughlin committed rL356308: [analyzer] Teach scan-build to find clang when installed in /usr/local/bin/.
[analyzer] Teach scan-build to find clang when installed in /usr/local/bin/
Mar 15 2019, 6:00 PM
dcoughlin closed D59406: [analyzer] Teach scan-build to find /usr/bin/clang when installed in /usr/local/bin/.
Mar 15 2019, 6:00 PM · Restricted Project, Restricted Project
dcoughlin updated the diff for D59406: [analyzer] Teach scan-build to find /usr/bin/clang when installed in /usr/local/bin/.

Update to restrict the new behavior to when Xcode is present.

Mar 15 2019, 5:54 PM · Restricted Project, Restricted Project
dcoughlin added a comment to D59406: [analyzer] Teach scan-build to find /usr/bin/clang when installed in /usr/local/bin/.

That's a good point. I've updated the patch to look for 'xcrun'

Mar 15 2019, 5:54 PM · Restricted Project, Restricted Project

Mar 14 2019

dcoughlin created D59406: [analyzer] Teach scan-build to find /usr/bin/clang when installed in /usr/local/bin/.
Mar 14 2019, 9:30 PM · Restricted Project, Restricted Project

Mar 13 2019

dcoughlin accepted D59123: [analyzer] RetainCount: Fix a crash when a function follows retain/autorelease naming convention but takes no arguments..

Sigh. Looks good!

Mar 13 2019, 6:55 PM · Restricted Project

Feb 21 2019

dcoughlin accepted D58529: [analyzer] MIGChecker: Enable by default..

LGTM.

Feb 21 2019, 3:51 PM · Restricted Project

Feb 19 2019

dcoughlin accepted D58397: [analyzer] MIGChecker: Pour more data into the checker..

Aah, MIG_NO_REPLY.

Feb 19 2019, 9:33 PM · Restricted Project, Restricted Project
dcoughlin accepted D58392: [analyzer] MIGChecker: Fix false negatives for releases in automatic destructors..

Oh, interesting. I'm glad you thought of this!

Feb 19 2019, 9:07 PM · Restricted Project, Restricted Project
dcoughlin accepted D58368: [analyzer] MIGChecker: Implement bug reporter visitors..

Looks good to me. I have some a minor diagnostic wording suggestion in line.

Feb 19 2019, 9:03 PM · Restricted Project, Restricted Project
dcoughlin accepted D58366: [analyzer] MIGChecker: Make use of the server routine annotation..

LGTM.

Feb 19 2019, 8:41 PM · Restricted Project
dcoughlin accepted D57558: [analyzer] MIGChecker: A checker for Mach Interface Generator calling convention..
Feb 19 2019, 8:34 PM · Restricted Project
dcoughlin added a comment to D57558: [analyzer] MIGChecker: A checker for Mach Interface Generator calling convention..

LGTM with a minor grammatical nits inline. It is great to see a checker for this!

Feb 19 2019, 8:34 PM · Restricted Project
dcoughlin accepted D58365: [attributes] Add a MIG server routine attribute..

This looks good to me as long as Aaron is happy with it.

Feb 19 2019, 8:19 PM · Restricted Project, Restricted Project

Feb 8 2019

dcoughlin accepted D57981: [analyzer] strlcat() syntax check: Fix an off-by-one error..

Looks good to me.

Feb 8 2019, 3:24 PM · Restricted Project, Restricted Project

Dec 20 2018

dcoughlin accepted D55907: [analyzer] RetainCount: Bluntly suppress the CFRetain detection heuristic on a couple of CM functions..

This seems reasonable to me, but please have George take a look too.

Dec 20 2018, 2:14 PM

Dec 19 2018

dcoughlin accepted D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types..

Looks good to me.

Dec 19 2018, 3:43 PM

Dec 18 2018

dcoughlin added a comment to D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types..

These seems reasonable, although it does also seem like there could be quite a few unintended consequences that we haven't discovered yet.

Dec 18 2018, 8:38 PM
dcoughlin accepted D55873: [analyzer] CStringChecker: Fix a crash when an argument of a weird type is encountered..

LGTM.

Dec 18 2018, 8:18 PM
dcoughlin accepted D55804: [analyzer] C++17: Fix leak false positives when an object with destructor is returned from the top frame..

This seems reasonable to me, although I have a question inline about why you are using makeZeroElementRegion().

Dec 18 2018, 8:13 PM

Dec 17 2018

dcoughlin added a comment to D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure..

This looks good to me. It is great to see a dumper for this!

Dec 17 2018, 2:16 PM
dcoughlin added a comment to D55671: [analyzer] Don't pretend that unknown block calls have one null parameter..

Looks good to me!

Dec 17 2018, 1:49 PM

Nov 29 2018

dcoughlin added a comment to D54560: [analyzer] MoveChecker Pt.3: Improve warning messages a bit..

Looks good! Some suggested minor tweaks to diagnostic text inline.

Nov 29 2018, 10:24 AM

Aug 1 2018

dcoughlin added inline comments to D50124: [analyzer] Record nullability implications on getting items from NSDictionary.
Aug 1 2018, 1:47 PM

Jul 31 2018

dcoughlin added a comment to D50103: [analyzer] Warn when a nil subscript is used on NSDictionary, as that crashes at runtime.

My mistake, sorry about that!

Jul 31 2018, 6:33 PM

Jul 23 2018

dcoughlin added inline comments to D49528: [analyzer] Syntactic matcher for leaks associated with run loop and autoreleasepool.
Jul 23 2018, 9:56 PM
dcoughlin added a comment to D49528: [analyzer] Syntactic matcher for leaks associated with run loop and autoreleasepool.

It is great to see a check for this!

Jul 23 2018, 9:50 PM

Jul 16 2018

dcoughlin added a comment to D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker.

It is really nice to see this checker take shape! Some drive by diagnostic comments in line.

Jul 16 2018, 6:26 PM

Jun 12 2018

dcoughlin added a comment to D48110: [analyzer] [WIP] Checker for detecting that the function was called with more function arguments than it could take.

This seems like a useful checker!

Jun 12 2018, 11:05 PM

May 2 2018

dcoughlin added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

Do you have some better choices?

I could do -allow-alpha-checks. What do you think?

May 2 2018, 1:45 PM

Apr 27 2018

dcoughlin added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

Note that it is completely off by default, and is not listed in documentation, clang-tidy --help,
so one would have to know of the existence of that hidden flag first, and only then when that flag is passed, those alpha checks could be enabled.

Apr 27 2018, 11:24 AM
dcoughlin added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

I'm curious about the use case for this, since I don't think it ever makes sense to run all the alpha checks at once. These checks are typically a work in progress (they're not finished yet) and often have false positives that haven't been fixed yet. Often they don't handle all facets of the language and can crash on unhandled constructs. While it makes sense to enable a specific alpha check (or even a set of related alpha checks) when developing these checks or when evaluating whether they are ready to graduate into non-alpha it seems to me that running all of them isn't useful, ever.

Apr 27 2018, 10:55 AM

Apr 22 2018

dcoughlin added reviewers for D45718: [analyzer] Allow registering custom statically-linked analyzer checkers: NoQ, xazax.hun, dcoughlin.
Apr 22 2018, 9:58 AM

Mar 29 2018

dcoughlin accepted D44722: [analyzer] Path-insensitive checker for writes into an auto-releasing pointer from the wrong pool.

Looks good with updated diagnostic text and a test for non-ARC behavior.

Mar 29 2018, 10:59 AM

Mar 21 2018

dcoughlin accepted D44341: [analyzer] Trust _Nonnull annotations for system framework.

Looks good to me.

Mar 21 2018, 9:39 PM
dcoughlin accepted D44653: [analyzer] Extend GCDAntipatternChecker to match group_enter/group_leave pattern.

Looks good to me!

Mar 21 2018, 9:34 PM
dcoughlin accepted D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts..

LGTM. I really appreciate the extra documentation you've added, too.

Mar 21 2018, 9:31 PM

Mar 14 2018

dcoughlin added inline comments to D44341: [analyzer] Trust _Nonnull annotations for system framework.
Mar 14 2018, 11:27 AM
dcoughlin accepted D44409: [analyzer] Fix crashes in RetainCountChecker when underlying region is not a var.

Looks good. It is nice to see this fixed. There is one testing nit inline

Mar 14 2018, 10:56 AM

Mar 9 2018

dcoughlin accepted D44228: [analyzer] Move the GCDAsyncSemaphoreChecker to optin.performance.

LGTM, but I do think Artem is right that we should push the user in the right direction here. There is a suggested addendum inline.

Mar 9 2018, 2:48 PM
dcoughlin added a comment to D44228: [analyzer] Move the GCDAsyncSemaphoreChecker to optin.performance.

I put some post-commit comments in https://reviews.llvm.org/D44059 about diagnostic text and naming that you should address as part of graduating this out of alpha.

Mar 9 2018, 10:31 AM
dcoughlin added a comment to D44059: [analyzer] AST-matching checker to detect global central dispatch performance anti-pattern.

This looks good. Some minor post-commit review inline.

Mar 9 2018, 10:21 AM

Mar 5 2018

dcoughlin accepted D44124: [analyzer] Support destruction and lifetime-extension of inlined function return values..

LGTM!

Mar 5 2018, 5:12 PM
dcoughlin accepted D44120: [CFG] [analyzer] Heavier CFGCXXRecordTypedCall elements..

This looks good to me. I'm not super happy with the name "CFGValueTypedCall" since it doesn't make it obvious that is reflects "a function call that returns a C++ object by value."

Mar 5 2018, 5:05 PM

Feb 28 2018

dcoughlin accepted D43804: [analyzer] Enable cfg-temporary-dtors by default?.

Yay! This looks good to me.

Feb 28 2018, 4:48 PM

Feb 27 2018

dcoughlin accepted D30691: [analyzer] Support for naive cross translational unit analysis.

Thanks Gabor, this looks good to me. Please commit!

Feb 27 2018, 8:13 PM
dcoughlin accepted D43798: [analyzer] UndefinedAssignment: Fix warning message on implicit copy/move constructors..

LGTM. I have a suggestion for a slight modification of the diagnostic text inline.

Feb 27 2018, 1:50 PM

Feb 25 2018

dcoughlin committed rL326062: [www] Update link to analyzer's "Building a Checker in 24 hours" video.
[www] Update link to analyzer's "Building a Checker in 24 hours" video
Feb 25 2018, 4:42 PM
dcoughlin committed rC326062: [www] Update link to analyzer's "Building a Checker in 24 hours" video.
[www] Update link to analyzer's "Building a Checker in 24 hours" video
Feb 25 2018, 4:42 PM

Feb 24 2018

dcoughlin accepted D43714: [analyzer] Don't do anything when trivial-copying an empty class object..

LGTM.

Feb 24 2018, 12:02 PM
dcoughlin accepted D43689: [analyzer] Disable constructor inlining when lifetime extension through fields occurs..
Feb 24 2018, 11:08 AM
dcoughlin added inline comments to D43666: [analyzer] When constructing a temporary without construction context, track it for destruction anyway..
Feb 24 2018, 10:48 AM
dcoughlin added a comment to D43421: [analyzer] Do not analyze bison-generated files.

Actually, after posting this I've realized that this behavior is somewhat inconsistent, and we should still produce an empty plist when requested to: just as we do for files with no generated reports.

Actually, after thinking about this some more, the answer is those plists should not be produced, as they are not produced in similar cases (disableAllChecks is set, etc).

Feb 24 2018, 9:02 AM
dcoughlin added a comment to D43218: [analyzer] Quickfix: do not overflow in calculating offset in RegionManager.

...turns out the previous version had a bug. Re-introduced logging, as I've needed that for the second time for debugging.

Feb 24 2018, 8:58 AM

Feb 20 2018

dcoughlin accepted D43483: [CFG] [analyzer] Add construction context when the constructor is on a branch of a ternary operator.

This seems reasonable to me.

Feb 20 2018, 9:03 PM
dcoughlin accepted D43481: [CFG] [analyzer] Add construction context when the constructor is being no-op-casted to a const value type..
Feb 20 2018, 8:52 PM
dcoughlin accepted D43480: [CFG] [analyzer] Add construction context when the constructor is treated like a functional cast..
Feb 20 2018, 8:50 PM
dcoughlin accepted D43477: [CFG] [analyzer] Add MaterializeTemporaryExpr into the construction context..

LGTM.

Feb 20 2018, 8:44 PM
dcoughlin accepted D43428: [CFG] [analyzer] NFC: Allow more complicated construction contexts..

It seems kind of sketchy to me that we're recursing over an expression to find construction contexts and then later doing it again for sub-expressions. I guess there is precedent here with VisitForTemporaryDtors(), but we should think about whether there is a better way to do this.

Feb 20 2018, 8:39 PM
dcoughlin accepted D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases..

Looks good to me. I have a comment about simplifying createTemporaryRegionIfNeeded() (if possible) inline.

Feb 20 2018, 7:56 PM
dcoughlin added a comment to D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases..

This looks great to me Artem! I'll be very curious to see how you extend this to handle initializer lists.

Feb 20 2018, 7:08 PM
dcoughlin accepted D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases..
Feb 20 2018, 7:08 PM

Feb 15 2018

dcoughlin committed rL325314: Add static analyzer projects for GSoC 2018 to the open projects page.
Add static analyzer projects for GSoC 2018 to the open projects page
Feb 15 2018, 8:13 PM

Feb 14 2018

dcoughlin added a comment to D43218: [analyzer] Quickfix: do not overflow in calculating offset in RegionManager.

We have a comment documenting the decision. Adding a printf doesn't seem like it would help very much here and adds untested code. I don't think we should have a policy of adding these printfs on every early return. Since these printfs make it harder to read the code and are duplicated with the comments, I think we should avoid adding one until we know it is actually generally useful.

Feb 14 2018, 11:15 AM

Feb 12 2018

dcoughlin added inline comments to D43218: [analyzer] Quickfix: do not overflow in calculating offset in RegionManager.
Feb 12 2018, 9:39 PM

Feb 9 2018

dcoughlin added a comment to D43138: [analyzer] Update the documentation to show the command line option.

We don't consider --analyze to be public UI, which is why it is not documented. The supported interface to the analyzer is scan-build.

Feb 9 2018, 5:56 PM

Feb 8 2018

dcoughlin added inline comments to D43104: [analyzer] Find correct region for simple temporary destructor calls and inline them if possible..
Feb 8 2018, 6:30 PM

Feb 7 2018

dcoughlin accepted D42779: [analyzer] NFC: Make sure we don't ever inline the constructor for which the temporary destructor is noreturn and missing..

This looks good to me, but I think you should include your explanatory comments in the commit message to the comment itself to help future violators of the assertion out!

Feb 7 2018, 3:47 PM

Feb 2 2018

dcoughlin accepted D42672: [CFG] [analyzer] Heavier CFGConstructor elements..

This looks good to me, although as I noted inline I think both the name and the comment for VisitForConstructionContext() are confusing. If you can be more precise I think it would help future maintainers.

Feb 2 2018, 1:10 PM

Jan 31 2018

dcoughlin added a comment to D42777: [analyzer] Fix yet-another-crash in body-farming std::call_once.

This seems reasonable to me.

Jan 31 2018, 11:50 PM
dcoughlin added a comment to D42699: [CFG] [analyzer] Add construction context for DeclStmt..

It is nice to see how cleanly this slides in.

Jan 31 2018, 11:47 PM
dcoughlin added inline comments to D42700: [CFG] [analyzer] Add construction context for CXXCtorInitializer..
Jan 31 2018, 11:47 PM
dcoughlin added a comment to D42672: [CFG] [analyzer] Heavier CFGConstructor elements..

I think it will be great to have a more explicit representation of construction in the CFG! Comments in line.

Jan 31 2018, 11:35 PM
dcoughlin accepted D42457: [analyzer] Don't communicate evaluation failures through memregion hierarchy..

This looks good to me. However, I do think you should take George's suggestion to have makeZeroElementRegion() have a boolean out parameter rather than a EvalCallOptions out parameter. This avoids unnecessarily coupling of makeZeroElementRegion() and EvalallOptions. You will need to make the boolean fields not bitfields (since we can't take a bitfield's address in C++). But EvalCallOptions is only on the stack (and we can pass it by const reference) so I don't think the increased size is something we should worry about.

Jan 31 2018, 6:17 PM

Jan 30 2018

dcoughlin accepted D42456: [analyzer] Fix detection of function-like macros.

Looks good to me.

Jan 30 2018, 3:20 PM

Jan 24 2018

dcoughlin added a comment to D42269: [analyzer] Show full analyzer invocation for reproducibility in HTML reports.

Remember that html reports are not just used by analyzer developers but also by end users. For end users, the clang invocation is an implementation detail of the analyzer and not part of its user model. We shouldn't display this to users.

Jan 24 2018, 9:04 PM

Jan 21 2018

dcoughlin added inline comments to D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy.
Jan 21 2018, 2:19 PM
dcoughlin accepted D41816: [analyzer] Model and check unrepresentable left shifts.

Looks good to me, thanks!

Jan 21 2018, 1:48 PM