llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (Seraphimt) <details> <summary>Changes</summary> According to standards volatile accesses is sideeffects and multiple unsequenced volatile accesses same variable is UB. --- Full diff: https://github.com/llvm/llvm-project/pull/180955.diff 3 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2) - (modified) clang/lib/Sema/SemaChecking.cpp (+30-12) - (modified) clang/test/SemaCXX/warn-unsequenced.cpp (+26) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f12677ac11600..defd168b2c629 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2581,6 +2581,8 @@ def warn_unsequenced_mod_mod : Warning< "multiple unsequenced modifications to %0">, InGroup<Unsequenced>; def warn_unsequenced_mod_use : Warning< "unsequenced modification and access to %0">, InGroup<Unsequenced>; +def warn_unsequenced_use_use_volatile : Warning< + "unsequenced volatile accesses to %0">, InGroup<Unsequenced>; def select_initialized_entity_kind : TextSubstitution< "%select{copying variable|copying parameter|initializing template parameter|" diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 89171246d0bcb..b9d1ae9240923 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -14015,7 +14015,7 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> { }; /// An object for which we can track unsequenced uses. - using Object = const NamedDecl *; + using Object = const ValueDecl *; /// Different flavors of object usage which we track. We only track the /// least-sequenced usage of each kind. @@ -14034,6 +14034,12 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> { UK_Count = UK_ModAsSideEffect + 1 }; + enum WarningKind { + WK_UseAndMod, + WK_ModAndMod, + WK_Volatile_UseAndUse + }; + /// Bundle together a sequencing region and the expression corresponding /// to a specific usage. One Usage is stored for each usage kind in UsageInfo. struct Usage { @@ -14179,7 +14185,7 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> { /// \p IsModMod is true when we are checking for a mod-mod unsequenced /// usage and false we are checking for a mod-use unsequenced usage. void checkUsage(Object O, UsageInfo &UI, const Expr *UsageExpr, - UsageKind OtherKind, bool IsModMod) { + UsageKind OtherKind, WarningKind WarnKind) { if (UI.Diagnosed) return; @@ -14192,11 +14198,22 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> { if (OtherKind == UK_Use) std::swap(Mod, ModOrUse); + unsigned DiagID = 0; + switch (WarnKind) { + case WK_UseAndMod: + DiagID = diag::warn_unsequenced_mod_use; + break; + case WK_ModAndMod: + DiagID = diag::warn_unsequenced_mod_mod; + break; + case WK_Volatile_UseAndUse: + DiagID = diag::warn_unsequenced_use_use_volatile; + break; + } + SemaRef.DiagRuntimeBehavior( Mod->getExprLoc(), {Mod, ModOrUse}, - SemaRef.PDiag(IsModMod ? diag::warn_unsequenced_mod_mod - : diag::warn_unsequenced_mod_use) - << O << SourceRange(ModOrUse->getExprLoc())); + SemaRef.PDiag(DiagID)<< O << SourceRange(ModOrUse->getExprLoc())); UI.Diagnosed = true; } @@ -14229,27 +14246,28 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> { void notePreUse(Object O, const Expr *UseExpr) { UsageInfo &UI = UsageMap[O]; // Uses conflict with other modifications. - checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsValue, /*IsModMod=*/false); + checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsValue, WK_UseAndMod); + // Volatile uses conflict with other uses. + if (O->getType().isVolatileQualified()) + checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_Use, WK_Volatile_UseAndUse); } void notePostUse(Object O, const Expr *UseExpr) { UsageInfo &UI = UsageMap[O]; - checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsSideEffect, - /*IsModMod=*/false); + checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsSideEffect, WK_UseAndMod); addUsage(O, UI, UseExpr, /*UsageKind=*/UK_Use); } void notePreMod(Object O, const Expr *ModExpr) { UsageInfo &UI = UsageMap[O]; // Modifications conflict with other modifications and with uses. - checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsValue, /*IsModMod=*/true); - checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_Use, /*IsModMod=*/false); + checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsValue, WK_ModAndMod); + checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_Use, WK_UseAndMod); } void notePostMod(Object O, const Expr *ModExpr, UsageKind UK) { UsageInfo &UI = UsageMap[O]; - checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsSideEffect, - /*IsModMod=*/true); + checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsSideEffect, WK_ModAndMod); addUsage(O, UI, ModExpr, /*UsageKind=*/UK); } diff --git a/clang/test/SemaCXX/warn-unsequenced.cpp b/clang/test/SemaCXX/warn-unsequenced.cpp index 50dde8f3a5789..3b4a8ec9380d0 100644 --- a/clang/test/SemaCXX/warn-unsequenced.cpp +++ b/clang/test/SemaCXX/warn-unsequenced.cpp @@ -815,3 +815,29 @@ void test_var() { } } // namespace templates + +namespace muliple_read_volatile { + volatile int v1; + + void PositiveTest(){ + int x = 0; + int y = 0; + x = v1 + v1; // cxx11-warning {{unsequenced accesses to volatile qualified 'v1'}} + // cxx17-warning@-1 {{unsequenced accesses to volatile qualified 'v1'}} + v1 = v1 * v1; // cxx11-warning {{unsequenced accesses to volatile qualified 'v1'}} + // cxx17-warning@-1 {{unsequenced accesses to volatile qualified 'v1'}} + x = v1 + (y++, v1); // cxx11-warning {{unsequenced accesses to volatile qualified 'v1'}} + // cxx17-warning@-1 {{unsequenced accesses to volatile qualified 'v1'}} + x = v1 + v1 || y; // cxx11-warning {{unsequenced accesses to volatile qualified 'v1'}} + // cxx17-warning@-1 {{unsequenced accesses to volatile qualified 'v1'}} + } + + void NegativeTest(){ + int x = 0; + int y = 0; + x = v1 + y; // no-warning + v1 = v1 * y; // no-warning + x = (v1, v1); // no-warning + x = v1 || v1; // no-warning + } +} // namespace muliple_read_volatile \ No newline at end of file `````````` </details> https://github.com/llvm/llvm-project/pull/180955 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
