Skip to content
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

Inconsistent behavior with str and &str[0] #1148

Closed
mwhicks1 opened this issue Aug 8, 2021 · 7 comments · Fixed by #1163
Closed

Inconsistent behavior with str and &str[0] #1148

mwhicks1 opened this issue Aug 8, 2021 · 7 comments · Fixed by #1163

Comments

@mwhicks1
Copy link
Contributor

mwhicks1 commented Aug 8, 2021

For this code

#include <stdio.h>
void foo(_Nt_array_ptr<char> str) {
  sprintf(str,"%s","hello");     // clang complains about incompatible type of str
  sprintf(&str[0],"%s","hello"); // no complaints
}

The stdio_checked.h file defines sprintf's first argument to be a char *. As such, passing str to it is rejected as incompatible (passing a checked type where an unchecked one was expected). But the second line is accepted, even though it is passing literally the same address to sprintf as the first. (The first line is accepted if you add an unsafe cast, e.g., sprintf((char *)str,"%s","hello");)

@kkjeer
Copy link
Contributor

kkjeer commented Aug 10, 2021

Based on section 2.5.1 of the Checked C spec, I believe that the compiler is exhibiting the expected behavior here.

2.5.1 Rules for the address-of operator
If an address-of operator (&) applied to an lvalue expression of type T, the following rules apply:

  1. If the operator occurs in a checked block, the address-of operator produces a value of type array_ptr<T>.
  2. Otherwise, the address-of operator produces a value of type T *.

Since the function foo is in an unchecked scope, and str[0] has type char, &str[0] has type char *.

If we create a dummy version of sprintf that can be called from a checked scope, clang emits an error when passing str and when passing &str[0]:

void fake_sprintf(char *s);

void foo(_Nt_array_ptr<char> str) _Checked {
  // Expected argument type is char * and type of str is _Nt_array_ptr<char>.
  fake_sprintf(str);     // clang complains about incompatible type of str.

  // Expected argument type is char * and type of &str[0] is _Array_ptr<char>.
  fake_sprintf(&str[0]); // clang complains about incompatible type of &str[0].
}

@mwhicks1
Copy link
Contributor Author

Thanks for the explanation. On reflection, I don't understand why rule 2.5.1 makes sense. Can you explain why being in a checked block changes the behavior of &e and the type of the e (as being checked or not) is not the most relevant factor?

I would posit that programmers will expect that in any circumstance &(*e) equals e and moreover that if e has type T, then &(*e) has type T too. Indeed, if we consider _Ptr<T> types and the dereference operator * specifically, that's what happens:

extern void foo(int *);
void bar(_Ptr<int> p) {
        foo(p);   // fails, since p has type _Ptr<int>
        foo(&*p); // fails since &*p has type _Ptr<int> too
}

Why does Checked C seem to do one thing for singleton pointers but something else for array pointers? For arrays, &str[0] is acting like an implicit cast to an unchecked type, which I think is pretty surprising. If you think it isn't, can you explain why?

@kkjeer
Copy link
Contributor

kkjeer commented Aug 11, 2021

If we pass &*str to sprintf, clang complains about incompatible types (the type of &*str is _Nt_array_ptr<char>):

void foo(_Nt_array_ptr<char> str) {
  sprintf(str,"%s","hello");     // clang complains about incompatible type of str (_Nt_array_ptr<char>)
  sprintf(&*str,"%s","hello");   // clang complains about incompatible type of &*str (_Nt_array_ptr<char>)
  sprintf(&str[0],"%s","hello"); // no complaints - type of &str[0] is char *
}

Starting at line 14044 of SemaExpr.cpp, there is logic for implementing C99 address-of rules: if an expression e has type T, then &*e also has type T. However, there does not seem to be corresponding logic for address-of array subscripts: we do not have a rule that if e has type T, then &e[idx] also has type T. I'm not sure yet whether this is a bug - I plan to check the C spec to see if this is a C99 rule similar to the rule for &*e.

@mwhicks1
Copy link
Contributor Author

OK sounds like we are on the same page. It would be interesting if this issue ultimately owes to lack of coverage in the C99 spec!

@mwhicks1
Copy link
Contributor Author

@kkjeer Any update on this?

@kkjeer
Copy link
Contributor

kkjeer commented Aug 18, 2021

@mwhicks1 sorry for the delay! I've created PRs in checkedc-clang and checkedc for this issue and am running tests on them now.

@mwhicks1
Copy link
Contributor Author

Great! Thanks for taking care of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants