[PATCH] Fix IRGen for referencing a static local before emitting its decl

Richard Smith richard at metafoo.co.uk
Mon Sep 29 11:39:56 PDT 2014


================
Comment at: lib/CodeGen/CGDecl.cpp:170
@@ +169,3 @@
+
+static llvm::Constant *adjustAddrSpaceAndUpdateMap(CodeGenModule &CGM,
+                                                   const VarDecl &D,
----------------
Would be useful for this function name to mention something about static local variables.

================
Comment at: lib/CodeGen/CGDecl.cpp:307-309
@@ -277,3 +306,5 @@
 
-  GV->setConstant(CGM.isTypeConstant(D.getType(), true));
-  GV->setInitializer(Init);
+  // Constant evaluation might fail when referencing the static local from some
+  // other scope and succeed when we enter the context of the function. If so,
+  // update the initializer.
+  assert(GV->getType()->getElementType() == Init->getType() &&
----------------
Do we ever need to re-emit the initializer for correctness? The only cases I can think of are like:

  extern const int n;
  auto f() {
    static const int k = n;
    return [] { return k; }
  }
  auto x = f();
  const int n = 3;

... where we can choose to use either static or dynamic initialization for `f()::k`.

================
Comment at: lib/CodeGen/CGExprConstant.cpp:921-922
@@ -918,3 +920,4 @@
     case Expr::AddrLabelExprClass: {
-      assert(CGF && "Invalid address of label expression outside function.");
+      if (!CGF)
+        return llvm::Constant::getNullValue(ConvertType(E->getType()));
       llvm::Constant *Ptr =
----------------
Oh, I see. This is why `EmitInitializerForStaticVarDecl` might need to re-emit the initializer. It seems a little scary for this to silently return `0` in other cases, but I suppose it's not too bad. A comment here would be useful (and also one in `EmitInitializerForStaticVarDecl` explaining why we re-emit).

I'm not convinced this is correct, though. Consider an inline function that has a static local with an initializer that uses the address of a label, and exposes a local lambda that uses the static local. Even if that function is not used in *this* TU, we must still make the label addresses be correct, in case the function is used in a different TU. For instance:

  inline auto foo(void *p) {
    if (p) goto *p;
    static void *q = &&label;
    while (1) {
      struct S { static void *get() { return q; } };
      return S();
    label:
      puts("hello world");
    }
  }

TU1:
  void *global = decltype(foo(0))::get();

TU2:
  int main() { extern void *global; foo(global); }

You might say that you don't care about this case. I'd be inclined to agree =) Perhaps we should `ErrorUnsupported` on an `AddrLabelExpr` with no `CGF`?

http://reviews.llvm.org/D4787






More information about the cfe-commits mailing list