Inserted `_Assume_bounds_cast` bounds should substitute function call arguments for parameters

[mwhicks1]

This code:

void baz(_Array_ptr<int> p : count(len), int len);
void foo(int *p, int n) {
  p = (int *)1;
  baz(p,n);
}

converts to

void baz(_Array_ptr<int> p : count(len), int len);
void foo(int *p : itype(_Array_ptr<int>) count(n), int n) {
  p = (int *)1;
  baz(_Assume_bounds_cast<_Array_ptr<int>>(p, count(len)),n);
}

The inserted _Assume_bounds_cast is what we want, but the count(len) is wrong -- it should be count(n).

[Machiry]

Not related to: #522

[mwhicks1]

What's happening here is that the call in foo to baz is looking at baz for the type, and just "copying" the type to the cast. This means that it will get the bounds from the declaration but won't translate that backwards to the expression used for the bounds, at the callsite. Not clear the easiest way to do that.

[john-h-kastner]

For reference, this code in CastPlacement.cpp is responsible.

checkedc-clang/clang/lib/3C/CastPlacement.cpp

Lines 150 to 160 in edf0ed7

	if (isa<ArrAtom>(CA) || isa<NTArrAtom>(CA)) {
	std::string Bounds = "";
	if (DstPVC->srcHasBounds())
	Bounds = DstPVC->getBoundsStr();
	else if (DstPVC->hasBoundsKey())
	Bounds = ABRewriter.getBoundsString(DstPVC, nullptr, true);
	if (Bounds.empty())
	Bounds = "byte_count(0)";
	Suffix = ", " + Bounds + ")";
	}

A trivial "fix" would be to not even try to get the correct bounds for the cast and always use byte_count(0), This would get around any issues with undefined variables in bounds, but will certainly cause other errors during bounds checking. Bounds checking errors are the type we're more willing to accept in alltypes, so maybe that's an alright trade off.

[mattmccutchen-cci]

A trivial "fix" would be to not even try to get the correct bounds for the cast and always use byte_count(0), This would get around any issues with undefined variables in bounds, but will certainly cause other errors during bounds checking. Bounds checking errors are the type we're more willing to accept in alltypes, so maybe that's an alright trade off.

This workaround sounds better than the status quo to me (except it might be worth trying to preserve constant bounds as you mentioned). If it's easy, could you stick the original bounds expression in a comment so we can see it during manual porting/testing?

We can keep this issue open for the full fix of substituting the arguments for the parameters.

[kyleheadley]

This is (most likely) also the cause of most of the non-bounds errors (27/34) in vsftpd conversion.
for example:

strlist.c:112:121: error (use_undecl): use of undeclared identifier 'num_elem'
    vsf_sysutil_qsort<struct mystr_list_node>(_Assume_bounds_cast<_Ptr<struct mystr_list_node>>(p_list->p_nodes,  count(num_elem)), p_list->list_len,

num_elem is the parameter name, copied here to use as count for an argument.