[PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

Martin Sebor msebor@gmail.com
Wed Oct 5 17:12:00 GMT 2016


On 10/05/2016 10:27 AM, Joseph Myers wrote:
> On Wed, 5 Oct 2016, Martin Sebor wrote:
>
>> The warning would only helpful if there was a target where wchar_t
>> didn't promote to a type with the same precision as wint_t, or in
>
> Or it could show up a logical error where the code should have had a
> wint_t variable and checked for WEOF somewhere but just assumed things
> would fit in wchar_t

WEOF that fits in a wint_t also fits in a wchar_t when the two types
have the same precision, the case where GCC issues the warning (and
where WEOF expands to 0xffffffffu).  So this isn't an example of
a problem that could arise on the targets that brought this up, or
any other targets that I'm aware of.

> (and so could be passing a negative value where the
> ABI means the callee expects wint_t values to be zero-extended, with
> consequent undefined behavior there).

Sign extension is also not an issue on the target where the warning
is issued (x86_64 ILP32) and where wchar_t is int and wint_t is
unsigned long.

> The correct fix involves thinking
> about where you know something is a wide character, and where something
> might be a wide character or WEOF.  It might then involve a cast to
> wint_t, or changing the type of a variable, or more changes than that.

There is nothing to fix when wchar_t and wint_t have the same
precision and  printf("%lc", L'x') does the same thing as
printf("%lc", (wint_t) L'x').  The cast is entirely superfluous.

Even so, the warning could have merit if there were a different
target to which the program might need to be ported where this
property didn't hold.  But as I said, I don't know of one and,
as it seems, neither do you.

I'm very much in favor of strict warnings that help point out
either real or potential bugs.  But I see no value in warning
about constructs that are perfectly safe.  To the contrary,
I think such warnings are harmful because they lead to
unnecessary churn and to bug injection.

It might be worthwhile to mention that Clang used to issue the
same warning as GCC until 2011 when they fixed it.  The bug is
here:

   https://llvm.org/bugs/show_bug.cgi?id=7981

(Although I think there is a flaw in the logic they used in
comment #2 it's only hypothetical because there is no target
that Clang supports where the submitted test case doesn't do
what it's expected to do either.)

Martin



More information about the Gcc-patches mailing list