Hi,
A status update on this patch: (Actually a good news!)
> On Jun 10, 2025, at 11:32, Qing Zhao <qing.z...@oracle.com> wrote:
>
>
>
>>>>
>>>
>>> It's difficult to do any meaningful pruning I think. Consider
>>>
>>> if (i == -1)
>>> tem = a[i];
>>>
>>> when we transform this to
>>>
>>> if (i == -1)
>>> tem = a[-1];
>>>
>>> and report the out-of-bounds access then we've lost the fact
>>> that -1 was originally 'i' and there's an interesting condition on that
>>> variable. So unless we preserve some kind of optimization history
>>> on a stmt like the above we are lost. The same is true for
>>> "altered" control edges where the most interesting are again those
>>> that are elided by optimization (the implied true/false conditions).
>>>
>>> That said, I would expect the user to not use -fdiagnostics-show-context by
>>> default but instead when being puzzled about a diagnostic, turn it on
>>> on-demand. So it's probably useful to report all of the chain or maybe
>>> have -fdiagnostic-show-context=N with N limiting the depth (or a separate
>>> knob for this).
>>>
>>> When there's still 'i' in a[i] but we figured out a range then we can see
>>> to do 1. to gather conditions that we consider. The backwards threader
>>> has "interestingness" heuristics that could eventually be repurposed.
>>> The "good" thing is that once we decide to emit a diagnostic it's fair
>>> to spend quite some cycles on improving it.
>>
>> So as a summary I'd say we'd want to have -fdiagnostic-show-context
>> (=1 for now) and simply display the immediate control condition for
>> the select diagnostics your patch series handled and start from there,
>> adding test coverage where that's useful (like for my simple a[i] example
>> above) and see in what cases that's lacking.
>
> Thanks for the summary, I agree.
>
> The following is my plan:
>
> 1. The first working version of -fdiagnostic-show-context=N should cover all
> the current
> known testing cases: all the PRs and your testing case and Kees’s new testing
> case.
I tried the following very simple heuristic to form the diagnostic path in my
current working-in-progress patch:
/* Starting from cur_bb, backtrace the CFG through the single predecessor
to form the path. For example:
B2
/ \
V \
B3 \
/ \ \
V \---->V
B4 B7
In the above CFG, if the STMT is in B4, we backtrace to its single
predecessor B3 first, and then back to B3's single predecessor B2,
till there is no single predecessor anymore, or the # of backtrace
depth exceeds the value of flag_diagnostics_show_context.
For each single predecessor block, locate the conditional statement
in the end of the block. determine whether the STMT is on the taken
path of the condition. Add these two information to each event of
the path. */
The good news is: With the above simple heuristic and a simple back tracing of
the CFG, all the
current testing cases for the following PRs passed without any issue:
PR109071
PR88771
PR85788
PR108770
PR106762
PR115274
PR117179
I will continue working on this to add more testing cases and also more
warnings per the following discussion:
https://gcc.gnu.org/pipermail/gcc-patches/2025-May/684126.html
https://gcc.gnu.org/pipermail/gcc-patches/2025-June/686323.html
https://gcc.gnu.org/pipermail/gcc-patches/2025-June/685852.html
https://gcc.gnu.org/pipermail/gcc-patches/2025-June/685854.html
Let me know if you have any comments or suggestions.
Thanks a lot for all the help.
Qing
>
> 2. Then we can improve it later if new uncovered testing case is reported.
>
> Is this reasonable?
>
> Qing
>>
>> Richard.
>>
>>>>>
>>>>>>
