Skip to content

Rewrite MachOCASWriter to be a derived class of MachObjectWriter #9086

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

Merged
merged 2 commits into from
Aug 14, 2024

Conversation

rastogishubham
Copy link

This patch rewrites MachOCASWriter to be a derived class of MachObjectWriter which fixes a lot of the issues we see from the changes made to MCAssembler. This also fixes the issue of the MCMachoStreamer only having calls to the getWriter() function that returns a MachOCASWriter.

It also reverts all the changes made to MCObjectWriter for rdar://133264719 and rdar://133001453

A significant amount of changes were made upstream to MCAssembler and
MachObjectWriter that broke MCCAS. To fix them, we moved a bunch of code
from MachObjectWriter to MCObjectWriter as a band-aid fix, but those are
full of layering violations, this patch reverts those changes in favor
of a correct fix in the next commit which involves rewriting
MachOCASWriter to be a derived class if MachObjectWriter.
@rastogishubham
Copy link
Author

@swift-ci please test llvm

@cachemeifyoucan
Copy link

Can you check if the CAS-backend verify mode is still working? I have the vague memory that I have to make sure I have basically two separate ObjectWriter instances to make it working but I could be wrong.

@rastogishubham
Copy link
Author

Can you check if the CAS-backend verify mode is still working? I have the vague memory that I have to make sure I have basically two separate ObjectWriter instances to make it working but I could be wrong.

I tried to check the verify flag by building a clang with `env CCC_OVERRIDE_OPTIONS=+-Xclang +-fcas-backend +-Xclang +-fcas-path +-Xclang +/tmp/cas +-fcas-backend-mode=verify' and it worked

Copy link

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. That is much simpler. Thanks!

Copy link

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More comments about test.

This patch rewrites MachOCASWriter to be a derived class of
MachObjectWriter which fixes a lot of the issues we see from the changes
made to MCAssembler. This also fixes the issue of the MCMachoStreamer
only having calls to the getWriter() function that returns a
MachOCASWriter.
Copy link

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks

@rastogishubham rastogishubham merged commit 877bd71 into swiftlang:next Aug 14, 2024
@rastogishubham rastogishubham deleted the MachOCASWriterRewrite branch August 14, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants