Skip to content
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

Assertion failed 'op1 != nullptr' in 'TestUtil.TestLog:VerifyOutput():int:this' during 'Morph - Global' (IL size 130) #298

Closed
MichalStrehovsky opened this issue Nov 4, 2020 · 4 comments · Fixed by dotnet/runtime#44398
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation

Comments

@MichalStrehovsky
Copy link
Member

This is a new failure that started showing up in the Pri-0 test suite.

>	clrjitilc.dll!Compiler::optNonNullAssertionProp_Call(unsigned __int64 * const & assertions, GenTreeCall * call) Line 3838	C++
 	clrjitilc.dll!Compiler::optAssertionProp_Call(unsigned __int64 * const & assertions, GenTreeCall * call, Statement * stmt) Line 3881	C++
 	clrjitilc.dll!Compiler::optAssertionProp(unsigned __int64 * const & assertions, GenTree * tree, Statement * stmt, BasicBlock * block) Line 4161	C++
 	clrjitilc.dll!Compiler::fgMorphTree(GenTree * tree, Compiler::MorphAddrContext * mac) Line 15507	C++
 	clrjitilc.dll!Compiler::fgMorphCast(GenTree * tree) Line 519	C++
 	clrjitilc.dll!Compiler::fgMorphSmpOp(GenTree * tree, Compiler::MorphAddrContext * mac) Line 11810	C++
 	clrjitilc.dll!Compiler::fgMorphTree(GenTree * tree, Compiler::MorphAddrContext * mac) Line 15544	C++
 	clrjitilc.dll!Compiler::fgMorphSmpOp(GenTree * tree, Compiler::MorphAddrContext * mac) Line 12465	C++
 	clrjitilc.dll!Compiler::fgMorphTree(GenTree * tree, Compiler::MorphAddrContext * mac) Line 15544	C++
 	clrjitilc.dll!Compiler::fgMorphSmpOp(GenTree * tree, Compiler::MorphAddrContext * mac) Line 12465	C++
 	clrjitilc.dll!Compiler::fgMorphTree(GenTree * tree, Compiler::MorphAddrContext * mac) Line 15544	C++
 	clrjitilc.dll!Compiler::fgMorphStmts(BasicBlock * block, bool * lnot, bool * loadw) Line 16430	C++
 	clrjitilc.dll!Compiler::fgMorphBlocks() Line 16673	C++

We're hitting this for code in the shape of:

if (String.Empty.Equals(diff))

The problem is that String.Empty field is an intrinsic, so we import it specially (as a literal string), and there's super special handling for CoreRT because empty string can be referred to as IAT_VALUE (no need for an indirection like on CoreCLR):

case IAT_VALUE:
// For CoreRT only - Constant object can be a frozen string.
if (!IsTargetAbi(CORINFO_CORERT_ABI))
{
// Non CoreRT - This case is illegal, creating a TYP_REF from an INT_CNS
noway_assert(!"unreachable IAT_VALUE case in gtNewStringLiteralNode");
}
tree = gtNewIconEmbHndNode(pValue, nullptr, GTF_ICON_STR_HDL, nullptr);
tree->gtType = TYP_REF;
#ifdef DEBUG
tree->AsIntCon()->gtTargetHandle = (size_t)pValue;
#endif
tree = gtNewOperNode(GT_NOP, TYP_REF, tree); // prevents constant folding
break;

Note we're wrapping the literal in a GT_NOP to prevent constant folding.

We then hit the GT_NOP handling here (because this argument to the Equals method call is the GT_NOP):

if (thisArg->OperIs(GT_NOP, GT_ASG) == false)
{
if ((thisArg->gtFlags & GTF_LATE_ARG) == 0)
{
return thisArg;
}
}

And end up returning null from gtGetThisArg which asserts optNonNullAssertionProp_Call.

This error mode can probably be simulated on CoreCLR by just whacking CEEJitInfo::emptyStringLiteral to return IAT_VALUE (it would generate bad code, but the assert is hit before the code finishes generating) and compiling String.Empty.Equals("some other string").

@sandreenko Would you be able to recommend the best course of action for this?

@MichalStrehovsky MichalStrehovsky added the area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation label Nov 4, 2020
@sandreenko
Copy link

Hi @MichalStrehovsky, thank you for the analysis, looking at this now.

@sandreenko
Copy link

Could you please give me Complus_JitDump for the method with complus_JitDumpBeforeAfterMorph=1?
I was able to get such tress, but gtGetThisArg returns GT_NOP from:

if (call->gtCallLateArgs != nullptr)
{
unsigned argNum = 0;
fgArgTabEntry* thisArgTabEntry = gtArgEntryByArgNum(call, argNum);
GenTree* result = thisArgTabEntry->GetNode();

@MichalStrehovsky
Copy link
Member Author

Thank you for looking into it! The gtCallLateArgs are null in my case. This is for Ngen, not sure if that matters.

Here's the ngendump with beforeaftermorph set:

ngendump.txt

There's something odd with how the trees look like: the this argument seems to be missing in the dump.

@MichalStrehovsky
Copy link
Member Author

Btw if I remove the tree = gtNewOperNode(GT_NOP, TYP_REF, tree); // prevents constant folding line, the ngendump looks saner and the issue is not hit:

ngendump2.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants