Re: Fwd: 5.8: LTO exposes some new issues

[Roman Perepelitsa <roman.perepelitsa@xxxxxxxxx>]

On Mon, Jul 27, 2020 at 2:47 PM Tomasz Kłoczko <kloczko.tomasz@xxxxxxxxx> wrote:
> Here is whole list of compile and linking warnings:
>
> utils.c: In function 'getkeystring':
> lto1: warning: function may return address of local variable [-Wreturn-local-addr]
> utils.c:6644:16: note: declared here
>  6644 |     char *buf, tmp[1];
>       |                ^

This one might be a real bug. At the end of getkeystring there is an
explicit check for `how & GETKEY_SINGLE_CHAR`. If this condition is
true at that point, the code runs into undefined behavior. First,
writing to `*t` is illegal because it points outside of `tmp`. Second,
returning `buf` is illegal because it holds a pointer to a local
variable (hence the warning).

I'm attaching a patch that keeps the branch (although I'm not sure
it's reachable) and makes the code less broken if it ever triggers. I
cannot verify that it gets rid of the warning because I don't get this
warning with unmodified code.

FYI: I won't be doing anything about the warning in gettempname (which
I'm not getting with my toolchain).

Roman.

diff --git a/Src/utils.c b/Src/utils.c
index 5151b89a8..e03f41468 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -7162,11 +7162,13 @@ getkeystring(char *s, int *len, int how, int *misc)
      */
     DPUTS((how & (GETKEY_DOLLAR_QUOTE|GETKEY_UPDATE_OFFSET)) ==
 	  GETKEY_DOLLAR_QUOTE, "BUG: unterminated $' substitution");
+    if (how & GETKEY_SINGLE_CHAR) {
+	*misc = 0;
+	return s;
+    }
     *t = '\0';
     if (how & GETKEY_DOLLAR_QUOTE)
 	*tdest = '\0';
-    if (how & GETKEY_SINGLE_CHAR)
-	*misc = 0;
     else
 	*len = ((how & GETKEY_DOLLAR_QUOTE) ? tdest : t) - buf;
     return buf;