Author: Balázs Benics Date: 2026-08-05T15:48:19Z New Revision: c63b6d6d1709de8975d937d4965cb0d32ff1477a
URL: https://github.com/llvm/llvm-project/commit/c63b6d6d1709de8975d937d4965cb0d32ff1477a DIFF: https://github.com/llvm/llvm-project/commit/c63b6d6d1709de8975d937d4965cb0d32ff1477a.diff LOG: [analyzer] Order UninitializedObject notes deterministically (#214208) 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 Added: clang/test/Analysis/cxx-uninitialized-object-note-order.cpp Modified: clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp index 4d54f11efe158..2137b649f7541 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -201,11 +201,30 @@ 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 so we might need a tie + // breaker. + // See the `cxx-uninitialized-object-note-order.cpp:fTwoInstances` test + // demonstrating this. + 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..6221d224db912 --- /dev/null +++ b/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp @@ -0,0 +1,80 @@ +// 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 + +// ATTENTION: +// We use FileCheck to ensure that the relative order of the notes is stable. +// These notes used to be emitted in a non-deterministic order, which is not checked by `-verify`. + +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. +// Because of this, we sort the notes by the message as well as a tie breaker. + +// 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' + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
