[clang] 178c2b4 - Correctly diagnose taking the address of a register variable in C

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2021-07-22T14:53:23-04:00
New Revision: 178c2b4c1eb12b2153adb384ac7f22a8791edc86

URL: https://github.com/llvm/llvm-project/commit/178c2b4c1eb12b2153adb384ac7f22a8791edc86
DIFF: https://github.com/llvm/llvm-project/commit/178c2b4c1eb12b2153adb384ac7f22a8791edc86.diff

LOG: Correctly diagnose taking the address of a register variable in C

We caught the cases where the user would explicitly use the & operator,
but we were missing implicit conversions such as array decay.

Fixes PR26336. Thanks to Samuel Neves for inspiration for the patch.

Added: 
    

Modified: 
    clang/lib/Sema/Sema.cpp
    clang/lib/Sema/SemaExpr.cpp
    clang/test/Sema/expr-address-of.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index fbbb347f57da..054a65265079 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -629,16 +629,36 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
   if (ExprTy == TypeTy)
     return E;
 
-  // C++1z [conv.array]: The temporary materialization conversion is applied.
-  // We also use this to fuel C++ DR1213, which applies to C++11 onwards.
-  if (Kind == CK_ArrayToPointerDecay && getLangOpts().CPlusPlus &&
-      E->getValueKind() == VK_PRValue) {
-    // The temporary is an lvalue in C++98 and an xvalue otherwise.
-    ExprResult Materialized = CreateMaterializeTemporaryExpr(
-        E->getType(), E, !getLangOpts().CPlusPlus11);
-    if (Materialized.isInvalid())
-      return ExprError();
-    E = Materialized.get();
+  if (Kind == CK_ArrayToPointerDecay) {
+    // C++1z [conv.array]: The temporary materialization conversion is applied.
+    // We also use this to fuel C++ DR1213, which applies to C++11 onwards.
+    if (getLangOpts().CPlusPlus && E->getValueKind() == VK_PRValue) {
+      // The temporary is an lvalue in C++98 and an xvalue otherwise.
+      ExprResult Materialized = CreateMaterializeTemporaryExpr(
+          E->getType(), E, !getLangOpts().CPlusPlus11);
+      if (Materialized.isInvalid())
+        return ExprError();
+      E = Materialized.get();
+    }
+    // C17 6.7.1p6 footnote 124: The implementation can treat any register
+    // declaration simply as an auto declaration. However, whether or not
+    // addressable storage is actually used, the address of any part of an
+    // object declared with storage-class specifier register cannot be
+    // computed, either explicitly(by use of the unary & operator as discussed
+    // in 6.5.3.2) or implicitly(by converting an array name to a pointer as
+    // discussed in 6.3.2.1).Thus, the only operator that can be applied to an
+    // array declared with storage-class specifier register is sizeof.
+    if (VK == VK_PRValue && !getLangOpts().CPlusPlus && !E->isPRValue()) {
+      if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+        if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+          if (VD->getStorageClass() == SC_Register) {
+            Diag(E->getExprLoc(), diag::err_typecheck_address_of)
+                << /*register variable*/ 3 << E->getSourceRange();
+            return ExprError();
+          }
+        }
+      }
+    }
   }
 
   if (ImplicitCastExpr *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3926c49077ce..d19eb8475e9f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -525,9 +525,13 @@ ExprResult Sema::DefaultFunctionArrayConversion(Expr *E, bool Diagnose) {
     // An lvalue or rvalue of type "array of N T" or "array of unknown bound of
     // T" can be converted to an rvalue of type "pointer to T".
     //
-    if (getLangOpts().C99 || getLangOpts().CPlusPlus || E->isLValue())
-      E = ImpCastExprToType(E, Context.getArrayDecayedType(Ty),
-                            CK_ArrayToPointerDecay).get();
+    if (getLangOpts().C99 || getLangOpts().CPlusPlus || E->isLValue()) {
+      ExprResult Res = ImpCastExprToType(E, Context.getArrayDecayedType(Ty),
+                                         CK_ArrayToPointerDecay);
+      if (Res.isInvalid())
+        return ExprError();
+      E = Res.get();
+    }
   }
   return E;
 }

diff  --git a/clang/test/Sema/expr-address-of.c b/clang/test/Sema/expr-address-of.c
index 480871afad2a..aed488284ab1 100644
--- a/clang/test/Sema/expr-address-of.c
+++ b/clang/test/Sema/expr-address-of.c
@@ -45,24 +45,27 @@ void f0() {
   int *_dummy1 = &(*(x1 + 1));
 }
 
-// FIXME: The checks for this function are broken; we should error
-// on promoting a register array to a pointer! (C99 6.3.2.1p3)
 void f1() {
   register int x0[10];
-  int *_dummy00 = x0; // fixme-error {{address of register variable requested}}
-  int *_dummy01 = &(*x0); // fixme-error {{address of register variable requested}}
+  int *_dummy00 = x0;     // expected-error {{address of register variable requested}}
+  int *_dummy01 = &(*x0); // expected-error {{address of register variable requested}}
 
   register int x1[10];
-  int *_dummy1 = &(*(x1 + 1)); // fixme-error {{address of register variable requested}}
+  int *_dummy1 = &(*(x1 + 1)); // expected-error {{address of register variable requested}}
 
   register int *x2;
   int *_dummy2 = &(*(x2 + 1));
 
   register int x3[10][10][10];
-  int (*_dummy3)[10] = &x3[0][0]; // expected-error {{address of register variable requested}}
+  int(*_dummy3)[10] = &x3[0][0]; // expected-error {{address of register variable requested}}
 
   register struct { int f0[10]; } x4;
   int *_dummy4 = &x4.f0[2]; // expected-error {{address of register variable requested}}
+
+  add_one(x0);      // expected-error {{address of register variable requested}}
+  (void)sizeof(x0); // OK, not an array decay.
+
+  int *p = ((int *)x0)++; // expected-error {{address of register variable requested}}
 }
 
 void f2() {
@@ -86,12 +89,8 @@ void f4() {
 void f5() {
   register int arr[2];
 
-  /* This is just here because if we happened to support this as an
-     lvalue we would need to give a warning. Note that gcc warns about
-     this as a register before it warns about it as an invalid
-     lvalue. */
-  int *_dummy0 = &(int*) arr; // expected-error {{cannot take the address of an rvalue}}
-  int *_dummy1 = &(arr + 1); // expected-error {{cannot take the address of an rvalue}}
+  int *_dummy0 = &(int*) arr; // expected-error {{address of register variable requested}}
+  int *_dummy1 = &(arr + 1); // expected-error {{address of register variable requested}}
 }
 
 void f6(register int x) {