Skip to content

Commit 9826150

Browse files
authored
[TIR]: Fix VerifyStream::Verify causes dereferencing an invalid pointer (#18479)
This PR is trying to fix issues #17798. ### Root Cause The error message construction used `it->second` unconditionally, but the `Verify()` condition was: - `it == currently_defined_.end() || redefine_is_allowed` This means when the condition evaluates to `false` (triggering the error), it could be due to: 1. `it != end() && !redefine_is_allowed` => Safe to access `it->second` 2. `it == end() && !redefine_is_allowed` => Invalid to access `it->second` ### Solution The fix ensures safe iterator access by: 1. **Storing the Verify result**: Instead of chaining error messages directly to `Verify()`, store the result in a variable 2. **Conditional dereferencing**: Only access `it->second` when `it != end()` 3. **Meaningful error messages**: Provide appropriate messages for both cases --------- Co-authored-by: cchung100m <[email protected]>
1 parent 4be951d commit 9826150

File tree

2 files changed

+98
-10
lines changed

2 files changed

+98
-10
lines changed

src/tir/analysis/verify_well_formed.cc

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -248,20 +248,25 @@ class UndefinedVarVerifier : public Verifier<UndefinedVarVerifier> {
248248
bool redefine_is_allowed = redefine_allowed_within_function_.count(var);
249249
{
250250
auto it = currently_defined_.find(var);
251-
Verify(it == currently_defined_.end() || redefine_is_allowed)
252-
<< "ValueError: "
253-
<< "TIR is ill-formed, "
254-
<< "due to multiple nested definitions of variable " << var
255-
<< ". It was first defined at " << it->second << ", and was re-defined at " << path;
251+
auto verify = Verify(it == currently_defined_.end() || redefine_is_allowed);
252+
verify << "ValueError: "
253+
<< "TIR is ill-formed, "
254+
<< "due to multiple nested definitions of variable " << var << ".";
255+
if (it != currently_defined_.end()) {
256+
verify << " It was first defined at " << it->second << ", and was re-defined at " << path;
257+
}
256258
}
257259

258260
{
259261
auto it = previously_defined_.find(var);
260-
Verify(it == previously_defined_.end() || redefine_is_allowed)
261-
<< "ValueError: "
262-
<< "TIR is ill-formed, "
263-
<< "due to multiple definitions of variable " << var << ". It was first defined at "
264-
<< it->second << ", and was later re-defined at " << path;
262+
auto verify = Verify(it == previously_defined_.end() || redefine_is_allowed);
263+
verify << "ValueError: "
264+
<< "TIR is ill-formed, "
265+
<< "due to multiple definitions of variable " << var << ".";
266+
if (it != previously_defined_.end()) {
267+
verify << " It was first defined at " << it->second << ", and was later re-defined at "
268+
<< path;
269+
}
265270
}
266271

267272
currently_defined_.insert({var, path});

tests/python/tir-analysis/test_tir_analysis_verify_well_formed.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,5 +345,88 @@ def func():
345345
tvm.tir.analysis.verify_well_formed(mod)
346346

347347

348+
def test_error_message_without_previous_definition_location():
349+
"""Test case 1: Error message without 'It was first defined at'
350+
351+
This tests the scenario where it == end(), so the error message should contain
352+
'TIR is ill-formed, due to multiple definitions of variable' but should NOT
353+
contain 'It was first defined at' since the iterator is invalid.
354+
"""
355+
356+
@T.prim_func(check_well_formed=False)
357+
def func():
358+
x = T.int32()
359+
360+
with T.LetStmt(42, var=x):
361+
T.evaluate(x)
362+
363+
with T.LetStmt(99, var=x): # This should trigger the error
364+
T.evaluate(x)
365+
366+
with pytest.raises(ValueError) as exc_info:
367+
tvm.tir.analysis.verify_well_formed(func, assert_mode=True)
368+
369+
error_msg = str(exc_info.value)
370+
371+
assert "TIR is ill-formed" in error_msg
372+
assert "multiple definitions of variable" in error_msg
373+
374+
375+
def test_error_message_with_previous_definition_location():
376+
"""Test case 2: Error message with 'It was first defined at'
377+
378+
This tests the scenario where it != end(), so the error message should contain
379+
both 'TIR is ill-formed, due to multiple definitions of variable' and should also
380+
contain 'It was first defined at' with the location information.
381+
"""
382+
383+
@T.prim_func(check_well_formed=False)
384+
def func():
385+
x = T.int32()
386+
387+
with T.LetStmt(42, var=x):
388+
with T.LetStmt(99, var=x): # This should trigger the error
389+
T.evaluate(x)
390+
391+
with pytest.raises(ValueError) as exc_info:
392+
tvm.tir.analysis.verify_well_formed(func, assert_mode=True)
393+
394+
error_msg = str(exc_info.value)
395+
396+
assert "TIR is ill-formed" in error_msg
397+
assert "multiple nested definitions of variable" in error_msg
398+
399+
# should contains location information since it != end()
400+
assert "It was first defined at" in error_msg
401+
assert "was re-defined at" in error_msg
402+
403+
404+
def test_sequential_redefinition_with_location():
405+
"""Test case 2b: Sequential redefinition that includes location info
406+
407+
This tests the previously_defined_ path where it != end()
408+
"""
409+
410+
@T.prim_func(check_well_formed=False)
411+
def func():
412+
x = T.int32()
413+
414+
with T.LetStmt(1, var=x):
415+
T.evaluate(x)
416+
417+
with T.LetStmt(2, var=x): # This should trigger the error
418+
T.evaluate(x)
419+
420+
with pytest.raises(ValueError) as exc_info:
421+
tvm.tir.analysis.verify_well_formed(func, assert_mode=True)
422+
423+
error_msg = str(exc_info.value)
424+
425+
assert "TIR is ill-formed" in error_msg
426+
assert "multiple definitions of variable" in error_msg
427+
assert "It was first defined at" in error_msg
428+
assert "later re-defined at" in error_msg
429+
430+
348431
if __name__ == "__main__":
349432
tvm.testing.main()

0 commit comments

Comments
 (0)