Skip to content

Allow the CustomAttribute metadata table to be updated by ApplyChanges, with correct ordering #53066

@davidwengier

Description

@davidwengier

There is a bug in Roslyn, dotnet/roslyn#52816, which results in Edit and Continue/Hot Reload duplicating attributes when emitting updates to a method/type/etc. Roslyn is currently adding rows to the CustomAttributes table with every delta, regardless of what was emitted in the original compilation.

To fix this I'm changing how we output the EncLog and EncMap tables, and from initial testing the runtime seems to be applying the updates correctly, updating existing CustomAttributes rows as necessary, and adding otherwise. Unfortunately the runtime expects the CustomAttributes table to be sorted by Parent, but does not currently maintain that sort order when applying updates.

From an investigation by @lambdageek:

For Mono:

We assume the CustomAttributes table is sorted on the Parent column (and do a binary search when looking for custom attributes). We assume the entries for a given parent are contiguous.
We could add a slow path and a separate lookaside table for any affected parents, but at the moment things will be broken.

For CoreCLR (please correct me if I’m wrong, this is just what I learned trying to do the Mono impl):

Some tables that are sorted by parent and assumed contiguous, have indirect Pointer variants. When updates are applied CoreCLR will actually convert from direct to indirect under some circumstances even if the indirect table wasn’t present in the baseline assembly in order to slot a new row for a parent in the right spot. (See CMiniMdRW::AddChildRowDirectForParent and its callers)

There is no indirect custom attributes table, however, as far as I know.

Before we can fix the Roslyn issue we probably need the runtime to be updated, otherwise it is unknown what things will break.

It's worth noting that this bug exists right now, and the CustomAttributes table will already be getting out of order with updates, but fixing the Roslyn bug would make it worse. Right now each update to the CustomAttributes table at least includes all of the attributes for the thing being updated in order and in a contiguous block. If a binary search stumbled across a block of rows it would at least behave consistently. Presumably this hasn't caused any real problems in the past because of the reflection caches, however those are now being cleared. Also in the past updates to attributes would be rude edits, but we're also changing that by adding a new runtime capability.

/cc @tmat @lambdageek @mikem8361 @tommcdon

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions