On Thursday, August 17, 2017 12:14:05 Paul Eggert wrote:
> On 08/17/2017 04:40 AM, Kamil Dudka wrote:
> > -typedef short int DELTA; /* to hold displacement within one context */
> > +typedef int DELTA; /* to hold displacement within one context */
>
> Thanks for the heads-up. Although that fixes things for that particular
> test case, it won't work for larger cases.
Do you have an example of the larger case? We could add a test-case for it.
> The type should be ptrdiff_t instead of int.
>
> As its FIXME comment says, ptx is riddled with integer-overflow bugs. I
> installed the attached patch to fix the bug that you mentioned along
> with the other low-hanging fruit that I found, and am marking the bug as
> fixed upstream. I expect some other integer-overflow bugs can still
> occur in practice, but at least this patch is a significant improvement.
>
> This patch prefers signed integer types like ptrdiff_t to unsigned types
> like size_t, as signed types allow for better checking when compiled
> with sanitization.
Your patch introduces the following warnings:
Error: CONSTANT_EXPRESSION_RESULT:
src/ptx.c:1939: result_independent_of_operands: "tmp <= 9223372036854775807L"
is always true regardless of the values of its operands. This occurs as the
logical second operand of "&&".
# 1937| intmax_t tmp;
# 1938| if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) ==
LONGINT_OK
# 1939|->&& 0 < tmp && tmp <= PTRDIFF_MAX))
# 1940| die (EXIT_FAILURE, 0, _("invalid gap width: %s"),
# 1941| quote (optarg));
Error: CONSTANT_EXPRESSION_RESULT:
src/ptx.c:1966: result_independent_of_operands: "tmp <= 9223372036854775807L"
is always true regardless of the values of its operands. This occurs as the
logical second operand of "&&".
# 1964| intmax_t tmp;
# 1965| if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) ==
LONGINT_OK
# 1966|->&& 0 < tmp && tmp <= PTRDIFF_MAX))
# 1967| die (EXIT_FAILURE, 0, _("invalid line width: %s"),
# 1968| quote (optarg));
Anyway, it fixes the original bug so I am fine with the patch as it is.
Thank you for pushing the fix!
Kamil
