Skip to content

[CodeGen] Port StackProtector to new pass manager #75334

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 1 commit into from
Jan 9, 2024

Conversation

paperchalice
Copy link
Contributor

The original StackProtector is both transform and analysis pass, break it into two passes now. getAnalysis<StackProtector>() could be now replaced by FAM.getResult<SSPLayoutAnalysis>(F) in new pass system.

Copy link

github-actions bot commented Dec 13, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@paperchalice paperchalice force-pushed the NPM/CodeGen/stack-protector branch 2 times, most recently from 2b10011 to 4b273ab Compare December 13, 2023 12:58
@paperchalice
Copy link
Contributor Author

Ping @arsenm

Comment on lines 112 to 118
unsigned SSPBufferSize = SSPLayoutInfo::DefaultSSPBufferSize;

// A prologue is generated.
bool HasPrologue = false;

// IR checking code is generated.
bool HasIRCheck = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

These fields look like they were moved to a new location, but not removed from the old one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replace these fields with new a member SSPLayoutInfo Info; is OK, just to want to modify the old code as little as possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't follow this. If you moved them the old references should be deleted? Changing less code isn't preferable to leaving dead code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merge them into one member LayoutInfo, but still need to keep the interface as an analysis pass.

@paperchalice paperchalice force-pushed the NPM/CodeGen/stack-protector branch from 4b273ab to af70d34 Compare January 9, 2024 08:03
@paperchalice paperchalice force-pushed the NPM/CodeGen/stack-protector branch from af70d34 to f67aa75 Compare January 9, 2024 10:11
@paperchalice paperchalice merged commit f9a1d15 into llvm:main Jan 9, 2024
@paperchalice paperchalice deleted the NPM/CodeGen/stack-protector branch January 20, 2024 05:58
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
The original `StackProtector` is both transform and analysis pass, break
it into two passes now. `getAnalysis<StackProtector>()` could be now
replaced by `FAM.getResult<SSPLayoutAnalysis>(F)` in new pass system.
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