aaron.ballman added inline comments.

================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:168-169
+  static Matcher matcher() {
+    // FIXME: What if the index is integer literal 0? Should this be
+    // a safe gadget in this case?
+    return stmt(
----------------
xazax.hun wrote:
> As per some of the discussions, in the future the compiler might be able to 
> recognize certain safe patterns, e.g., when there is a simple for loop with 
> known bounds, or when both the index and the array size is statically known.
> 
> I think here we need to make a very important design decision: Do we want the 
> gadgets to have the right "safety" category when it is created (e.g., we have 
> to be able to decide if a gadget is safe or not using matchers), or do we 
> want some mechanisms to be able to promote an unsafe gadget to be a safe one? 
> (E.g., do we want to be able to prove some unsafe gadgets safe using dataflow 
> analysis in a later pass?)
(FWIW, this is a great question and I really appreciate you asking it!)


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:16
+void testArraySubscripts(int *p, int **pp) {
+  foo(p[0],             // expected-warning{{unchecked operation on raw buffer 
in expression}}
+      pp[0][0],         // expected-warning2{{unchecked operation on raw 
buffer in expression}}
----------------
One test case I'd like to see is: `sizeof(p[0])` -- should code in an 
unevaluated context be warned?


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:43
+}
+
+void testArraySubscriptsWithAuto(int *p, int **pp) {
----------------
Can you also add tests for function declarations like:
```
void foo(int not_really_an_array[10]) { ... }

template <int N>
void bar(int (&actually_an_array)[N]) { ... }

// Using a dependent type but we know it's a pointer.
template <typename Ty>
void baz(Ty *ptr) { ... }

// Using a dependent type where we have no idea if it's a pointer.
template <typename Ty>
void quux(Ty ptr) { ... }
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137379/new/

https://reviews.llvm.org/D137379

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to