On 08/22/2018 10:28 AM, Martin Sebor wrote: > On 08/22/2018 08:41 AM, Bernd Edlinger wrote: >> Hi! >> >> >> This patch adds some more checks to c_getstr to fix PR middle-end/87053 >> wrong code bug. >> >> Unfortunately this patch alone is not sufficient to fix the problem, >> but also the patch for PR 86714 that hardens c_getstr is necessary >> to prevent the wrong folding. >> >> >> Bootstrapped and reg-tested on top of my PR 86711/86714 patch. >> Is it OK for trunk? > > This case is also the subject of the patch I submitted back in > July for 86711/86714 and 86552. With it, GCC avoid folding > the strlen call early and warns for the missing nul: > > warning: ‘__builtin_strlen’ argument missing terminating nul > [-Wstringop-overflow=] > if (__builtin_strlen (u.z) != 7) > ^~~~~~~~~~~~~~~~ > > The patch doesn't doesn't prevent all such strings from being > folded and it eventually lets fold_builtin_strlen() do its thing: > > /* To avoid warning multiple times about unterminated > arrays only warn if its length has been determined > and is being folded to a constant. */ > if (nonstr) > warn_string_no_nul (loc, NULL_TREE, fndecl, nonstr); > > return fold_convert_loc (loc, type, len); > > Handling this case is a matter of avoiding the folding here as > well and moving the warning later. > > Since my patch is still in the review queue and does much more > than just prevent folding of non-nul terminated arrays it should > be reviewed first. I think we need to address 86711/86714 first. However, neither approach to 86711/86714 (Bernd's or yours) is sufficient alone to fix this bug. This patch can be layered on top of either approach to 86711/86714 to fix 87053 (I've actually tested that).
So let's table this, hopefully for just a day or so. It's getting late and I've got more tests to run on the 86714/86711 patches for comparison purposes and some loose ends I want to look at in Martin's patch. I'd hoped to be finished already, but wasn't able to move things as fast as I hoped. jeff
