two undefined behaviour occurrences in m4
Bruno Haible <[email protected]> Sat, 02 Dec 2023 06:41:17 +0100
| Newsgroups | gmane.comp.gnu.m4.bugs |
|---|---|
| Message-ID | <18274337.aqqpvuueIm@nimes> |
Hi,
After fixing the Gnulib bug reported at
<https://lists.gnu.org/archive/html/bug-m4/2023-02/msg00000.html>
I wanted to check whether the newest m4 snapshot passes its tests
with
CC="clang -fsanitize=address,undefined,signed-integer-overflow,shift,integer-divide-by-zero -fsanitize-address-use-after-scope -fno-sanitize-recover=all"
And it doesn't. There are two problems:
1) In the "make check" run (attached: make-check-1.log), there are many
occurrences of
path.c:72:23: runtime error: applying non-zero offset 1 to null pointer
This code
----------------------------------------
path_end = strchr (path, ':');
if (path_end)
*path_end = '\0';
add_include_directory (path);
path = path_end + 1;
----------------------------------------
computes a NULL pointer + 1. Which is invalid according to ISO C 23 § 6.5.6.(9)
"... If the pointer operand and the result do not point to elements
of the same array object or one past the last element of the array object,
the behavior is undefined..."
2) After fixing this, in the next "make check" run (attached: make-check-2.log),
there are many occurrences of
macro.c:388:3: runtime error: addition of unsigned offset to 0x521000008d28 overflowed to 0x521000008d10
The problem here is that the code is adding a pointer value such as
0x521000008d28 with an unsigned offset of 0xffffffffffffffe8, and
this sum overflows. It is invalid according to ISO C 23 § 6.5.6.(9)
"If the addition or subtraction produces an overflow, the behavior
is undefined."
Find attached a patch that fixes both issues.
OK to push it?
make-check-1.log
(text/x-log, 209.5 KB) - not displayed
make-check-2.log
(text/x-log, 212.8 KB) - not displayed
0001-Fix-two-occurrences-of-undefined-behaviour.patch
(text/x-patch, 1.5 KB)
From 7f5cabf73de36c8d9a8eb60040db116f3fd1dcb5 Mon Sep 17 00:00:00 2001 From: Bruno Haible <[email protected]> Date: Sat, 2 Dec 2023 06:38:14 +0100 Subject: [PATCH] Fix two occurrences of undefined behaviour. * src/path.c (include_env_init): When path_end becomes NULL, terminate the loop without computing path_end + 1. * src/macro.c (expand_macro): Pass a signed negative value to obstack_blank_fast. This avoids a pointer overflow. --- src/macro.c | 2 +- src/path.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/macro.c b/src/macro.c index 6052cec4..15167d68 100644 --- a/src/macro.c +++ b/src/macro.c @@ -385,5 +385,5 @@ expand_macro (symbol *sym) obstack_free (&argc_stack, argv[0]); else obstack_free (&arguments, NULL); - obstack_blank_fast (&argv_stack, -argc * sizeof (token_data *)); + obstack_blank_fast (&argv_stack, - argc * (ptrdiff_t) sizeof (token_data *)); } diff --git a/src/path.c b/src/path.c index bba197cd..09ecf5e9 100644 --- a/src/path.c +++ b/src/path.c @@ -50,7 +50,6 @@ void include_env_init (void) { char *path; - char *path_end; char *env_path; if (no_gnu_extensions) @@ -63,15 +62,16 @@ include_env_init (void) env_path = xstrdup (env_path); path = env_path; - do + for (;;) { - path_end = strchr (path, ':'); + char *path_end = strchr (path, ':'); if (path_end) *path_end = '\0'; add_include_directory (path); + if (!path_end) + break; path = path_end + 1; } - while (path_end); free (env_path); } -- 2.34.1