Improper conversion of `&e` to `_Ptr<...>` in some cases

[mwhicks1]

The porting tool improperly treats pointers created with & as _Ptr, but it should not do so in every case. In particular, if we have something like

int *x = (int *)5;
int *p = &(*x);

then the porting tool should not convert any pointers in this program, but it incorrectly converts p to _Ptr<int>. Likewise if we had

int *x = (int *)5;
int *p = &x[1];

then no conversion should take place. The observation is that the target expression of & may be unchecked, and if so it should prevent pointer from being checked.

[mwhicks1]

This is handled in ConstraintResolver.cpp in the following code:

      else if (UO->getOpcode() == UO_AddrOf) {
        if (T.empty()) { // doing &e where e is a non-pointer
          tmp.insert(PVConstraint::getPtrPVConstraint(Info.getConstraints()));
        } else {
          assert(T.size() == 1 && "AddrOf only for lvals; shouldn't have multiple PVconstraints");
          auto &CV = *T.begin();
          if (PVConstraint *PVC = dyn_cast<PVConstraint>(CV)) {
            tmp.insert(addAtom(PVC, CS.getPtr(), CS));
          } // no-op for FPs

You can see the issue is that it unconditionally considers the expression to have type PTR. The example above is coming from the first case, with comment doing &e where e is a non-pointer; in the failing example e is *x which seems to just be a non-pointer, but it should "remember where it came from," i.e., that x is WILD, and not PTR. I'm not sure the best way to do this, TBH ...

[mwhicks1]

Note: most recent commit https://github.com/plum-umd/checkedc-clang/commit/18ab275c90f7dcefbe53fb1311eabb947c83310c changed the code mentioned above, but the logic is roughly the same, it's just part of a switch statement now, rather than a (more convoluted) if/else

[john-h-kastner]

This is partially resolved by #99. the first example is handled correctly, but the same issue exists for the second.

[john-h-kastner]

This patch would fix the second example, but it causes problems in some regression tests, so I left it out of the PR.

diff --git a/clang/lib/CConv/ConstraintResolver.cpp b/clang/lib/CConv/ConstraintResolver.cpp
index ef15b2b5fdd..5bab2fd0e71 100644
--- a/clang/lib/CConv/ConstraintResolver.cpp
+++ b/clang/lib/CConv/ConstraintResolver.cpp
@@ -265,6 +265,10 @@ std::set<ConstraintVariable *> ConstraintResolver::getExprConstraintVars(
             return getExprConstraintVars(
                 LHSConstraints, SubUO->getSubExpr(), RvalCons,
                 SubUO->getSubExpr()->getType(), IsAssigned);
+          } else if (ArraySubscriptExpr *ASE = dyn_cast<ArraySubscriptExpr>(UOExpr)) {
+            return getExprConstraintVars(
+                LHSConstraints, ASE->getBase(), RvalCons,
+                ASE->getBase()->getType(), IsAssigned);
           } else {
             // TODO: There should be a case for ArraySubscriptExpr so that
             // &(a[0]) is the same is a.

[mwhicks1]

You can add it in comments on your next push. I'll include it when I merge in with my branch.