bug#22085: 25.0.50; src/gmalloc.c hit by gcc >= 5 optimization

bug#22085: 25.0.50; src/gmalloc.c hit by gcc >= 5 optimization

[Wolfgang Jenkner]

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

On systems which use src/gmalloc.c but don't define HYBRID_MALLOC that
file defines global replacements for malloc and other memory management
functions.

Strictly speaking, this results in undefined behaviour according to ISO
C11 (wg14 n1570 draft) 7.1.3 and 7.22.3.

But it used to work.  However, with recent gcc versions and the default
optimization level (-O2), most of the definition of calloc is replaced
with a call to calloc (which the linker resolves to a call to that same
calloc).  Chances are temacs crashes because some library calls calloc.

This happens with the gcc 5.2.0 release and also with the 6.0.0 20151108
snapshot.

Please find below a self-contained test program which shows the problem
(try gcc5 -Wall -g -O2 callocopt.c -o callocopt && ./callocopt 666, and
then with -O instead of -O2, or even with -O2 -fno-optimize-strlen).

For comparison purposes, I note that, meanwhile, the issue has also
surfaced here

http://permalink.gmane.org/gmane.os.freebsd.devel.cvs.src/210733

Back to the problem with gmalloc.c, I'd suggest the simple
toolchain-independent work-around given in the patch below.

A real fix is to use hybrid malloc.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Test program --]
[-- Type: text/x-csrc, Size: 495 bytes --]

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
#include <string.h>

void *
calloc (size_t bytes, size_t size)
{
	void *result;
	static int cnt;

	if (cnt++) {
		fprintf(stderr, "Been there, done that.\n");
		abort();
	}

	result = malloc(bytes);
	if (result)
	return memset(result, 0, bytes);
	return result;
}

int
main (int argc, char *argv[])
{

	if (argc != 2)
		return (1);

	size_t bytes = strtoul(argv[1], NULL, 10);

	return (calloc(bytes, 1) ? 42 : 0);
}

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: gcc 5 optimization work-around for gmalloc --]
[-- Type: text/x-diff, Size: 2066 bytes --]

From 91a5b2c6f46db7f6882fb1bf514209f45802732e Mon Sep 17 00:00:00 2001
From: Wolfgang Jenkner <wjenkner@inode.at>
Date: Mon, 16 Nov 2015 13:15:00 +0100
Subject: [PATCH 1/5] * src/gmalloc.c: Always define gmalloc and friends.

This is a work-around to prevent the compiler from using semantic
knowledge about malloc for optimization purposes.  E.g., newer gcc
with -O2 replaces most of calloc's definition by a call to calloc.
---
 src/gmalloc.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/gmalloc.c b/src/gmalloc.c
index a88f4ab..90a52a1 100644
--- a/src/gmalloc.c
+++ b/src/gmalloc.c
@@ -60,7 +60,6 @@ extern void emacs_abort (void);
    which HYBRID_MACRO is defined.  Any other platform that wants to
    define it will have to define the macros DUMPED and
    ALLOCATED_BEFORE_DUMPING, defined below for Cygwin.  */
-#ifdef HYBRID_MALLOC
 #undef malloc
 #undef realloc
 #undef calloc
@@ -70,7 +69,6 @@ extern void emacs_abort (void);
 #define calloc gcalloc
 #define aligned_alloc galigned_alloc
 #define free gfree
-#endif  /* HYBRID_MALLOC */
 
 #ifdef CYGWIN
 extern void *bss_sbrk (ptrdiff_t size);
@@ -1711,13 +1709,13 @@ valloc (size_t size)
   return aligned_alloc (pagesize, size);
 }
 
-#ifdef HYBRID_MALLOC
 #undef malloc
 #undef realloc
 #undef calloc
 #undef aligned_alloc
 #undef free
 
+#ifdef HYBRID_MALLOC
 /* Declare system malloc and friends.  */
 extern void *malloc (size_t size);
 extern void *realloc (void *ptr, size_t size);
@@ -1816,6 +1814,38 @@ hybrid_get_current_dir_name (void)
 }
 #endif
 
+#else	/* ! HYBRID_MALLOC */
+
+void *
+malloc (size_t size)
+{
+  return gmalloc (size);
+}
+
+void *
+calloc (size_t nmemb, size_t size)
+{
+  return gcalloc (nmemb, size);
+}
+
+void
+free (void *ptr)
+{
+  gfree (ptr);
+}
+
+void *
+aligned_alloc (size_t alignment, size_t size)
+{
+  return galigned_alloc (alignment, size);
+}
+
+void *
+realloc (void *ptr, size_t size)
+{
+  return grealloc (ptr, size);
+}
+
 #endif	/* HYBRID_MALLOC */
 
 #ifdef GC_MCHECK
-- 
2.6.3

bug#22085: 25.0.50; src/gmalloc.c hit by gcc >= 5 optimization

[Wolfgang Jenkner]

On Thu, Dec 03 2015, Wolfgang Jenkner wrote:

> A real fix is to use hybrid malloc.

For which, please see

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22086

bug#22085: 25.0.50; src/gmalloc.c hit by gcc >= 5 optimization

[Paul Eggert]

Thanks for the fix; I have applied this patch to the emacs-25 branch and am 
closing Bug#22085. The companion Bug#22086 will require more thinking, and I 
plan to follow up there.

bug#22085: 25.0.50; src/gmalloc.c hit by gcc >= 5 optimization

[Wolfgang Jenkner]

On Sat, Dec 26 2015, Paul Eggert wrote:

> Thanks for the fix; I have applied this patch to the emacs-25 branch
> and am closing Bug#22085. The companion Bug#22086 will require more
> thinking, and I plan to follow up there.

I just wanted to wait until your plan for bug#22086 becomes more
concrete (I do have push access), but thanks anyway (I'm actually
surprised that you are willing to live with this work-around for
a little while :-)