Page MenuHomePhabricator

martong (Gabor Marton)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 10 2017, 8:01 AM (147 w, 5 d)

Recent Activity

Fri, Aug 7

martong added inline comments to D85319: [analyzer][RFC] Get info from the LLVM IR for precision.
Fri, Aug 7, 1:30 AM · Restricted Project

Wed, Aug 5

martong added inline comments to D85319: [analyzer][RFC] Get info from the LLVM IR for precision.
Wed, Aug 5, 12:17 PM · Restricted Project
martong retitled D85319: [analyzer][RFC] Get info from the LLVM IR for precision from [analyzer] Get info from the LLVM IR for precision to [analyzer][RFC] Get info from the LLVM IR for precision.
Wed, Aug 5, 12:04 PM · Restricted Project
martong added a comment to D85319: [analyzer][RFC] Get info from the LLVM IR for precision.

This seems a huge architectural change that we need to talk about.

Wed, Aug 5, 12:02 PM · Restricted Project
martong added a comment to D85319: [analyzer][RFC] Get info from the LLVM IR for precision.

What will happen with the ability to analyse a translation unit whose target was not part of LLVM_TARGETS_TO_BUILD of the current clang binary? Will it break, because the binary lacks the information on how to generate for the given target?

Wed, Aug 5, 9:17 AM · Restricted Project
martong added inline comments to D85319: [analyzer][RFC] Get info from the LLVM IR for precision.
Wed, Aug 5, 9:14 AM · Restricted Project
martong added a reviewer for D85319: [analyzer][RFC] Get info from the LLVM IR for precision: xazax.hun.
Wed, Aug 5, 8:43 AM · Restricted Project
martong updated the diff for D85319: [analyzer][RFC] Get info from the LLVM IR for precision.
  • Remove dummy test file
Wed, Aug 5, 8:32 AM · Restricted Project
martong requested review of D85319: [analyzer][RFC] Get info from the LLVM IR for precision.
Wed, Aug 5, 8:27 AM · Restricted Project

Tue, Jul 28

martong accepted D83992: [ASTImporter] Add Visitor for TypedefNameDecl's.

LGTM! Thanks for the test!

Tue, Jul 28, 8:38 AM · Restricted Project

Mon, Jul 27

martong added inline comments to D83997: [os_log] Improve the way we extend the lifetime of objects passed to __builtin_os_log_format.
Mon, Jul 27, 9:15 AM · Restricted Project
martong accepted D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC)..

AFAIK, BuiltinBug is deprecated and we should be using BugType anyway.
LGTM!

Mon, Jul 27, 9:07 AM · Restricted Project
martong added a comment to D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute.

Also, I would like to add that the current test present in this diff does not fail in the existing system but it was the best I and Vassil could come up with to replicate our problem.

Is there a way to run something like creduce to come up with a test case that does replicate the issue? I'd like to ensure we have a test case that fails without the patch applied so that we can be sure we don't regress the behavior.

Not easily. That bug comes from internal infrastructure which uses clang's API and happens when calling directly the mangler. Running creduce would require failure to be reproducible with bare clang. I was hoping @rsmith had something up his sleeve.

Drat. While the code looks reasonable, this makes me less confident in the fix. I'm adding some more reviewers who have touched the AST serialization and modules before in case they also have ideas for test cases.

Mon, Jul 27, 9:02 AM · Restricted Project

Fri, Jul 24

martong added inline comments to D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics.
Fri, Jul 24, 2:34 AM · Restricted Project
martong updated the diff for D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics.
  • Use Twine
Fri, Jul 24, 2:34 AM · Restricted Project
martong added a comment to D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics.

Sure, this is an improvement because we display more information, but I'd argue that it isn't really a more readable warning message :) How about

<argno>th argument to the call to <function name>

  • cannot be represented with a character
  • is a null pointer
  • ...

, which violates the function's preconditions.

WDYT?

Fri, Jul 24, 2:31 AM · Restricted Project
martong updated the diff for D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics.
  • Rebase to master
Fri, Jul 24, 1:53 AM · Restricted Project
martong accepted D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition.

Unlike RecordDecl case which uses DefinitionCompleter to force completeDefinition we don't seem to have a similar mechanism for ObjCInterfaceDecl.

It is unfortunate that the AST does not have something similar for ObjC interfaces.
So, this seems to be the right way unless we want to modify DeclObjC.h.
LGTM!

Fri, Jul 24, 12:45 AM · Restricted Project, Restricted Project

Thu, Jul 23

Herald added a project to D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions: Restricted Project.
Thu, Jul 23, 7:59 AM · Restricted Project

Tue, Jul 21

Herald added a project to D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions: Restricted Project.
Tue, Jul 21, 9:57 AM · Restricted Project
martong accepted D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests.

Ok, LGTM! Thanks!

Tue, Jul 21, 6:08 AM · Restricted Project

Mon, Jul 20

martong committed rG3ff220de9009: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions (authored by martong).
[analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions
Mon, Jul 20, 9:18 PM
martong closed D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
Mon, Jul 20, 9:18 PM · Restricted Project
martong added a comment to D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.

Ping :)

Mon, Jul 20, 2:37 AM · Restricted Project

Tue, Jul 14

Artem Dergachev <adergachev@apple.com> committed rG59ed9d5d93e5: [analyzer] Skip analysis of inherited ctor as top-level function (authored by martong).
[analyzer] Skip analysis of inherited ctor as top-level function
Tue, Jul 14, 4:40 PM

Jul 10 2020

martong accepted D83555: [analyzer] Fix ctu-on-demand-parsing: REQUIRES: linux -> REQUIRES: system-linux.

Thanks!

Jul 10 2020, 7:36 AM · Restricted Project
martong added inline comments to D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
Jul 10 2020, 7:33 AM · Restricted Project
martong updated the diff for D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
  • Make Signature to be an Optional member of the Summary
Jul 10 2020, 12:08 AM · Restricted Project

Jul 9 2020

martong added inline comments to D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
Jul 9 2020, 9:41 AM · Restricted Project
martong updated the diff for D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
  • Reuse summaries with accept and other functions when sockaddr is a union of pointers
Jul 9 2020, 9:38 AM · Restricted Project
martong added inline comments to D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
Jul 9 2020, 7:36 AM · Restricted Project
martong committed rGd12d0b73f1c9: [analyzer] Add CTUImportCppThreshold for C++ files (authored by martong).
[analyzer] Add CTUImportCppThreshold for C++ files
Jul 9 2020, 6:37 AM
martong closed D83475: [analyzer] Add CTUImportCppThreshold for C++ files.
Jul 9 2020, 6:37 AM · Restricted Project
martong updated the diff for D83475: [analyzer] Add CTUImportCppThreshold for C++ files.
  • Remove extra whitespace
Jul 9 2020, 6:02 AM · Restricted Project
martong added a comment to D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.

Are not some functions missing from the list (shutdown, sockatmark, socket) (these come from <sys/socket.h>)? And getnameinfo comes from <netdb.h> with many other functions that are not added.

Jul 9 2020, 5:55 AM · Restricted Project
martong updated the diff for D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
  • Remove redundant line from test code
Jul 9 2020, 5:52 AM · Restricted Project
martong updated the diff for D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
  • Add getRestrictTy
Jul 9 2020, 5:50 AM · Restricted Project
Herald added a project to D83475: [analyzer] Add CTUImportCppThreshold for C++ files: Restricted Project.
Jul 9 2020, 5:30 AM · Restricted Project

Jul 8 2020

martong added inline comments to D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions.
Jul 8 2020, 10:01 AM · Restricted Project
Herald added a project to D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions: Restricted Project.
Jul 8 2020, 9:53 AM · Restricted Project

Jul 2 2020

martong added a comment to D75740: [ASTImporter] Corrected import of repeated friend declarations..

Ping :)

Jul 2 2020, 10:14 AM · Restricted Project
martong accepted D83006: [ASTImporter] Add unittest case for friend decl import.

Thanks, looks good!

Jul 2 2020, 6:25 AM · Restricted Project
martong committed rGdb4d5f7048a2: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions (authored by martong).
[analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions
Jul 2 2020, 5:53 AM
martong closed D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions.
Jul 2 2020, 5:53 AM · Restricted Project
martong abandoned D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases.

Abandoning for a better fix in D83009

Jul 2 2020, 2:38 AM · Restricted Project
martong accepted D83009: [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization.

I cherry picked this change and retried the case that was failing. This change addresses the issue I was seeing. Thanks.

Jul 2 2020, 2:38 AM · Restricted Project

Jul 1 2020

martong added a comment to D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases.

I'm not certain I understand, but I'm removing my LG while we figure it out. Can you expound a bit further?

Take a look at the AST dump of a small modification of the reduced test case which doesn't trigger the assertion:

void should_not_crash_with_switch_in_lambda() {
  switch (1)
  default:;
  auto f = [] {
    switch (1)
    default:;
  };
}

Before the serialization of LambdaExpr for the LambdaExpr *E, we have E->getBody() == E->getCallOperator()->getBody().
After serialization this is not true anymore because we are writing and reading the body directly when visiting the LambdaExpr
(the captures have the same problem).

Jul 1 2020, 10:15 AM · Restricted Project
martong added a comment to D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases.

In case of the reproducer the problem is that we clear the switch ids in GetExternalDeclStmt. But we read the SwitchCase with the same ID twice from the same call of GetExternalDeclStmt (bold-italic in the below call stacks):

  • 1)

clang::ASTReader::RecordSwitchCaseID (this=0x4d5710, SC=0x5022f0, ID=0) at ../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:9070
0x00007ffff0313097 in clang::ASTRecordReader::recordSwitchCaseID (this=0x7fffffff7ca8, SC=0x5022f0, ID=0) at ../../git/llvm-project/clang/include/clang/Serialization/ASTRecordReader.h:331
0x00007ffff0300ebc in clang::ASTStmtReader::VisitSwitchCase (this=0x7fffffff7c98, S=0x5022f0) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:165
0x00007ffff0300f3f in clang::ASTStmtReader::VisitCaseStmt (this=0x7fffffff7c98, S=0x5022f0) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:171
0x00007ffff031e768 in clang::StmtVisitorBase<std::add_pointer, clang::ASTStmtReader, void>::Visit (this=0x7fffffff7c98, S=0x5022f0) at tools/clang/include/clang/AST/StmtNodes.inc:561
0x00007ffff0312b7c in clang::ASTReader::ReadStmtFromStream (this=0x4d5710, F=...) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:3896
0x00007ffff030eb2e in clang::ASTReader::ReadStmt (this=0x4d5710, F=...) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:2696
0x00007ffff0312e2d in clang::ASTReader::ReadExpr (this=0x4d5710, F=...) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:2705
0x00007ffff01cbb10 in clang::ASTRecordReader::readExpr (this=0x7fffffff8450) at ../../git/llvm-project/clang/include/clang/Serialization/ASTRecordReader.h:131
0x00007ffff0293b23 in clang::ASTDeclReader::VisitVarDeclImpl (this=0x7fffffff8408, VD=0x5010d0) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:1419
0x00007ffff02c40ad in clang::ASTDeclReader::VisitVarDecl (this=0x7fffffff8408, VD=0x5010d0) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:371
0x00007ffff02be24c in clang::declvisitor::Base<std::add_pointer, clang::ASTDeclReader, void>::Visit (this=0x7fffffff8408, D=0x5010d0) at tools/clang/include/clang/AST/DeclNodes.inc:453
0x00007ffff028e5ca in clang::ASTDeclReader::Visit (this=0x7fffffff8408, D=0x5010d0) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:519
0x00007ffff02b938f in clang::ASTReader::ReadDeclRecord (this=0x4d5710, ID=29) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:4044
0x00007ffff018c714 in clang::ASTReader::GetDecl (this=0x4d5710, ID=29) at ../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:7422
0x00007ffff021f9fa in clang::ASTReader::ReadDecl (this=0x4d5710, F=..., R=llvm::SmallVector of Size 3, Capacity 64 = {...}, I=@0x7fffffff9830: 3) at ../../git/llvm-project/clang/include/clang/Serialization/ASTReader.h:1863
0x00007ffff021f9a6 in clang::ASTRecordReader::readDecl (this=0x7fffffff9818) at ../../git/llvm-project/clang/include/clang/Serialization/ASTRecordReader.h:192
0x00007ffff0313d88 in clang::ASTStmtReader::readDecl (this=0x7fffffff9808) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:92
0x00007ffff0301d6a in clang::ASTStmtReader::VisitDeclStmt (this=0x7fffffff9808, S=0x5010b0) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:340
0x00007ffff031e078 in clang::StmtVisitorBase<std::add_pointer, clang::ASTStmtReader, void>::Visit (this=0x7fffffff9808, S=0x5010b0) at tools/clang/include/clang/AST/StmtNodes.inc:97
0x00007ffff0312b7c in clang::ASTReader::ReadStmtFromStream (this=0x4d5710, F=...) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:3896
0x00007ffff019a07c in clang::ASTReader::GetExternalDeclStmt (this=0x4d5710, Offset=107033) at ../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:7476

Jul 1 2020, 7:00 AM · Restricted Project
martong added a comment to D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions.

Ping

Jul 1 2020, 3:11 AM · Restricted Project
martong added a comment to D81787: [clang] Fix the serialization of LambdaExpr and the bogus mutation in LambdaExpr::getBody.

This patch causes a regression (crash) during the deserialization of certain functions. Please see the details and review the supposed fix here: https://reviews.llvm.org/D82940

Jul 1 2020, 3:11 AM · Restricted Project
martong created D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases.
Jul 1 2020, 2:39 AM · Restricted Project

Jun 30 2020

martong accepted D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition..

Ok, I checked and it seems the parent map context is used only by the AST matchers, and this is okay because it looks like there is not any storing or caching of the parent map objects (the type is never copied).

Jun 30 2020, 12:29 PM · Restricted Project
martong accepted D82882: [ASTImporter] Fix AST import crash for a friend decl.

Thanks Vince, looks good to me!

Jun 30 2020, 11:56 AM · Restricted Project
martong added inline comments to D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition..
Jun 30 2020, 4:51 AM · Restricted Project

Jun 26 2020

martong accepted D82561: [analyzer][CrossTU] Lower CTUImportThreshold default value.
Jun 26 2020, 2:08 AM · Restricted Project

Jun 24 2020

martong added a comment to D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions.

We use NoEvalCall in all functions in this patch

I just now realized that it is actually not true, sorry about that. So, seems like fnmatch, strcasecmp, strncasecmp are EvalCallAsPure. I could skip them from this patch, if you guys consider this as dangerous and to bolster my previous comment.

Jun 24 2020, 9:09 AM · Restricted Project
martong added a comment to D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions.

Thanks for the review guys!
I understand there are concerns about evalCall, so I am trying to dissolve those concerns below. :)

Jun 24 2020, 9:09 AM · Restricted Project
martong updated the diff for D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions.
  • Add 'Hide' and 'InAlpha', move Off_tMax inside the brace
Jun 24 2020, 7:31 AM · Restricted Project
martong added inline comments to D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions.
Jun 24 2020, 7:31 AM · Restricted Project

Jun 23 2020

martong added inline comments to D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc.
Jun 23 2020, 7:58 AM · Restricted Project
martong added a comment to D79425: [analyzer] StdLibraryFunctionsChecker: Add overload for adding the same summary for different names.

I'd prefer to have this functionality committed together its first actual use with tests.

Okay, at the current state we add a summary individually for each name, that makes it easier to modify a summary for each function without effecting other functions.

Jun 23 2020, 2:04 AM · Restricted Project
martong abandoned D79425: [analyzer] StdLibraryFunctionsChecker: Add overload for adding the same summary for different names.

At the current state we add a summary individually for each name, that makes it easier to modify a summary for each function without effecting other functions. I'll add this functionality in the future when a real use cases comes up, if that really comes up.

Jun 23 2020, 2:04 AM · Restricted Project
martong added a comment to D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc.

I think testing summaries this way can be really hard to manage in the future.
I see two possible ways forward to make this easier:
a) Make something like https://reviews.llvm.org/D78118 that will also dump the actual summary in a textual form, not only the fact that a summary was loaded and check for that textual form.
b) Make a small helper library for testing summaries. E.g., most of the buffer handling functions have similar summaries, so we could have only a couple of functions for testing buffer related summaries and we could just pass in function pointers (as templates or params) instead of duplicating code. Similarly, testing output ranges could be generalized.

I think b) might be a bit better solution.
Although I do see why some people like to keep tests simple without additional layers of abstractions, so I do not insist :)

Jun 23 2020, 2:04 AM · Restricted Project

Jun 22 2020

martong created D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions.
Jun 22 2020, 3:10 AM · Restricted Project
martong abandoned D79430: [analyzer] StdLibraryFunctionsChecker: Add LazyRanges to support type dependent Max.

Since D80016 we are able to look up types in the TU, so we can get the maximum value from the found type.

Jun 22 2020, 2:38 AM · Restricted Project
martong abandoned D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures.

Since D80016 we are able to look up types in the TU, so this patch is no longer needed. We'd like to be as precise with the function signatures as possible.

Jun 22 2020, 2:38 AM · Restricted Project
martong abandoned D79433: [analyzer] StdLibraryFunctionsChecker: Add summaries for POSIX.

This patch is no longer needed because I am going to upstream the POSIX functions in groups, otherwise the patch would be hideously large.
The groups are: file handling, networking, pthread_, mq_, dbm_, regex related, sched_, time, signal, terminal related, ...

Jun 22 2020, 2:38 AM · Restricted Project

Jun 18 2020

martong added a comment to D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints.

arc adds many unneeded tags from Phabricator. You can drop Reviewers: Subscribers: Tags: and the text Summary: with the following script:

arcfilter () {
  arc amend
  git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people (https://lists.llvm.org/pipermail/llvm-dev/2020-January/137889.html). You should keep the tag. (I started to use --date=now because some people find author date != committer date annoying. The committer date is usually what people care.))

Jun 18 2020, 7:34 AM · Restricted Project

Jun 16 2020

martong added inline comments to D81745: [analyzer][MallocChecker] PR46253: Correctly recognize standard realloc.
Jun 16 2020, 8:16 AM · Restricted Project
martong accepted D81916: [analyzer] Fix StdLibraryFunctionsChecker crash on macOS.
Jun 16 2020, 6:37 AM · Restricted Project
martong added a comment to D81916: [analyzer] Fix StdLibraryFunctionsChecker crash on macOS.
In D81916#2095106, @NoQ wrote:

Thanks!

@martong, thoughts on this? :)

Also i guess in this test we're unable to obtain the value for EOF, what would be a good fixme-test to demonstrate that?

Jun 16 2020, 6:04 AM · Restricted Project

Jun 9 2020

martong added a comment to D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'..

I can make comparisons if more functions are added to the checker, fputs only is not sufficient.

Jun 9 2020, 1:36 AM · Restricted Project

Jun 8 2020

martong accepted D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order.

Thank you Kristof for working on this. Looks good to me as it is!

Jun 8 2020, 6:30 AM · Restricted Project

May 29 2020

martong added a comment to D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types.

A high level comment.

Trying to match function signatures is not a unique problem! In fact, almost all of the checks the analyzer have is trying to solve the very some problem.
One of the methods we have at this point is called CallDescription, see here: https://github.com/llvm/llvm-project/blob/master/clang/lib/StaticAnalyzer/Core/CallEvent.cpp#L358

Moreover, I would assume something similar needs to be done for APINotes.

Do you think it would be possible to extend the CallDescription interface to match your needs? In that case all of the checks could profit from this work.
What do you think?

May 29 2020, 1:06 PM · Restricted Project
martong committed rG634258b80606: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types (authored by martong).
[analyzer] StdLibraryFunctionsChecker: Add support to lookup types
May 29 2020, 9:14 AM
martong closed D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types.
May 29 2020, 9:13 AM · Restricted Project
martong added inline comments to D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types.
May 29 2020, 8:41 AM · Restricted Project
martong committed rG16506d789084: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints (authored by martong).
[analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints
May 29 2020, 8:08 AM
martong closed D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints.
May 29 2020, 8:07 AM · Restricted Project
martong committed rG41928c97b6a1: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved (authored by martong).
[analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved
May 29 2020, 7:35 AM
martong committed rGbd03ef19beb8: [analyzer] ApiModeling: Add buffer size arg constraint (authored by martong).
[analyzer] ApiModeling: Add buffer size arg constraint
May 29 2020, 7:35 AM
martong closed D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved.
May 29 2020, 7:35 AM · Restricted Project
martong closed D77066: [analyzer] ApiModeling: Add buffer size arg constraint.
May 29 2020, 7:34 AM · Restricted Project
martong updated the diff for D77066: [analyzer] ApiModeling: Add buffer size arg constraint.
  • Remove SValBuilder param
May 29 2020, 6:29 AM · Restricted Project
martong added inline comments to D77066: [analyzer] ApiModeling: Add buffer size arg constraint.
May 29 2020, 6:29 AM · Restricted Project
martong added a comment to D80786: Rename APIs in unittests/AST/Language.h in preparation to share them.

Thanks! LGTM!

May 29 2020, 6:29 AM · Restricted Project
martong added a comment to D80786: Rename APIs in unittests/AST/Language.h in preparation to share them.

Just out of curiosity. In what way do you prepare to share these test? For which component are you planning to reuse this test infrastructure?

May 29 2020, 4:50 AM · Restricted Project
martong added a comment to D80786: Rename APIs in unittests/AST/Language.h in preparation to share them.

Thank you for working on this.

May 29 2020, 4:50 AM · Restricted Project
martong updated the diff for D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints.
  • Add comments, remove auto from 'for' loops, matches -> matchesAndSet
May 29 2020, 4:50 AM · Restricted Project
martong accepted D75665: [analyzer] On-demand parsing capability for CTU.

LGTM!

May 29 2020, 3:13 AM · Restricted Project
martong updated the diff for D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved.
May 29 2020, 1:35 AM · Restricted Project

May 28 2020

martong updated the diff for D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints.
  • Rename: validate -> checkValidity, sanityCheck -> checkSpecificValidity
  • Rename: Summary::checkValidity -> validateByConstraints
  • Add more docs to the Summary class
  • Rename matchesSignature -> matches
May 28 2020, 9:50 AM · Restricted Project
martong added inline comments to D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints.
May 28 2020, 9:50 AM · Restricted Project
martong added inline comments to D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`.
May 28 2020, 9:15 AM · Restricted Project
martong updated the diff for D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints.
  • Rebase to master
May 28 2020, 8:06 AM · Restricted Project
martong updated the diff for D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types.
  • Add if (FileTy)
May 28 2020, 7:02 AM · Restricted Project
martong added inline comments to D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types.
May 28 2020, 7:02 AM · Restricted Project
martong updated the diff for D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types.
  • Rebase to master
May 28 2020, 6:30 AM · Restricted Project
martong added a comment to D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`.

In this version of the patch you are using handleConstructionContext to "retrieve" or read an SVal for the construction. However, it seems like that function was designed to store values in the GDM for individual construction context items. To mix a read-only functionality into a setter seems to be error-prone to me.

Please instead re-use the code that computes the object under construction. That'll save you ~50 lines of code and will be more future-proof

I am not sure that this is the best option here. In my viewpoint, ConstructionContext and ConstructionContextItem are both "variants" (i.e. tagged unions) and we perform operations on these. Each operation is actually a visitation on the different kind of values (i.e. a switch). One operation is transforming a ConstructionContext to a ConstructionContextItem. This is what is needed to be able to call the getObjectUnderConstruction function. And this operation - the reading - is way different than the other - the store -, see Adam's concerns. This yields to different functions for each op. But this is perfectly natural once we work with "variants". I mean we can easily extend the operations on a variant with a new operation as a form of a visitation of the kinds (contrary to traditional class hierarchies where adding a new op is hard, but adding a new type is easy).

May 28 2020, 5:57 AM · Restricted Project
martong added a comment to D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`.

In this version of the patch you are using handleConstructionContext to "retrieve" or read an SVal for the construction. However, it seems like that function was designed to store values in the GDM for individual construction context items. To mix a read-only functionality into a setter seems to be error-prone to me.

May 28 2020, 5:57 AM · Restricted Project