Revert "[vm] fix confusing use of m_pCanonMT (NFC) (#102936)"

[lambdageek]

This reverts commit 1ab6888.

The change broke the DAC.

[dotnet-policy-service]

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

[lambdageek]

I don't completely understand why the DAC broke.

The scenario I saw was when TADDR canonicalMethodTable is 0.

The m_pEEClass / m_pCanonMT union is declared like this:

    union {
        DPTR(EEClass) m_pEEClass;
        TADDR m_pCanonMT;
    };

So on the DAC side it's either a smart pointer of an EEClass or a target address. But __DPtr<EEClass> has just a single TADDR m_addr member so casting back and forth ought to work...

Original code (works, apparently):

        return PTR_EEClass(PTR_MethodTable(canonicalMethodTable)->m_pCanonMT);

This gets the TADDR from __DPtr<MethodTable>(canonicalMethodTable) and instantiates a __DPtr<EEClass> from it.

If canonicalMethodTable is (TADDR)0, it seems like operator -> will call DacInstantiateTypeByAddress which returns host-side NULL, and then try to dereference the m_pCanonMT field from that null pointer. Seems like it ought to crash. But doesn't?

My code that we're reverting (doesn't work):

        return PTR_MethodTable(canonicalMethodTable)->m_pEEClass;

This gets a __DPtr<EEClass> from the __DPtr<MethodTable>(canonicalMethodTable) - again by calling operator -> and also trying to get a m_pEEClass member from a host-side null pointer.

[lambdageek]

I think the old code crashed too.

Here’s a self-contained repo with a lot of copy/pasted code for the __DPtr<> template

https://github.com/lambdageek/dac-nullptr

build with:

  cmake -S . -B out
  cmake --build out

both the old and new versions of the methodtable code segfault.

[jkotas]

both the old and new versions of the methodtable code segfault.

Right, it is what I would expect.

It is expected that this method may segfault for invalid MethodTable. AV = AccessViolation = segfault.

The change broke the DAC.

What is the manifestation of the break?

[lambdageek]

What is the manifestation of the break?

@mikem8361 has the details. Apparently some diagnostic test started segfaulting on Alpine roughly coinciding with this m_pCanonMT change landing. The scenario has MethodTable::GetClassWithPossibleAV() called and it gets into the branch with canonicalMethodTable == 0

[lambdageek]

Closed in favor of #104128