aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: clang/lib/Sema/SemaType.cpp:8338
   // The attribute vector size must match -mrvv-vector-bits.
-  if (VecSize != VScale->first * MinElts * EltSize) {
+  unsigned ExpectedSize = VScale->first * MinElts * EltSize;
+  if (VecSize != ExpectedSize) {
----------------
craig.topper wrote:
> aaron.ballman wrote:
> > Random thought I didn't think to ask earlier: is it possible for this 
> > multiplication to overflow (if so, we should add test coverage to make sure 
> > we don't do bad things)?
> The largest value for -mrvv-vector-bits is 65536 which makes VScale->first 
> 65536/64 == 1024. MinElts and EltSize should be small enough that it won't 
> overflow.
> 
> I'm not sure if there's any protection if someone bypasses the driver with 
> `-Xclang -mvscale-min=` and uses a larger value.
> The largest value for -mrvv-vector-bits is 65536 which makes VScale->first 
> 65536/64 == 1024. MinElts and EltSize should be small enough that it won't 
> overflow.

Fantastic, thank you!

> I'm not sure if there's any protection if someone bypasses the driver with 
> -Xclang -mvscale-min= and uses a larger value.

IMO, using cc1 options is compiler YOLO-mode, so I don't mind if this has no 
protection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150926

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to