efriedma added inline comments.
================ Comment at: clang/lib/AST/ExprConstant.cpp:11740 + LVal.getLValueBase().dyn_cast<const ValueDecl *>()); + Result += VD->getFlexibleArrayInitChars(Info.Ctx); + } ---------------- nickdesaulniers wrote: > efriedma wrote: > > nickdesaulniers wrote: > > > nickdesaulniers wrote: > > > > erichkeane wrote: > > > > > nickdesaulniers wrote: > > > > > > erichkeane wrote: > > > > > > > Isn't this a possible null-deref? > > > > > > I don't think so; in fact, I can use `cast` and `get` rather than > > > > > > `dyn_cast_or_null` and `dyn_cast` here. > > > > > > > > > > > > Just because we have a pointer doesn't mean it's possibly > > > > > > `nullptr`; I don't think we can reach this code patch for > > > > > > evaluating the `__builtin_object_size` of a struct with a flexible > > > > > > array member if the LValue doesn't have a corresponding VarDecl. > > > > > Of course that is a possibility. You shouldn't use dyn_cast right > > > > > before a dereference, this should definitely be using `cast`/`get`, > > > > > since they assert. > > > > > > > > > > What does the LValue have when the dynamic struct does not have an > > > > > initializer? Or just an empty one? > > > > s/code patch/code path/ > > > > What does the LValue have when the dynamic struct does not have an > > > > initializer? Or just an empty one? > > > > > > Tested: > > > ``` > > > struct foo { > > > int x; > > > int y[]; > > > }; > > > > > > static struct foo instance = { > > > .x = 3, > > > // .y = { 5, 10, 15, }, > > > }; > > > > > > unsigned long foo (void) { > > > return __builtin_object_size(&instance, 1); > > > } > > > ``` > > > With the following values for `.y` in `instance`: > > > 1. `.y = { 5, 10, 15 }`: `foo` returns `16`. > > > 2. `.y = {},`: `foo` returns `4`. > > > 3. <`.y` is not specified or explicitly initialized>: `foo` returns `4`. > > This might have been missed in the discussion here, but what actually > > ensures we have a VarDecl here? I guess most other LValueBases can't > > actually have a struct type, but a CompoundLiteralExpr can. (Looking > > briefly at the code, I can't find any other possibilities at the moment, > > but in any case, the assumption seems pretty fragile.) > I assume that the code works as such: > ``` > struct foo my_foo; > return __builtin_object_size(&foo, 1); > ``` > `foo` is a `VarDecl` otherwise how do you take the address? What other > expression could be passed to `__builtin_object_size` that isn't a reference > to a variable? `__builtin_object_size(&(struct S){}, 1)`? Not really a useful construct, but we don't reject it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150892/new/ https://reviews.llvm.org/D150892 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits