[clang] [clang][analyzer] Support `fputs` in the StreamChecker (PR #73335)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/73335
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputs` in the StreamChecker (PR #73335)

[Balázs Kéri via cfe-commits]

@@ -774,26 +780,45 @@ void StreamChecker::evalFgetcFputc(const FnDescription 
*Desc,
 
   // `fgetc` returns the read character on success, otherwise returns EOF.
   // `fputc` returns the written character on success, otherwise returns EOF.
+  // `fputs` returns a non negative value on sucecess, otherwise returns EOF.
 
-  // Generate a transition for the success state of fputc.
+  SValBuilder &SVB = C.getSValBuilder();
+  auto &ASTC = C.getASTContext();
   if (!IsRead) {
-std::optional PutVal = Call.getArgSVal(0).getAs();
-if (!PutVal)
-  return;
-ProgramStateRef StateNotFailed =
-State->BindExpr(CE, C.getLocationContext(), *PutVal);
-StateNotFailed =
-StateNotFailed->set(StreamSym, 
StreamState::getOpened(Desc));
-C.addTransition(StateNotFailed);
+// Generddate a transition for the success state of `fputc`.
+if (SingleChar) {
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed = StateNotFailed->set(
+  StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateNotFailed);
+}
+// Generddate a transition for the success state of `fputs`.
+else {
+  NonLoc RetVal = makeRetVal(C, CE).castAs();
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), RetVal);
+  auto Cond =
+  SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ASTC.IntTy),
+SVB.getConditionType())
+  .getAs();
+  if (!Cond)
+return;
+  StateNotFailed = StateNotFailed->assume(*Cond, true);
+  if (!StateNotFailed)
+return;
+  C.addTransition(StateNotFailed);
+}

balazske wrote:

This function is becoming too large. Here almost all code for `fputs` is in a 
new branch and really there are 4 condition branches for all 4 functions (if 
`fgets` is added) with not much common code. It looks better to make an 
`evalFputX` and `evalFgetX` function (but `fgets` can be too much different 
than the other). Functions for the common code at begin and end of these `eval` 
functions (that can be reused by the others too) could be good too (and 
separate eval functions for all get and put cases), but this belongs into a 
separate PR. Probably I can improve the code, my next plans contain anyway to 
work with `StreamChecker`.

https://github.com/llvm/llvm-project/pull/73335
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputs` in the StreamChecker (PR #73335)

[Balázs Kéri via cfe-commits]

@@ -50,6 +50,7 @@ size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
 int fgetc(FILE *stream);
 int fputc(int ch, FILE *stream);
+int fputs(const char *str, FILE *stream);

balazske wrote:

The exact POSIX declaration is `int fputs(const char *restrict s, FILE 
*restrict stream)`.

https://github.com/llvm/llvm-project/pull/73335
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputs` in the StreamChecker (PR #73335)

[Balázs Kéri via cfe-commits]

https://github.com/balazske requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/73335
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Improve structural equivalence of overloadable operators. (PR #72242)

[Balázs Kéri via cfe-commits]

@@ -2252,6 +2252,176 @@ TEST_F(StructuralEquivalenceStmtTest, 
UnaryOperatorDifferentOps) {
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceStmtTest,
+   CXXOperatorCallExprVsUnaryBinaryOperator) {
+  auto t = makeNamedDecls(
+  R"(
+  template 
+  class A;
+  template 
+  void foo(
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A y)>,
+A,
+A> y)>,
+A,
+A,
+A,
+A= y>,
+A y>,
+A,
+A,
+A,
+A,
+A,
+A* y>,
+A y>
+  );
+  )",
+  R"(
+  struct Bar {
+Bar& operator=(Bar&);
+Bar& operator->();
+  };
+
+  Bar& operator+(Bar&, Bar&);
+  Bar& operator+(Bar&);
+  Bar& operator-(Bar&, Bar&);
+  Bar& operator-(Bar&);
+  Bar& operator*(Bar&, Bar&);
+  Bar& operator*(Bar&);
+  Bar& operator/(Bar&, Bar&);
+  Bar& operator%(Bar&, Bar&);
+  Bar& operator^(Bar&, Bar&);
+  Bar& operator&(Bar&, Bar&);
+  Bar& operator&(Bar&);
+  Bar& operator|(Bar&, Bar&);
+  Bar& operator~(Bar&);
+  Bar& operator!(Bar&);
+  Bar& operator<(Bar&, Bar&);
+  Bar& operator>(Bar&, Bar&);
+  Bar& operator+=(Bar&, Bar&);
+  Bar& operator-=(Bar&, Bar&);
+  Bar& operator*=(Bar&, Bar&);
+  Bar& operator/=(Bar&, Bar&);
+  Bar& operator%=(Bar&, Bar&);
+  Bar& operator^=(Bar&, Bar&);
+  Bar& operator&=(Bar&, Bar&);
+  Bar& operator|=(Bar&, Bar&);
+  Bar& operator<<(Bar&, Bar&);
+  Bar& operator>>(Bar&, Bar&);
+  Bar& operator<<=(Bar&, Bar&);
+  Bar& operator>>=(Bar&, Bar&);
+  Bar& operator==(Bar&, Bar&);
+  Bar& operator!=(Bar&, Bar&);
+  Bar& operator<=(Bar&, Bar&);
+  Bar& operator>=(Bar&, Bar&);
+  Bar& operator<=>(Bar&, Bar&);
+  Bar& operator&&(Bar&, Bar&);
+  Bar& operator||(Bar&, Bar&);
+  Bar& operator++(Bar&);
+  Bar& operator--(Bar&);
+  Bar& operator,(Bar&, Bar&);
+  Bar& operator->*(Bar&, Bar&);
+
+  template 
+  class A;
+  template 
+  void foo(
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A,
+A y)>,
+A,
+A> y)>,
+A,
+A,
+A,
+A= y>,
+A y>,
+A,
+A,
+A,
+A,
+A,
+A* y>,
+A y>
+  );
+  )",
+  Lang_CXX20);
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceStmtTest,
+   CXXOperatorCallExprVsUnaryBinaryOperatorNe) {
+  auto t = makeNamedDecls(
+  R"(
+  template 
+  class A;
+  template 
+  void foo(
+A
+  );
+  )",
+  R"(
+  struct Bar;
+
+  Bar& operator+(Bar&, Bar&);

balazske wrote:

I do not know exactly what was the purpose, but likely it should be `operator 
-` instead.

https://github.com/llvm/llvm-project/pull/72242
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Improve structural equivalence of overloadable operators. (PR #72242)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/72242
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Improve structural equivalence of overloadable operators. (PR #72242)

[Balázs Kéri via cfe-commits]

@@ -437,12 +439,67 @@ class StmtComparer {
 };
 } // namespace
 
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const UnaryOperator *E1,
+ const CXXOperatorCallExpr *E2) {
+  return UnaryOperator::getOverloadedOperator(E1->getOpcode()) ==
+ E2->getOperator() &&
+ IsStructurallyEquivalent(Context, E1->getSubExpr(), E2->getArg(0));
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const CXXOperatorCallExpr *E1,
+ const UnaryOperator *E2) {
+  return E1->getOperator() ==
+ UnaryOperator::getOverloadedOperator(E2->getOpcode()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(0), E2->getSubExpr());
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const BinaryOperator *E1,
+ const CXXOperatorCallExpr *E2) {
+  return BinaryOperator::getOverloadedOperator(E1->getOpcode()) ==
+ E2->getOperator() &&
+ IsStructurallyEquivalent(Context, E1->getLHS(), E2->getArg(0)) &&
+ IsStructurallyEquivalent(Context, E1->getRHS(), E2->getArg(1));
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const CXXOperatorCallExpr *E1,
+ const BinaryOperator *E2) {
+  return E1->getOperator() ==
+ BinaryOperator::getOverloadedOperator(E2->getOpcode()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(0), E2->getLHS()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(1), E2->getRHS());
+}
+
 /// Determine structural equivalence of two statements.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
  const Stmt *S1, const Stmt *S2) {
   if (!S1 || !S2)
 return S1 == S2;
 
+  // Check for statements with similar syntax but different AST.
+  // The unary and binary operators (like '+', '&') can be parsed as
+  // CXXOperatorCall too (and UnaryOperator or BinaryOperator).
+  // This depends on arguments with unresolved type and on the name lookup.

balazske wrote:

The idea was coming from this post:
https://discourse.llvm.org/t/different-ast-translations-for-the-same-body-why/22741
The presence of `operator+` declaration is needed to change the parsed AST from 
`BinaryOperator` to `CXXOverloadedOperatorCallExpr`.

https://github.com/llvm/llvm-project/pull/72242
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix import of SubstTemplateTypeParmType in return type of function. (PR #69724)

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/69724
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] IdentifierInfo of Attribute should be set using 'ToASTContext' (PR #73290)

[Balázs Kéri via cfe-commits]

https://github.com/balazske requested changes to this pull request.

I think it is better to add the import of AttrName to the attribute import code 
(function `Import(const Attr *FromAttr)` and what is called from it). Probably 
it works to add it to `AttrImporter::cloneAttr` and do it like `const 
IdentifierInfo *ToAttrName = Importer.Import(FromAttr->getAttrName());`.

https://github.com/llvm/llvm-project/pull/73290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Switch to PostStmt callbacks in ArrayBoundV2 (PR #72107)

[Balázs Kéri via cfe-commits]

@@ -64,6 +100,28 @@ double arrayInStructPtr(struct vec *pv) {
   // expected-note@-2 {{Access of the field 'elems' at index 64, while it 
holds only 64 'double' elements}}
 }
 
+struct item {
+  int a, b;
+} itemArray[20] = {0};
+
+int structOfArrays(void) {

balazske wrote:

This is rather "arrayOfStructs"?

https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.


https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix import of variable template redeclarations. (PR #72841)

[Balázs Kéri via cfe-commits]

balazske wrote:

I plan to fix import of `VarTemplateSpecializationDecl` in a different PR. The 
indicated assertion "Missing call to MapImported?" is likely to related to this 
part.

https://github.com/llvm/llvm-project/pull/72841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST][ASTMerge] prevent AST nodes from being deallocated early (PR #73096)

[Balázs Kéri via cfe-commits]

balazske wrote:

You have found that reason for the crash is that references to `IdentifierInfo` 
are remaining in `OnDiskChainedHashTableGenerator` and previously deallocated 
by `ASTUnit` destruction? In this case why is the `ASTUnit` (or something in 
it, probably `ASTContext`) the owner of the data, and not 
`OnDiskChainedHashTableGenerator`?
By using `ASTImporter` it is possible that it moves data from the "old" AST to 
the "new" without copy (if yes this is a bug) and such a situation can cause 
memory errors.

https://github.com/llvm/llvm-project/pull/73096
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -768,26 +772,65 @@ void StreamChecker::evalFputc(const FnDescription *Desc, 
const CallEvent &Call,
 
   assertStreamStateOpened(OldSS);
 
+  // `fgetc` returns the read character on success, otherwise returns EOF.
   // `fputc` returns the written character on success, otherwise returns EOF.
 
-  // Generate a transition for the success state.
-  std::optional PutVal = Call.getArgSVal(0).getAs();
-  if (!PutVal)
-return;
-  ProgramStateRef StateNotFailed =
-  State->BindExpr(CE, C.getLocationContext(), *PutVal);
-  StateNotFailed =
-  StateNotFailed->set(StreamSym, StreamState::getOpened(Desc));
-  C.addTransition(StateNotFailed);
+  // Generate a transition for the success state of fputc.
+  if (!IsRead) {
+std::optional PutVal = Call.getArgSVal(0).getAs();
+if (!PutVal)
+  return;
+ProgramStateRef StateNotFailed =
+State->BindExpr(CE, C.getLocationContext(), *PutVal);
+StateNotFailed =
+StateNotFailed->set(StreamSym, 
StreamState::getOpened(Desc));
+C.addTransition(StateNotFailed);
+  }
+  // Generate a transition for the success state of fgetc.
+  // If we know the state to be FEOF at fgetc, do not add a success state.
+  else if (OldSS->ErrorState != ErrorFEof) {
+NonLoc RetVal = makeRetVal(C, CE).castAs();
+ProgramStateRef StateNotFailed =
+State->BindExpr(CE, C.getLocationContext(), RetVal);
+SValBuilder &SVB = C.getSValBuilder();
+// The returned 'unsigned char' of `fgetc` is converted to 'int',
+// so we need to check if it is in range [0, 255].
+auto CondLow = SVB.evalBinOp(State, BO_GE, RetVal,
+ SVB.makeZeroVal(C.getASTContext().IntTy),
+ SVB.getConditionType())
+   .getAs();
+auto CondHigh = SVB.evalBinOp(State, BO_LE, RetVal,
+  SVB.makeIntVal(255, C.getASTContext().IntTy),
+  SVB.getConditionType())
+.getAs();

balazske wrote:

Value "255" can be replaced with 
`SVB.getBasicValueFactory().getMaxValue(C.getASTContext().UnsignedCharTy).getLimitedValue()`
 (this is probably not always 255). Probably ASTContext can be saved into a 
variable.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -101,6 +101,30 @@ void error_fwrite(void) {
   Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already 
closed}}
 }
 
+void error_fgetc(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int Ret = fgetc(F);
+  if (0 <= Ret && Ret <= 255) {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+clang_analyzer_eval(Ret == EOF);  // expected-warning {{TRUE}}

balazske wrote:

A line `clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning 
{{FALSE}}` can be added to check that no state is produced when none of the 
state flags are set.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -259,14 +283,33 @@ void error_indeterminate_clearerr(void) {
   fclose(F);
 }
 
+void error_indeterminate_fgetc(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc) {
+if (feof(F)) {
+  clang_analyzer_warnIfReached(); // no warning
+  fgetc(F);   // no warning
+} else if (ferror(F)) {
+  fgetc(F); // expected-warning {{might be 'indeterminate'}}
+} else {
+  fgetc(F); // expected-warning {{might be 'indeterminate'}}
+}
+  }
+  fclose(F);
+}
+
 void error_indeterminate_fputc(void) {
   FILE *F = fopen("file", "r+");
   if (!F)
 return;
   int rc = fseek(F, 0, SEEK_SET);
   if (rc) {
 if (feof(F)) {

balazske wrote:

The branch `feof(F)` is not needed at all here. There is a previous test case 
that tells that `feof(F)` is never true when fseek to a 0 position is called.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -303,3 +346,29 @@ void error_indeterminate_feof2(void) {
   }
   fclose(F);
 }
+
+void error_indeterminate_feof3(void) {

balazske wrote:

This test seems to be not necessary (previous tests cover these conditions), 
otherwise a more meaningful name should be chosen for it.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

https://github.com/balazske requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -303,3 +346,29 @@ void error_indeterminate_feof2(void) {
   }
   fclose(F);
 }
+
+void error_indeterminate_feof3(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+return;
+  if (fgetc(F) == EOF) {
+if (feof(F)) {
+  // error is feof, should be non-indeterminate
+  fputc('A', F); // no warning
+}
+  }
+  fclose(F);
+}
+
+void error_indeterminate_feof4(void) {

balazske wrote:

This test seems to be not necessary (previous tests cover these conditions), 
otherwise a more meaningful name should be chosen for it.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -259,14 +283,33 @@ void error_indeterminate_clearerr(void) {
   fclose(F);
 }
 
+void error_indeterminate_fgetc(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc) {
+if (feof(F)) {

balazske wrote:

First branch of `if` is not needed. But I still think that these tests (with 
`fgetc` and `fputc`) are not testing different conditions than the other test 
(the condition that we have a warning for _might be 'indeterminate'_), 
therefore can be removed.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -101,6 +101,30 @@ void error_fwrite(void) {
   Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already 
closed}}
 }
 
+void error_fgetc(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int Ret = fgetc(F);
+  if (0 <= Ret && Ret <= 255) {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+clang_analyzer_eval(Ret == EOF);  // expected-warning {{TRUE}}
+if (feof(F)) {
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}

balazske wrote:

These `warnIfReached` calls are not necessary, because the presence of the next 
warning tells anyway that the code is reachable.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fgetc` in StreamChecker (PR #72627)

[Balázs Kéri via cfe-commits]

@@ -768,26 +772,56 @@ void StreamChecker::evalFputc(const FnDescription *Desc, 
const CallEvent &Call,
 
   assertStreamStateOpened(OldSS);
 
+  // `fgetc` returns the read character on success, otherwise returns EOF.
   // `fputc` returns the written character on success, otherwise returns EOF.
 
-  // Generate a transition for the success state.
-  std::optional PutVal = Call.getArgSVal(0).getAs();
-  if (!PutVal)
-return;
-  ProgramStateRef StateNotFailed =
-  State->BindExpr(CE, C.getLocationContext(), *PutVal);
-  StateNotFailed =
-  StateNotFailed->set(StreamSym, StreamState::getOpened(Desc));
-  C.addTransition(StateNotFailed);
+  // Generate a transition for the success state of fputc.
+  if (!IsRead) {
+std::optional PutVal = Call.getArgSVal(0).getAs();
+if (!PutVal)
+  return;
+ProgramStateRef StateNotFailed =
+State->BindExpr(CE, C.getLocationContext(), *PutVal);
+StateNotFailed =
+StateNotFailed->set(StreamSym, 
StreamState::getOpened(Desc));
+C.addTransition(StateNotFailed);
+  }
+  // Generate a transition for the success state of fgetc.
+  // If we know the state to be FEOF at fgetc, do not add a success state.
+  else if (OldSS->ErrorState != ErrorFEof) {
+NonLoc RetVal = makeRetVal(C, CE).castAs();
+ProgramStateRef StateNotFailed =
+State->BindExpr(CE, C.getLocationContext(), RetVal);
+SValBuilder &SVB = C.getSValBuilder();
+auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
+  SVB.makeZeroVal(C.getASTContext().IntTy),
+  SVB.getConditionType())
+.getAs();
+if (!Cond)
+  return;

balazske wrote:

The function returns an `unsigned char` value converted to `int`, another 
condition should be added for the upper limit.

https://github.com/llvm/llvm-project/pull/72627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix import of variable template redeclarations. (PR #72841)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/72841

From 99d6169f62862b7b1147da7fd26a85df20a0aba5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Mon, 20 Nov 2023 10:14:52 +0100
Subject: [PATCH 1/2] [clang][ASTImporter] Fix import of variable template
 redeclarations.

In some cases variable templates (specially if static member of record)
were not correctly imported and an assertion "Missing call to MapImported?"
could happen.
---
 clang/lib/AST/ASTImporter.cpp   |  27 +++---
 clang/unittests/AST/ASTImporterTest.cpp | 105 
 2 files changed, 122 insertions(+), 10 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c4e931e220f69b5..7a5e3d665328532 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6245,17 +6245,21 @@ ExpectedDecl 
ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
   D->getTemplatedDecl()))
 continue;
   if (IsStructuralMatch(D, FoundTemplate)) {
-// The Decl in the "From" context has a definition, but in the
-// "To" context we already have a definition.
+// FIXME Check for ODR error if the two definitions have
+// different initializers?
 VarTemplateDecl *FoundDef = getTemplateDefinition(FoundTemplate);
-if (D->isThisDeclarationADefinition() && FoundDef)
-  // FIXME Check for ODR error if the two definitions have
-  // different initializers?
-  return Importer.MapImported(D, FoundDef);
-if (FoundTemplate->getDeclContext()->isRecord() &&
-D->getDeclContext()->isRecord())
-  return Importer.MapImported(D, FoundTemplate);
-
+if (D->getDeclContext()->isRecord()) {
+  assert(FoundTemplate->getDeclContext()->isRecord() &&
+ "Member variable template imported as non-member, "
+ "inconsistent imported AST?");
+  if (FoundDef)
+return Importer.MapImported(D, FoundDef);
+  if (!D->isThisDeclarationADefinition())
+return Importer.MapImported(D, FoundTemplate);
+} else {
+  if (FoundDef && D->isThisDeclarationADefinition())
+return Importer.MapImported(D, FoundDef);
+}
 FoundByLookup = FoundTemplate;
 break;
   }
@@ -6374,7 +6378,10 @@ ExpectedDecl 
ASTNodeImporter::VisitVarTemplateSpecializationDecl(
 // variable.
 return Importer.MapImported(D, FoundDef);
   }
+  // FIXME HandleNameConflict
+  return make_error(ASTImportError::NameConflict);
 }
+return Importer.MapImported(D, D2);
   } else {
 TemplateArgumentListInfo ToTAInfo;
 if (const ASTTemplateArgumentListInfo *Args = D->getTemplateArgsInfo()) {
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 5f4d8d040772cb1..d439a14b7b9985f 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -5050,6 +5050,111 @@ TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
   EXPECT_EQ(ToTUX, ToX);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateDeclConflict) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  constexpr int X = 1;
+  )",
+  Lang_CXX14);
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  constexpr int X = 2;
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromX = FirstDeclMatcher().match(
+  FromTU, varTemplateDecl(hasName("X")));
+  auto *ToX = Import(FromX, Lang_CXX11);
+  // FIXME: This import should fail.
+  EXPECT_TRUE(ToX);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateStaticDefinition) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  struct A {
+template 
+static int X;
+  };
+  )",
+  Lang_CXX14);
+  auto *ToX = FirstDeclMatcher().match(
+  ToTU, varTemplateDecl(hasName("X")));
+  ASSERT_FALSE(ToX->isThisDeclarationADefinition());
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct A {
+template 
+static int X;
+  };
+  template 
+  int A::X = 2;
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromXDef = LastDeclMatcher().match(
+  FromTU, varTemplateDecl(hasName("X")));
+  ASSERT_TRUE(FromXDef->isThisDeclarationADefinition());
+  auto *ToXDef = Import(FromXDef, Lang_CXX14);
+  EXPECT_TRUE(ToXDef);
+  EXPECT_TRUE(ToXDef->isThisDeclarationADefinition());
+  EXPECT_EQ(ToXDef->getPreviousDecl(), ToX);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateSpecializationDeclValue) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  constexpr int X = U::Value;
+  struct A { static constexpr int Value = 1; };
+  constexpr int Y = X;
+  )",
+  Lang_CXX14);
+
+  auto *ToTUX = FirstDeclMatcher().match(
+  ToTU, varTemplateSpecializationDecl(hasName("X")));
+  D

[clang] [clang-tools-extra] [llvm] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/69469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [clang-tools-extra] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/69469

From 0fdc57b49002afd8aa54043837ee4155688b4c36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Tue, 17 Oct 2023 11:51:27 +0200
Subject: [PATCH 1/2] [clang][Analyzer] Move checker 'alpha.unix.Errno' to
 'unix.Errno'.

---
 clang/docs/ReleaseNotes.rst   |   4 +
 clang/docs/analyzer/checkers.rst  | 140 +-
 .../clang/StaticAnalyzer/Checkers/Checkers.td |  24 +--
 clang/test/Analysis/analyzer-config.c |   2 +-
 .../test/Analysis/analyzer-enabled-checkers.c |   1 +
 clang/test/Analysis/errno-notes.c |   4 +-
 clang/test/Analysis/errno-options.c   |   8 +-
 .../errno-stdlibraryfunctions-notes.c |   2 +-
 .../test/Analysis/errno-stdlibraryfunctions.c |   2 +-
 clang/test/Analysis/errno.c   |  16 +-
 ...c-library-functions-arg-enabled-checkers.c |   1 +
 clang/test/Analysis/stream-errno-note.c   |   8 +-
 clang/test/Analysis/stream-errno.c|   6 +-
 clang/test/Analysis/stream-noopen.c   |   4 +-
 14 files changed, 114 insertions(+), 108 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9782c123f4c9372..8cd53d310c784b7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -662,6 +662,10 @@ Static Analyzer
 - Added a new checker ``core.BitwiseShift`` which reports situations where
   bitwise shift operators produce undefined behavior (because some operand is
   negative or too large).
+
+- Move checker ``alpha.unix.Errno`` out of the ``alpha`` package
+  to ``unix.Errno``.
+
 - Move checker ``alpha.unix.StdCLibraryFunctions`` out of the ``alpha`` package
   to ``unix.StdCLibraryFunctions``.
 
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 597ffcc4a10a25b..9c6c059303834bf 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -934,6 +934,76 @@ Check calls to various UNIX/Posix functions: ``open, 
pthread_once, calloc, mallo
 .. literalinclude:: checkers/unix_api_example.c
 :language: c
 
+.. _unix-Errno:
+
+unix.Errno (C)
+""
+
+Check for improper use of ``errno``.
+This checker implements partially CERT rule
+`ERR30-C. Set errno to zero before calling a library function known to set 
errno,
+and check errno only after the function returns a value indicating failure
+`_.
+The checker can find the first read of ``errno`` after successful standard
+function calls.
+
+The C and POSIX standards often do not define if a standard library function
+may change value of ``errno`` if the call does not fail.
+Therefore, ``errno`` should only be used if it is known from the return value
+of a function that the call has failed.
+There are exceptions to this rule (for example ``strtol``) but the affected
+functions are not yet supported by the checker.
+The return values for the failure cases are documented in the standard Linux 
man
+pages of the functions and in the `POSIX standard 
`_.
+
+.. code-block:: c
+
+ int unsafe_errno_read(int sock, void *data, int data_size) {
+   if (send(sock, data, data_size, 0) != data_size) {
+ // 'send' can be successful even if not all data was sent
+ if (errno == 1) { // An undefined value may be read from 'errno'
+   return 0;
+ }
+   }
+   return 1;
+ }
+
+The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
+warnings from this checker. The supported functions are the same as by
+:ref:`unix-StdCLibraryFunctions`. The ``ModelPOSIX`` option of that
+checker affects the set of checked functions.
+
+**Parameters**
+
+The ``AllowErrnoReadOutsideConditionExpressions`` option allows read of the
+errno value if the value is not used in a condition (in ``if`` statements,
+loops, conditional expressions, ``switch`` statements). For example ``errno``
+can be stored into a variable without getting a warning by the checker.
+
+.. code-block:: c
+
+ int unsafe_errno_read(int sock, void *data, int data_size) {
+   if (send(sock, data, data_size, 0) != data_size) {
+ int err = errno;
+ // warning if 'AllowErrnoReadOutsideConditionExpressions' is false
+ // no warning if 'AllowErrnoReadOutsideConditionExpressions' is true
+   }
+   return 1;
+ }
+
+Default value of this option is ``true``. This allows save of the errno value
+for possible later error handling.
+
+**Limitations**
+
+ - Only the very first usage of ``errno`` is checked after an affected function
+   call. Value of ``errno`` is not followed when it is stored into a variable
+   or returned from a function.
+ - Documentation of function ``lseek`` is not clear about what happens if the
+   function returns different value than the expected file position but not -1.
+   To avoid possible false-posit

[llvm] [clang-tools-extra] [clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/69469

From 0fdc57b49002afd8aa54043837ee4155688b4c36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Tue, 17 Oct 2023 11:51:27 +0200
Subject: [PATCH] [clang][Analyzer] Move checker 'alpha.unix.Errno' to
 'unix.Errno'.

---
 clang/docs/ReleaseNotes.rst   |   4 +
 clang/docs/analyzer/checkers.rst  | 140 +-
 .../clang/StaticAnalyzer/Checkers/Checkers.td |  24 +--
 clang/test/Analysis/analyzer-config.c |   2 +-
 .../test/Analysis/analyzer-enabled-checkers.c |   1 +
 clang/test/Analysis/errno-notes.c |   4 +-
 clang/test/Analysis/errno-options.c   |   8 +-
 .../errno-stdlibraryfunctions-notes.c |   2 +-
 .../test/Analysis/errno-stdlibraryfunctions.c |   2 +-
 clang/test/Analysis/errno.c   |  16 +-
 ...c-library-functions-arg-enabled-checkers.c |   1 +
 clang/test/Analysis/stream-errno-note.c   |   8 +-
 clang/test/Analysis/stream-errno.c|   6 +-
 clang/test/Analysis/stream-noopen.c   |   4 +-
 14 files changed, 114 insertions(+), 108 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9782c123f4c9372..8cd53d310c784b7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -662,6 +662,10 @@ Static Analyzer
 - Added a new checker ``core.BitwiseShift`` which reports situations where
   bitwise shift operators produce undefined behavior (because some operand is
   negative or too large).
+
+- Move checker ``alpha.unix.Errno`` out of the ``alpha`` package
+  to ``unix.Errno``.
+
 - Move checker ``alpha.unix.StdCLibraryFunctions`` out of the ``alpha`` package
   to ``unix.StdCLibraryFunctions``.
 
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 597ffcc4a10a25b..9c6c059303834bf 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -934,6 +934,76 @@ Check calls to various UNIX/Posix functions: ``open, 
pthread_once, calloc, mallo
 .. literalinclude:: checkers/unix_api_example.c
 :language: c
 
+.. _unix-Errno:
+
+unix.Errno (C)
+""
+
+Check for improper use of ``errno``.
+This checker implements partially CERT rule
+`ERR30-C. Set errno to zero before calling a library function known to set 
errno,
+and check errno only after the function returns a value indicating failure
+`_.
+The checker can find the first read of ``errno`` after successful standard
+function calls.
+
+The C and POSIX standards often do not define if a standard library function
+may change value of ``errno`` if the call does not fail.
+Therefore, ``errno`` should only be used if it is known from the return value
+of a function that the call has failed.
+There are exceptions to this rule (for example ``strtol``) but the affected
+functions are not yet supported by the checker.
+The return values for the failure cases are documented in the standard Linux 
man
+pages of the functions and in the `POSIX standard 
`_.
+
+.. code-block:: c
+
+ int unsafe_errno_read(int sock, void *data, int data_size) {
+   if (send(sock, data, data_size, 0) != data_size) {
+ // 'send' can be successful even if not all data was sent
+ if (errno == 1) { // An undefined value may be read from 'errno'
+   return 0;
+ }
+   }
+   return 1;
+ }
+
+The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
+warnings from this checker. The supported functions are the same as by
+:ref:`unix-StdCLibraryFunctions`. The ``ModelPOSIX`` option of that
+checker affects the set of checked functions.
+
+**Parameters**
+
+The ``AllowErrnoReadOutsideConditionExpressions`` option allows read of the
+errno value if the value is not used in a condition (in ``if`` statements,
+loops, conditional expressions, ``switch`` statements). For example ``errno``
+can be stored into a variable without getting a warning by the checker.
+
+.. code-block:: c
+
+ int unsafe_errno_read(int sock, void *data, int data_size) {
+   if (send(sock, data, data_size, 0) != data_size) {
+ int err = errno;
+ // warning if 'AllowErrnoReadOutsideConditionExpressions' is false
+ // no warning if 'AllowErrnoReadOutsideConditionExpressions' is true
+   }
+   return 1;
+ }
+
+Default value of this option is ``true``. This allows save of the errno value
+for possible later error handling.
+
+**Limitations**
+
+ - Only the very first usage of ``errno`` is checked after an affected function
+   call. Value of ``errno`` is not followed when it is stored into a variable
+   or returned from a function.
+ - Documentation of function ``lseek`` is not clear about what happens if the
+   function returns different value than the expected file position but not -1.
+   To avoid possible false-positives

[clang] [clang][ASTImporter] Fix import of variable template redeclarations. (PR #72841)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/72841

In some cases variable templates (specially if static member of record) were 
not correctly imported and an assertion "Missing call to MapImported?" could 
happen.

From 99d6169f62862b7b1147da7fd26a85df20a0aba5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Mon, 20 Nov 2023 10:14:52 +0100
Subject: [PATCH] [clang][ASTImporter] Fix import of variable template
 redeclarations.

In some cases variable templates (specially if static member of record)
were not correctly imported and an assertion "Missing call to MapImported?"
could happen.
---
 clang/lib/AST/ASTImporter.cpp   |  27 +++---
 clang/unittests/AST/ASTImporterTest.cpp | 105 
 2 files changed, 122 insertions(+), 10 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c4e931e220f69b5..7a5e3d665328532 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6245,17 +6245,21 @@ ExpectedDecl 
ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
   D->getTemplatedDecl()))
 continue;
   if (IsStructuralMatch(D, FoundTemplate)) {
-// The Decl in the "From" context has a definition, but in the
-// "To" context we already have a definition.
+// FIXME Check for ODR error if the two definitions have
+// different initializers?
 VarTemplateDecl *FoundDef = getTemplateDefinition(FoundTemplate);
-if (D->isThisDeclarationADefinition() && FoundDef)
-  // FIXME Check for ODR error if the two definitions have
-  // different initializers?
-  return Importer.MapImported(D, FoundDef);
-if (FoundTemplate->getDeclContext()->isRecord() &&
-D->getDeclContext()->isRecord())
-  return Importer.MapImported(D, FoundTemplate);
-
+if (D->getDeclContext()->isRecord()) {
+  assert(FoundTemplate->getDeclContext()->isRecord() &&
+ "Member variable template imported as non-member, "
+ "inconsistent imported AST?");
+  if (FoundDef)
+return Importer.MapImported(D, FoundDef);
+  if (!D->isThisDeclarationADefinition())
+return Importer.MapImported(D, FoundTemplate);
+} else {
+  if (FoundDef && D->isThisDeclarationADefinition())
+return Importer.MapImported(D, FoundDef);
+}
 FoundByLookup = FoundTemplate;
 break;
   }
@@ -6374,7 +6378,10 @@ ExpectedDecl 
ASTNodeImporter::VisitVarTemplateSpecializationDecl(
 // variable.
 return Importer.MapImported(D, FoundDef);
   }
+  // FIXME HandleNameConflict
+  return make_error(ASTImportError::NameConflict);
 }
+return Importer.MapImported(D, D2);
   } else {
 TemplateArgumentListInfo ToTAInfo;
 if (const ASTTemplateArgumentListInfo *Args = D->getTemplateArgsInfo()) {
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 5f4d8d040772cb1..d439a14b7b9985f 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -5050,6 +5050,111 @@ TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
   EXPECT_EQ(ToTUX, ToX);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateDeclConflict) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  constexpr int X = 1;
+  )",
+  Lang_CXX14);
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  constexpr int X = 2;
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromX = FirstDeclMatcher().match(
+  FromTU, varTemplateDecl(hasName("X")));
+  auto *ToX = Import(FromX, Lang_CXX11);
+  // FIXME: This import should fail.
+  EXPECT_TRUE(ToX);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateStaticDefinition) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  struct A {
+template 
+static int X;
+  };
+  )",
+  Lang_CXX14);
+  auto *ToX = FirstDeclMatcher().match(
+  ToTU, varTemplateDecl(hasName("X")));
+  ASSERT_FALSE(ToX->isThisDeclarationADefinition());
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct A {
+template 
+static int X;
+  };
+  template 
+  int A::X = 2;
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromXDef = LastDeclMatcher().match(
+  FromTU, varTemplateDecl(hasName("X")));
+  ASSERT_TRUE(FromXDef->isThisDeclarationADefinition());
+  auto *ToXDef = Import(FromXDef, Lang_CXX14);
+  EXPECT_TRUE(ToXDef);
+  EXPECT_TRUE(ToXDef->isThisDeclarationADefinition());
+  EXPECT_EQ(ToXDef->getPreviousDecl(), ToX);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateSpecializationDeclValue) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  constexpr int X = U::Value;
+  struct A { static constexpr int Value = 1; };
+ 

[clang] [clang][analyzer] Improve 'errno' handling in StdLibraryFunctionsChecker. (PR #71392)

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/71392
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

balazske wrote:

> I also think that the 
> [reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=New&report-id=3253666&report-hash=b4e0b723164236269fe6078ad32a0456&report-filepath=%2apg_basebackup.c)
>  
> [from](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=New&report-id=3254052&report-hash=619eb5d998324adb5e02d9e3302bb4d5&report-filepath=%2acopy.c)
>  
> [postgres](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=New&report-id=3256373&report-hash=32f8e213c6fb419277ec76c40bfa3956&report-filepath=%2afe-connect.c)
>  that you mentioned are too confusing.
> 

After the change in #71392 the report looks like 
[this](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_test_71392_base&newcheck=postgres_REL_13_0_test_71392&is-unique=on&diff-type=Unresolved&checker-name=alpha.unix.Errno&report-hash=0b2d4e61a48cd556bb982b39969b81d2&report-id=3386353&report-filepath=%2acopy.c).
 This looks somewhat better, probably still false positive because the "len" 
can not be 0, but the checker does not have more information.

https://github.com/llvm/llvm-project/pull/69469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.


https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Use condition type for comparison in several checkers (PR #72358)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.


https://github.com/llvm/llvm-project/pull/72358
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Restrict 'fopen' modeling to POSIX versions in SimpleStreamChecker (PR #72016)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.

The change is OK, automated tests show a failure but it is in an unrelated test.

https://github.com/llvm/llvm-project/pull/72016
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

@@ -101,6 +101,23 @@ void error_fwrite(void) {
   Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already 
closed}}
 }
 
+void error_fputc(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int Ret = fputc('X', F);
+  if (Ret == EOF) {

balazske wrote:

I would add a line with `clang_analyzer_eval(feof(F));` expected as FALSE to 
both of the branches of `if`. This test is analogous to `error_fwrite` only 
with `fputc`.

https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

@@ -268,3 +302,20 @@ void error_indeterminate_feof2(void) {
   }
   fclose(F);
 }
+
+void error_indeterminate_feof3(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+return;
+  char Buf[10];
+  if (fread(Buf, 1, 10, F) < 10) {
+if (feof(F)) {
+  // error is feof, should be non-indeterminate
+  fputc(';', F); // no warning
+}
+if (ferror(F)) {
+  fputc('=', F); // expected-warning {{might be 'indeterminate'}}
+}
+  }
+  fclose(F);
+}

balazske wrote:

I think that these 2 added "indeterminate" tests should be combined into one, 
or this last can be removed. The tests called `error_indeterminate_eof` have 
the role to test when the stream becomes indeterminate (or not) and use the 
function `fwrite` only to check for the presence of indeterminate state, not to 
test `fwrite` itself. So it is not necessary to repeat similar tests with 
`fputc`. But a test is needed to check if a warning for indeterminate state is 
produced by `fputc` in the correct cases, and no warnings for EOF, the function 
`error_indeterminate_fputc` does this already.

It would be better if there would be new debug functions in the checker that 
can get if the stream is "indeterminate" and one that can be used to set values 
of indeterminate, ferror, feof in a stream. Then the tests can be simplified. 
But this is only an idea for another PR, probably I will do this later.

https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Improve structural equivalence of overloadable operators. (PR #72242)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/72242

Operators that are overloadable may be parsed as `CXXOperatorCallExpr` or as 
`UnaryOperator` (or `BinaryOperator`). This depends on the context and can be 
different if a similar construct is imported into an existing AST. The two 
"forms" of the operator call AST nodes should be detected as equivalent to 
allow AST import of these cases.

This fix has probably other consequences because if a structure is imported 
that has `CXXOperatorCallExpr` into an AST with an existing similar structure 
that has `UnaryOperator` (or binary), the additional data in the 
`CXXOperatorCallExpr` node is lost at the import (because the existing node 
will be used). I am not sure if this can cause problems.

From 5300f979c96eb2f88c298872f0519e274c155cfe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Tue, 14 Nov 2023 11:53:20 +0100
Subject: [PATCH] [clang][ASTImporter] Improve structural equivalence of
 overloadable operators.

Operators that are overloadable may be parsed as `CXXOperatorCallExpr`
or as `UnaryOperator` (or `BinaryOperator`). This depends on the context
and can be different if a similar construct is imported into an existing AST.
The two "forms" of the operator call AST nodes should be detected as
equivalent to allow AST import of these cases.

This fix has probably other consequences because if a structure is imported
that has `CXXOperatorCallExpr` into an AST with an existing similar structure
that has `UnaryOperator` (or binary), the additional data in the
`CXXOperatorCallExpr` node is lost at the import (because the existing node
will be used). I am not sure if this can cause problems.
---
 clang/lib/AST/ASTStructuralEquivalence.cpp|  57 ++
 .../AST/StructuralEquivalenceTest.cpp | 170 ++
 2 files changed, 227 insertions(+)

diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp 
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 6bb4bf14b873d7b..b937ff0579ca02d 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -98,6 +98,8 @@ static bool 
IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
  QualType T1, QualType T2);
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
  Decl *D1, Decl *D2);
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const Stmt *S1, const Stmt *S2);
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
  const TemplateArgument &Arg1,
  const TemplateArgument &Arg2);
@@ -437,12 +439,67 @@ class StmtComparer {
 };
 } // namespace
 
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const UnaryOperator *E1,
+ const CXXOperatorCallExpr *E2) {
+  return UnaryOperator::getOverloadedOperator(E1->getOpcode()) ==
+ E2->getOperator() &&
+ IsStructurallyEquivalent(Context, E1->getSubExpr(), E2->getArg(0));
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const CXXOperatorCallExpr *E1,
+ const UnaryOperator *E2) {
+  return E1->getOperator() ==
+ UnaryOperator::getOverloadedOperator(E2->getOpcode()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(0), E2->getSubExpr());
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const BinaryOperator *E1,
+ const CXXOperatorCallExpr *E2) {
+  return BinaryOperator::getOverloadedOperator(E1->getOpcode()) ==
+ E2->getOperator() &&
+ IsStructurallyEquivalent(Context, E1->getLHS(), E2->getArg(0)) &&
+ IsStructurallyEquivalent(Context, E1->getRHS(), E2->getArg(1));
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const CXXOperatorCallExpr *E1,
+ const BinaryOperator *E2) {
+  return E1->getOperator() ==
+ BinaryOperator::getOverloadedOperator(E2->getOpcode()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(0), E2->getLHS()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(1), E2->getRHS());
+}
+
 /// Determine structural equivalence of two statements.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
  const Stmt *S1, const Stmt *S2) {
   if (!S1 || !S2)
 return S1 == S2;
 
+  // Check for statements with similar syntax but different AST.
+  // The unary and binary operators (like '+', '&') can be parsed as
+  // CXXOpe

[clang] [clang][analyzer] Restrict 'fopen' modeling to POSIX versions in SimpleStreamChecker (PR #72016)

[Balázs Kéri via cfe-commits]

balazske wrote:

The functional change looks good but the reformatting should be put into a 
separate change.

https://github.com/llvm/llvm-project/pull/72016
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Improve StdLibraryFunctionsChecker 'readlink' modeling. (PR #71373)

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/71373
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Improve 'errno' handling in StdLibraryFunctionsChecker. (PR #71392)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/71392

From e92bf72fc80bb6823996cb71cb811d238b304aaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Thu, 2 Nov 2023 18:12:32 +0100
Subject: [PATCH 1/3] [clang][analyzer] Improve 'errno' handling in
 StdLibraryFunctionsChecker.

The checker now displays one combined note tag for errno-related and
"case"-related notes. Previous functions in the errno-modeling part
that were used for construction of note tags are removed. The note tag
added by StdLibraryFunctionsChecker contains the code to display the
note tag for 'errno' (this was done previously by these removed
functions).
---
 .../StaticAnalyzer/Checkers/ErrnoModeling.cpp |  12 ---
 .../StaticAnalyzer/Checkers/ErrnoModeling.h   |  16 +--
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 101 ++
 .../errno-stdlibraryfunctions-notes.c |   4 +-
 .../std-c-library-functions-path-notes.c  |   9 ++
 clang/test/Analysis/stream-errno-note.c   |  41 +--
 6 files changed, 101 insertions(+), 82 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index be2fa91b994a26d..1b34ea0e056e563 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -312,18 +312,6 @@ ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef 
State,
   return setErrnoState(State, MustBeChecked);
 }
 
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
-  return getErrnoNoteTag(
-  C, llvm::formatv(
- "'errno' may be undefined after successful call to '{0}'", Fn));
-}
-
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn) {
-  return getErrnoNoteTag(
-  C, llvm::formatv("'{0}' indicates failure only by setting 'errno'", Fn));
-}
-
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 0707fd16d6e60a2..6b53572fe5e2d93 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -84,8 +84,7 @@ const NoteTag *getErrnoNoteTag(CheckerContext &C, const 
std::string &Message);
 
 /// Set errno state for the common case when a standard function is successful.
 /// Set \c ErrnoCheckState to \c MustNotBeChecked (the \c errno value is not
-/// affected). At the state transition a note tag created by
-/// \c getNoteTagForStdSuccess can be used.
+/// affected).
 ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State, CheckerContext 
&C);
 
 /// Set errno state for the common case when a standard function fails.
@@ -100,23 +99,10 @@ ProgramStateRef setErrnoForStdFailure(ProgramStateRef 
State, CheckerContext &C,
 /// Set errno state for the common case when a standard function indicates
 /// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
 /// invalidates the errno region (clear of previous value).
-/// At the state transition a note tag created by
-/// \c getNoteTagForStdMustBeChecked can be used.
 /// \arg \c InvalE Expression that causes invalidation of \c errno.
 ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
  CheckerContext &C, const Expr 
*InvalE);
 
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoForStdSuccess .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
-
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoStdMustBeChecked .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn);
-
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 13bb9cef5e490ed..035314c6f7751d5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -533,13 +533,11 @@ class StdLibraryFunctionsChecker
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary,
   CheckerContext &C) const = 0;
-/// Get a NoteTag about the changes made to 'errno' and the possible bug.
-/// It may return \c nullptr (if no bug report from \c ErrnoChecker is
-/// expected).
-virtual const NoteTag *describe(CheckerContext &C,
-

[clang] [clang][analyzer] Improve StdLibraryFunctionsChecker 'readlink' modeling. (PR #71373)

[Balázs Kéri via cfe-commits]

balazske wrote:

The checker was already tested on some projects, but much more is needed to 
find such corner cases. It can be better to manually check the functions for 
cases when a 0 return value is not possible or only at a special (known) case.

https://github.com/llvm/llvm-project/pull/71373
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Improve StdLibraryFunctionsChecker 'readlink' modeling. (PR #71373)

[Balázs Kéri via cfe-commits]

balazske wrote:

I tested on vim and the problematic report disappeared, no other changes were 
detected.

https://github.com/llvm/llvm-project/pull/71373
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

@@ -745,6 +751,46 @@ void StreamChecker::evalFreadFwrite(const FnDescription 
*Desc,
 C.addTransition(StateFailed);
 }
 
+void StreamChecker::evalFgetcFputc(const FnDescription *Desc,

balazske wrote:

I do not know if there are enough similarities between `fputc` and `fgetc` to 
put these in a single function (return value of `fgetc` is differently 
constructed and different errors can happen including FEOF). The function 
should now be called only `evalFputc` because there is no "IsRead" argument, if 
later this is reused for `fgetc` it can be renamed.

https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

@@ -14,6 +14,12 @@ void check_fwrite(void) {
   fclose(fp);
 }
 
+void check_fgetc(void) {

balazske wrote:

This should be `check_fputc`.

https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

@@ -111,6 +117,14 @@ void f_use_after_close(void) {
   clearerr(p); // expected-warning {{Stream might be already closed}}
 }
 
+void f_write_after_close(void) {

balazske wrote:

This is not needed if the other indicated test is added.

https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

@@ -268,3 +285,41 @@ void error_indeterminate_feof2(void) {
   }
   fclose(F);
 }
+
+void error_indeterminate_feof3(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+return;
+  char Buf[10];
+  if (fread(Buf, 1, 10, F) < 10) {
+if (feof(F)) {
+  // error is feof, should be non-indeterminate
+  fputc(';', F); // no warning
+}
+if (ferror(F)) {
+  fputc('=', F); // expected-warning {{might be 'indeterminate'}}
+}
+  }
+  fclose(F);
+}
+
+void error_indeterminate_feof4(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+return;
+  if (fputc('Y', F) == EOF) {
+fputc('W', F); // expected-warning {{might be 'indeterminate'}}
+  } else {
+fputc('H', F); // no warning
+  }
+  fclose(F);
+}
+
+void determinate_fputc(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+return;
+  if (fputc('Q', F) == 'Q')
+fputc('X', F); // no warning
+  fclose(F);
+}

balazske wrote:

This one test could be sufficient to add to this file:
```
void error_fputc(void) {
  FILE *F = tmpfile();
  if (!F)
return;
  int Ret = fputc('X', F);
  if (Ret == EOF) {
clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
fputc('Y', F); // expected-warning {{might be 'indeterminate'}}
  } else {
clang_analyzer_eval(Ret == 'X'); // expected-warning {{TRUE}}
clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
fputc('Y', F); // no-warning
  }
  fclose(F);
  fputc('A', F); // expected-warning {{Stream might be already closed}}
}
```

https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fputc` in StreamChecker (PR #71518)

[Balázs Kéri via cfe-commits]

https://github.com/balazske requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/71518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Improve StdLibraryFunctionsChecker 'readlink' modeling. (PR #71373)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/71373

From 653aeb7f5b0d0f200b3f706bad770a9be643669c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Fri, 3 Nov 2023 09:48:18 +0100
Subject: [PATCH 1/2] [clang][analyzer] Improve StdLibraryFunctionsChecker
 'readlink' modeling.

The functions 'readlink' and 'readlinkat' do return 0 only if the
'bufsize' argument is 0.
---
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 18 +
 .../Analysis/std-c-library-functions-POSIX.c  | 20 +++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 13bb9cef5e490ed..54a41b8bd7843dd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -2865,9 +2865,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Signature(ArgTypes{ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTy},
   RetType{Ssize_tTy}),
 Summary(NoEvalCall)
-.Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+.Case({ArgumentCondition(2, WithinRange, Range(1, IntMax)),
+   ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+   ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case({ArgumentCondition(2, WithinRange, SingleValue(0)),
+   ReturnValueCondition(WithinRange, SingleValue(0))},
+  ErrnoMustNotBeChecked,
+  "Assuming that argument 'bufsize' is 0")
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(NotNull(ArgNo(0)))
 .ArgConstraint(NotNull(ArgNo(1)))
@@ -2884,9 +2889,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 ArgTypes{IntTy, ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTy},
 RetType{Ssize_tTy}),
 Summary(NoEvalCall)
-.Case({ReturnValueCondition(LessThanOrEq, ArgNo(3)),
-   ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+.Case({ArgumentCondition(3, WithinRange, Range(1, IntMax)),
+   ReturnValueCondition(LessThanOrEq, ArgNo(3)),
+   ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case({ArgumentCondition(3, WithinRange, SingleValue(0)),
+   ReturnValueCondition(WithinRange, SingleValue(0))},
+  ErrnoMustNotBeChecked,
+  "Assuming that argument 'bufsize' is 0")
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
 .ArgConstraint(NotNull(ArgNo(1)))
diff --git a/clang/test/Analysis/std-c-library-functions-POSIX.c 
b/clang/test/Analysis/std-c-library-functions-POSIX.c
index 84ce0f21e569fb5..daa4d904c3ac5ed 100644
--- a/clang/test/Analysis/std-c-library-functions-POSIX.c
+++ b/clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -205,3 +205,23 @@ void test_sendmsg(int sockfd, const struct msghdr *msg, 
int flags) {
   ssize_t Ret = sendmsg(sockfd, msg, flags);
   clang_analyzer_eval(Ret != 0); // expected-warning{{TRUE}}
 }
+
+void test_readlink_bufsize_zero(char *Buf, size_t Bufsize) {
+  ssize_t Ret = readlink("path", Buf, Bufsize);
+  if (Ret == 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{TRUE}}
+  else if (Ret > 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{FALSE}}
+  else
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{UNKNOWN}}
+}
+
+void test_readlinkat_bufsize_zero(int fd, char *Buf, size_t Bufsize) {
+  ssize_t Ret = readlinkat(fd, "path", Buf, Bufsize);
+  if (Ret == 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{TRUE}}
+  else if (Ret > 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{FALSE}}
+  else
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{UNKNOWN}}
+}

From d66440541d1fbbf50f5b750f306f79a00d3aaedc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Wed, 8 Nov 2023 16:17:15 +0100
Subject: [PATCH 2/2] add missing test

---
 clang/test/Analysis/std-c-library-functions-path-notes.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/clang/test/Analysis/std-c-library-functions-path-notes.c 
b/clang/test/Analysis/std-c-library-functions-path-notes.c
index d0957483c1391ad..4df00fe1e60646f 100644
--- a/clang/test/Analysis/std-c-library-functions-path-notes.c
+++ b/clang/test/Analysis/std-c-library-functions-path-notes.c
@@ -80,3 +80,12 @@ int test_fileno_arg_note(FILE

[clang] [clang][analyzer] Improve 'errno' handling in StdLibraryFunctionsChecker. (PR #71392)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/71392

From e92bf72fc80bb6823996cb71cb811d238b304aaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Thu, 2 Nov 2023 18:12:32 +0100
Subject: [PATCH 1/2] [clang][analyzer] Improve 'errno' handling in
 StdLibraryFunctionsChecker.

The checker now displays one combined note tag for errno-related and
"case"-related notes. Previous functions in the errno-modeling part
that were used for construction of note tags are removed. The note tag
added by StdLibraryFunctionsChecker contains the code to display the
note tag for 'errno' (this was done previously by these removed
functions).
---
 .../StaticAnalyzer/Checkers/ErrnoModeling.cpp |  12 ---
 .../StaticAnalyzer/Checkers/ErrnoModeling.h   |  16 +--
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 101 ++
 .../errno-stdlibraryfunctions-notes.c |   4 +-
 .../std-c-library-functions-path-notes.c  |   9 ++
 clang/test/Analysis/stream-errno-note.c   |  41 +--
 6 files changed, 101 insertions(+), 82 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index be2fa91b994a26d..1b34ea0e056e563 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -312,18 +312,6 @@ ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef 
State,
   return setErrnoState(State, MustBeChecked);
 }
 
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
-  return getErrnoNoteTag(
-  C, llvm::formatv(
- "'errno' may be undefined after successful call to '{0}'", Fn));
-}
-
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn) {
-  return getErrnoNoteTag(
-  C, llvm::formatv("'{0}' indicates failure only by setting 'errno'", Fn));
-}
-
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 0707fd16d6e60a2..6b53572fe5e2d93 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -84,8 +84,7 @@ const NoteTag *getErrnoNoteTag(CheckerContext &C, const 
std::string &Message);
 
 /// Set errno state for the common case when a standard function is successful.
 /// Set \c ErrnoCheckState to \c MustNotBeChecked (the \c errno value is not
-/// affected). At the state transition a note tag created by
-/// \c getNoteTagForStdSuccess can be used.
+/// affected).
 ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State, CheckerContext 
&C);
 
 /// Set errno state for the common case when a standard function fails.
@@ -100,23 +99,10 @@ ProgramStateRef setErrnoForStdFailure(ProgramStateRef 
State, CheckerContext &C,
 /// Set errno state for the common case when a standard function indicates
 /// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
 /// invalidates the errno region (clear of previous value).
-/// At the state transition a note tag created by
-/// \c getNoteTagForStdMustBeChecked can be used.
 /// \arg \c InvalE Expression that causes invalidation of \c errno.
 ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
  CheckerContext &C, const Expr 
*InvalE);
 
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoForStdSuccess .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
-
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoStdMustBeChecked .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn);
-
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 13bb9cef5e490ed..035314c6f7751d5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -533,13 +533,11 @@ class StdLibraryFunctionsChecker
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary,
   CheckerContext &C) const = 0;
-/// Get a NoteTag about the changes made to 'errno' and the possible bug.
-/// It may return \c nullptr (if no bug report from \c ErrnoChecker is
-/// expected).
-virtual const NoteTag *describe(CheckerContext &C,
-

[clang] [clang][analyzer][NFC] Remove redundant code in StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.


https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer][NFC] Remove redundant code in StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

balazske wrote:

This change looks not very useful to me. These removed return statements 
indicate that at the end of the `if` branch there is no more work to do in this 
function, and it reduces complexity (less execution paths). The code becomes a 
bit shorter but not necessarily more easy to understand. Optimizing code for 
execution speed or binary size is not important because I am sure that compiler 
optimizations can handle this case.

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

balazske wrote:

With the current code it is a corner case if this change makes the code more 
readable. Probably it can be useful if new functions are added to the checker. 
But the rule here is that there is one "ensure" function to check one aspect of 
the state, and the pre-callbacks call all of the ensure functions that are 
needed in that case. It would not much more readable if multiple combinations 
of ensure functions are made, for example 
`ensureStreamNonNullAndOpenedAndNoFilePositionIndeterminate`.

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

@@ -639,12 +644,7 @@ void StreamChecker::preFreadFwrite(const FnDescription 
*Desc,
bool IsFread) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))

balazske wrote:

The function returns a state that is modified further by the following 
functions. Otherwise the state changes applied in `basicCheck` are lost.  
(`ensureStreamNonNull` does a state change, `ensureStreamOpened` does not, but 
all return a state to make the usage similar.)

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

@@ -342,6 +342,11 @@ class StreamChecker : public Checkerhttps://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

@@ -639,12 +644,7 @@ void StreamChecker::preFreadFwrite(const FnDescription 
*Desc,
bool IsFread) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))

balazske wrote:

```suggestion
  State = ensureStreamNonNullAndOpened(Desc, Call, C, State, StreamVal);
  if (!State)
return;
```

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

https://github.com/balazske requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

balazske wrote:

PRs #71373 and #71392 are created to improve the indicated problems.

https://github.com/llvm/llvm-project/pull/69469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Improve 'errno' handling in StdLibraryFunctionsChecker. (PR #71392)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/71392

The checker now displays one combined note tag for errno-related and 
"case"-related notes. Previous functions in the errno-modeling part that were 
used for construction of note tags are removed. The note tag added by 
StdLibraryFunctionsChecker contains the code to display the note tag for 
'errno' (this was done previously by these removed functions).

From e92bf72fc80bb6823996cb71cb811d238b304aaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Thu, 2 Nov 2023 18:12:32 +0100
Subject: [PATCH] [clang][analyzer] Improve 'errno' handling in
 StdLibraryFunctionsChecker.

The checker now displays one combined note tag for errno-related and
"case"-related notes. Previous functions in the errno-modeling part
that were used for construction of note tags are removed. The note tag
added by StdLibraryFunctionsChecker contains the code to display the
note tag for 'errno' (this was done previously by these removed
functions).
---
 .../StaticAnalyzer/Checkers/ErrnoModeling.cpp |  12 ---
 .../StaticAnalyzer/Checkers/ErrnoModeling.h   |  16 +--
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 101 ++
 .../errno-stdlibraryfunctions-notes.c |   4 +-
 .../std-c-library-functions-path-notes.c  |   9 ++
 clang/test/Analysis/stream-errno-note.c   |  41 +--
 6 files changed, 101 insertions(+), 82 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index be2fa91b994a26d..1b34ea0e056e563 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -312,18 +312,6 @@ ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef 
State,
   return setErrnoState(State, MustBeChecked);
 }
 
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
-  return getErrnoNoteTag(
-  C, llvm::formatv(
- "'errno' may be undefined after successful call to '{0}'", Fn));
-}
-
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn) {
-  return getErrnoNoteTag(
-  C, llvm::formatv("'{0}' indicates failure only by setting 'errno'", Fn));
-}
-
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 0707fd16d6e60a2..6b53572fe5e2d93 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -84,8 +84,7 @@ const NoteTag *getErrnoNoteTag(CheckerContext &C, const 
std::string &Message);
 
 /// Set errno state for the common case when a standard function is successful.
 /// Set \c ErrnoCheckState to \c MustNotBeChecked (the \c errno value is not
-/// affected). At the state transition a note tag created by
-/// \c getNoteTagForStdSuccess can be used.
+/// affected).
 ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State, CheckerContext 
&C);
 
 /// Set errno state for the common case when a standard function fails.
@@ -100,23 +99,10 @@ ProgramStateRef setErrnoForStdFailure(ProgramStateRef 
State, CheckerContext &C,
 /// Set errno state for the common case when a standard function indicates
 /// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
 /// invalidates the errno region (clear of previous value).
-/// At the state transition a note tag created by
-/// \c getNoteTagForStdMustBeChecked can be used.
 /// \arg \c InvalE Expression that causes invalidation of \c errno.
 ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
  CheckerContext &C, const Expr 
*InvalE);
 
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoForStdSuccess .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
-
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoStdMustBeChecked .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn);
-
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 13bb9cef5e490ed..035314c6f7751d5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -533,13 +533,11 @@ class StdLibraryFunctionsChecker
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
  

[clang] [clang][analyzer] Improve StdLibraryFunctionsChecker 'readlink' modeling. (PR #71373)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/71373

The functions 'readlink' and 'readlinkat' do return 0 only if the 'bufsize' 
argument is 0.

From 653aeb7f5b0d0f200b3f706bad770a9be643669c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Fri, 3 Nov 2023 09:48:18 +0100
Subject: [PATCH] [clang][analyzer] Improve StdLibraryFunctionsChecker
 'readlink' modeling.

The functions 'readlink' and 'readlinkat' do return 0 only if the
'bufsize' argument is 0.
---
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 18 +
 .../Analysis/std-c-library-functions-POSIX.c  | 20 +++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 13bb9cef5e490ed..54a41b8bd7843dd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -2865,9 +2865,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Signature(ArgTypes{ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTy},
   RetType{Ssize_tTy}),
 Summary(NoEvalCall)
-.Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+.Case({ArgumentCondition(2, WithinRange, Range(1, IntMax)),
+   ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+   ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case({ArgumentCondition(2, WithinRange, SingleValue(0)),
+   ReturnValueCondition(WithinRange, SingleValue(0))},
+  ErrnoMustNotBeChecked,
+  "Assuming that argument 'bufsize' is 0")
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(NotNull(ArgNo(0)))
 .ArgConstraint(NotNull(ArgNo(1)))
@@ -2884,9 +2889,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 ArgTypes{IntTy, ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTy},
 RetType{Ssize_tTy}),
 Summary(NoEvalCall)
-.Case({ReturnValueCondition(LessThanOrEq, ArgNo(3)),
-   ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+.Case({ArgumentCondition(3, WithinRange, Range(1, IntMax)),
+   ReturnValueCondition(LessThanOrEq, ArgNo(3)),
+   ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case({ArgumentCondition(3, WithinRange, SingleValue(0)),
+   ReturnValueCondition(WithinRange, SingleValue(0))},
+  ErrnoMustNotBeChecked,
+  "Assuming that argument 'bufsize' is 0")
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
 .ArgConstraint(NotNull(ArgNo(1)))
diff --git a/clang/test/Analysis/std-c-library-functions-POSIX.c 
b/clang/test/Analysis/std-c-library-functions-POSIX.c
index 84ce0f21e569fb5..daa4d904c3ac5ed 100644
--- a/clang/test/Analysis/std-c-library-functions-POSIX.c
+++ b/clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -205,3 +205,23 @@ void test_sendmsg(int sockfd, const struct msghdr *msg, 
int flags) {
   ssize_t Ret = sendmsg(sockfd, msg, flags);
   clang_analyzer_eval(Ret != 0); // expected-warning{{TRUE}}
 }
+
+void test_readlink_bufsize_zero(char *Buf, size_t Bufsize) {
+  ssize_t Ret = readlink("path", Buf, Bufsize);
+  if (Ret == 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{TRUE}}
+  else if (Ret > 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{FALSE}}
+  else
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{UNKNOWN}}
+}
+
+void test_readlinkat_bufsize_zero(int fd, char *Buf, size_t Bufsize) {
+  ssize_t Ret = readlinkat(fd, "path", Buf, Bufsize);
+  if (Ret == 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{TRUE}}
+  else if (Ret > 0)
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{FALSE}}
+  else
+clang_analyzer_eval(Bufsize == 0); // expected-warning{{UNKNOWN}}
+}

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

[clang] [clang][analyzer] Restrict 'fopen' & 'tmpfile' modeling to POSIX versions in StreamChecker (PR #70540)

[Balázs Kéri via cfe-commits]

@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -fno-builtin 
-analyzer-checker=core,alpha.unix.Stream -verify %s
+// expected-no-diagnostics
+
+typedef struct _FILE FILE;
+
+// These functions are not standard C library functions.
+FILE *tmpfile(const char *restrict path); // Real 'tmpfile' should have 
exactly 0 formal parameters.
+FILE *fopen(const char *restrict path);   // Real 'fopen' should have exactly 
2 formal parameters.
+
+void test_fopen_non_posix(void) {
+  FILE *fp = fopen("file"); // no-leak: this isn't the standard POSIX fopen, 
we don't the semantics of this call.
+}
+
+void test_tmpfile_non_posix(void) {
+  FILE *fp = tmpfile("file"); // no-leak: this isn't the standard POSIX 
tmpfile, we don't the semantics of this call.

balazske wrote:

The comment looks like something is missing from it.

https://github.com/llvm/llvm-project/pull/70540
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Restrict 'fopen' & 'tmpfile' modeling to POSIX versions in StreamChecker (PR #70540)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.


https://github.com/llvm/llvm-project/pull/70540
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Restrict 'fopen' & 'tmpfile' modeling to POSIX versions in StreamChecker (PR #70540)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/70540
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer][NFC] Add more tests of 'StreamChecker' about 'tmpfile' (PR #70540)

[Balázs Kéri via cfe-commits]

balazske wrote:

I would like to have the new test in `stream.c` (not `stream.cpp`) because the 
C++ test file contains only C++ related StreamChecker tests (`tmpfile` belongs 
not here). And a "FIXME" could be added to the test to indicate that this is a 
faulty behavior (in the current case). The test and code change can be in a 
single PR, this is a bugfix and addition of a related test, these are done 
usually together.

https://github.com/llvm/llvm-project/pull/70540
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer][NFC] Add more information to CallDescriptions in StreamChecker (PR #70540)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/70540
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer][NFC] Add more information to CallDescriptions in StreamChecker (PR #70540)

[Balázs Kéri via cfe-commits]

https://github.com/balazske commented:

The change is good but the title is too general, for example "Update 
CallDescription in StreamChecker for `tmpfile`", and this is not a NFC (it 
fixes a problem).

https://github.com/llvm/llvm-project/pull/70540
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Add 'tmpfile' as an open function to SimpleStreamChecker (PR #70539)

[Balázs Kéri via cfe-commits]

balazske wrote:

If I remember correctly there is somewhere in the clang documentation a file 
that refers to this checker, and the purpose of this checker is mostly 
documentation. I do not know if it is good to change the code or to extend this 
checker, because there is `StreamChecker` that should do the same checks but in 
more advanced way. Maybe @haoNoQ can tell more about this.

https://github.com/llvm/llvm-project/pull/70539
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix crash when template class static member imported to other translation unit. (PR #68774)

[Balázs Kéri via cfe-commits]

https://github.com/balazske commented:

The fix looks good but the formatting of the test code could be better, like in 
the other tests.

https://github.com/llvm/llvm-project/pull/68774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix crash when template class static member imported to other translation unit. (PR #68774)

[Balázs Kéri via cfe-commits]

@@ -1373,6 +1373,40 @@ TEST_P(ASTImporterOptionSpecificTestBase, 
ImportCorrectTemplatedDecl) {
   ASSERT_EQ(ToTemplated1, ToTemplated);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportTemplateSpecializationStaticMember) {
+  auto FromCode = R"(
+  template  class Test{
+  public:
+ static const unsigned int length;
+  };
+
+  template<> const unsigned int Test::length;
+  template<> const unsigned int Test::length = 0;
+  )";
+  auto ToCode = R"(
+template  class Test
+{
+public:
+   static const unsigned int length;
+};
+template<> const unsigned int Test::length;
+
+void foo(){
+int i = 1 / Test::length;
+}
+)";

balazske wrote:

It is better to have the usual code formatting in the test code.

https://github.com/llvm/llvm-project/pull/68774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix crash when template class static member imported to other translation unit. (PR #68774)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/68774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix import of SubstTemplateTypeParmType in return type of function. (PR #69724)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/69724

From 2bfad6f0fbb7a5757318c77ef76335986d23ab83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Thu, 19 Oct 2023 16:27:24 +0200
Subject: [PATCH 1/2] [clang][ASTImporter] Fix import of
 SubstTemplateTypeParmType in return type of function.

Import of a function with `auto` return type that is expanded to a
`SubstTemplateTypeParmType` could fail if the function itself is the template
specialization where the parameter was replaced.
---
 clang/lib/AST/ASTImporter.cpp   | 10 ++
 clang/unittests/AST/ASTImporterTest.cpp | 23 ---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 628a2b2bbca3986..dbcd4e616ecf03c 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3513,6 +3513,14 @@ class IsTypeDeclaredInsideVisitor
 return {};
   }
 
+  std::optional
+  VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T) {
+// The "associated declaration" can be the same as ParentDC.
+if (isAncestorDeclContextOf(ParentDC, T->getAssociatedDecl()))
+  return true;
+return {};
+  }
+
   std::optional VisitConstantArrayType(const ConstantArrayType *T) {
 if (T->getSizeExpr() && isAncestorDeclContextOf(ParentDC, 
T->getSizeExpr()))
   return true;
@@ -3573,6 +3581,8 @@ class IsTypeDeclaredInsideVisitor
 };
 } // namespace
 
+/// This function checks if the function has 'auto' return type that contains
+/// a reference (in any way) to a declaration inside the same function.
 bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
   QualType FromTy = D->getType();
   const auto *FromFPT = FromTy->getAs();
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index f1f09a0be2b8d0c..21c24c2fcdc0007 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -6766,10 +6766,13 @@ TEST_P(ASTImporterOptionSpecificTestBase,
 }
 
 struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {
-  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14) 
{
+  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14,
+  bool FindLast = false) {
 Decl *FromTU = getTuDecl(Code, Lang, "input0.cc");
-FunctionDecl *From = FirstDeclMatcher().match(
-FromTU, functionDecl(hasName("foo")));
+FunctionDecl *From = FindLast ? LastDeclMatcher().match(
+FromTU, functionDecl(hasName("foo")))
+  : FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("foo")));
 
 FunctionDecl *To = Import(From, Lang);
 EXPECT_TRUE(To);
@@ -7208,6 +7211,20 @@ TEST_P(ImportAutoFunctions, ReturnWithTypeInSwitch) {
   Lang_CXX17);
 }
 
+TEST_P(ImportAutoFunctions, ReturnWithAutoTemplateType) {
+  testImport(
+  R"(
+  template
+  struct S {};
+  template
+  auto foo() {
+return S{};
+  }
+  auto a = foo(1);
+  )",
+  Lang_CXX14, /*FindLast=*/true);
+}
+
 struct ImportSourceLocations : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ImportSourceLocations, PreserveFileIDTreeStructure) {

From b76ab2c27a4982ef7f14795a388aef87b11a1e49 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Wed, 25 Oct 2023 18:33:12 +0200
Subject: [PATCH 2/2] fix test code

---
 clang/unittests/AST/ASTImporterTest.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 21c24c2fcdc0007..0ac9d8344ad5128 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -7220,7 +7220,7 @@ TEST_P(ImportAutoFunctions, ReturnWithAutoTemplateType) {
   auto foo() {
 return S{};
   }
-  auto a = foo(1);
+  auto a = foo();
   )",
   Lang_CXX14, /*FindLast=*/true);
 }

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

[clang] [analyzer][NFC] Combine similar methods of StreamChecker (PR #70170)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.


https://github.com/llvm/llvm-project/pull/70170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Combine similar methods of StreamChecker (PR #70170)

[Balázs Kéri via cfe-commits]

@@ -1349,7 +1349,7 @@ def warn_omp_extra_tokens_at_eol : Warning<
   "extra tokens at the end of '#pragma omp %0' are ignored">,
   InGroup;
 def err_omp_multiple_step_or_linear_modifier : Error<
-  "multiple %select{'step size'|'linear modifier'}0 found in linear clause">; 
+  "multiple %select{'step size'|'linear modifier'}0 found in linear clause">;

balazske wrote:

This change is not related (or not intentional), it belongs into another commit.

https://github.com/llvm/llvm-project/pull/70170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Combine similar methods of StreamChecker (PR #70170)

[Balázs Kéri via cfe-commits]

https://github.com/balazske requested changes to this pull request.

Probably add [clang] tag to the commit message, and remove the change in 
DiagnosticParseKinds.td, otherwise the change looks good.

https://github.com/llvm/llvm-project/pull/70170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Combine similar methods of StreamChecker (PR #70170)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/70170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

@@ -934,6 +934,76 @@ Check calls to various UNIX/Posix functions: ``open, 
pthread_once, calloc, mallo
 .. literalinclude:: checkers/unix_api_example.c
 :language: c
 
+.. _unix-Errno:
+
+unix.Errno (C)
+""
+
+Check for improper use of ``errno``.
+This checker implements partially CERT rule
+`ERR30-C. Set errno to zero before calling a library function known to set 
errno,
+and check errno only after the function returns a value indicating failure
+`_.
+The checker can find the first read of ``errno`` after successful standard
+function calls.
+
+The C and POSIX standards often do not define if a standard library function
+may change value of ``errno`` if the call does not fail.
+Therefore, ``errno`` should only be used if it is known from the return value
+of a function that the call has failed.
+There are exceptions to this rule (for example ``strtol``) but the affected
+functions are not yet supported by the checker.
+The return values for the failure cases are documented in the standard Linux 
man
+pages of the functions and in the `POSIX standard 
`_.
+
+.. code-block:: c
+
+ int unsafe_errno_read(int sock, void *data, int data_size) {
+   if (send(sock, data, data_size, 0) != data_size) {
+ // 'send' can be successful even if not all data was sent
+ if (errno == 1) { // An undefined value may be read from 'errno'
+   return 0;
+ }
+   }
+   return 1;
+ }
+
+The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
+warnings from this checker. The supported functions are the same as by

balazske wrote:

The dependency is only to have modeling checkers that are dependencies of other 
(non-modeling) checkers. Weak dependency is to have a fixed order of the 
checkers. Only a new type of dependency could work for this case. (The problem 
is that `StdCLibraryFunctionsChecker` is both a modeling and non-modeling 
checker. The situation can be improved only with bigger changes.)

https://github.com/llvm/llvm-project/pull/69469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix import of SubstTemplateTypeParmType in return type of function. (PR #69724)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/69724

Import of a function with `auto` return type that is expanded to a 
`SubstTemplateTypeParmType` could fail if the function itself is the template 
specialization where the parameter was replaced.

From 2bfad6f0fbb7a5757318c77ef76335986d23ab83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Thu, 19 Oct 2023 16:27:24 +0200
Subject: [PATCH] [clang][ASTImporter] Fix import of SubstTemplateTypeParmType
 in return type of function.

Import of a function with `auto` return type that is expanded to a
`SubstTemplateTypeParmType` could fail if the function itself is the template
specialization where the parameter was replaced.
---
 clang/lib/AST/ASTImporter.cpp   | 10 ++
 clang/unittests/AST/ASTImporterTest.cpp | 23 ---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 628a2b2bbca3986..dbcd4e616ecf03c 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3513,6 +3513,14 @@ class IsTypeDeclaredInsideVisitor
 return {};
   }
 
+  std::optional
+  VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T) {
+// The "associated declaration" can be the same as ParentDC.
+if (isAncestorDeclContextOf(ParentDC, T->getAssociatedDecl()))
+  return true;
+return {};
+  }
+
   std::optional VisitConstantArrayType(const ConstantArrayType *T) {
 if (T->getSizeExpr() && isAncestorDeclContextOf(ParentDC, 
T->getSizeExpr()))
   return true;
@@ -3573,6 +3581,8 @@ class IsTypeDeclaredInsideVisitor
 };
 } // namespace
 
+/// This function checks if the function has 'auto' return type that contains
+/// a reference (in any way) to a declaration inside the same function.
 bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
   QualType FromTy = D->getType();
   const auto *FromFPT = FromTy->getAs();
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index f1f09a0be2b8d0c..21c24c2fcdc0007 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -6766,10 +6766,13 @@ TEST_P(ASTImporterOptionSpecificTestBase,
 }
 
 struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {
-  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14) 
{
+  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14,
+  bool FindLast = false) {
 Decl *FromTU = getTuDecl(Code, Lang, "input0.cc");
-FunctionDecl *From = FirstDeclMatcher().match(
-FromTU, functionDecl(hasName("foo")));
+FunctionDecl *From = FindLast ? LastDeclMatcher().match(
+FromTU, functionDecl(hasName("foo")))
+  : FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("foo")));
 
 FunctionDecl *To = Import(From, Lang);
 EXPECT_TRUE(To);
@@ -7208,6 +7211,20 @@ TEST_P(ImportAutoFunctions, ReturnWithTypeInSwitch) {
   Lang_CXX17);
 }
 
+TEST_P(ImportAutoFunctions, ReturnWithAutoTemplateType) {
+  testImport(
+  R"(
+  template
+  struct S {};
+  template
+  auto foo() {
+return S{};
+  }
+  auto a = foo(1);
+  )",
+  Lang_CXX14, /*FindLast=*/true);
+}
+
 struct ImportSourceLocations : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ImportSourceLocations, PreserveFileIDTreeStructure) {

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

[clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

balazske wrote:

The checker was tested additionally on projects libwebm, bitcoin, contour and 
produced no results.

https://github.com/llvm/llvm-project/pull/69469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

balazske wrote:

This checker was dependent on `unix.StdCLibraryFunctions`. After that checker 
was moved out of alpha, it is possible to move `alpha.unix.Errno` out of alpha.
The checker was tested first on the following projects. The following links 
were automatically generated to compare the checker results before and after 
the change. What matters only is the "new reports" where the reports from the 
checker are shown with the new name (and in the "lost reports" with the old 
name), there should be no other difference. I did not find obvious false 
positives, and there are not many results. Most interesting are the cases found 
in `postgres`, even these do not look as false positive. (The checker assumes 
that functions like `fwrite` may write 0 bytes and return 0 if the number to 
write is 1, and this is different from failure of the function when it returns 
-1.)


memcached_1.6.8_errno_alpha_unix_vs_memcached_1.6.8_errno_unix  [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_errno_alpha_unix&newcheck=memcached_1.6.8_errno_unix&is-unique=on&diff-type=New)
   [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_errno_alpha_unix&newcheck=memcached_1.6.8_errno_unix&is-unique=on&diff-type=Resolved)
tmux_2.6_errno_alpha_unix_vs_tmux_2.6_errno_unix[New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_errno_alpha_unix&newcheck=tmux_2.6_errno_unix&is-unique=on&diff-type=New)
 [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_errno_alpha_unix&newcheck=tmux_2.6_errno_unix&is-unique=on&diff-type=Resolved)
curl_curl-7_66_0_errno_alpha_unix_vs_curl_curl-7_66_0_errno_unix[New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_errno_alpha_unix&newcheck=curl_curl-7_66_0_errno_unix&is-unique=on&diff-type=New)
 [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_errno_alpha_unix&newcheck=curl_curl-7_66_0_errno_unix&is-unique=on&diff-type=Resolved)
twin_v0.8.1_errno_alpha_unix_vs_twin_v0.8.1_errno_unix  [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_errno_alpha_unix&newcheck=twin_v0.8.1_errno_unix&is-unique=on&diff-type=New)
   [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_errno_alpha_unix&newcheck=twin_v0.8.1_errno_unix&is-unique=on&diff-type=Resolved)
vim_v8.2.1920_errno_alpha_unix_vs_vim_v8.2.1920_errno_unix  [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_errno_alpha_unix&newcheck=vim_v8.2.1920_errno_unix&is-unique=on&diff-type=New)
   [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_errno_alpha_unix&newcheck=vim_v8.2.1920_errno_unix&is-unique=on&diff-type=Resolved)
openssl_openssl-3.0.0-alpha7_errno_alpha_unix_vs_openssl_openssl-3.0.0-alpha7_errno_unix
[New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_errno_alpha_unix&newcheck=openssl_openssl-3.0.0-alpha7_errno_unix&is-unique=on&diff-type=New)
 [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_errno_alpha_unix&newcheck=openssl_openssl-3.0.0-alpha7_errno_unix&is-unique=on&diff-type=Resolved)
sqlite_version-3.33.0_errno_alpha_unix_vs_sqlite_version-3.33.0_errno_unix  
[New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_errno_alpha_unix&newcheck=sqlite_version-3.33.0_errno_unix&is-unique=on&diff-type=New)
   [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_errno_alpha_unix&newcheck=sqlite_version-3.33.0_errno_unix&is-unique=on&diff-type=Resolved)
ffmpeg_n4.3.1_errno_alpha_unix_vs_ffmpeg_n4.3.1_errno_unix  [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_errno_alpha_unix&newcheck=ffmpeg_n4.3.1_errno_unix&is-unique=on&diff-type=New)
   [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_errno_alpha_unix&newcheck=ffmpeg_n4.3.1_errno_unix&is-unique=on&diff-type=Resolved)
postgres_REL_13_0_errno_alpha_unix_vs_postgres_REL_13_0_errno_unix  [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=New)
   [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=Resolved)

https://github.com/llvm/llvm-project/pull/69469

[clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/69469

None

From 0fdc57b49002afd8aa54043837ee4155688b4c36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Tue, 17 Oct 2023 11:51:27 +0200
Subject: [PATCH] [clang][Analyzer] Move checker 'alpha.unix.Errno' to
 'unix.Errno'.

---
 clang/docs/ReleaseNotes.rst   |   4 +
 clang/docs/analyzer/checkers.rst  | 140 +-
 .../clang/StaticAnalyzer/Checkers/Checkers.td |  24 +--
 clang/test/Analysis/analyzer-config.c |   2 +-
 .../test/Analysis/analyzer-enabled-checkers.c |   1 +
 clang/test/Analysis/errno-notes.c |   4 +-
 clang/test/Analysis/errno-options.c   |   8 +-
 .../errno-stdlibraryfunctions-notes.c |   2 +-
 .../test/Analysis/errno-stdlibraryfunctions.c |   2 +-
 clang/test/Analysis/errno.c   |  16 +-
 ...c-library-functions-arg-enabled-checkers.c |   1 +
 clang/test/Analysis/stream-errno-note.c   |   8 +-
 clang/test/Analysis/stream-errno.c|   6 +-
 clang/test/Analysis/stream-noopen.c   |   4 +-
 14 files changed, 114 insertions(+), 108 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9782c123f4c9372..8cd53d310c784b7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -662,6 +662,10 @@ Static Analyzer
 - Added a new checker ``core.BitwiseShift`` which reports situations where
   bitwise shift operators produce undefined behavior (because some operand is
   negative or too large).
+
+- Move checker ``alpha.unix.Errno`` out of the ``alpha`` package
+  to ``unix.Errno``.
+
 - Move checker ``alpha.unix.StdCLibraryFunctions`` out of the ``alpha`` package
   to ``unix.StdCLibraryFunctions``.
 
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 597ffcc4a10a25b..9c6c059303834bf 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -934,6 +934,76 @@ Check calls to various UNIX/Posix functions: ``open, 
pthread_once, calloc, mallo
 .. literalinclude:: checkers/unix_api_example.c
 :language: c
 
+.. _unix-Errno:
+
+unix.Errno (C)
+""
+
+Check for improper use of ``errno``.
+This checker implements partially CERT rule
+`ERR30-C. Set errno to zero before calling a library function known to set 
errno,
+and check errno only after the function returns a value indicating failure
+`_.
+The checker can find the first read of ``errno`` after successful standard
+function calls.
+
+The C and POSIX standards often do not define if a standard library function
+may change value of ``errno`` if the call does not fail.
+Therefore, ``errno`` should only be used if it is known from the return value
+of a function that the call has failed.
+There are exceptions to this rule (for example ``strtol``) but the affected
+functions are not yet supported by the checker.
+The return values for the failure cases are documented in the standard Linux 
man
+pages of the functions and in the `POSIX standard 
`_.
+
+.. code-block:: c
+
+ int unsafe_errno_read(int sock, void *data, int data_size) {
+   if (send(sock, data, data_size, 0) != data_size) {
+ // 'send' can be successful even if not all data was sent
+ if (errno == 1) { // An undefined value may be read from 'errno'
+   return 0;
+ }
+   }
+   return 1;
+ }
+
+The checker :ref:`unix-StdCLibraryFunctions` must be turned on to get the
+warnings from this checker. The supported functions are the same as by
+:ref:`unix-StdCLibraryFunctions`. The ``ModelPOSIX`` option of that
+checker affects the set of checked functions.
+
+**Parameters**
+
+The ``AllowErrnoReadOutsideConditionExpressions`` option allows read of the
+errno value if the value is not used in a condition (in ``if`` statements,
+loops, conditional expressions, ``switch`` statements). For example ``errno``
+can be stored into a variable without getting a warning by the checker.
+
+.. code-block:: c
+
+ int unsafe_errno_read(int sock, void *data, int data_size) {
+   if (send(sock, data, data_size, 0) != data_size) {
+ int err = errno;
+ // warning if 'AllowErrnoReadOutsideConditionExpressions' is false
+ // no warning if 'AllowErrnoReadOutsideConditionExpressions' is true
+   }
+   return 1;
+ }
+
+Default value of this option is ``true``. This allows save of the errno value
+for possible later error handling.
+
+**Limitations**
+
+ - Only the very first usage of ``errno`` is checked after an affected function
+   call. Value of ``errno`` is not followed when it is stored into a variable
+   or returned from a function.
+ - Documentation of function ``lseek`` is not clear about what happens if the
+   function returns different value than the expected file position but not -1.
+   To avoid possible false-pos

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/66207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

https://github.com/balazske reopened 
https://github.com/llvm/llvm-project/pull/66207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/66207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix of possible crash "Did not find base!". (PR #67680)

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/67680
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix crash when import `VarTemplateDecl` in record (PR #67522)

[Balázs Kéri via cfe-commits]

https://github.com/balazske approved this pull request.


https://github.com/llvm/llvm-project/pull/67522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix crash when import `VarTemplateDecl` in record (PR #67522)

[Balázs Kéri via cfe-commits]

@@ -4988,6 +4988,39 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   }
 }
 
+TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  class A {
+  public:
+template 
+static constexpr bool X = true;
+  };
+  )",
+  Lang_CXX14);
+
+  auto *Fwd = FirstDeclMatcher().match(
+  ToTU, varTemplateDecl(hasName("X")));
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  class A {
+  public:
+template 
+static constexpr bool X = true;
+  };
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromA = FirstDeclMatcher().match(
+  FromTU, varTemplateDecl(hasName("X")));
+  auto *ToA = Import(FromA, Lang_CXX11);
+  EXPECT_TRUE(ToA);
+  EXPECT_EQ(Fwd->getTemplatedDecl(), ToA->getTemplatedDecl());

balazske wrote:

Is not a check `EXPECT_EQ(ToA, Fwd)` better here?

https://github.com/llvm/llvm-project/pull/67522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix crash when import `VarTemplateDecl` in record (PR #67522)

[Balázs Kéri via cfe-commits]

@@ -4988,6 +4988,39 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   }
 }
 
+TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  class A {
+  public:
+template 
+static constexpr bool X = true;
+  };
+  )",
+  Lang_CXX14);
+
+  auto *Fwd = FirstDeclMatcher().match(
+  ToTU, varTemplateDecl(hasName("X")));
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  class A {
+  public:
+template 
+static constexpr bool X = true;
+  };
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromA = FirstDeclMatcher().match(

balazske wrote:

`FromA` and `ToA` looks again not correct, these should be `FromX` and `ToX`.

https://github.com/llvm/llvm-project/pull/67522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix crash when import `VarTemplateDecl` in record (PR #67522)

[Balázs Kéri via cfe-commits]

@@ -4988,6 +4988,39 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   }
 }
 
+TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  class A {
+  public:
+template 
+static constexpr bool X = true;
+  };
+  )",
+  Lang_CXX14);
+
+  auto *Fwd = FirstDeclMatcher().match(

balazske wrote:

Name `Fwd` seems not to match with the meaning.of the variable.

https://github.com/llvm/llvm-project/pull/67522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix of possible crash "Did not find base!". (PR #67680)

[Balázs Kéri via cfe-commits]

balazske wrote:

The format checker job looks faulty, it fails in other patches too. I remember 
that clang-format was used on the code, because I usually don't add line breaks 
to long code lines.

https://github.com/llvm/llvm-project/pull/67680
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

balazske wrote:

@haoNoQ Could you check if this change is OK to merge?

https://github.com/llvm/llvm-project/pull/66207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/66207

From 5b9ad350fedad88a4d2ac93bafc29bae893c32e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Wed, 13 Sep 2023 14:56:18 +0200
Subject: [PATCH 1/2] [clang][analyzer] Move checker
 alpha.unix.StdCLibraryFunctions out of alpha.

This checker can be good enough to move out of alpha.
I am not sure about the exact requirements, this review can be a place
for discussion about what should be fixed (if any).

Reviewed By: steakhal

Differential Revision: https://reviews.llvm.org/D152436
---
 clang/docs/ReleaseNotes.rst   |   2 +
 clang/docs/analyzer/checkers.rst  | 188 +-
 .../clang/StaticAnalyzer/Checkers/Checkers.td |  43 ++--
 clang/test/Analysis/PR49642.c |   2 +-
 clang/test/Analysis/analyzer-config.c |   4 +-
 .../test/Analysis/analyzer-enabled-checkers.c |   1 +
 clang/test/Analysis/conversion.c  |   4 +-
 .../errno-stdlibraryfunctions-notes.c |   4 +-
 .../test/Analysis/errno-stdlibraryfunctions.c |   4 +-
 .../std-c-library-functions-POSIX-lookup.c|   6 +-
 ...ibrary-functions-POSIX-socket-sockaddr.cpp |   6 +-
 .../Analysis/std-c-library-functions-POSIX.c  |  12 +-
 ...ry-functions-arg-constraints-note-tags.cpp |   4 +-
 ...ibrary-functions-arg-constraints-notes.cpp |   4 +-
 ...functions-arg-constraints-tracking-notes.c |   2 +-
 .../std-c-library-functions-arg-constraints.c |   8 +-
 ...td-c-library-functions-arg-constraints.cpp |   2 +-
 ...library-functions-arg-cstring-dependency.c |   4 +-
 ...c-library-functions-arg-enabled-checkers.c |  10 +-
 .../std-c-library-functions-arg-weakdeps.c|  10 +-
 .../Analysis/std-c-library-functions-eof.c|  10 +-
 .../std-c-library-functions-inlined.c |  10 +-
 .../Analysis/std-c-library-functions-lookup.c |   4 +-
 .../std-c-library-functions-lookup.cpp|   4 +-
 .../std-c-library-functions-path-notes.c  |   4 +-
 .../std-c-library-functions-restrict.c|   4 +-
 .../std-c-library-functions-restrict.cpp  |   4 +-
 ...td-c-library-functions-vs-stream-checker.c |   8 +-
 clang/test/Analysis/std-c-library-functions.c |  12 +-
 .../test/Analysis/std-c-library-functions.cpp |   2 +-
 .../test/Analysis/std-c-library-posix-crash.c |   4 +-
 clang/test/Analysis/stream-errno-note.c   |   4 +-
 clang/test/Analysis/stream-errno.c|   4 +-
 clang/test/Analysis/stream-noopen.c   |   8 +-
 clang/test/Analysis/stream-note.c |   4 +-
 .../Analysis/stream-stdlibraryfunctionargs.c  |  10 +-
 clang/test/Analysis/weak-dependencies.c   |   2 +-
 37 files changed, 211 insertions(+), 207 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cdad2f7b9f0e5a..dd10e707b2f561c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -408,6 +408,8 @@ Static Analyzer
 - Added a new checker ``core.BitwiseShift`` which reports situations where
   bitwise shift operators produce undefined behavior (because some operand is
   negative or too large).
+- Move checker ``alpha.unix.StdCLibraryFunctions`` out of the ``alpha`` package
+  to ``unix.StdCLibraryFunctions``.
 
 .. _release-notes-sanitizers:
 
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 54ea49e7426cc86..998a9e888f3a3b3 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1026,6 +1026,100 @@ Check for null pointers being passed as arguments to C 
string functions:
return strlen(0); // warn
  }
 
+.. _unix-StdCLibraryFunctions:
+
+unix.StdCLibraryFunctions (C)
+"""
+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the ``int
+isalnum(int ch)`` function the behavior is undefined if the value of ``ch`` is
+not representable as unsigned char and is not equal to ``EOF``.
+
+.. code-block:: c
+
+  #define EOF -1
+  void test_alnum_concrete(int v) {
+int ret = isalnum(256); // \
+// warning: Function argument outside of allowed range
+(void)ret;
+  }
+
+  void buffer_size_violation(FILE *file) {
+enum { BUFFER_SIZE = 1024 };
+wchar_t wbuf[BUFFER_SIZE];
+
+const size_t size = sizeof(*wbuf);   // 4
+const size_t nitems = sizeof(wbuf);  // 4096
+
+// Below we receive a warning because the 3rd parameter should be the
+// number of elements to read, not the size in bytes. This case is a known
+// vulnerability described by the ARR38-C SEI-CERT rule.
+fread(wbuf, size, nitems, file);
+  }
+
+You can think of this checker as defining restrictions (pre- and 
postconditions)
+on standard library functions. Preconditions are checked, and when they are
+violated, a warning is emitted. Post conditions are added to the analysis, e.g.
+that the return value must be no greater than 

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

@@ -1026,6 +1026,100 @@ Check for null pointers being passed as arguments to C 
string functions:
return strlen(0); // warn
  }
 
+.. _unix-StdCLibraryFunctions:
+
+unix.StdCLibraryFunctions (C)
+"""
+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the ``int
+isalnum(int ch)`` function the behavior is undefined if the value of ``ch`` is
+not representable as unsigned char and is not equal to ``EOF``.
+
+.. code-block:: c
+
+  #define EOF -1
+  void test_alnum_concrete(int v) {
+int ret = isalnum(256); // \
+// warning: Function argument outside of allowed range
+(void)ret;
+  }
+
+  void buffer_size_violation(FILE *file) {
+enum { BUFFER_SIZE = 1024 };
+wchar_t wbuf[BUFFER_SIZE];
+
+const size_t size = sizeof(*wbuf);   // 4
+const size_t nitems = sizeof(wbuf);  // 4096
+
+// Below we receive a warning because the 3rd parameter should be the
+// number of elements to read, not the size in bytes. This case is a known
+// vulnerability described by the ARR38-C SEI-CERT rule.
+fread(wbuf, size, nitems, file);
+  }
+
+You can think of this checker as defining restrictions (pre- and 
postconditions)
+on standard library functions. Preconditions are checked, and when they are
+violated, a warning is emitted. Post conditions are added to the analysis, e.g.
+that the return value must be no greater than 255.
+
+For example if an argument to a function must be in between 0 and 255, but the
+value of the argument is unknown, the analyzer will conservatively assume that
+it is in this interval. Similarly, if a function mustn't be called with a null
+pointer and the null value of the argument can not be proven, the analyzer will
+assume that it is non-null.
+
+These are the possible checks on the values passed as function arguments:
+ - The argument has an allowed range (or multiple ranges) of values. The 
checker
+   can detect if a passed value is outside of the allowed range and show the
+   actual and allowed values.
+ - The argument has pointer type and is not allowed to be null pointer. Many
+   (but not all) standard functions can produce undefined behavior if a null
+   pointer is passed, these cases can be detected by the checker.
+ - The argument is a pointer to a memory block and the minimal size of this
+   buffer is determined by another argument to the function, or by
+   multiplication of two arguments (like at function ``fread``), or is a fixed
+   value (for example ``asctime_r`` requires at least a buffer of size 26). The
+   checker can detect if the buffer size is too small and in optimal case show
+   the size of the buffer and the values of the corresponding arguments.
+
+.. code-block:: c
+
+  int test_alnum_symbolic(int x) {
+int ret = isalnum(x);
+// after the call, ret is assumed to be in the range [-1, 255]
+
+if (ret > 255)  // impossible (infeasible branch)
+  if (x == 0)
+return ret / x; // division by zero is not reported
+return ret;
+  }
+
+Additionally to the argument and return value conditions, this checker also 
adds
+state of the value ``errno`` if applicable to the analysis. Many system
+functions set the ``errno`` value only if an error occurs (together with a
+specific return value of the function), otherwise it becomes undefined. This
+checker changes the analysis state to contain such information. This data is
+used by other checkers, for example :ref:`alpha-unix-Errno`.
+
+**Limitations**
+
+The checker can not always provide notes about the values of the arguments.
+Without this information it is hard to confirm if the constraint is indeed
+violated. The argument values are shown if they are known constants or the 
value
+is determined by previous (not too complicated) assumptions.
+
+The checker can produce false positives in cases such as if the program has
+invariants not known to the analyzer engine or the bug report path contains
+calls to unknown functions. In these cases the analyzer fails to detect the 
real
+range of the argument.
+
+**Parameters**
+
+The checker models functions (and emits diagnostics) from the C standard by
+default. The ``ModelPOSIX`` option enables modeling (and emit diagnostics) of
+additional functions that are defined in the POSIX standard. This option is
+disabled by default.
+

balazske wrote:

If I remember correctly the list of functions was already rejected by reviewers 
(because maintenance problems). Otherwise I think it is good to have an exact 
list of modeled functions. 

https://github.com/llvm/llvm-project/pull/66207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix of possible crash "Did not find base!". (PR #67680)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/67680

A problem with AST import could lead to multiple instances of the same template 
class specialization, with different template arguments. The difference was 
caused by pointers to different declarations of the same function.
Problem is fixed by using the canonical declaration at import.

From 73bd7473bf3192a8854614df6430945af855235d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Thu, 28 Sep 2023 12:29:50 +0200
Subject: [PATCH] [clang][ASTImporter] Fix of possible crash "Did not find
 base!".

A problem with AST import could lead to multiple instances of
the same template class specialization, with different template arguments.
The difference was caused by pointers to different declarations of the
same function.
Problem is fixed by using the canonical declaration at import.
---
 clang/lib/AST/ASTImporter.cpp   |  3 +-
 clang/unittests/AST/ASTImporterTest.cpp | 58 +
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c7c2aecc8b179a4..bea6ab40a3437e3 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -811,7 +811,8 @@ ASTNodeImporter::import(const TemplateArgument &From) {
 ExpectedType ToTypeOrErr = import(From.getParamTypeForDecl());
 if (!ToTypeOrErr)
   return ToTypeOrErr.takeError();
-return TemplateArgument(*ToOrErr, *ToTypeOrErr, From.getIsDefaulted());
+return 
TemplateArgument(dyn_cast((*ToOrErr)->getCanonicalDecl()),
+*ToTypeOrErr, From.getIsDefaulted());
   }
 
   case TemplateArgument::NullPtr: {
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 65a5b630e462d0f..abdb80373a5f90e 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9147,6 +9147,64 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   EXPECT_TRUE(ToXType->typeMatchesDecl());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportTemplateArgumentWithPointerToDifferentInstantiation) {
+  const char *CodeTo =
+  R"(
+  template
+  A f1() {
+   return A();
+  }
+  template
+  class X {};
+
+  X> x;
+  )";
+  const char *CodeFrom =
+  R"(
+  template
+  A f1();
+  template
+  class X {};
+
+  X> x;
+  )";
+  Decl *ToTU = getToTuDecl(CodeTo, Lang_CXX11);
+  Decl *FromTU = getTuDecl(CodeFrom, Lang_CXX11);
+
+  auto *ToF1 = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasName("f1"), isInstantiated()));
+  auto *FromF1 = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f1"), isInstantiated()));
+  EXPECT_TRUE(ToF1->isThisDeclarationADefinition());
+  EXPECT_FALSE(FromF1->isThisDeclarationADefinition());
+
+  auto *ToX = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  auto *FromX = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+
+  Decl *ToTArgF = ToX->getTemplateArgs().get(1).getAsDecl();
+  Decl *FromTArgF = FromX->getTemplateArgs().get(1).getAsDecl();
+  EXPECT_EQ(ToTArgF, ToF1);
+  EXPECT_EQ(FromTArgF, FromF1);
+
+  auto *ToXImported = Import(FromX, Lang_CXX11);
+  // The template argument 1 of 'X' in the "From" code points to a function
+  // that has no definition. The import must ensure that this template argument
+  // is imported in a way that it will point to the existing 'f1' function, not
+  // to the 'f1' that is imported. In this way when specialization of 'X' is
+  // imported it will have the same template arguments as the existing one.
+  EXPECT_EQ(ToXImported, ToX);
+  // FIXME: This matcher causes a crash "Tried to match orphan node".
+  // The code is removed until the problem is fixed.
+  // auto *ToF1Imported =
+  //LastDeclMatcher().match(ToTU,
+  //functionDecl(hasName("f1"),isInstantiated()));
+  // EXPECT_NE(ToF1Imported, ToF1);
+  // EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
  DefaultTestValuesForRunOptions);
 

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

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

https://github.com/balazske review_requested 
https://github.com/llvm/llvm-project/pull/66207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. (PR #66207)

[Balázs Kéri via cfe-commits]

https://github.com/balazske review_requested 
https://github.com/llvm/llvm-project/pull/66207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Fix StdLibraryFunctionsChecker crash on surprising sink node (PR #66109)

[Balázs Kéri via cfe-commits]

@@ -1427,8 +1427,13 @@ void StdLibraryFunctionsChecker::checkPostCall(const 
CallEvent &Call,
 });
 Pred = C.addTransition(NewState, Pred, Tag);
   }
-  if (!Pred)
+  if (!Pred || Pred->isSink()) {
+// Pred may be:
+//  - a nullpointer, when the generated node is not new;
+//  - a sink, when NewState is posteriorly overconstrained.
+// In these situations we cannot add the errno note tag.

balazske wrote:

I like more if the comment is added before the `if` and the braces are not 
added.

https://github.com/llvm/llvm-project/pull/66109
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 52ac71f - [clang][analyzer] Improve StdCLibraryFunctions socket send/recv functions.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2023-08-07T10:45:09+02:00
New Revision: 52ac71f92d38f75df5cb88e9c090ac5fd5a71548

URL: 
https://github.com/llvm/llvm-project/commit/52ac71f92d38f75df5cb88e9c090ac5fd5a71548
DIFF: 
https://github.com/llvm/llvm-project/commit/52ac71f92d38f75df5cb88e9c090ac5fd5a71548.diff

LOG: [clang][analyzer] Improve StdCLibraryFunctions socket send/recv functions.

The modeling of send, recv, sendmsg, recvmsg, sendto, recvfrom is changed:
These functions do not return 0, except if the message length is 0.
(In sendmsg, recvmsg the length is not checkable but it is more likely
that a message with 0 length is invalid for these functions.)

Reviewed By: donat.nagy

Differential Revision: https://reviews.llvm.org/D155715

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-POSIX.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index d18e6f63df4484..f5f6e3a9123768 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -3096,7 +3096,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 auto Recvfrom =
 Summary(NoEvalCall)
 .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+   ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
+  ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case({ReturnValueCondition(WithinRange, SingleValue(0)),
+   ArgumentCondition(2, WithinRange, SingleValue(0))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
@@ -3123,7 +3126,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 auto Sendto =
 Summary(NoEvalCall)
 .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+   ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
+  ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case({ReturnValueCondition(WithinRange, SingleValue(0)),
+   ArgumentCondition(2, WithinRange, SingleValue(0))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
@@ -3161,7 +3167,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   RetType{Ssize_tTy}),
 Summary(NoEvalCall)
 .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+   ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
+  ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case({ReturnValueCondition(WithinRange, SingleValue(0)),
+   ArgumentCondition(2, WithinRange, SingleValue(0))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
@@ -3179,7 +3188,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Signature(ArgTypes{IntTy, StructMsghdrPtrTy, IntTy},
   RetType{Ssize_tTy}),
 Summary(NoEvalCall)
-.Case({ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+.Case({ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(
@@ -3191,7 +3200,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Signature(ArgTypes{IntTy, ConstStructMsghdrPtrTy, IntTy},
   RetType{Ssize_tTy}),
 Summary(NoEvalCall)
-.Case({ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
+.Case({ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))},
   ErrnoMustNotBeChecked, GenericSuccessMsg)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(
@@ -3233,7 +3242,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   RetType{Ssize_tTy}),
 Summary(NoEvalCall)
 .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange

[clang] f443838 - [clang][ASTImporter] Fix import of recursive field initializer.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2023-07-27T09:34:34+02:00
New Revision: f4438385d4d9b7e652b41f908250e55f75695ab6

URL: 
https://github.com/llvm/llvm-project/commit/f4438385d4d9b7e652b41f908250e55f75695ab6
DIFF: 
https://github.com/llvm/llvm-project/commit/f4438385d4d9b7e652b41f908250e55f75695ab6.diff

LOG: [clang][ASTImporter] Fix import of recursive field initializer.

Import of field initializers with circular reference was not working,
this is fixed now.

Fixes issue #63120

Reviewed By: steakhal

Differential Revision: https://reviews.llvm.org/D155574

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 39c7a8fa397048..eb8e28ccd3e622 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3925,7 +3925,6 @@ ExpectedDecl ASTNodeImporter::VisitFieldDecl(FieldDecl 
*D) {
   auto ToTInfo = importChecked(Err, D->getTypeSourceInfo());
   auto ToBitWidth = importChecked(Err, D->getBitWidth());
   auto ToInnerLocStart = importChecked(Err, D->getInnerLocStart());
-  auto ToInitializer = importChecked(Err, D->getInClassInitializer());
   if (Err)
 return std::move(Err);
   const Type *ToCapturedVLAType = nullptr;
@@ -3948,12 +3947,24 @@ ExpectedDecl ASTNodeImporter::VisitFieldDecl(FieldDecl 
*D) {
 return std::move(Err);
   ToField->setAccess(D->getAccess());
   ToField->setLexicalDeclContext(LexicalDC);
-  if (ToInitializer)
-ToField->setInClassInitializer(ToInitializer);
   ToField->setImplicit(D->isImplicit());
   if (ToCapturedVLAType)
 ToField->setCapturedVLAType(cast(ToCapturedVLAType));
   LexicalDC->addDeclInternal(ToField);
+  // Import initializer only after the field was created, it may have recursive
+  // reference to the field.
+  auto ToInitializer = importChecked(Err, D->getInClassInitializer());
+  if (Err)
+return std::move(Err);
+  if (ToInitializer) {
+auto *AlreadyImported = ToField->getInClassInitializer();
+if (AlreadyImported)
+  assert(ToInitializer == AlreadyImported &&
+ "Duplicate import of in-class initializer.");
+else
+  ToField->setInClassInitializer(ToInitializer);
+  }
+
   return ToField;
 }
 

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 3a1058f5e3fe90..c6ae2693930409 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -8165,6 +8165,83 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   EXPECT_TRUE(ToX->getInClassInitializer());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportRecursiveFieldInitializer) {
+  const char *Code =
+  R"(
+  struct AP_TECS;
+
+  struct AP_Landing {
+AP_TECS *TECS_controller;
+  };
+
+  struct AP_TECS {
+AP_Landing landing;
+  };
+
+  class Plane {
+AP_TECS TECS_controller{landing};
+AP_Landing landing{&TECS_controller};
+  };
+  )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromR = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("Plane")));
+  for (FieldDecl *F : FromR->fields())
+EXPECT_TRUE(F->getInClassInitializer());
+  auto *ToR = Import(FromR, Lang_CXX11);
+  for (FieldDecl *F : ToR->fields())
+EXPECT_TRUE(F->getInClassInitializer());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportFieldInitializerWithItself) {
+  const char *Code =
+  R"(
+  class A {
+int a{a};
+  };
+  )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+  auto *FromA = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("A")));
+  EXPECT_TRUE(FromA->field_begin()->getInClassInitializer());
+  auto *ToA = Import(FromA, Lang_CXX11);
+  EXPECT_TRUE(ToA->field_begin()->getInClassInitializer());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportRecursiveFieldInitializer1) {
+  // FIXME: This is a example of recursive field initialization that is not
+  // supported.
+  // The following import chain occurs (not complete):
+  // import of A => A.a => in-class initializer of A.a => ref_B() => B => B.b
+  // => in-class initializer of B.b => ref_A() => CXXConstructExpr for A =>
+  // CXXDefaultInitExpr for A.a => in-class initializer of A.a
+  // in-class initializer of A.a is created in two 
diff erent instances in this
+  // case (import of FieldDecl and CXXDefaultInitExpr). Probably not a big
+  // problem because it is an Expr (the second construction can be ignored
+  // instead of assert). But such recursive init code should not occur in
+  // practice.
+  const char *Code =
+  R"(
+  static int ref_A();
+  static int ref_B();
+  struct A {
+int a = ref_B();
+  };
+  struct B {
+int b = ref_A();
+  };
+  int ref_B() { B b; return b.b; }
+  int ref_A() { A a; return a.a;

[clang] e271049 - [clang][analyzer] StdLibraryFunctionsChecker: Allow NULL buffer in `fread` and `fwrite` if size is zero.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2023-07-19T09:58:14+02:00
New Revision: e271049bc6a1408aa4e53771321117b3da6440ab

URL: 
https://github.com/llvm/llvm-project/commit/e271049bc6a1408aa4e53771321117b3da6440ab
DIFF: 
https://github.com/llvm/llvm-project/commit/e271049bc6a1408aa4e53771321117b3da6440ab.diff

LOG: [clang][analyzer] StdLibraryFunctionsChecker: Allow NULL buffer in `fread` 
and `fwrite` if size is zero.

Reviewed By: donat.nagy

Differential Revision: https://reviews.llvm.org/D154509

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 683b0369504dc8..d18e6f63df4484 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -403,6 +403,53 @@ class StdLibraryFunctionsChecker
 }
   };
 
+  /// Check null or non-null-ness of an argument that is of pointer type.
+  /// The argument is meant to be a buffer that has a size constraint, and it
+  /// is allowed to have a NULL value if the size is 0. The size can depend on
+  /// 1 or 2 additional arguments, if one of these is 0 the buffer is allowed 
to
+  /// be NULL. This is useful for functions like `fread` which have this 
special
+  /// property.
+  class NotNullBufferConstraint : public ValueConstraint {
+using ValueConstraint::ValueConstraint;
+ArgNo SizeArg1N;
+std::optional SizeArg2N;
+// This variable has a role when we negate the constraint.
+bool CannotBeNull = true;
+
+  public:
+NotNullBufferConstraint(ArgNo ArgN, ArgNo SizeArg1N,
+std::optional SizeArg2N,
+bool CannotBeNull = true)
+: ValueConstraint(ArgN), SizeArg1N(SizeArg1N), SizeArg2N(SizeArg2N),
+  CannotBeNull(CannotBeNull) {}
+
+ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+  const Summary &Summary,
+  CheckerContext &C) const override;
+
+void describe(DescriptionKind DK, const CallEvent &Call,
+  ProgramStateRef State, const Summary &Summary,
+  llvm::raw_ostream &Out) const override;
+
+bool describeArgumentValue(const CallEvent &Call, ProgramStateRef State,
+   const Summary &Summary,
+   llvm::raw_ostream &Out) const override;
+
+ValueConstraintPtr negate() const override {
+  NotNullBufferConstraint Tmp(*this);
+  Tmp.CannotBeNull = !this->CannotBeNull;
+  return std::make_shared(Tmp);
+}
+
+  protected:
+bool checkSpecificValidity(const FunctionDecl *FD) const override {
+  const bool ValidArg = getArgType(FD, ArgN)->isPointerType();
+  assert(ValidArg &&
+ "This constraint should be applied only on a pointer type");
+  return ValidArg;
+}
+  };
+
   // Represents a buffer argument with an additional size constraint. The
   // constraint may be a concrete value, or a symbolic value in an argument.
   // Example 1. Concrete value as the minimum buffer size.
@@ -1140,6 +1187,54 @@ bool 
StdLibraryFunctionsChecker::NotNullConstraint::describeArgumentValue(
   return true;
 }
 
+ProgramStateRef StdLibraryFunctionsChecker::NotNullBufferConstraint::apply(
+ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+CheckerContext &C) const {
+  SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+  DefinedOrUnknownSVal L = V.castAs();
+  if (!isa(L))
+return State;
+
+  std::optional SizeArg1 =
+  getArgSVal(Call, SizeArg1N).getAs();
+  std::optional SizeArg2;
+  if (SizeArg2N)
+SizeArg2 = getArgSVal(Call, *SizeArg2N).getAs();
+
+  auto IsArgZero = [State](std::optional Val) {
+if (!Val)
+  return false;
+auto [IsNonNull, IsNull] = State->assume(*Val);
+return IsNull && !IsNonNull;
+  };
+
+  if (IsArgZero(SizeArg1) || IsArgZero(SizeArg2))
+return State;
+
+  return State->assume(L, CannotBeNull);
+}
+
+void StdLibraryFunctionsChecker::NotNullBufferConstraint::describe(
+DescriptionKind DK, const CallEvent &Call, ProgramStateRef State,
+const Summary &Summary, llvm::raw_ostream &Out) const {
+  assert(CannotBeNull &&
+ "Describe should not be used when the value must be NULL");
+  if (DK == Violation)
+Out << "should not be NULL";
+  else
+Out << "is not NULL";
+}
+
+bool 
StdLibraryFunctionsChecker::NotNullBufferConstraint::describeArgumentValue(
+const CallEvent &Call, ProgramStateRef State, const Summary &Summary,
+llvm::raw_ostream &Out) const {
+  assert(!CannotBeNull && "This function is used when the value is NULL"

[clang] 6dccf5b - [clang][analyzer] Add all success/failure messages to StdLibraryFunctionsChecker.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2023-07-18T09:29:15+02:00
New Revision: 6dccf5b8d550911f06e492a3a75c640c05efdab3

URL: 
https://github.com/llvm/llvm-project/commit/6dccf5b8d550911f06e492a3a75c640c05efdab3
DIFF: 
https://github.com/llvm/llvm-project/commit/6dccf5b8d550911f06e492a3a75c640c05efdab3.diff

LOG: [clang][analyzer] Add all success/failure messages to 
StdLibraryFunctionsChecker.

Success or failure messages are now shown at all checked functions, if the call
(return value) is interesting.
Additionally new functions are added: open, openat, socket, shutdown

Reviewed By: donat.nagy

Differential Revision: https://reviews.llvm.org/D154423

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
clang/test/Analysis/std-c-library-functions-POSIX.c
clang/test/Analysis/std-c-library-functions-arg-constraints.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 1fb248b8ed0e97..683b0369504dc8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -2181,6 +2181,26 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, LongMax;
 
+// int open(const char *path, int oflag, ...);
+addToFunctionSummaryMap(
+"open", Signature(ArgTypes{ConstCharPtrTy, IntTy}, RetType{IntTy}),
+Summary(NoEvalCall)
+.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked,
+  GenericSuccessMsg)
+.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
+.ArgConstraint(NotNull(ArgNo(0;
+
+// int openat(int fd, const char *path, int oflag, ...);
+addToFunctionSummaryMap(
+"openat",
+Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy}, RetType{IntTy}),
+Summary(NoEvalCall)
+.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked,
+  GenericSuccessMsg)
+.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
+.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
+.ArgConstraint(NotNull(ArgNo(1;
+
 // int access(const char *pathname, int amode);
 addToFunctionSummaryMap(
 "access", Signature(ArgTypes{ConstCharPtrTy, IntTy}, RetType{IntTy}),
@@ -2195,8 +2215,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy, IntTy},
   RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsZero, ErrnoMustNotBeChecked)
-.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+.Case(ReturnsZero, ErrnoMustNotBeChecked, GenericSuccessMsg)
+.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
 .ArgConstraint(NotNull(ArgNo(1;
 
@@ -2204,8 +2224,9 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 addToFunctionSummaryMap(
 "dup", Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
-.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked,
+  GenericSuccessMsg)
+.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(
 ArgumentCondition(0, WithinRange, Range(0, IntMax;
 
@@ -2213,20 +2234,21 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 addToFunctionSummaryMap(
 "dup2", Signature(ArgTypes{IntTy, IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
-.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked,
+  GenericSuccessMsg)
+.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
 .ArgConstraint(
 ArgumentCondition(1, WithinRange, Range(0, IntMax;
 
 // int fdatasync(int fildes);
-addToFunctionSummaryMap("fdatasync",
-Signature(ArgTypes{IntTy}, RetType{IntTy}),
-Summary(NoEvalCall)
-.Case(ReturnsZero, ErrnoMustNotBeChecked)
-.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
-.ArgCo