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