* crash/hang with gcc 5+ -O2 and --enable-zsh-mem @ 2018-02-24 19:01 Mikael Magnusson 2018-02-25 0:13 ` Joey Pabalinas 0 siblings, 1 reply; 6+ messages in thread From: Mikael Magnusson @ 2018-02-24 19:01 UTC (permalink / raw) To: zsh workers I figured out this was because of -foptimize-strlen which somehow causes calloc() to recurse infinitely and crash (with -O2 which also enables -foptimize-sibling-calls it just hangs with no crash because it doesn't consume extra stack, i presume). What I don't know is how to fix it, or the causal relationship between strlen optimization and calloc recursing infinitely. I've tried gcc 5.4, 6.2 and 6.4 with the same result. Compiling only Src/mem.c with the problematic flag is enough so it's something in there, but removing the one strlen() call in the file has no effect. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: crash/hang with gcc 5+ -O2 and --enable-zsh-mem 2018-02-24 19:01 crash/hang with gcc 5+ -O2 and --enable-zsh-mem Mikael Magnusson @ 2018-02-25 0:13 ` Joey Pabalinas 2018-02-25 7:06 ` Joey Pabalinas 0 siblings, 1 reply; 6+ messages in thread From: Joey Pabalinas @ 2018-02-25 0:13 UTC (permalink / raw) To: Mikael Magnusson; +Cc: zsh workers, Joey Pabalinas [-- Attachment #1: Type: text/plain, Size: 2259 bytes --] On Sat, Feb 24, 2018 at 08:01:28PM +0100, Mikael Magnusson wrote: > I figured out this was because of -foptimize-strlen which somehow > causes calloc() to recurse infinitely and crash (with -O2 which also > enables -foptimize-sibling-calls it just hangs with no crash because > it doesn't consume extra stack, i presume). What I don't know is how > to fix it, or the causal relationship between strlen optimization and > calloc recursing infinitely. I've tried gcc 5.4, 6.2 and 6.4 with the > same result. Compiling only Src/mem.c with the problematic flag is > enough so it's something in there, but removing the one strlen() call > in the file has no effect. I am also seeing the same behavior, and to be completely honest I have no idea how strlen() is related either. Some testing in gdb did show that you were right; the problem is a loop caused by a repeated jump back to line 1719, which definitely lines up with the `-foptimize-sibling-calls` part. The only fix I could find which didn't requiring substantial reimplementation of the memory management functions was to replace the malloc() call in calloc() with realloc() instead. With a NULL `p` argument realloc() behaves exactly the same as malloc() does, and (at least on my system) gcc doesn't seem to consider realloc() a candidate for sibling call optimizations; give this patch a try and _hopefully_ this is a viable solution. Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com> 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Src/mem.c b/Src/mem.c index 840bbb6e4a4eb6fd73..8b0dafed6fdae35a61 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -1714,12 +1714,18 @@ MALLOC_RET_T calloc(MALLOC_ARG_T n, MALLOC_ARG_T size) { long l; - char *r; + char *r = NULL; if (!(l = n * size)) return (MALLOC_RET_T) m_high; - r = malloc(l); + /* + * use realloc() (with a NULL `p` argument it behaves exactly the same + * as malloc() does) to prevent an infinite loop caused by sibling-call + * optimizations (the malloc() call would otherwise be replaced by an + * unconditional branch back to line 1719 ad infinitum). + */ + r = realloc(r, l); memset(r, 0, l); -- 2.16.2 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: crash/hang with gcc 5+ -O2 and --enable-zsh-mem 2018-02-25 0:13 ` Joey Pabalinas @ 2018-02-25 7:06 ` Joey Pabalinas 2018-02-25 7:07 ` Joey Pabalinas 2018-02-26 20:24 ` Peter Stephenson 0 siblings, 2 replies; 6+ messages in thread From: Joey Pabalinas @ 2018-02-25 7:06 UTC (permalink / raw) To: Mikael Magnusson; +Cc: zsh workers, Joey Pabalinas [-- Attachment #1: Type: text/plain, Size: 1513 bytes --] On Sat, Feb 24, 2018 at 02:13:34PM -1000, Joey Pabalinas wrote: > The only fix I could find which didn't requiring substantial > reimplementation of the memory management functions was to replace > the malloc() call in calloc() with realloc() instead. With a NULL `p` > argument realloc() behaves exactly the same as malloc() does, and > (at least on my system) gcc doesn't seem to consider realloc() a > candidate for sibling call optimizations; give this patch a try > and _hopefully_ this is a viable solution. On second thought, doing it this way is probably a *little* bit better; the needless initialization of `r` to NULL is avoided, and it also makes the purpose of using realloc() over malloc() a *tiny* bit more explicit: Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com> 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Src/mem.c b/Src/mem.c index 840bbb6e4a4eb6fd73..f1208197b3ddac2139 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -1719,7 +1719,13 @@ calloc(MALLOC_ARG_T n, MALLOC_ARG_T size) if (!(l = n * size)) return (MALLOC_RET_T) m_high; - r = malloc(l); + /* + * use realloc() (with a NULL `p` argument it behaves exactly the same + * as malloc() does) to prevent an infinite loop caused by sibling-call + * optimizations (the malloc() call would otherwise be replaced by an + * unconditional branch back to line 1719 ad infinitum). + */ + r = realloc(NULL, l); memset(r, 0, l); -- 2.16.2 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: crash/hang with gcc 5+ -O2 and --enable-zsh-mem 2018-02-25 7:06 ` Joey Pabalinas @ 2018-02-25 7:07 ` Joey Pabalinas 2018-02-26 20:24 ` Peter Stephenson 1 sibling, 0 replies; 6+ messages in thread From: Joey Pabalinas @ 2018-02-25 7:07 UTC (permalink / raw) To: Mikael Magnusson; +Cc: zsh workers, Joey Pabalinas [-- Attachment #1: Type: text/plain, Size: 361 bytes --] On Sat, Feb 24, 2018 at 09:06:37PM -1000, Joey Pabalinas wrote: > On second thought, doing it this way is probably a *little* bit better; the > needless initialization of `r` to NULL is avoided, and it also makes the > purpose of using realloc() over malloc() a *tiny* bit more explicit: Whoops, sorry about the double send there. -- Joey Pabalinas [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: crash/hang with gcc 5+ -O2 and --enable-zsh-mem 2018-02-25 7:06 ` Joey Pabalinas 2018-02-25 7:07 ` Joey Pabalinas @ 2018-02-26 20:24 ` Peter Stephenson 2018-02-27 1:13 ` Joey Pabalinas 1 sibling, 1 reply; 6+ messages in thread From: Peter Stephenson @ 2018-02-26 20:24 UTC (permalink / raw) To: zsh workers On Sat, 24 Feb 2018 21:06:37 -1000 Joey Pabalinas <joeypabalinas@gmail.com> wrote: > diff --git a/Src/mem.c b/Src/mem.c > index 840bbb6e4a4eb6fd73..f1208197b3ddac2139 100644 > --- a/Src/mem.c > +++ b/Src/mem.c > @@ -1719,7 +1719,13 @@ calloc(MALLOC_ARG_T n, MALLOC_ARG_T size) > if (!(l = n * size)) > return (MALLOC_RET_T) m_high; > > - r = malloc(l); > + /* > + * use realloc() (with a NULL `p` argument it behaves exactly the same > + * as malloc() does) to prevent an infinite loop caused by sibling-call > + * optimizations (the malloc() call would otherwise be replaced by an > + * unconditional branch back to line 1719 ad infinitum). > + */ > + r = realloc(NULL, l); > > memset(r, 0, l); Was going to object some older realloc()s don't support that behaviour, but this is quite specifically in zsh memory management where realloc() is explicitly defined, duh. Can't see a problem with this apart from a minor performance hit. calloc isn't used anywhere this would be a big deal, I don't think. I committed it, thanks. I suppose gcc really wants looking at too... pws ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: crash/hang with gcc 5+ -O2 and --enable-zsh-mem 2018-02-26 20:24 ` Peter Stephenson @ 2018-02-27 1:13 ` Joey Pabalinas 0 siblings, 0 replies; 6+ messages in thread From: Joey Pabalinas @ 2018-02-27 1:13 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh workers [-- Attachment #1: Type: text/plain, Size: 880 bytes --] On Mon, Feb 26, 2018 at 08:24:46PM +0000, Peter Stephenson wrote: > Can't see a problem with this apart from a minor performance hit. > calloc isn't used anywhere this would be a big deal, I don't think. I > committed it, thanks. > > I suppose gcc really wants looking at too... Cheers. I honestly still am completely confused at how strlen() could in any way be related to this (and yet, it most certainly is as far as gcc is concered) and _why_ calloc() is even a viable candidate for malloc() sibling-call optimization in the first place. I am also sort of thinking this is a gcc bug, but I have no idea where to even start looking in that abyss known to some as the gcc codebase. I'll keep my ear to the ground regardless; I find it *very* hard to believe that zsh is the only codebase affected by this optimization peculiarity. -- Joey Pabalinas [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-27 1:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-24 19:01 crash/hang with gcc 5+ -O2 and --enable-zsh-mem Mikael Magnusson 2018-02-25 0:13 ` Joey Pabalinas 2018-02-25 7:06 ` Joey Pabalinas 2018-02-25 7:07 ` Joey Pabalinas 2018-02-26 20:24 ` Peter Stephenson 2018-02-27 1:13 ` Joey Pabalinas
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).