nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land.
Thanks for the patch! ================ Comment at: clang/test/Sema/warn-fortify-source.c:100-102 + __builtin_snprintf(buf, 2, "%#x", n); + __builtin_snprintf(buf, 2, "%#X", n); + __builtin_snprintf(buf, 2, "%#o", n); ---------------- hazohelet wrote: > nickdesaulniers wrote: > > Note that GCC -Wformat-truncation can warn on some of these. > > > > https://godbolt.org/z/jE3axWe1W > > > > Looks like the diagnostic keeps an up and lower bounds on the estimated > > format string expansion. > > > > Trunk for Clang also warns for these, so is this change a regression? Or > > are both GCC and Clang (trunk) incorrect? > Clang trunk is saying something factually incorrect because it says the > output `will always be truncated`, when in fact `__builtin_snprintf(buf, 2, > "%#x", n);` doesn't trigger truncation if `n` is zero. > > GCC is correct but is more conservative than clang's `ALWAYS be truncated` > diagnostics. > GCC's warning message is `... directive output MAY BE truncated` . > GCC doesn't warn on it when `n` is known to be zero. > (https://godbolt.org/z/E51a3Pfhr) > > GCC's behavior makes sense here because the truncation does happen whenever > `n` is not zero. If the user knows `n` is zero then they have no reason to > use `%#x` specifier. > So, I think it makes sense to assume `n` is not zero and emit diagnostics, > but it definitely needs diagnostics rewording like `is likely to be > truncated`. I see; thanks for the explanation. Sorry for the code review delay; I missed this comment in my inbox. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159138/new/ https://reviews.llvm.org/D159138 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
