https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/214208
From 7fb2db93f71611a793492e12b05d5ce781f81fa0 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Wed, 5 Aug 2026 12:36:10 +0100 Subject: [PATCH 1/3] [analyzer] Order UninitializedObject notes deterministically 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 --- .../UninitializedObjectChecker.cpp | 37 ++++++++- .../cxx-uninitialized-object-note-order.cpp | 75 +++++++++++++++++++ 2 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 clang/test/Analysis/cxx-uninitialized-object-note-order.cpp 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' + From 62a8832070b75c2ba717868362d61b0ea4001f0a Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Wed, 5 Aug 2026 15:19:45 +0100 Subject: [PATCH 2/3] NFC Reshuffle the comments; add clarifications --- .../UninitializedObjectChecker.cpp | 18 ++++-------------- .../cxx-uninitialized-object-note-order.cpp | 5 +++++ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp index 6585a4e0b0df8..2137b649f7541 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -215,20 +215,10 @@ void UninitializedObjectChecker::checkEndFunction( 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. + // 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; }); diff --git a/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp b/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp index 9557028f628fc..7982d0b07209e 100644 --- a/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp +++ b/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp @@ -5,6 +5,10 @@ // RUN: %{run} -verify // RUN: %{run} 2>&1 | FileCheck %s +// ATTENTION: +// We use FileCheck to ensure that the relative order of the notes are in the expected order. +// These notes used to be emitted in a non-deterministic order, which wasn't checked by `-verify`. + struct MultipleSiblings { int a; // expected-note {{uninitialized field 'this->a'}} int b; // expected-note {{uninitialized field 'this->b'}} @@ -66,6 +70,7 @@ void fTwoInstances() { // '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' From e01951e48c847e04afa785728912b318d885d367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Benics?= <[email protected]> Date: Wed, 5 Aug 2026 15:57:59 +0100 Subject: [PATCH 3/3] Update clang/test/Analysis/cxx-uninitialized-object-note-order.cpp Co-authored-by: Arseniy Zaostrovnykh <[email protected]> --- clang/test/Analysis/cxx-uninitialized-object-note-order.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp b/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp index 7982d0b07209e..6221d224db912 100644 --- a/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp +++ b/clang/test/Analysis/cxx-uninitialized-object-note-order.cpp @@ -6,8 +6,8 @@ // RUN: %{run} 2>&1 | FileCheck %s // ATTENTION: -// We use FileCheck to ensure that the relative order of the notes are in the expected order. -// These notes used to be emitted in a non-deterministic order, which wasn't checked by `-verify`. +// 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'}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
