[clang] [clang][Sema] Propagate qualifiers during derived-to-base conversion (PR #127824)

[Antonio Frighetto via cfe-commits]

https://github.com/antoniofrighetto updated https://github.com/llvm/llvm-project/pull/127824

>From aebd5455e9cf583b9f5a29c68d5217f49c7a49b5 Mon Sep 17 00:00:00 2001
From: Antonio Frighetto <me at antoniofrighetto.com>
Date: Wed, 19 Feb 2025 16:47:18 +0100
Subject: [PATCH 1/2] [clang][Sema] Propagate qualifiers during derived-to-base
 conversion

When accessing a field member through a derived-to-base conversion,
ensure qualifiers are propagated to the base class subobject.

Fixes: https://github.com/llvm/llvm-project/issues/127683.
---
 clang/docs/ReleaseNotes.rst                   |  2 ++
 clang/lib/Sema/SemaExpr.cpp                   |  6 ++--
 clang/test/CodeGenCXX/derived-to-base.cpp     | 25 ++++++++++++++
 .../SemaCXX/derived-to-base-cv-qualifiers.cpp | 34 +++++++++++++++++++
 4 files changed, 65 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/SemaCXX/derived-to-base-cv-qualifiers.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 62a64ff57599d..c96c79c8e6d6c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -171,6 +171,8 @@ Bug Fixes to C++ Support
 - The initialization kind of elements of structured bindings
   direct-list-initialized from an array is corrected to direct-initialization.
 - Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327)
+- Clang was previously coalescing volatile writes to members of volatile base class subobjects.
+  The issue has been addressed by propagating qualifiers during derived-to-base conversions in the AST. (#GH127824)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fad15bf95c415..c4d18609316ab 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3107,8 +3107,10 @@ Sema::PerformObjectMemberConversion(Expr *From,
                                    /*IgnoreAccess=*/true))
     return ExprError();
 
-  return ImpCastExprToType(From, DestType, CK_UncheckedDerivedToBase,
-                           VK, &BasePath);
+  DestType = Context.getQualifiedType(DestType, FromType.getQualifiers());
+
+  return ImpCastExprToType(From, DestType, CK_UncheckedDerivedToBase, VK,
+                           &BasePath);
 }
 
 bool Sema::UseArgumentDependentLookup(const CXXScopeSpec &SS,
diff --git a/clang/test/CodeGenCXX/derived-to-base.cpp b/clang/test/CodeGenCXX/derived-to-base.cpp
index c8dbd5bf5cb05..c09a12b728bbb 100644
--- a/clang/test/CodeGenCXX/derived-to-base.cpp
+++ b/clang/test/CodeGenCXX/derived-to-base.cpp
@@ -46,4 +46,29 @@ namespace test3 {
   }
 }
 
+// Ensure volatile is preserved during derived-to-base conversion. 
+namespace PR127683 {
+
+struct Base {
+  int Val;
+};
+  
+struct Derived : Base { };
+  
+volatile Derived Obj;
+
+// CHECK-LABEL: define void @_ZN8PR12768319test_volatile_storeEv()
+// CHECK:         store volatile i32 0, ptr @_ZN8PR1276833ObjE, align 4
+void test_volatile_store() {
+  Obj.Val = 0;
+}
+
+// CHECK-LABEL: define void @_ZN8PR12768318test_volatile_loadEv()
+// CHECK:         %0 = load volatile i32, ptr @_ZN8PR1276833ObjE, align 4
+void test_volatile_load() {
+  [[maybe_unused]] int Val = Obj.Val;
+}
+
+}
+
 // CHECK: attributes [[NUW]] = { mustprogress noinline nounwind{{.*}} }
diff --git a/clang/test/SemaCXX/derived-to-base-cv-qualifiers.cpp b/clang/test/SemaCXX/derived-to-base-cv-qualifiers.cpp
new file mode 100644
index 0000000000000..02a5bf8f6cecf
--- /dev/null
+++ b/clang/test/SemaCXX/derived-to-base-cv-qualifiers.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -ast-dump %s | FileCheck %s
+
+// Ensure volatile is preserved during derived-to-base conversion. 
+namespace PR127683 {
+
+struct Base {
+  int Val;
+};
+
+struct Derived : Base { };
+
+volatile Derived Obj;
+
+// CHECK:      |-FunctionDecl {{.*}} test_volatile_store 'void ()'
+// CHECK-NEXT:   `-CompoundStmt {{.*}}
+// CHECK-NEXT:     `-BinaryOperator {{.*}} 'volatile int' lvalue '='
+// CHECK-NEXT:       |-MemberExpr {{.*}} 'volatile int' lvalue .Val
+// CHECK-NEXT:       | `-ImplicitCastExpr {{.*}} 'volatile PR127683::Base' lvalue <UncheckedDerivedToBase (Base)>
+void test_volatile_store() {
+  Obj.Val = 0;
+}
+
+// CHECK:      `-FunctionDecl {{.*}} test_volatile_load 'void ()'
+// CHECK-NEXT:   `-CompoundStmt {{.*}}
+// CHECK-NEXT:     `-DeclStmt {{.*}}
+// CHECK-NEXT:       `-VarDecl {{.*}} Val 'int' cinit
+// CHECK-NEXT:         |-ImplicitCastExpr {{.*}} 'int' <LValueToRValue>
+// CHECK-NEXT:         | `-MemberExpr {{.*}} 'volatile int' lvalue .Val
+// CHECK-NEXT:         |   `-ImplicitCastExpr {{.*}} 'volatile PR127683::Base' lvalue <UncheckedDerivedToBase (Base)>
+void test_volatile_load() {
+  [[maybe_unused]] int Val = Obj.Val;
+}
+
+}

>From da6e4602f7381946d8a0c0d0d41b89ffc951de6a Mon Sep 17 00:00:00 2001
From: Antonio Frighetto <me at antoniofrighetto.com>
Date: Sat, 22 Feb 2025 16:42:36 +0100
Subject: [PATCH 2/2] !fixup fix ci issue

---
 clang/lib/Sema/SemaExpr.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c4d18609316ab..54df2eaf12115 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3107,7 +3107,7 @@ Sema::PerformObjectMemberConversion(Expr *From,
                                    /*IgnoreAccess=*/true))
     return ExprError();
 
-  DestType = Context.getQualifiedType(DestType, FromType.getQualifiers());
+  DestType.withCVRQualifiers(FromType.getCVRQualifiers());
 
   return ImpCastExprToType(From, DestType, CK_UncheckedDerivedToBase, VK,
                            &BasePath);