Skip to content

Avoid object layout mismatch between coreclr and mscordaccore by comparing them during the build. #12770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cshung opened this issue May 29, 2019 · 6 comments
Labels
area-Diagnostics-coreclr backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity

Comments

@cshung
Copy link
Contributor

cshung commented May 29, 2019

No description provided.

@cshung cshung changed the title Avoid object layout mismatch between coreclr.dll and mscordaccore.dll by comparing them during the build. Avoid object layout mismatch between coreclr and mscordaccore by comparing them during the build. May 29, 2019
@sdmaclea
Copy link
Contributor

@cshung Since the dac is built using coreclr code how do you expect these to get disconnected? Is this catching a dacize error?

@cshung
Copy link
Contributor Author

cshung commented Oct 30, 2019

@sdmaclea Accidental #ifdef will cause failures like this.

struct BadStruct
{
#ifdef !DACCESS_COMPILE
    int x;
#endif
   int y;
};

If it turns out that the daccess code never access x but it access y, the cross-compilation would succeed, but the generated code will access x instead of y when we ask for y.

This is not theoretical, this is a post-mortem work item for the issue https://github.com/dotnet/corefx/issues/38071 fixed by dotnet/coreclr#24798.

Catching this type of issue upstream at build time would eliminate a source of the issue once and for all.

@jkotas
Copy link
Member

jkotas commented Oct 30, 2019

a source of the issue once and for all

You can have the same accidental ifdef in the algorithm that the layout check won't find.

We had this tool for desktop. The tool worked by comparing symbol information for coreclr.dll and cross-compiled mscordacwks.dll . It did catch a few issues, but it also generated a ton of false positives - for example, the symbols for C runtime have the same names, but different layout between platforms.

I think it would be better to spend energy on building universal DAC that is based on clearly defined and tested contracts. The universal DAC is actually going to fix this issue once and for all.

@cshung
Copy link
Contributor Author

cshung commented Oct 31, 2019

IMHO, it is even more important for us to have an (ideally - build time) validation that the memory layout the universal DAC will depend on matches its expectation.

Instead of using a tool that matches symbols blindly. We may consider the AsmOffsetsVerify approach. Having the dependencies spelled out manually can make it crystal clear what is being depended on, and it can also drive the number of dependencies low [for one have to work hard to author these offsets]

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@tommcdon tommcdon added the tracking This issue is tracking the completion of other related issues. label May 6, 2020
@tommcdon tommcdon modified the milestones: 5.0, Future May 6, 2020
@tommcdon tommcdon modified the milestones: Future, 6.0.0 Jul 8, 2020
@tommcdon tommcdon removed the tracking This issue is tracking the completion of other related issues. label Oct 14, 2020
@tommcdon tommcdon modified the milestones: 6.0.0, Future Oct 14, 2020
Copy link
Contributor

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@dotnet-policy-service dotnet-policy-service bot added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Apr 15, 2025
Copy link
Contributor

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@dotnet-policy-service dotnet-policy-service bot removed this from the Future milestone Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Diagnostics-coreclr backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity
Projects
None yet
Development

No branches or pull requests

5 participants