Hello,

    This change was motivated by "Mismatched free()/delete/delete[]"
errors reported by valgrind and mused about in Squid source code.

I speculate that the old new/delete replacement code was the result of
slow accumulation of working hacks to accomodate various environments,
as compiler support for the feature evolved. The cumulative result does
not actually work well (see the above paragraph), and the replacement
functions had the following visible coding problems according to [1,2]:

a) Declared with non-standard profiles that included throw specifiers.

b) Declared inline. C++ says that the results of inline declarations
   have unspecified effects. In Squid, they probably necessitated
   complex compiler-specific "extern inline" workarounds.

c) Defined in the header file. C++ says that defining replacements "in
   any source file" is enough and that multiple replacements per
   program (which is what a header file definition produces) result in
   "undefined behavior".

d) Declared inconsistently (only 2 out of 4 flavors). Declaring one base
   flavor should be sufficient, but if we declare more, we should
   declare all of them.

[1] http://en.cppreference.com/w/cpp/memory/new/operator_new
[2] http://en.cppreference.com/w/cpp/memory/new/operator_delete

The replacements were not provided to clang (trunk r13219), but there
was no explanation why. This patch does not change that exclusion.


I have no idea whether any of the old hacks are still necessary in some
cases. However, I suspect that either we do not care much if the
replacements are not enabled on some poorly supported platforms OR we
can disable them (or make them work) using much simpler hacks for the
platforms we do care about.


If the patch is approved, the SquidNew.h file should be removed. The
patch does not contain this change, but I hope that removing that file
should not be difficult -- it is not even mentioned in Makefile.am files
AFAICT.

The attached patch does not remove Debug::OutStream class that was
created to work around some of the above problems. There is a different
pending patch ("Do not hide important/critical messages") which also
removes that class completely (for somewhat different reasons):
http://lists.squid-cache.org/pipermail/squid-dev/2016-March/005510.html


Thank you,

Alex.
Replace new/delete operators using modern C++ rules.

This change was motivated by "Mismatched free()/delete/delete[]" errors
reported by valgrind and mused about in Squid source code.

I speculate that the old new/delete replacement code was the result of
slow accumulation of working hacks to accomodate various environments,
as compiler support for the feature evolved. The cumulative result does
not actually work well (see the above paragraph), and the replacement
functions had the following visible coding problems according to [1,2]:

a) Declared with non-standard profiles that included throw specifiers.
b) Declared inline. C++ says that the results of inline declarations
   have unspecified effects. In Squid, they probably necessitated
   complex compiler-specific "extern inline" workarounds.
c) Defined in the header file. C++ says that defining replacements "in
   any source file" is enough and that multiple replacements per
   program (which is what a header file definition produces) result in
   "undefined behavior".
d) Declared inconsistently (only 2 out of 4 flavors). Declaring one base
   flavor should be sufficient, but if we declare more, we should
   declare all of them.

[1] http://en.cppreference.com/w/cpp/memory/new/operator_new
[2] http://en.cppreference.com/w/cpp/memory/new/operator_delete

The replacements were not provided to clang (trunk r13219), but there
was no explanation why. This patch does not change that exclusion.

I have no idea whether any of the old hacks are still necessary in some
cases. However, I suspect that either we do not care much if the
replacements are not enabled on some poorly supported platforms OR we
can disable them (or make them work) using much simpler hacks for the
platforms we do care about.

=== modified file 'compat/os/macosx.h'
--- compat/os/macosx.h	2016-01-01 00:12:18 +0000
+++ compat/os/macosx.h	2016-04-07 06:09:43 +0000
@@ -11,28 +11,23 @@
 
 #if _SQUID_APPLE_
 
 /****************************************************************************
  *--------------------------------------------------------------------------*
  * DO *NOT* MAKE ANY CHANGES below here unless you know what you're doing...*
  *--------------------------------------------------------------------------*
  ****************************************************************************/
 
 /*
  *   This OS has at least one version that defines these as private
  *   kernel macros commented as being 'non-standard'.
  *   We need to use them, much nicer than the OS-provided __u*_*[]
  */
 //#define s6_addr8  __u6_addr.__u6_addr8
 //#define s6_addr16 __u6_addr.__u6_addr16
 #define s6_addr32 __u6_addr.__u6_addr32
 
 #include "compat/cmsg.h"
 
-// MacOS GCC 4.0.1 and 4.2.1 supply __GNUC_GNU_INLINE__ but do not actually define  __attribute__((gnu_inline))
-#if defined(__cplusplus) && !defined(_SQUID_EXTERNNEW_)
-#define _SQUID_EXTERNNEW_ extern inline
-#endif
-
 #endif /* _SQUID_APPLE_ */
 #endif /* SQUID_OS_MACOSX_H */
 

=== modified file 'compat/os/sgi.h'
--- compat/os/sgi.h	2016-01-01 00:12:18 +0000
+++ compat/os/sgi.h	2016-04-07 06:09:43 +0000
@@ -8,32 +8,23 @@
 
 #ifndef SQUID_OS_SGI_H
 #define SQUID_OS_SGI_H
 
 #if _SQUID_SGI_
 
 /****************************************************************************
  *--------------------------------------------------------------------------*
  * DO *NOT* MAKE ANY CHANGES below here unless you know what you're doing...*
  *--------------------------------------------------------------------------*
  ****************************************************************************/
 
 #if !defined(_SVR4_SOURCE)
 #define _SVR4_SOURCE        /* for tempnam(3) */
 #endif
 
 #if USE_ASYNC_IO
 #define _ABI_SOURCE
 #endif /* USE_ASYNC_IO */
 
-#if defined(__cplusplus) && !defined(_SQUID_EXTERNNEW_) && !defined(_GNUC_)
-/*
- * The gcc compiler treats extern inline functions as being extern,
- * while the SGI MIPSpro compilers treat them as inline. To get equivalent
- * behavior, remove the inline keyword.
- */
-#define _SQUID_EXTERNNEW_ extern
-#endif
-
 #endif /* _SQUID_SGI_ */
 #endif /* SQUID_OS_SGI_H */
 

=== modified file 'compat/os/solaris.h'
--- compat/os/solaris.h	2016-01-01 00:12:18 +0000
+++ compat/os/solaris.h	2016-04-07 06:09:43 +0000
@@ -42,47 +42,40 @@ typedef union {
 
 /**
  * prototypes for system function missing from system includes
  * NP: sys/resource.h and sys/time.h are apparently order-dependant.
  */
 #include <sys/time.h>
 #include <sys/resource.h>
 SQUIDCEXTERN int getrusage(int, struct rusage *);
 
 /**
  * prototypes for system function missing from system includes
  * on some Solaris systems.
  */
 SQUIDCEXTERN int getpagesize(void);
 #if !defined(_XPG4_2) && !(defined(__EXTENSIONS__) || \
 (!defined(_POSIX_C_SOURCE) && !defined(_XOPEN_SOURCE)))
 SQUIDCEXTERN int gethostname(char *, int);
 #endif
 
 /*
- * SunPro CC handles extern inline as inline, PLUS extern symbols.
- */
-#if !defined(_SQUID_EXTERNNEW_) && defined(__SUNPRO_CC)
-#define _SQUID_EXTERNNEW_ extern
-#endif
-
-/*
  * SunStudio CC does not define C++ portability API __FUNCTION__
  */
 #if defined(__SUNPRO_CC) && !defined(__FUNCTION__)
 #define __FUNCTION__ ""
 #endif
 
 /* Bug 2500: Solaris 10/11 require s6_addr* defines. */
 //#define s6_addr8   _S6_un._S6_u8
 //#define s6_addr16  _S6_un._S6_u16
 #define s6_addr32  _S6_un._S6_u32
 
 /* Bug 3057: Solaris 9 defines struct addrinfo with size_t instead of socklen_t
  *           this causes binary incompatibility on 64-bit systems.
  *  Fix this by bundling a copy of the OpenSolaris 10 netdb.h to use instead.
  */
 #if defined(__sparcv9)
 #include "compat/os/opensolaris_10_netdb.h"
 #endif
 
 /* Solaris 10 lacks SUN_LEN */

=== modified file 'include/SquidNew.h'
--- include/SquidNew.h	2016-01-01 00:12:18 +0000
+++ include/SquidNew.h	2016-04-07 06:09:43 +0000
@@ -1,41 +1,15 @@
 /*
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 #ifndef SQUID_NEW_H
 #define SQUID_NEW_H
 
-#if !defined(__SUNPRO_CC) && !defined(__clang__)
-/* Any code using libstdc++ must have externally resolvable overloads
- * for void * operator new - which means in the .o for the binary,
- * or in a shared library. static libs don't propogate the symbol
- * so, look in the translation unit containing main() in squid
- * for the extern version in squid
- */
-#include <new>
-
-_SQUID_EXTERNNEW_ void *operator new(size_t size) throw (std::bad_alloc)
-{
-    return xmalloc(size);
-}
-_SQUID_EXTERNNEW_ void operator delete (void *address) throw()
-{
-    xfree(address);
-}
-_SQUID_EXTERNNEW_ void *operator new[] (size_t size) throw (std::bad_alloc)
-{
-    return xmalloc(size);
-}
-_SQUID_EXTERNNEW_ void operator delete[] (void *address) throw()
-{
-    xfree(address);
-}
-
-#endif /* !__SUNPRO_CC && !__clang__*/
+// TODO: Remove this file.
 
 #endif /* SQUID_NEW_H */
 

=== modified file 'include/util.h'
--- include/util.h	2016-01-01 00:12:18 +0000
+++ include/util.h	2016-04-07 06:09:43 +0000
@@ -2,57 +2,40 @@
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 #ifndef SQUID_UTIL_H
 #define SQUID_UTIL_H
 
 #if HAVE_TIME_H
 #include <time.h>
 #endif
 #if HAVE_ARPA_INET_H
 #include <arpa/inet.h>
 #endif
 
 SQUIDCEXTERN int tvSubUsec(struct timeval, struct timeval);
 SQUIDCEXTERN double tvSubDsec(struct timeval, struct timeval);
 SQUIDCEXTERN void Tolower(char *);
-#if defined(__cplusplus)
-/*
- * Any code using libstdc++ must have externally resolvable overloads
- * for void * operator new - which means in the .o for the binary,
- * or in a shared library. static libs don't propogate the symbol
- * so, look in the translation unit containing main() in squid
- * for the extern version in squid
- */
-#if !defined(_SQUID_EXTERNNEW_)
-#if defined(__GNUC_STDC_INLINE__) || defined(__GNUC_GNU_INLINE__)
-#define _SQUID_EXTERNNEW_ extern inline __attribute__((gnu_inline))
-#else
-#define _SQUID_EXTERNNEW_ extern inline
-#endif
-#endif
-#include "SquidNew.h"
-#endif
 
 SQUIDCEXTERN time_t parse_iso3307_time(const char *buf);
 
 SQUIDCEXTERN double xpercent(double part, double whole);
 SQUIDCEXTERN int xpercentInt(double part, double whole);
 SQUIDCEXTERN double xdiv(double nom, double denom);
 
 SQUIDCEXTERN const char *xitoa(int num);
 SQUIDCEXTERN const char *xint64toa(int64_t num);
 
 typedef struct {
     size_t count;
     size_t bytes;
     size_t gb;
 } gb_t;
 
 /* gb_type operations */
 #define gb_flush_limit (0x3FFFFFFF)
 #define gb_inc(gb, delta) { if ((gb)->bytes > gb_flush_limit || delta > gb_flush_limit) gb_flush(gb); (gb)->bytes += delta; (gb)->count++; }
 #define gb_incb(gb, delta) { if ((gb)->bytes > gb_flush_limit || delta > gb_flush_limit) gb_flush(gb); (gb)->bytes += delta; }

=== modified file 'src/SquidNew.cc'
--- src/SquidNew.cc	2016-01-01 00:12:18 +0000
+++ src/SquidNew.cc	2016-04-07 06:09:43 +0000
@@ -1,36 +1,51 @@
 /*
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 /* DEBUG: none          Memory Allocation */
 
-#define _SQUID_EXTERNNEW_
-
 #include "squid.h"
 
-#ifdef __SUNPRO_CC
+#if !defined(__clang__)
 
 #include <new>
-void *operator new(size_t size) throw (std::bad_alloc)
+
+void *operator new(size_t size)
 {
     return xmalloc(size);
 }
-void operator delete (void *address) throw()
+void operator delete (void *address)
 {
     xfree (address);
 }
-void *operator new[] (size_t size) throw (std::bad_alloc)
+void *operator new[] (size_t size)
 {
     return xmalloc(size);
 }
-void operator delete[] (void *address) throw()
+void operator delete[] (void *address)
 {
     xfree (address);
 }
 
-#endif /* __SUNPRO_CC */
+void *operator new(size_t size, const std::nothrow_t &tag)
+{
+    return xmalloc(size);
+}
+void operator delete (void *address, const std::nothrow_t &tag)
+{
+    xfree(address);
+}
+void *operator new[] (size_t size, const std::nothrow_t &tag)
+{
+    return xmalloc(size);
+}
+void operator delete[] (void *address, const std::nothrow_t &tag)
+{
+    xfree(address);
+}
 
+#endif /* !defined(__clang__) */

_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to