rapidsna wrote:
> > > Or just convert `counted_by` into `sized_by` with no need for another
> > > spelling (i.e. I could nudge this patch slightly so that
> > > `CountAttributedType::SizedBy` gets set earlier in Sema and then `
> > > CodeGenFunction::emitCountedByPointerSize` would need no changes, etc.)
> >
> >
> > I.e. the only logic change (i.e. keep all the tests) needed would be this:
> > ```diff
> > diff --git a/clang/lib/Sema/SemaDeclAttr.cpp
> > b/clang/lib/Sema/SemaDeclAttr.cpp
> > index 964a2a791e18..c97ec9cea0c6 100644
> > --- a/clang/lib/Sema/SemaDeclAttr.cpp
> > +++ b/clang/lib/Sema/SemaDeclAttr.cpp
> > @@ -6577,6 +6577,20 @@ static void handleCountedByAttrField(Sema &S, Decl
> > *D, const ParsedAttr &AL) {
> > llvm_unreachable("unexpected counted_by family attribute");
> > }
> >
> > + // In GNU mode, silently convert counted_by/counted_by_or_null on void*
> > + // to sized_by/sized_by_or_null, since void has size 1 in GNU pointer
> > + // arithmetic.
> > + if (!CountInBytes && FD->getType()->isPointerType()) {
> > + QualType PointeeTy = FD->getType()->getPointeeType();
> > + if (PointeeTy->isVoidType() && S.getLangOpts().GNUMode) {
> > + // Emit diagnostic before conversion
> > + S.Diag(FD->getBeginLoc(), diag::ext_gnu_counted_by_void_ptr)
> > + << (OrNull ? CountAttributedType::CountedByOrNull
> > + : CountAttributedType::CountedBy);
> > + CountInBytes = true; // Convert to sized_by variant
> > + }
> > + }
> > +
> > if (S.CheckCountedByAttrOnField(FD, CountExpr, CountInBytes, OrNull))
> > return;
> >
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > This is, perhaps, much cleaner?
>
> That seems a lot easier to maintain! You may have to live with diagnostics
> saying `__sized_by` despite the code saying `__counted_by` though.
Yes, this would make things easier, as we wouldn't need to update the rest of
our code that currently assumes __counted_by isn't used with void *.
However, even if we do that, we should still preserve the original attribute
kind so the AST can recover the attribute as originally written (e.g., enabling
TypePrinter to print it as counted_by rather than sized_by). This aligns with
Clang AST's principle of "faithfulness"—the AST should faithfully represent the
original source code so that generating source code from the AST can reproduce
the original source
https://clang.llvm.org/docs/InternalsManual.html#faithfulness. Maintaining the
original attribute kind will also address the mismatched diagnostic issue.
That's some work, and keeping two attribute kinds might be error-prone, so it
might actually be simpler to just keep the attribute as is and update some of
our code relying on "counted_by != void *" assumption for -fbounds-safety
instead. The rest of Clang already has code assuming the stride of void * is 1
byte, so this would be consistent with existing behavior.
https://github.com/llvm/llvm-project/pull/164737
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
