Qing Zhao <qing.z...@oracle.com> writes: > Hi, > > This is the 3rd version of the patch for fixing PR109071. > > Thanks a lot for San James's help to test the previous 2nd version of > the patch on a lot of packages in the wild and provide detailed analysis > and filed new bugs. (PR117179, PR117180, etc).
Thank you Qing! > > From his testing results, we all feel that this work will be a general > improvement in this area and will be very helpful to the end-users. > Absolutely. Both in terms of improving safety as the whole point of these warnings is, but also stopping users from being panicked. They sometimes believe the warnings imply miscompilation because they can't understand how they would happen otherwise. An earlier version of these patches already helped find a real bug in GNU wget. I will be testing these patches and reporting any issues I see (either in GCC or to projects). > In this patch, we try to provide more contexts for -Warray-bounds, > -Wstringop-* > warning messages due to code movements from various compiler transformations. > > Control this with a new option -fdiagnostics-details. > > Compared to the 2nd version: > > https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657150.html > > Which is limited to fix PR109071, there are the following major improvement > to the patch: > > 1. All the following current open PRs were identified as the duplications > of PR109071 and were studied and fixed by this 3rd version of the patch. > each testing case was added to the patch as a unit-test case: > > PR88771 > PR85788 > PR108770 > PR106762 > PR115274 > PR117179 > > 2. Change the name of the new option from -fdiagnostics-explain-harder > to -fdiagnostics-details; The new name sounds good to me. > > 3. Change the name of the new data structure from "copy_history" to > "move_history" due to the following reason: > > The key feature of the compiler transformation that might provide more > accurate information to the value-range analysis is: moving a statement > from the joint point of one specific condition to this condition's > TRUE or FALSE path. > > For example, threadjump and isolate-path transformation make duplicated > basic block "Ba'" of the original basic block "Ba" that on the joint > point of a condition "cond", and then move the two basic block "Ba'" > and "Ba" to the TRUE path and FALSE path of the condition "cond"; on the > otherhand, tree sink transformation just move some of the statements from > the joint point of a condition "cond" to one specific path of this > condition. > > So, the new data structure "move_history" will include the following > information: > A. the "condition" that triggers the code movement; > B. whether the code movement is on the true path of the "condition"; > C. the "compiler transformation" that triggers the code movement. > > 4. In addition to backward threadjump, this patch can handle more compiler > transformations: > A. forward threadjump; > B. isolate-path; > B. tree-sinking; > > 5. In addition to -Warray-bound, making -Wstringop-* work as well. stringop* are really the most notorious for this so this is very welcome. > > 6. Adding debugging mechanism to the new data structure “move_history”; > 7. Adding all the testing cases of the duplicated bugs as the testing cases. Can you tag each of those PRs in the ChangeLog so the commit hook updates those too? > 8. More detailed comments in the patch; > > bootstrapping and regression testing on both x86 and aarch64. > > Please let me know any comment and suggestion. > > Thanks. > > Qing > > Qing Zhao (2): > Provide more contexts for -Warray-bounds, -Wstringop-* warning > messages due to code movements from compiler transformation > [PR109071] > Add debugging for move history.
