https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78586

--- Comment #6 from Martin Sebor <msebor at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> There are still various weird spots in format_integer.
> E.g.
>   else if (TREE_CODE (TREE_TYPE (arg)) == INTEGER_TYPE
>            || TREE_CODE (TREE_TYPE (arg)) == POINTER_TYPE)
>     {
>       /* Determine the type of the provided non-constant argument.  */
>       if (TREE_CODE (arg) == NOP_EXPR)
>         arg = TREE_OPERAND (arg, 0);
>       else if (TREE_CODE (arg) == CONVERT_EXPR)
>         arg = TREE_OPERAND (arg, 0);
>       if (TREE_CODE (arg) == COMPONENT_REF)
>         arg = TREE_OPERAND (arg, 1);
>           
>       argtype = TREE_TYPE (arg);
>     }
> When would this actually happen?

I checked by running the builtin-sprintf*.c tests and none of the conditions is
true for any of them so it looks like it might be dead code from an earlier
versions of the pass that I never took out.

> 2) for SSA_NAMEs with VR_RANGE, you only look at the argument's ranges,
> while for something VR_VARYING and similar you consider both dirtype and
> argtype's precision.  Isn't it always UB if the precision is different? 

For %hhi and %hi the argument type can be char, short, or int, and considering
the type of the directive's "formal" argument avoids warning on cases where the
actual argument would be too big to fit if it were formatted using a directive
for its own type.  Like this:

  char d[4];
  return __builtin_sprintf (d, "%hhu", 123456);


> Corner case might be C bitfields, not sure what exact type one gets when
> passing that to varargs.  For UB wouldn't it be better to punt (return don't
> know value)?

Yes, the pass shouldn't try to make any assumptions when it detects undefined
behavior.  I didn't intentionally encode any such assumptions in it but I may
have done that inadvertently.  Those would be bugs.

> 3) "When precision is specified but unknown, use zero as the minimum since
> it results in no bytes on output (unless width is specified to be greater
> than 0)."
> When is 0 printed as nothing?  I thought it is printed as 0 (or 0x0 etc.).

I thought the same until last week.  Apparently we both thought wrong. 
printf("%.0i", 0) is required to print nothing.  The text is under the  d, i,
o, u, and x/X conversion specifiers: "The result of converting zero with an
explicit precision of zero shall be no characters."

> 4)               if (code == NOP_EXPR)
>                 argtype = TREE_TYPE (gimple_assign_rhs1 (def));
> Similar case to 1), except it can happen, but argtype could be anything,
> boolean type, perhaps fixed point type, enum, whatever.

I'm not quite sure I follow what you are trying to say here.  This code avoids
false positives for unknown arguments of wider types that are cast to narrower
types, as in: 'sprintf ((char[4]){ }, "%hhi", (unsigned char)i)' where i is an
int with an unknown value.  It strips the NOP_EXPR that promotes the argument
to int and lets the function use the range of the type of the argument before
the promotion.  It could be that I'm missing something so a test case would be
helpful.

Reply via email to