mstorsjo added a comment.
In D105169#3069555 <https://reviews.llvm.org/D105169#3069555>, @aqjune wrote:
> I see, the crash is happening at the line because SimplifyCFG removes the
> `sti->index_entries` null pointer check (which seems valid to me).
> If `stl->index_entries` was null, it would lead to uses of uninitialized
> variables as function arguments, which is UB.
> SimplifyCFG relies on the information and removes the `stl->index_entries`
> null check.
Thanks! That explains it. Although the actual crash is due to yet another
removed condition:
int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit; // pos_min and
pos_max are undef
...
if (sti->index_entries) {
index = av_index_search_timestamp();
if (index >= 0) {
... (dereference sti->index_entries+index)
... (initialize pos_min and pos_max)
}
}
// If sti->index_entries was NULL, UB must happen at the call below because
undef is passed to ff_gen_search's noundef arg.
pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit,
ts_min, ts_max, flags, &ts, avif->read_timestamp);
Both of the nested if conditions are removed:
int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit;
...
if (true) { // Changed to true!
index = av_index_search_timestamp();
if (true) { // Also changed to true
... (dereference sti->index_entries+index) // This can crash with
index = -1
... (initialize pos_min and pos_max)
}
}
pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit,
ts_min, ts_max, flags, &ts, avif->read_timestamp);
> I think the solution is to initialize `pos_min` and `pos_max` to some value.
> If `sti->index_entries` is null, they are never used inside `ff_gen_search`
> anyway, so initializing it into any value (e.g. 0) will work.
Yes, any value should be fine (and I guess 0 is the easiest to optimize for the
compiler). Just as background - this is in a codebase that really tries to
avoid default variable initializations unless they're proven to be necessary.
But here they clearly are due to the UB rules of the language.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105169/new/
https://reviews.llvm.org/D105169
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
