Inconsistency with bounds on NT arrays and NT array pointers

[mwhicks1]

I am wondering if there is an inconsistency in the way NT arrays and NT array pointers are handled. I think there is a off-by-one bug somewhere.

In particular, consider this code:

int main() {
  char s0 _Nt_checked[4] = "hello"; // disallowed; initializer too long
  char s1 _Nt_checked[5] = "hello"; // allowed, sizeof(s1) = 5
  char s2 _Nt_checked[6] = "hello"; // allowed, sizeof(s2) = 6
  _Nt_array_ptr<char> p1 : count(5) = s1; // disallowed; bounds too long
  _Nt_array_ptr<char> p2 : count(5) = s2; // allowed
}

String s1 is deemed acceptable with an initializer of 6 bytes, when considering the NULL terminator. That tells me I should interpret the declared size 5 to be one less than the real size. This would make sense assuming that the _Nt_checked annotation is indicating that one unmentioned byte is reserved for the NULL terminator.

But if that's true, then I don't understand why the assignment from s1 to p1 is rejected. We just established that s1 has bounds 5 not counting the null terminator, and that is the interpretation of p1 as well. This is why standard string library functions take _Nt_array_ptr<char> arguments with no bounds annotation (interpreted as a bounds of count(0)) -- the NULL terminator is not counted.

So it would seem to me that either the initialization of s1 is treated incorrectly (the size should include the null terminator) or the initialization of p1 is treated incorrectly. I'm not seeing a consistent way to understand them both to be correct.

[mwhicks1]

Let me add: I would really like sizeof(buf) and the count on the corresponding array pointer to match up. Otherwise, you end up with calls to library functions that have to subtract one, e.g., like fgets(...,buf,sizeof(buf)-1). You can see this happening in this port: checkedc/checkedc-tiny-bignum-c@d36ea0b

[dtarditi]

This looks like a bug: the compiler should issue an error for s1. Given that the declared array size, the compiler is checking that the last character in the declared array is null. It looks like there is off-by-one error in this commit: 8ee16f3.

It looks like there is an error in SemaDecl.cpp in Sema::ValidateNTCheckedType at line 12469 in our current sources. The line:

        if (*DeclaredArraySize < InitializerString->getLength()) {

should use <=. (when the declared array size is greater than the string initializer length, the remaining elements are filled with 0, so we don't need to check that case).

[mgrang]

Fixed in PR #942
Tests in PR microsoft/checkedc#429