shafik added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3764-3767
if (!checkFunctionOrMethodParameterIndex(S, D, AL, 1, IdxExpr, Idx))
return;
-
+ if (Idx.getASTIndex() >= getFunctionOrMethodNumParams(D))
+ return;
----------------
tbaeder wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > Did you look into fixing this within
> > > `checkFunctionOrMethodParameterIndex()` instead? That way, all callers of
> > > the API get the correct behavior instead of having to find individual
> > > attributes to check the logic (I believe there are other attributes with
> > > the same underlying problem).
> > I would also expect any such issue to diagnose. Also, isn't there an
> > off-by-one error here in the case of variadic args?
> `handleFormatAttr()` has:
> ```
> if (Idx < 1 || Idx > NumArgs) {
> S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
> << AL << 2 << IdxExpr->getSourceRange();
> return;
> }
> ```
>
> which is also what I'd expect to see here.
>
>
I agree w/ @aaron.ballman here we should handle this in
`checkFunctionOrMethodParameterIndex` it is used in many places and AFAICT they
all have the same issue, none diagnose this case but paper it over.
There is definitely a bigger clean-up here but we can do a targeted fix now and
remove a crash bug and leave the larger fixing for a follow-up.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147037/new/
https://reviews.llvm.org/D147037
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
