[Clang] Warn when trying to deferencing void pointers in C

[junaire]

Summary

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 <[e-mail]>

[junaire]

I'd admit that passing a boolean between these functions is ugly but surprisingly it works! even for `&(void_ptr)! But yeah, please suggest if you have better solutions ;D

clang/test/C/drs/dr1xx.c

141–142

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

[junaire]

clang/lib/Sema/SemaExpr.cpp

14541

I don't know why we don't have getLangOpts().C89. I'm a bit confused about how we deal with different C standards...

[aaron.ballman]

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

clang/include/clang/Basic/DiagnosticSemaKinds.td

6921–6924

I think we want the extension in both C and C++

clang/lib/Sema/SemaExpr.cpp

14541

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?

clang/test/Analysis/misc-ps.m

136

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.

clang/test/C/drs/dr1xx.c

141–142

What kind of problems are you running into?

[erichkeane]

clang/include/clang/Parse/Parser.h

1792 ↗

(On Diff #462243)

So I thought 'isAddressOfOperand' might be good enough for this, is this not the case?

[junaire]

Address comments from reviewers.

[junaire]

So I thought 'isAddressOfOperand' might be good enough for this, is this not the case?

Yeah, that works, I somehow missed it ;D Thank you, Erich!

clang/lib/Sema/SemaExpr.cpp

14541

Thanks for the explanation! That works for me!

[junaire]

Add &(*p) test case in dr1xx.c. (Previous seperate test is removed as it mostly duplicate with the DR test)

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

6922

Swapping these so the logic is more straightforward elsewhere.

clang/lib/Sema/SemaExpr.cpp

14538–14544

Simplifying the logic a bit.

clang/test/C/drs/dr1xx.c

140

Can you switch all of the warning changes in this file to use this style where each expected diagnostic is on its own line? That makes it easier to notice which diagnostics happen on the line (it's easy to lose sight of the trailing expected diagnostics otherwise).

143

This looks wrong to me -- this should be an expected-warning instead of a c89only-warning, same as two lines above, right?

[junaire]

clang/test/C/drs/dr1xx.c

140

Yes, basically, the language rule is that &*void_ptr is well defined in C99 and later

IIUC, you mean this should be warned in C89 mode, right?

143

I'm a bit confused. Maybe I made the trailing comments too hard to read. This is an expected-warning, c89only-warning is for ISO C forbids taking the address of an expression of type 'void', which is not part of this patch.

[junaire]

Address comments.

[aaron.ballman]

LGTM aside from a minor nit in the tests. Thank you for this!

clang/test/C/drs/dr1xx.c

140

Yes, basically, the language rule is that &*void_ptr is well defined in C99 and later

IIUC, you mean this should be warned in C89 mode, right?

Correct; I think there's two warnings that would fire there in C89: dereferencing a void pointer is one, and taking the address of void is the other.

143

You're right, I think I just confused myself. Sorry for the noise! Though it is a bit strange that ISO C does not allow indirection on operand of type 'void *' is a c89 only warning on line 140 while it's an expected warning on line 146. Something odd is happening there, but I don't think it's the fault of this patch.

182–185

Can you separate these diagnostics onto their own lines as well?

[nathanchance]

This warning is quite noisy for the Linux kernel due to a couple of places where a void * is dereferenced as part of compile time checking.

Against an x86_64 allmodconfig build (which builds the majority of the code in the kernel), I see:

$ rg -c -- -Wvoid-ptr-dereference build.log
1588001

My original commentary is available on our GitHub issue tracker (https://github.com/ClangBuiltLinux/linux/issues/1720) but I will summarize it here:

The first major instance of this warning comes from the __is_constexpr macro, which is used a lot in common macros do fancy things at compile time (change that added this: https://git.kernel.org/linus/3c8ba0d61d04ced9f8d9ff93977995a9e4e96e91):

/*
 * This returns a constant expression while determining if an argument is
 * a constant expression, most importantly without evaluating the argument.
 * Glory to Martin Uecker <[e-mail]>
 */
#define __is_constexpr(x) \
	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))

Sample warnings:

In file included from scripts/mod/devicetable-offsets.c:3:
In file included from ./include/linux/mod_devicetable.h:13:
In file included from ./include/linux/uuid.h:12:
In file included from ./include/linux/string.h:253:
./include/linux/fortify-string.h:159:10: warning: ISO C does not allow indirection on operand of type 'void *' [-Wvoid-ptr-dereference]
        q_len = strlen(q);
                ^~~~~~~~~
./include/linux/fortify-string.h:131:24: note: expanded from macro 'strlen'
        __builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),      \
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/const.h:12:25: note: expanded from macro '__is_constexpr'
        (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In file included from arch/x86/kernel/asm-offsets.c:9:
In file included from ./include/linux/crypto.h:20:
In file included from ./include/linux/slab.h:15:
In file included from ./include/linux/gfp.h:7:
In file included from ./include/linux/mmzone.h:8:
In file included from ./include/linux/spinlock.h:55:
In file included from ./include/linux/preempt.h:78:
In file included from ./arch/x86/include/asm/preempt.h:7:
In file included from ./include/linux/thread_info.h:60:
In file included from ./arch/x86/include/asm/thread_info.h:53:
In file included from ./arch/x86/include/asm/cpufeature.h:5:
In file included from ./arch/x86/include/asm/processor.h:22:
In file included from ./arch/x86/include/asm/msr.h:11:
In file included from ./arch/x86/include/asm/cpumask.h:5:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:9:
./include/linux/find.h:40:17: warning: ISO C does not allow indirection on operand of type 'void *' [-Wvoid-ptr-dereference]
                val = *addr & GENMASK(size - 1, offset);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/bits.h:38:3: note: expanded from macro 'GENMASK'
        (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
         ^~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/bits.h:25:3: note: expanded from macro 'GENMASK_INPUT_CHECK'
                __is_constexpr((l) > (h)), (l) > (h), 0)))
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/const.h:12:25: note: expanded from macro '__is_constexpr'
        (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
                               ^
./include/linux/build_bug.h:16:62: note: expanded from macro 'BUILD_BUG_ON_ZERO'
#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
                                                             ^

The second major instance is the type checking in container_of when ptr is a void * (change that added this: https://git.kernel.org/linus/c7acec713d14c6ce8a20154f9dfda258d6bcad3b):

/**
 * container_of - cast a member of a structure out to the containing structure
 * @ptr:	the pointer to the member.
 * @type:	the type of the container struct this is embedded in.
 * @member:	the name of the member within the struct.
 *
 */
#define container_of(ptr, type, member) ({				\
	void *__mptr = (void *)(ptr);					\
	static_assert(__same_type(*(ptr), ((type *)0)->member) ||	\
		      __same_type(*(ptr), void),			\
		      "pointer type mismatch in container_of()");	\
	((type *)(__mptr - offsetof(type, member))); })

#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))

Sample warning:

In file included from sound/soc/sof/core.c:14:
In file included from ./include/sound/sof.h:14:
./include/linux/pci.h:600:9: warning: ISO C does not allow indirection on operand of type 'void *' [-Wvoid-ptr-dereference]
        return container_of(priv, struct pci_host_bridge, private);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/container_of.h:20:21: note: expanded from macro 'container_of'
                      __same_type(*(ptr), void),                        \
                      ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/compiler_types.h:295:63: note: expanded from macro '__same_type'
#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
                                                              ^
./include/linux/build_bug.h:77:50: note: expanded from macro 'static_assert'
#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
                                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:78:56: note: expanded from macro '__static_assert'
#define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
                                                       ^~~~

I am aware these are quite specialized cases so warning is likely appropriate. If so, we can just disable this warning for the kernel but I figured it was worth a comment in case the diagnostic can be improved in any way.

[aaron.ballman]

In D134461#3815298, @nathanchance wrote:

This warning is quite noisy for the Linux kernel due to a couple of places where a void * is dereferenced as part of compile time checking.

Thank you for letting us know!

Against an x86_64 allmodconfig build (which builds the majority of the code in the kernel), I see:

$ rg -c -- -Wvoid-ptr-dereference build.log
1588001

My original commentary is available on our GitHub issue tracker (https://github.com/ClangBuiltLinux/linux/issues/1720) but I will summarize it here:

The first major instance of this warning comes from the __is_constexpr macro, which is used a lot in common macros do fancy things at compile time (change that added this: https://git.kernel.org/linus/3c8ba0d61d04ced9f8d9ff93977995a9e4e96e91):

/*
 * This returns a constant expression while determining if an argument is
 * a constant expression, most importantly without evaluating the argument.
 * Glory to Martin Uecker <[e-mail]>
 */
#define __is_constexpr(x) \
	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))

Sample warnings:

In file included from scripts/mod/devicetable-offsets.c:3:
In file included from ./include/linux/mod_devicetable.h:13:
In file included from ./include/linux/uuid.h:12:
In file included from ./include/linux/string.h:253:
./include/linux/fortify-string.h:159:10: warning: ISO C does not allow indirection on operand of type 'void *' [-Wvoid-ptr-dereference]
        q_len = strlen(q);
                ^~~~~~~~~
./include/linux/fortify-string.h:131:24: note: expanded from macro 'strlen'
        __builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),      \
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/const.h:12:25: note: expanded from macro '__is_constexpr'
        (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one is interesting to me -- I was on the fence about whether we want to diagnose this in an unevaluated context or not. The clang/test/Analysis/misc-ps.m test was why I was considering it, and I eventually convinced myself that the dereference there was a harmless UB when we support pointer arithmetic on void as an extension, but UB nonetheless. Now I wonder whether we want different behavior in the sizeof case -- it seems odd to me that we warn by default on sizeof(*vp) but not sizeof(void): https://godbolt.org/z/PcMrW7b5W so I think we might want to silence sizeof(*vp) diagnostics.

WDYT?

In file included from arch/x86/kernel/asm-offsets.c:9:
In file included from ./include/linux/crypto.h:20:
In file included from ./include/linux/slab.h:15:
In file included from ./include/linux/gfp.h:7:
In file included from ./include/linux/mmzone.h:8:
In file included from ./include/linux/spinlock.h:55:
In file included from ./include/linux/preempt.h:78:
In file included from ./arch/x86/include/asm/preempt.h:7:
In file included from ./include/linux/thread_info.h:60:
In file included from ./arch/x86/include/asm/thread_info.h:53:
In file included from ./arch/x86/include/asm/cpufeature.h:5:
In file included from ./arch/x86/include/asm/processor.h:22:
In file included from ./arch/x86/include/asm/msr.h:11:
In file included from ./arch/x86/include/asm/cpumask.h:5:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:9:
./include/linux/find.h:40:17: warning: ISO C does not allow indirection on operand of type 'void *' [-Wvoid-ptr-dereference]

val = *addr & GENMASK(size - 1, offset);
              ^~~~~~~~~~~~~~~~~~~~~~~~~

./include/linux/bits.h:38:3: note: expanded from macro 'GENMASK'

(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
 ^~~~~~~~~~~~~~~~~~~~~~~~~

./include/linux/bits.h:25:3: note: expanded from macro 'GENMASK_INPUT_CHECK'

__is_constexpr((l) > (h)), (l) > (h), 0)))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

./include/linux/const.h:12:25: note: expanded from macro '__is_constexpr'

(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
                       ^

./include/linux/build_bug.h:16:62: note: expanded from macro 'BUILD_BUG_ON_ZERO'
#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))

^

The second major instance is the type checking in `container_of` when ptr is a `void *` (change that added this: https://git.kernel.org/linus/c7acec713d14c6ce8a20154f9dfda258d6bcad3b):

/**

• container_of - cast a member of a structure out to the containing structure
• @ptr: the pointer to the member.
• @type: the type of the container struct this is embedded in.
• @member: the name of the member within the struct. * */

#define container_of(ptr, type, member) ({ \
void *mptr = (void *)(ptr); \
static_assert(same_type(*(ptr), ((type *)0)->member) || \

		      __same_type(*(ptr), void),			\
		      "pointer type mismatch in container_of()");	\

((type *)(__mptr - offsetof(type, member))); })

#define same_type(a, b) builtin_types_compatible_p(typeof(a), typeof(b))

This is another interesting case involving an unevaluated operand. typeof doesn't evaluate its argument and so it seems somewhat defensible to allow typeof(*vp) for generic programming cases; it should just yield void as a type.

Sample warning:

In file included from sound/soc/sof/core.c:14:
In file included from ./include/sound/sof.h:14:
./include/linux/pci.h:600:9: warning: ISO C does not allow indirection on operand of type 'void *' [-Wvoid-ptr-dereference]
        return container_of(priv, struct pci_host_bridge, private);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/container_of.h:20:21: note: expanded from macro 'container_of'
                      __same_type(*(ptr), void),                        \
                      ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/compiler_types.h:295:63: note: expanded from macro '__same_type'
#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
                                                              ^
./include/linux/build_bug.h:77:50: note: expanded from macro 'static_assert'
#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
                                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:78:56: note: expanded from macro '__static_assert'
#define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
                                                       ^~~~

I am aware these are quite specialized cases so warning is likely appropriate. If so, we can just disable this warning for the kernel but I figured it was worth a comment in case the diagnostic can be improved in any way.

All this said, I think in both cases we want -pedantic to warn on these situations even if we would silence by default. e.g.,

void func(void *vp) {
  sizeof(void); // No warning by default, does get pedantic warning
  sizeof(*vp); // No warning by default, does get pedantic warning
  sizeof(*(vp))`; // No warning by default, does get pedantic warning
  void inner(typeof(*vp)); // No warning by default, does get pedantic warning

  (void)*vp; // Warning by default
}

What do folks think of that idea?

[nathanchance]

In D134461#3815458, @aaron.ballman wrote:

What do folks think of that idea?

I think that all sounds reasonable to me (although I am far from an authority on these matters). As far as I understand it, the kernel cannot use -pedantic due to its liberal use of GNU C extensions so moving those specific constructs to a -pedantic version of the warning will be the same as just turning them off altogether, which obviously works fine for us. I am happy to take the suggested changes for a spin against the kernel once a patch is available to see if there are any other interesting places where this warning triggers to make sure it does not need further adjustment, since we openly welcome new diagnostics. Right now, it is pretty much impossible to sift through them all because of how often it fires.

[nickdesaulniers]

In D134461#3815458, @aaron.ballman wrote:

In D134461#3815298, @nathanchance wrote:

This warning is quite noisy for the Linux kernel due to a couple of places where a void * is dereferenced as part of compile time checking.

Thank you for letting us know!

In particular offsetof and __is_constexpr are _heavily_ used throughout the codebase.

This one is interesting to me -- I was on the fence about whether we want to diagnose this in an unevaluated context or not. The clang/test/Analysis/misc-ps.m test was why I was considering it, and I eventually convinced myself that the dereference there was a harmless UB when we support pointer arithmetic on void as an extension, but UB nonetheless. Now I wonder whether we want different behavior in the sizeof case -- it seems odd to me that we warn by default on sizeof(*vp) but not sizeof(void): https://godbolt.org/z/PcMrW7b5W so I think we might want to silence sizeof(*vp) diagnostics.

WDYT?

SGTM

All this said, I think in both cases we want -pedantic to warn on these situations even if we would silence by default. e.g.,

void func(void *vp) {
  sizeof(void); // No warning by default, does get pedantic warning
  sizeof(*vp); // No warning by default, does get pedantic warning
  sizeof(*(vp))`; // No warning by default, does get pedantic warning
  void inner(typeof(*vp)); // No warning by default, does get pedantic warning

  (void)*vp; // Warning by default
}

What do folks think of that idea?

SGTM; either warn via -Wpedantic or silenced with -Wno-gnu.

[aaron.ballman]

In D134461#3815502, @nathanchance wrote:

In D134461#3815458, @aaron.ballman wrote:

What do folks think of that idea?

I think that all sounds reasonable to me (although I am far from an authority on these matters). As far as I understand it, the kernel cannot use -pedantic due to its liberal use of GNU C extensions so moving those specific constructs to a -pedantic version of the warning will be the same as just turning them off altogether, which obviously works fine for us. I am happy to take the suggested changes for a spin against the kernel once a patch is available to see if there are any other interesting places where this warning triggers to make sure it does not need further adjustment, since we openly welcome new diagnostics. Right now, it is pretty much impossible to sift through them all because of how often it fires.

Thank you for the offer! @junaire -- would you like to try making the changes?

[erichkeane]

FWIW, I think the idea of allowing dereference of void* in an unevaluated context seems somewhat sensible, if sad. I suspect the kernel is not the only place to do this foolishness and take advantage of the extensions we/gcc permit for the void type. So I'll say "sounds good to me", but I'm going to be grumpy about it.

[junaire]

Sorry for missing the conversation, I was in sleeping mode at that time ;D

This warning is quite noisy for the Linux kernel due to a couple of places where a void * is dereferenced as part of compile time checking.

I'm actually surprised to see that, I thought Clang is just "following" what GCC does, (more strictly in fact). So I wonder why there's no issue for GCC but Clang?

All this said, I think in both cases we want -pedantic to warn on these situations even if we would silence by default. e.g.,

void func(void *vp) {
  sizeof(void); // No warning by default, does get pedantic warning
  sizeof(*vp); // No warning by default, does get pedantic warning
  sizeof(*(vp))`; // No warning by default, does get pedantic warning
  void inner(typeof(*vp)); // No warning by default, does get pedantic warning

  (void)*vp; // Warning by default
}

What do folks think of that idea?

Nice summary!

Thank you for the offer! @junaire -- would you like to try making the changes?

Sure, I'm willing to submit another patch!

clang/docs/ReleaseNotes.rst

- New analysis pass will now help preserve sugar when combining deductions, in an
   in a way that strips the sugar which is different between terms, and preserves
   those which are common.
 - Correctly diagnose use of an integer literal without a suffix whose
   underlying type is ``long long`` or ``unsigned long long`` as an extension in
   C89 mode . Clang previously only diagnosed if the literal had an explicit
   ``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
 -------------------------------------------------
 - It's now possible to set the crash diagnostics directory through
   the environment variable ``CLANG_CRASH_DIAGNOSTICS_DIR``.
   The ``-fcrash-diagnostics-dir`` flag takes precedence.
 - When using header modules, inclusion of a private header and violations of
   the `use-declaration rules

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_typecheck_addrof_temporary : Error<
   "taking the address of a temporary object of type %0">;
 def err_typecheck_addrof_dtor : Error<
   "taking the address of a destructor">;
 def err_typecheck_unary_expr : Error<
   "invalid argument type %0 to unary expression">;
 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">,
   InGroup<NullDereference>;
 def warn_binding_null_to_reference : Warning<
   "binding dereferenced null pointer to reference has undefined behavior">,
   InGroup<NullDereference>;
 def note_indirection_through_null : Note<
   "consider using __builtin_trap() or qualifying pointer with 'volatile'">;
 def warn_pointer_indirection_from_incompatible_type : Warning<

clang/include/clang/Sema/Sema.h

ExprResult CreateGenericSelectionExpr(SourceLocation KeyLoc,
                                         SourceLocation DefaultLoc,
                                         SourceLocation RParenLoc,
                                         Expr *ControllingExpr,
                                         ArrayRef<TypeSourceInfo *> Types,
                                         ArrayRef<Expr *> Exprs);
 
   // 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);
 
   bool CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N);
 
   ExprResult CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
                                             SourceLocation OpLoc,

clang/lib/Parse/ParseExpr.cpp

case tok::minusminus: {  // unary-expression: '--' unary-expression [C99]
     return Res;
   }
   case tok::amp: {         // unary-expression: '&' cast-expression
     if (NotPrimaryExpression)
       *NotPrimaryExpression = true;
     // 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);
       if (Res.isInvalid())
         Res = Actions.CreateRecoveryExpr(Tok.getLocation(), Arg->getEndLoc(),
                                          Arg);
     }
     return Res;
   }
 
   case tok::star:          // unary-expression: '*' cast-expression
   case tok::plus:          // unary-expression: '+' cast-expression
   case tok::minus:         // unary-expression: '-' cast-expression
   case tok::tilde:         // unary-expression: '~' cast-expression
   case tok::exclaim:       // unary-expression: '!' cast-expression
   case tok::kw___real:     // unary-expression: '__real' cast-expression [GNU]
   case tok::kw___imag: {   // unary-expression: '__imag' cast-expression [GNU]
     if (NotPrimaryExpression)
       *NotPrimaryExpression = true;
     SourceLocation SavedLoc = ConsumeToken();
     PreferredType.enterUnary(Actions, Tok.getLocation(), SavedKind, SavedLoc);
     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);
     }
     return Res;
   }
 
   case tok::kw_co_await: {  // unary-expression: 'co_await' cast-expression
     if (NotPrimaryExpression)

clang/lib/Sema/SemaExpr.cpp

if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(Param->getDeclContext()))
     if (!FD->hasAttr<NonNullAttr>() && !Param->hasAttr<NonNullAttr>())
       return;
   if (FunctionScopeInfo *FD = S.getCurFunction())
     FD->ModifiedNonNullParams.insert(Param);
 }
 
 /// 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;
 
   ExprResult ConvResult = S.UsualUnaryConversions(Op);
   if (ConvResult.isInvalid())
     return QualType();
   Op = ConvResult.get();
   QualType OpTy = Op->getType();
static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK,
   }
 
   if (Result.isNull()) {
     S.Diag(OpLoc, diag::err_typecheck_indirection_requires_pointer)
       << OpTy << Op->getSourceRange();
     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:
-  //
+  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
-  if (S.getLangOpts().CPlusPlus && Result->isVoidType())
+    if (S.getLangOpts().CPlusPlus)
       S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer)
-      << OpTy << Op->getSourceRange();
+          << /* C++ mode */ 0 << OpTy << Op->getSourceRange();
+    else if (!S.getLangOpts().CPlusPlus)
+      if (!(S.getLangOpts().C99 && IsAfterAmp))
+        S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer)
+            << /* C mode */ 1 << OpTy << Op->getSourceRange();
+  }
 
   // Dereferences are usually l-values...
   VK = VK_LValue;
 
   // ...except that certain expressions are never l-values in C.
   if (!S.getLangOpts().CPlusPlus && Result.isCForbiddenLValueType())
     VK = VK_PRValue;
 
static bool isOverflowingIntegerType(ASTContext &Ctx, QualType T) {
 
   if (!T->isPromotableIntegerType())
     return true;
 
   return Ctx.getIntWidth(T) >= Ctx.getIntWidth(Ctx.IntTy);
 }
 
 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;
   QualType resultType;
   bool CanOverflow = false;
 
   bool ConvertHalfVec = false;
   if (getLangOpts().OpenCL) {
ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
   case UO_AddrOf:
     resultType = CheckAddressOfOperand(Input, OpLoc);
     CheckAddressOfNoDeref(InputExpr);
     RecordModifiableNonNullParam(*this, InputExpr);
     break;
   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:
   case UO_Minus:
     CanOverflow = Opc == UO_Minus &&
                   isOverflowingIntegerType(Context, Input.get()->getType());
     Input = UsualUnaryConversions(Input.get());
     if (Input.isInvalid()) return ExprError();
if (UnresolvedLookupExpr *ULE = dyn_cast<UnresolvedLookupExpr>(E)) {
 
     return false;
   }
 
   return false;
 }
 
 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()) {
     // Increment and decrement of pseudo-object references.
     if (pty->getKind() == BuiltinType::PseudoObject &&
         UnaryOperator::isIncrementDecrementOp(Opc))
       return checkPseudoObjectIncDec(S, OpLoc, Opc, Input);
 
if (getLangOpts().CPlusPlus && Input->getType()->isOverloadableType() &&
     UnresolvedSet<16> Functions;
     OverloadedOperatorKind OverOp = UnaryOperator::getOverloadedOperator(Opc);
     if (S && OverOp != OO_None)
       LookupOverloadedOperatorName(OverOp, S, Functions);
 
     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".
 ExprResult Sema::ActOnAddrLabel(SourceLocation OpLoc, SourceLocation LabLoc,
                                 LabelDecl *TheDecl) {
   TheDecl->markUsed(Context);
   // Create the AST node.  The address of a label always has type 'void*'.
   return new (Context) AddrLabelExpr(OpLoc, LabLoc, TheDecl,

clang/test/Analysis/misc-ps.m

if (sizeof(void) == 1)
       return;
     // Infeasible.
     *p = 1; // no-warning
   }
   
   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
   }
     
   // Infeasible.
   *p = 1; // no-warning
 }

clang/test/C/drs/dr1xx.c

 
 /* WG14 DR106: yes
  * When can you dereference a void pointer?
  *
  * NB: This is a partial duplicate of DR012.
  */
 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
  * Can a macro identifier hide a keyword?
  */
 void dr108(void) {
 #define const
   const int i = 12;
void dr112(void *vp) {
    */
   (void)(vp > (void*)0);
 }
 
 /* WG14 DR113: yes
  * 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
  * Initialization of multi-dimensional char array objects
  */
 void dr114(void) {
   char array[2][5] = { "defghi" }; /* expected-warning {{initializer-string for char array is too long}} */
 }

clang/test/Sema/asm.c

void test3(void) {
   int x;
   asm(L"foo" : "=r"(x)); // expected-error {{wide string}}
   asm("foo" : L"=r"(x)); // expected-error {{wide string}}
 }
 
 // <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'}}
 
     asm ("nop" : : "m"(f())); // expected-error {{invalid lvalue in asm input for constraint 'm'}}
 }
 
 // <rdar://problem/6512595>

clang/test/Sema/builtins-arm.c

 void __clear_cache(char*, char*); // expected-error {{conflicting types for '__clear_cache'}}
 void __clear_cache(void*, void*);
 
 #if defined(__ARM_PCS) || defined(__ARM_EABI__)
 // va_list on ARM AAPCS is struct { void* __ap }.
 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*.
 void test1(void) {
   __builtin_va_list ptr;
   ptr.__ap = "x";  // expected-error {{member reference base type '__builtin_va_list' is not a structure or union}}
   *(ptr.__ap) = '0';// expected-error {{member reference base type '__builtin_va_list' is not a structure or union}}
 }
 
 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
 
 void test3(void) {
   __builtin_arm_dsb(16); // expected-error-re {{argument value {{.*}} is outside the valid range}}
   __builtin_arm_dmb(17); // expected-error-re {{argument value {{.*}} is outside the valid range}}
   __builtin_arm_isb(18); // expected-error-re {{argument value {{.*}} is outside the valid range}}
 }

clang/test/Sema/conditional-expr.c

 // RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify -pedantic -Wsign-conversion %s
 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;
 
   dp = vp;
   vp = dp;
   ip = dp; // expected-warning {{incompatible pointer types assigning to 'int *' from 'double *'}}
   dp = ip; // expected-warning {{incompatible pointer types assigning to 'double *' from 'int *'}}

clang/test/Sema/expr-address-of.c

 char* f7(void) {
   register struct {char* x;} t1 = {"Hello"};
   char* dummy1 = &(t1.x[0]);
 
   struct {int a : 10; struct{int b : 10;};} t2;
   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) {
   void *dummy0 = &f8(); // expected-error {{cannot take the address of an rvalue of type 'void'}}
 
   extern void v;
   void *dummy1 = &(1 ? v : f8()); // expected-error {{cannot take the address of an rvalue of type 'void'}}
 

clang/test/Sema/i-c-e.c

 // RUN: %clang_cc1 %s -ffreestanding -Wno-int-to-pointer-cast -fsyntax-only -verify -pedantic -fpascal-strings -std=c99
 
 #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;
 char w[__builtin_constant_p(expr) ? expr : 1];
 
 char v[sizeof(__builtin_constant_p(0)) == sizeof(int) ? 1 : -1];
 
 int implicitConversion = 1.0;