aaron.ballman created this revision.
aaron.ballman added reviewers: nickdesaulniers, erichkeane, rjmccall, efriedma.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.
The C standard hints in a footnote attached to C17 6.7.3p5 that `const` objects
which are not `volatile` can be placed in read-only memory, which implies that
`volatile` objects can never be placed there. GCC appears to be following that
behavior: https://godbolt.org/z/9WEq18TWz and this changes Clang to behave the
same.
Fixes #56468
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131012
Files:
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCXX/const-global-linkage.cpp
Index: clang/test/CodeGenCXX/const-global-linkage.cpp
===================================================================
--- clang/test/CodeGenCXX/const-global-linkage.cpp
+++ clang/test/CodeGenCXX/const-global-linkage.cpp
@@ -4,7 +4,7 @@
const int y = 20;
const volatile int z = 30;
// CHECK-NOT: @x
-// CHECK: @z = {{(dso_local )?}}constant i32 30
+// CHECK: @z = {{(dso_local )?}}global i32 30
// CHECK: @_ZL1y = internal constant i32 20
const int& b() { return y; }
@@ -12,6 +12,6 @@
const char z2[] = "zxcv";
const volatile char z3[] = "zxcv";
// CHECK-NOT: @z1
-// CHECK: @z3 = {{(dso_local )?}}constant
+// CHECK: @z3 = {{(dso_local )?}}global
// CHECK: @_ZL2z2 = internal constant
const char* b2() { return z2; }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4165,6 +4165,15 @@
if (!Ty.isConstant(Context) && !Ty->isReferenceType())
return false;
+ // If the type is also marked as volatile, it should not be treated as a
+ // constant according to C17 6.7.3p5 and footnote 133, though there does not
+ // appear to be a normative requirement. That said, GCC does not put the
+ // object into a read only section, and we're following suit. C++ says even
+ // less about this situation, but [decl.type.cv]p6 implies that when it comes
+ // to volatile, C++ follows C's lead, so we'll do the same here.
+ if (Ty.isVolatileQualified())
+ return false;
+
if (Context.getLangOpts().CPlusPlus) {
if (const CXXRecordDecl *Record
= Context.getBaseElementType(Ty)->getAsCXXRecordDecl())
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -52,6 +52,9 @@
- Fixes an accepts-invalid bug in C when using a ``_Noreturn`` function
specifier on something other than a function declaration. This fixes
`Issue 56800 <https://github.com/llvm/llvm-project/issues/56800>`_.
+- ``const volatile`` global variables are no longer placed in a read-only
+ section, but are instead treated as a non-const global. This fixes
+ `Issue 56468 <https://github.com/llvm/llvm-project/issues/56468>`_.
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Index: clang/test/CodeGenCXX/const-global-linkage.cpp
===================================================================
--- clang/test/CodeGenCXX/const-global-linkage.cpp
+++ clang/test/CodeGenCXX/const-global-linkage.cpp
@@ -4,7 +4,7 @@
const int y = 20;
const volatile int z = 30;
// CHECK-NOT: @x
-// CHECK: @z = {{(dso_local )?}}constant i32 30
+// CHECK: @z = {{(dso_local )?}}global i32 30
// CHECK: @_ZL1y = internal constant i32 20
const int& b() { return y; }
@@ -12,6 +12,6 @@
const char z2[] = "zxcv";
const volatile char z3[] = "zxcv";
// CHECK-NOT: @z1
-// CHECK: @z3 = {{(dso_local )?}}constant
+// CHECK: @z3 = {{(dso_local )?}}global
// CHECK: @_ZL2z2 = internal constant
const char* b2() { return z2; }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4165,6 +4165,15 @@
if (!Ty.isConstant(Context) && !Ty->isReferenceType())
return false;
+ // If the type is also marked as volatile, it should not be treated as a
+ // constant according to C17 6.7.3p5 and footnote 133, though there does not
+ // appear to be a normative requirement. That said, GCC does not put the
+ // object into a read only section, and we're following suit. C++ says even
+ // less about this situation, but [decl.type.cv]p6 implies that when it comes
+ // to volatile, C++ follows C's lead, so we'll do the same here.
+ if (Ty.isVolatileQualified())
+ return false;
+
if (Context.getLangOpts().CPlusPlus) {
if (const CXXRecordDecl *Record
= Context.getBaseElementType(Ty)->getAsCXXRecordDecl())
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -52,6 +52,9 @@
- Fixes an accepts-invalid bug in C when using a ``_Noreturn`` function
specifier on something other than a function declaration. This fixes
`Issue 56800 <https://github.com/llvm/llvm-project/issues/56800>`_.
+- ``const volatile`` global variables are no longer placed in a read-only
+ section, but are instead treated as a non-const global. This fixes
+ `Issue 56468 <https://github.com/llvm/llvm-project/issues/56468>`_.
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
