Skip to content

Clang crashes and mis-handles aggregate initialization with base initializers in certain cases #80510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rnk opened this issue Feb 2, 2024 · 1 comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. crash Prefer [crash-on-valid] or [crash-on-invalid]

Comments

@rnk
Copy link
Collaborator

rnk commented Feb 2, 2024

I've identified a few loops over InitListExprs for records that don't account for the changes in C++17 which allowed base initializers to appear in an aggregate initialization expression, namely here and here:

for (const auto *Field : RD->fields()) {

https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprConstant.cpp#L696

The first case represents an actual bug, since it means we don't check the remaining initializer list expressions, as in this example:

struct Empty {};
struct Foo : Empty {
  int x;
  int y;
};
int f();
Foo o = (Foo){{}, 1, f()};

Clang accepts this, but if you remove the Empty base and its initializer, it rejects it:

struct Foo  {
  int x;
  int y;
};
int f();
Foo o = (Foo){1, f()};

-->

t.cpp:9:19: error: initializer element is not a compile-time constant
    9 | Foo o = (Foo){1, f()};
      |                   ^~~
1 error generated.

Something is not right, and the dumped AST doesn't look right, it classifies f() as a ConstantExpr:

`-VarDecl 0x55c70d2d2190 <line:7:1, col:25> col:5 o 'Foo' cinit
  `-CompoundLiteralExpr 0x55c70d2d25c8 <col:9, col:25> 'Foo'
    `-InitListExpr 0x55c70d2d2390 <col:14, col:25> 'Foo'
      |-ConstantExpr 0x55c70d2d2580 <col:15, col:16> 'Empty'
      | `-InitListExpr 0x55c70d2d23e8 <col:15, col:16> 'Empty'
      |-ConstantExpr 0x55c70d2d2598 <col:19> 'int'
      | `-IntegerLiteral 0x55c70d2d2248 <col:19> 'int' 1
      `-ConstantExpr 0x55c70d2d25b0 <col:22, col:24> 'int'
        `-CallExpr 0x55c70d2d2318 <col:22, col:24> 'int'
          `-ImplicitCastExpr 0x55c70d2d2300 <col:22> 'int (*)()' <FunctionToPointerDecay>
            `-DeclRefExpr 0x55c70d2d22b0 <col:22> 'int ()' lvalue Function 0x55c70d2d2058 'f' 'int ()'

In this larger test case, this misclassification of this initializer as a constant ultimately results in a crash during codegen:

struct Empty {};
struct Foo : Empty {
  int x;
  int y;
};
int getint();
struct Span {
  Span(const Foo (&p)[1]);
};
Span defs = (Foo[1]){{.x = 0, getint()}};

-->

clang: ../clang/lib/CodeGen/CGExprConstant.cpp:932: ConstantAddress (anonymous namespace)::tryEmitGlobalCompoundLiteral(ConstantEmitter &, const CompoundLiteralExpr *): Assertion `!E->isFileScope() && "f
ile-scope compound literal did not have constant initializer!"' failed.                                                                                                                                    
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:                                                                                                                                                                                                
...
 #9 0x0000555717168e8e /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExprConstant.cpp:931:5
#10 0x0000555717168a75 clang::CodeGen::CodeGenModule::GetAddrOfConstantCompoundLiteral(clang::CompoundLiteralExpr const*) /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExprConstant.cpp:2243:1     
#11 0x0000555717127b31 clang::CodeGen::CodeGenFunction::EmitCompoundLiteralLValue(clang::CompoundLiteralExpr const*) /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExpr.cpp:4945:27                      
#12 0x0000555717122c19 clang::CodeGen::CodeGenFunction::EmitLValueHelper(clang::Expr const*, clang::CodeGen::KnownNonNull_t) /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExpr.cpp:0:12
...

I believe the solution is to update isConstantInitializer to iterate over bases and not just fields, but that may uncover more issues.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Feb 2, 2024
@EugeneZelenko EugeneZelenko added clang:codegen IR generation bugs: mangling, exceptions, etc. crash Prefer [crash-on-valid] or [crash-on-invalid] and removed clang Clang issues not falling into any other category labels Feb 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 2, 2024

@llvm/issue-subscribers-clang-codegen

Author: Reid Kleckner (rnk)

I've identified a few loops over InitListExprs for records that don't account for the changes in C++17 which allowed base initializers to appear in an aggregate initialization expression, namely here and here: https://github.com/llvm/llvm-project/blob/7a94acb2da5b20d12f13f3c5f4eb0f3f46e78e73/clang/lib/AST/Expr.cpp#L3345 https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprConstant.cpp#L696

The first case represents an actual bug, since it means we don't check the remaining initializer list expressions, as in this example:

struct Empty {};
struct Foo : Empty {
  int x;
  int y;
};
int f();
Foo o = (Foo){{}, 1, f()};

Clang accepts this, but if you remove the Empty base and its initializer, it rejects it:

struct Foo  {
  int x;
  int y;
};
int f();
Foo o = (Foo){1, f()};

-->

t.cpp:9:19: error: initializer element is not a compile-time constant
    9 | Foo o = (Foo){1, f()};
      |                   ^~~
1 error generated.

Something is not right, and the dumped AST doesn't look right, it classifies f() as a ConstantExpr:

`-VarDecl 0x55c70d2d2190 &lt;line:7:1, col:25&gt; col:5 o 'Foo' cinit
  `-CompoundLiteralExpr 0x55c70d2d25c8 &lt;col:9, col:25&gt; 'Foo'
    `-InitListExpr 0x55c70d2d2390 &lt;col:14, col:25&gt; 'Foo'
      |-ConstantExpr 0x55c70d2d2580 &lt;col:15, col:16&gt; 'Empty'
      | `-InitListExpr 0x55c70d2d23e8 &lt;col:15, col:16&gt; 'Empty'
      |-ConstantExpr 0x55c70d2d2598 &lt;col:19&gt; 'int'
      | `-IntegerLiteral 0x55c70d2d2248 &lt;col:19&gt; 'int' 1
      `-ConstantExpr 0x55c70d2d25b0 &lt;col:22, col:24&gt; 'int'
        `-CallExpr 0x55c70d2d2318 &lt;col:22, col:24&gt; 'int'
          `-ImplicitCastExpr 0x55c70d2d2300 &lt;col:22&gt; 'int (*)()' &lt;FunctionToPointerDecay&gt;
            `-DeclRefExpr 0x55c70d2d22b0 &lt;col:22&gt; 'int ()' lvalue Function 0x55c70d2d2058 'f' 'int ()'

In this larger test case, this misclassification of this initializer as a constant ultimately results in a crash during codegen:

struct Empty {};
struct Foo : Empty {
  int x;
  int y;
};
int getint();
struct Span {
  Span(const Foo (&amp;p)[1]);
};
Span defs = (Foo[1]){{.x = 0, getint()}};

-->

clang: ../clang/lib/CodeGen/CGExprConstant.cpp:932: ConstantAddress (anonymous namespace)::tryEmitGlobalCompoundLiteral(ConstantEmitter &amp;, const CompoundLiteralExpr *): Assertion `!E-&gt;isFileScope() &amp;&amp; "f
ile-scope compound literal did not have constant initializer!"' failed.                                                                                                                                    
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:                                                                                                                                                                                                
...
 #<!-- -->9 0x0000555717168e8e /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExprConstant.cpp:931:5
#<!-- -->10 0x0000555717168a75 clang::CodeGen::CodeGenModule::GetAddrOfConstantCompoundLiteral(clang::CompoundLiteralExpr const*) /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExprConstant.cpp:2243:1     
#<!-- -->11 0x0000555717127b31 clang::CodeGen::CodeGenFunction::EmitCompoundLiteralLValue(clang::CompoundLiteralExpr const*) /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExpr.cpp:4945:27                      
#<!-- -->12 0x0000555717122c19 clang::CodeGen::CodeGenFunction::EmitLValueHelper(clang::Expr const*, clang::CodeGen::KnownNonNull_t) /b/f/w/set_by_reclient/a/../clang/lib/CodeGen/CGExpr.cpp:0:12
...

I believe the solution is to update isConstantInitializer to iterate over bases and not just fields, but that may uncover more issues.

rnk added a commit that referenced this issue Feb 8, 2024
…#80519)

This code was correct as written prior to C++17, which allowed bases to
appear in the initializer list.

This was observable by creating non-constant aggregate initialization at
file scope in a compound literal, but since that behavior will change
soon if we implement support for dynamic initialization, I also added a
unit test for `isConstantInitializer`.

This fixes at least one part of issue #80510 .

---------

Co-authored-by: Aaron Ballman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. crash Prefer [crash-on-valid] or [crash-on-invalid]
Projects
None yet
Development

No branches or pull requests

3 participants