Fix infinite isoltest expectation update loop on values not taking full slots

[cameel]

I ran into this while working on #12668. Isoltest falls into an infinite expectation update loop on some tests, e.g. malformed_panic_2.sol. This happens when the test fails for whatever reason and you accept the expectation update. The updated value does not really match what is returned from the EVM and it asks you to update again, ad infinitum.

Example of the problem

Let's take a look at malformed_panic_2.sol. The expectation that triggers the problem is this:

// b() -> FAILURE, hex"4e487b710000"

Isoltest will reformat it as something like this:

Obtained result:
// b() -> FAILURE, 35408467139433450592217433187231851964680881867096292958875400487594354540544
Warning: The call to "b()" returned
[4e,48,7b,71,0,0,0,0,0,0,0,0,0,0,0,0,39,3,0,0,3f,3,0,0,21,0,0,0,0,0,0,0]

the specific number is different each time and is the original expectation followed by some random garbage. Note also that the integer does not match the bytes shown below it. Its hex value is instead:

0x4e487b71000000000000000000000000703c5d549f5500004100000000000000

The cause

There are two separate bugs here.

The first one is that when formatting the expectation, BytesUtils assumes a specific format for the output (described by the ParameterList type). If the output happens to be shorter than the format, it increments an iterator past the end of the vector containing it, grabbing some random garbage from memory. That's what we see in the expectations.

Identical bug exists in both BytesUtils::formatBytesRange() and BytesUtils::formatRawBytes(). The first one we use to format the expectation comment, the second to display the array of individual bytes. That's why they don't match in the example I have shown above.

The second bug is that by default the expectation is always formatted as a sequence of 32-byte integers. When isoltest rereads such an expectation, it sees each integer as 32 bytes (because integers are right-aligned). That will never match output whose length is not a multiple of 32 bytes.

The only reason it does not happen more often is that we have some predefined "templates" for what expectations should look like and we only use default ones when the output length does not match those expected ones. For example if the function returns a proper Panic or Error, it will match the parameter format returned by ContractABIUtils::failureParameters() and we'll use that. To trigger it you need a weird example like the above, where something that looks like a Panic is returned (same selector), but the length of revert data is different than it would be in an actual Panic. Still, it happens often enough to be noticeable. I ran into this quite a few times in the past, never knowing why it's happening.

[ekpyron]

I trust you tested this locally :-)? Then I'm fine with merging - nice to finally get this fixed, it doesn't happen that often, but if it did, it was pretty annoying.

[cameel]

Yeah, I tested it manually on these spinning cases I mentioned and a few other that do not have this behavior. Also used it to update expectations on my branch with changes to SolidityExecutionFramework (lots of updates there). Still, it's tricky to test exhaustively since it only kicks in on tests where expectations do not match.

It seems to me that it's working as expected now, but in the end, we'll only see in practice :) At least the potential for damage is limited since it's only isoltest (so won't affect CI) and only in that corner case where you have final bytes not taking up a whole slot.

[cameel]

Just noticed that I forgot to squash a fixup. Oh well... It's still on top so I could technically fix it, but that seems like too much hassle with the settings we have now :)