[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

3355 ↗

(On Diff #156167)

'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

9372 ↗

(On Diff #156152)

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

3426 ↗

(On Diff #156634)

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

3426 ↗

(On Diff #156634)

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

1031 ↗

(On Diff #156634)

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

9420 ↗

(On Diff #156634)

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

987 ↗

(On Diff #156634)

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

1027–1029 ↗

(On Diff #156634)

I believe these can be private.

1037 ↗

(On Diff #156634)

I believe this can be private.

1040–1041 ↗

(On Diff #156634)

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

lib/Sema/SemaExpr.cpp

4168 ↗

(On Diff #156634)

const auto *

4259 ↗

(On Diff #156634)

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().

4260 ↗

(On Diff #156634)

const auto *

4266 ↗

(On Diff #156634)

Similar suggestion here as above.

4284 ↗

(On Diff #156634)

const auto *

4284–4291 ↗

(On Diff #156634)

Can elide braces.

4286 ↗

(On Diff #156634)

const auto *

4293 ↗

(On Diff #156634)

const auto *

4296 ↗

(On Diff #156634)

const auto *

4297 ↗

(On Diff #156634)

Can elide braces.

lib/Sema/SemaExprMember.cpp

1724 ↗

(On Diff #156634)

Missing full stop at the end of the sentence.

1729 ↗

(On Diff #156634)

const auto *

test/Frontend/noderef.c

2 ↗

(On Diff #156634)

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

3426 ↗

(On Diff #156634)

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

2 ↗

(On Diff #156634)

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

3426 ↗

(On Diff #156634)

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

9420 ↗

(On Diff #156826)

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

test/Frontend/noderef.c

2 ↗

(On Diff #156634)

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

1556 ↗

(On Diff #156826)

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 ↗

(On Diff #156826)

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 ↗

(On Diff #156826)

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

3489 ↗

(On Diff #158423)

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

3490 ↗

(On Diff #158423)

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

include/clang/Basic/DiagnosticSemaKinds.td

9436 ↗

(On Diff #158423)

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

14251–14253 ↗

(On Diff #159409)

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 ↗

(On Diff #160403)

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 ↗

(On Diff #160403)

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 ↗

(On Diff #160403)

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 ↗

(On Diff #160403)

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 ↗

(On Diff #160451)

Missing a full stop at the end of the comment.

4167 ↗

(On Diff #160451)

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

test/Frontend/noderef.c

153 ↗

(On Diff #160451)

Typo: Parentheses

[rsmith]

lib/Parse/ParseExpr.cpp

1126 ↗

(On Diff #160892)

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

14230–14242 ↗

(On Diff #156826)

This comment is marked done but has not been addressed.

4267 ↗

(On Diff #160892)

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

4291 ↗

(On Diff #160892)

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

lib/Sema/SemaType.cpp

4209 ↗

(On Diff #160892)

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

4816 ↗

(On Diff #160892)

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

4816 ↗

(On Diff #160892)

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 ↗

(On Diff #160892)

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

14230–14242 ↗

(On Diff #156826)

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.

4291 ↗

(On Diff #160892)

Forgot to remove this.

lib/Sema/SemaType.cpp

4816 ↗

(On Diff #160892)

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 ↗

(On Diff #160892)

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

14230–14242 ↗

(On Diff #156826)

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.

4164–4177 ↗

(On Diff #161785)

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

4263–4266 ↗

(On Diff #161785)

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.

4269 ↗

(On Diff #161785)

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

4278 ↗

(On Diff #161785)

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.

4289 ↗

(On Diff #161785)

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.)

4297 ↗

(On Diff #161785)

QualTypes should be held by value, not by reference.

4302–4308 ↗

(On Diff #161785)

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

12865–12866 ↗

(On Diff #161785)

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.

14251–14253 ↗

(On Diff #159409)

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 ↗

(On Diff #160892)

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

4289 ↗

(On Diff #161785)

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

3513 ↗

(On Diff #164706)

Should noderef appear somewhere in this example? :)

include/clang/Basic/DiagnosticSemaKinds.td

9452–9453 ↗

(On Diff #162126)

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 ↗

(On Diff #160892)

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

4289 ↗

(On Diff #161785)

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.

4164 ↗

(On Diff #162126)

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

4257–4259 ↗

(On Diff #162126)

Replace these three lines with just

LastRecord.PossibleDerefs.erase(StrippedExpr);

test/Frontend/noderef_on_non_pointers.cpp

4 ↗

(On Diff #164706)

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

4289 ↗

(On Diff #161785)

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

4 ↗

(On Diff #164706)

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

7869–7885

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

[dblaikie]

lib/Sema/SemaType.cpp

4913–4916 ↗

(On Diff #176908)

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

4913–4916 ↗

(On Diff #176908)

Yup, fixed in r360089

clang/include/clang/Basic/Attr.td

 def Regparm : TypeAttr {
   let Spellings = [GCC<"regparm">];
   let Args = [UnsignedArgument<"NumParams">];
   let Documentation = [RegparmDocs];
   // Represented as part of the enclosing function type.
   let ASTNode = 0;
 }
 
+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];
 }

clang/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 __attribute__((noderef)) &y = x;  // warning: 'noderef' can only be used on an array or pointer type
+
+.. code-block: objc
+
+  id __attribute__((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

clang/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">;

clang/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<IgnoredAttributes>;
+def warn_noderef_to_dereferenceable_pointer : Warning<
+  "casting to dereferenceable pointer removes 'noderef' attribute">, InGroup<NoDeref>;
 } // end of sema component.

clang/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;
 
+    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,
struct ExpressionEvaluationContextRecord {
     bool isConstantEvaluated() const {
       return Context == ExpressionEvaluationContext::ConstantEvaluated;
     }
   };
 
   /// A stack of expression evaluation contexts.
   SmallVector<ExpressionEvaluationContextRecord, 8> ExprEvalContexts;
 
+  /// Emit a warning for all pending noderef expressions that we recorded.
+  void WarnOnPendingNoDerefs(ExpressionEvaluationContextRecord &Rec);
+
   /// 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(
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;
     }
   };
 
 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;
   };

clang/lib/AST/TypePrinter.cpp

#include "clang/Basic/AttrList.inc"
   case attr::IntelOclBicc: OS << "inteloclbicc"; break;
   case attr::PreserveMost:
     OS << "preserve_most";
     break;
 
   case attr::PreserveAll:
     OS << "preserve_all";
     break;
+  case attr::NoDeref:
+    OS << "noderef";
+    break;
   }
   OS << "))";
 }
 
 void TypePrinter::printObjCInterfaceBefore(const ObjCInterfaceType *T,
                                            raw_ostream &OS) {
   OS << T->getDecl()->getName();
   spaceBeforePlaceHolder(OS);

clang/lib/Sema/SemaExpr.cpp

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();
+
+  // For expressions like `&(*s).b`, the base is recorded and what should be
+  // checked.
+  const MemberExpr *Member = nullptr;
+  while ((Member = dyn_cast<MemberExpr>(StrippedExpr)) && !Member->isArrow())
+    StrippedExpr = Member->getBase()->IgnoreParenImpCasts();
+
+  LastRecord.PossibleDerefs.erase(StrippedExpr);
+}
+
+void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  QualType ResultTy = E->getType();
+  ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
+
+  // Bail if the element is an array since it is not memory access.
+  if (isa<ArrayType>(ResultTy))
+    return;
+
+  if (ResultTy->hasAttr(attr::NoDeref)) {
+    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();
+  QualType BaseTy = Base->getType();
+  if (!(isa<ArrayType>(BaseTy) || isa<PointerType>(BaseTy)))
+    // Not a pointer access
+    return;
+
+  const MemberExpr *Member = nullptr;
+  while ((Member = dyn_cast<MemberExpr>(Base->IgnoreParenCasts())) &&
+         Member->isArrow())
+    Base = Member->getBase();
+
+  if (const auto *Ptr = dyn_cast<PointerType>(Base->getType())) {
+    if (Ptr->getPointeeType()->hasAttr(attr::NoDeref))
+      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(
Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
   assert((ConvertRHS || !Diagnose) && "can't indicate whether we diagnosed");
 
   // If ConvertRHS is false, we want to leave the caller's RHS untouched. Sadly,
   // we can't avoid *all* modifications at the moment, so we need some somewhere
   // to put the updated value.
   ExprResult LocalRHS = CallerRHS;
   ExprResult &RHS = ConvertRHS ? CallerRHS : LocalRHS;
 
+  if (const auto *LHSPtrType = LHSType->getAs<PointerType>()) {
+    if (const auto *RHSPtrType = RHS.get()->getType()->getAs<PointerType>()) {
+      if (RHSPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
+          !LHSPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
+        Diag(RHS.get()->getExprLoc(),
+             diag::warn_noderef_to_dereferenceable_pointer)
+            << RHS.get()->getSourceRange();
+      }
+    }
+  }
+
   if (getLangOpts().CPlusPlus) {
     if (!LHSType->isRecordType() && !LHSType->isAtomicType()) {
       // C++ 5.17p3: If the left operand is not of class type, the
       // expression is implicitly converted (C++ 4) to the
       // cv-unqualified type of the left operand.
       QualType RHSType = RHS.get()->getType();
       if (Diagnose) {
         RHS = PerformImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
if (getLangOpts().ObjC &&
       // can find further errors.
       RHS = E;
       return Compatible;
     }
 
     if (ConvertRHS)
       RHS = ImpCastExprToType(E, Ty, Kind);
   }
+
   return result;
 }
 
 namespace {
 /// The original operand to an operator, prior to the application of the usual
 /// arithmetic conversions and converting the arguments of a builtin operator
 /// candidate.
 struct OriginalOperand {
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 && UO->getType()->hasAttr(attr::NoDeref) &&
+      !isa<ArrayType>(UO->getType().getDesugaredType(Context)))
+    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 {
+
+const DeclRefExpr *CheckPossibleDeref(Sema &S, const Expr *PossibleDeref) {
+  PossibleDeref = PossibleDeref->IgnoreParenImpCasts();
+  if (const auto *E = dyn_cast<UnaryOperator>(PossibleDeref)) {
+    if (E->getOpcode() == UO_Deref)
+      return CheckPossibleDeref(S, E->getSubExpr());
+  } else if (const auto *E = dyn_cast<ArraySubscriptExpr>(PossibleDeref)) {
+    return CheckPossibleDeref(S, E->getBase());
+  } else if (const auto *E = dyn_cast<MemberExpr>(PossibleDeref)) {
+    return CheckPossibleDeref(S, E->getBase());
+  } else if (const auto E = dyn_cast<DeclRefExpr>(PossibleDeref)) {
+    QualType Inner;
+    QualType Ty = E->getType();
+    if (const auto *Ptr = Ty->getAs<PointerType>())
+      Inner = Ptr->getPointeeType();
+    else if (const auto *Arr = S.Context.getAsArrayType(Ty))
+      Inner = Arr->getElementType();
+    else
+      return nullptr;
+
+    if (Inner->hasAttr(attr::NoDeref))
+      return E;
+  }
+  return nullptr;
+}
+
+} // namespace
+
+void Sema::WarnOnPendingNoDerefs(ExpressionEvaluationContextRecord &Rec) {
+  for (const Expr *E : Rec.PossibleDerefs) {
+    const DeclRefExpr *DeclRef = CheckPossibleDeref(*this, E);
+    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();
+    }
+  }
+  Rec.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)) {
if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || Rec.isUnevaluated() ||
       // during lambda expression creation.
       for (auto *Lambda : Rec.Lambdas) {
         for (auto *C : Lambda->capture_inits())
           MarkDeclarationsReferencedInExpr(C);
       }
     }
   }
 
+  WarnOnPendingNoDerefs(Rec);
+
   // When are coming out of an unevaluated context, clear out any
   // temporaries that we may have created as part of the evaluation of
   // the expression in that context: they aren't relevant because they
   // will never be constructed.
   if (Rec.isUnevaluated() || Rec.isConstantEvaluated()) {
     ExprCleanupObjects.erase(ExprCleanupObjects.begin() + Rec.NumCleanupObjects,
                              ExprCleanupObjects.end());
     Cleanup = Rec.ParentCleanup;

clang/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 since this returns an array
+  // lvalue and does not actually dereference memory.
+  if (isa<ArrayType>(ResultTy))
+    return;
+
+  if (E->isArrow()) {
+    if (const auto *Ptr = dyn_cast<PointerType>(
+            E->getBase()->getType().getDesugaredType(Context))) {
+      if (Ptr->getPointeeType()->hasAttr(attr::NoDeref))
+        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:

clang/lib/Sema/SemaInit.cpp

case SK_LValueToRValue: {
       CurInit = ImplicitCastExpr::Create(S.Context, Step->Type,
                                          CK_LValueToRValue, CurInit.get(),
                                          /*BasePath=*/nullptr, VK_RValue);
       break;
     }
 
     case SK_ConversionSequence:
     case SK_ConversionSequenceNoNarrowing: {
+      if (const auto *FromPtrType =
+              CurInit.get()->getType()->getAs<PointerType>()) {
+        if (const auto *ToPtrType = Step->Type->getAs<PointerType>()) {
+          if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
+              !ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
+            S.Diag(CurInit.get()->getExprLoc(),
+                   diag::warn_noderef_to_dereferenceable_pointer)
+                << CurInit.get()->getSourceRange();
+          }
+        }
+      }
+
       Sema::CheckedConversionKind CCK
         = Kind.isCStyleCast()? Sema::CCK_CStyleCast
         : Kind.isFunctionalCast()? Sema::CCK_FunctionalCast
         : Kind.isExplicitCast()? Sema::CCK_OtherCast
         : Sema::CCK_ImplicitConversion;
       ExprResult CurInitExprRes =
         S.PerformImplicitConversion(CurInit.get(), Step->Type, *Step->ICS,
                                     getAssignmentAction(Entity), CCK);
case SK_ZeroInitialization: {
       } else {
         CurInit = new (S.Context) ImplicitValueInitExpr(Step->Type);
       }
       break;
     }
 
     case SK_CAssignment: {
       QualType SourceType = CurInit.get()->getType();
+
       // Save off the initial CurInit in case we need to emit a diagnostic
       ExprResult InitialCurInit = CurInit;
       ExprResult Result = CurInit;
       Sema::AssignConvertType ConvTy =
         S.CheckSingleAssignmentConstraints(Step->Type, Result, true,
             Entity.getKind() == InitializedEntity::EK_Parameter_CF_Audited);
       if (Result.isInvalid())
         return ExprError();
       CurInit = Result;
 
       // If this is a call, allow conversion to a transparent union.
       ExprResult CurInitExprRes = CurInit;
       if (ConvTy != Sema::Compatible &&
           Entity.isParameterKind() &&
           S.CheckTransparentUnionArgumentConstraints(Step->Type, CurInitExprRes)
             == Sema::Compatible)
         ConvTy = Sema::Compatible;
       if (CurInitExprRes.isInvalid())
         return ExprError();
       CurInit = CurInitExprRes;
 
       bool Complained;
       if (S.DiagnoseAssignmentResult(ConvTy, Kind.getLocation(),
                                      Step->Type, SourceType,
                                      InitialCurInit.get(),

clang/lib/Sema/SemaType.cpp

class TypeProcessingState {
     // FIXME: The two-phase mechanism by which we construct Types and fill
     // their TypeLocs makes it hard to correctly assign these. We keep the
     // attributes in creation order as an attempt to make them line up
     // properly.
     using TypeAttrPair = std::pair<const AttributedType*, const Attr*>;
     SmallVector<TypeAttrPair, 8> AttrsForTypes;
     bool AttrsForTypesSorted = true;
 
+    /// Flag to indicate we parsed a noderef attribute. This is used for
+    /// validating that noderef was used on a pointer or array.
+    bool parsedNoDeref;
+
   public:
     TypeProcessingState(Sema &sema, Declarator &declarator)
         : sema(sema), declarator(declarator),
-        chunkIndex(declarator.getNumTypeObjects()),
-        trivial(true), hasSavedAttrs(false) {}
+          chunkIndex(declarator.getNumTypeObjects()), trivial(true),
+          hasSavedAttrs(false), parsedNoDeref(false) {}
 
     Sema &getSema() const {
       return sema;
     }
 
     Declarator &getDeclarator() const {
       return declarator;
     }
const Attr *takeAttrForAttributedType(const AttributedType *AT) {
           It->second = nullptr;
           return Result;
         }
       }
 
       llvm_unreachable("no Attr* for AttributedType*");
     }
 
+    void setParsedNoDeref(bool parsed) { parsedNoDeref = parsed; }
+
+    bool didParseNoDeref() const { return parsedNoDeref; }
+
     ~TypeProcessingState() {
       if (trivial) return;
 
       restoreDeclSpecAttrs();
     }
 
   private:
     DeclSpec &getMutableDeclSpec() const {
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);
+}
+
 template<typename AttrT>
 static AttrT *createSimpleAttr(ASTContext &Ctx, ParsedAttr &Attr) {
   Attr.setUsedAsTypeAttr();
   return ::new (Ctx)
       AttrT(Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex());
 }
 
 static Attr *createNullabilityAttr(ASTContext &Ctx, ParsedAttr &Attr,
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 ExpectNoDerefChunk =
+      state.getCurrentAttributes().hasAttribute(ParsedAttr::AT_NoDeref);
+
   // 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 (DeclType.Kind != DeclaratorChunk::Paren) {
+      if (ExpectNoDerefChunk) {
+        if (!IsNoDerefableChunk(DeclType))
+          S.Diag(DeclType.Loc, diag::warn_noderef_on_non_pointer_or_array);
+        ExpectNoDerefChunk = false;
+      }
+
+      ExpectNoDerefChunk = state.didParseNoDeref();
     }
+  }
+
+  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
static void processTypeAttrs(TypeProcessingState &state, QualType &type,
   // attributes (such as __address_space__, __vector_size__, etc) apply to the
   // type, but others can be present in the type specifiers even though they
   // apply to the decl.  Here we apply type attributes and ignore the rest.
 
   // This loop modifies the list pretty frequently, but we still need to make
   // sure we visit every element once. Copy the attributes list, and iterate
   // over that.
   ParsedAttributesView AttrsCopy{attrs};
+
+  state.setParsedNoDeref(false);
+
   for (ParsedAttr &attr : AttrsCopy) {
 
     // Skip attributes that were marked to be invalid.
     if (attr.isInvalid())
       continue;
 
     if (attr.isCXX11Attribute()) {
       // [[gnu::...]] attributes are treated as declaration attributes, so may
case ParsedAttr::AT_OpenCLAccess:
       HandleOpenCLAccessAttr(type, attr, state.getSema());
       attr.setUsedAsTypeAttr();
       break;
     case ParsedAttr::AT_LifetimeBound:
       if (TAL == TAL_DeclChunk)
         HandleLifetimeBoundAttr(state, type, attr);
       break;
 
+    case ParsedAttr::AT_NoDeref: {
+      ASTContext &Ctx = state.getSema().Context;
+      type = state.getAttributedType(createSimpleAttr<NoDerefAttr>(Ctx, attr),
+                                     type, type);
+      attr.setUsedAsTypeAttr();
+      state.setParsedNoDeref(true);
+      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

clang/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 34 {{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
+  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 expression marked as 'noderef'}}
+
+  *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}}
+
+  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;
+  &(*(*s2).s2).b;
+
+  // Subscript access
+  x = p[1];    // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  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];
+  struct S2 NODEREF *s2_arr2[10][10];
+
+  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];
+
+  // Casting to dereferenceable pointer
+  p2 = p;             // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  p2 = *q;            // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  p2 = q[0];          // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  s2 = s2_arr[1];     // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  s2 = s2_arr2[1][1]; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  p2 = p, p2 = *q;    // expected-warning 2 {{casting to dereferenceable pointer removes 'noderef' attribute}}
+
+  // 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;
+
+  typedef int(int_t);
+  typedef int_t NODEREF *(noderef_int_t);
+  typedef noderef_int_t *noderef_int_nested_t;
+  noderef_int_nested_t noderef_int_nested_ptr;
+  *noderef_int_nested_ptr;
+  **noderef_int_nested_ptr; // expected-warning{{dereferencing expression marked as 'noderef'}}
+
+  typedef int_t *(NODEREF noderef_int2_t);
+  typedef noderef_int2_t *noderef_int2_nested_t;
+  noderef_int2_nested_t noderef_int2_nested_ptr; // expected-note{{noderef_int2_nested_ptr declared here}}
+  *noderef_int2_nested_ptr;                      // expected-warning{{dereferencing noderef_int2_nested_ptr; was declared with a 'noderef' type}}
+
+  typedef int_t *(noderef_int3_t);
+  typedef noderef_int3_t(NODEREF(*(noderef_int3_nested_t)));
+  noderef_int3_nested_t noderef_int3_nested_ptr; // expected-note{{noderef_int3_nested_ptr declared here}}
+  *noderef_int3_nested_ptr;                      // expected-warning{{dereferencing noderef_int3_nested_ptr; was declared with a 'noderef' type}}
+
+  // Parentheses
+  (((*((p))))); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
+  (*(*(&(p)))); // expected-warning{{dereferencing expression marked as 'noderef'}}
+
+  (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 expression marked as 'noderef'}}
+
+  // 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}}
+}

clang/test/Frontend/noderef.cpp

+// RUN: %clang_cc1 -fblocks -verify %s
+
+/**
+ * Test 'noderef' attribute with c++ constructs.
+ */
+
+#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;   // expected-note 2 {{i_ptr declared here}}
+  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}}
+
+  int i2 = *i_ptr;     // expected-warning{{dereferencing i_ptr; was declared with a 'noderef' type}}
+  int &i3 = *i_ptr;    // expected-warning{{dereferencing i_ptr; was declared with a 'noderef' type}}
+  int *i_ptr5 = i_ptr; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  int *i_ptr6(i_ptr);  // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+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}}
+};
+
+void test_lambdas() {
+  auto l = [](int NODEREF *x){  // expected-note{{x declared here}}
+    return *x;  // expected-warning{{dereferencing x; was declared with a 'noderef' type}}
+  };
+}
+
+int NODEREF *glob_ptr;  // expected-note{{glob_ptr declared here}}
+int glob_int = *glob_ptr;  // expected-warning{{dereferencing glob_ptr; was declared with a 'noderef' type}}

clang/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}}
+}

clang/test/Frontend/noderef_templates.cpp

+// RUN: %clang_cc1 -verify %s
+
+#define NODEREF __attribute__((noderef))
+
+template <typename T>
+int func(T NODEREF *a) { // expected-note 2 {{a declared here}}
+  return *a + 1;         // expected-warning 2 {{dereferencing a; was declared with a 'noderef' type}}
+}
+
+void func() {
+  float NODEREF *f;
+  int NODEREF *i;
+  func(f); // expected-note{{in instantiation of function template specialization 'func<float>' requested here}}
+  func(i); // expected-note{{in instantiation of function template specialization 'func<int>' requested here}}
+}