llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Balázs Benics (steakhal) <details> <summary>Changes</summary> UninitFieldMap is keyed by FieldRegion pointers, so iterating it to emit the notes ordered them by where those regions happened to be allocated. The order therefore varied between runs: on cxx-uninitialized-object.cpp the two notes of the report at line 363 swapped in roughly 3 of 12 runs. - Emit the notes in source order instead, tie-broken by the note message. - -verify matches notes by line, text and count and ignores their order, so the new test pins the order down with FileCheck. Assisted-By: claude --- Full diff: https://github.com/llvm/llvm-project/pull/214208.diff 2 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (+33-4) - (added) clang/test/Analysis/cxx-uninitialized-object-note-order.cpp (+75) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp index 4d54f11efe158..6585a4e0b0df8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -201,11 +201,40 @@ void UninitializedObjectChecker::checkEndFunction( BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, Node->getStackFrame()->getDecl()); - for (const auto &Pair : UninitFields) { - Report->addNote(Pair.second, - PathDiagnosticLocation::create(Pair.first->getDecl(), - Context.getSourceManager())); + using NoteTy = std::pair<PathDiagnosticLocation, StringRef>; + SmallVector<NoteTy> Notes; + const auto &SM = Context.getSourceManager(); + for (const auto &[FieldRegion, NoteMsg] : UninitFields) { + auto FieldLoc = PathDiagnosticLocation::create(FieldRegion->getDecl(), SM); + Notes.emplace_back(FieldLoc, NoteMsg); } + + // Make the order deterministic. + llvm::sort(Notes, [](const NoteTy &LHS, const NoteTy &RHS) { + FullSourceLoc L = LHS.first.asLocation(); + FullSourceLoc R = RHS.first.asLocation(); + if (L != R) + return L.isBeforeInTranslationUnitThan(R); + // Comparing the field locs might not be enough: + // struct TwoInstances { + // Inner first; + // Inner second; + // int z; + // TwoInstances() { z = 0; } // warn: 4 uninitialized fields + // }; + // Then creating an instance of `TwoInstances` would trigger 4 notes: + // - note: uninitialized field 'this->first.x' <-- FieldDecl{Inner.x} + // - note: uninitialized field 'this->second.x' <-- FieldDecl{Inner.x} + // - note: uninitialized field 'this->first.y' <-- FieldDecl{Inner.y} + // - note: uninitialized field 'this->second.y' <-- FieldDecl{Inner.y} + // Note that the FieldDecls are pairwise the same, thus we need a + // tie breaker: the note message. + return LHS.second < RHS.second; + }); + + for (const auto &[Loc, NoteMsg] : Notes) + Report->addNote(NoteMsg, Loc); + Context.emitReport(std::move(Report)); } diff --git a/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp b/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp new file mode 100644 index 0000000000000..9557028f628fc --- /dev/null +++ b/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp @@ -0,0 +1,75 @@ +// DEFINE: %{run} = %clang_analyze_cc1 \ +// DEFINE: -analyzer-checker=core,optin.cplusplus.UninitializedObject \ +// DEFINE: -analyzer-output=text -fno-caret-diagnostics %s + +// RUN: %{run} -verify +// RUN: %{run} 2>&1 | FileCheck %s + +struct MultipleSiblings { + int a; // expected-note {{uninitialized field 'this->a'}} + int b; // expected-note {{uninitialized field 'this->b'}} + int c; // expected-note {{uninitialized field 'this->c'}} + int d; + MultipleSiblings() { d = 0; } + // expected-warning@-1 {{3 uninitialized fields}} + // expected-note@-2 {{3 uninitialized fields}} +}; + +void fMultipleSiblings() { + MultipleSiblings s; // expected-note {{Calling default constructor for 'MultipleSiblings'}} +} + +// CHECK-LABEL: warning: 3 uninitialized fields at the end of the constructor call +// CHECK-NEXT: note: uninitialized field 'this->a' +// CHECK-NEXT: note: uninitialized field 'this->b' +// CHECK-NEXT: note: uninitialized field 'this->c' + +struct Inner { + int x; + // expected-note@-1 {{uninitialized field 'this->i.x'}} + // expected-note@-2 {{uninitialized field 'this->first.x'}} + // expected-note@-3 {{uninitialized field 'this->second.x'}} + int y; + // expected-note@-1 {{uninitialized field 'this->i.y'}} + // expected-note@-2 {{uninitialized field 'this->first.y'}} + // expected-note@-3 {{uninitialized field 'this->second.y'}} +}; + +struct Nested { + Inner i; + int z; + Nested() { z = 0; } + // expected-warning@-1 {{2 uninitialized fields}} + // expected-note@-2 {{2 uninitialized fields}} +}; + +void fNested() { + Nested n; // expected-note {{Calling default constructor for 'Nested'}} +} + +// CHECK-LABEL: warning: 2 uninitialized fields at the end of the constructor call +// CHECK-NEXT: note: uninitialized field 'this->i.x' +// CHECK-NEXT: note: uninitialized field 'this->i.y' + +struct TwoInstances { + Inner first; + Inner second; + int z; + TwoInstances() { z = 0; } + // expected-warning@-1 {{4 uninitialized fields}} + // expected-note@-2 {{4 uninitialized fields}} +}; + +void fTwoInstances() { + TwoInstances t; // expected-note {{Calling default constructor for 'TwoInstances'}} +} + +// 'first' and 'second' have the same type, so all four notes point at the two members of Inner. +// Ordering by source location alone does not separate them. + +// CHECK-LABEL: warning: 4 uninitialized fields at the end of the constructor call +// CHECK-NEXT: note: uninitialized field 'this->first.x' +// CHECK-NEXT: note: uninitialized field 'this->second.x' +// CHECK-NEXT: note: uninitialized field 'this->first.y' +// CHECK-NEXT: note: uninitialized field 'this->second.y' + `````````` </details> https://github.com/llvm/llvm-project/pull/214208 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
