[clang] e07ead8 - [Clang] Warn when trying to dereference void pointers in C

Jun Zhang via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 24 07:20:08 PDT 2022


Author: Jun Zhang
Date: 2022-09-24T22:18:04+08:00
New Revision: e07ead85a368173a56e96a21d6841aa497ad80f8

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

LOG: [Clang] Warn when trying to dereference void pointers in C

Previously we only have an extension that warn void pointer deferencing
in C++, but for C we did nothing.

C2x 6.5.3.2p4 says The unary * operator denotes indirection. If it points
to an object, the result is an lvalue designating the object. However, there
is no way to form an lvalue designating an object of an incomplete type as
6.3.2.1p1 says "an lvalue is an expression (with an object type other than
void)", so the behavior is undefined.

Fixes https://github.com/llvm/llvm-project/issues/53631

Signed-off-by: Jun Zhang <jun at junz.org>

Differential Revision: https://reviews.llvm.org/D134461

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/include/clang/Sema/Sema.h
    clang/lib/Parse/ParseExpr.cpp
    clang/lib/Sema/SemaExpr.cpp
    clang/test/Analysis/misc-ps.m
    clang/test/C/drs/dr0xx.c
    clang/test/C/drs/dr1xx.c
    clang/test/Sema/asm.c
    clang/test/Sema/builtins-arm.c
    clang/test/Sema/conditional-expr.c
    clang/test/Sema/deref.c
    clang/test/Sema/expr-address-of.c
    clang/test/Sema/i-c-e.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ed262823d004d..a5e72fc917f38 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -210,6 +210,8 @@ Improvements to Clang's diagnostics
   ``LL`` suffix.
 - Clang now correctly diagnoses index that refers past the last possible element
   of FAM-like arrays.
+- Clang now correctly diagnoses a warning when defercencing a void pointer in C mode.
+  This fixes `Issue 53631 <https://github.com/llvm/llvm-project/issues/53631>`_
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7647aa929b75e..834b60a2744d6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6919,7 +6919,7 @@ def err_typecheck_unary_expr : Error<
 def err_typecheck_indirection_requires_pointer : Error<
   "indirection requires pointer operand (%0 invalid)">;
 def ext_typecheck_indirection_through_void_pointer : ExtWarn<
-  "ISO C++ does not allow indirection on operand of type %0">,
+  "ISO %select{C|C++}0 does not allow indirection on operand of type %1">,
   InGroup<DiagGroup<"void-ptr-dereference">>;
 def warn_indirection_through_null : Warning<
   "indirection of non-volatile null pointer will be deleted, not trap">,

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8628ed48481f2..b6fd8174e1d87 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5604,11 +5604,11 @@ class Sema final {
 
   // Binary/Unary Operators.  'Tok' is the token for the operator.
   ExprResult CreateBuiltinUnaryOp(SourceLocation OpLoc, UnaryOperatorKind Opc,
-                                  Expr *InputExpr);
-  ExprResult BuildUnaryOp(Scope *S, SourceLocation OpLoc,
-                          UnaryOperatorKind Opc, Expr *Input);
-  ExprResult ActOnUnaryOp(Scope *S, SourceLocation OpLoc,
-                          tok::TokenKind Op, Expr *Input);
+                                  Expr *InputExpr, bool IsAfterAmp = false);
+  ExprResult BuildUnaryOp(Scope *S, SourceLocation OpLoc, UnaryOperatorKind Opc,
+                          Expr *Input, bool IsAfterAmp = false);
+  ExprResult ActOnUnaryOp(Scope *S, SourceLocation OpLoc, tok::TokenKind Op,
+                          Expr *Input, bool IsAfterAmp = false);
 
   bool isQualifiedMemberAccess(Expr *E);
   QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc);

diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 77e8b9488a8c9..9b3c39e7b43fb 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1360,7 +1360,8 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind,
     // Special treatment because of member pointers
     SourceLocation SavedLoc = ConsumeToken();
     PreferredType.enterUnary(Actions, Tok.getLocation(), tok::amp, SavedLoc);
-    Res = ParseCastExpression(AnyCastExpr, true);
+
+    Res = ParseCastExpression(AnyCastExpr, /*isAddressOfOperand=*/true);
     if (!Res.isInvalid()) {
       Expr *Arg = Res.get();
       Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Arg);
@@ -1385,7 +1386,8 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind,
     Res = ParseCastExpression(AnyCastExpr);
     if (!Res.isInvalid()) {
       Expr *Arg = Res.get();
-      Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Arg);
+      Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Arg,
+                                 isAddressOfOperand);
       if (Res.isInvalid())
         Res = Actions.CreateRecoveryExpr(SavedLoc, Arg->getEndLoc(), Arg);
     }

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 0f25801c9a2d7..acbf7d534d49f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14493,7 +14493,8 @@ static void RecordModifiableNonNullParam(Sema &S, const Expr *Exp) {
 
 /// CheckIndirectionOperand - Type check unary indirection (prefix '*').
 static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK,
-                                        SourceLocation OpLoc) {
+                                        SourceLocation OpLoc,
+                                        bool IsAfterAmp = false) {
   if (Op->isTypeDependent())
     return S.Context.DependentTy;
 
@@ -14530,18 +14531,15 @@ static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK,
     return QualType();
   }
 
-  // Note that per both C89 and C99, indirection is always legal, even if Result
-  // is an incomplete type or void.  It would be possible to warn about
-  // dereferencing a void pointer, but it's completely well-defined, and such a
-  // warning is unlikely to catch any mistakes. In C++, indirection is not valid
-  // for pointers to 'void' but is fine for any other pointer type:
-  //
-  // C++ [expr.unary.op]p1:
-  //   [...] the expression to which [the unary * operator] is applied shall
-  //   be a pointer to an object type, or a pointer to a function type
-  if (S.getLangOpts().CPlusPlus && Result->isVoidType())
-    S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer)
-      << OpTy << Op->getSourceRange();
+  if (Result->isVoidType()) {
+    // C++ [expr.unary.op]p1:
+    //   [...] the expression to which [the unary * operator] is applied shall
+    //   be a pointer to an object type, or a pointer to a function type
+    LangOptions LO = S.getLangOpts();
+    if (LO.CPlusPlus || !(LO.C99 && IsAfterAmp))
+      S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer)
+          << LO.CPlusPlus << OpTy << Op->getSourceRange();
+  }
 
   // Dereferences are usually l-values...
   VK = VK_LValue;
@@ -15530,8 +15528,8 @@ static bool isOverflowingIntegerType(ASTContext &Ctx, QualType T) {
 }
 
 ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
-                                      UnaryOperatorKind Opc,
-                                      Expr *InputExpr) {
+                                      UnaryOperatorKind Opc, Expr *InputExpr,
+                                      bool IsAfterAmp) {
   ExprResult Input = InputExpr;
   ExprValueKind VK = VK_PRValue;
   ExprObjectKind OK = OK_Ordinary;
@@ -15581,7 +15579,8 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
   case UO_Deref: {
     Input = DefaultFunctionArrayLvalueConversion(Input.get());
     if (Input.isInvalid()) return ExprError();
-    resultType = CheckIndirectionOperand(*this, Input.get(), VK, OpLoc);
+    resultType =
+        CheckIndirectionOperand(*this, Input.get(), VK, OpLoc, IsAfterAmp);
     break;
   }
   case UO_Plus:
@@ -15801,7 +15800,8 @@ bool Sema::isQualifiedMemberAccess(Expr *E) {
 }
 
 ExprResult Sema::BuildUnaryOp(Scope *S, SourceLocation OpLoc,
-                              UnaryOperatorKind Opc, Expr *Input) {
+                              UnaryOperatorKind Opc, Expr *Input,
+                              bool IsAfterAmp) {
   // First things first: handle placeholders so that the
   // overloaded-operator check considers the right type.
   if (const BuiltinType *pty = Input->getType()->getAsPlaceholderType()) {
@@ -15840,13 +15840,14 @@ ExprResult Sema::BuildUnaryOp(Scope *S, SourceLocation OpLoc,
     return CreateOverloadedUnaryOp(OpLoc, Opc, Functions, Input);
   }
 
-  return CreateBuiltinUnaryOp(OpLoc, Opc, Input);
+  return CreateBuiltinUnaryOp(OpLoc, Opc, Input, IsAfterAmp);
 }
 
 // Unary Operators.  'Tok' is the token for the operator.
-ExprResult Sema::ActOnUnaryOp(Scope *S, SourceLocation OpLoc,
-                              tok::TokenKind Op, Expr *Input) {
-  return BuildUnaryOp(S, OpLoc, ConvertTokenKindToUnaryOpcode(Op), Input);
+ExprResult Sema::ActOnUnaryOp(Scope *S, SourceLocation OpLoc, tok::TokenKind Op,
+                              Expr *Input, bool IsAfterAmp) {
+  return BuildUnaryOp(S, OpLoc, ConvertTokenKindToUnaryOpcode(Op), Input,
+                      IsAfterAmp);
 }
 
 /// ActOnAddrLabel - Parse the GNU address of label extension: "&&foo".

diff  --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m
index e9e56315eb268..4e3783dfc93ac 100644
--- a/clang/test/Analysis/misc-ps.m
+++ b/clang/test/Analysis/misc-ps.m
@@ -133,7 +133,7 @@ void handle_sizeof_void(unsigned flag) {
   void* q;
   
   if (!flag) {
-    if (sizeof(*q) == 1)
+    if (sizeof(*q) == 1) // expected-warning {{ISO C does not allow indirection on operand of type 'void *'}}
       return;
     // Infeasibe.
     *p = 1; // no-warning

diff  --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 183382406475c..942e83bef886c 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -160,7 +160,8 @@ void dr011(void) {
  */
 void dr012(void *p) {
   /* The behavior changed between C89 and C99. */
-  (void)&*p; /* c89only-warning {{ISO C forbids taking the address of an expression of type 'void'}} */
+  (void)&*p; /* c89only-warning {{ISO C forbids taking the address of an expression of type 'void'}}
+                c89only-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
 }
 
 /* WG14 DR013: yes

diff  --git a/clang/test/C/drs/dr1xx.c b/clang/test/C/drs/dr1xx.c
index 70435b523765d..400807ed5da84 100644
--- a/clang/test/C/drs/dr1xx.c
+++ b/clang/test/C/drs/dr1xx.c
@@ -137,11 +137,21 @@ void dr105(void) {
  */
 void dr106(void *p, int i) {
   /* The behavior changed between C89 and C99. */
-  (void)&*p; /* c89only-warning {{ISO C forbids taking the address of an expression of type 'void'}} */
+  (void)&*p; /* c89only-warning {{ISO C forbids taking the address of an expression of type 'void'}}
+                c89only-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
+
   /* The behavior of all three of these is undefined. */
-  (void)*p;
-  (void)(i ? *p : *p);
-  (void)(*p, *p); /* expected-warning {{left operand of comma operator has no effect}} */
+  (void)*p; /* expected-warning {{ISO C does not allow indirection on operand of type 'void *'}}*/
+
+  (void)&(*p); /* c89only-warning {{ISO C forbids taking the address of an expression of type 'void'}}
+                  expected-warning {{ISO C does not allow indirection on operand of type 'void *'}}*/
+
+  (void)(i ? *p : *p); /* expected-warning {{ISO C does not allow indirection on operand of type 'void *'}}
+                          expected-warning {{ISO C does not allow indirection on operand of type 'void *'}}*/
+
+  (void)(*p, *p); /* expected-warning {{left operand of comma operator has no effect}}
+                     expected-warning {{ISO C does not allow indirection on operand of type 'void *'}}
+		     expected-warning {{ISO C does not allow indirection on operand of type 'void *'}}*/
 }
 
 /* WG14 DR108: yes
@@ -178,10 +188,12 @@ void dr112(void *vp) {
  * Return expressions in functions declared to return qualified void
  */
 volatile void dr113_v(volatile void *vvp) { /* expected-warning {{function cannot return qualified void type 'volatile void'}} */
-  return *vvp; /* expected-warning {{void function 'dr113_v' should not return void expression}} */
+  return *vvp; /* expected-warning {{void function 'dr113_v' should not return void expression}}
+                  expected-warning{{ISO C does not allow indirection on operand of type 'volatile void *'}} */
 }
 const void dr113_c(const void *cvp) { /* expected-warning {{function cannot return qualified void type 'const void'}} */
-  return *cvp; /* expected-warning {{void function 'dr113_c' should not return void expression}} */
+  return *cvp; /* expected-warning {{void function 'dr113_c' should not return void expression}}
+                  expected-warning{{ISO C does not allow indirection on operand of type 'const void *'}} */
 }
 
 /* WG14 DR114: yes

diff  --git a/clang/test/Sema/asm.c b/clang/test/Sema/asm.c
index 3c7952f410c6d..9f3fb3176b4e8 100644
--- a/clang/test/Sema/asm.c
+++ b/clang/test/Sema/asm.c
@@ -50,8 +50,9 @@ void test3(void) {
 // <rdar://problem/6156893>
 void test4(const volatile void *addr)
 {
-    asm ("nop" : : "r"(*addr)); // expected-error {{invalid type 'const volatile void' in asm input for constraint 'r'}}
-    asm ("nop" : : "m"(*addr));
+    asm ("nop" : : "r"(*addr)); /* expected-error {{invalid type 'const volatile void' in asm input for constraint 'r'}}
+                                   expected-warning {{ISO C does not allow indirection on operand of type 'const volatile void *'}} */
+    asm ("nop" : : "m"(*addr)); // expected-warning {{ISO C does not allow indirection on operand of type 'const volatile void *'}}
 
     asm ("nop" : : "r"(test4(addr))); // expected-error {{invalid type 'void' in asm input for constraint 'r'}}
     asm ("nop" : : "m"(test4(addr))); // expected-error {{invalid lvalue in asm input for constraint 'm'}}

diff  --git a/clang/test/Sema/builtins-arm.c b/clang/test/Sema/builtins-arm.c
index 7c731204da093..0a59fd555beee 100644
--- a/clang/test/Sema/builtins-arm.c
+++ b/clang/test/Sema/builtins-arm.c
@@ -18,7 +18,8 @@ void __clear_cache(void*, void*);
 void test1(void) {
   __builtin_va_list ptr;
   ptr.__ap = "x";
-  *(ptr.__ap) = '0'; // expected-error {{incomplete type 'void' is not assignable}}
+  *(ptr.__ap) = '0'; /* expected-error {{incomplete type 'void' is not assignable}}
+                        expected-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
 }
 #else
 // va_list on ARM apcs-gnu is void*.
@@ -30,7 +31,9 @@ void test1(void) {
 
 void test2(void) {
   __builtin_va_list ptr = "x";
-  *ptr = '0'; // expected-error {{incomplete type 'void' is not assignable}}
+  *ptr = '0'; /* expected-error {{incomplete type 'void' is not assignable}}
+                 expected-warning {{ISO C does not allow indirection on operand of type '__builtin_va_list'}} */
+
 }
 #endif
 

diff  --git a/clang/test/Sema/conditional-expr.c b/clang/test/Sema/conditional-expr.c
index ff4fd73f4c5c8..81997459c1f20 100644
--- a/clang/test/Sema/conditional-expr.c
+++ b/clang/test/Sema/conditional-expr.c
@@ -2,11 +2,16 @@
 void foo(void) {
   *(0 ? (double *)0 : (void *)0) = 0;
   // FIXME: GCC doesn't consider the following two statements to be errors.
-  *(0 ? (double *)0 : (void *)(int *)0) = 0; // expected-error {{incomplete type 'void' is not assignable}}
-  *(0 ? (double *)0 : (void *)(double *)0) = 0; // expected-error {{incomplete type 'void' is not assignable}}
-  *(0 ? (double *)0 : (int *)(void *)0) = 0; // expected-error {{incomplete type 'void' is not assignable}} expected-warning {{pointer type mismatch ('double *' and 'int *')}}
+  *(0 ? (double *)0 : (void *)(int *)0) = 0; /* expected-error {{incomplete type 'void' is not assignable}}
+                                                expected-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
+  *(0 ? (double *)0 : (void *)(double *)0) = 0; /* expected-error {{incomplete type 'void' is not assignable}}
+                                                   expected-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
+  *(0 ? (double *)0 : (int *)(void *)0) = 0; /* expected-error {{incomplete type 'void' is not assignable}}
+                                                expected-warning {{pointer type mismatch ('double *' and 'int *')}}
+                                                expected-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
   *(0 ? (double *)0 : (double *)(void *)0) = 0;
-  *((void *) 0) = 0; // expected-error {{incomplete type 'void' is not assignable}}
+  *((void *) 0) = 0; /* expected-error {{incomplete type 'void' is not assignable}}
+                        expected-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
   double *dp;
   int *ip;
   void *vp;

diff  --git a/clang/test/Sema/deref.c b/clang/test/Sema/deref.c
index 845b28645a483..45c921967bccf 100644
--- a/clang/test/Sema/deref.c
+++ b/clang/test/Sema/deref.c
@@ -18,7 +18,8 @@ void foo2 (void)
 void foo3 (void)
 {
  void* x = 0;
- void* y = &*x; /* expected-warning{{address of an expression of type 'void'}} */
+ void* y = &*x; /* expected-warning{{address of an expression of type 'void'}}
+                   expected-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
 }
 
 extern const void cv1;

diff  --git a/clang/test/Sema/expr-address-of.c b/clang/test/Sema/expr-address-of.c
index 8881038b3241c..057b7843ce6b7 100644
--- a/clang/test/Sema/expr-address-of.c
+++ b/clang/test/Sema/expr-address-of.c
@@ -105,7 +105,7 @@ char* f7(void) {
   int* dummy2 = &(t2.a); // expected-error {{address of bit-field requested}}
   int* dummy3 = &(t2.b); // expected-error {{address of bit-field requested}}
 
-  void* t3 = &(*(void*)0);
+  void* t3 = &*(void*)0;
 }
 
 void f8(void) {

diff  --git a/clang/test/Sema/i-c-e.c b/clang/test/Sema/i-c-e.c
index b348b4f3c43ee..31dad5836549c 100644
--- a/clang/test/Sema/i-c-e.c
+++ b/clang/test/Sema/i-c-e.c
@@ -3,7 +3,8 @@
 #include <stdint.h>
 #include <limits.h>
 
-int a(void) {int p; *(1 ? &p : (void*)(0 && (a(),1))) = 10;} // expected-error {{incomplete type 'void' is not assignable}}
+int a(void) {int p; *(1 ? &p : (void*)(0 && (a(),1))) = 10;} /* expected-error {{incomplete type 'void' is not assignable}}
+                                                                expected-warning {{ISO C does not allow indirection on operand of type 'void *'}} */
 
 // rdar://6091492 - ?: with __builtin_constant_p as the operand is an i-c-e.
 int expr;


        


More information about the cfe-commits mailing list