JIT: don't trust field offsets in R2R for nullcheck bypass (#16492)
authorAndy Ayers <andya@microsoft.com>
Fri, 23 Feb 2018 16:51:15 +0000 (08:51 -0800)
committerGitHub <noreply@github.com>
Fri, 23 Feb 2018 16:51:15 +0000 (08:51 -0800)
When the jit is forming an field address to pass off to points unknown
it will nullcheck at the point of creation, unless it can prove that the
field is at offset zero. Unfortunately in R2R mode field offsets may not
final and so a zero value seen when prejitting may end up being nonzero
when the code is loaded and fixed up and fool the jit into omitting a null
check that is potentially needed.

So in R2R mode, if a field offset is going to be fixed up, always emit null
checks.

Fixes #16454.

src/jit/morph.cpp

index abcd995..265e5d2 100644 (file)
@@ -6691,11 +6691,22 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac)
                 }
                 else
                 {
+                    // In R2R mode the field offset for some fields may change when the code
+                    // is loaded. So we can't rely on a zero offset here to suppress the null check.
+                    //
+                    // See GitHub issue #16454.
+                    bool fieldHasChangeableOffset = false;
+
+#ifdef FEATURE_READYTORUN_COMPILER
+                    fieldHasChangeableOffset = (tree->gtField.gtFieldLookup.addr != nullptr);
+#endif
+
 #if CONSERVATIVE_NULL_CHECK_BYREF_CREATION
-                    addExplicitNullCheck = (mac->m_kind == MACK_Addr && (mac->m_totalOffset + fldOffset > 0));
+                    addExplicitNullCheck = (mac->m_kind == MACK_Addr) &&
+                                           ((mac->m_totalOffset + fldOffset > 0) || fieldHasChangeableOffset);
 #else
                     addExplicitNullCheck = (objRef->gtType == TYP_BYREF && mac->m_kind == MACK_Addr &&
-                                            (mac->m_totalOffset + fldOffset > 0));
+                                            ((mac->m_totalOffset + fldOffset > 0) || fieldHasChangeableOffset));
 #endif
                 }
             }