Re: r336776 - [AST] Structural equivalence of methods

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

Hi,
I am aware of the problem, there should be already a commit to (hopefully)
correct the failure.

Balázs Kéri

Galina Kistanova  ezt írta (időpont: 2018. júl. 11.,
Sze, 20:01):

> Hello Balazs,
>
> This commit broke at least one of our builders:
>
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/10897
>
> . . .
> Failing Tests (1):
> Clang-Unit :: AST/./ASTTests.exe/StructuralEquivalenceRecordTest.Name
>
> Please have a look?
> It is not good idea to keep the bot red for too long. This hides new
> problem which later hard to track down.
>
> Thanks
>
> Galina
>
>
> On Wed, Jul 11, 2018 at 2:37 AM, Balazs Keri via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: balazske
>> Date: Wed Jul 11 02:37:24 2018
>> New Revision: 336776
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=336776&view=rev
>> Log:
>> [AST] Structural equivalence of methods
>>
>> Summary:
>> Added structural equivalence check for C++ methods.
>> Improved structural equivalence tests.
>> Added related ASTImporter tests.
>>
>> Reviewers: a.sidorin, szepet, xazax.hun, martong, a_sidorin
>>
>> Reviewed By: martong, a_sidorin
>>
>> Subscribers: a_sidorin, rnkovacs, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D48628
>>
>> Modified:
>> cfe/trunk/lib/AST/ASTImporter.cpp
>> cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
>> cfe/trunk/unittests/AST/ASTImporterTest.cpp
>> cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
>>
>> Modified: cfe/trunk/lib/AST/ASTImporter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=336776&r1=336775&r2=336776&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ASTImporter.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Jul 11 02:37:24 2018
>> @@ -230,6 +230,7 @@ namespace clang {
>>  bool IsStructuralMatch(EnumConstantDecl *FromEC, EnumConstantDecl
>> *ToEC);
>>  bool IsStructuralMatch(FunctionTemplateDecl *From,
>> FunctionTemplateDecl *To);
>> +bool IsStructuralMatch(FunctionDecl *From, FunctionDecl *To);
>>  bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl
>> *To);
>>  bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
>>  Decl *VisitDecl(Decl *D);
>> @@ -1525,6 +1526,13 @@ bool ASTNodeImporter::IsStructuralMatch(
>>return Ctx.IsStructurallyEquivalent(From, To);
>>  }
>>
>> +bool ASTNodeImporter::IsStructuralMatch(FunctionDecl *From, FunctionDecl
>> *To) {
>> +  StructuralEquivalenceContext Ctx(
>> +  Importer.getFromContext(), Importer.getToContext(),
>> +  Importer.getNonEquivalentDecls(), false, false);
>> +  return Ctx.IsStructurallyEquivalent(From, To);
>> +}
>> +
>>  bool ASTNodeImporter::IsStructuralMatch(EnumConstantDecl *FromEC,
>>  EnumConstantDecl *ToEC) {
>>const llvm::APSInt &FromVal = FromEC->getInitVal();
>> @@ -2433,13 +2441,15 @@ Decl *ASTNodeImporter::VisitFunctionDecl
>>if (auto *FoundFunction = dyn_cast(FoundDecl)) {
>>  if (FoundFunction->hasExternalFormalLinkage() &&
>>  D->hasExternalFormalLinkage()) {
>> -  if (Importer.IsStructurallyEquivalent(D->getType(),
>> -
>> FoundFunction->getType())) {
>> -  if (D->doesThisDeclarationHaveABody() &&
>> -  FoundFunction->hasBody())
>> -return Importer.Imported(D, FoundFunction);
>> -  FoundByLookup = FoundFunction;
>> -  break;
>> +  if (IsStructuralMatch(D, FoundFunction)) {
>> +const FunctionDecl *Definition = nullptr;
>> +if (D->doesThisDeclarationHaveABody() &&
>> +FoundFunction->hasBody(Definition)) {
>> +  return Importer.Imported(
>> +  D, const_cast(Definition));
>> +}
>> +FoundByLookup = FoundFunction;
>> +break;
>>}
>>
>>// FIXME: Check for overloading more carefully, e.g., by
>> boosting
>>
>> Modified: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp?rev=336776&r1=336775&r2=336776&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp Wed Jul 11 02:37:24
>> 2018
>> @@ -250,6 +250,9 @@ static bool IsStructurallyEquivalent(Str
>>if (T1.isNull() || T2.isNull())
>>  return T1.isNull() && T2.isNull();
>>
>> +  QualType OrigT1 = T1;
>> +  QualType OrigT2 = T2;
>> +
>>if (!Context.StrictTypeSpelling) {
>>  // We aren't being strict about token-to-token equivalence of types,
>>  // so map down to the canonical type.
>> @@ -422,6 +425,7 @@ static bool IsStructurallyEquivalent(Str
>>case Type:

[clang] f7c9f77 - [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

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

Author: Balázs Kéri
Date: 2020-05-20T09:40:57+02:00
New Revision: f7c9f77ef3721c956499b0ccf5e585de105aae4e

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

LOG: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Summary:
Stream functions `fread` and `fwrite` are evaluated
and preconditions checked.
A new bug type is added for a (non fatal) warning if `fread`
is called in EOF state.

Reviewers: Szelethus, NoQ, dcoughlin, baloghadamsoftware, martong, xazax.hun

Reviewed By: Szelethus

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index c94cae045239..2e90be4350a0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -51,6 +51,8 @@ struct StreamErrorState {
 return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError;
   }
 
+  bool operator!=(const StreamErrorState &ES) const { return !(*this == ES); }
+
   StreamErrorState operator|(const StreamErrorState &E) const {
 return {NoError || E.NoError, FEof || E.FEof, FError || E.FError};
   }
@@ -171,7 +173,7 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef 
State,
 class StreamChecker
 : public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
-  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak;
+  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof;
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -189,8 +191,12 @@ class StreamChecker
   {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
   {{"fclose", 1},
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
-  {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
-  {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+  {{"fread", 4},
+   {&StreamChecker::preFread,
+std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
+  {{"fwrite", 4},
+   {&StreamChecker::preFwrite,
+std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
   {{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
   {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
@@ -232,6 +238,15 @@ class StreamChecker
   void evalFclose(const FnDescription *Desc, const CallEvent &Call,
   CheckerContext &C) const;
 
+  void preFread(const FnDescription *Desc, const CallEvent &Call,
+CheckerContext &C) const;
+
+  void preFwrite(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+  void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
+   CheckerContext &C, bool IsFread) const;
+
   void preFseek(const FnDescription *Desc, const CallEvent &Call,
 CheckerContext &C) const;
   void evalFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -271,6 +286,12 @@ class StreamChecker
   ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
ProgramStateRef State) const;
 
+  /// Generate warning about stream in EOF state.
+  /// There will be always a state transition into the passed State,
+  /// by the new non-fatal error node or (if failed) a normal transition,
+  /// to ensure uniform handling.
+  void reportFEofWarning(CheckerContext &C, ProgramStateRef State) const;
+
   /// Find the description data of the function called by a call event.
   /// Returns nullptr if no function is recognized.
   const FnDescription *lookupFn(const CallEvent &Call) const {
@@ -418,6 +439,120 @@ void StreamChecker::evalFclose(const FnDescription *Desc, 
const CallEvent &Call,
   C.addTransition(State);
 }
 
+void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SVal StreamVal = getStreamArg(Desc, Call);
+  State = ensureStreamNonNull(StreamVal, C, State);
+  if (!State)
+return;
+  State = ensureStreamOpened(StreamVal, C, State);
+  if (!State)
+return;
+
+  SymbolRef Sym = StreamVal.getAsSymbol();
+  if (Sym && State->get(Sym)) {
+const StreamState *SS

[clang] 9081fa2 - [Analyzer][StreamChecker] Added check for "indeterminate file position".

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

Author: Balázs Kéri
Date: 2020-05-28T08:21:57+02:00
New Revision: 9081fa20991d101728434b354a96283b26495b71

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

LOG: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Summary:
According to the standard, after a `wread` or `fwrite` call the file position
becomes "indeterminate". It is assumable that a next read or write causes
undefined behavior, so a (fatal error) warning is added for this case.
The indeterminate position can be cleared by some operations, for example
`fseek` or `freopen`, not with `clearerr`.

Reviewers: Szelethus, baloghadamsoftware, martong, NoQ, xazax.hun, dcoughlin

Reviewed By: Szelethus

Subscribers: rnkovacs, NoQ, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2e90be4350a0..63ebfaf90dc8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -92,7 +92,27 @@ struct StreamState {
 
   /// State of the error flags.
   /// Ignored in non-opened stream state but must be NoError.
-  StreamErrorState ErrorState;
+  StreamErrorState const ErrorState;
+
+  /// Indicate if the file has an "indeterminate file position indicator".
+  /// This can be set at a failing read or write or seek operation.
+  /// If it is set no more read or write is allowed.
+  /// This value is not dependent on the stream error flags:
+  /// The error flag may be cleared with `clearerr` but the file position
+  /// remains still indeterminate.
+  /// This value applies to all error states in ErrorState except FEOF.
+  /// An EOF+indeterminate state is the same as EOF state.
+  bool const FilePositionIndeterminate = false;
+
+  StreamState(const FnDescription *L, KindTy S, const StreamErrorState &ES,
+  bool IsFilePositionIndeterminate)
+  : LastOperation(L), State(S), ErrorState(ES),
+FilePositionIndeterminate(IsFilePositionIndeterminate) {
+assert((!ES.isFEof() || !IsFilePositionIndeterminate) &&
+   "FilePositionIndeterminate should be false in FEof case.");
+assert((State == Opened || ErrorState.isNoError()) &&
+   "ErrorState should be None in non-opened stream state.");
+  }
 
   bool isOpened() const { return State == Opened; }
   bool isClosed() const { return State == Closed; }
@@ -102,24 +122,27 @@ struct StreamState {
 // In not opened state error state should always NoError, so comparison
 // here is no problem.
 return LastOperation == X.LastOperation && State == X.State &&
-   ErrorState == X.ErrorState;
+   ErrorState == X.ErrorState &&
+   FilePositionIndeterminate == X.FilePositionIndeterminate;
   }
 
   static StreamState getOpened(const FnDescription *L,
-   const StreamErrorState &ES = {}) {
-return StreamState{L, Opened, ES};
+   const StreamErrorState &ES = ErrorNone,
+   bool IsFilePositionIndeterminate = false) {
+return StreamState{L, Opened, ES, IsFilePositionIndeterminate};
   }
   static StreamState getClosed(const FnDescription *L) {
-return StreamState{L, Closed, {}};
+return StreamState{L, Closed, {}, false};
   }
   static StreamState getOpenFailed(const FnDescription *L) {
-return StreamState{L, OpenFailed, {}};
+return StreamState{L, OpenFailed, {}, false};
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
 ID.AddPointer(LastOperation);
 ID.AddInteger(State);
 ID.AddInteger(ErrorState);
+ID.AddBoolean(FilePositionIndeterminate);
   }
 };
 
@@ -173,7 +196,8 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef 
State,
 class StreamChecker
 : public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
-  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof;
+  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof,
+  BT_IndeterminatePosition;
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -279,6 +303,16 @@ class StreamChecker
   ProgramStateRef ensureStreamOpened(SVal StreamVal, CheckerContext &C,
  ProgramStateRef State) const;
 
+  /// Check that the stream has not an invalid ("indeterminate") file position,
+  /// generate warning for it.

[Differential] D82845: [Analyzer][StreamChecker] Report every leak, clean up state.

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9b7c43d341da: [Analyzer][StreamChecker] Report every leak, 
clean up state. (authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D82845?vs=278755&id=278998#toc

Repository:
  rG LLVM Github Monorepo

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


  https://reviews.llvm.org/D82845

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -46,3 +46,34 @@
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
 // expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_leak_2(int c) {
+  FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
+  if (!F1)
+// expected-note@-1 {{'F1' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+// expected-note@-3 {{'F1' is non-null}}
+// expected-note@-4 {{Taking false branch}}
+return;
+  FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
+  if (!F2) {
+// expected-note@-1 {{'F2' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+// expected-note@-3 {{'F2' is non-null}}
+// expected-note@-4 {{Taking false branch}}
+fclose(F1);
+return;
+  }
+  if (c)
+// expected-note@-1 {{Assuming 'c' is not equal to 0}}
+// expected-note@-2 {{Taking true branch}}
+// expected-note@-3 {{Assuming 'c' is not equal to 0}}
+// expected-note@-4 {{Taking true branch}}
+return;
+  // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+  // expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+  // expected-warning@-3 {{Opened stream never closed. Potential resource leak}}
+  // expected-note@-4 {{Opened stream never closed. Potential resource leak}}
+  fclose(F1);
+  fclose(F2);
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -337,6 +337,12 @@
   /// to ensure uniform handling.
   void reportFEofWarning(CheckerContext &C, ProgramStateRef State) const;
 
+  /// Emit resource leak warnings for the given symbols.
+  /// Createn a non-fatal error node for these, and returns it (if any warnings
+  /// were generated). Return value is non-null.
+  ExplodedNode *reportLeaks(const SmallVector &LeakedSyms,
+CheckerContext &C, ExplodedNode *Pred) const;
+
   /// Find the description data of the function called by a call event.
   /// Returns nullptr if no function is recognized.
   const FnDescription *lookupFn(const CallEvent &Call) const {
@@ -956,28 +962,19 @@
   C.addTransition(State);
 }
 
-void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
- CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
-
-  // TODO: Clean up the state.
-  const StreamMapTy &Map = State->get();
-  for (const auto &I : Map) {
-SymbolRef Sym = I.first;
-const StreamState &SS = I.second;
-if (!SymReaper.isDead(Sym) || !SS.isOpened())
-  continue;
-
-ExplodedNode *N = C.generateErrorNode();
-if (!N)
-  continue;
+ExplodedNode *
+StreamChecker::reportLeaks(const SmallVector &LeakedSyms,
+   CheckerContext &C, ExplodedNode *Pred) const {
+  // Do not warn for non-closed stream at program exit.
+  // FIXME: Use BugType::SuppressOnSink instead.
+  if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement())
+return Pred;
 
-// Do not warn for non-closed stream at program exit.
-ExplodedNode *Pred = C.getPredecessor();
-if (Pred && Pred->getCFGBlock() &&
-Pred->getCFGBlock()->hasNoReturnElement())
-  continue;
+  ExplodedNode *Err = C.generateNonFatalErrorNode(C.getState(), Pred);
+  if (!Err)
+return Pred;
 
+  for (SymbolRef LeakSym : LeakedSyms) {
 // Resource leaks can result in multiple warning that describe the same kind
 // of programming error:
 //  void f() {
@@ -989,8 +986,7 @@
 // from a different kinds of errors), the reduction in redundant reports
 // makes this a worthwhile heuristic.
 // FIXME: Add a checker option to turn this uniqueing feature off.
-
-const ExplodedNode *StreamOpenNode = getAcquisitionSite(N, Sym, C);
+const ExplodedNode *StreamOpenNode = getAcquisitionSite(Err, LeakSym, C);
 assert(StreamOpenNode && "Could not find place of stream opening.");
 PathDiagnosticLocation LocUsedForUniqueing =
 PathDiagnosticLocation::createBegin(
@@ -10

[clang] 9b7c43d - [Analyzer][StreamChecker] Report every leak, clean up state.

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

Author: Balázs Kéri
Date: 2020-07-20T11:49:00+02:00
New Revision: 9b7c43d341da319c69b11205ee1deb642f286e59

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

LOG: [Analyzer][StreamChecker] Report every leak, clean up state.

Summary:
Report resource leaks with non-fatal error.
Report every resource leak.
Stream state is cleaned up at `checkDeadSymbols`.

Reviewers: Szelethus, baloghadamsoftware, NoQ

Reviewed By: Szelethus

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-note.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index f6abbe4f8f03..1e963249156f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -337,6 +337,12 @@ class StreamChecker : public Checker &LeakedSyms,
+CheckerContext &C, ExplodedNode *Pred) const;
+
   /// Find the description data of the function called by a call event.
   /// Returns nullptr if no function is recognized.
   const FnDescription *lookupFn(const CallEvent &Call) const {
@@ -956,28 +962,19 @@ void StreamChecker::reportFEofWarning(CheckerContext &C,
   C.addTransition(State);
 }
 
-void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
- CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
-
-  // TODO: Clean up the state.
-  const StreamMapTy &Map = State->get();
-  for (const auto &I : Map) {
-SymbolRef Sym = I.first;
-const StreamState &SS = I.second;
-if (!SymReaper.isDead(Sym) || !SS.isOpened())
-  continue;
-
-ExplodedNode *N = C.generateErrorNode();
-if (!N)
-  continue;
+ExplodedNode *
+StreamChecker::reportLeaks(const SmallVector &LeakedSyms,
+   CheckerContext &C, ExplodedNode *Pred) const {
+  // Do not warn for non-closed stream at program exit.
+  // FIXME: Use BugType::SuppressOnSink instead.
+  if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement())
+return Pred;
 
-// Do not warn for non-closed stream at program exit.
-ExplodedNode *Pred = C.getPredecessor();
-if (Pred && Pred->getCFGBlock() &&
-Pred->getCFGBlock()->hasNoReturnElement())
-  continue;
+  ExplodedNode *Err = C.generateNonFatalErrorNode(C.getState(), Pred);
+  if (!Err)
+return Pred;
 
+  for (SymbolRef LeakSym : LeakedSyms) {
 // Resource leaks can result in multiple warning that describe the same 
kind
 // of programming error:
 //  void f() {
@@ -989,8 +986,7 @@ void StreamChecker::checkDeadSymbols(SymbolReaper 
&SymReaper,
 // from a 
diff erent kinds of errors), the reduction in redundant reports
 // makes this a worthwhile heuristic.
 // FIXME: Add a checker option to turn this uniqueing feature off.
-
-const ExplodedNode *StreamOpenNode = getAcquisitionSite(N, Sym, C);
+const ExplodedNode *StreamOpenNode = getAcquisitionSite(Err, LeakSym, C);
 assert(StreamOpenNode && "Could not find place of stream opening.");
 PathDiagnosticLocation LocUsedForUniqueing =
 PathDiagnosticLocation::createBegin(
@@ -1000,12 +996,38 @@ void StreamChecker::checkDeadSymbols(SymbolReaper 
&SymReaper,
 std::unique_ptr R =
 std::make_unique(
 BT_ResourceLeak,
-"Opened stream never closed. Potential resource leak.", N,
+"Opened stream never closed. Potential resource leak.", Err,
 LocUsedForUniqueing,
 StreamOpenNode->getLocationContext()->getDecl());
-R->markInteresting(Sym);
+R->markInteresting(LeakSym);
 C.emitReport(std::move(R));
   }
+
+  return Err;
+}
+
+void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
+ CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+
+  llvm::SmallVector LeakedSyms;
+
+  const StreamMapTy &Map = State->get();
+  for (const auto &I : Map) {
+SymbolRef Sym = I.first;
+const StreamState &SS = I.second;
+if (!SymReaper.isDead(Sym))
+  continue;
+if (SS.isOpened())
+  LeakedSyms.push_back(Sym);
+State = State->remove(Sym);
+  }
+
+  ExplodedNode *N = C.getPredecessor();
+  if (!LeakedSyms.empty())
+N = reportLeaks(LeakedSyms, C, N);
+
+  C.addTransition(State, N);
 }
 
 ProgramStateRef StreamChecker::checkPointerEscape(

diff  --git a/clang/test/Analysis/stream-note.c 
b/clang/test

[clang] 65fd651 - [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak report.

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

Author: Balázs Kéri
Date: 2020-07-23T11:53:25+02:00
New Revision: 65fd651980a8ad965363807cc334c513e4c8ffe4

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

LOG: [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak 
report.

Summary:
Use the built-in functionality BugType::SuppressOnSink
instead of a manual solution in StreamChecker.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 1e963249156f..6b176b3c4e2b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -204,7 +204,8 @@ class StreamChecker : public Checker &LeakedSyms,
CheckerContext &C, ExplodedNode *Pred) const {
-  // Do not warn for non-closed stream at program exit.
-  // FIXME: Use BugType::SuppressOnSink instead.
-  if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement())
-return Pred;
-
   ExplodedNode *Err = C.generateNonFatalErrorNode(C.getState(), Pred);
   if (!Err)
 return Pred;

diff  --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index cd5d28ae8455..c57f3159fcc7 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -261,7 +261,9 @@ void check_leak_noreturn_2() {
   if (!F1)
 return;
   if (Test == 1) {
-return; // expected-warning {{Opened stream never closed. Potential 
resource leak}}
+return; // no warning
   }
   rewind(F1);
-} // no warning
+} // expected-warning {{Opened stream never closed. Potential resource leak}}
+// FIXME: This warning should be placed at the `return` above.
+// See https://reviews.llvm.org/D83120 about details.



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

[clang] b22b97b - [Analyzer] Use of BugType in DereferenceChecker (NFC).

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

Author: Balázs Kéri
Date: 2020-07-30T08:33:12+02:00
New Revision: b22b97b3d0c08485d478073d6a2a6e769af7a2af

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

LOG: [Analyzer] Use of BugType in DereferenceChecker (NFC).

Use of BuiltinBug is replaced by BugType.
Class BuiltinBug seems to have no benefits and is confusing.

Reviewed By: Szelethus, martong, NoQ, vsavchenko

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index 2411f0e2d058..9a87729de8fd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@ class DereferenceChecker
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::LogicError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C) 
const;
 
@@ -123,11 +124,6 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@ void DereferenceChecker::checkLocation(SVal l, bool 
isLoad, const Stmt* S,
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), 
*report);
   C.emitReport(std::move(report));
 }
@@ -219,9 +211,10 @@ void DereferenceChecker::checkLocation(SVal l, bool 
isLoad, const Stmt* S,
   ProgramStateRef notNullState, nullState;
   std::tie(notNullState, nullState) = state->assume(location);
 
-  // The explicit NULL case.
   if (nullState) {
 if (!notNullState) {
+  // We know that 'location' can only be null.  This is what
+  // we call an "explicit" null dereference.
   const Expr *expr = getDereferenceExpr(S);
   if (!suppressReport(expr)) {
 reportBug(nullState, expr, C);



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

[clang] 1745ba4 - [Analyzer] Remove inclusion of uniqueing decl from diagnostic profile.

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

Author: Balázs Kéri
Date: 2020-07-30T09:52:28+02:00
New Revision: 1745ba41b196d80d8a6739dffcbb6f613d371f29

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

LOG: [Analyzer] Remove inclusion of uniqueing decl from diagnostic profile.

The uniqueing decl in PathDiagnostic is the declaration with the
uniqueing loc, as stated by documentation comments.
It is enough to include the uniqueing loc in the profile. It is possible
to have objects with different uniqueing decl but same location, at
least with templates. These belong to the same class and should have
same profile.

Reviewed By: vsavchenko, NoQ

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

Added: 
clang/test/Analysis/report-uniqueing.cpp

Modified: 
clang/lib/Analysis/PathDiagnostic.cpp

Removed: 




diff  --git a/clang/lib/Analysis/PathDiagnostic.cpp 
b/clang/lib/Analysis/PathDiagnostic.cpp
index 9aa3386129d7..f80b99b99806 100644
--- a/clang/lib/Analysis/PathDiagnostic.cpp
+++ b/clang/lib/Analysis/PathDiagnostic.cpp
@@ -1134,7 +1134,6 @@ void 
PathDiagnosticPopUpPiece::Profile(llvm::FoldingSetNodeID &ID) const {
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
   ID.Add(getLocation());
   ID.Add(getUniqueingLoc());
-  ID.AddPointer(getUniqueingLoc().isValid() ? getUniqueingDecl() : nullptr);
   ID.AddString(BugType);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);

diff  --git a/clang/test/Analysis/report-uniqueing.cpp 
b/clang/test/Analysis/report-uniqueing.cpp
new file mode 100644
index ..0e4d50e13a20
--- /dev/null
+++ b/clang/test/Analysis/report-uniqueing.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=security
+
+void bzero(void *, unsigned long);
+
+template  void foo(T l) {
+  // The warning comes from multiple instances and with
+  // 
diff erent declarations that have same source location.
+  // One instance should be shown.
+  bzero(l, 1); // expected-warning{{The bzero() function is obsoleted}}
+}
+
+void p(int *p, unsigned *q) {
+  foo(p);
+  foo(q);
+}



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

[clang] 11bd3e5 - [Analyzer][StreamChecker] Introduction of stream error handling.

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

Author: Balázs Kéri
Date: 2020-04-08T11:30:19+02:00
New Revision: 11bd3e5c6549a4983be454ccfbeb16e88c9532db

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

LOG: [Analyzer][StreamChecker] Introduction of stream error handling.

Summary:
Store the error flags (EOF or error) of a stream.
Support the functions feof, ferror, clearerr.
Added a test checker for setting the error flags.

Reviewers: Szelethus, NoQ, Charusso, baloghadamsoftware, xazax.hun

Reviewed By: Szelethus

Subscribers: steakhal, ASDenysPetrov, rnkovacs, xazax.hun, baloghadamsoftware, 
szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, 
Charusso, martong, cfe-commits

Tags: #clang

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

Added: 
clang/test/Analysis/stream-error.c

Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/Inputs/system-header-simulator.h
clang/test/Analysis/stream.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 6a577940e313..31e7e02c192b 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1400,6 +1400,11 @@ def TaintTesterChecker : Checker<"TaintTest">,
   HelpText<"Mark tainted symbols as such.">,
   Documentation;
 
+def StreamTesterChecker : Checker<"StreamTester">,
+  HelpText<"Add test functions to StreamChecker for test and debugging 
purposes.">,
+  Dependencies<[StreamChecker]>,
+  Documentation;
+
 def ExprInspectionChecker : Checker<"ExprInspection">,
   HelpText<"Check the analyzer's understanding of expressions">,
   Documentation;

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 4b20fcdc9a15..b38a645432f4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -19,33 +19,67 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include 
 
 using namespace clang;
 using namespace ento;
-using namespace std::placeholders;
 
 namespace {
 
+/// Full state information about a stream pointer.
 struct StreamState {
-  enum Kind { Opened, Closed, OpenFailed, Escaped } K;
-
-  StreamState(Kind k) : K(k) {}
-
-  bool isOpened() const { return K == Opened; }
-  bool isClosed() const { return K == Closed; }
-  bool isOpenFailed() const { return K == OpenFailed; }
-  //bool isEscaped() const { return K == Escaped; }
+  /// State of a stream symbol.
+  /// FIXME: We need maybe an "escaped" state later.
+  enum KindTy {
+Opened, /// Stream is opened.
+Closed, /// Closed stream (an invalid stream pointer after it was closed).
+OpenFailed /// The last open operation has failed.
+  } State;
+
+  /// The error state of a stream.
+  /// Valid only if the stream is opened.
+  /// It is assumed that feof and ferror flags are never true at the same time.
+  enum ErrorKindTy {
+/// No error flag is set (or stream is not open).
+NoError,
+/// EOF condition (`feof` is true).
+FEof,
+/// Other generic (non-EOF) error (`ferror` is true).
+FError,
+  } ErrorState = NoError;
+
+  bool isOpened() const { return State == Opened; }
+  bool isClosed() const { return State == Closed; }
+  bool isOpenFailed() const { return State == OpenFailed; }
+
+  bool isNoError() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == NoError;
+  }
+  bool isFEof() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == FEof;
+  }
+  bool isFError() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == FError;
+  }
 
-  bool operator==(const StreamState &X) const { return K == X.K; }
+  bool operator==(const StreamState &X) const {
+// In not opened state error should always NoError.
+return State == X.State && ErrorState == X.ErrorState;
+  }
 
-  static StreamState getOpened() { return StreamState(Opened); }
-  static StreamState getClosed() { return StreamState(Closed); }
-  static StreamState getOpenFailed() { return StreamState(OpenFailed); }
-  static StreamState getEscaped() { return StreamState(Escaped); }
+  static StreamState getOpened() { return StreamState{Opened}; }
+  static StreamState getClosed() { return StreamState{Closed}; }
+  static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
+  static St

[clang] 37ac1c1 - [Analyzer][VLASize] Support multi-dimensional arrays.

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

Author: Balázs Kéri
Date: 2020-04-14T10:26:51+02:00
New Revision: 37ac1c19bed7b7d22e9312dfa61e7a4506ed4e49

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

LOG: [Analyzer][VLASize] Support multi-dimensional arrays.

Summary:
Check the size constraints for every (variable) dimension of the array.
Try to compute array size by multiplying size for every dimension.

Reviewers: Szelethus, martong, baloghadamsoftware, gamesh411

Reviewed By: Szelethus, martong

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
clang/test/Analysis/vla.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 21e8b9653039..0c7961a2a28b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -34,6 +34,9 @@ class VLASizeChecker : public Checker< 
check::PreStmt > {
   mutable std::unique_ptr BT;
   enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted, VLA_Negative };
 
+  ProgramStateRef checkVLASize(CheckerContext &C, ProgramStateRef State,
+   const Expr *SizeE) const;
+
   void reportBug(VLASize_Kind Kind, const Expr *SizeE, ProgramStateRef State,
  CheckerContext &C,
  std::unique_ptr Visitor = nullptr) const;
@@ -43,6 +46,65 @@ class VLASizeChecker : public Checker< 
check::PreStmt > {
 };
 } // end anonymous namespace
 
+ProgramStateRef VLASizeChecker::checkVLASize(CheckerContext &C,
+ ProgramStateRef State,
+ const Expr *SizeE) const {
+  SVal SizeV = C.getSVal(SizeE);
+
+  if (SizeV.isUndef()) {
+reportBug(VLA_Garbage, SizeE, State, C);
+return nullptr;
+  }
+
+  // See if the size value is known. It can't be undefined because we would 
have
+  // warned about that already.
+  if (SizeV.isUnknown())
+return nullptr;
+
+  // Check if the size is tainted.
+  if (isTainted(State, SizeV)) {
+reportBug(VLA_Tainted, SizeE, nullptr, C,
+  std::make_unique(SizeV));
+return nullptr;
+  }
+
+  // Check if the size is zero.
+  DefinedSVal SizeD = SizeV.castAs();
+
+  ProgramStateRef StateNotZero, StateZero;
+  std::tie(StateNotZero, StateZero) = State->assume(SizeD);
+
+  if (StateZero && !StateNotZero) {
+reportBug(VLA_Zero, SizeE, StateZero, C);
+return nullptr;
+  }
+
+  // From this point on, assume that the size is not zero.
+  State = StateNotZero;
+
+  // Check if the size is negative.
+  SValBuilder &SVB = C.getSValBuilder();
+
+  QualType SizeTy = SizeE->getType();
+  DefinedOrUnknownSVal Zero = SVB.makeZeroVal(SizeTy);
+
+  SVal LessThanZeroVal = SVB.evalBinOp(State, BO_LT, SizeD, Zero, SizeTy);
+  if (Optional LessThanZeroDVal =
+  LessThanZeroVal.getAs()) {
+ConstraintManager &CM = C.getConstraintManager();
+ProgramStateRef StatePos, StateNeg;
+
+std::tie(StateNeg, StatePos) = CM.assumeDual(State, *LessThanZeroDVal);
+if (StateNeg && !StatePos) {
+  reportBug(VLA_Negative, SizeE, State, C); // FIXME: StateNeg ?
+  return nullptr;
+}
+State = StatePos;
+  }
+
+  return State;
+}
+
 void VLASizeChecker::reportBug(
 VLASize_Kind Kind, const Expr *SizeE, ProgramStateRef State,
 CheckerContext &C, std::unique_ptr Visitor) const {
@@ -89,98 +151,72 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, 
CheckerContext &C) const {
 return;
 
   ASTContext &Ctx = C.getASTContext();
+  SValBuilder &SVB = C.getSValBuilder();
+  ProgramStateRef State = C.getState();
+
   const VariableArrayType *VLA = Ctx.getAsVariableArrayType(VD->getType());
   if (!VLA)
 return;
 
-  // FIXME: Handle multi-dimensional VLAs.
-  const Expr *SE = VLA->getSizeExpr();
-  ProgramStateRef state = C.getState();
-  SVal sizeV = C.getSVal(SE);
-
-  if (sizeV.isUndef()) {
-reportBug(VLA_Garbage, SE, state, C);
-return;
-  }
-
-  // See if the size value is known. It can't be undefined because we would 
have
-  // warned about that already.
-  if (sizeV.isUnknown())
-return;
-
-  // Check if the size is tainted.
-  if (isTainted(state, sizeV)) {
-reportBug(VLA_Tainted, SE, nullptr, C,
-  std::make_unique(sizeV));
-return;
-  }
-
-  // Check if the size is zero.
-  DefinedSVal sizeD = sizeV.castAs();
-
-  ProgramStateRef stateNotZero, stateZero;
-  std::tie(stateNotZero, stateZero) = state->assume(sizeD);
-
-  if (stateZero &&

[clang] f2b5e60 - [Analyzer][StreamChecker] Added evaluation of fseek.

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

Author: Balázs Kéri
Date: 2020-04-14T12:35:28+02:00
New Revision: f2b5e60dfd09f99d036197c078179c774f8b4582

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

LOG: [Analyzer][StreamChecker] Added evaluation of fseek.

Summary:
Function `fseek` is now evaluated with setting error return value
and error flags.

Reviewers: Szelethus, NoQ, xazax.hun, rnkovacs, dcoughlin, baloghadamsoftware, 
martong

Reviewed By: Szelethus

Subscribers: ASDenysPetrov, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index b38a645432f4..76dd62d30ddf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -25,8 +25,13 @@ using namespace ento;
 
 namespace {
 
+struct FnDescription;
+
 /// Full state information about a stream pointer.
 struct StreamState {
+  /// The last file operation called in the stream.
+  const FnDescription *LastOperation;
+
   /// State of a stream symbol.
   /// FIXME: We need maybe an "escaped" state later.
   enum KindTy {
@@ -45,6 +50,9 @@ struct StreamState {
 FEof,
 /// Other generic (non-EOF) error (`ferror` is true).
 FError,
+/// Unknown error flag is set (or none), the meaning depends on the last
+/// operation.
+Unknown
   } ErrorState = NoError;
 
   bool isOpened() const { return State == Opened; }
@@ -63,28 +71,47 @@ struct StreamState {
 assert(State == Opened && "Error undefined for closed stream.");
 return ErrorState == FError;
   }
+  bool isUnknown() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == Unknown;
+  }
 
   bool operator==(const StreamState &X) const {
 // In not opened state error should always NoError.
-return State == X.State && ErrorState == X.ErrorState;
+return LastOperation == X.LastOperation && State == X.State &&
+   ErrorState == X.ErrorState;
   }
 
-  static StreamState getOpened() { return StreamState{Opened}; }
-  static StreamState getClosed() { return StreamState{Closed}; }
-  static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
-  static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; }
-  static StreamState getOpenedWithFError() {
-return StreamState{Opened, FError};
+  static StreamState getOpened(const FnDescription *L) {
+return StreamState{L, Opened};
+  }
+  static StreamState getOpened(const FnDescription *L, ErrorKindTy E) {
+return StreamState{L, Opened, E};
+  }
+  static StreamState getClosed(const FnDescription *L) {
+return StreamState{L, Closed};
   }
+  static StreamState getOpenFailed(const FnDescription *L) {
+return StreamState{L, OpenFailed};
+  }
+
+  /// Return if the specified error kind is possible on the stream in the
+  /// current state.
+  /// This depends on the stored `LastOperation` value.
+  /// If the error is not possible returns empty value.
+  /// If the error is possible returns the remaining possible error type
+  /// (after taking out `ErrorKind`). If a single error is possible it will
+  /// return that value, otherwise unknown error.
+  Optional getRemainingPossibleError(ErrorKindTy ErrorKind) const;
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
+ID.AddPointer(LastOperation);
 ID.AddInteger(State);
 ID.AddInteger(ErrorState);
   }
 };
 
 class StreamChecker;
-struct FnDescription;
 using FnCheck = std::function;
 
@@ -95,8 +122,28 @@ struct FnDescription {
   FnCheck PreFn;
   FnCheck EvalFn;
   ArgNoTy StreamArgNo;
+  // What errors are possible after this operation.
+  // Used only if this operation resulted in Unknown state
+  // (otherwise there is a known single error).
+  // Must contain 2 or 3 elements, or zero.
+  llvm::SmallVector PossibleErrors = {};
 };
 
+Optional
+StreamState::getRemainingPossibleError(ErrorKindTy ErrorKind) const {
+  assert(ErrorState == Unknown &&
+ "Function to be used only if error is unknown.");
+  llvm::SmallVector NewPossibleErrors;
+  for (StreamState::ErrorKindTy E : LastOperation->PossibleErrors)
+if (E != ErrorKind)
+  NewPossibleErrors.push_back(E);
+  if (NewPossibleErrors.size() == LastOperation->PossibleErrors.size())
+return {};
+  if (NewPossibleErrors.size() == 1)
+return NewPossibleErrors.front();
+  return Unknown;
+}
+
 /// Get the value of the stream argument out

[clang] 0bfd70b - [Analyzer][StreamChecker] Updated initialization of BugType's.

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

Author: Balázs Kéri
Date: 2020-06-04T16:06:07+02:00
New Revision: 0bfd70bdad7e4ac22d96503fa78a5dd55d4b430e

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

LOG: [Analyzer][StreamChecker] Updated initialization of BugType's.

Summary:
BugType objects are initialized in-class instead of by lazy initialization.
FuchsiaHandleChecker does this already.

Reviewers: Szelethus, baloghadamsoftware, martong

Reviewed By: Szelethus

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 63ebfaf90dc8..bfb788c24e24 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -195,9 +195,29 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef 
State,
 
 class StreamChecker
 : public Checker {
-  mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
-  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof,
-  BT_IndeterminatePosition;
+  BuiltinBug BT_FileNull{this, "NULL stream pointer",
+ "Stream pointer might be NULL."};
+  BuiltinBug BT_UseAfterClose{
+  this, "Closed stream",
+  "Stream might be already closed. Causes undefined behaviour."};
+  BuiltinBug BT_UseAfterOpenFailed{this, "Invalid stream",
+   "Stream might be invalid after "
+   "(re-)opening it has failed. "
+   "Can cause undefined behaviour."};
+  BuiltinBug BT_IndeterminatePosition{
+  this, "Invalid stream state",
+  "File position of the stream might be 'indeterminate' "
+  "after a failed operation. "
+  "Can cause undefined behavior."};
+  BuiltinBug BT_IllegalWhence{this, "Illegal whence argument",
+  "The whence argument to fseek() should be "
+  "SEEK_SET, SEEK_END, or SEEK_CUR."};
+  BuiltinBug BT_StreamEof{this, "Stream already in EOF",
+  "Read function called when stream is in EOF state. "
+  "Function has no effect."};
+  BuiltinBug BT_ResourceLeak{
+  this, "Resource Leak",
+  "Opened File never closed. Potential Resource leak."};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -755,11 +775,8 @@ StreamChecker::ensureStreamNonNull(SVal StreamVal, 
CheckerContext &C,
 
   if (!StateNotNull && StateNull) {
 if (ExplodedNode *N = C.generateErrorNode(StateNull)) {
-  if (!BT_nullfp)
-BT_nullfp.reset(new BuiltinBug(this, "NULL stream pointer",
-   "Stream pointer might be NULL."));
   C.emitReport(std::make_unique(
-  *BT_nullfp, BT_nullfp->getDescription(), N));
+  BT_FileNull, BT_FileNull.getDescription(), N));
 }
 return nullptr;
   }
@@ -783,12 +800,8 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal 
StreamVal,
 // according to cppreference.com .
 ExplodedNode *N = C.generateErrorNode();
 if (N) {
-  if (!BT_UseAfterClose)
-BT_UseAfterClose.reset(new BuiltinBug(this, "Closed stream",
-  "Stream might be already closed. 
"
-  "Causes undefined behaviour."));
   C.emitReport(std::make_unique(
-  *BT_UseAfterClose, BT_UseAfterClose->getDescription(), N));
+  BT_UseAfterClose, BT_UseAfterClose.getDescription(), N));
   return nullptr;
 }
 
@@ -802,14 +815,8 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal 
StreamVal,
 // failed to open.
 ExplodedNode *N = C.generateErrorNode();
 if (N) {
-  if (!BT_UseAfterOpenFailed)
-BT_UseAfterOpenFailed.reset(
-new BuiltinBug(this, "Invalid stream",
-   "Stream might be invalid after "
-   "(re-)opening it has failed. "
-   "Can cause undefined behaviour."));
   C.emitReport(std::make_unique(
-  *BT_UseAfterOpenFailed, BT_UseAfterOpenFailed->getDescription(), N));
+  BT_UseAfterOpenFailed, BT_UseAfterOpenFailed.getDescription(), N));
   return nullptr;
 }
 return State;
@@ -831,13 +838,6 @@ ProgramStateRef 
StreamChecker::ensureNoFilePositionIndeterminate(
   assert(SS->isOpened() && "First ensure that stream is opened.

[clang] efa8b6e - [Analyzer][StreamChecker] Add check for pointer escape.

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

Author: Balázs Kéri
Date: 2020-06-15T15:43:23+02:00
New Revision: efa8b6e884aa4c3ad74ab44489352c0f80ac9741

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

LOG: [Analyzer][StreamChecker] Add check for pointer escape.

Summary:
After an escaped FILE* stream handle it is not possible to make
reliable checks on it because any function call can have effect
on it.

Reviewers: Szelethus, baloghadamsoftware, martong, NoQ

Reviewed By: NoQ

Subscribers: NoQ, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index bfb788c24e24..7e10b2aa4356 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -193,8 +193,8 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef 
State,
   return State;
 }
 
-class StreamChecker
-: public Checker {
+class StreamChecker : public Checker 
{
   BuiltinBug BT_FileNull{this, "NULL stream pointer",
  "Stream pointer might be NULL."};
   BuiltinBug BT_UseAfterClose{
@@ -223,6 +223,10 @@ class StreamChecker
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+  ProgramStateRef checkPointerEscape(ProgramStateRef State,
+ const InvalidatedSymbols &Escaped,
+ const CallEvent *Call,
+ PointerEscapeKind Kind) const;
 
   /// If true, evaluate special testing stream functions.
   bool TestMode = false;
@@ -448,10 +452,14 @@ void StreamChecker::evalFreopen(const FnDescription *Desc,
 
   SymbolRef StreamSym = StreamVal->getAsSymbol();
   // Do not care about concrete values for stream ("(FILE *)0x12345"?).
-  // FIXME: Are stdin, stdout, stderr such values?
+  // FIXME: Can be stdin, stdout, stderr such values?
   if (!StreamSym)
 return;
 
+  // Do not handle untracked stream. It is probably escaped.
+  if (!State->get(StreamSym))
+return;
+
   // Generate state for non-failed case.
   // Return value is the passed stream pointer.
   // According to the documentations, the stream is closed first
@@ -918,6 +926,28 @@ void StreamChecker::checkDeadSymbols(SymbolReaper 
&SymReaper,
   }
 }
 
+ProgramStateRef StreamChecker::checkPointerEscape(
+ProgramStateRef State, const InvalidatedSymbols &Escaped,
+const CallEvent *Call, PointerEscapeKind Kind) const {
+  // Check for file-handling system call that is not handled by the checker.
+  // FIXME: The checker should be updated to handle all system calls that take
+  // 'FILE*' argument. These are now ignored.
+  if (Kind == PSK_DirectEscapeOnCall && Call->isInSystemHeader())
+return State;
+
+  for (SymbolRef Sym : Escaped) {
+// The symbol escaped.
+// From now the stream can be manipulated in unknown way to the checker,
+// it is not possible to handle it any more.
+// Optimistically, assume that the corresponding file handle will be closed
+// somewhere else.
+// Remove symbol from state so the following stream calls on this symbol 
are
+// not handled by the checker.
+State = State->remove(Sym);
+  }
+  return State;
+}
+
 void ento::registerStreamChecker(CheckerManager &Mgr) {
   Mgr.registerChecker();
 }

diff  --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 554a234c20d5..dbbcc8715e0d 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -192,4 +192,51 @@ void check_freopen_3() {
 rewind(f1); // expected-warning {{Stream might be invalid}}
 fclose(f1);
   }
-}
\ No newline at end of file
+}
+
+extern FILE *GlobalF;
+extern void takeFile(FILE *);
+
+void check_escape1() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fwrite("1", 1, 1, F); // may fail
+  GlobalF = F;
+  fwrite("1", 1, 1, F); // no warning
+}
+
+void check_escape2() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fwrite("1", 1, 1, F); // may fail
+  takeFile(F);
+  fwrite("1", 1, 1, F); // no warning
+}
+
+void check_escape3() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  takeFile(F);
+  F = freopen(0, "w", F);
+  if (!F)
+return;
+  fwrite("1", 1, 1, F); // may fail
+  fwrite("1", 1, 1, F); // no warning
+}
+
+void check_escape4() {
+  

[clang] 85f5d12 - [ASTImporter] Corrected import of repeated friend declarations.

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

Author: Balázs Kéri
Date: 2020-07-07T16:24:24+02:00
New Revision: 85f5d1261c9a3f0abc4ae370005a1127174f2ce1

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

LOG: [ASTImporter] Corrected import of repeated friend declarations.

Summary:
Import declarations in correct order if a class contains
multiple redundant friend (type or decl) declarations.
If the order is incorrect this could cause false structural
equivalences and wrong declaration chains after import.

Reviewers: a.sidorin, shafik, a_sidorin

Reviewed By: shafik

Subscribers: dkrupp, Szelethus, gamesh411, teemperor, martong, cfe-commits

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index a47be2da4aab..8ec6db622f0a 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3646,6 +3646,54 @@ ExpectedDecl 
ASTNodeImporter::VisitIndirectFieldDecl(IndirectFieldDecl *D) {
   return ToIndirectField;
 }
 
+/// Used as return type of getFriendCountAndPosition.
+struct FriendCountAndPosition {
+  /// Number of similar looking friends.
+  unsigned int TotalCount;
+  /// Index of the specific FriendDecl.
+  unsigned int IndexOfDecl;
+};
+
+template 
+FriendCountAndPosition getFriendCountAndPosition(
+const FriendDecl *FD,
+std::function GetCanTypeOrDecl) {
+  unsigned int FriendCount = 0;
+  llvm::Optional FriendPosition;
+  const auto *RD = cast(FD->getLexicalDeclContext());
+
+  T TypeOrDecl = GetCanTypeOrDecl(FD);
+
+  for (const FriendDecl *FoundFriend : RD->friends()) {
+if (FoundFriend == FD) {
+  FriendPosition = FriendCount;
+  ++FriendCount;
+} else if (!FoundFriend->getFriendDecl() == !FD->getFriendDecl() &&
+   GetCanTypeOrDecl(FoundFriend) == TypeOrDecl) {
+  ++FriendCount;
+}
+  }
+
+  assert(FriendPosition && "Friend decl not found in own parent.");
+
+  return {FriendCount, *FriendPosition};
+}
+
+FriendCountAndPosition getFriendCountAndPosition(const FriendDecl *FD) {
+  if (FD->getFriendType())
+return getFriendCountAndPosition(FD, [](const FriendDecl *F) {
+  if (TypeSourceInfo *TSI = F->getFriendType())
+return TSI->getType().getCanonicalType();
+  llvm_unreachable("Wrong friend object type.");
+});
+  else
+return getFriendCountAndPosition(FD, [](const FriendDecl *F) {
+  if (Decl *D = F->getFriendDecl())
+return D->getCanonicalDecl();
+  llvm_unreachable("Wrong friend object type.");
+});
+}
+
 ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
   // Import the major distinguishing characteristics of a declaration.
   DeclContext *DC, *LexicalDC;
@@ -3654,25 +3702,37 @@ ExpectedDecl 
ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
 
   // Determine whether we've already imported this decl.
   // FriendDecl is not a NamedDecl so we cannot use lookup.
-  auto *RD = cast(DC);
+  // We try to maintain order and count of redundant friend declarations.
+  const auto *RD = cast(DC);
   FriendDecl *ImportedFriend = RD->getFirstFriend();
+  SmallVector ImportedEquivalentFriends;
 
   while (ImportedFriend) {
+bool Match = false;
 if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
-  if (IsStructuralMatch(D->getFriendDecl(), 
ImportedFriend->getFriendDecl(),
-/*Complain=*/false))
-return Importer.MapImported(D, ImportedFriend);
-
+  Match =
+  IsStructuralMatch(D->getFriendDecl(), 
ImportedFriend->getFriendDecl(),
+/*Complain=*/false);
 } else if (D->getFriendType() && ImportedFriend->getFriendType()) {
-  if (Importer.IsStructurallyEquivalent(
-D->getFriendType()->getType(),
-ImportedFriend->getFriendType()->getType(), true))
-return Importer.MapImported(D, ImportedFriend);
+  Match = Importer.IsStructurallyEquivalent(
+  D->getFriendType()->getType(),
+  ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
 }
+if (Match)
+  ImportedEquivalentFriends.push_back(ImportedFriend);
+
 ImportedFriend = ImportedFriend->getNextFriend();
   }
+  FriendCountAndPosition CountAndPosition = getFriendCountAndPosition(D);
+
+  assert(ImportedEquivalentFriends.size() <= CountAndPosition.TotalCount &&
+ "Class with non-matching friends is imported, ODR check wrong?");
+  if (ImportedEquivalentFriends.size() == CountAndPosition.TotalCount)
+return Importer.MapImported(
+D, ImportedEquivalentFriends[CountAndPosition.IndexOfDecl]);
 
   // Not found

[clang] 22a084c - [Analyzer] Report every bug if only uniqueing location differs.

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

Author: Balázs Kéri
Date: 2020-07-15T12:19:25+02:00
New Revision: 22a084cfa337d5e5ea90eba5261f7937e28d250b

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

LOG: [Analyzer] Report every bug if only uniqueing location differs.

Summary:
Two CSA bug reports where only the uniqueing location is different
should be treated as different problems. The role of uniqueing location
is to differentiate bug reports.

Reviewers: Szelethus, baloghadamsoftware, NoQ, vsavchenko, xazax.hun, martong

Reviewed By: NoQ

Subscribers: NoQ, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Analysis/PathDiagnostic.cpp
clang/test/Analysis/malloc.c
clang/test/Analysis/pr22954.c

Removed: 




diff  --git a/clang/lib/Analysis/PathDiagnostic.cpp 
b/clang/lib/Analysis/PathDiagnostic.cpp
index c88e6c1e1535..9aa3386129d7 100644
--- a/clang/lib/Analysis/PathDiagnostic.cpp
+++ b/clang/lib/Analysis/PathDiagnostic.cpp
@@ -327,6 +327,10 @@ static Optional comparePath(const PathPieces &X, 
const PathPieces &Y) {
 }
 
 static bool compareCrossTUSourceLocs(FullSourceLoc XL, FullSourceLoc YL) {
+  if (XL.isInvalid() && YL.isValid())
+return true;
+  if (XL.isValid() && YL.isInvalid())
+return false;
   std::pair XOffs = XL.getDecomposedLoc();
   std::pair YOffs = YL.getDecomposedLoc();
   const SourceManager &SM = XL.getManager();
@@ -349,6 +353,10 @@ static bool compare(const PathDiagnostic &X, const 
PathDiagnostic &Y) {
   FullSourceLoc YL = Y.getLocation().asLocation();
   if (XL != YL)
 return compareCrossTUSourceLocs(XL, YL);
+  FullSourceLoc XUL = X.getUniqueingLoc().asLocation();
+  FullSourceLoc YUL = Y.getUniqueingLoc().asLocation();
+  if (XUL != YUL)
+return compareCrossTUSourceLocs(XUL, YUL);
   if (X.getBugType() != Y.getBugType())
 return X.getBugType() < Y.getBugType();
   if (X.getCategory() != Y.getCategory())
@@ -357,20 +365,27 @@ static bool compare(const PathDiagnostic &X, const 
PathDiagnostic &Y) {
 return X.getVerboseDescription() < Y.getVerboseDescription();
   if (X.getShortDescription() != Y.getShortDescription())
 return X.getShortDescription() < Y.getShortDescription();
-  if (X.getDeclWithIssue() != Y.getDeclWithIssue()) {
-const Decl *XD = X.getDeclWithIssue();
-if (!XD)
+  auto CompareDecls = [&XL](const Decl *D1, const Decl *D2) -> Optional {
+if (D1 == D2)
+  return None;
+if (!D1)
   return true;
-const Decl *YD = Y.getDeclWithIssue();
-if (!YD)
+if (!D2)
   return false;
-SourceLocation XDL = XD->getLocation();
-SourceLocation YDL = YD->getLocation();
-if (XDL != YDL) {
+SourceLocation D1L = D1->getLocation();
+SourceLocation D2L = D2->getLocation();
+if (D1L != D2L) {
   const SourceManager &SM = XL.getManager();
-  return compareCrossTUSourceLocs(FullSourceLoc(XDL, SM),
-  FullSourceLoc(YDL, SM));
+  return compareCrossTUSourceLocs(FullSourceLoc(D1L, SM),
+  FullSourceLoc(D2L, SM));
 }
+return None;
+  };
+  if (auto Result = CompareDecls(X.getDeclWithIssue(), Y.getDeclWithIssue()))
+return *Result;
+  if (XUL.isValid()) {
+if (auto Result = CompareDecls(X.getUniqueingDecl(), Y.getUniqueingDecl()))
+  return *Result;
   }
   PathDiagnostic::meta_iterator XI = X.meta_begin(), XE = X.meta_end();
   PathDiagnostic::meta_iterator YI = Y.meta_begin(), YE = Y.meta_end();
@@ -1118,6 +1133,8 @@ void 
PathDiagnosticPopUpPiece::Profile(llvm::FoldingSetNodeID &ID) const {
 
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
   ID.Add(getLocation());
+  ID.Add(getUniqueingLoc());
+  ID.AddPointer(getUniqueingLoc().isValid() ? getUniqueingDecl() : nullptr);
   ID.AddString(BugType);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);

diff  --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 714c73c3c793..a26b51196781 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -791,7 +791,8 @@ void mallocEscapeMalloc() {
 void mallocMalloc() {
   int *p = malloc(12);
   p = malloc(12);
-} // expected-warning {{Potential leak of memory pointed to by}}
+} // expected-warning {{Potential leak of memory pointed to by}}\
+  // expected-warning {{Potential leak of memory pointed to by}}
 
 void mallocFreeMalloc() {
   int *p = malloc(12);

diff  --git a/clang/test/Analysis/pr22954.c b/clang/test/Analysis/pr22954.c
index e88acdc29d39..093f6311a505 100644
--- a/clang/test/Analysis/pr22954.c
+++ b/clang/test/Analysis/pr22954.

[clang] e935a54 - [Analyzer][StreamChecker] Add note tags for file opening.

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

Author: Balázs Kéri
Date: 2020-06-22T11:15:35+02:00
New Revision: e935a540ea29d5de297595801aed1fb70fabfbf6

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

LOG: [Analyzer][StreamChecker] Add note tags for file opening.

Summary:
Bug reports of resource leak are now improved.
If there are multiple resource leak paths for the same stream,
only one wil be reported.

Reviewers: Szelethus, xazax.hun, baloghadamsoftware, NoQ

Reviewed By: Szelethus, NoQ

Subscribers: NoQ, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 
clang/test/Analysis/stream-note.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c
clang/test/Analysis/stream.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 7e10b2aa4356..8fe097826fad 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -216,8 +216,8 @@ class StreamChecker : public CheckergetState();
+  // When bug type is resource leak, exploded node N may not have state info
+  // for leaked file descriptor, but predecessor should have it.
+  if (!State->get(StreamSym))
+N = N->getFirstPred();
+
+  const ExplodedNode *Pred = N;
+  while (N) {
+State = N->getState();
+if (!State->get(StreamSym))
+  return Pred;
+Pred = N;
+N = N->getFirstPred();
+  }
+
+  return nullptr;
+}
+
 void StreamChecker::checkPreCall(const CallEvent &Call,
  CheckerContext &C) const {
   const FnDescription *Desc = lookupFn(Call);
@@ -421,7 +469,8 @@ void StreamChecker::evalFopen(const FnDescription *Desc, 
const CallEvent &Call,
   StateNull =
   StateNull->set(RetSym, StreamState::getOpenFailed(Desc));
 
-  C.addTransition(StateNotNull);
+  C.addTransition(StateNotNull,
+  constructNoteTag(C, RetSym, "Stream opened here"));
   C.addTransition(StateNull);
 }
 
@@ -476,7 +525,8 @@ void StreamChecker::evalFreopen(const FnDescription *Desc,
   StateRetNull =
   StateRetNull->set(StreamSym, 
StreamState::getOpenFailed(Desc));
 
-  C.addTransition(StateRetNotNull);
+  C.addTransition(StateRetNotNull,
+  constructNoteTag(C, StreamSym, "Stream reopened here"));
   C.addTransition(StateRetNull);
 }
 
@@ -921,8 +971,38 @@ void StreamChecker::checkDeadSymbols(SymbolReaper 
&SymReaper,
 if (!N)
   continue;
 
-C.emitReport(std::make_unique(
-BT_ResourceLeak, BT_ResourceLeak.getDescription(), N));
+// Do not warn for non-closed stream at program exit.
+ExplodedNode *Pred = C.getPredecessor();
+if (Pred && Pred->getCFGBlock() &&
+Pred->getCFGBlock()->hasNoReturnElement())
+  continue;
+
+// Resource leaks can result in multiple warning that describe the same 
kind
+// of programming error:
+//  void f() {
+//FILE *F = fopen("a.txt");
+//if (rand()) // state split
+//  return; // warning
+//  } // warning
+// While this isn't necessarily true (leaking the same stream could result
+// from a 
diff erent kinds of errors), the reduction in redundant reports
+// makes this a worthwhile heuristic.
+// FIXME: Add a checker option to turn this uniqueing feature off.
+
+const ExplodedNode *StreamOpenNode = getAcquisitionSite(N, Sym, C);
+assert(StreamOpenNode && "Could not find place of stream opening.");
+PathDiagnosticLocation LocUsedForUniqueing =
+PathDiagnosticLocation::createBegin(
+StreamOpenNode->getStmtForDiagnostics(), C.getSourceManager(),
+StreamOpenNode->getLocationContext());
+
+std::unique_ptr R =
+std::make_unique(
+BT_ResourceLeak, BT_ResourceLeak.getDescription(), N,
+LocUsedForUniqueing,
+StreamOpenNode->getLocationContext()->getDecl());
+R->markInteresting(Sym);
+C.emitReport(std::move(R));
   }
 }
 

diff  --git a/clang/test/Analysis/stream-note.c 
b/clang/test/Analysis/stream-note.c
new file mode 100644
index ..08927e8902be
--- /dev/null
+++ b/clang/test/Analysis/stream-note.c
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream 
-analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void check_note_at_correct_open() {
+  FILE *F1 = tmpfile(); // expected-note {{Stream opened here}}
+  if (!F1)
+// expected-note@-1 {{'F1' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  FILE *F2 = 

[clang] d1df560 - [Analyzer][StreamChecker] Use BugType instead of BuiltinBug (NFC) .

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

Author: Balázs Kéri
Date: 2020-06-29T15:37:13+02:00
New Revision: d1df56023132914b877e34f6cf475758a96540f2

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

LOG: [Analyzer][StreamChecker] Use BugType instead of BuiltinBug (NFC) .

Summary:
I do not like the BuiltinBug class.
And it takes no SuppressOnSink parameter that may be needed in the future.

Reviewers: Szelethus, baloghadamsoftware, gamesh411

Reviewed By: Szelethus

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 8fe097826fad..f6abbe4f8f03 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -195,29 +195,16 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef 
State,
 
 class StreamChecker : public Checker 
{
-  BuiltinBug BT_FileNull{this, "NULL stream pointer",
- "Stream pointer might be NULL."};
-  BuiltinBug BT_UseAfterClose{
-  this, "Closed stream",
-  "Stream might be already closed. Causes undefined behaviour."};
-  BuiltinBug BT_UseAfterOpenFailed{this, "Invalid stream",
-   "Stream might be invalid after "
-   "(re-)opening it has failed. "
-   "Can cause undefined behaviour."};
-  BuiltinBug BT_IndeterminatePosition{
-  this, "Invalid stream state",
-  "File position of the stream might be 'indeterminate' "
-  "after a failed operation. "
-  "Can cause undefined behavior."};
-  BuiltinBug BT_IllegalWhence{this, "Illegal whence argument",
-  "The whence argument to fseek() should be "
-  "SEEK_SET, SEEK_END, or SEEK_CUR."};
-  BuiltinBug BT_StreamEof{this, "Stream already in EOF",
-  "Read function called when stream is in EOF state. "
-  "Function has no effect."};
-  BuiltinBug BT_ResourceLeak{
-  this, "Resource leak",
-  "Opened stream never closed. Potential resource leak."};
+  BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"};
+  BugType BT_UseAfterClose{this, "Closed stream", "Stream handling error"};
+  BugType BT_UseAfterOpenFailed{this, "Invalid stream",
+"Stream handling error"};
+  BugType BT_IndeterminatePosition{this, "Invalid stream state",
+   "Stream handling error"};
+  BugType BT_IllegalWhence{this, "Illegal whence argument",
+   "Stream handling error"};
+  BugType BT_StreamEof{this, "Stream already in EOF", "Stream handling error"};
+  BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error"};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -834,7 +821,7 @@ StreamChecker::ensureStreamNonNull(SVal StreamVal, 
CheckerContext &C,
   if (!StateNotNull && StateNull) {
 if (ExplodedNode *N = C.generateErrorNode(StateNull)) {
   C.emitReport(std::make_unique(
-  BT_FileNull, BT_FileNull.getDescription(), N));
+  BT_FileNull, "Stream pointer might be NULL.", N));
 }
 return nullptr;
   }
@@ -859,7 +846,8 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal 
StreamVal,
 ExplodedNode *N = C.generateErrorNode();
 if (N) {
   C.emitReport(std::make_unique(
-  BT_UseAfterClose, BT_UseAfterClose.getDescription(), N));
+  BT_UseAfterClose,
+  "Stream might be already closed. Causes undefined behaviour.", N));
   return nullptr;
 }
 
@@ -874,7 +862,11 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal 
StreamVal,
 ExplodedNode *N = C.generateErrorNode();
 if (N) {
   C.emitReport(std::make_unique(
-  BT_UseAfterOpenFailed, BT_UseAfterOpenFailed.getDescription(), N));
+  BT_UseAfterOpenFailed,
+  "Stream might be invalid after "
+  "(re-)opening it has failed. "
+  "Can cause undefined behaviour.",
+  N));
   return nullptr;
 }
 return State;
@@ -885,6 +877,11 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal 
StreamVal,
 
 ProgramStateRef StreamChecker::ensureNoFilePositionIndeterminate(
 SVal StreamVal, CheckerContext &C, ProgramStateRef State) const {
+  static const char *BugMessage =
+  "File position of the stream might be 'inde

[clang] f3b3446 - [clang][CrossTU] Invalidate parent map after get cross TU definition.

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

Author: Balázs Kéri
Date: 2020-07-01T09:13:05+02:00
New Revision: f3b34466104877b024e168cac054bded6b9279a0

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

LOG: [clang][CrossTU] Invalidate parent map after get cross TU definition.

Summary:
Parent map of ASTContext is built once. If this happens and later
the TU is modified by getCrossTUDefinition the parent map does not
contain the newly imported objects and has to be re-created.

Invalidation of the parent map is added to the CrossTranslationUnitContext.
It could be added to ASTImporter as well but for now this task remains the
responsibility of the user of ASTImporter. Reason for this is mostly that
ASTImporter calls itself recursively.

Reviewers: gamesh411, martong

Reviewed By: gamesh411

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, martong, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Removed: 




diff  --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp 
b/clang/lib/CrossTU/CrossTranslationUnit.cpp
index 24c3143db410..11cc2afbc36d 100644
--- a/clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -12,6 +12,7 @@
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/AST/ASTImporter.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CrossTU/CrossTUDiagnostic.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -718,6 +719,9 @@ CrossTranslationUnitContext::importDefinitionImpl(const T 
*D, ASTUnit *Unit) {
   assert(hasBodyOrInit(ToDecl) && "Imported Decl should have body or init.");
   ++NumGetCTUSuccess;
 
+  // Parent map is invalidated after changing the AST.
+  ToDecl->getASTContext().getParentMapContext().clear();
+
   return ToDecl;
 }
 

diff  --git a/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp 
b/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
index 36a697d54c8a..ceb1437f2520 100644
--- a/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ b/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
@@ -44,6 +45,10 @@ class CTUASTConsumer : public clang::ASTConsumer {
 assert(FD && FD->getName() == "f");
 bool OrigFDHasBody = FD->hasBody();
 
+const DynTypedNodeList ParentsBeforeImport =
+Ctx.getParentMapContext().getParents(*FD);
+ASSERT_FALSE(ParentsBeforeImport.empty());
+
 // Prepare the index file and the AST file.
 int ASTFD;
 llvm::SmallString<256> ASTFileName;
@@ -105,10 +110,29 @@ class CTUASTConsumer : public clang::ASTConsumer {
 EXPECT_EQ(OrigSLoc, FDWithDefinition->getLocation());
   }
 }
+
+// Check parent map.
+const DynTypedNodeList ParentsAfterImport =
+Ctx.getParentMapContext().getParents(*FD);
+const DynTypedNodeList ParentsOfImported =
+Ctx.getParentMapContext().getParents(*NewFD);
+EXPECT_TRUE(
+checkParentListsEq(ParentsBeforeImport, ParentsAfterImport));
+EXPECT_FALSE(ParentsOfImported.empty());
   }
 }
   }
 
+  static bool checkParentListsEq(const DynTypedNodeList &L1,
+ const DynTypedNodeList &L2) {
+if (L1.size() != L2.size())
+  return false;
+for (unsigned int I = 0; I < L1.size(); ++I)
+  if (L1[I] != L2[I])
+return false;
+return true;
+  }
+
 private:
   CrossTranslationUnitContext CTU;
   bool *Success;



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

[clang] 3b9b3d5 - [Analyzer] Include typedef statements in CFG build.

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

Author: Balázs Kéri
Date: 2020-04-27T12:36:26+02:00
New Revision: 3b9b3d56efaa6f611458899d5a1cdc74f36d72a4

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

LOG: [Analyzer] Include typedef statements in CFG build.

Summary:
Array size expressions in typedef statements with a VLA
(variable-length array) are handled from now as in plain
(non-typedef) VLA declarations.
Type-aliases with VLA are handled too
(but main focus is on C code).

Reviewers: Szelethus, aaron.ballman, NoQ, xazax.hun

Reviewed By: aaron.ballman, xazax.hun

Subscribers: rnkovacs, NoQ, efriedma, xazax.hun, baloghadamsoftware, szepet, 
a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, 
martong, ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 
clang/test/Analysis/cfg.c

Modified: 
clang/lib/Analysis/CFG.cpp
clang/test/Analysis/cfg.cpp

Removed: 




diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 8091625703bc..bedc8455366f 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2839,11 +2839,30 @@ CFGBlock *CFGBuilder::VisitDeclStmt(DeclStmt *DS) {
 /// DeclStmts and initializers in them.
 CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
   assert(DS->isSingleDecl() && "Can handle single declarations only.");
+
+  if (const auto *TND = dyn_cast(DS->getSingleDecl())) {
+// If we encounter a VLA, process its size expressions.
+const Type *T = TND->getUnderlyingType().getTypePtr();
+if (!T->isVariablyModifiedType())
+  return Block;
+
+autoCreateBlock();
+appendStmt(Block, DS);
+
+CFGBlock *LastBlock = Block;
+for (const VariableArrayType *VA = FindVA(T); VA != nullptr;
+ VA = FindVA(VA->getElementType().getTypePtr())) {
+  if (CFGBlock *NewBlock = addStmt(VA->getSizeExpr()))
+LastBlock = NewBlock;
+}
+return LastBlock;
+  }
+
   VarDecl *VD = dyn_cast(DS->getSingleDecl());
 
   if (!VD) {
-// Of everything that can be declared in a DeclStmt, only VarDecls impact
-// runtime semantics.
+// Of everything that can be declared in a DeclStmt, only VarDecls and the
+// exceptions above impact runtime semantics.
 return Block;
   }
 
@@ -2905,6 +2924,8 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
   }
 
   // If the type of VD is a VLA, then we must process its size expressions.
+  // FIXME: This does not find the VLA if it is embedded in other types,
+  // like here: `int (*p_vla)[x];`
   for (const VariableArrayType* VA = FindVA(VD->getType().getTypePtr());
VA != nullptr; VA = FindVA(VA->getElementType().getTypePtr())) {
 if (CFGBlock *newBlock = addStmt(VA->getSizeExpr()))
@@ -3997,6 +4018,11 @@ CFGBlock 
*CFGBuilder::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E,
   }
 
   // VLA types have expressions that must be evaluated.
+  // Evaluation is done only for `sizeof`.
+
+  if (E->getKind() != UETT_SizeOf)
+return Block;
+
   CFGBlock *lastBlock = Block;
 
   if (E->isArgumentType()) {

diff  --git a/clang/test/Analysis/cfg.c b/clang/test/Analysis/cfg.c
new file mode 100644
index ..3494dd6ac2a3
--- /dev/null
+++ b/clang/test/Analysis/cfg.c
@@ -0,0 +1,120 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple 
x86_64-apple-darwin12 -fheinous-gnu-extensions %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+
+// This file is the C version of cfg.cpp.
+// Tests that are C-specific should go into this file.
+
+// CHECK-LABEL: void checkWrap(int i)
+// CHECK: ENTRY
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B1]
+// CHECK: Succs (21): B2 B3 B4 B5 B6 B7 B8 B9
+// CHECK: B10 B11 B12 B13 B14 B15 B16 B17 B18 B19
+// CHECK: B20 B21 B0
+// CHECK: [B0 (EXIT)]
+// CHECK-NEXT: Preds (21): B2 B3 B4 B5 B6 B7 B8 B9
+// CHECK-NEXT: B10 B11 B12 B13 B14 B15 B16 B17 B18 B19
+// CHECK-NEXT: B20 B21 B1
+void checkWrap(int i) {
+  switch(i) {
+case 0: break;
+case 1: break;
+case 2: break;
+case 3: break;
+case 4: break;
+case 5: break;
+case 6: break;
+case 7: break;
+case 8: break;
+case 9: break;
+case 10: break;
+case 11: break;
+case 12: break;
+case 13: break;
+case 14: break;
+case 15: break;
+case 16: break;
+case 17: break;
+case 18: break;
+case 19: break;
+  }
+}
+
+// CHECK-LABEL: void checkGCCAsmRValueOutput()
+// CHECK: [B2 (ENTRY)]
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B1]
+// CHECK-NEXT:   1: int arg
+// CHECK-NEXT:   2: arg
+// CHECK-NEXT:   3: (int)[B1.2] (CStyleCastExpr, NoOp, int)
+// CHECK-NEXT:   4: asm ("" : "=r" ([B1.3]));
+// CHECK-NEXT:   5: arg
+// CHECK-NEXT:   6: asm ("" : "=r" ([B1.5]));
+void checkGCCAsmRValue

[clang] 497d060 - [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

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

Author: Balázs Kéri
Date: 2020-08-11T10:10:13+02:00
New Revision: 497d060d0a741e13dd5e6218ba7301a7ec96f332

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

LOG: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

Report undefined pointer dereference in similar way as null pointer dereference.

Reviewed By: NoQ

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

Added: 
clang/test/Analysis/invalid-deref.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
clang/test/Analysis/misc-ps-region-store.m

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index 9a87729de8fd..adfc2f8cb8fe 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,11 +30,14 @@ class DereferenceChecker
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
+  enum DerefKind { NullPointer, UndefinedPointerValue };
+
   BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
   BugType BT_Undef{this, "Dereference of undefined pointer value",
categories::LogicError};
 
-  void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C) 
const;
+  void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S,
+ CheckerContext &C) const;
 
 public:
   void checkLocation(SVal location, bool isLoad, const Stmt* S,
@@ -117,8 +120,24 @@ static bool isDeclRefExprToReference(const Expr *E) {
   return false;
 }
 
-void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
-   CheckerContext &C) const {
+void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State,
+   const Stmt *S, CheckerContext &C) const {
+  const BugType *BT = nullptr;
+  llvm::StringRef DerefStr1;
+  llvm::StringRef DerefStr2;
+  switch (K) {
+  case DerefKind::NullPointer:
+BT = &BT_Null;
+DerefStr1 = " results in a null pointer dereference";
+DerefStr2 = " results in a dereference of a null pointer";
+break;
+  case DerefKind::UndefinedPointerValue:
+BT = &BT_Undef;
+DerefStr1 = " results in an undefined pointer dereference";
+DerefStr2 = " results in a dereference of an undefined pointer value";
+break;
+  };
+
   // Generate an error node.
   ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
@@ -135,7 +154,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
 const ArraySubscriptExpr *AE = cast(S);
 AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext());
-os << " results in a null pointer dereference";
+os << DerefStr1;
 break;
   }
   case Stmt::OMPArraySectionExprClass: {
@@ -143,11 +162,11 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
 const OMPArraySectionExpr *AE = cast(S);
 AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext());
-os << " results in a null pointer dereference";
+os << DerefStr1;
 break;
   }
   case Stmt::UnaryOperatorClass: {
-os << "Dereference of null pointer";
+os << BT->getDescription();
 const UnaryOperator *U = cast(S);
 AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
State.get(), N->getLocationContext(), true);
@@ -156,8 +175,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
   case Stmt::MemberExprClass: {
 const MemberExpr *M = cast(S);
 if (M->isArrow() || isDeclRefExprToReference(M->getBase())) {
-  os << "Access to field '" << M->getMemberNameInfo()
- << "' results in a dereference of a null pointer";
+  os << "Access to field '" << M->getMemberNameInfo() << "'" << DerefStr2;
   AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
  State.get(), N->getLocationContext(), true);
 }
@@ -165,8 +183,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
   }
   case Stmt::ObjCIvarRefExprClass: {
 const ObjCIvarRefExpr *IV = cast(S);
-os << "Access to instance variable '" << *IV->getDecl()
-   << "' results in a dereference of a null pointer";
+os << "Access to instance variable '" << *IV->getDecl() << "'" << 
DerefStr2;
 AddDerefSource(os, Ranges, IV->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext(), true);
 break;
@@ -176,7 +193,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State,

[clang] cb1eeb4 - [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

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

Author: Balázs Kéri
Date: 2020-05-14T14:30:05+02:00
New Revision: cb1eeb42c03c31d4dadd00dbaec693e6d7516099

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

LOG: [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

Summary:
The check of VLA size was done previously for variable declarations
(of VLA type) only. Now it is done for typedef (and type-alias)
and sizeof expressions with VLA too.

Reviewers: Szelethus, martong

Reviewed By: Szelethus, martong

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
clang/test/Analysis/vla.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 0c7961a2a28b..3bd2520f013a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -30,10 +30,26 @@ using namespace ento;
 using namespace taint;
 
 namespace {
-class VLASizeChecker : public Checker< check::PreStmt > {
+class VLASizeChecker
+: public Checker,
+ check::PreStmt> {
   mutable std::unique_ptr BT;
   enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted, VLA_Negative };
 
+  /// Check a VLA for validity.
+  /// Every dimension of the array is checked for validity, and
+  /// dimension sizes are collected into 'VLASizes'. 'VLALast' is set to the
+  /// innermost VLA that was encountered.
+  /// In "int vla[x][2][y][3]" this will be the array for index "y" (with type
+  /// int[3]). 'VLASizes' contains 'x', '2', and 'y'. Returns null or a new
+  /// state where the size is validated for every dimension.
+  ProgramStateRef checkVLA(CheckerContext &C, ProgramStateRef State,
+   const VariableArrayType *VLA,
+   const VariableArrayType *&VLALast,
+   llvm::SmallVector &VLASizes) const;
+
+  /// Check one VLA dimension for validity.
+  /// Returns null or a new state where the size is validated.
   ProgramStateRef checkVLASize(CheckerContext &C, ProgramStateRef State,
const Expr *SizeE) const;
 
@@ -43,9 +59,37 @@ class VLASizeChecker : public Checker< 
check::PreStmt > {
 
 public:
   void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
+  void checkPreStmt(const UnaryExprOrTypeTraitExpr *UETTE,
+CheckerContext &C) const;
 };
 } // end anonymous namespace
 
+ProgramStateRef
+VLASizeChecker::checkVLA(CheckerContext &C, ProgramStateRef State,
+ const VariableArrayType *VLA,
+ const VariableArrayType *&VLALast,
+ llvm::SmallVector &VLASizes) const {
+  assert(VLA && "Function should be called with non-null VLA argument.");
+
+  VLALast = nullptr;
+  // Walk over the VLAs for every dimension until a non-VLA is found.
+  // There is a VariableArrayType for every dimension (fixed or variable) until
+  // the most inner array that is variably modified.
+  while (VLA) {
+const Expr *SizeE = VLA->getSizeExpr();
+State = checkVLASize(C, State, SizeE);
+if (!State)
+  return nullptr;
+VLASizes.push_back(SizeE);
+VLALast = VLA;
+VLA = C.getASTContext().getAsVariableArrayType(VLA->getElementType());
+  };
+  assert(VLALast &&
+ "Array should have at least one variably-modified dimension.");
+
+  return State;
+}
+
 ProgramStateRef VLASizeChecker::checkVLASize(CheckerContext &C,
  ProgramStateRef State,
  const Expr *SizeE) const {
@@ -146,36 +190,34 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, 
CheckerContext &C) const {
   if (!DS->isSingleDecl())
 return;
 
-  const VarDecl *VD = dyn_cast(DS->getSingleDecl());
-  if (!VD)
-return;
-
   ASTContext &Ctx = C.getASTContext();
   SValBuilder &SVB = C.getSValBuilder();
   ProgramStateRef State = C.getState();
+  QualType TypeToCheck;
+
+  const VarDecl *VD = dyn_cast(DS->getSingleDecl());
+
+  if (VD)
+TypeToCheck = VD->getType().getCanonicalType();
+  else if (const auto *TND = dyn_cast(DS->getSingleDecl()))
+TypeToCheck = TND->getUnderlyingType().getCanonicalType();
+  else
+return;
 
-  const VariableArrayType *VLA = Ctx.getAsVariableArrayType(VD->getType());
+  const VariableArrayType *VLA = Ctx.getAsVariableArrayType(TypeToCheck);
   if (!VLA)
 return;
 
+  // Check the VLA si

[clang] 22d40cc - [Analyzer][StreamChecker] Changed representation of stream error state - NFC.

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

Author: Balázs Kéri
Date: 2020-05-18T16:18:59+02:00
New Revision: 22d40cc3a724fa3df259c52009571a21a3a3a632

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

LOG: [Analyzer][StreamChecker] Changed representation of stream error state - 
NFC.

Summary:
State of error flags for a stream is handled by having separate flags
that allow combination of multiple error states to be described with one
error state object.
After a failed function the error state is set in the stream state
and must not be determined later based on the last failed function
like before this change. The error state can not always be determined
from the last failed function and it was not the best design.

Reviewers: Szelethus

Reviewed By: Szelethus

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 76dd62d30ddf..d079951221d0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -19,14 +19,62 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include 
 
 using namespace clang;
 using namespace ento;
+using namespace std::placeholders;
 
 namespace {
 
 struct FnDescription;
 
+/// State of the stream error flags.
+/// Sometimes it is not known to the checker what error flags are set.
+/// This is indicated by setting more than one flag to true.
+/// This is an optimization to avoid state splits.
+/// A stream can either be in FEOF or FERROR but not both at the same time.
+/// Multiple flags are set to handle the corresponding states together.
+struct StreamErrorState {
+  /// The stream can be in state where none of the error flags set.
+  bool NoError = true;
+  /// The stream can be in state where the EOF indicator is set.
+  bool FEof = false;
+  /// The stream can be in state where the error indicator is set.
+  bool FError = false;
+
+  bool isNoError() const { return NoError && !FEof && !FError; }
+  bool isFEof() const { return !NoError && FEof && !FError; }
+  bool isFError() const { return !NoError && !FEof && FError; }
+
+  bool operator==(const StreamErrorState &ES) const {
+return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError;
+  }
+
+  StreamErrorState operator|(const StreamErrorState &E) const {
+return {NoError || E.NoError, FEof || E.FEof, FError || E.FError};
+  }
+
+  StreamErrorState operator&(const StreamErrorState &E) const {
+return {NoError && E.NoError, FEof && E.FEof, FError && E.FError};
+  }
+
+  StreamErrorState operator~() const { return {!NoError, !FEof, !FError}; }
+
+  /// Returns if the StreamErrorState is a valid object.
+  operator bool() const { return NoError || FEof || FError; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+ID.AddBoolean(NoError);
+ID.AddBoolean(FEof);
+ID.AddBoolean(FError);
+  }
+};
+
+const StreamErrorState ErrorNone{true, false, false};
+const StreamErrorState ErrorFEof{false, true, false};
+const StreamErrorState ErrorFError{false, false, true};
+
 /// Full state information about a stream pointer.
 struct StreamState {
   /// The last file operation called in the stream.
@@ -40,53 +88,24 @@ struct StreamState {
 OpenFailed /// The last open operation has failed.
   } State;
 
-  /// The error state of a stream.
-  /// Valid only if the stream is opened.
-  /// It is assumed that feof and ferror flags are never true at the same time.
-  enum ErrorKindTy {
-/// No error flag is set (or stream is not open).
-NoError,
-/// EOF condition (`feof` is true).
-FEof,
-/// Other generic (non-EOF) error (`ferror` is true).
-FError,
-/// Unknown error flag is set (or none), the meaning depends on the last
-/// operation.
-Unknown
-  } ErrorState = NoError;
+  /// State of the error flags.
+  /// Ignored in non-opened stream state but must be NoError.
+  StreamErrorState ErrorState;
 
   bool isOpened() const { return State == Opened; }
   bool isClosed() const { return State == Closed; }
   bool isOpenFailed() const { return State == OpenFailed; }
 
-  bool isNoError() const {
-assert(State == Opened && "Error undefined for closed stream.");
-return ErrorState == NoError;
-  }
-  bool isFEof() const {
-assert(State == Opened && "Error u

[clang] 1907f28 - [Analyzer][StreamChecker] Fixed compile error - NFC.

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

Author: Balázs Kéri
Date: 2020-05-18T17:14:39+02:00
New Revision: 1907f28b47cfe9c951df43309d121679895b0edf

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

LOG: [Analyzer][StreamChecker] Fixed compile error - NFC.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index d079951221d0..c94cae045239 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -108,10 +108,10 @@ struct StreamState {
 return StreamState{L, Opened, ES};
   }
   static StreamState getClosed(const FnDescription *L) {
-return StreamState{L, Closed};
+return StreamState{L, Closed, {}};
   }
   static StreamState getOpenFailed(const FnDescription *L) {
-return StreamState{L, OpenFailed};
+return StreamState{L, OpenFailed, {}};
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {



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

[clang] 51bb212 - [Analyzer][VLASizeChecker] Check for VLA size overflow.

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

Author: Balázs Kéri
Date: 2020-05-19T09:44:46+02:00
New Revision: 51bb2128ef03985fddf2a84f17d3276f4ae2c6ad

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

LOG: [Analyzer][VLASizeChecker] Check for VLA size overflow.

Summary:
Variable-length array (VLA) should have a size that fits into
a size_t value. According to the standard: "std::size_t can
store the maximum size of a theoretically possible object of
any type (including array)" (this is applied to C too).

The size expression is evaluated at the definition of the
VLA type even if this is a typedef.
The evaluation of the size expression in itself might cause
problems if it overflows.

Reviewers: Szelethus, baloghadamsoftware, martong, gamesh411

Reviewed By: Szelethus, martong, gamesh411

Subscribers: whisperity, rnkovacs, xazax.hun, baloghadamsoftware, szepet, 
a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, 
martong, ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 
clang/test/Analysis/vla-overflow.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 3bd2520f013a..de487042fb8a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -34,24 +34,24 @@ class VLASizeChecker
 : public Checker,
  check::PreStmt> {
   mutable std::unique_ptr BT;
-  enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted, VLA_Negative };
+  enum VLASize_Kind {
+VLA_Garbage,
+VLA_Zero,
+VLA_Tainted,
+VLA_Negative,
+VLA_Overflow
+  };
 
   /// Check a VLA for validity.
-  /// Every dimension of the array is checked for validity, and
-  /// dimension sizes are collected into 'VLASizes'. 'VLALast' is set to the
-  /// innermost VLA that was encountered.
-  /// In "int vla[x][2][y][3]" this will be the array for index "y" (with type
-  /// int[3]). 'VLASizes' contains 'x', '2', and 'y'. Returns null or a new
-  /// state where the size is validated for every dimension.
-  ProgramStateRef checkVLA(CheckerContext &C, ProgramStateRef State,
-   const VariableArrayType *VLA,
-   const VariableArrayType *&VLALast,
-   llvm::SmallVector &VLASizes) const;
-
-  /// Check one VLA dimension for validity.
+  /// Every dimension of the array and the total size is checked for validity.
   /// Returns null or a new state where the size is validated.
-  ProgramStateRef checkVLASize(CheckerContext &C, ProgramStateRef State,
-   const Expr *SizeE) const;
+  /// 'ArraySize' will contain SVal that refers to the total size (in char)
+  /// of the array.
+  ProgramStateRef checkVLA(CheckerContext &C, ProgramStateRef State,
+   const VariableArrayType *VLA, SVal &ArraySize) 
const;
+  /// Check a single VLA index size expression for validity.
+  ProgramStateRef checkVLAIndexSize(CheckerContext &C, ProgramStateRef State,
+const Expr *SizeE) const;
 
   void reportBug(VLASize_Kind Kind, const Expr *SizeE, ProgramStateRef State,
  CheckerContext &C,
@@ -64,20 +64,25 @@ class VLASizeChecker
 };
 } // end anonymous namespace
 
-ProgramStateRef
-VLASizeChecker::checkVLA(CheckerContext &C, ProgramStateRef State,
- const VariableArrayType *VLA,
- const VariableArrayType *&VLALast,
- llvm::SmallVector &VLASizes) const {
+ProgramStateRef VLASizeChecker::checkVLA(CheckerContext &C,
+ ProgramStateRef State,
+ const VariableArrayType *VLA,
+ SVal &ArraySize) const {
   assert(VLA && "Function should be called with non-null VLA argument.");
 
-  VLALast = nullptr;
+  const VariableArrayType *VLALast = nullptr;
+  llvm::SmallVector VLASizes;
+
   // Walk over the VLAs for every dimension until a non-VLA is found.
   // There is a VariableArrayType for every dimension (fixed or variable) until
   // the most inner array that is variably modified.
+  // Dimension sizes are collected into 'VLASizes'. 'VLALast' is set to the
+  // innermost VLA that was encountered.
+  // In "int vla[x][2][y][3]" this will be the array for index "y" (with type
+  // int[3]). 'VLASizes' contains 'x', '2', and 'y'.
   while (VLA) {
 const Expr *SizeE = VLA->getSizeExpr();
-State = checkVLASize(C, State, SizeE);
+State = checkVLAIndexSize(C, State, SizeE);
 if (!State)
   ret

[clang] 56079e1 - [Analyzer][VLASizeChecker] Try to fix vla.c test problems.

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

Author: Balázs Kéri
Date: 2020-05-19T12:12:28+02:00
New Revision: 56079e1de1129837aa7569d8b3bb5e50afc0f1ea

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

LOG: [Analyzer][VLASizeChecker] Try to fix vla.c test problems.

Added: 


Modified: 
clang/test/Analysis/vla.c

Removed: 




diff  --git a/clang/test/Analysis/vla.c b/clang/test/Analysis/vla.c
index 062bb0828e23..a269ef334c32 100644
--- a/clang/test/Analysis/vla.c
+++ b/clang/test/Analysis/vla.c
@@ -18,11 +18,11 @@ void check_uninit_sized_VLA() {
 }
 
 // Negative VLAs.
-static void vla_allocate_signed(int x) {
+static void vla_allocate_signed(short x) {
   int vla[x]; // expected-warning{{Declared variable-length array (VLA) has 
negative size}}
 }
 
-static void vla_allocate_unsigned(unsigned int x) {
+static void vla_allocate_unsigned(unsigned short x) {
   int vla[x]; // no-warning
 }
 
@@ -35,12 +35,12 @@ void check_negative_sized_VLA_2() {
 }
 
 void check_negative_sized_VLA_3() {
-  int x = -1;
+  short x = -1;
   int vla[x]; // expected-warning{{Declared variable-length array (VLA) has 
negative size}}
 }
 
 void check_negative_sized_VLA_4() {
-  unsigned int x = -1;
+  unsigned short x = -1;
   int vla[x]; // no-warning
 }
 
@@ -79,12 +79,12 @@ void check_negative_sized_VLA_10(int x) {
 check_negative_sized_VLA_10_sub(x);
 }
 
-static void check_negative_sized_VLA_11_sub(int x)
+static void check_negative_sized_VLA_11_sub(short x)
 {
   int vla[x]; // no-warning
 }
 
-void check_negative_sized_VLA_11(int x) {
+void check_negative_sized_VLA_11(short x) {
   if (x > 0)
 check_negative_sized_VLA_11_sub(x);
 }



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

[clang] d56a1c6 - [clang][analyzer] Errno modeling code refactor (NFC).

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

Author: Balázs Kéri
Date: 2022-09-01T09:05:59+02:00
New Revision: d56a1c68247751e94c4fc46dda282643d3739689

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

LOG: [clang][analyzer] Errno modeling code refactor (NFC).

Some of the code used in StdLibraryFunctionsChecker is applicable to
other checkers, this is put into common functions. Errno related
parts of the checker are simplified and renamed. Documentations in
errno_modeling functions are updated.

This change makes it available to have more checkers that perform
modeling of some standard functions. These can set the errno state
with common functions and the bug report messages (note tags) can
look similar.

Reviewed By: steakhal, martong

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index 618f7e97f6e89..a9e249de38047 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -264,6 +264,12 @@ bool isErrno(const Decl *D) {
   return false;
 }
 
+const char *describeErrnoCheckState(ErrnoCheckState CS) {
+  assert(CS == errno_modeling::MustNotBeChecked &&
+ "Errno description not applicable.");
+  return "may be undefined after the call and should not be used";
+}
+
 const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message) {
   return C.getNoteTag([Message](PathSensitiveBugReport &BR) -> std::string {
 const MemRegion *ErrnoR = 
BR.getErrorNode()->getState()->get();
@@ -275,6 +281,32 @@ const NoteTag *getErrnoNoteTag(CheckerContext &C, const 
std::string &Message) {
   });
 }
 
+ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State,
+  CheckerContext &C) {
+  return setErrnoState(State, MustNotBeChecked);
+}
+
+ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
+  NonLoc ErrnoSym) {
+  SValBuilder &SVB = C.getSValBuilder();
+  NonLoc ZeroVal = SVB.makeZeroVal(C.getASTContext().IntTy).castAs();
+  DefinedOrUnknownSVal Cond =
+  SVB.evalBinOp(State, BO_NE, ErrnoSym, ZeroVal, SVB.getConditionType())
+  .castAs();
+  State = State->assume(Cond, true);
+  if (!State)
+return nullptr;
+  return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
+}
+
+const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
+  return getErrnoNoteTag(
+  C, (Twine("Assuming that function '") + Twine(Fn) +
+  Twine("' is successful, in this case the value 'errno' ") +
+  Twine(describeErrnoCheckState(MustNotBeChecked)))
+ .str());
+}
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 3757e25e1afe6..d46a403d59a54 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -21,16 +21,23 @@ namespace clang {
 namespace ento {
 namespace errno_modeling {
 
+/// Describe how reads and writes of \c errno are handled by the checker.
 enum ErrnoCheckState : unsigned {
   /// We do not know anything about 'errno'.
+  /// Read and write is always allowed.
   Irrelevant = 0,
 
   /// Value of 'errno' should be checked to find out if a previous function 
call
   /// has failed.
+  /// When this state is set \c errno must be read by the program before a next
+  /// standard function call or other overwrite of \c errno follows, otherwise
+  /// a bug report is emitted.
   MustBeChecked = 1,
 
   /// Value of 'errno' is not allowed to be read, it can contain an unspecified
   /// value.
+  /// When this state is set \c errno is not allowed to be read by the program
+  /// until it is overwritten or invalidated.
   MustNotBeChecked = 2
 };
 
@@ -67,10 +74,38 @@ ProgramStateRef setErrnoState(ProgramStateRef State, 
ErrnoCheckState EState);
 /// declaration.
 bool isErrno(const Decl *D);
 
+/// Produce a textual description about how \c errno is allowed to be used
+/// (in a \c ErrnoCheckState).
+/// The returned string is insertable into a longer warning message in the form
+/// "the value 'errno' <...>".
+/// Currently only the \c errno_modeling::MustNotBeChecked state is supported,
+/// others are not used by the clients.
+const char *describeErrnoCheckState(ErrnoCheckState CS);
+
 /// Create a NoteTag that displays the message if t

[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] 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][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][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]

@@ -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]

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]

@@ -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]

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] 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] 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] 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] 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] 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] 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 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]

@@ -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]

@@ -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]

@@ -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] 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] 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 '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]

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] 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][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] 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][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][NFC] Supplement comments in `evalFtell` of StreamChecker (PR #74291)

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

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


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

[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

@@ -1188,6 +1192,18 @@ void StreamChecker::evalSetFeofFerror(const 
FnDescription *Desc,
   C.addTransition(State);
 }
 
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call,
+  CheckerContext &C) const {
+  // Skip if the stream is NULL/nullptr, which means flush all streams.
+  if (!Call.getArgExpr(Desc->StreamArgNo)

balazske wrote:

This check does not work in this case:
```
FILE *F = NULL;
fflush(F);
```
I think you should copy function `ensureStreamNonNull` to here and change it 
such that it makes a check for opened stream in the non-null case. In case of 
null stream no error is needed, and at unknown stream no assumption is needed 
about the non-nullness of the stream.

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

[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

@@ -299,6 +299,15 @@ void error_fseek_0(void) {
   fclose(F);
 }
 
+void error_fflush(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fclose(F);
+  fflush(F);// expected-warning {{Stream might be already closed}}
+  fflush(NULL); // no-warning

balazske wrote:

The `fflush(NULL)` should be before `fflush(F)` (probably at the start of the 
function) because analysis may stop when a warning is emitted (and following 
code is not reachable).

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

[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

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

[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

@@ -299,6 +299,15 @@ void error_fseek_0(void) {
   fclose(F);
 }
 
+void error_fflush(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fclose(F);
+  fflush(F);// expected-warning {{Stream might be already closed}}
+  fflush(NULL); // no-warning
+}
+

balazske wrote:

A new test is needed when the passed stream is NULL but not a null constant.

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

[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

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

For now only the pre-condition is added, the `evalFflush` function is missing.

https://github.com/llvm/llvm-project/pull/74296
___
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/3] [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 c4e931e220f69..7a5e3d6653285 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 5f4d8d040772c..d439a14b7b998 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")));
+  Decl *Fro

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

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

@@ -5050,6 +5050,59 @@ TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
   EXPECT_EQ(ToTUX, ToX);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateDeclConflict) {
+  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 

balazske wrote:

Yes, this code was incorrect. It is still a bug that the test did not fail, my 
next planned fixes should improve this situation.

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][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

@@ -1191,6 +1199,49 @@ void StreamChecker::evalSetFeofFerror(const 
FnDescription *Desc,
   C.addTransition(State);
 }
 
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call,
+  CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SVal StreamVal = getStreamArg(Desc, Call);
+  std::optional Stream = StreamVal.getAs();
+  if (!Stream)
+return;
+
+  ConstraintManager::ProgramStatePair SP =
+  C.getConstraintManager().assumeDual(State, *Stream);
+  if (State = SP.first)
+if (State = ensureStreamOpened(StreamVal, C, State))
+  C.addTransition(State);

balazske wrote:

I think another `addTransition` is needed for `SP.second` if it is non-null.

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

[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

@@ -1191,6 +1199,49 @@ void StreamChecker::evalSetFeofFerror(const 
FnDescription *Desc,
   C.addTransition(State);
 }
 
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call,
+  CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SVal StreamVal = getStreamArg(Desc, Call);
+  std::optional Stream = StreamVal.getAs();
+  if (!Stream)
+return;
+
+  ConstraintManager::ProgramStatePair SP =
+  C.getConstraintManager().assumeDual(State, *Stream);
+  if (State = SP.first)
+if (State = ensureStreamOpened(StreamVal, C, State))
+  C.addTransition(State);
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent 
&Call,
+   CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+
+  // We will check the result even if the input is `NULL`,
+  // but do nothing if the input state is unknown.
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (StreamSym) {
+const StreamState *OldSS = State->get(StreamSym);
+if (!OldSS)
+  return;
+assertStreamStateOpened(OldSS);
+  }
+
+  const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  // `fflush` returns 0 on success, otherwise returns EOF.
+  ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+
+  // This function does not affect the stream state.
+  // Still we add success and failure state with the appropriate return value.

balazske wrote:

The stream state can be affected in some way, if the stream was in failed 
(`ferror`) state or the position is indeterminate, probably the `fflush` can be 
allowed but the error (and indeterminate position) should be reset to 
non-error. If the argument is null we can do this reset for all of the known 
streams. It is somewhat questionable what should happen at a failing `fflush`, 
but probably it is OK to leave the error flags as it was before.

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

[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

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

@@ -299,6 +299,21 @@ void error_fseek_0(void) {
   fclose(F);
 }
 
+void error_fflush(void) {
+  FILE *F = tmpfile();
+  int Ret;
+  fflush(NULL);  // no-warning
+  if (!F) {
+if ((Ret = fflush(F)) != EOF)// no-warning
+  clang_analyzer_eval(Ret == 0); // expected-warning {{TRUE}}
+return;
+  }
+  if ((Ret = fflush(F)) != 0)
+clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+  fclose(F);
+  fflush(F); // expected-warning {{Stream might be 
already closed}}
+}
+

balazske wrote:

If the stream error state is reset by `fflush` another test is needed to check 
if the reset works.

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

[clang] [clang][ASTImporter] Improve import of friend class templates. (PR #74627)

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

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

A friend template that is in a dependent context is not linked into declaration 
chains (for example with the definition of the befriended template). This 
condition was not correctly handled by `ASTImporter`.

From cbcb81ebdbc49e3bd11b6f716ac14658a729b787 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Tue, 5 Dec 2023 15:23:37 +0100
Subject: [PATCH] [clang][ASTImporter] Improve import of friend class
 templates.

A friend template that is in a dependent context is not linked into
declaration chains (for example with the definition of the befriended
template). This condition was not correctly handled by ASTImporter.
---
 clang/lib/AST/ASTImporter.cpp   |  26 +++-
 clang/unittests/AST/ASTImporterTest.cpp | 162 
 2 files changed, 182 insertions(+), 6 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a..a243bf65cca27 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5919,15 +5919,26 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   if (ToD)
 return ToD;
 
-  bool IsFriendTemplate = D->getFriendObjectKind() != Decl::FOK_None;
-  bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
+  // Should check if a declaration is friend in a dependent context.
+  // Such templates are not linked together in a declaration chain.
+  // The ASTImporter strategy is to map existing forward declarations to
+  // imported ones only if strictly necessary, otherwise import these as new
+  // forward declarations. In case of the "dependent friend" declarations, new
+  // declarations are created, but not linked in a declaration chain.
+  auto IsDependentFriend = [](ClassTemplateDecl *TD) {
+bool IsFriendTemplate = TD->getFriendObjectKind() != Decl::FOK_None;
+DeclContext *DC = TD->getDeclContext();
+DeclContext *LexicalDC = TD->getLexicalDeclContext();
+bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
 : DC->isDependentContext();
-  bool DependentFriend = IsFriendTemplate && IsDependentContext;
+return IsFriendTemplate && IsDependentContext;
+  };
+  bool DependentFriend = IsDependentFriend(D);
 
   ClassTemplateDecl *FoundByLookup = nullptr;
 
   // We may already have a template of the same name; try to find and match it.
-  if (!DependentFriend && !DC->isFunctionOrMethod()) {
+  if (!DC->isFunctionOrMethod()) {
 SmallVector ConflictingDecls;
 auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
 for (auto *FoundDecl : FoundDecls) {
@@ -5943,10 +5954,13 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
 
 // FIXME: sufficient conditon for 'IgnoreTemplateParmDepth'?
 bool IgnoreTemplateParmDepth =
-FoundTemplate->getFriendObjectKind() != Decl::FOK_None &&
-!D->specializations().empty();
+(FoundTemplate->getFriendObjectKind() != Decl::FOK_None) !=
+(D->getFriendObjectKind() != Decl::FOK_None);
 if (IsStructuralMatch(D, FoundTemplate, /*Complain=*/true,
   IgnoreTemplateParmDepth)) {
+  if (DependentFriend || IsDependentFriend(FoundTemplate))
+continue;
+
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);
   if (D->isThisDeclarationADefinition() && TemplateWithDef)
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf..f9ee73626c948 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4528,6 +4528,168 @@ TEST_P(ImportFriendClasses, 
ImportOfClassDefinitionAndFwdFriendShouldBeLinked) {
   EXPECT_EQ(ImportedFwd, ImportedDef->getPreviousDecl());
 }
 
+TEST_P(ImportFriendClasses,
+   ImportFriendTemplatesInDependentContext_DefToFriend) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template
+  struct X {
+template
+friend struct Y;
+  };
+  )",
+  Lang_CXX03);
+  auto *ToYFriend = FirstDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+  R"(
+  template
+  struct Y {};
+  )",
+  Lang_CXX03, "input0.cc");
+  auto *FromYDef = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYDef = Import(FromYDef, Lang_CXX03);
+  EXPECT_TRUE(ImportedYDef);
+  EXPECT_FALSE(ImportedYDef->getPreviousDecl());
+  EXPECT_NE(ImportedYDef, ToYFriend);
+}
+
+TEST_P(ImportFriendClasses,
+   ImportFriendTemplatesInDependentContext_DefToFriend_NE) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template
+  struct X {
+template
+friend struct Y;
+  };
+  )",
+  Lang_CXX03);
+  

[clang] [clang][ASTImporter] Improve import of friend class templates. (PR #74627)

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

@@ -5943,10 +5954,13 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
 
 // FIXME: sufficient conditon for 'IgnoreTemplateParmDepth'?
 bool IgnoreTemplateParmDepth =

balazske wrote:

Change of this condition was needed to make a failing test 
`ImportOfRecursiveFriendClassTemplateWithNonTypeParm` to pass.

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

[clang] [clang][ASTImporter] Improve import of friend class templates. (PR #74627)

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

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

From cbcb81ebdbc49e3bd11b6f716ac14658a729b787 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Tue, 5 Dec 2023 15:23:37 +0100
Subject: [PATCH 1/2] [clang][ASTImporter] Improve import of friend class
 templates.

A friend template that is in a dependent context is not linked into
declaration chains (for example with the definition of the befriended
template). This condition was not correctly handled by ASTImporter.
---
 clang/lib/AST/ASTImporter.cpp   |  26 +++-
 clang/unittests/AST/ASTImporterTest.cpp | 162 
 2 files changed, 182 insertions(+), 6 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a..a243bf65cca27 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5919,15 +5919,26 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   if (ToD)
 return ToD;
 
-  bool IsFriendTemplate = D->getFriendObjectKind() != Decl::FOK_None;
-  bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
+  // Should check if a declaration is friend in a dependent context.
+  // Such templates are not linked together in a declaration chain.
+  // The ASTImporter strategy is to map existing forward declarations to
+  // imported ones only if strictly necessary, otherwise import these as new
+  // forward declarations. In case of the "dependent friend" declarations, new
+  // declarations are created, but not linked in a declaration chain.
+  auto IsDependentFriend = [](ClassTemplateDecl *TD) {
+bool IsFriendTemplate = TD->getFriendObjectKind() != Decl::FOK_None;
+DeclContext *DC = TD->getDeclContext();
+DeclContext *LexicalDC = TD->getLexicalDeclContext();
+bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
 : DC->isDependentContext();
-  bool DependentFriend = IsFriendTemplate && IsDependentContext;
+return IsFriendTemplate && IsDependentContext;
+  };
+  bool DependentFriend = IsDependentFriend(D);
 
   ClassTemplateDecl *FoundByLookup = nullptr;
 
   // We may already have a template of the same name; try to find and match it.
-  if (!DependentFriend && !DC->isFunctionOrMethod()) {
+  if (!DC->isFunctionOrMethod()) {
 SmallVector ConflictingDecls;
 auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
 for (auto *FoundDecl : FoundDecls) {
@@ -5943,10 +5954,13 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
 
 // FIXME: sufficient conditon for 'IgnoreTemplateParmDepth'?
 bool IgnoreTemplateParmDepth =
-FoundTemplate->getFriendObjectKind() != Decl::FOK_None &&
-!D->specializations().empty();
+(FoundTemplate->getFriendObjectKind() != Decl::FOK_None) !=
+(D->getFriendObjectKind() != Decl::FOK_None);
 if (IsStructuralMatch(D, FoundTemplate, /*Complain=*/true,
   IgnoreTemplateParmDepth)) {
+  if (DependentFriend || IsDependentFriend(FoundTemplate))
+continue;
+
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);
   if (D->isThisDeclarationADefinition() && TemplateWithDef)
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf..f9ee73626c948 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4528,6 +4528,168 @@ TEST_P(ImportFriendClasses, 
ImportOfClassDefinitionAndFwdFriendShouldBeLinked) {
   EXPECT_EQ(ImportedFwd, ImportedDef->getPreviousDecl());
 }
 
+TEST_P(ImportFriendClasses,
+   ImportFriendTemplatesInDependentContext_DefToFriend) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template
+  struct X {
+template
+friend struct Y;
+  };
+  )",
+  Lang_CXX03);
+  auto *ToYFriend = FirstDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+  R"(
+  template
+  struct Y {};
+  )",
+  Lang_CXX03, "input0.cc");
+  auto *FromYDef = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("Y")));
+  auto *ImportedYDef = Import(FromYDef, Lang_CXX03);
+  EXPECT_TRUE(ImportedYDef);
+  EXPECT_FALSE(ImportedYDef->getPreviousDecl());
+  EXPECT_NE(ImportedYDef, ToYFriend);
+}
+
+TEST_P(ImportFriendClasses,
+   ImportFriendTemplatesInDependentContext_DefToFriend_NE) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template
+  struct X {
+template
+friend struct Y;
+  };
+  )",
+  Lang_CXX03);
+  auto *ToYFriend = FirstDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("Y")));
+  Decl *FromTU = getTuDecl(
+  R"(
+  template
+  struct Y {};
+  )",
+  Lang_CXX03, "input0.cc

[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][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] 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] 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] 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][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; };
+ 

[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

[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

[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

[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][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][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]

@@ -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]

@@ -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]

@@ -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]

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) {

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)) {
+  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]

@@ -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]

@@ -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] 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][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][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 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] 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] 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]

@@ -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] 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][ASTImporter] Support Importer of BuiltinBitCastExpr (PR #74813)

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

@@ -9284,6 +9284,24 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   // EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
 }
 
+const internal::VariadicDynCastAllOfMatcher
+builtinBitCastExpr;
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportBuiltinBitCastExpr) {
+  const char *CodeFrom =
+  R"(
+  void foo(int T) { (void)__builtin_bit_cast(float, T); }
+  )";
+  Decl *FromTU = getTuDecl(CodeFrom, Lang_CXX20);
+  auto *FromFoo = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ToFoo = Import(FromFoo, Lang_CXX20);
+  EXPECT_TRUE(ToFoo);
+  auto *ToBuiltinBitCastExpr =
+  FirstDeclMatcher().match(ToFoo, 
builtinBitCastExpr());
+  EXPECT_TRUE(ToBuiltinBitCastExpr);
+}

balazske wrote:

Instead of this test a more simple test can be added in the `ImportExpr` 
section of **ASTImporterTest.cpp**.

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

[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)

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

balazske wrote:

The problem may be related to the fact that template parameter declarations can 
have the `TranslationUnitDecl` as parent until the template (with these 
parameters) is finally created. In the temporary phase the object 
(`TemplateTypeParmDecl`) is found with lookup if it has the same name as an 
other imported object. Does the crash happen if in the test code the 
`TypeAliasTemplateDecl` is replaced with a plain `ClassTemplateDecl` or 
`FunctionTemplateDecl`? If yes the change is needed at these import functions 
too.

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

[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)

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

balazske wrote:

The `VisitTypeAliasTemplateDecl` function should be re-designed to check for 
structural equivalence at import. The following test does not pass because an 
existing `TypeAliasTemplateDecl` declaration with the same name is always found 
and returned, without check for structural equivalence.
```
TEST_P(ASTImporterOptionSpecificTestBase, ImportTypeAliasTemplateDecl1) {
  const char *ToCode =
  R"(
  struct S;
  template 
  using Callable = S;
  )";
  const char *Code =
  R"(
  struct S;
  template 
  using Callable = S;
  )";
  Decl *ToTU = getToTuDecl(ToCode, Lang_CXX17);
  Decl *FromTU = getTuDecl(Code, Lang_CXX17);

  auto *FromCallable = FirstDeclMatcher().match(
  FromTU, typeAliasTemplateDecl(hasName("Callable")));

  auto *ToCallable = FirstDeclMatcher().match(
  ToTU, typeAliasTemplateDecl(hasName("Callable")));

  auto *ImportedCallable = Import(FromCallable, Lang_CXX17);
  EXPECT_TRUE(ImportedCallable);
  EXPECT_NE(ImportedCallable, ToCallable);
}
```

Additionally I discovered that import of `ClassTemplateDecl` is not correct 
too: If there is an object with the same name that is not a 
`ClassTemplateDecl`, it is just ignored at import. This is not correct, the 
existing object may cause name conflict (for example it can be a non-template 
`RecordDecl`). (I found this when checking the questions in my last comment.) 
This is an independent problem but should be fixed.

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

[clang] [clang][ASTImporter] add processing of SubstNonTypeTemplateParmExpr in isAncestorDeclContextOf (PR #74991)

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

@@ -9284,6 +9284,26 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   // EXPECT_EQ(ToF1Imported->getPreviousDecl(), ToF1);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportFunctionAutoType) {

balazske wrote:

This test belongs to `ImportAutoFunctions` and a better name is 
`ReturnWithSubstNonTypeTemplateParmExpr`.

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

[clang] [clang][ASTImporter] Import AlignValueAttr correctly. (PR #75308)

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

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

Expression of attribute `align_value` was not imported. Import of the attribute 
is corrected, a test for it is added, other related tests with FIXME are 
updated.
Fixes #75054.

From 2e6fe315bdebea705d84b4152a831e5934b659eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Wed, 13 Dec 2023 10:23:48 +0100
Subject: [PATCH] [clang][ASTImporter] Import AlignValueAttr correctly.

Expression of attribute `align_value` was not imported.
Import of the attribute is corrected, a test for it is added,
other related tests with FIXME are updated.
---
 clang/lib/AST/ASTImporter.cpp   |  6 +++
 clang/unittests/AST/ASTImporterTest.cpp | 69 +
 2 files changed, 30 insertions(+), 45 deletions(-)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..cc29f4356ad755 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -9101,6 +9101,12 @@ Expected ASTImporter::Import(const Attr 
*FromAttr) {
 break;
   }
 
+  case attr::AlignValue: {
+auto *From = cast(FromAttr);
+AI.importAttr(From, AI.importArg(From->getAlignment()).value());
+break;
+  }
+
   case attr::Format: {
 const auto *From = cast(FromAttr);
 AI.importAttr(From, Import(From->getType()), From->getFormatIdx(),
diff --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 4dd7510bf8ddf8..da47e6b6653095 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -7425,67 +7425,46 @@ void ImportAttributes::checkImported(const Decl 
*From, const Decl *To) {
 ToAST->getASTContext().getTranslationUnitDecl());
 }
 
-// FIXME: Use ImportAttributes for this test.
-TEST_P(ASTImporterOptionSpecificTestBase, ImportExprOfAlignmentAttr) {
-  // Test if import of these packed and aligned attributes does not trigger an
-  // error situation where source location from 'From' context is referenced in
-  // 'To' context through evaluation of the alignof attribute.
-  // This happens if the 'alignof(A)' expression is not imported correctly.
-  Decl *FromTU = getTuDecl(
+TEST_P(ImportAttributes, ImportAligned) {
+  AlignedAttr *FromAttr, *ToAttr;
+  importAttr(
   R"(
   struct __attribute__((packed)) A { int __attribute__((aligned(8))) X; };
-  struct alignas(alignof(A)) S {};
+  struct alignas(alignof(A)) test {};
   )",
-  Lang_CXX11, "input.cc");
-  auto *FromD = FirstDeclMatcher().match(
-  FromTU, cxxRecordDecl(hasName("S"), unless(isImplicit(;
-  ASSERT_TRUE(FromD);
-
-  auto *ToD = Import(FromD, Lang_CXX11);
-  ASSERT_TRUE(ToD);
-
-  auto *FromAttr = FromD->getAttr();
-  auto *ToAttr = ToD->getAttr();
-  EXPECT_EQ(FromAttr->isInherited(), ToAttr->isInherited());
-  EXPECT_EQ(FromAttr->isPackExpansion(), ToAttr->isPackExpansion());
-  EXPECT_EQ(FromAttr->isImplicit(), ToAttr->isImplicit());
-  EXPECT_EQ(FromAttr->getSyntax(), ToAttr->getSyntax());
-  EXPECT_EQ(FromAttr->getSemanticSpelling(), ToAttr->getSemanticSpelling());
-  EXPECT_TRUE(ToAttr->getAlignmentExpr());
+  FromAttr, ToAttr);
+  checkImported(FromAttr->getAlignmentExpr(), ToAttr->getAlignmentExpr());
 
   auto *ToA = FirstDeclMatcher().match(
-  ToD->getTranslationUnitDecl(),
+  ToAST->getASTContext().getTranslationUnitDecl(),
   cxxRecordDecl(hasName("A"), unless(isImplicit(;
   // Ensure that 'struct A' was imported (through reference from attribute of
   // 'S').
   EXPECT_TRUE(ToA);
 }
 
-// FIXME: Use ImportAttributes for this test.
-TEST_P(ASTImporterOptionSpecificTestBase, ImportFormatAttr) {
-  Decl *FromTU = getTuDecl(
+TEST_P(ImportAttributes, ImportAlignValue) {
+  AlignValueAttr *FromAttr, *ToAttr;
+  importAttr(
+  R"(
+  void *test __attribute__((align_value(64)));
+  )",
+  FromAttr, ToAttr);
+  checkImported(FromAttr->getAlignment(), ToAttr->getAlignment());
+}
+
+TEST_P(ImportAttributes, ImportFormat) {
+  FormatAttr *FromAttr, *ToAttr;
+  importAttr(
   R"(
-  int foo(const char * fmt, ...)
+  int test(const char * fmt, ...)
   __attribute__ ((__format__ (__scanf__, 1, 2)));
   )",
-  Lang_CXX03, "input.cc");
-  auto *FromD = FirstDeclMatcher().match(
-  FromTU, functionDecl(hasName("foo")));
-  ASSERT_TRUE(FromD);
+  FromAttr, ToAttr);
 
-  auto *ToD = Import(FromD, Lang_CXX03);
-  ASSERT_TRUE(ToD);
-  ToD->dump(); // Should not crash!
-
-  auto *FromAttr = FromD->getAttr();
-  auto *ToAttr = ToD->getAttr();
-  EXPECT_EQ(FromAttr->isInherited(), ToAttr->isInherited());
-  EXPECT_EQ(FromAttr->isPackExpansion(), ToAttr->isPackExpansion());
-  EXPECT_EQ(FromAttr->isImplicit(), ToAttr->isImplicit());
-  EXPECT_EQ(FromAttr->getSyntax(), ToAttr->getSyntax());
-  EXPECT_EQ(FromAttr->getAttributeSpellingListIndex(),
-ToAttr->getAttributeSpellingListIndex());
   EXPECT_EQ(F

[clang] [clang][ASTImporter] skip TemplateTypeParmDecl in VisitTypeAliasTemplateDecl (PR #74919)

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

balazske wrote:

`VisitTypeAliasTemplateDecl` looks wrong and some fix should be made, this PR 
is good for the fix. This import function is different from the others because 
`ConflictingDecls` is populated when an object with different type is found, 
but this is the correct way. I think that `TemplateTypeParmDecl` objects are 
found with the lookup. A good fix would be to change the loop in 
`VisitTypeAliasTemplateDecl` to check for structural equivalence too, if 
equivalent is found use `MapImported`, otherwise add to `ConflictingDecls`. But 
skip all found objects that are not `TypeAliasTemplateDecl`. This makes it 
similar to the others, even if not correct, the remaining problem can be fixed 
later (and looks less important).

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