Merge from Microsoft 2021-01-27

[mattmccutchen-cci]

Merging from Microsoft in preparation for submitting an omnibus PR. (When this PR is ready, I will normal-merge it from the command line, not squash-merge it in the GitHub UI. I'm filing a PR so we can discuss the test failures; I may perform subsequent merges without filing a PR.)

It seems Microsoft made bounds checking stricter (checkedc#903) and this is causing three of our regression tests to fail. Is anyone familiar with what to do about this? It would probably take a while for me to figure it out on my own.

[john-h-kastner]

PR microsoft#973 has been merged into Microsoft's master and resolves one of the failing tests

[mattmccutchen-cci]

Thanks John for the update and for getting one of the failures resolved! I decided to recheck where we stand.

I'm starting to look into the remaining failures:

• manyprotos:

  int foo(_Array_ptr<int> x : count(y), int y);
  int bar(_Array_ptr<int> x : count(c), int c) { 
    return foo(x, 1) + 3;  // Checked C compile error
  }

  I think the Checked C compiler is right to reject our code. Without an assumption that c >= 1, there is no way to prove that the call foo(x, 1) is consistent with the signature of foo. @aaronjeline It looks like you added this test back in 1ebc2c1. Can you comment on what you were trying to test? Should we just change the call to be foo(x, c) or should we expect the error?
• liberal_itype_ptrptr: This is a weird one. After one 3C pass with -alltypes, the relevant part of the code reads:

  void itype_defined_ptrptr(int **p : itype(_Ptr<_Ptr<int>>)) _Checked { }
  void itype_defined_caller() {
    _Ptr<int *> e = ((void *)0);
    itype_defined_ptrptr(_Assume_bounds_cast<_Ptr<_Ptr<int>>>(e));
  }

  With my limited knowledge of Checked C, I'd think that should be fine because we are casting e to the exact itype of the parameter. But the Checked C compiler gives this error message:

  liberal_itypes_ptrptr.checked.c:146:24: error: it is not possible to prove argument meets declared bounds for 1st parameter
    itype_defined_ptrptr(_Assume_bounds_cast<_Ptr<_Ptr<int>>>(e));
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  liberal_itypes_ptrptr.checked.c:146:24: note: the expected argument bounds use the variable 'e' and there is no relational information involving 'e' and any of the expressions used by the inferred bounds
  liberal_itypes_ptrptr.checked.c:146:24: note: (expanded) expected argument bounds are 'bounds((_Array_ptr<_Ptr<int>>)_Assume_bounds_cast<_Ptr<_Ptr<int>>>e(count(1)), (_Array_ptr<_Ptr<int>>)_Assume_bounds_cast<_Ptr<_Ptr<int>>>e(count(1)) + 1)'
  liberal_itypes_ptrptr.checked.c:146:24: note: (expanded) inferred bounds are 'bounds((_Array_ptr<_Ptr<int>>)value of e, (_Array_ptr<_Ptr<int>>)value of e + 1)'
  

  It looks like the Checked C compiler is keeping the cast in the expression for the expected bounds but not the actual bounds, which leads to the mismatch and an error. Interestingly, if I change the definition of itype_defined_ptrptr to just void itype_defined_ptrptr(_Ptr<_Ptr<int>> p) _Checked { } rather than using an itype, then the error goes away. Or if I change the definition of itype_defined_caller to store the result of the cast in a local variable first:

  void itype_defined_caller() {
    _Ptr<int *> e = ((void *)0);
    _Ptr<_Ptr<int>> f = _Assume_bounds_cast<_Ptr<_Ptr<int>>>(e);
    itype_defined_ptrptr(f);
  }

  then the error also goes away. @john-h-kastner If you've already investigated this and learned anything, please let me know ASAP, otherwise I'll look into it a bit further and probably file an issue with Microsoft.

[john-h-kastner]

It's not clear to me when exactly CheckedC should emit a compile-time error for bounds rather than inserting runtime bounds checks, so I don't know for sure if the first test failure is legitimate. Since it's in an --alltypes test, this isn't too important for us. For this merge with Microsoft, I would be ok with either changing the function call to be foo(x, c) or passing the test -f3c-tool. I don't think the specifics of the function call are important to the test, so unless Aaron say otherwise, I think we should change the function call.

Another modification of the second test case that works is

void itype_defined_ptrptr(int **p : itype(_Ptr<_Ptr<int>>)) _Checked { }
void itype_defined_caller() {
  _Ptr<int *> e = ((void *)0);
  itype_defined_ptrptr((int**)e);
}

If we need to fix this in our code, changing the generated cast to (int**) should not be difficult. It does, however, seem wrong to me that CheckedC rejects the cast. Given that Microsoft has already fixed one issue in CheckedC that caused a failing test here, it's definitely worth asking them about this one.

[mattmccutchen-cci]

I've tentatively added the manyprotos fix to this PR pending Aaron's approval (update: Aaron approved), so the only remaining failure is liberal_itype_ptrptr. Hopefully that will be fixed in checkedc#974 and then I can merge this PR.

[mattmccutchen-cci]

Mike decided it was better to add an expected-error to the liberal_itype_ptrptr test than to wait longer for a fix from Microsoft, so I've done so and then rebased this PR.