[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen added a comment.

In D108479#3140688 <https://reviews.llvm.org/D108479#3140688>, @rjmccall wrote:

> In D108479#3140638 <https://reviews.llvm.org/D108479#3140638>, @samitolvanen wrote:
>
>> Sure, I can take a look at how that would work. Basically, in `PointerExprEvaluator::VisitBuiltinCallExpr` we should not evaluate the l-value and just leave it at `Result.set(E)`?
>
> Yes, exactly.  Since the builtin already requires a constant operand in non-dependent contexts, that should be enough.

This does indeed solve comparison issues and allows me to drop the changes to `APValue`, but it breaks initializing globals in C because Clang doesn't know the expression is a compile-time constant:

  $ cat test.c
  void a() {}
  const void *p = __builtin_function_start(a);
  $ clang -c test.c
  test.c:2:17: error: initializer element is not a compile-time constant
  const void *p = __builtin_function_start(a);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 error generated.

I worked around this for now by explicitly allowing `__builtin_function_start` in `CheckLValueConstantExpression`, but this seems terribly hacky. What would be the correct way to solve this issue?



================
Comment at: clang/lib/Sema/SemaChecking.cpp:208
+    if (UnaryOp->getOpcode() == UnaryOperator::Opcode::UO_AddrOf)
+      E = UnaryOp->getSubExpr();
+
----------------
rjmccall wrote:
> It would be more general to allow any expression that we can constant-evaluate to a specific function / member function reference.  That allows callers to do stuff like `__builtin_function_start((int (A::*)() const) &A::x)` to resolve overloaded function references.
> 
> You should delay this check if the operand is value-dependent.
> It would be more general to allow any expression that we can constant-evaluate to a specific function / member function reference.  That allows callers to do stuff like `__builtin_function_start((int (A::*)() const) &A::x)` to resolve overloaded function references.

I looked into using `Expr::EvaluateAsConstantExpr` here and while it works, I'm not sure if allowing arbitrary expressions as the argument provides any value. We can allow resolving overloaded function references without constant-evaluating the expression (and I added tests for this). Did you have any other use cases in mind where this might be useful?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:221
+  if (ResultType.isNull())
+    return true;
+
----------------
rjmccall wrote:
> I think we decided that the result type should be `void*`.  Are there other semantic checks from `CheckAddressOfOperand` that still meaningfully apply?
I thought `Sema::checkAddressOfFunctionIsAvailable` would be useful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108479/new/

https://reviews.llvm.org/D108479