[llvm-branch-commits] [compiler-rt] [asan] Record container poisoning in poison history (PR #195674)

[Thurston Dang via llvm-branch-commits]

thurstond wrote:

> > > > It may be unintuitive to users that the poison history size is being used up by container overflow annotations.
> > > 
> > > 
> > > Ring buffer usage is unclear for users unless you read carefully the code. Other than that it's just a parameter: more is better, but more RAM usage.
> > > > Additionally, it becomes very difficult or even impossible for users to set an adequate poison_history_size, because the history might get overwritten by container annotations.
> > > 
> > > 
> > > This is imaginary problem comparing to issue when container is poisoned and it's hard to guess when it happend.
> > > So it's clearly improvement in user experience.
> > 
> > 
> > It's a clear improvement for users who encounter container OOB. But for users who only care about manual poisoning, this is a regression.
> > Can we add a flag to control whether container annotations use poison history? That way, if the user encounters a manual poisoning crash, they can re-run it with poison history only for manual poisoning, without the RAM/CPU overhead of tracking container annotation poison history. (To treat both usages fairly, one could also make it configurable whether manual poisoning uses the poison history, but that's more work.)
> 
> We can add flag at any moment if future, removing it way harder without breaking users. This is "EXPERIMENTAL" feature, I don't expect any complains.
> 
> I don't know how you use this one, I just put 100000000 every time without thinking,
> 
> Also with standalone partial granule, which possible with both poisoning, not clear which one look and.
> 
> Also we have detect_container_overflow and allow_user_poisoning which can be used in rare cases when resource usage makes a difference.

SGTM

https://github.com/llvm/llvm-project/pull/195674