llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.22k stars 11.65k forks source link

Assertion failed: ((!Result.isArray() || Result.getArrayInitializedElts() == 0) && "zero-initialized array shouldn't have any initialized elts" #65784

Closed ahatanak closed 1 year ago

ahatanak commented 1 year ago

$ cat test.cpp

struct ty { int *ptr; };
const struct ty arr[] = {(int *)("")};
void f(void) {
  int *x;
  x = arr[0].ptr;
}

$ clang++ -std=c++98 -c test.cpp Assertion failed: ((!Result.isArray() || Result.getArrayInitializedElts() == 0) && "zero-initialized array shouldn't have any initialized elts"), function VisitCXXParenListOrInitListExpr, file ExprConstant.cpp, line 11315

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-frontend

ahatanak commented 1 year ago

clang doesn't crash if I revert 610ec954e1f81c0e8fcadedcd25afe643f5a094e

@nickdesaulniers

nickdesaulniers commented 1 year ago

cc @efriedma-quic presumably this is related to the !Ctx.getLangOpts().CPlusPlus11 from the removed guard.

nickdesaulniers commented 1 year ago

Slightly reduced test case:

struct {
  int *ptr;
} const arr[] = {(int *)""};
void f() { *arr; }
nickdesaulniers commented 1 year ago

The ASTs look the same between language modes.

ArrayExprEvaluator::VisitCXXParenListOrInitListExpr gets called twice; once where Result.isArray is false for the top most InitListExpr. The second time it's true for c++98 but still false for c++11. I wonder why that is? (digging)

nickdesaulniers commented 1 year ago

Not really sure where Result is getting reset.

gdb watch points seem to think that performLifetimeExtension is somehow resetting these objects, but if I add print statements to that code or breakpoints to that function, it seems they are never run.

@efriedma-quic can we just delete the assertion? Removing it doesn't seem to cause any test failures for clang.

efriedma-quic commented 1 year ago

The assertion was added in 1b9f2eb76; I think the idea is that zero-initialization is supposed to happen at a particular point, and not later.

If Result.getArrayInitializedElts() is non-zero, that means there's something in the array already; what are the elements?

nickdesaulniers commented 1 year ago

Result.dump():

Array size=1
`-element: Struct
  `-field: LValue <todo>
nickdesaulniers commented 1 year ago
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 60c80f2b0753..6109829cf20a 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2555,10 +2555,10 @@ APValue *VarDecl::evaluateValueImpl(SmallVectorImpl<PartialDiagnosticAt> &Notes,
   bool Result = Init->EvaluateAsInitializer(Eval->Evaluated, Ctx, this, Notes,
                                             IsConstantInitialization);

-  // In C++11, this isn't a constant initializer if we produced notes. In that
+  // In C++, this isn't a constant initializer if we produced notes. In that
   // case, we can't keep the result, because it may only be correct under the
   // assumption that the initializer is a constant context.
-  if (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus11 &&
+  if (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus &&
       !Notes.empty())
     Result = false;

perhaps?

I believe the code directly below it is what's resetting the value, but only for C++11 (and newer). https://github.com/llvm/llvm-project/blob/0f052a972ebe9bdf2c1eb56bddf6abb04eec50d6/clang/lib/AST/Decl.cpp#L2555-L2569