Hi Paul, > > The attached patch (for the 'branch-1.4' branch) fixes the issue. > > I think the actual bug lies elsewhere: m4 is written as if int32_t arithmetic > wraps around silently, but that's not what C does. If I'm right, the bug is > not > in the FreeBSD C compiler, it's in m4.
OK, let's clean up the incorrect assumptions about signed integer arithmetic first. Here's a patch that does this, in the caller of the 'ntoa' function. I don't see incorrect assumptions in the code of 'ntoa' itself. Unfortunately, this patch does not fix the test failure on FreeBSD 12.0. That is, after this patch, my previous patch is still needed. Bruno
>From 8160ff54537e454e5653d6bc7e735c28f2b99c10 Mon Sep 17 00:00:00 2001 From: Bruno Haible <br...@clisp.org> Date: Sun, 3 May 2020 11:49:00 +0200 Subject: [PATCH] Don't assume that signed integer arithmetic wraps around like unsigned. Reported by Paul Eggert. * src/builtin.c (m4_eval): Cast 'int' value to 'unsigned int' before doing arithmetic that goes beyond the range 0..INT_MAX. --- src/builtin.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/builtin.c b/src/builtin.c index 4eaccc6..14d8eaf 100644 --- a/src/builtin.c +++ b/src/builtin.c @@ -1,6 +1,6 @@ /* GNU m4 -- A simple macro processor - Copyright (C) 1989-1994, 2000, 2004, 2006-2014, 2016-2017 Free + Copyright (C) 1989-1994, 2000, 2004, 2006-2014, 2016-2017, 2020 Free Software Foundation, Inc. This file is part of GNU M4. @@ -1095,7 +1095,7 @@ m4_eval (struct obstack *obs, int argc, token_data **argv) if (*ARG (2) && !numeric_arg (argv[0], ARG (2), &radix)) return; - if (radix < 1 || radix > (int) strlen (digits)) + if (radix < 1 || (unsigned int) radix > strlen (digits)) { M4ERROR ((warning_status, 0, "radix %d in builtin `%s' out of range", @@ -1120,15 +1120,18 @@ m4_eval (struct obstack *obs, int argc, token_data **argv) if (radix == 1) { + uint32_t abs_value; + if (value < 0) { obstack_1grow (obs, '-'); - value = -value; + abs_value = - (uint32_t) value; } - /* This assumes 2's-complement for correctly handling INT_MIN. */ - while (min-- - value > 0) + else + abs_value = (uint32_t) value; + for (; (unsigned int) min > abs_value; min--) obstack_1grow (obs, '0'); - while (value-- != 0) + for (; abs_value > 0; abs_value--) obstack_1grow (obs, '1'); obstack_1grow (obs, '\0'); return; -- 2.7.4
>From aaa68d60ab6c84783cf050a9e43aa6f40c81e4df Mon Sep 17 00:00:00 2001 From: Bruno Haible <br...@clisp.org> Date: Sun, 3 May 2020 03:00:46 +0200 Subject: [PATCH] Work around a compiler optimization bug on FreeBSD 12.0. * src/builtin.c (ntoa): Declare value parameter as 'volatile'. --- src/builtin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/builtin.c b/src/builtin.c index 4eaccc6..988cc93 100644 --- a/src/builtin.c +++ b/src/builtin.c @@ -449,8 +449,9 @@ numeric_arg (token_data *macro, const char *arg, int *valuep) /* Digits for number to ASCII conversions. */ static char const digits[] = "0123456789abcdefghijklmnopqrstuvwxyz"; +/* The 'volatile' neutralizes a clang 6.0.1 optimization bug. */ const char * -ntoa (int32_t value, int radix) +ntoa (int32_t volatile value, int radix) { bool negative; uint32_t uvalue; -- 2.7.4