Stack allocated boxes end up address exposed

[hez2010]

Description

Repro:

using System.Globalization;
using System.Runtime.CompilerServices;

Test();

static void Test()
{
	PrintNegatedBoolean(true);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static void PrintNegatedBoolean(bool b)
{
	var converted = new MyConverter().Convert(b, typeof(bool), null!, null!);
	if (converted is bool convertedBool)
	{
		Console.WriteLine(convertedBool);
	}
}

class MyConverter : IValueConverter
{
	public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
	{
		if (value is bool b && targetType == typeof(bool))
		{
			return !b;
		}

		return Throw();
	}

	private object Throw() => throw new NotSupportedException();

	public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
	{
		throw new NotImplementedException();
	}
}

public interface IValueConverter
{
	object Convert(object value, Type targetType, object parameter, CultureInfo culture);
	object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture);
}

Codegen for Test (tier 1):

G_M000_IG01:                ;; offset=0x0000
       push     rbx
       sub      rsp, 48

G_M000_IG02:                ;; offset=0x0005
       vxorps   xmm0, xmm0, xmm0
       vmovups  xmmword ptr [rsp+0x20], xmm0
       mov      rcx, 0x7FFA2F1B2D40
       mov      qword ptr [rsp+0x20], rcx
       mov      byte  ptr [rsp+0x28], 0
       lea      rbx, [rsp+0x20]
       test     rbx, rbx
       je       SHORT G_M000_IG05

G_M000_IG03:                ;; offset=0x002D
       mov      rdx, qword ptr [rbx]
       cmp      rdx, rcx
       je       SHORT G_M000_IG07
       jmp      SHORT G_M000_IG05

G_M000_IG04:                ;; offset=0x0037
       movzx    rcx, byte  ptr [rbx+0x08]
       call     [System.Console:WriteLine(ubyte)]

G_M000_IG05:                ;; offset=0x0041
       nop

G_M000_IG06:                ;; offset=0x0042
       add      rsp, 48
       pop      rbx
       ret

G_M000_IG07:                ;; offset=0x0048
       cmp      rdx, rcx
       je       SHORT G_M000_IG04

G_M000_IG08:                ;; offset=0x004D
       call     CORINFO_HELP_UNBOX_TYPETEST
       jmp      SHORT G_M000_IG04

Configuration

53a8a01

/cc: @AndyAyersMS

[AndyAyersMS]

The return Throw(); causes problems because during escape analysis the JIT thinks the result of the inlined call to Convert might be a heap or stack object, and this blocks some of the exposure-avoiding rewriting in local morph. We don't fix up control flow for the throw until we get to morph and by then it's too late.

If you rewrite as

	public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
	{
		if (value is bool b && targetType == typeof(bool))
		{
			return !b;
		}

                throw new NotSupportedException();
	}

you would get the expected cleanup:

; Method Program:<<Main>$>g__Test|0_0() (FullOpts)
G_M10120_IG01:  ;; offset=0x0000
						;; size=0 bbWeight=1 PerfScore 0.00

G_M10120_IG02:  ;; offset=0x0000
       xor      ecx, ecx
						;; size=2 bbWeight=1 PerfScore 0.25

G_M10120_IG03:  ;; offset=0x0002
       tail.jmp [System.Console:WriteLine(ubyte)]
						;; size=6 bbWeight=1 PerfScore 2.00
; Total bytes of code: 8

[AndyAyersMS]

Since the local address visitor is also now join-sensitive perhaps it would be worth trying to trim this control flow earlier. But it requires IR walks... unless, say we detect this during inlining and are able to handle things there. That is, if the inline fail reason is "no return," then add the node/stmt/etc to some worklist, and after inlining is done, we run down the list, split trees just before the calls, discard what comes after, change the block to BBJ_THROW, and trim of successor edges. Maybe.

[jakobbotsch]

Definitely seems like general goodness if we can refine the control flow earlier. One alternative is to implement something like #94563 in local morph too, and track which predecessors can reach which successors. We could support these no-return calls as well as the conditional branches that are null checks/address comparisons that local morph is now able to decide.

[AndyAyersMS]

FWIW there are likely several other reasons why stack-allocated boxes end up getting exposed, and this generally leads to relatively poor codegen like we see in this issue (code may well end up slower overall from the prolog zeroing, though we save on heap allocation). It would be good to systematically try and improve here.

[AndyAyersMS]

Marking this as future, though I may try and work on it sooner.