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

Umbrella issue for rare low-priority rewriting bugs #659

Open
mattmccutchen-cci opened this issue Jul 29, 2021 · 0 comments
Open

Umbrella issue for rare low-priority rewriting bugs #659

mattmccutchen-cci opened this issue Jul 29, 2021 · 0 comments

Comments

@mattmccutchen-cci
Copy link
Member

mattmccutchen-cci commented Jul 29, 2021

As suggested by John et al., I'm starting an issue where we can collect 3C rewriting bugs that are rare and low-priority without cluttering the issue tracker as much as individual GitHub issues. Having one GitHub "issue" for many issues is awkward and we considered alternatives such as lightweight cards on a GitHub project board, but we decided it was more important to have the information searchable on the issue tracker where any users who care would be likely to look for it. Let's use letters for the sub-issues: #659(a) is more readable than something like #659(1).

(a) Multi-decls containing FunctionDecls

While doing research related to #657, I learned that a multi-decl in C can contain FunctionDecls, or even a mix of FunctionDecls and VarDecls. None of the 3C code is prepared to handle a FunctionDecl as part of a multi-decl. This can lead to behavior similar to #651 where one rewrite overwrites another. Some examples:

int *f1(void), *f2(void);
int *f2(void) { return 0; }

int *g1(void), *g2(void);
int *g1(void) { return 0; }

int *x1, *h2(void);
int *h2(void) { return 0; }

int *u1(void), *u2(void), *u3;

void foo(void) {
  int *xx1, *hh2(void), *xx3;
}

int *hh2(void) { return 0; }

Output:

_Ptr<int> f2(void);  // Lost f1
_Ptr<int> f2(void) { return 0; }

_Ptr<int> g1(void), *g2(void);  // Return type of g2 became a double pointer
_Ptr<int> g1(void) { return 0; }

_Ptr<int> h2(void);  // Lost x1
_Ptr<int> h2(void) { return 0; }

_Ptr<int> u3 = ((void *)0);  // Lost u1 and u2

void foo(void) {
  _Ptr<in;  // What???
int *hh2();
_Ptr<int> xx3 = ((void *)0);

}

_Ptr<int> hh2(void) { return 0; }

Currently, rewriteMultiDecl can even be called with a list of rewrites that includes a FunctionDecl if the FunctionDecl was in a DeclStmt (as in the foo example), but not for a global multi-decl, since that code path only considers VarDecls. (#657 is likely to change this situation in some way.) If the foo example is modified as follows:

void foo(void) {
  int *xx1, *hh2(void), *xx3 = (int *)1;
}

int *hh2(void) { return 0; }

then in DeclRewriter::rewriteDecls, the xx1 rewrite never gets inserted into RewriteThese. This appears to be because the hh2 rewrite gets inserted first and the two rewrites incorrectly compare equal under DComp because the Statement field of a FunctionDeclReplacement is never filled in even when the FunctionDecl is part of a DeclStmt. In fact, DComp may not be a valid strict weak order in this case, so the behavior of the std::set might technically be undefined. Stepping back, the DComp implementation looks a bit complex and possibly fragile in general, and maybe I should consider trying to clean it up as part of #657.

If we wanted to handle multi-decls containing FunctionDecls correctly, we'd need to make sure that all of 3C's special handling for FunctionDecls (such as the ability to rewrite only the parameters or only the return type) works correctly in the context of rewriteMultiDecl. I don't have a good sense of how much work that is likely to be.

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

No branches or pull requests

1 participant