Fix undefined behavior found with g++-12 ubsan

[AaronRobinsonMSFT]

Contributes to #73043

Enabling ubsan with g++-12 the following null related behavior was detected.

Passing null as either argument to memcpy is undefined, even if count is 0.
Dereference then take address-of a null.
Use of uninitialized memory - particularly loading of the verbose value in the JIT.

/cc @RobertHenry6bev

[RobertHenry6bev]

lgtm +1

[AaronRobinsonMSFT]

Failure is #73247.

[BruceForstall]

Seems odd there was a 0.03% TP regressions on Linux/x64 but 0.02% TP improvement on Linux/arm32. Why would that be?

https://dev.azure.com/dnceng/public/_build/results?buildId=1925798&view=ms.vss-build-web.run-extensions-tab

[AaronRobinsonMSFT]

Seems odd there was a 0.03% TP regressions on Linux/x64 but 0.02% TP improvement on Linux/arm32. Why would that be?

That does seem odd. Is this possibly noise in the system? I'm surprised there is much impact overall. Most of the logic was simply moved outside of the for loop statement and into the body.

[jakobbotsch]

Seems odd there was a 0.03% TP regressions on Linux/x64 but 0.02% TP improvement on Linux/arm32. Why would that be?

Yes, it is pretty curious. As you know these runs are all windows-xarch hosted cross jits, so presumably resolveRegisters is almost exactly the same codegen and the only difference is in the shapes of IR/ref positions the function sees, but tracking down what exactly is hard. May be some architecture specific thing like split args/multireg args causing some of these loops to be executed significantly different amount of times.

Is this possibly noise in the system?

I don't think so, the throughput runs use instrumentation counting number of instructions executed. The variance is only around 0.005%.

[markples]

I don't think so, the throughput runs use instrumentation counting number of instructions executed. The variance is only around 0.005%.

Looking at some recent runs, there are definitely cases that measure at 0.00%. An earlier version of this change was +0.04% to +0.10% on 64 bit and +0.02% to +0.04% on 32 bit (all arch/os). A different PR showed +0.01% to +0.04% on 64 bit and 0.00% to -0.02% on 32 bit. There's not enough to tell if both PRs had some impact or if there is new noise, though given the larger previous change (with an explanation for reduced work in the final version) it seems like there's probably something going on.

[jakobbotsch]

it seems like there's probably something going on.

I have not run into noise in these jobs before, and looking at some recent superpmi-diffs runs, I still think it is the case. E.g. here is a recent PR that touches nothing which shows no impact across the board:
https://dev.azure.com/dnceng/public/_build/results?buildId=1925811&view=ms.vss-build-web.run-extensions-tab

In that PR we have

libraries.pmi.Linux.arm.checked.mch	258,094,239,131	258,094,350,775	+0.00%

This is a 0.0000004% difference in number of instructions executed.

[markples]

I'm not sure if this actually makes me feel better, but #73589 (https://dev.azure.com/dnceng/public/_build/results?buildId=1930797&view=ms.vss-build-web.run-extensions-tab) is showing that it gets back the 0.03% (as well as 0.02% on win32).