Open Cookiee235 opened 2 months ago
Looks like a bug in your layer_norm
implementation. The rxplaceholder_red_temp_v0
and rxplaceholder_red_temp_v1
both have shape [64,64]
, but are being accessed at indices [0:1, 0:512]
. Are these buffers intended to have shape [1,512]
?
@Lunderberg Indeed, the indices are out of the boundary. However, Segmentation fault
is a dangerous behavior and is often considered a vulnerability. Do we need an isolation mechanism to check the validity in the tir
level?
True. There is a very old mechanism that uses the "tir.instrument_bound_checkers"
config option to add buffer bounds, but the code path for it is only used for tir functions produced from TE schedules. The annotations it provides haven't been useful since #9727 a few years ago, as all BufferLoad
and BufferStore
instances have have sufficient information to do bounds-checking anyways. There's a newer tir::transform::OOBChecker()
that does a better bounds checking, introduced in #12352, but it doesn't seem to be used anywhere.
I like the idea of having this check applied by default. Placing it either at the beginning or end of the TIR lowering pipeline would have caught this error during compilation.
@Lunderberg Thanks! The tvm.tir.analysis.OOBChecker()
is very useful. I like it! It successfully identifies the OOB issue and avoids the segfault. But It was disabled by default. Why not enable it by default?
Overall, no significant reasons not to. There may be some initial failures in cases where the buffer size is unknown, and which use a shape of [1]
as a fallback, but those probably should be fixed anyways.
Actual behavior
Steps to reproduce
CC @Lunderberg @vinx13