Skip to content
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

Add support for literal zero in FnPtrNull rewriter pass #438

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ayrtonm
Copy link
Contributor

@ayrtonm ayrtonm commented Oct 9, 2024

We previously assumed that initializing, assigning or comparing function pointers with NULL would not use an integer literal. @randomPoison hit this issue with zlib so this PR adds support for those cases and new tests. It also takes into account the cases with and without a cast on the zero. I've covered most of the new cases, except comparisons since those interact with AST matchers in FnPtrEq so I'm thinking about the best way to handle them.

This adds LIT checks for a few existing cases at the top and some new tests. The
two LIT checks at the bottom are valid code, but I'm not sure if that's the
simplest way to rewrite that.
@ayrtonm ayrtonm requested a review from randomPoison October 9, 2024 17:30
@ayrtonm
Copy link
Contributor Author

ayrtonm commented Oct 9, 2024

Got ast matchers for the null in the three cases in the broken wip commit

f = 0;
f = (typeof(f))0;
f = NULL;

but I'm trying to find how to get the right charSourceRange for the second case so rn it's rewritten as f = (typeof(f)) { 0 }typeof(f)) 0;.

After this it should be straightforward to copy the relevant matchers from FnPtrNull to FnPtrEq and use them to omit the IA2_ADDR in #411.

Comment on lines +84 to +85
// REWRITER: fn = (typeof(fn)) { NULL };
fn = NULL;
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 think this case is handled correctly if using a define other than NULL. In zlib they use a custom ZNULL define (still just 0), and when running the rewriter on this branch it seems like ZNULL gets replaced by 0. For example:

strm->zalloc = Z_NULL;

gets rewritten to

strm->zfree = (typeof(strm->zfree)) { 0 };

I'm not sure if that's something we can or care to fix, since it doesn't really have a functional difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The output is totatlly broken rn but that's a good catch about custom NULL macros. What I had in mind would've generated (typeof(f)) { NULL } which wouldn't be the best thing to do. None of this is developer-facing though so I might just have all cases use 0 instead of the corresponding macros.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants