[Sema/Attribute] Check for noderef attribute

[leonardchan]

Summary

This patch adds the noderef attribute in clang and checks for dereferences of types that have this attribute. This attribute is currently used by sparse and would like to be ported to clang.

The following are examples of when a warning would be raised on dereferencing a noderef type.

int __attribute__((noderef)) *p;
int x = *p;  // warning: dereference of noderef expression [-Wignored-attributes]

int __attribute__((noderef)) **p2;
x = **p2;  // warning: dereference of noderef expression [-Wignored-attributes]

int * __attribute__((noderef)) *p3;
p = *p3;  // warning: dereference of noderef expression [-Wignored-attributes]

struct S {
  int a;
};
struct S __attribute__((noderef)) *s;
x = s->a;    // warning: dereference of noderef expression [-Wignored-attributes]
x = (*s).a;   // warning: dereference of noderef expression [-Wignored-attributes]

Not all accesses may raise a warning if the value directed by the pointer may not be accessed. The following are examples where no warning may be raised:

int *q;
int __attribute__((noderef)) *p;
q = &*p;
q = *&p;

struct S {
  int a;
};
struct S __attribute__((noderef)) *s;
p = &s->a;
p = &(*s).a;

More examples of existing usage of noderef in sparse can be found in https://git.kernel.org/pub/scm/devel/sparse/sparse.git/tree/validation/noderef.c

[leonardchan]

• Added checks for expressions in statements other than declarations or expression statements

[rsmith]

The way in which you're checking for the problematic cases is unnecessarily expensive. Instead of performing a separate AST traversal, please detect whether you should be producing the warning directly when forming the problematic expressions. (For example, you could store a list of pending "dereference of noderef" expressions in the ExpressionEvaluationContextRecord, remove items from the list if you find they're the operand of a unary address-of operator, and diagnose any remaining items once you've left the evaluation context.)

[erik.pilkington]

Thanks for working on this! CCing Aaron, who is code owner for attributes.

include/clang/Basic/AttrDocs.td

3465

'allows for showing' sounds a bit strange... maybe: "The `noderef` attribute causes clang to diagnose dereferences of annotated pointer types" or something?

include/clang/Basic/DiagnosticSemaKinds.td

9445

I think -Wignored-attributes is a group for cases where clang ignores an attribute, you should probably add you own diagnostic group, say -Wdereferenced-noderef or something for this diagnostic. You should add this to DiagnosticGroups.td.

[leonardchan]

In D49511#1168692, @rsmith wrote:

The way in which you're checking for the problematic cases is unnecessarily expensive. Instead of performing a separate AST traversal, please detect whether you should be producing the warning directly when forming the problematic expressions. (For example, you could store a list of pending "dereference of noderef" expressions in the ExpressionEvaluationContextRecord, remove items from the list if you find they're the operand of a unary address-of operator, and diagnose any remaining items once you've left the evaluation context.)

Done. I opted for not using ExpressionEvaluationContextRecord because I don't think I was using it correctly. A few other tests in sema failed when I tried pushing and popping from the stack holding these. I still process expressions as they are parsed though without using AST traversal on every expression.

[aaron.ballman]

include/clang/Basic/AttrDocs.td

3465

The compiler doesn't "throw" warnings, so I would probably reword this in terms of "diagnoses" similar to what @erik.pilkington was suggesting.

3465

This leaves me wondering what happens with references in C++? Or Obj-C pointer types that aren't modeled as a PointerType.

include/clang/Basic/DiagnosticGroups.td

1034

There's no need for this diagnostic group to be spelled out here as there's only one diagnostic in the group. You can use InGroup<"dereferenced-noderef"> directly in the diagnostic instead.

include/clang/Basic/DiagnosticSemaKinds.td

9445

The wording here isn't really accurate as the expression isn't what's marked noderef. Also, there may be multiple variables involved in the expression, so naming the variable would be useful.

How about: dereferencing %0; was declared with a noderef` type`

I'd also like to see a "declared here" note with the diagnostic.

include/clang/Sema/Sema.h

989

This is unrelated to the patch, but feel free to commit the fix separately (no code review needed).

1029–1031

I believe these can be private.

1039

I believe this can be private.

1042–1043

Rather than switch access control levels in the middle, can you move these with the other private data members?

lib/Sema/SemaExpr.cpp

4165

const auto *

4256

Rather than performing the lookup with find() and then performing the lookup a second time with erase(<key>), it'd be better to just use the iterator from the call to find().

4257

const auto *

4263

Similar suggestion here as above.

4281

const auto *

4281–4288

Can elide braces.

4283

const auto *

4290

const auto *

4293

const auto *

4294

Can elide braces.

lib/Sema/SemaExprMember.cpp

1724

Missing full stop at the end of the sentence.

1729

const auto *

test/Frontend/noderef.c

3

I would rather see a separate C++ test file that tests things like pointers to members, references, templates, etc.

[leonardchan]

include/clang/Basic/AttrDocs.td

3465

noderef doesn't do anything with references or ObjC pointers for now at least. I was initially attempting to replicate the noderef logic from sparse's validation tests which provides examples in a C file which was my only intention for now.

I would be open to extending this to work with references or ObjC pointers if there's demand for it or there are examples of how it could be used with these types.

test/Frontend/noderef.c

3

I probably shouldn't have included the -x c++ in the first place since, as far as I know, usage of noderef in sparse is only in C which is mainly what we're trying to support for now. Though I wouldn't mind extending this to C++ once I find examples of usage of this in C++ or fully flesh out how this should be used with references, templates, etc.

[leonardchan]

@aaron.ballman @rsmith Any more feedback on this patch?

[aaron.ballman]

include/clang/Basic/AttrDocs.td

3465

noderef doesn't do anything with references or ObjC pointers for now at least. I was initially attempting to replicate the noderef logic from sparse's validation tests which provides examples in a C file which was my only intention for now.

I'm fine with that for the initial patch, but the documentation should clearly spell out that references and Objective-C pointer types are not supported, and there should be logic to diagnose cases where the attribute does is applied to a non-pointer type.

include/clang/Basic/DiagnosticSemaKinds.td

9445

The formatting looks off here, seems to be > 80 col.

test/Frontend/noderef.c

3

Being primarily an attribute for C doesn't mean we should ignore the behavior of the attribute in C++ for testing purposes. This should have a C++ test file that tests the C++ spelling and ensures there are no surprises on C++ types that may be pointer-like (references, member pointers, etc). I think the same is true for Obj-C given that they have some odd pointer types. It's not that the attribute needs to be *supported* on those constructs, but we should have tests demonstrating what the actual behavior is expected to be for right now (e.g., ensuring it doesn't crash or assert, diagnoses as expected, etc).

This is also missing test cases ensuring the attribute appertains to the correct types, doesn't accept arguments, etc.

[rsmith]

In D49511#1170693, @leonardchan wrote:

Done. I opted for not using ExpressionEvaluationContextRecord because I don't think I was using it correctly. A few other tests in sema failed when I tried pushing and popping from the stack holding these. I still process expressions as they are parsed though without using AST traversal on every expression.

You shouldn't be adding your own ExpressionEvaluationContextRecords. What I was suggesting is that you store a list of pending noderef expressions on the record, and diagnose them when the record is popped (if they've not been removed since). Your current counter-based approach doesn't work very well in the case where we switch to another context while processing an expression (for example, during template instantiation): you'll defer all the diagnostics for the inner construct until the outer construct is complete. Generally global Sema state doesn't work very well for this reason.

That said... have you considered changing the specification of your attribute so that you warn on lvalue-to-rvalue conversions instead of warning on dereference-like things with a list of exceptions? That would be both simpler to implement and more precise (and it would naturally extend to C++, where a reference-to-noderef would be a reasonable type to support).

include/clang/Sema/Sema.h

1561

Do not use unordered_set here; see https://llvm.org/docs/ProgrammersManual.html#picking-the-right-data-structure-for-a-task

llvm::SmallPtrSet or llvm::DenseSet would be better choices here, but I think just using a SmallVector would be fine.

lib/Sema/SemaExpr.cpp

14230–14242

I don't see any reason to assume that the DeclRefExpr found by this visitor will be the one you're interested in (the one the noderef attribute came from).

How about instead you walk over only expressions that you *know* preserve noderef, and if you can't find a declaration as the source of the noderef attribute, produce a differently-worded diagnostic that doesn't give a variable name?

14252–14257

This will crash if the DeclRefFinder doesn't find a DeclRefExpr. What justifies the assumption that it will always succeed?

[mcgrathr]

I think this wants to be a hard error rather than a warning. Though since we use -Werror anyway if others feel strongly contrary I won't object.

[leonardchan]

• Added tests for checking that noderef can only be used for C style pointers and arrays (not usable for references or ObjC pointers)
• Added warnings for dereferencing a noderef expression where we cannot trace back the original declrefexpr.
• Added warning for adding noderef on a type that noderef cannot be attatched to.
• Added warning group.
• Use SmallPtrSet instead of std::unordered_set

[leonardchan]

You shouldn't be adding your own ExpressionEvaluationContextRecords. What I was suggesting is that you store a list of pending noderef expressions on the record, and diagnose them when the record is popped (if they've not been removed since).

This is what I initially tried but the last record left on the stack, which is what I add them to, wasn't getting popped. While parsing, the stack of records (from what I've observed) has only one record that doesn't get popped via PopExpressionEvaluationContext. I'm still looking into ways to try to do this though.

That said... have you considered changing the specification of your attribute so that you warn on lvalue-to-rvalue conversions instead of warning on dereference-like things with a list of exceptions?

I'll look into this for the next revision on this patch.

[leonardchan]

Your current counter-based approach doesn't work very well in the case where we switch to another context while processing an expression (for example, during template instantiation): you'll defer all the diagnostics for the inner construct until the outer construct is complete. Generally global Sema state doesn't work very well for this reason.

Alternatively, I could move the set, counter, and logic using them into the ExpressionEvaluationContextRecord. Instead of tracking relative to a global Sema state, it would be relative to the latest record on the stack.

[leonardchan]

• Moved the counter and set into ExpressionEvaluationContextRecord

[aaron.ballman]

include/clang/Basic/AttrDocs.td

3505

I would drop the "C style" and just say it's only supported for pointers and arrays.

3506

I'd clarify this a little bit to Objective-C object pointers.

include/clang/Basic/DiagnosticSemaKinds.td

9445

Please use single quotes instead of backticks in the quoting around noderef in these diagnostics.

[leonardchan]

• Changed tick to single quote in diagnostic

[leonardchan]

@rsmith any more feedback on this current version? If it still looks incorrect to use the record this way, I don't mind simplifying it to work on lvalue to rvalue conversions without checking for a leading address space operation.

[aaron.ballman]

lib/Sema/SemaExpr.cpp

14242–14244

I don't think this strips off sugar from the type, so I suspect it won't properly handle a typedef to a pointer type, for instance, or a type including parens. You should add tests for these cases.

[aaron.ballman]

Oops, I hit "Send" too soon. I was going to say that you should also keep an eye on D50526 because that may impact this patch (or vice versa if this one lands first).

[leonardchan]

• Checks for sugared types and expressions wrapped in parenthesis

[aaron.ballman]

lib/Sema/SemaExpr.cpp

14249

The sugar was stripped off at the pointer level, but not at the pointee level. e.g.,

typedef int (bobble);
typedef bobble * (frobble);
typedef frobble * yobble;

yobble gobble;

I think you can handle this within TypeHasNoDeref() and that should fix up all the callers.

[leonardchan]

@rsmith

lib/Sema/SemaExpr.cpp

14249

So TypeHasNoDeref() checks for the attribute on the base type already and is called after the pointer is stripped off. Attempting to desugar via getDesugaredType() here also removes the address_space attribute from the type I'm checking.

Do you know another method that essentially "expands" the typedefs without stripping the qualifiers? Otherwise I think I do need do the desugaring at the pointer level. Alternatively I could also change this method such that it accepts pointers instead of pointees since it appears I already call getDesugaredType() for almost every pointer who's pointee I'm passing to TypeHasNoDeref().

Also I tested with your example and the warning still seems to be thrown appropriately.

[aaron.ballman]

lib/Sema/SemaExpr.cpp

14249

I think you have to do the desugaring manually in a loop with getSingleStepDesugaredType() so that you don't strip off attributed type information along with the rest of the type sugar.

Also I tested with your example and the warning still seems to be thrown appropriately.

The example may depend on where you put the attribute (inside the parens vs outside the parens, for instance); it was an off-the-cuff example, so it may need some tweaking.

[leonardchan]

• Remove sugar from pointee types

lib/Sema/SemaExpr.cpp

14249

Done. Also added different variations of your example to the tests.

[aaron.ballman]

A few more minor nits to be cleared up, but otherwise LGTM. You should wait for @rsmith to sign off before committing in case he has further comments, however.

lib/Sema/SemaExpr.cpp

4165

Missing a full stop at the end of the comment.

4167

I'd prefer to use !isa<AttributedType>(Ty); I think that's more clear than looking at the enumeration.

test/Frontend/noderef.c

154

Typo: Parentheses

[rsmith]

lib/Parse/ParseExpr.cpp

1126

This parser-driven start/stop mechanism will not work in C++ templates. Instead, can you remove the "start" part and check the noderef exprs as part of popping the ExprEvaluationContextRecord?

lib/Sema/SemaExpr.cpp

4260

You need to loop/recurse here; there could be a sequence of . member accesses to strip.

4284

Did you mean to use ElemTy somewhere below? (I'd guess you intended to check whether it's marked noderef?)

14230–14242

This comment is marked done but has not been addressed.

lib/Sema/SemaType.cpp

4209

This is not correct (there could be multiple attributes on the type). See below.

4816

Move this after the check below (directly setting ExpectNoDerefChunk instead), and remove the unnecessary IsNoDeref variable.

4816

This is not correct: there could be multiple attributes at this level. You could fix this locally either by looping through the attributes on the type or iterating through the ParsedAttrs on the chunk. But I think my preference would be to store state indicating that we have an unvalidated noderef attribute on the TypeProcessingState at the point where you create the AttributedType, and check and clear that flag here when creating a type chunk.

[rsmith]

In D49511#1194716, @leonardchan wrote:

@rsmith any more feedback on this current version? If it still looks incorrect to use the record this way, I don't mind simplifying it to work on lvalue to rvalue conversions without checking for a leading address space operation.

I've been thinking more about cleaner ways to implement this (and in particular, approaches that will provide more reasonable semantics in C++ -- allowing references to noderef, for example). We want to disallow operands of type noderef T to all operations by default, and only allow very specific operations on lvalues of type noderef T -- taking the address, performing member accesses, lvalue casts. The most natural way to get that effect would be to add a new form of placeholder type for a "dereferenced noderef" expression, that CheckPlaceholderExpr rejects, and that we add explicit support for in the contexts where such a construct is valid. (This is similar to how we handle overloaded function names and bound member function expressions in C++, for example.) (When we reach a context that "consumes" a dereferenced noderef expression, we'd need to go back and patch up its type, but I think that can be handled in a straightforward way.)

I think we should also treat noderef more like a type qualifier: as an important example, if we have a pointer or reference to noderef struct X, then member access for a member of type T should give an lvalue of type noderef T.

[rsmith]

In D49511#1206265, @rsmith wrote:

In D49511#1194716, @leonardchan wrote:

@rsmith any more feedback on this current version? If it still looks incorrect to use the record this way, I don't mind simplifying it to work on lvalue to rvalue conversions without checking for a leading address space operation.

I've been thinking more about cleaner ways to implement this (and in particular, approaches that will provide more reasonable semantics in C++ -- allowing references to noderef, for example). We want to disallow operands of type noderef T to all operations by default, and only allow very specific operations on lvalues of type noderef T -- taking the address, performing member accesses, lvalue casts. The most natural way to get that effect would be to add a new form of placeholder type for a "dereferenced noderef" expression, that CheckPlaceholderExpr rejects, and that we add explicit support for in the contexts where such a construct is valid. (This is similar to how we handle overloaded function names and bound member function expressions in C++, for example.) (When we reach a context that "consumes" a dereferenced noderef expression, we'd need to go back and patch up its type, but I think that can be handled in a straightforward way.)

I think we should also treat noderef more like a type qualifier: as an important example, if we have a pointer or reference to noderef struct X, then member access for a member of type T should give an lvalue of type noderef T.

Sorry, I forgot to say: I do not think we need to do this for the initial version of this functionality. This is mostly about cleanly extending the functionality to cover more C++ constructs.

[leonardchan]

lib/Parse/ParseExpr.cpp

1126

I'm not sure if I should remove the the start and stop methods because for a regular C program, the Push/PopExprEvaluationContextRecord functions don't seem to be called, and even when they are called in C++, the initial record that exists on the stack isn't popped at all.

Since pending noderef expressions are still parsed and added to the last record during template instantiation, doing another check when popping covers all noderef exprs added during instantiation.

lib/Sema/SemaExpr.cpp

4284

Forgot to remove this.

14230–14242

My bad. My response to this was adding a check in StopCheckingNoDerefAndWarn where we check if the DeclRefExpr is nullptr and throw the appropriate error if it is found or not.

Although I can't come up with an example where this doesn't get the declref we want, since the expressions we pass to this visitor are all our marked noderef expressions (which we check via the attribute on the type). So even if we have a complex expression that has multiple declrefs, all noderef declrefs should be checked by this.

lib/Sema/SemaType.cpp

4816

After https://reviews.llvm.org/D50526, it seems that the attribute can also be applied in ConvertDeclSpecToType, so I added another check for the attributes there.

[leonardchan]

In D49511#1206267, @rsmith wrote:

In D49511#1206265, @rsmith wrote:

In D49511#1194716, @leonardchan wrote:

@rsmith any more feedback on this current version? If it still looks incorrect to use the record this way, I don't mind simplifying it to work on lvalue to rvalue conversions without checking for a leading address space operation.

I've been thinking more about cleaner ways to implement this (and in particular, approaches that will provide more reasonable semantics in C++ -- allowing references to noderef, for example). We want to disallow operands of type noderef T to all operations by default, and only allow very specific operations on lvalues of type noderef T -- taking the address, performing member accesses, lvalue casts. The most natural way to get that effect would be to add a new form of placeholder type for a "dereferenced noderef" expression, that CheckPlaceholderExpr rejects, and that we add explicit support for in the contexts where such a construct is valid. (This is similar to how we handle overloaded function names and bound member function expressions in C++, for example.) (When we reach a context that "consumes" a dereferenced noderef expression, we'd need to go back and patch up its type, but I think that can be handled in a straightforward way.)

I think we should also treat noderef more like a type qualifier: as an important example, if we have a pointer or reference to noderef struct X, then member access for a member of type T should give an lvalue of type noderef T.

Sorry, I forgot to say: I do not think we need to do this for the initial version of this functionality. This is mostly about cleanly extending the functionality to cover more C++ constructs.

No problem. After this, I will make another patch that does this.

[rsmith]

lib/Parse/ParseExpr.cpp

1126

PushExpressionEvaluationContext / PopExpressionEvaluationContext are called regardless of which language we're parsing. If we're missing ExpressionEvaluationContext records around some expression parsing, we should fix that. We should never be creating expressions within the initial ExpressionEvaluationContext record (except perhaps during error recovery).

Since pending noderef expressions are still parsed and added to the last record during template instantiation, doing another check when popping covers all noderef exprs added during instantiation.

That's not how template instantiation works in Clang. We don't re-parse, we perform a recursive tree transformation that does not involve the parser.

lib/Sema/SemaExpr.cpp

4164–4177

You can just use Ty->hasAttr(attr::NoDeref) for this now.

4256–4259

Do you need to do this both here and after walking into the . expressions? It seems to me that it would be sufficient to first walk into the LHS of any . expressions, and then remove the result from PossibleDerefs.

4262

You should check the MemberExpr is a . rather than a -> before stepping into it.

4271

This type check seems incorrect, because you don't propagate the type through -> operators. In particular, given

`&noderef_ptr->x.y`

you will step back to noderef_ptr->x, which will not have a noderef type, and not remove it from the set as a result. It seems best to drop this type check.

4282

Do you ensure that the noderef attribute will be on the innermost level of the array type? I think you'll need to do so in order to warn on:

typedef int A[32];
typedef A __attribute__((noderef)) *B;
int f(B b) { return (*B)[1]; }

(Here, we have a pointer to a noderef annotated array of non-noderef-annotated int. So I think we will not emit a warning from the first dereference, because we have a pointer to an array, and we will not emit a warning from the second dereference in the array indexing, because the result type does not have the noderef attribute.)

4290

QualTypes should be held by value, not by reference.

4295–4301

There could be a sequence of MemberExprs here, and watch out for . vs ->.

12866–12867

You should probably not add the expression to the list if it's of array type. Given:

typedef int A[32];
typedef A __attribute__((noderef)) *B;
B b;
int __attribute__((noderef)) *p = *b;

... you should not warn on the "dereference" here. That should either be handled here or by treating an array-to-pointer decay operation the same as an "address of" operation and removing pending derefs from the list.

14230–14242

How about this:

int __attribute__((noderef)) *a;
int __attribute__((noderef)) *b;
int __attribute__((noderef)) *c;
*(a = b, c);

The first noderef pointer the visitor finds will be a, but that's not the one that was dereferenced.

14242–14244

Calling getDesugaredType is not the right way to address this: use Ty->getAs<PointerType>() and Context.getAsArrayType(Ty) instead.

[leonardchan]

lib/Parse/ParseExpr.cpp

1126

So when should a new ExpressionEvaluationContext be pushed or popped?

For the following code:

#define NODEREF __attribute__((noderef))

void func(){
  int NODEREF *x;
  *x;
}

int main(){
  func();
}

A new context is pushed then popped in C++ but not for C. From what I can tell based off my observations and looking for where Push/Pop get called in code, new ones would get added when we enter a new GNU style statement expression, captured region after a pragma, or different error blocks.

lib/Sema/SemaExpr.cpp

4282

Hmmmm, so normally in order to check what's correct, I usually run these examples through sparse since that's the tool that actually checks noderef for gcc, but it seems that sparse instead diagnoses a warning on the array indexing instead and nothing for the first dereference.

This shouldn't be though since, as you pointed out, the array does not have noderef types. For a simple array,

int __attribute__((noderef)) x[10];
x[0];

sparse diagnoses the appropriate warning for the array index. Personally though, I would chalk this up to an edge case that wasn't thought of before in sparse, since something like this isn't handled on their existing validation tests.

Currently, this diagnoses a warning on the first dereference, but I can also understand why it shouldn't warn because accessing noderef structs shouldn't warn if the member accessed is an array. The only case though this applies in sparse's tests are with structs and they don't provide a test for dereferencing a pointer to an array.

I think the reason for why the struct example still passes is that if the member is an array, none of the actual data in the struct is being accessed. Instead, the value returned is just a pointer to the start of the array within the struct and equivalent to just adding an offset to the struct pointer. I think that no warning should also be thrown for this example if it is guaranteed that array A can similarly be found from simple pointer arithmetic on pointer B. I don't think it can unless B were an array or struct also, but I could be wrong or overthinking this.

[leonardchan]

@rsmith ping

[leonardchan]

• Push and pop a new ExpressionEvaluationContext when we enter and exit a compound statement.
• Remove Start/StopCheckingNoderef functions since we can now warn whenever we pop

[leonardchan]

• Push and pop contexts for every parsed statement inside ParseStatementOrDeclaration instead of at the start and end of compound statements

[rsmith]

include/clang/Basic/AttrDocs.td

3511

Should noderef appear somewhere in this example? :)

include/clang/Basic/DiagnosticSemaKinds.td

9448–9449

Putting this under -Wonderef doesn't really seem appropriate to me -- you're using -Wnoderef above to mean "warn about cases where a noderef pointer is dereferenced", whereas this warning is about invalid use of an attribute. I think -Wignored-attributes (InGroup<IgnoredAttributes>) would be a better choice.

lib/Parse/ParseExpr.cpp

1126

Hmm, it looks like we don't push/pop expression evaluation context records for function definitions or variable initializers currently (instead we have a single global "potentially evaluated" context which wraps all such contexts and appears to never be popped).

The fact that we share a single record for all functions / variables is likely simply because it never previously mattered that we were merging notionally-distinct contexts in this way. I suspect actually fixing that will uncover more problems (most likely, pre-existing bugs, but barriers to your progress nonetheless).

lib/Parse/ParseStmt.cpp

102–104 ↗

(On Diff #164706)

Hmm, we call ParseStatementOrDeclaration rather a lot. Can we pull this up into its ultimate callers instead:

• Parser::ParseFunctionDefinition
• Parser::ParseLateTemplatedFuncDef
• Parser::ParseLexedMethodDef
• Parser::ParseLexedObjCMethodDefs

? Actually, we can do better than that: those functions all call either ActOnStartOfFunctionDef or ActOnStartOfObjCMethodDef first, and ActOnFinishFunctionBody when they're done. We should put the PushExpressionEvaluationContext and PopExpressionEvaluationContext calls in those functions instead.

We're missing at least one other place: when parsing the initializer for a global variable, in C++ we call Sema::ActOnCXXEnterDeclInitializer, which pushes a potentially-evaluated context. But in C, the InitializerScopeRAII object (in Parser::ParseDeclaratoinAfterDeclaratorAndAttributes) doesn't call into Sema and we don't ever push a suitable expression evaluation context.

You can find if any other places are missing by removing Sema's global ExpressionEvaluationContext and adding an assert that fires if we try to parse an expression with no ExpressionEvaluationContext, and then running the unit tests. (And we should check in that change to ensure this doesn't regress.)

lib/Sema/SemaExpr.cpp

4164

If it makes sense for this to exist at all, it should be a member of Type instead.

4263–4265

Replace these three lines with just

LastRecord.PossibleDerefs.erase(StrippedExpr);

4282

Arrays-of-arrays and structs-containing-arrays are the same in this regard: in both cases, accessing the first level (array element or struct member) gives you an array lvalue, and only accessing that second array will actually dereference memory. So I think the treatment of the two cases should be consistent. For member access, you defer the check if the element type is an array, and I think you should do the same thing here.

There are two consequences of this:

1. If the array element type is an array, you should bail out of here and not add E to PossibleDerefs.
2. If the array type (not the element type) has the noderef attribute, you should propagate it down to the array element type so that we can catch a later pointer dereference / array indexing operation.

For point 2, what this means in my example is that the expression *B should have type "array of 32 noderef ints" so that further noderef checks apply to it. The easiest way to accomplish this would be to check, when applying noderef to a type, whether the type is an array type, and if so also apply noderef to the array's element type.

test/Frontend/noderef_on_non_pointers.cpp

5

I'd like to see a test that we get a wraning for attempting to bind a reference to a dereferenced noderef pointer.

In C++, should it be valid to assign a noderef pointer to a dereferenceable pointer without a cast?

[mcgrathr]

The motivating use case always pairs noderef with an address_space attribute, and it's already invalid to convert between pointer types in different address spaces without a cast.
So I don't think we have a strong opinion one way or the other. In the abstract, I'd say noderef feels like a "constraining" qualifier a la const/volatile so that going from unconstrained to constrained implicitly is OK but not vice versa.

[leonardchan]

lib/Parse/ParseStmt.cpp

102–104 ↗

(On Diff #164706)

@rsmith Would it be simpler to instead push and pop at the start and end of Parser::ParseExternalDeclaration? I'm currently working on your suggestion of removing the Sema global ExpressionEvaluationContext and find that a lot of accesses to the ExprEvalContexts stack stem from this method.

ActOnStartOfFunctionDef and ActOnFinishFunctionBody still work on anything in a function body, but I keep running into many cases for declarations declared globally that could be easily caught if instead I push and pop contexts at the start and end of Parser::ParseExternalDeclaration. Do you think this is a good idea or is there something that I may be missing from pushing and popping here?

This still accomplishes the goal of not reusing the global Sema context and I will still be able to check for noderef on every push and pop this way.

[leonardchan]

lib/Parse/ParseStmt.cpp

102–104 ↗

(On Diff #164706)

Made a separate patch (https://reviews.llvm.org/D54014) that does the push and pop for ActOnStartOfFunctionDef and ActOnFinishFunctionBody.

I can continue doing this for the remaining places that depend on the global Sema context in other patches since I ran into a couple of issues running into fixing everything in one big patch and think fixing each individual place in smaller separate patches would be simpler.

lib/Sema/SemaExpr.cpp

4282

There currently doesn't seem to be a simple way, that I could find, for editing the element type of an array type. I could alternatively just recreate the array type when wrapping one with the noderef AttributedType, but this would be difficult if the array type was surrounded by type sugar since the sugar would need to somehow be recreated on the new AttributedType.

Is it possible to do something like a tree transformation where I can change the element type of an array type itself without having to recreate the whole type with sugar?

test/Frontend/noderef_on_non_pointers.cpp

5

Ah, I forgot about this case. Sparse normally has a different warning for casting involving it's attributes (which includes a warning when casting from a noderef pointer to regular pointer) , but I didn't focus on that since it seemed like a whole other feature.

I can see though how it would be problematic if a user unknowingly did something like cast to a dereferenceable pointer. Added a warning and tests for this.

[leonardchan]

*ping* @rsmith Any more comments on this patch or the one before it (https://reviews.llvm.org/D54014)? That one has the fix for pushing and popping another ExprEvalContext before acting on a function body in this patch.

[rsmith]

Thanks for your patience.

clang/lib/Sema/SemaInit.cpp

7838–7854 ↗

(On Diff #172695)

CheckSingleAssignmentConstraints also warns in this case; is the explicit warning emission above necessary?

[dblaikie]

lib/Sema/SemaType.cpp

4820–4823

Pointed out in PR41774 there's a dead store to ExpectNoDerefChunk here.

Should line 4913 be removed (& then the two if conditions can be collapsed together)?

[leonardchan]

lib/Sema/SemaType.cpp

4820–4823

Yup, fixed in r360089

include/clang/AST/Type.h

enum Kind {
     attr_objc_ownership,
     attr_pcs,
     attr_pcs_vfp,
 
     FirstEnumOperandKind = attr_objc_gc,
     LastEnumOperandKind = attr_pcs_vfp,
 
     // No operand.
+    attr_noderef,
     attr_noreturn,
     attr_nocf_check,
     attr_cdecl,
     attr_fastcall,
     attr_stdcall,
     attr_thiscall,
     attr_regcall,
     attr_pascal,

include/clang/Basic/Attr.td

 }
 
 def Regparm : TypeAttr {
   let Spellings = [GCC<"regparm">];
   let Args = [UnsignedArgument<"NumParams">];
   let Documentation = [RegparmDocs];
 }
 
+def NoDeref : TypeAttr {
+  let Spellings = [Clang<"noderef">];
+  let Documentation = [NoDerefDocs];
+}
+
 def ReqdWorkGroupSize : InheritableAttr {
   // Does not have a [[]] spelling because it is an OpenCL-related attribute.
   let Spellings = [GNU<"reqd_work_group_size">];
   let Args = [UnsignedArgument<"XDim">, UnsignedArgument<"YDim">,
               UnsignedArgument<"ZDim">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [Undocumented];
 }

include/clang/Basic/AttrDocs.td

def ArtificialDocs : Documentation {
   let Content = [{
 The ``artificial`` attribute can be applied to an inline function. If such a
 function is inlined, the attribute indicates that debuggers should associate
 the resulting instructions with the call site, rather than with the
 corresponding line within the inlined callee.
   }];
 }
 
+def NoDerefDocs : Documentation {
+  let Category = DocCatType;
+  let Content = [{
+The ``noderef`` attribute causes clang to diagnose dereferences of annotated pointer types.
+This is ideally used with pointers that point to special memory which cannot be read
+from or written to, but allowing for the pointer to be used in pointer arithmetic.
+The following are examples of valid expressions where dereferences are diagnosed:
+
+.. code-block:: c
+
+  int __attribute__((noderef)) *p;
+  int x = *p;  // warning
+
+  int __attribute__((noderef)) **p2;
+  x = **p2;  // warning
+
+  int * __attribute__((noderef)) *p3;
+  p = *p3;  // warning
+
+  struct S {
+    int a;
+  };
+  struct S __attribute__((noderef)) *s;
+  x = s->a;    // warning
+  x = (*s).a;  // warning
+
+Not all dereferences may diagnose a warning if the value directed by the pointer may not be
+accessed. The following are examples of valid expressions where may not be diagnosed:
+
+.. code-block:: c
+
+  int *q;
+  int __attribute__((noderef)) *p;
+  q = &*p;
+  q = *&p;
+
+  struct S {
+    int a;
+  };
+  struct S __attribute__((noderef)) *s;
+  p = &s->a;
+  p = &(*s).a;
+
+``noderef`` is currently only supported for pointers and arrays and not usable for
+references or Objective-C object pointers.
+
+.. code-block: c++
+
+  int x = 2;
+  int &y = x;  // warning: 'noderef' can only be used on an array or pointer type
+
+.. code-block: objc
+
+  id NODEREF obj = [NSObject new]; // warning: 'noderef' can only be used on an array or pointer type
+}];
+}
+
 def ReinitializesDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 The ``reinitializes`` attribute can be applied to a non-static, non-const C++
 member function to indicate that this member function reinitializes the entire
 object to a known state, independent of the previous state of the object.
 
 This attribute can be interpreted by static analyzers that warn about uses of an

include/clang/Basic/DiagnosticGroups.td

 def SpirCompat : DiagGroup<"spir-compat">;
 
 // Warning for the experimental-isel options.
 def ExperimentalISel : DiagGroup<"experimental-isel">;
 
 // A warning group specifically for warnings related to function
 // multiversioning.
 def FunctionMultiVersioning : DiagGroup<"function-multiversion">;
+
+def NoDeref : DiagGroup<"noderef">;

include/clang/Basic/DiagnosticSemaKinds.td

def err_spaceship_argument_narrowing : Error<
   "%select{cannot be narrowed from type %1 to %2|"
   "evaluates to %1, which cannot be narrowed to type %2}0">;
 def err_std_compare_type_not_supported : Error<
   "standard library implementation of %0 is not supported; "
    "%select{member '%2' does not have expected form|"
    "member '%2' is missing|"
    "the type is not trivially copyable|"
    "the type does not have the expected form}1">;
+
+def warn_dereference_of_noderef_type : Warning<
+  "dereferencing %0; was declared with a 'noderef' type">, InGroup<NoDeref>;
+def warn_dereference_of_noderef_type_no_decl : Warning<
+  "dereferencing expression marked as 'noderef'">, InGroup<NoDeref>;
+def warn_noderef_on_non_pointer_or_array : Warning<
+  "'noderef' can only be used on an array or pointer type">, InGroup<NoDeref>;
 } // end of sema component.

include/clang/Sema/Sema.h

struct ExpressionEvaluationContextRecord {
     /// If we are processing a decltype type, a set of call expressions
     /// for which we have deferred checking the completeness of the return type.
     SmallVector<CallExpr *, 8> DelayedDecltypeCalls;
 
     /// If we are processing a decltype type, a set of temporary binding
     /// expressions for which we have deferred checking the destructor.
     SmallVector<CXXBindTemporaryExpr *, 8> DelayedDecltypeBinds;
 
+    unsigned NoDerefCallCount;
+    llvm::SmallPtrSet<const Expr *, 8> PossibleDerefs;
+
     /// \brief Describes whether we are in an expression constext which we have
     /// to handle differently.
     enum ExpressionKind {
       EK_Decltype, EK_TemplateArgument, EK_Other
     } ExprContext;
 
     ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
                                       unsigned NumCleanupObjects,
                                       CleanupInfo ParentCleanup,
                                       Decl *ManglingContextDecl,
                                       ExpressionKind ExprContext)
         : Context(Context), ParentCleanup(ParentCleanup),
           NumCleanupObjects(NumCleanupObjects), NumTypos(0),
           ManglingContextDecl(ManglingContextDecl), MangleNumbering(),
-          ExprContext(ExprContext) {}
+          NoDerefCallCount(0), ExprContext(ExprContext) {}
 
     /// Retrieve the mangling numbering context, used to consistently
     /// number constructs like lambdas for mangling.
     MangleNumberingContext &getMangleNumberingContext(ASTContext &Ctx);
 
     bool isUnevaluated() const {
       return Context == ExpressionEvaluationContext::Unevaluated ||
              Context == ExpressionEvaluationContext::UnevaluatedAbstract ||
              Context == ExpressionEvaluationContext::UnevaluatedList;
     }
     bool isConstantEvaluated() const {
       return Context == ExpressionEvaluationContext::ConstantEvaluated;
     }
   };
 
   /// A stack of expression evaluation contexts.
   SmallVector<ExpressionEvaluationContextRecord, 8> ExprEvalContexts;
 
+  /// Not all expressions need to be checked, so we only need to start keeping
+  /// track of noderef expressions on expressions that involve dereferencing
+  /// (indirection, member access, array subscript, ...).
+  void StartCheckingNoDeref() { ExprEvalContexts.back().NoDerefCallCount++; }
+  void StopCheckingNoDerefAndWarn();
+
   /// Compute the mangling number context for a lambda expression or
   /// block literal.
   ///
   /// \param DC - The DeclContext containing the lambda expression or
   /// block literal.
   /// \param[out] ManglingContextDecl - Returns the ManglingContextDecl
   /// associated with the context, if relevant.
   MangleNumberingContext *getCurrentMangleNumberContext(
     const DeclContext *DC,
     Decl *&ManglingContextDecl);
 
 
   /// SpecialMemberOverloadResult - The overloading result for a special member
   /// function.
   ///
   /// This is basically a wrapper around PointerIntPair. The lowest bits of the
   /// integer are used to determine whether overload resolution succeeded.
   class SpecialMemberOverloadResult {
   public:
     enum Kind {
       NoMemberOrDeleted,
       Ambiguous,
       Success
     };
public:
 
     void diagnose(Sema &S, SourceLocation Loc, QualType T) override {
       const SemaDiagnosticBuilder &DB = S.Diag(Loc, DiagID);
       emit(DB, llvm::index_sequence_for<Ts...>());
       DB << T;
     }
   };
 
+  static bool TypeHasNoDeref(const QualType &Ty);
+
 private:
+  /// Methods for marking which expressions involve dereferencing a pointer
+  /// marked with the 'noderef' attribute. Expressions are checked bottom up as
+  /// they are parsed, meaning that a noderef pointer may not be accessed. For
+  /// example, in `&*p` where `p` is a noderef pointer, we will first parse the
+  /// `*p`, but need to check that `address of` is called on it. This requires
+  /// keeping a container of all pending expressions and checking if the address
+  /// of them are eventually taken.
+  void CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E);
+  void CheckAddressOfNoDeref(const Expr *E);
+  void CheckMemberAccessOfNoDeref(const MemberExpr *E);
+
   bool RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
                                TypeDiagnoser *Diagnoser);
 
   struct ModuleScope {
     clang::Module *Module = nullptr;
     bool ModuleInterface = false;
     VisibleModuleSet OuterVisibleModules;
   };
   /// The modules we're currently parsing.
   llvm::SmallVector<ModuleScope, 16> ModuleScopes;
 
   /// Get the module whose scope we are currently within.
   Module *getCurrentModule() const {
     return ModuleScopes.empty() ? nullptr : ModuleScopes.back().Module;
   }

lib/AST/Type.cpp

bool AttributedType::isQualifier() const {
   case AttributedType::attr_pascal:
   case AttributedType::attr_swiftcall:
   case AttributedType::attr_vectorcall:
   case AttributedType::attr_inteloclbicc:
   case AttributedType::attr_preserve_most:
   case AttributedType::attr_preserve_all:
   case AttributedType::attr_ms_abi:
   case AttributedType::attr_sysv_abi:
+  case AttributedType::attr_noderef:
   case AttributedType::attr_ptr32:
   case AttributedType::attr_ptr64:
   case AttributedType::attr_sptr:
   case AttributedType::attr_uptr:
   case AttributedType::attr_objc_kindof:
   case AttributedType::attr_ns_returns_retained:
   case AttributedType::attr_nocf_check:
     return false;
bool AttributedType::isMSTypeSpec() const {
   case attr_uptr:
     return true;
   }
   llvm_unreachable("invalid attr kind");
 }
 
 bool AttributedType::isCallingConv() const {
   switch (getAttrKind()) {
+  case attr_noderef:
   case attr_ptr32:
   case attr_ptr64:
   case attr_sptr:
   case attr_uptr:
   case attr_address_space:
   case attr_regparm:
   case attr_vector_size:
   case attr_neon_vector_type:

lib/AST/TypePrinter.cpp

void TypePrinter::printAttributedAfter(const AttributedType *T,
   case AttributedType::attr_inteloclbicc: OS << "inteloclbicc"; break;
   case AttributedType::attr_preserve_most:
     OS << "preserve_most";
     break;
 
   case AttributedType::attr_preserve_all:
     OS << "preserve_all";
     break;
+  case AttributedType::attr_noderef:
+    OS << "noderef";
+    break;
   }
   OS << "))";
 }
 
 void TypePrinter::printObjCInterfaceBefore(const ObjCInterfaceType *T,
                                            raw_ostream &OS) {
   OS << T->getDecl()->getName();
   spaceBeforePlaceHolder(OS);

lib/Parse/ParseExpr.cpp

case tok::minusminus: {  // unary-expression: '--' unary-expression [C99]
     if (!Res.isInvalid())
       Res = Actions.ActOnUnaryOp(getCurScope(), SavedTok.getLocation(),
                                  SavedKind, Res.get());
     return Res;
   }
   case tok::amp: {         // unary-expression: '&' cast-expression
     // Special treatment because of member pointers
     SourceLocation SavedLoc = ConsumeToken();
+
+    Actions.StartCheckingNoDeref();
+
     Res = ParseCastExpression(false, true);
     if (!Res.isInvalid())
       Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Res.get());
+
+    Actions.StopCheckingNoDerefAndWarn();
+
     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]
     SourceLocation SavedLoc = ConsumeToken();
+
+    Actions.StartCheckingNoDeref();
+
     Res = ParseCastExpression(false);
     if (!Res.isInvalid())
       Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Res.get());
+
+    Actions.StopCheckingNoDerefAndWarn();
+
     return Res;
   }
 
   case tok::kw_co_await: {  // unary-expression: 'co_await' cast-expression
     SourceLocation CoawaitLoc = ConsumeToken();
     Res = ParseCastExpression(false);
     if (!Res.isInvalid())
       Res = Actions.ActOnCoawaitExpr(getCurScope(), CoawaitLoc, Res.get());
case tok::l_square: {  // postfix-expression: p-e '[' expression ']'
 
       ExprResult OrigLHS = LHS;
       if (!LHS.isInvalid() && !Idx.isInvalid() && !Length.isInvalid() &&
           Tok.is(tok::r_square)) {
         if (ColonLoc.isValid()) {
           LHS = Actions.ActOnOMPArraySectionExpr(LHS.get(), Loc, Idx.get(),
                                                  ColonLoc, Length.get(), RLoc);
         } else {
+          Actions.StartCheckingNoDeref();
+
           LHS = Actions.ActOnArraySubscriptExpr(getCurScope(), LHS.get(), Loc,
                                                 Idx.get(), RLoc);
+
+          Actions.StopCheckingNoDerefAndWarn();
         }
       } else {
         LHS = ExprError();
       }
       if (LHS.isInvalid()) {
         (void)Actions.CorrectDelayedTyposInExpr(OrigLHS);
         (void)Actions.CorrectDelayedTyposInExpr(Idx);
         (void)Actions.CorrectDelayedTyposInExpr(Length);
case tok::period: {
                                     /*AllowConstructorName=*/
                                       getLangOpts().MicrosoftExt,
                                     /*AllowDeductionGuide=*/false,
                                     ObjectType, &TemplateKWLoc, Name)) {
         (void)Actions.CorrectDelayedTyposInExpr(LHS);
         LHS = ExprError();
       }
 
-      if (!LHS.isInvalid())
+      if (!LHS.isInvalid()) {
+        Actions.StartCheckingNoDeref();
+
         LHS = Actions.ActOnMemberAccessExpr(getCurScope(), LHS.get(), OpLoc,
                                             OpKind, SS, TemplateKWLoc, Name,
                                  CurParsedObjCImpl ? CurParsedObjCImpl->Dcl
                                                    : nullptr);
+
+        Actions.StopCheckingNoDerefAndWarn();
+      }
       if (!LHS.isInvalid() && Tok.is(tok::less))
         checkPotentialAngleBracket(LHS);
       break;
     }
     case tok::plusplus:    // postfix-expression: postfix-expression '++'
     case tok::minusminus:  // postfix-expression: postfix-expression '--'
       if (!LHS.isInvalid()) {
         LHS = Actions.ActOnPostfixUnaryOp(getCurScope(), Tok.getLocation(),

lib/Sema/SemaExpr.cpp

 
 static bool isMSPropertySubscriptExpr(Sema &S, Expr *Base) {
   auto *BaseNoParens = Base->IgnoreParens();
   if (auto *MSProp = dyn_cast<MSPropertyRefExpr>(BaseNoParens))
     return MSProp->getPropertyDecl()->getType()->isArrayType();
   return isa<MSPropertySubscriptExpr>(BaseNoParens);
 }
 
+bool Sema::TypeHasNoDeref(const QualType &Ty) {
+  if (const auto *AT = dyn_cast<AttributedType>(Ty)) {
+    AttributedType::Kind AttrKind = AT->getAttrKind();
+    return AttrKind == AttributedType::attr_noderef;
+  }
+  return false;
+}
+
 ExprResult
 Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc,
                               Expr *idx, SourceLocation rbLoc) {
   if (base && !base->getType().isNull() &&
       base->getType()->isSpecificPlaceholderType(BuiltinType::OMPArraySection))
     return ActOnOMPArraySectionExpr(base, lbLoc, idx, SourceLocation(),
                                     /*Length=*/nullptr, rbLoc);
 
   // Since this might be a postfix expression, get rid of ParenListExprs.
   if (isa<ParenListExpr>(base)) {
     ExprResult result = MaybeConvertParenListExprToParenExpr(S, base);
     if (result.isInvalid()) return ExprError();
     base = result.get();
   }
Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, SourceLocation lbLoc,
   // to overload resolution and so should not take this path.
   if (getLangOpts().CPlusPlus &&
       (base->getType()->isRecordType() ||
        (!base->getType()->isObjCObjectPointerType() &&
         idx->getType()->isRecordType()))) {
     return CreateOverloadedArraySubscriptExpr(lbLoc, rbLoc, base, idx);
   }
 
-  return CreateBuiltinArraySubscriptExpr(base, lbLoc, idx, rbLoc);
+  ExprResult Res = CreateBuiltinArraySubscriptExpr(base, lbLoc, idx, rbLoc);
+
+  if (!Res.isInvalid() && isa<ArraySubscriptExpr>(Res.get()))
+    CheckSubscriptAccessOfNoDeref(cast<ArraySubscriptExpr>(Res.get()));
+
+  return Res;
+}
+
+void Sema::CheckAddressOfNoDeref(const Expr *E) {
+  ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
+  const Expr *StrippedExpr = E->IgnoreParenImpCasts();
+  auto FoundExpr = LastRecord.PossibleDerefs.find(StrippedExpr);
+  if (FoundExpr != LastRecord.PossibleDerefs.end()) {
+    LastRecord.PossibleDerefs.erase(*FoundExpr);
+  } else if (const auto *Member = dyn_cast<MemberExpr>(StrippedExpr)) {
+    // Getting the address of a member expr in the form &(*s).b
+    const Expr *Base = Member->getBase()->IgnoreParenImpCasts();
+    const QualType &BaseTy = Base->getType();
+    auto FoundBase = LastRecord.PossibleDerefs.find(Base);
+    if (TypeHasNoDeref(BaseTy) &&
+        FoundBase != LastRecord.PossibleDerefs.end()) {
+      LastRecord.PossibleDerefs.erase(*FoundBase);
+    }
+  }
+}
+
+void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  const QualType &ResultTy = E->getType();
+  ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
+
+  if (TypeHasNoDeref(ResultTy)) {
+    LastRecord.PossibleDerefs.insert(E);
+    return;
+  }
+
+  // Check if the base type is a pointer to a member access of a struct
+  // marked with noderef.
+  const Expr *Base = E->getBase();
+  const QualType &BaseTy = Base->getType();
+  QualType ElemTy;
+  if (const auto *ArrayTy = dyn_cast<ArrayType>(BaseTy))
+    ElemTy = ArrayTy->getElementType();
+  else if (const auto *PointerTy = dyn_cast<PointerType>(BaseTy))
+    ElemTy = PointerTy->getPointeeType();
+  else
+    // Not a pointer access
+    return;
+
+  if (const auto *Member = dyn_cast<MemberExpr>(Base->IgnoreParenCasts())) {
+    const QualType &MemberBaseTy = Member->getBase()->getType();
+    if (const auto *Ptr = dyn_cast<PointerType>(MemberBaseTy)) {
+      if (TypeHasNoDeref(Ptr->getPointeeType()))
+        LastRecord.PossibleDerefs.insert(E);
+    }
+  }
 }
 
 ExprResult Sema::ActOnOMPArraySectionExpr(Expr *Base, SourceLocation LBLoc,
                                           Expr *LowerBound,
                                           SourceLocation ColonLoc, Expr *Length,
                                           SourceLocation RBLoc) {
   if (Base->getType()->isPlaceholderType() &&
       !Base->getType()->isSpecificPlaceholderType(
           BuiltinType::OMPArraySection)) {
     ExprResult Result = CheckPlaceholderExpr(Base);
resultType = CheckIncrementDecrementOperand(*this, Input.get(), VK, OK,
                                                 Opc == UO_PreInc ||
                                                 Opc == UO_PostInc,
                                                 Opc == UO_PreInc ||
                                                 Opc == UO_PreDec);
     CanOverflow = isOverflowingIntegerType(Context, resultType);
     break;
   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);
     break;
   }
ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
   // except for the '*' and '&' operators that have to be handled specially
   // by CheckArrayAccess (as there are special cases like &array[arraysize]
   // that are explicitly defined as valid by the standard).
   if (Opc != UO_AddrOf && Opc != UO_Deref)
     CheckArrayAccess(Input.get());
 
   auto *UO = new (Context)
       UnaryOperator(Input.get(), Opc, resultType, VK, OK, OpLoc, CanOverflow);
+
+  if (Opc == UO_Deref && TypeHasNoDeref(UO->getType()))
+    ExprEvalContexts.back().PossibleDerefs.insert(UO);
+
   // Convert the result back to a half vector.
   if (ConvertHalfVec)
     return convertVector(UO, Context.HalfTy, *this);
   return UO;
 }
 
 /// Determine whether the given expression is a qualified member
 /// access expression, of a form that could be turned into a pointer to member

 void
 Sema::PushExpressionEvaluationContext(
     ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t,
     ExpressionEvaluationContextRecord::ExpressionKind ExprContext) {
   Decl *ClosureContextDecl = ExprEvalContexts.back().ManglingContextDecl;
   PushExpressionEvaluationContext(NewContext, ClosureContextDecl, ExprContext);
 }
 
+namespace {
+
+class DeclRefFinder : public ConstEvaluatedExprVisitor<DeclRefFinder> {
+public:
+  typedef ConstEvaluatedExprVisitor<DeclRefFinder> Inherited;
+
+  DeclRefFinder(ASTContext &Ctx) : Inherited(Ctx) {}
+
+  void VisitDeclRefExpr(const DeclRefExpr *E) {
+    if (DeclRef)
+      return; // Found already
+
+    QualType Inner;
+    QualType Ty = E->getType().getDesugaredType(Context);
+    if (const auto *Ptr = dyn_cast<PointerType>(Ty))
+      Inner = Ptr->getPointeeType();
+    else if (const auto *Arr = dyn_cast<ArrayType>(Ty))
+      Inner = Arr->getElementType();
+    else
+      return;
+
+    if (Sema::TypeHasNoDeref(Inner))
+      DeclRef = E;
+  }
+
+  const DeclRefExpr *GetDeclRef() const { return DeclRef; }
+
+private:
+  const DeclRefExpr *DeclRef = nullptr;
+};
+
+} // namespace
+
+void Sema::StopCheckingNoDerefAndWarn() {
+  ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
+  LastRecord.NoDerefCallCount--;
+  if (LastRecord.NoDerefCallCount == 0) {
+    for (const Expr *E : LastRecord.PossibleDerefs) {
+      DeclRefFinder Finder(Context);
+      Finder.Visit(E);
+      const DeclRefExpr *DeclRef = Finder.GetDeclRef();
+      if (DeclRef) {
+        const ValueDecl *Decl = DeclRef->getDecl();
+        Diag(E->getExprLoc(), diag::warn_dereference_of_noderef_type)
+            << Decl->getName() << E->getSourceRange();
+        Diag(Decl->getLocation(), diag::note_previous_decl) << Decl->getName();
+      } else {
+        Diag(E->getExprLoc(), diag::warn_dereference_of_noderef_type_no_decl)
+            << E->getSourceRange();
+      }
+    }
+    LastRecord.PossibleDerefs.clear();
+  }
+}
+
 void Sema::PopExpressionEvaluationContext() {
   ExpressionEvaluationContextRecord& Rec = ExprEvalContexts.back();
   unsigned NumTypos = Rec.NumTypos;
 
   if (!Rec.Lambdas.empty()) {
     using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
     if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || Rec.isUnevaluated() ||
         (Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17)) {

lib/Sema/SemaExprMember.cpp

ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base,
   if (Base->getType()->isDependentType() || Name.isDependentName() ||
       isDependentScopeSpecifier(SS)) {
     return ActOnDependentMemberExpr(Base, Base->getType(), IsArrow, OpLoc, SS,
                                     TemplateKWLoc, FirstQualifierInScope,
                                     NameInfo, TemplateArgs);
   }
 
   ActOnMemberAccessExtraArgs ExtraArgs = {S, Id, ObjCImpDecl};
-  return BuildMemberReferenceExpr(Base, Base->getType(), OpLoc, IsArrow, SS,
-                                  TemplateKWLoc, FirstQualifierInScope,
-                                  NameInfo, TemplateArgs, S, &ExtraArgs);
+  ExprResult Res = BuildMemberReferenceExpr(
+      Base, Base->getType(), OpLoc, IsArrow, SS, TemplateKWLoc,
+      FirstQualifierInScope, NameInfo, TemplateArgs, S, &ExtraArgs);
+
+  if (!Res.isInvalid() && isa<MemberExpr>(Res.get()))
+    CheckMemberAccessOfNoDeref(cast<MemberExpr>(Res.get()));
+
+  return Res;
+}
+
+void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  QualType ResultTy = E->getType();
+
+  // Do not warn on member accesses to arrays.
+  if (isa<ArrayType>(ResultTy))
+    return;
+
+  if (E->isArrow()) {
+    if (const auto *Ptr = dyn_cast<PointerType>(
+            E->getBase()->getType().getDesugaredType(Context))) {
+      if (TypeHasNoDeref(Ptr->getPointeeType()))
+        ExprEvalContexts.back().PossibleDerefs.insert(E);
+    }
+  }
 }
 
 ExprResult
 Sema::BuildFieldReferenceExpr(Expr *BaseExpr, bool IsArrow,
                               SourceLocation OpLoc, const CXXScopeSpec &SS,
                               FieldDecl *Field, DeclAccessPair FoundDecl,
                               const DeclarationNameInfo &MemberNameInfo) {
   // x.a is an l-value if 'a' has a reference type. Otherwise:

lib/Sema/SemaType.cpp

while (i != 0) {
     case DeclaratorChunk::Pipe:
       // These are invalid anyway, so just ignore.
       break;
     }
   }
   return false;
 }
 
+static bool IsNoDerefableChunk(DeclaratorChunk Chunk) {
+  return (Chunk.Kind == DeclaratorChunk::Pointer ||
+          Chunk.Kind == DeclaratorChunk::Array);
+}
+
 static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
                                                 QualType declSpecType,
                                                 TypeSourceInfo *TInfo) {
   // The TypeSourceInfo that this function returns will not be a null type.
   // If there is an error, this function will fill in a dummy type as fallback.
   QualType T = declSpecType;
   Declarator &D = state.getDeclarator();
   Sema &S = state.getSema();
if (complainAboutMissingNullability == CAMN_Yes &&
         T->isArrayType() && !T->getNullability(S.Context) && !isVaList(T) &&
         D.isPrototypeContext() &&
         !hasOuterPointerLikeChunk(D, D.getNumTypeObjects())) {
       checkNullabilityConsistency(S, SimplePointerKind::Array,
                                   D.getDeclSpec().getTypeSpecTypeLoc());
     }
   }
 
+  bool IsNoDeref = false;
+  if (const auto *AT = dyn_cast<AttributedType>(T))
+    IsNoDeref = AT->getAttrKind() == AttributedType::attr_noderef;
+  bool ExpectNoDerefChunk = IsNoDeref;
+
   // Walk the DeclTypeInfo, building the recursive type as we go.
   // DeclTypeInfos are ordered from the identifier out, which is
   // opposite of what we want :).
   for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
     unsigned chunkIndex = e - i - 1;
     state.setCurrentChunkIndex(chunkIndex);
     DeclaratorChunk &DeclType = D.getTypeObject(chunkIndex);
     IsQualifiedFunction &= DeclType.Kind == DeclaratorChunk::Paren;
for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
 
     if (T.isNull()) {
       D.setInvalidType(true);
       T = Context.IntTy;
     }
 
     // See if there are any attributes on this declarator chunk.
     processTypeAttrs(state, T, TAL_DeclChunk, DeclType.getAttrs());
+
+    if (const auto *AT = dyn_cast<AttributedType>(T))
+      IsNoDeref = AT->getAttrKind() == AttributedType::attr_noderef;
+    else
+      IsNoDeref = false;
+
+    if (DeclType.Kind != DeclaratorChunk::Paren) {
+      if (ExpectNoDerefChunk) {
+        if (!IsNoDerefableChunk(DeclType))
+          S.Diag(DeclType.Loc, diag::warn_noderef_on_non_pointer_or_array);
+        ExpectNoDerefChunk = false;
       }
 
+      ExpectNoDerefChunk = IsNoDeref;
+    }
+  }
+
+  if (ExpectNoDerefChunk)
+    S.Diag(state.getDeclarator().getBeginLoc(),
+           diag::warn_noderef_on_non_pointer_or_array);
+
   // GNU warning -Wstrict-prototypes
   //   Warn if a function declaration is without a prototype.
   //   This warning is issued for all kinds of unprototyped function
   //   declarations (i.e. function type typedef, function pointer etc.)
   //   C99 6.7.5.3p14:
   //   The empty list in a function declarator that is not part of a definition
   //   of that function specifies that no information about the number or types
   //   of the parameters is supplied.
static ParsedAttr::Kind getAttrListKind(AttributedType::Kind kind) {
   case AttributedType::attr_ms_abi:
     return ParsedAttr::AT_MSABI;
   case AttributedType::attr_sysv_abi:
     return ParsedAttr::AT_SysVABI;
   case AttributedType::attr_preserve_most:
     return ParsedAttr::AT_PreserveMost;
   case AttributedType::attr_preserve_all:
     return ParsedAttr::AT_PreserveAll;
+  case AttributedType::attr_noderef:
+    return ParsedAttr::AT_NoDeref;
   case AttributedType::attr_ptr32:
     return ParsedAttr::AT_Ptr32;
   case AttributedType::attr_ptr64:
     return ParsedAttr::AT_Ptr64;
   case AttributedType::attr_sptr:
     return ParsedAttr::AT_SPtr;
   case AttributedType::attr_uptr:
     return ParsedAttr::AT_UPtr;
for (ParsedAttr &attr : AttrsCopy) {
     case ParsedAttr::AT_LifetimeBound:
       if (TAL == TAL_DeclChunk) {
         HandleLifetimeBoundAttr(type, attr, state.getSema(),
                                 state.getDeclarator());
         attr.setUsedAsTypeAttr();
       }
       break;
 
+    case ParsedAttr::AT_NoDeref:
+      type = state.getSema().Context.getAttributedType(
+          AttributedType::attr_noderef, type, type);
+      attr.setUsedAsTypeAttr();
+      break;
+
     MS_TYPE_ATTRS_CASELIST:
       if (!handleMSPointerTypeQualifierAttr(state, attr, type))
         attr.setUsedAsTypeAttr();
       break;
 
 
     NULLABILITY_TYPE_ATTRS_CASELIST:
       // Either add nullability here or try to distribute it.  We

test/Frontend/noderef.c

+// RUN: %clang_cc1 -Wno-unused-value -verify %s
+
+#define NODEREF __attribute__((noderef))
+
+struct S {
+  int a;
+  int b;
+};
+
+struct S2 {
+  int a[2];
+  int NODEREF a2[2];
+  int *b;
+  int NODEREF *b2;
+  struct S *s;
+  struct S NODEREF *s2;
+};
+
+int NODEREF *func(int NODEREF *arg) {  // expected-note{{arg declared here}}
+  int y = *arg; // expected-warning{{dereferencing arg; was declared with a 'noderef' type}}
+  return arg;
+}
+
+void func2(int x) {}
+
+int test() {
+  int NODEREF *p; // expected-note 37 {{p declared here}}
+  int *p2;
+
+  int x = *p;               // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  x = *((int NODEREF *)p2); // expected-warning{{dereferencing expression marked as 'noderef'}}
+
+  int NODEREF **q;
+  int *NODEREF *q2; // expected-note 4 {{q2 declared here}}
+
+  // Indirection
+  p2 = *q;  // ok
+  x = **q;  // expected-warning{{dereferencing expression marked as 'noderef'}}
+  p2 = *q2; // expected-warning{{dereferencing q2; was declared with a 'noderef' type}}
+
+  **q; // expected-warning{{dereferencing expression marked as 'noderef'}}
+
+  p = *&*q;
+  p = **&q;
+  q = &**&q;
+  p = &*p;
+  p = *&p;
+  p = &(*p);
+  p = *(&p);
+  x = **&p; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+
+  *p = 2;   // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  *q = p;   // ok
+  **q = 2;  // expected-warning{{dereferencing expression marked as 'noderef'}}
+  *q2 = p2; // expected-warning{{dereferencing q2; was declared with a 'noderef' type}}
+
+  p2 = p;
+  p = p + 1;
+  p = &*(p + 1);
+
+  // Struct member access
+  struct S NODEREF *s;  // expected-note 2 {{s declared here}}
+  x = s->a;   // expected-warning{{dereferencing s; was declared with a 'noderef' type}}
+  x = (*s).b; // expected-warning{{dereferencing s; was declared with a 'noderef' type}}
+  p = &s->a;
+  p = &(*s).b;
+
+  // Nested struct access
+  struct S2 NODEREF *s2_noderef;    // expected-note 5 {{s2_noderef declared here}}
+  p = s2_noderef->a;  // ok since result is an array in a struct
+  p = s2_noderef->a2; // ok
+  p = s2_noderef->b;  // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
+  p = s2_noderef->b2; // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
+  s = s2_noderef->s;  // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
+  s = s2_noderef->s2; // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
+  p = s2_noderef->a + 1;
+
+  struct S2 *s2;
+  p = s2->a;
+  p = s2->a2;
+  p = s2->b;
+  p = s2->b2;
+  s = s2->s;
+  s = s2->s2;
+
+  // Subscript access
+  x = p[1];    // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  p2 = q[0];   // ok
+  x = q[0][0]; // expected-warning{{dereferencing expression marked as 'noderef'}}
+  p2 = q2[0];  // expected-warning{{dereferencing q2; was declared with a 'noderef' type}}
+  p = q[*p];   // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  x = p[*p];   // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+               // expected-warning@-1{{dereferencing p; was declared with a 'noderef' type}}
+
+  int NODEREF arr[10];    // expected-note 1 {{arr declared here}}
+  x = arr[1]; // expected-warning{{dereferencing arr; was declared with a 'noderef' type}}
+
+  int NODEREF *(arr2[10]);
+  int NODEREF *elem = *arr2;
+
+  int NODEREF(*arr3)[10];
+  elem = *arr3;
+
+  // Combinations between indirection, subscript, and member access
+  struct S2 NODEREF *s2_arr[10];
+  s2 = s2_arr[1];
+  struct S2 NODEREF *s2_arr2[10][10];
+  s2 = s2_arr2[1][1];
+
+  p = s2_arr[1]->a;
+  p = s2_arr[1]->b; // expected-warning{{dereferencing expression marked as 'noderef'}}
+  int **bptr = &s2_arr[1]->b;
+
+  x = s2->s2->a;        // expected-warning{{dereferencing expression marked as 'noderef'}}
+  x = s2_noderef->a[1]; // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
+  p = &s2_noderef->a[1];
+
+  // typedefs
+  typedef int NODEREF *ptr_t;
+  ptr_t ptr; // expected-note 2 {{ptr declared here}}
+  ptr_t *ptr2;
+  *ptr; // expected-warning{{dereferencing ptr; was declared with a 'noderef' type}}
+  *ptr2;
+  **ptr2; // expected-warning{{dereferencing expression marked as 'noderef'}}
+
+  typedef struct S2 NODEREF *s2_ptr_t;
+  s2_ptr_t s2_ptr; // expected-note 4 {{s2_ptr declared here}}
+  s2_ptr->a;       // ok since result is an array in a struct
+  s2_ptr->a2;      // ok
+  s2_ptr->b;       // expected-warning{{dereferencing s2_ptr; was declared with a 'noderef' type}}
+  s2_ptr->b2;      // expected-warning{{dereferencing s2_ptr; was declared with a 'noderef' type}}
+  s2_ptr->s;       // expected-warning{{dereferencing s2_ptr; was declared with a 'noderef' type}}
+  s2_ptr->s2;      // expected-warning{{dereferencing s2_ptr; was declared with a 'noderef' type}}
+  s2_ptr->a + 1;
+
+  // Parenthesis
+  (((*((p))))); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  (*(*(&(p)))); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+
+  (p[1]);      // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  (q[0]);      // ok
+  (q[0][0]);   // expected-warning{{dereferencing expression marked as 'noderef'}}
+  (q2[0]);     // expected-warning{{dereferencing q2; was declared with a 'noderef' type}}
+  (q[(*(p))]); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  (p[(*(p))]); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+               // expected-warning@-1{{dereferencing p; was declared with a 'noderef' type}}
+
+  (*(ptr)); // expected-warning{{dereferencing ptr; was declared with a 'noderef' type}}
+  (*(ptr2));
+  (*(*(ptr2))); // expected-warning{{dereferencing expression marked as 'noderef'}}
+
+  // Functions
+  x = *(func(p)); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+
+  // Casting is ok
+  q = (int NODEREF **)&p;
+  q = (int NODEREF **)&p2;
+  q = &p;
+  q = &p2;
+  x = s2->s2->a; // expected-warning{{dereferencing expression marked as 'noderef'}}
+
+  // Other expressions
+  func2(*p);         // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  func2(*p + 1);     // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  func2(!*p);        // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  func2((x = *p));   // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  func2((char)(*p)); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+
+  // Other statements
+  if (*p) {}          // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  else if (*p) {}     // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  switch (*p){}       // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  for (*p; *p; *p){}  // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+                      // expected-warning@-1{{dereferencing p; was declared with a 'noderef' type}}
+                      // expected-warning@-2{{dereferencing p; was declared with a 'noderef' type}}
+  for (*p; *p;){}     // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+                      // expected-warning@-1{{dereferencing p; was declared with a 'noderef' type}}
+  for (*p;; *p){}     // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+                      // expected-warning@-1{{dereferencing p; was declared with a 'noderef' type}}
+  for (; *p; *p){}    // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+                      // expected-warning@-1{{dereferencing p; was declared with a 'noderef' type}}
+  for (*p;;){}        // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  for (;*p;){}        // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  for (;;*p){}        // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  while (*p){}        // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  do {} while (*p);   // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  return *p;          // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+}

test/Frontend/noderef_on_non_pointers.cpp

+// RUN: %clang_cc1 -fblocks -verify %s
+
+/**
+ * Test 'noderef' attribute against other pointer-like types.
+ */
+
+#define NODEREF __attribute__((noderef))
+
+void Normal() {
+  int NODEREF i;        // expected-warning{{'noderef' can only be used on an array or pointer type}}
+  int NODEREF *i_ptr;   // ok
+  int NODEREF **i_ptr2; // ok
+  int *NODEREF i_ptr3;  // expected-warning{{'noderef' can only be used on an array or pointer type}}
+  int *NODEREF *i_ptr4; // ok
+
+  auto NODEREF *auto_i_ptr = i_ptr;
+  auto NODEREF auto_i = i; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+
+  struct {
+    int x;
+    int y;
+  } NODEREF *s;
+
+  int __attribute__((noderef(10))) * no_args; // expected-error{{'noderef' attribute takes no arguments}}
+}
+
+const int NODEREF *const_i_ptr;
+static int NODEREF *static_i_ptr;
+
+void ParenTypes() {
+  int NODEREF(*i_ptr);    // ok (same as `int NODEREF *`)
+  int NODEREF *(*i_ptr2); // ok (same as `int NODEREF **`)
+}
+
+// Function declarations
+int NODEREF func();   // expected-warning{{'noderef' can only be used on an array or pointer type}}
+int NODEREF *func2(); // ok (returning pointer)
+
+typedef int NODEREF (*func3)(int); // expected-warning{{'noderef' can only be used on an array or pointer type}}
+typedef int NODEREF *(*func4)(int);
+
+void Arrays() {
+  int NODEREF i_arr[10];      // ok
+  int NODEREF i_arr2[10][10]; // ok
+  int NODEREF *i_arr3[10];    // ok
+  int NODEREF i_arr4[] = {1, 2};
+}
+
+void ParenArrays() {
+  int NODEREF(i_ptr[10]);
+  int NODEREF(i_ptr2[10])[10];
+}
+
+typedef int NODEREF *(*func5[10])(int);
+
+// Arguments
+void func6(int NODEREF x); // expected-warning{{'noderef' can only be used on an array or pointer type}}
+void func7(int NODEREF *x);
+void func8() NODEREF;
+
+void References() {
+  int x = 2;
+  int NODEREF &y = x; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+  int *xp = &x;
+  int NODEREF *&a = xp; // ok (reference to a NODEREF *)
+  int *NODEREF &b = xp; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
+
+void BlockPointers() {
+  typedef int NODEREF (^IntBlock)(); // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
+
+class A {
+public:
+  int member;
+  int NODEREF *member2;
+  int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+};
+
+void MemberPointer() {
+  int NODEREF A::*var = &A::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
+
+template <class Ty>
+class B {
+  Ty NODEREF *member;
+  Ty NODEREF member2; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+};

test/Frontend/noderef_on_non_pointers.m

+// RUN: %clang_cc1 -verify %s
+
+#define NODEREF __attribute__((noderef))
+
+@interface NSObject
++ (id)new;
+@end
+
+void func() {
+  id NODEREF obj = [NSObject new]; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}