-
Notifications
You must be signed in to change notification settings - Fork 342
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
Rewrite MachOCASWriter to be a derived class of MachObjectWriter #9086
Conversation
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.
@swift-ci please test llvm |
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 |
There was a problem hiding this 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!
0f3ea61
to
849db0f
Compare
There was a problem hiding this 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.
849db0f
to
0259ef4
Compare
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.
0259ef4
to
895cd5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thanks
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