[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D76790#1957061 , @Szelethus wrote:

> Yup, looks great


Can confirm.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1525232e2761: [analyzer] StdLibraryFunctionsChecker: fix bug 
with arg constraints (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D76790?vs=253595=254530#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -3,6 +3,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -verify=report
@@ -12,6 +13,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-output=text \
@@ -85,3 +87,30 @@
 // bugpath-warning{{Function argument constraint is not satisfied}} \
 // bugpath-note{{Function argument constraint is not satisfied}}
 }
+
+int __two_constrained_args(int, int);
+void test_constraints_on_multiple_args(int x, int y) {
+  // State split should not happen here. I.e. x == 1 should not be evaluated
+  // FALSE.
+  __two_constrained_args(x, y);
+  clang_analyzer_eval(x == 1); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}}
+  clang_analyzer_eval(y == 1); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}}
+}
+
+int __arg_constrained_twice(int);
+void test_multiple_constraints_on_same_arg(int x) {
+  __arg_constrained_twice(x);
+  // Check that both constraints are applied and only one branch is there.
+  clang_analyzer_eval(x < 1 || x > 2); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{Assuming 'x' is < 1}} \
+  // bugpath-note{{Left side of '||' is true}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -306,7 +306,11 @@
   void checkPostCall(const CallEvent , CheckerContext ) const;
   bool evalCall(const CallEvent , CheckerContext ) const;
 
-  enum CheckKind { CK_StdCLibraryFunctionArgsChecker, CK_NumCheckKinds };
+  enum CheckKind {
+CK_StdCLibraryFunctionArgsChecker,
+CK_StdCLibraryFunctionsTesterChecker,
+CK_NumCheckKinds
+  };
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
@@ -455,23 +459,26 @@
   const Summary  = *FoundSummary;
   ProgramStateRef State = C.getState();
 
+  ProgramStateRef NewState = State;
   for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
-ProgramStateRef SuccessSt = VC->apply(State, Call, Summary);
-ProgramStateRef FailureSt = VC->negate()->apply(State, Call, Summary);
+ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary);
+ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary);
 // The argument constraint is not satisfied.
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(State))
+  if (ExplodedNode *N = C.generateErrorNode(NewState))
 reportBug(Call, N, C);
   break;
 } else {
-  // Apply the constraint even if we cannot reason about the argument. This
-  // means both SuccessSt and FailureSt can be true. If we weren't applying
-  // the constraint that would mean that symbolic execution continues on a
-  // code whose behaviour is undefined.
+  // We will apply the constraint even if we cannot reason about the
+  // argument. This means both SuccessSt and FailureSt can be true. If we
+  // weren't applying the constraint that would mean that symbolic
+  // execution continues on a code whose behaviour is undefined.
   assert(SuccessSt);
-  C.addTransition(SuccessSt);
+  NewState = SuccessSt;
 }
   }
+  if (NewState && NewState != State)
+C.addTransition(NewState);
 }
 
 void 

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

Yup, looks great, sorry for the slack!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 253595.
martong added a comment.

- Test multiple constraints on the same arg


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -3,6 +3,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -verify=report
@@ -12,6 +13,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-output=text \
@@ -85,3 +87,30 @@
 // bugpath-warning{{Function argument constraint is not satisfied}} \
 // bugpath-note{{Function argument constraint is not satisfied}}
 }
+
+int __two_constrained_args(int, int);
+void test_constraints_on_multiple_args(int x, int y) {
+  // State split should not happen here. I.e. x == 1 should not be evaluated
+  // FALSE.
+  __two_constrained_args(x, y);
+  clang_analyzer_eval(x == 1); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}}
+  clang_analyzer_eval(y == 1); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}}
+}
+
+int __arg_constrained_twice(int);
+void test_multiple_constraints_on_same_arg(int x) {
+  __arg_constrained_twice(x);
+  // Check that both constraints are applied and only one branch is there.
+  clang_analyzer_eval(x < 1 || x > 2); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{Assuming 'x' is < 1}} \
+  // bugpath-note{{Left side of '||' is true}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -303,7 +303,11 @@
   void checkPostCall(const CallEvent , CheckerContext ) const;
   bool evalCall(const CallEvent , CheckerContext ) const;
 
-  enum CheckKind { CK_StdCLibraryFunctionArgsChecker, CK_NumCheckKinds };
+  enum CheckKind {
+CK_StdCLibraryFunctionArgsChecker,
+CK_StdCLibraryFunctionsTesterChecker,
+CK_NumCheckKinds
+  };
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
@@ -452,23 +456,26 @@
   const Summary  = *FoundSummary;
   ProgramStateRef State = C.getState();
 
+  ProgramStateRef NewState = State;
   for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
-ProgramStateRef SuccessSt = VC->apply(State, Call, Summary);
-ProgramStateRef FailureSt = VC->negate()->apply(State, Call, Summary);
+ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary);
+ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary);
 // The argument constraint is not satisfied.
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(State))
+  if (ExplodedNode *N = C.generateErrorNode(NewState))
 reportBug(Call, N, C);
   break;
 } else {
-  // Apply the constraint even if we cannot reason about the argument. This
-  // means both SuccessSt and FailureSt can be true. If we weren't applying
-  // the constraint that would mean that symbolic execution continues on a
-  // code whose behaviour is undefined.
+  // We will apply the constraint even if we cannot reason about the
+  // argument. This means both SuccessSt and FailureSt can be true. If we
+  // weren't applying the constraint that would mean that symbolic
+  // execution continues on a code whose behaviour is undefined.
   assert(SuccessSt);
-  C.addTransition(SuccessSt);
+  NewState = SuccessSt;
 }
   }
+  if (NewState && NewState != State)
+C.addTransition(NewState);
 }
 
 void StdLibraryFunctionsChecker::checkPostCall(const CallEvent ,
@@ -933,6 +940,32 @@
   {"getdelim", Summaries{Getline(IntTy, IntMax), Getline(LongTy, LongMax),
  

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 253174.
martong added a comment.

- Add tests with a subchecker


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -3,6 +3,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -verify=report
@@ -12,6 +13,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-output=text \
@@ -85,3 +87,18 @@
 // bugpath-warning{{Function argument constraint is not satisfied}} \
 // bugpath-note{{Function argument constraint is not satisfied}}
 }
+
+int __test_fun_with_2_arg_constraints(int, int);
+void test_multiple_constraints(int x, int y) {
+  // State split should not happen here. I.e. x == 1 should not be evaluated
+  // FALSE.
+  __test_fun_with_2_arg_constraints(x, y);
+  clang_analyzer_eval(x == 1); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}}
+  clang_analyzer_eval(y == 1); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -303,7 +303,11 @@
   void checkPostCall(const CallEvent , CheckerContext ) const;
   bool evalCall(const CallEvent , CheckerContext ) const;
 
-  enum CheckKind { CK_StdCLibraryFunctionArgsChecker, CK_NumCheckKinds };
+  enum CheckKind {
+CK_StdCLibraryFunctionArgsChecker,
+CK_StdCLibraryFunctionsTesterChecker,
+CK_NumCheckKinds
+  };
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
@@ -452,23 +456,26 @@
   const Summary  = *FoundSummary;
   ProgramStateRef State = C.getState();
 
+  ProgramStateRef NewState = State;
   for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
-ProgramStateRef SuccessSt = VC->apply(State, Call, Summary);
-ProgramStateRef FailureSt = VC->negate()->apply(State, Call, Summary);
+ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary);
+ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary);
 // The argument constraint is not satisfied.
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(State))
+  if (ExplodedNode *N = C.generateErrorNode(NewState))
 reportBug(Call, N, C);
   break;
 } else {
-  // Apply the constraint even if we cannot reason about the argument. This
-  // means both SuccessSt and FailureSt can be true. If we weren't applying
-  // the constraint that would mean that symbolic execution continues on a
-  // code whose behaviour is undefined.
+  // We will apply the constraint even if we cannot reason about the
+  // argument. This means both SuccessSt and FailureSt can be true. If we
+  // weren't applying the constraint that would mean that symbolic
+  // execution continues on a code whose behaviour is undefined.
   assert(SuccessSt);
-  C.addTransition(SuccessSt);
+  NewState = SuccessSt;
 }
   }
+  if (NewState && NewState != State)
+C.addTransition(NewState);
 }
 
 void StdLibraryFunctionsChecker::checkPostCall(const CallEvent ,
@@ -933,6 +940,26 @@
   {"getdelim", Summaries{Getline(IntTy, IntMax), Getline(LongTy, LongMax),
  Getline(LongLongTy, LongLongMax)}},
   };
+
+  // Functions for testing.
+  if (ChecksEnabled[CK_StdCLibraryFunctionsTesterChecker]) {
+llvm::StringMap TestFunctionSummaryMap = {
+{"__test_fun_with_2_arg_constraints",
+ Summaries{
+ Summary(ArgTypes{IntTy, IntTy}, RetType{IntTy}, EvalCallAsPure)
+ .ArgConstraint(
+ ArgumentCondition(0U, WithinRange, SingleValue(1)))
+  

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

The fix looks great!

In D76790#1941857 , @martong wrote:

> I was thinking about the below test, but then I reverted back because I don't 
> want to add "fake" summaries for testing purposes. Perhaps adding a new 
> checker option could enable these "fake" summaries, @Szelethus what's your 
> opinion?


The fake summary sounds great. You could hide it behind a debug checker, if we 
are to be that cautious.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: NoQ, Szelethus, baloghadamsoftware.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
xazax.hun, whisperity.
Herald added a project: clang.
martong added a comment.

I was thinking about the below test, but then I reverted back because I don't 
want to add "fake" summaries for testing purposes. Perhaps adding a new checker 
option could enable these "fake" summaries, @Szelethus what's your opinion?

  diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  index 64a412bb4db..c1022492429 100644
  --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  @@ -736,6 +736,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 };
  
 FunctionSummaryMap = {
  +  {
  +"__test",
  +Summaries{
  +  Summary(ArgTypes{IntTy, IntTy}, RetType{IntTy}, EvalCallAsPure)
  +.ArgConstraint(ArgumentCondition(0U, WithinRange, 
SingleValue(1)))
  +.ArgConstraint(ArgumentCondition(1U, WithinRange, 
SingleValue(1)))
  +}
  +  },
 // The isascii() family of functions.
 // The behavior is undefined if the value of the argument is not
 // representable as unsigned char or is not equal to EOF. See e.g. C99
  diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c 
b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
  index a20b90ad1cc..01109e28b99 100644
  --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
  +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
  @@ -85,3 +85,18 @@ void test_notnull_symbolic2(FILE *fp, int *buf) {
   // bugpath-warning{{Function argument constraint is not satisfied}} \
   // bugpath-note{{Function argument constraint is not satisfied}}
   }
  +
  +int __test(int, int);
  +
  +void test_multiple_constraints(int x, int y) {
  +  __test(x, y);
  +  clang_analyzer_eval(x == 1); // \
  +  // report-warning{{TRUE}} \
  +  // bugpath-warning{{TRUE}} \
  +  // bugpath-note{{TRUE}}
  +  clang_analyzer_eval(y == 1); // \
  +  // report-warning{{TRUE}} \
  +  // bugpath-warning{{TRUE}} \
  +  // bugpath-note{{TRUE}}
  +}
  +


Previously we induced a state split if there were multiple argument
constraints given for a function. This was because we called
`addTransition` inside the for loop.
The fix is to is to store the state and apply the next argument
constraint on that. And once the loop is finished we call `addTransition`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76790

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -452,23 +452,26 @@
   const Summary  = *FoundSummary;
   ProgramStateRef State = C.getState();
 
+  ProgramStateRef NewState = State;
   for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
-ProgramStateRef SuccessSt = VC->apply(State, Call, Summary);
-ProgramStateRef FailureSt = VC->negate()->apply(State, Call, Summary);
+ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary);
+ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary);
 // The argument constraint is not satisfied.
 if (FailureSt && !SuccessSt) {
-  if (ExplodedNode *N = C.generateErrorNode(State))
+  if (ExplodedNode *N = C.generateErrorNode(NewState))
 reportBug(Call, N, C);
   break;
 } else {
-  // Apply the constraint even if we cannot reason about the argument. This
-  // means both SuccessSt and FailureSt can be true. If we weren't applying
-  // the constraint that would mean that symbolic execution continues on a
-  // code whose behaviour is undefined.
+  // We will apply the constraint even if we cannot reason about the
+  // argument. This means both SuccessSt and FailureSt can be true. If we
+  // weren't applying the constraint that would mean that symbolic
+  // execution continues on a code whose behaviour is undefined.
   assert(SuccessSt);
-  C.addTransition(SuccessSt);
+  NewState = SuccessSt;
 }
   }
+  if (NewState && NewState != State)
+C.addTransition(NewState);
 }
 
 void StdLibraryFunctionsChecker::checkPostCall(const CallEvent ,


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ 

[PATCH] D76790: [analyzer] StdLibraryFunctionsChecker: fix bug with arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

I was thinking about the below test, but then I reverted back because I don't 
want to add "fake" summaries for testing purposes. Perhaps adding a new checker 
option could enable these "fake" summaries, @Szelethus what's your opinion?

  diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  index 64a412bb4db..c1022492429 100644
  --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  @@ -736,6 +736,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 };
  
 FunctionSummaryMap = {
  +  {
  +"__test",
  +Summaries{
  +  Summary(ArgTypes{IntTy, IntTy}, RetType{IntTy}, EvalCallAsPure)
  +.ArgConstraint(ArgumentCondition(0U, WithinRange, 
SingleValue(1)))
  +.ArgConstraint(ArgumentCondition(1U, WithinRange, 
SingleValue(1)))
  +}
  +  },
 // The isascii() family of functions.
 // The behavior is undefined if the value of the argument is not
 // representable as unsigned char or is not equal to EOF. See e.g. C99
  diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c 
b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
  index a20b90ad1cc..01109e28b99 100644
  --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
  +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
  @@ -85,3 +85,18 @@ void test_notnull_symbolic2(FILE *fp, int *buf) {
   // bugpath-warning{{Function argument constraint is not satisfied}} \
   // bugpath-note{{Function argument constraint is not satisfied}}
   }
  +
  +int __test(int, int);
  +
  +void test_multiple_constraints(int x, int y) {
  +  __test(x, y);
  +  clang_analyzer_eval(x == 1); // \
  +  // report-warning{{TRUE}} \
  +  // bugpath-warning{{TRUE}} \
  +  // bugpath-note{{TRUE}}
  +  clang_analyzer_eval(y == 1); // \
  +  // report-warning{{TRUE}} \
  +  // bugpath-warning{{TRUE}} \
  +  // bugpath-note{{TRUE}}
  +}
  +


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76790/new/

https://reviews.llvm.org/D76790



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits