[PATCH] D134461: [Clang] Diagnose an error when trying to deferencing void pointers in C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: clang-language-wg, jyknight.
aaron.ballman added a comment.

Thank you for working on this! Adding a few more reviewers for exposure.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6921-6926
 def ext_typecheck_indirection_through_void_pointer : ExtWarn<
   "ISO C++ does not allow indirection on operand of type %0">,
   InGroup<DiagGroup<"void-ptr-dereference">>;
+def warn_deference_void_pointer : Warning<
+  "deference a void pointer has undefined behavior">, InGroup<
+  DiagGroup<"deference-void-pointer">>;
----------------
I think we want the extension in both C and C++


================
Comment at: clang/lib/Sema/SemaExpr.cpp:14541
+      Kind = diag::ext_typecheck_indirection_through_void_pointer;
+    else if (S.getLangOpts().C99 && !IsAfterAmp)
+      Kind = diag::warn_deference_void_pointer;
----------------
junaire wrote:
> I don't know why we don't have `getLangOpts().C89`. I'm a bit confused about how we deal with different C standards...
This trips up folks somewhat often, unfortunately! All of the language mode options are cumulative, so if the user specifies C11, then C99 and C11 will both be true. However, we don't have an explicit C89 language mode, instead we rely on `!CPlusPlus` to tell us we're in C mode and `!C99` to tell us we're in C89 mode. Does that make sense?


================
Comment at: clang/test/Analysis/misc-ps.m:136
   if (!flag) {
-    if (sizeof(*q) == 1)
+    if (sizeof(*q) == 1) // expected-warning {{deference a void pointer has undefined behavior}}
       return;
----------------
This is why I think we want to reuse the existing extension diagnostic -- there's kind of no UB in this case because the dereference is never actually evaluated, but telling the user "this isn't allowed per spec" will at least tell them about the portability issue.


================
Comment at: clang/test/C/drs/dr1xx.c:142
   /* The behavior of all three of these is undefined. */
-  (void)*p;
-  (void)(i ? *p : *p);
+  (void)*p; /* expected-warning {{deference a void pointer has undefined behavior}} */
+  (void)(i ? *p : *p); /* expected-warning {{deference a void pointer has undefined behavior}} */
----------------
junaire wrote:
> I seem not dealing with the tests correctly, mostly because we're specifying multiple run lines with different standards. Can you help me a little bit with this? @aaron.ballman 
What kind of problems are you running into?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134461/new/

https://reviews.llvm.org/D134461