[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D105169#3069417 <https://reviews.llvm.org/D105169#3069417>, @mstorsjo wrote:

> Yes, I believe so. If the branch is not taken, `pos_min` and `pos_max` are undefined when entering `ff_gen_search`. (I would assume that their value isn't ever used within `ff_gen_search` in that case.) But regardless of that, in this case, the generated code crashes around this line, https://gist.github.com/aqjune/3bd0ea19bbc12b4744843c0c070e994c#file-ff_seek_frame_binary-c-L39, before entering `ff_gen_search` - and within that branch, those variables are properly set before they're used.

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.

  int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit; // pos_min and pos_max are undef
  ...
  if (sti->index_entries) {
     ... (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);

SimplifyCFG optimizes this to the code below:

  int64_t pos_min=pos_min, pos_max=pos_max, pos, pos_limit;
  ...
  if (true) { // Changed to true!
     ... (dereference sti->index_entries+index) // This can crash.
     ... (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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105169/new/

https://reviews.llvm.org/D105169