In CodeGen::CodeGenFunction::EmitCallArgs: Assertion `[...] && "type mismatch in call argument!"' failed

[mstorsjo]

Bugzilla Link	45000
Resolution	FIXED
Resolved on	May 06, 2020 20:10
Version	trunk
OS	All
Blocks	#44654
Attachments	Reduced testcase
CC	@AaronBallman,@aaronpuchert,@zygoloid,@spavloff,@tstellar
Fixed by commit(s)	f43859a 9c80516

Extended Description

Building current Qt dev branch for mingw targets triggers failed asserts in Clang. This is not a regression, but the same code seems to trigger failed asserts similarly in the last few releases at least. The reduced testcase triggers errors for any target, not only for mingw.

Not sure how it behaves in builds with asserts disabled (working fine i.e. assert incorrect for this case, or triggering undefined behaviours) though.

To reproduce:

$ cat qnetworkaccess-reduced.cpp 
struct a {};
struct b {
  enum { c };
};
class d {
public:
  int e;
  int f;
  int alignment;
  int flags;
  int g;
  using h = void *;
  h i;
};
template <typename> class j {
public:
  static d k;
};
template <typename l>
d j<l>::k{0, sizeof(l), alignof(l), b::c, 0, [](a = {}) -> char * {}()};
template <typename> void m() {
  using n = int;
  j<n>::k;
}
void o() { m<int>; }
$ bin/clang++ -w -c qnetworkaccess-reduced.cpp 
clang++: ../tools/clang/lib/CodeGen/CodeGenFunction.h:4411: void clang::CodeGen::CodeGenFunction::EmitCallArgs(clang::CodeGen::CallArgList&, const T*, llvm::iterator_range<clang::Stmt::CastIterator<clang::Expr, const clang::Expr* const, const clang::Stmt* const> >, clang::CodeGen::CodeGenFunction::AbstractCallee, unsigned int, clang::CodeGen::CodeGenFunction::EvaluationOrder) [with T = clang::FunctionProtoType]: Assertion `(isGenericMethod || ((*I)->isVariablyModifiedType() || (*I).getNonReferenceType()->isObjCRetainableType() || getContext() .getCanonicalType((*I).getNonReferenceType()) .getTypePtr() == getContext() .getCanonicalType((*Arg)->getType()) .getTypePtr())) && "type mismatch in call argument!"' failed.
Stack dump:
0.	Program arguments: bin/clang++ -w -c qnetworkaccess-reduced.cpp 
1.	<eof> parser at end of file
2.	qnetworkaccess-reduced.cpp:21:26: instantiating function definition 'm<int>'
3.	qnetworkaccess-reduced.cpp:17:12: instantiating variable definition 'j<int>::k'
4.	qnetworkaccess-reduced.cpp:20:9: Generating code for declaration 'j<int>::k'
 #&#8203;0 0x000055638a9fbf1a llvm::sys::PrintStackTrace(llvm::raw_ostream&) (bin/clang+++0x4a83f1a)
 #&#8203;1 0x000055638a9f9974 llvm::sys::RunSignalHandlers() (bin/clang+++0x4a81974)
 #&#8203;2 0x000055638a9f9be5 llvm::sys::CleanupOnSignal(unsigned long) (bin/clang+++0x4a81be5)
 #&#8203;3 0x000055638a965bf8 CrashRecoverySignalHandler(int) (bin/clang+++0x49edbf8)
 #&#8203;4 0x00007f0f1655d890 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #&#8203;5 0x00007f0f1520ee97 raise /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #&#8203;6 0x00007f0f15210801 abort /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
 #&#8203;7 0x00007f0f1520039a __assert_fail_base /build/glibc-OTsEL5/glibc-2.27/assert/assert.c:89:0
 #&#8203;8 0x00007f0f15200412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
 #&#8203;9 0x000055638ad3e3c0 void clang::CodeGen::CodeGenFunction::EmitCallArgs<clang::FunctionProtoType>(clang::CodeGen::CallArgList&, clang::FunctionProtoType const*, llvm::iterator_range<clang::Stmt::CastIterator<clang::Expr, clang::Expr const* const, clang::Stmt const* const> >, clang::CodeGen::CodeGenFunction::AbstractCallee, unsigned int, clang::CodeGen::CodeGenFunction::EvaluationOrder) (bin/clang+++0x4dc63c0)
#&#8203;10 0x000055638ad50530 commonEmitCXXMemberOrOperatorCall(clang::CodeGen::CodeGenFunction&, clang::CXXMethodDecl const*, llvm::Value*, llvm::Value*, clang::QualType, clang::CallExpr const*, clang::CodeGen::CallArgList&, clang::CodeGen::CallArgList*) (bin/clang+++0x4dd8530)
#&#8203;11 0x000055638ad508c7 clang::CodeGen::CodeGenFunction::EmitCXXMemberOrOperatorCall(clang::CXXMethodDecl const*, clang::CodeGen::CGCallee const&, clang::CodeGen::ReturnValueSlot, llvm::Value*, llvm::Value*, clang::QualType, clang::CallExpr const*, clang::CodeGen::CallArgList*) (bin/clang+++0x4dd88c7)
#&#8203;12 0x000055638ad527b0 clang::CodeGen::CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(clang::CallExpr const*, clang::CXXMethodDecl const*, clang::CodeGen::ReturnValueSlot, bool, clang::NestedNameSpecifier*, bool, clang::Expr const*) (bin/clang+++0x4dda7b0)
#&#8203;13 0x000055638ad533f2 clang::CodeGen::CodeGenFunction::EmitCXXOperatorMemberCallExpr(clang::CXXOperatorCallExpr const*, clang::CXXMethodDecl const*, clang::CodeGen::ReturnValueSlot) (bin/clang+++0x4ddb3f2)
#&#8203;14 0x000055638ada5943 clang::CodeGen::CodeGenFunction::EmitCallExpr(clang::CallExpr const*, clang::CodeGen::ReturnValueSlot) (bin/clang+++0x4e2d943)
#&#8203;15 0x000055638ad12766 (anonymous namespace)::ScalarExprEmitter::VisitCallExpr(clang::CallExpr const*) (bin/clang+++0x4d9a766)
#&#8203;16 0x000055638ad1097c (anonymous namespace)::ScalarExprEmitter::Visit(clang::Expr*) (bin/clang+++0x4d9897c)
#&#8203;17 0x000055638ad1627f (anonymous namespace)::ScalarExprEmitter::VisitCastExpr(clang::CastExpr*) (bin/clang+++0x4d9e27f)
#&#8203;18 0x000055638ad11322 (anonymous namespace)::ScalarExprEmitter::Visit(clang::Expr*) (bin/clang+++0x4d99322)
#&#8203;19 0x000055638ad11e33 clang::CodeGen::CodeGenFunction::EmitScalarExpr(clang::Expr const*, bool) (bin/clang+++0x4d99e33)
#&#8203;20 0x000055638ace320a clang::CodeGen::CodeGenFunction::EmitScalarInit(clang::Expr const*, clang::ValueDecl const*, clang::CodeGen::LValue, bool) (bin/clang+++0x4d6b20a)
#&#8203;21 0x000055638adc14bf (anonymous namespace)::AggExprEmitter::EmitInitializationToLValue(clang::Expr*, clang::CodeGen::LValue) (bin/clang+++0x4e494bf)
#&#8203;22 0x000055638adc3d75 (anonymous namespace)::AggExprEmitter::VisitInitListExpr(clang::InitListExpr*) (bin/clang+++0x4e4bd75)
#&#8203;23 0x000055638adc01d4 (anonymous namespace)::AggExprEmitter::Visit(clang::Expr*) (bin/clang+++0x4e481d4)
#&#8203;24 0x000055638adc06a4 (anonymous namespace)::AggExprEmitter::Visit(clang::Expr*) (bin/clang+++0x4e486a4)
#&#8203;25 0x000055638adc097e clang::CodeGen::CodeGenFunction::EmitAggExpr(clang::Expr const*, clang::CodeGen::AggValueSlot) (bin/clang+++0x4e4897e)
#&#8203;26 0x000055638ae38b85 clang::CodeGen::CodeGenFunction::EmitCXXGlobalVarDeclInit(clang::VarDecl const&, llvm::Constant*, bool) (bin/clang+++0x4ec0b85)
#&#8203;27 0x000055638b014921 (anonymous namespace)::ItaniumCXXABI::EmitGuardedInit(clang::CodeGen::CodeGenFunction&, clang::VarDecl const&, llvm::GlobalVariable*, bool) (bin/clang+++0x509c921)
#&#8203;28 0x000055638ae39583 clang::CodeGen::CodeGenFunction::GenerateCXXGlobalVarDeclInitFunc(llvm::Function*, clang::VarDecl const*, llvm::GlobalVariable*, bool) (bin/clang+++0x4ec1583)
#&#8203;29 0x000055638ae39a0a clang::CodeGen::CodeGenModule::EmitCXXGlobalVarDeclInitFunc(clang::VarDecl const*, llvm::GlobalVariable*, bool) (bin/clang+++0x4ec1a0a)
#&#8203;30 0x000055638ac7987b clang::CodeGen::CodeGenModule::EmitGlobalVarDefinition(clang::VarDecl const*, bool) (bin/clang+++0x4d0187b)
#&#8203;31 0x000055638ac8e9e1 clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl, llvm::GlobalValue*) (bin/clang+++0x4d169e1)
#&#8203;32 0x000055638ac8f3e3 clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) (bin/clang+++0x4d173e3)
#&#8203;33 0x000055638ac9588c clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) (.part.5915) (bin/clang+++0x4d1d88c)
#&#8203;34 0x000055638ca9a198 clang::Sema::InstantiateVariableDefinition(clang::SourceLocation, clang::VarDecl*, bool, bool, bool) (bin/clang+++0x6b22198)
#&#8203;35 0x000055638ca9ae23 clang::Sema::PerformPendingInstantiations(bool) (bin/clang+++0x6b22e23)
#&#8203;36 0x000055638ca9bb38 clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool, bool) (bin/clang+++0x6b23b38)
#&#8203;37 0x000055638ca9ac9e clang::Sema::PerformPendingInstantiations(bool) (bin/clang+++0x6b22c9e)
#&#8203;38 0x000055638c3eb34b clang::Sema::ActOnEndOfTranslationUnitFragment(clang::Sema::TUFragmentKind) (.part.1380) (bin/clang+++0x647334b)
#&#8203;39 0x000055638c3ebaef clang::Sema::ActOnEndOfTranslationUnit() (bin/clang+++0x6473aef)
#&#8203;40 0x000055638c308b23 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, bool) (bin/clang+++0x6390b23)
#&#8203;41 0x000055638c2fe799 clang::ParseAST(clang::Sema&, bool, bool) (bin/clang+++0x6386799)
#&#8203;42 0x000055638b46a418 clang::CodeGenAction::ExecuteAction() (bin/clang+++0x54f2418)
#&#8203;43 0x000055638b3842f9 clang::FrontendAction::Execute() (bin/clang+++0x540c2f9)
#&#8203;44 0x000055638b2eb4f2 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (bin/clang+++0x53734f2)
#&#8203;45 0x000055638b4444dc clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (bin/clang+++0x54cc4dc)
#&#8203;46 0x00005563894d15b4 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (bin/clang+++0x35595b4)
#&#8203;47 0x00005563894ca4c9 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) (bin/clang+++0x35524c9)
#&#8203;48 0x000055638b1cd2a5 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, bool*) const::'lambda'()>(long) (bin/clang+++0x52552a5)
#&#8203;49 0x000055638a965cd3 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (bin/clang+++0x49edcd3)
#&#8203;50 0x000055638b1cdd88 clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, bool*) const (.part.147) (bin/clang+++0x5255d88)
#&#8203;51 0x000055638b1a7165 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&) const (bin/clang+++0x522f165)
#&#8203;52 0x000055638b1a7c21 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*> >&) const (bin/clang+++0x522fc21)
#&#8203;53 0x000055638b1b02a9 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*> >&) (bin/clang+++0x52382a9)
#&#8203;54 0x00005563894cea3f main (bin/clang+++0x3556a3f)
#&#8203;55 0x00007f0f151f1b97 __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:344:0
#&#8203;56 0x00005563894ca02a _start (bin/clang+++0x355202a)
clang-11: error: clang frontend command failed due to signal (use -v to see invocation)
clang version 11.0.0 (https://github.com/llvm/llvm-project 82879c2913da69ef2deadee9d075140a84eb6e8c)
Target: x86_64-unknown-linux-gnu

[mstorsjo]

assigned to @aaronpuchert

[aaronpuchert]

Had a brief look in the debugger. (*Arg)->getType() is the BuiltinType "void", whereas (*I).getNonReferenceType() is a RecordType referring to the RecordDecl with name "a". Indeed, when running with -fsyntax-only -Xclang -ast-dump we get this for [](a = {}) -> char * {}() (minus addresses and source info):

ImplicitCastExpr 'd::h':'void *' <BitCast>
`-CXXOperatorCallExpr 'char *'
  |-ImplicitCastExpr char *' <FunctionToPointerDecay>
  | `-DeclRefExpr char *'
  |-ImplicitCastExpr 'const j<int>::(lambda)' lvalue <NoOp>
  | `-MaterializeTemporaryExpr 'j<int>::(lambda)' lvalue
  |   `-LambdaExpr 'j<int>::(lambda at qnetworkaccess-reduced.cpp:20:46)'
  |     |-CXXRecordDecl implicit class definition
  |     | |-[...]
  |     | |-CXXMethodDecl char *' inline
  |     | | |-ParmVarDecl 'a' cinit
  |     | | | `-InitListExpr 'void'
  |     | | `-CompoundStmt
  |     | `-[...]
  |     `-CompoundStmt
  `-CXXDefaultArgExpr 'void'

So there is a type mismatch in the AST, which makes this a frontend issue.

[aaronpuchert]

I was able to simplify the reproducer:

struct a {};

template <typename l>
int k{[](a = {}) -> int {}()};

template int k<int>;

The type of the InitListExpr is 'a' for the VarTemplateDecl, but 'void' for the VarTemplateSpecializationDecl with argument 'int'. So this looks like a bug in the template instantiation.

[mstorsjo]

I was able to simplify the reproducer:

struct a {};

template
int k{[](a = {}) -> int {}()};

template int k;

The type of the InitListExpr is 'a' for the VarTemplateDecl, but 'void' for
the VarTemplateSpecializationDecl with argument 'int'. So this looks like a
bug in the template instantiation.

Thanks for looking into it!

As I'm quite unfamiliar with those areas of Clang, would you have time to look further into it, or who would be most familiar with that particular area?

[aaronpuchert]

As I'm quite unfamiliar with those areas of Clang, would you have time to
look further into it, or who would be most familiar with that particular
area?

Most familiar is probably Richard Smith (already on CC). I have a healthy respect for the template instantiation machinery. But I also have a bug of my own in that area, so who knows, maybe I can carve out the time.

[mstorsjo]

I think I've narrowed down of the critical bits to this, in Sema::SubstParmVarDecl

llvm-project/clang/lib/Sema/SemaTemplateInstantiate.cpp

Lines 2340 to 2359 in b4c3a76

	} else if (Expr *Arg = OldParm->getDefaultArg()) {
	FunctionDecl *OwningFunc = cast<FunctionDecl>(OldParm->getDeclContext());
	if (OwningFunc->isLexicallyWithinFunctionOrMethod()) {
	// Instantiate default arguments for methods of local classes (DR1484)
	// and non-defining declarations.
	Sema::ContextRAII SavedContext(*this, OwningFunc);
	LocalInstantiationScope Local(*this, true);
	ExprResult NewArg = SubstExpr(Arg, TemplateArgs);
	if (NewArg.isUsable()) {
	// It would be nice if we still had this.
	SourceLocation EqualLoc = NewArg.get()->getBeginLoc();
	SetParamDefaultArgument(NewParm, NewArg.get(), EqualLoc);
	}
	} else {
	// FIXME: if we non-lazily instantiated non-dependent default args for
	// non-dependent parameter types we could remove a bunch of duplicate
	// conversion warnings for such arguments.
	NewParm->setUninstantiatedDefaultArg(Arg);
	}
	}

:

} else if (Expr *Arg = OldParm->getDefaultArg()) {
FunctionDecl *OwningFunc = cast(OldParm->getDeclContext());
if (OwningFunc->isLexicallyWithinFunctionOrMethod()) {
// Instantiate default arguments for methods of local classes (DR1484)
// and non-defining declarations.
Sema::ContextRAII SavedContext(*this, OwningFunc);
LocalInstantiationScope Local(*this, true);
ExprResult NewArg = SubstExpr(Arg, TemplateArgs);
if (NewArg.isUsable()) {
// It would be nice if we still had this.
SourceLocation EqualLoc = NewArg.get()->getBeginLoc();
SetParamDefaultArgument(NewParm, NewArg.get(), EqualLoc);
}
} else {
// FIXME: if we non-lazily instantiated non-dependent default args for
// non-dependent parameter types we could remove a bunch of duplicate
// conversion warnings for such arguments.
NewParm->setUninstantiatedDefaultArg(Arg);
}
}

If I force this to take the isLexicallyWithinFunctionOrMethod() case, the type of the InitListExpr remains correct and there's no inconsistency in the AST.

[aaronpuchert]

If I force this to take the isLexicallyWithinFunctionOrMethod() case, the
type of the InitListExpr remains correct and there's no inconsistency in the
AST.

What is strange is that default arguments are otherwise instantiated when building the CallExpr via Sema::CheckCXXDefaultArgExpr, but Param->hasUninstantiatedDefaultArg() is false in this case.

In any event, I think we need to find out whether DR1484 applies to this case, i.e. do we need to instantiate the default argument even if it isn't needed? The way Clang sees it, this can't be a local class because a VarDecl isn't a DeclContext.

[aaronpuchert]

Turns out that lambda default arguments are instantiated anyway in TreeTransform::TransformLambdaExpr, specifically in this loop:

  for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
       I != NumParams; ++I) {
    auto *P = NewCallOperator->getParamDecl(I);
    if (P->hasUninstantiatedDefaultArg()) {
      EnterExpressionEvaluationContext Eval(
          getSema(),
          Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed, P);
      ExprResult R = getDerived().TransformExpr(
          E->getCallOperator()->getParamDecl(I)->getDefaultArg());
      P->setDefaultArg(R.get());
    }
  }

This is also where the void type is coming from, because from TransformInitListExpr eventually calls Sema::BuildInitList, which does

  E->setType(Context.VoidTy); // FIXME: just a place holder for now.

before returning E. That's understandable, we don't know the type here. In general, there has to be a proper initialization. We have to replace

      P->setDefaultArg(R.get());

by

      getSema().SetParamDefaultArgument(P, R.get(), R.get()->getBeginLoc());

So I proposed https://reviews.llvm.org/D76038. Still unsure how to test this, but given that we were missing an initialization there is a lot we can choose from, this also works:

template <typename T>
int k = [](T x = 0.0) -> int { return x; }();

template int k<int>;

Currently we produce:

  ParmVarDecl <col:12, col:16> col:14 'int':'int' cinit
  `-FloatingLiteral <col:16> 'double' 0.000000e+00

Instead of the correct

  ParmVarDecl <col:12, col:16> col:14 'int':'int' cinit
  `-ImplicitCastExpr <col:16> 'int':'int' <FloatingToIntegral>
    `-FloatingLiteral <col:16> 'double' 0.000000e+00

[mstorsjo]

Awesome, thanks for taking the time to fix this!

[aaronpuchert]

Change has landed in master. Maybe you could go and try if this fixes your original issue? Not sure if this should be ported back to the 10.0.x codeline, do you have an opinion on that?

[mstorsjo]

Change has landed in master. Maybe you could go and try if this fixes your
original issue? Not sure if this should be ported back to the 10.0.x
codeline, do you have an opinion on that?

It does indeed seem to fix my original issue as well - thanks for taking the time for fixing it!

Regarding backporting to 10.0.x, I think that'd be a great idea as it fixes a rare but valid cornercase - but I guess that's mostly up to Richard about whether he thinks there's too much risk of regression associated with it.

[aaronpuchert]

Change has landed in master. Maybe you could go and try if this fixes your
original issue? Not sure if this should be ported back to the 10.0.x
codeline, do you have an opinion on that?

Regarding backporting to 10.0.x, I think that'd be a great idea as it fixes
a rare but valid cornercase - but I guess that's mostly up to Richard about
whether he thinks there's too much risk of regression associated with it.

My opinion is that it might not be wrong to wait a bit and let the early adopters (= users of Clang master) test this is a bit. But you're right, Richard can probably best assess what the risks are.

@​Tom: What do you think about adding this to the 10.0.1 blockers but not act on it for now, deciding later whether we want this?

[tstellar]

@​Tom: What do you think about adding this to the 10.0.1 blockers but not act
on it for now, deciding later whether we want this?

That's fine.

[zygoloid]

Regarding backporting to 10.0.x, I think that'd be a great idea as it fixes
a rare but valid cornercase - but I guess that's mostly up to Richard about
whether he thinks there's too much risk of regression associated with it.

My opinion is that it might not be wrong to wait a bit and let the early
adopters (= users of Clang master) test this is a bit.

I think that's reasonable. The patch seems fine to merge to me if no-one complains for a week or so.

[mstorsjo]

Regarding backporting to 10.0.x, I think that'd be a great idea as it fixes
a rare but valid cornercase - but I guess that's mostly up to Richard about
whether he thinks there's too much risk of regression associated with it.

My opinion is that it might not be wrong to wait a bit and let the early
adopters (= users of Clang master) test this is a bit.

I think that's reasonable. The patch seems fine to merge to me if no-one
complains for a week or so.

Now one week later - I don't think there's been any complaints (at least I don't see any in the phab review thread) - so hopefully we can dare to backport it?