[Sema/Attribute] Ignore noderef attribute in unevaluated context

[thejh]

Summary

The noderef attribute is for catching code that accesses pointers in
a different address space. Unevaluated code is always safe in that regard.

[thejh]

re-uploading to trigger a new build, since the build error looks unrelated.
maybe current trunk is flaky?

[leonardchan]

LGTM but you should probably also wait for @aaron.ballman or @rsmith to also chime in.

[aaron.ballman]

clang/test/Frontend/noderef.c

71

Can you add tests for the weird situations where the expression actually is evaluated? e.g.,

struct S {
  virtual ~S(); // Make S polymorphic
};

S NODEREF *s;
typeid(*s); // Actually evaluates *s at runtime

struct T {
  unsigned i;
};

T t1;
T NODEREF *t2 = &t1;

sizeof(int[++t2->i]); // Actually evaluates t2->i at runtime

[thejh]

As requested by @aaron.ballman, added tests for edgecases where
sizeof()/typeid() can cause memory access.

[thejh]

clang/test/Frontend/noderef.c

71

Oh jeez, good point. I'm extremely happy that other people have done the hard work of building the ExpressionEvaluationContext logic... I've added tests for the cases you described.

[aaron.ballman]

LGTM!

[thejh]

@aaron.ballman Can you land it for me? I don't have commit access.

[aaron.ballman]

In D91828#2411203, @thejh wrote:

@aaron.ballman Can you land it for me? I don't have commit access.

Happy to do so -- are you okay with "Jann Horn <[e-mail]>" for author attribution?

[thejh]

In D91828#2411207, @aaron.ballman wrote:

In D91828#2411203, @thejh wrote:

@aaron.ballman Can you land it for me? I don't have commit access.

Happy to do so -- are you okay with "Jann Horn <[e-mail]>" for author attribution?

Yes, thanks.

[aaron.ballman]

In D91828#2411210, @thejh wrote:

In D91828#2411207, @aaron.ballman wrote:

In D91828#2411203, @thejh wrote:

@aaron.ballman Can you land it for me? I don't have commit access.

Happy to do so -- are you okay with "Jann Horn <[e-mail]>" for author attribution?

Yes, thanks.

Thank you for the patch! I've committed the changes for you in 00dad9d028ce31739b992a3ce2df5de054a9fa3c

clang/lib/Sema/SemaExpr.cpp

void Sema::CheckAddressOfNoDeref(const Expr *E) {
   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) {
+  if (isUnevaluatedContext())
+    return;
+
   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)) {
ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
   if (Opc != UO_AddrOf && Opc != UO_Deref)
     CheckArrayAccess(Input.get());
 
   auto *UO =
       UnaryOperator::Create(Context, Input.get(), Opc, resultType, VK, OK,
                             OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-      !isa<ArrayType>(UO->getType().getDesugaredType(Context)))
+      !isa<ArrayType>(UO->getType().getDesugaredType(Context)) &&
+      !isUnevaluatedContext())
     ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.
   if (ConvertHalfVec)
     return convertVector(UO, Context.HalfTy, *this);
   return UO;
 }
 

clang/lib/Sema/SemaExprMember.cpp

ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base,
 
   if (!Res.isInvalid() && isa<MemberExpr>(Res.get()))
     CheckMemberAccessOfNoDeref(cast<MemberExpr>(Res.get()));
 
   return Res;
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+    return;
+
   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()) {

clang/test/Frontend/noderef.c

int test() {
 
   // 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;
 
+  // Nothing in sizeof() actually accesses memory
+  x = sizeof(s->a);          // ok
+  x = sizeof(*s);            // ok
+  x = sizeof(s[0]);          // ok
+  x = sizeof(s->a + (s->b)); // ok
+
   // 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}}