Skip to content

3C introduces compile error into unsafe C-style cast to a typedef (regression from #690) #695

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

Open
mattmccutchen-cci opened this issue Sep 3, 2021 · 2 comments
Labels
cast regression The things which were working before are breaking now typedef

Comments

@mattmccutchen-cci
Copy link
Member

mattmccutchen-cci commented Sep 3, 2021

Consider the following code (which was based on a test I was writing for #651):

typedef int *T;
void bar(T x, T y) {
  x = 1;  // warning: incompatible integer to pointer conversion assigning to 'T' (aka 'int *') from 'int'
  y = (T)1;
}

Before #690, 3C makes no changes to this code. After #690, 3C converts the code to:

typedef _Ptr<int> T;
void bar(int *x : itype(T), int *y : itype(T)) {
  x = 1;  // warning: incompatible integer to pointer conversion assigning to 'T' (aka 'int *') from 'int'
  y = (T)1;  // error: expression has unknown bounds, cast to ptr<T> expects source to have bounds
}

which has a compile error. checkedc#1169 does not fix the problem; apparently it is with the C-style cast from int to T (i.e., _Ptr<int>), which is never a valid cast, not the assignment to y. I guess when 3C sees the unsafe cast from int to T, it needs to either constrain T to wild or expand T to int *, a bit like it does in the function parameters.

@mattmccutchen-cci mattmccutchen-cci added regression The things which were working before are breaking now typedef cast labels Sep 3, 2021
@john-h-kastner
Copy link
Collaborator

john-h-kastner commented Sep 3, 2021

It looks like the fundamental issue here is that the constraint variable allocated for a CastExpression doesn't record typedef information. This gives a better conversion for your example, but I haven't looked into it deeper.

diff --git a/clang/lib/3C/ConstraintResolver.cpp b/clang/lib/3C/ConstraintResolver.cpp
index 0964e27e6938..9477cc45defc 100644
--- a/clang/lib/3C/ConstraintResolver.cpp
+++ b/clang/lib/3C/ConstraintResolver.cpp
@@ -810,6 +810,7 @@ bool ConstraintResolver::isCastofGeneric(CastExpr *C) {
 // if the expression is inside a macro.
 PVConstraint *ConstraintResolver::getRewritablePVConstraint(Expr *E) {
   PVConstraint *P = new PVConstraint(E, Info, *Context);
+  Info.unifyIfTypedef(E->getType(), *Context, P);
   auto PSL = PersistentSourceLoc::mkPSL(E, *Context);
   Info.constrainWildIfMacro(P, E->getExprLoc(), &PSL);
   return P;

The new conversion is

typedef int *T;
void bar(T x : itype(_Ptr<int>), T y : itype(_Ptr<int>)) {
  x = 1;  // warning: incompatible integer to pointer conversion assigning to 'T' (aka 'int *') from 'int'
  y = (T)1;
}

It would be nice if a proper fix to this issue could modify the ConstraintVariable constructor so that unifyIfTypedef doesn't need to be called whenever one is constructed. Although, I'm entirely not entirely confident that every variable whose type is a TypedefType should be constrained as a typedef by 3C. For instance, if a variable has a typedef type, but that type is never written anywhere in the source code, then it doesn't need to constrained as a typedef. This might come up for internal constraint variables generated for implicit casts.

@mattmccutchen-cci
Copy link
Member Author

Did you mean "I'm not entirely confident"?

For instance, if a variable has a typedef type, but that type is never written anywhere in the source code, then it doesn't need to constrained as a typedef. This might come up for internal constraint variables generated for implicit casts.

That sounds like an issue similar to #618. Maybe we can implement one test for whether the type is written in the source code and use it for both purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cast regression The things which were working before are breaking now typedef
Projects
None yet
Development

No branches or pull requests

2 participants