C++/CLI x86 marshalling misses copy ctor/dtor calls resulting in C++ STL iterator debugging breaking #100033

Activity

1. ghost added

   needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area ownersAn area label is needed to ensure this gets routed to the appropriate area owners

   on Mar 20, 2024
2. dotnet-policy-service

   added

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on Mar 20, 2024
3. AaronRobinsonMSFT

   added

   area-Interop-coreclr

   and removed

   needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area ownersAn area label is needed to ensure this gets routed to the appropriate area owners

   on Mar 20, 2024
4. jkoritzinsky commented on Mar 20, 2024

   

   jkoritzinsky

   on Mar 20, 2024

   Member

   More actions

   For reference, this behavior has been since .NET Core 3.1 since the introduction of C++/CLI support: dotnet/coreclr#22805

   👍React with 👍1Reacted by Aaron R Robinson
5. jkoritzinsky commented on Mar 20, 2024

   

   jkoritzinsky

   on Mar 20, 2024

   Member

   More actions

   As we now know, there is a breaking change in dotnet/coreclr#22805. This can be observed with a type that escapes the this pointer (or a pointer relative to the this pointer) in a constructor/copy constructor and expects to find it again in the destructor. Without escaping this or storing a pointer that is relative to this in a C++ struct passed between managed and unmanaged by value, the breaking change is not observable.

   The change was caused by a removal of machinery that injected additional copy-constructor/destructor calls in locations unrepresentable in IL (ie in the middle of a call instruction and in the unmanaged->managed stub).

   This is only observable in x86 (other platforms have a different calling convention), and the most likely scenario where this pops up (STL iterators) is only observable with _ITERATOR_DEBUG_LEVEL=2 when passed an iterator (or a struct that has an iterator field).

   Since removing this logic, we have removed a significant amount of the machinery that supported these features, to the point that restoring the prior machinery (especially in the unmanaged->managed scenario) is not possible. We would need to implement a new approach integrated into the JIT to add support for this scenario.

   Below is a minimal repro of the code-patterns that can observe the breaking change:

   #pragma unmanaged
   #include <vector>
   #include <iostream>
   
   struct Relative;
   
   std::vector<Relative*> relatives;
   
   struct Relative
   {
       void* relative;
       Relative()
       {
           std::cout << "Registering " << std::hex << this << "\n";
           relatives.push_back(this); // Escaping this
           relative = this - 1; // Relative pointer stored in object
       }
   
       Relative(const Relative& other)
       {
           std::cout << "Registering copy of " << std::hex << &other << " at " << this << "\n";
           relatives.push_back(this); // Escaping this
           relative = this - 1; // Relative pointer stored in object
       }
   
       ~Relative()
       {
           auto location = std::find(relatives.begin(), relatives.end(), this); // Find escaped this
           if (location != relatives.end())
           {
               std::cout << "Unregistering " << std::hex << this << "\n";
               relatives.erase(location);
           }
           else
           {
               std::cout << "Error: Relative object " << std::hex << this << " not registered\n";
           }
   
           if (relative != this - 1) // Checking relative pointer
           {
               std::cout << " Error: Relative object " << std::hex << this << " has invalid relative pointer " << std::hex << relative << "\n";
           }
       }
   };
   
   void UseRelative(Relative rel)
   {
       std::cout << "Unmanaged: Using relative at address " << std::hex << &rel << "\n";
   }
   
   void UseRelativeManaged(Relative rel);
   
   void CallRelative()
   {
       Relative rel;
       UseRelativeManaged(rel);
   }
   
   #pragma managed
   
   extern "C" void __declspec(dllexport) Run()
   {
       // Managed to unmanaged
       {
           Relative rel;
           UseRelative(rel);
       }
   
       // Unmanaged to managed
       CallRelative();
   }
   
   void UseRelativeManaged(Relative rel)
   {
       std::cout << "Managed: Using relative at address " << std::hex << &rel << "\n";
   }
   

   Output:

   Running test
   Registering 0031FCC8
   Registering copy of 0031FCC8 at 0031FCCC
   Registering copy of 0031FCCC at 0031FCA0
   Unregistering 0031FCCC
   Unmanaged: Using relative at address 0031FC70
   Error: Relative object 0031FC70 not registered
    Error: Relative object 0031FC70 has invalid relative pointer 0031FC9C
   Unregistering 0031FCC8
   Registering 0031FC64
   Registering copy of 0031FC64 at 0031FC54
   Managed: Using relative at address 0031FC40
   Error: Relative object 0031FC40 not registered
    Error: Relative object 0031FC40 has invalid relative pointer 0031FC50
   Unregistering 0031FC64
   

   I'm looking into costing what a new approach to fixing this would be.
6. AaronRobinsonMSFT

   closed this as completedin #100050on Mar 24, 2024
7. dotnet-policy-service

   removed

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on Mar 24, 2024
8. github-actions

   locked and limited conversation to collaborators on Apr 24, 2024
9. JulieLeeMSFT

   reopened this on Aug 14, 2024
10. dotnet-policy-service

    added

    untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

    on Aug 14, 2024
11. jkoritzinsky commented on Aug 14, 2024

    

    jkoritzinsky

    on Aug 14, 2024

    Member

    More actions

    This is still fixed as per #100050
12. jkoritzinsky

    closed this as completedon Aug 14, 2024
13. dotnet-policy-service

    removed

    untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

    on Aug 14, 2024