hazohelet added inline comments.

================
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);
----------------
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`.


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

Reply via email to