> On Jun 30, 2025, at 07:33, Richard Biener <richard.guent...@gmail.com> wrote:
>
> On Fri, Jun 27, 2025 at 3:39 PM Qing Zhao <qing.z...@oracle.com> wrote:
>>
>> 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.
>
> Good. Then we are sure we stop at a dominator of B4.
>
>> 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
>
> Nice. An incremental improvement would be to instead of handling
> single predecessors, skip to the immediate dominator - this way
> uninteresting intermediate CFG is skipped. This might be confusing
> and the next "step" could be very far away, not sure how to best
> handle this. But it would handle intermediate conditional code, like
>
> if (i < 10)
> {
> if (dollar)
> printf ("dollar");
> else
> printf ("euro");
> printf ("%d", amout[i]);
> }
>
> where you'd not able to go up to if (i < 10).
Yes, immediate dominator should work for such cases.
> Maybe a simple heuristic
> on number of line to not skip too far works, or simply ignore this
> for now, or simply only use immediate dominators for the first
> condition to be printed?
I will experiment a little bit on this. Do you have any more complicate
testing cases? That will be very helpful.
Thanks a lot for all the help.
Qing
>
> Thanks,
> Richard.
>
>> 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.
>>>>
>>>>>>>
>>>>>>>>
>>
