[cfe-commits] [PATCH] llvm.annotation support

[Julien Lerouge]

On Mon, Sep 05, 2011 at 01:15:39PM -0700, John McCall wrote:
> On Aug 30, 2011, at 10:36 PM, Julien Lerouge wrote:
> > Here is the second revision of this patch. It should address all the
> > previous comments. Thanks for all the feedback!
> 
> This is looking a lot better, thanks.
> 
> +++ b/lib/CodeGen/CGBuiltin.cpp
> @@ -456,7 +456,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
>    }
>  
>    case Builtin::BI__builtin_isfinite: {
> -    // isfinite(x) --> x == x && fabs(x) != infinity; }
> +    // isfinite(x) --> x == x && fabs(x) != infinity;
> 
> This is fine, but please commit it separately.

Sure, will do.

> 
> +  case Builtin::BI__builtin_annotation: {
> +    llvm::Value *AnnVal = EmitScalarExpr(E->getArg(0));
> +    llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::annotation,
> +                                      AnnVal->getType());
> +
> +    // Get the annotation string, go through casts.
> +    const Expr *AnnotationStrExpr = E->getArg(1)->IgnoreParenCasts();
> +    llvm::StringRef Str = cast<StringLiteral>(AnnotationStrExpr)->getString();
> +    return RValue::get(EmitAnnotationCall(F, AnnVal, Str, E->getExprLoc()));
> +  }
> 
> Please have this comment say something like "Sema requires this to be a non-wide string literal, potentially casted", so that it's clear why this cast<> will always succeed.
> 
> And then you should make that true by adding the appropriate case to Sema::CheckBuiltinFunctionCall. :)

Ok, the extra code is below. Thanks, for rewieving this! Let me know if
there is anything wrong.

 include/clang/Basic/DiagnosticSemaKinds.td |    3 +++
 include/clang/Sema/Sema.h                  |    1 +
 lib/CodeGen/CGBuiltin.cpp                  |    3 ++-
 lib/Sema/SemaChecking.cpp                  |   16 ++++++++++++++++
 test/Sema/annotate.c                       |    3 +++
 5 files changed, 25 insertions(+), 1 deletions(-)



-- 
Julien Lerouge
PGP Key Id: 0xB1964A62
PGP Fingerprint: 392D 4BAD DB8B CE7F 4E5F FA3C 62DB 4AA7 B196 4A62
PGP Public Key from: keyserver.pgp.com
-------------- next part --------------
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index f5d0656..7936cdd 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4423,6 +4423,9 @@ def err_return_in_block_expression : Error<
 def err_block_returning_array_function : Error<
   "block cannot return %select{array|function}0 type %1">;
 
+// Builtin annotation string.
+def err_builtin_annotation_not_string_constant : Error<
+  "__builtin_annotation requires a non wide string constant">;
 
 // CFString checking
 def err_cfstring_literal_not_string_constant : Error<
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 007c783..703bcc6 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -5991,6 +5991,7 @@ private:
 
   bool CheckablePrintfAttr(const FormatAttr *Format, CallExpr *TheCall);
   bool CheckObjCString(Expr *Arg);
+  bool CheckBuiltinAnnotationString(Expr *Arg);
 
   ExprResult CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp
index b5b385a..0c93ed3 100644
--- a/lib/CodeGen/CGBuiltin.cpp
+++ b/lib/CodeGen/CGBuiltin.cpp
@@ -996,7 +996,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
     llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::annotation,
                                       AnnVal->getType());
 
-    // Get the annotation string, go through casts.
+    // Get the annotation string, go through casts. Sema requires this to be a
+    // non-wide string literal, potentially casted, so the cast<> is safe.
     const Expr *AnnotationStrExpr = E->getArg(1)->IgnoreParenCasts();
     llvm::StringRef Str = cast<StringLiteral>(AnnotationStrExpr)->getString();
     return RValue::get(EmitAnnotationCall(F, AnnVal, Str, E->getExprLoc()));
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index f5dec06..806f16a 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -184,6 +184,10 @@ Sema::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
   case Builtin::BI__sync_lock_release:
   case Builtin::BI__sync_swap:
     return SemaBuiltinAtomicOverloaded(move(TheCallResult));
+  case Builtin::BI__builtin_annotation:
+    if (CheckBuiltinAnnotationString(TheCall->getArg(1)))
+      return ExprError();
+    break;
   }
   
   // Since the target specific builtins for each arch overlap, only check those
@@ -633,6 +637,18 @@ Sema::SemaBuiltinAtomicOverloaded(ExprResult TheCallResult) {
   return move(TheCallResult);
 }
 
+/// CheckBuiltinAnnotationString - Checks that string argument to the builtin
+/// annotation is a non wide string literal.
+bool Sema::CheckBuiltinAnnotationString(Expr *Arg) {
+  Arg = Arg->IgnoreParenCasts();
+  StringLiteral *Literal = dyn_cast<StringLiteral>(Arg);
+  if (!Literal || !Literal->isAscii()) {
+    Diag(Arg->getLocStart(), diag::err_builtin_annotation_not_string_constant)
+      << Arg->getSourceRange();
+    return true;
+  }
+  return false;
+}
 
 /// CheckObjCString - Checks that the argument to the builtin
 /// CFString constructor is correct
diff --git a/test/Sema/annotate.c b/test/Sema/annotate.c
index 6f81491..5b27277 100644
--- a/test/Sema/annotate.c
+++ b/test/Sema/annotate.c
@@ -4,4 +4,7 @@ void __attribute__((annotate("foo"))) foo(float *a) {
   __attribute__((annotate("bar"))) int x;
   __attribute__((annotate(1))) int y; // expected-error {{argument to annotate attribute was not a string literal}}
   __attribute__((annotate("bar", 1))) int z; // expected-error {{attribute takes one argument}}
+  int u = __builtin_annotation(z, (char*) 0); // expected-error {{__builtin_annotation requires a non wide string constant}}
+  int v = __builtin_annotation(z, (char*) L"bar"); // expected-error {{__builtin_annotation requires a non wide string constant}}
+  int w = __builtin_annotation(z, "foo");
 }