Fix use of uninitialized memory for Vector3 constants

[jkotas]

Found while investigating #72149.

In a good case, it made the size and content of readonly data non-deterministic.
In a bad case, it exposed downstream bugs like #72149 intermittently.

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	jkotas
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[EgorBo]

I wonder if it should always be zero in the first place, it looks like that field is not zero-initialized when GenTreeVecCon is created

runtime/src/coreclr/jit/gentree.h

Lines 3510 to 3511 in 51f3ca6

	GenTreeVecCon(var_types type, CorInfoType simdBaseJitType)
	: GenTree(GT_CNS_VEC, type), gtSimdBaseJitType((unsigned char)simdBaseJitType)

and e.g. here it gets a garbage value in the 4th element:

runtime/src/coreclr/jit/assertionprop.cpp

Lines 3186 to 3187 in 51f3ca6

	GenTreeVecCon* vecCon = gtNewVconNode(tree->TypeGet(), CORINFO_TYPE_FLOAT);
	vecCon->gtSimd12Val = value;

since we create a non-zero initialized veccon tree and set only 12 bytes

[jakobbotsch]

It is technically UB to access the inactive union member there, so I think the code should rather be changed to access gtSimd12Val and widen it explicitly. There is a similar occurrence in instr.cpp it seems.

[jkotas]

I wonder if it should always be zero in the first place, it looks like that field is not zero-initialized when GenTreeVecCon is created

I am not following. The line that you have highlighted is initializing gtSimdBaseJitType field. This PR is fixing uninitialized value of gtSimd*Val field.

[jakobbotsch]

This should get the same fix:

runtime/src/coreclr/jit/instr.cpp

Lines 765 to 770 in 9e8d261

	case TYP_SIMD12:
	case TYP_SIMD16:
	{
	simd16_t constValue = op->AsVecCon()->gtSimd16Val;
	return OperandDesc(emit->emitSimd16Const(constValue));
	}

[jkotas]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2965504715

[EgorBo]

I am not following. The line that you have highlighted is initializing gtSimdBaseJitType field.

@jkotas That's the point - it doesn't initialize gtSimd32Val there leaving, potentially, garbage in it, does it?

[jkotas]

it doesn't initialize gtSimd32Val there leaving, potentially, garbage in it, does it?

Right. The type of the tree is TYP_SIMD12, so one can naturally expect that the value is stored in gtSimd12Val field.

I think it would be unnatural to require that the top 4 bytes of gtSimd16Val field have to be initialized for TYP_SIMD12 values.