[PATCH] D77374: Fix -fsanitize=array-bounds with comma operator

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:882-887
+  while (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+    if (!BO->isCommaOp())
+      break;
+    E = BO->getRHS();
+    E = E->IgnoreParens();
+  }
----------------
vitalybuka wrote:
> rsmith wrote:
> > If we're going to further extend what Clang considers to be a flexible array access, we should do so consistently across our warning machinery and our sanitizers. Perhaps we could start by unifying this function with `IsTailPaddedMemberArray` in `SemaChecking`?
> There is one place in external code which is blocking me from enabling this at Google.
> 
> How much work it's going to be? To me these functions looks very different.
If you don't want to do the refactoring, please at least update `Sema::CheckArrayAccess` to skip over commas when looking for a member access in `BaseExpr`. Testcase:

```
struct X { int a; int b[1]; } *p;
int n = (0, p->b)[3];
```

... currently warns and trips the array-bounds sanitizer; with this change it would still warn but not trip the sanitizer, which seems bad. (Though I suppose the opposite case is worse.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77374