patch 9.0.1908: undefined behaviour upper/lower function ptrs
Commit:
https://github.com/vim/vim/commit/d25021cf036c63d539f845a1ee05b03ea21d61ff
Author: Yee Cheng Chin <ychin....@gmail.com>
Date: Mon Sep 18 19:51:56 2023 +0200
patch 9.0.1908: undefined behaviour upper/lower function ptrs
Problem: undefined behaviour upper/lower function ptrs
Solution: Fix UBSAN error in regexp and simplify upper/lowercase
modifier code
The implementation of \u / \U / \l / \L modifiers in the substitute
command relies on remembering the state by setting function pointers on
func_all/func_one in the code. The code signature of `fptr_T` is
supposed to return void* (due to C function signatures not being able to
return itself due to type recursion), and the definition of the
functions (e.g. to_Upper) didn't follow this rule, and so the code tries
to cast functions of different signatures, resulting in undefined
behavior error under UBSAN in Clang 17. See #12745.
We could just fix `do_Upper`/etc to just return void*, which would fix
the problem. However, these functions actually do not need to return
anything at all. It used to be the case that there was only one pointer
"func" to store the pointer, which is why the function needs to either
return itself or NULL to indicate whether it's a one time or ongoing
modification. However, c2c355df6f094cdb9e599fd395a78c14486ec697
(7.3.873) already made that obsolete by introducing `func_one` and
`func_all` to store one-time and ongoing operations separately, so these
functions don't actually need to return anything anymore because it's
implicit whether it's a one-time or ongoing operation. Simplify the code
to reflect that.
closes: #13117
Signed-off-by: Christian Brabandt <c...@256bit.org>
Co-authored-by: Yee Cheng Chin <ychin....@gmail.com>
diff --git a/src/regexp.c b/src/regexp.c
index 2c740d303..a64672856 100644
--- a/src/regexp.c
+++ b/src/regexp.c
@@ -1709,46 +1709,20 @@ cstrchr(char_u *s, int c)
// regsub stuff //
////////////////////////////////////////////////////////////////
-/*
- * We should define ftpr as a pointer to a function returning a pointer to
- * a function returning a pointer to a function ...
- * This is impossible, so we declare a pointer to a function returning a
- * void pointer. This should work for all compilers.
- */
-typedef void (*(*fptr_T)(int *, int));
+typedef void (*fptr_T)(int *, int);
static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int
destlen, int flags);
- static fptr_T
+ static void
do_upper(int *d, int c)
{
*d = MB_TOUPPER(c);
-
- return (fptr_T)NULL;
-}
-
- static fptr_T
-do_Upper(int *d, int c)
-{
- *d = MB_TOUPPER(c);
-
- return (fptr_T)do_Upper;
}
- static fptr_T
+ static void
do_lower(int *d, int c)
{
*d = MB_TOLOWER(c);
-
- return (fptr_T)NULL;
-}
-
- static fptr_T
-do_Lower(int *d, int c)
-{
- *d = MB_TOLOWER(c);
-
- return (fptr_T)do_Lower;
}
/*
@@ -2203,13 +2177,13 @@ vim_regsub_both(
{
switch (*src++)
{
- case 'u': func_one = (fptr_T)do_upper;
+ case 'u': func_one = do_upper;
continue;
- case 'U': func_all = (fptr_T)do_Upper;
+ case 'U': func_all = do_upper;
continue;
- case 'l': func_one = (fptr_T)do_lower;
+ case 'l': func_one = do_lower;
continue;
- case 'L': func_all = (fptr_T)do_Lower;
+ case 'L': func_all = do_lower;
continue;
case 'e':
case 'E': func_one = func_all = (fptr_T)NULL;
@@ -2276,11 +2250,12 @@ vim_regsub_both(
// Write to buffer, if copy is set.
if (func_one != (fptr_T)NULL)
- // Turbo C complains without the typecast
- func_one = (fptr_T)(func_one(&cc, c));
+ {
+ func_one(&cc, c);
+ func_one = NULL;
+ }
else if (func_all != (fptr_T)NULL)
- // Turbo C complains without the typecast
- func_all = (fptr_T)(func_all(&cc, c));
+ func_all(&cc, c);
else // just copy
cc = c;
@@ -2424,11 +2399,12 @@ vim_regsub_both(
c = *s;
if (func_one != (fptr_T)NULL)
- // Turbo C complains without the typecast
- func_one = (fptr_T)(func_one(&cc, c));
+ {
+ func_one(&cc, c);
+ func_one = NULL;
+ }
else if (func_all != (fptr_T)NULL)
- // Turbo C complains without the typecast
- func_all = (fptr_T)(func_all(&cc, c));
+ func_all(&cc, c);
else // just copy
cc = c;
diff --git a/src/version.c b/src/version.c
index 655b1d1dd..18c12fae5 100644
--- a/src/version.c
+++ b/src/version.c
@@ -699,6 +699,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 1908,
/**/
1907,
/**/
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/vim_dev/E1qiIXa-000hHZ-Ty%40256bit.org.
