nickdesaulniers added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:11740
+ LVal.getLValueBase().dyn_cast<const ValueDecl *>());
+ Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+ }
----------------
efriedma wrote:
> 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.
https://reviews.llvm.org/D151148
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
