Bad IBC data causing official build failures

[jkoritzinsky]

First occurrence: https://dev.azure.com/dnceng/internal/_build/results?buildId=548138&view=results

The commits that were included as part of that build: 08565e9, 33229ab, and 2d0ebf1

[Dotnet-GitSync-Bot]

I couldn't add an area label to this Issue.

Checkout this page to find out which area owner to ping, or please add exactly one area label to help train me in the future.

[BruceForstall]

System.NullReferenceException: Object reference not set to an instance of an object.
     at IBCRead.parseData(Byte[] data, Int32& pos) in /_/src/IbcMerge/ibcmerge.cs:line 5224
     at IBCRead.readResourceFromFile(String fileName) in /_/src/IbcMerge/ibcmerge.cs:line 8184
     at IBCRead.readPE() in /_/src/IbcMerge/ibcmerge.cs:line 8153
     at IBCRead.readIncrementalIBCFromFile(String fileName) in /_/src/IbcMerge/ibcmerge.cs:line 7111
     at IBCMerge.Merge(String[] args) in /_/src/IbcMerge/ibcmerge.cs:line 10954
     at IBCMerge.Main(String[] args) in /_/src/IbcMerge/ibcmerge.cs:line 10616

Seems like even bad IBC data shouldn't cause IBCMerge to crash. @davidwrighton @briansull

[davidwrighton]

Alright, so the issue appears to be that one of the input files to ibc merge doesn't actually have any Ibc data in it. This is the fairly terrible way in which ibcmerge detects that an input file that is supposed to have Ibc data does not. I'll put together the fix to adjust the IbcMerge tool to produce better errors in this case, but that won't fix this problem at all. The problem would actually be with the optimization data, which I believe is updated outside of the normal commit workflow. I'm not actually familiar with the part of the build which feeds that data into our build or how to diagnose it, but I believe @tmat is. (Additionally, I don't have access to a Mac machine, which is where I think these builds are failing.)

[tmat]

I am not familiar with how dotnet/runtime does it either. @brianrob ?

[brianrob]

This should (and will) happen via dependency flow, but until it does, it's a manual process. It's weird that yesterday is the first occurrence, as I manually updated the data and the change was merged last weekend. The build that contains the updated data succeeded (https://dev.azure.com/dnceng/internal/_build/results?buildId=541998&view=results) as did several others for the next few days.

I am suspicious that something else has happened here local to the build rather than due to the reference to the data being updated. A quick review of Versions.props shows that the optimization data references haven't changed since I modified them via 02409fb.

[davidwrighton]

Does anyone on this thread have access to a Mac machine that can run the build? From what you say, this should be reproduceable, and given a reproduceable build, or at least the set of binaries input to ibcmerge, it should be fairly straightforward for @briansull or I to diagnose the actual problem.

[brianrob]

Another thing worth noting - we have not historically applied IBC data to OSX builds. We only train on Windows and Linux. So, I'm not even sure which data is being used. That might actually be the problem. I wonder if as part of the repo unification this happened, and we've just been getting away with it.

[jkoritzinsky]

I get the same failure for a local release build since I have IBCMerge restored on my machine, but I only started seeing it a few days ago.

[jaredpar]

@jkoritzinsky can you please drive this to resolution? If this is impacting a single binary then we should consider a PR that simply excludes it from IBC for the time being (goal being to get official builds off the floor).

@davidwrighton have you filed a follow up issue to make the IBCMerge error better here?

[jkoritzinsky]

Will do

[jkoritzinsky]

Opened #33393 to disable IBCMerge on OSX. That should unblock us.

[BrennanConroy]

Make a PR to release/5.0-preview2 as well please

[davidwrighton]

@jaredpar The error message here has been improved as of version "5.0.7-beta.20159.1" of the IbcMerge tool. I've put together a PR into the arcade repo to update the version dotnet/arcade#5009

Do you know what needs to be done to verify the updated IbcMerge version?

[jaredpar]

@davidwrighton you should be able to verify by lookiing at the versions in the arcade NuPkg file that we download.

[jkoritzinsky]

Merged in the change for 5.0p2. Removing the blocking-release label.

[ViktorHofer]

@jkoritzinsky looks like IBC data is still disabled for OSX? Are you planning to re-enable its consumption?

runtime/eng/codeOptimization.targets

Lines 7 to 12 in 56b6964

	<!-- There's an issue causing IBCMerge failures because of mismatched MVIDs
	across many of our assemblies on Mac, so disable
	IBCMerge optimizations on Mac for now to unblock the offical build.
	See issue https://github.com/dotnet/runtime/issues/33303
	-->
	<IsEligibleForNgenOptimization Condition="'$(TargetOS)' == 'OSX' or '$(TargetsMobile)' == 'true'">false</IsEligibleForNgenOptimization>

[ghost]

Tagging subscribers to this area: @safern, @ViktorHofer
Notify danmosemsft if you want to be subscribed.

[ViktorHofer]

@jkoritzinsky assigning to you until we figured out if this needs to happen for 5.0.

[jkoritzinsky]

I've started a test official build to see if we can re-enable it: https://dev.azure.com/dnceng/internal/_build/results?buildId=741875&view=results

[jkoritzinsky]

This has started to fail again now that we've fixed IBC merging to actually happen.

[brianrob]

This may just be a matter of updating the data. As of today, the data does not automatically flow, but it does get produced and saved.

[karelz]

@jkoritzinsky is it needed for 5.0 servicing (5.0.x milestone), or for 6.0?

[jkoritzinsky]

This is 6.0

[jkoritzinsky]

The old IBC data pull has been deleted, so we can close this issue.