Skip to content

[CodeGen][NPM] Port MachineSanitizerBinaryMetadata to NPM #130069

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

Conversation

optimisan
Copy link
Contributor

@optimisan optimisan commented Mar 6, 2025

Didn't find a test for this (but there are tests for the Function version of this pass)

@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_removeloadsintofakeuses_to_npm branch from e986593 to f06e7f4 Compare March 7, 2025 10:12
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_machinesanitizerbinarymetadata_to_npm branch from 003ff87 to 5c5dde0 Compare March 7, 2025 10:12
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_removeloadsintofakeuses_to_npm branch from f06e7f4 to ffd8728 Compare March 10, 2025 04:45
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_machinesanitizerbinarymetadata_to_npm branch from 5c5dde0 to ca3edf5 Compare March 10, 2025 04:45
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_removeloadsintofakeuses_to_npm branch from ffd8728 to 49cfcf2 Compare March 10, 2025 05:22
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_machinesanitizerbinarymetadata_to_npm branch from ca3edf5 to 2a60a24 Compare March 10, 2025 05:22
@optimisan optimisan marked this pull request as ready for review March 10, 2025 05:28
MachineFunctionAnalysisManager &MFAM) {
if (!MachineSanitizerBinaryMetadata().run(MF))
return PreservedAnalyses::all();
return getMachineFunctionPassPreservedAnalyses();
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it should preserve nearly everything, particularly CFG

Copy link
Contributor

Choose a reason for hiding this comment

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

This pass also violates the assumption that machine passes never modify the underlying IR

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed #130553

Comment on lines 65 to 69
return PreservedAnalyses::all();
return getMachineFunctionPassPreservedAnalyses();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return PreservedAnalyses::all();
return getMachineFunctionPassPreservedAnalyses();
return PreservedAnalyses::all();
return getMachineFunctionPassPreservedAnalyses();

@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_removeloadsintofakeuses_to_npm branch from 49cfcf2 to 10b5b57 Compare March 11, 2025 09:49
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_machinesanitizerbinarymetadata_to_npm branch from 2a60a24 to 17c20b1 Compare March 11, 2025 09:51
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_removeloadsintofakeuses_to_npm branch from 10b5b57 to edd9320 Compare April 14, 2025 06:13
Base automatically changed from users/optimisan/03-06-_codegen_npm_port_removeloadsintofakeuses_to_npm to main April 14, 2025 07:28
@optimisan optimisan force-pushed the users/optimisan/03-06-_codegen_npm_port_machinesanitizerbinarymetadata_to_npm branch from 17c20b1 to 0bc54f7 Compare April 14, 2025 11:42
@optimisan optimisan merged commit f133eae into main Apr 14, 2025
10 of 11 checks passed
@optimisan optimisan deleted the users/optimisan/03-06-_codegen_npm_port_machinesanitizerbinarymetadata_to_npm branch April 14, 2025 15:22
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
Didn't find a test for this (but there are tests for the `Function`
version of this pass)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants