hubert-reinterpretcast wrote:

> > > We need a real SourceLocation().
> > 
> > 
> > The approach from 
> > [vasu-the-sharma#1](https://github.com/vasu-the-sharma/llvm-project/pull/1) 
> > will get us real source locations (but does not cover all cases where 
> > `EmitAggregateCopy` is called).
> 
> For UBSAN we need to keep the location. It's supposed to be usable without 
> debug info. Without location it's hard to act on such reports.
> 
> However, how hard to pass it into EmitAggregateCopy? Maybe we can fork 
> EmitAggregateCopy -> EmitAggregateCopyCurrentOne + 
> EmitAggregateCopyWithSourceLocation and incrementally transition from first 
> to another?

The source location is not the only issue. As noted in 
https://github.com/llvm/llvm-project/pull/164548#discussion_r2529214150, if we 
use `EmitCheckedLValue` while we still have the source expression, we get 
better checking for array subscript operations.

Additionally, the approach in 
https://github.com/vasu-the-sharma/llvm-project/pull/1 also covers some cases 
where we don't (always) call `EmitAggregateCopy` such as in 
https://github.com/vasu-the-sharma/llvm-project/blob/bc7f979bcb205b82d43d99fd940889f3d8801ca4/clang/lib/CodeGen/CGExprAgg.cpp#L1331-L1341.

TL;DR: The issue is not necessarily with `EmitAggregateCopy` not generating 
UBSAN checks. The case can be made that the issue is that the `LValue` 
arguments did not come from calls to `EmitCheckedLValue`.

https://github.com/llvm/llvm-project/pull/164548
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to