public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix for PR68159 in Libiberty Demangler (6)
@ 2016-05-06  6:14 Marcel Böhme
  2016-05-06  7:10 ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Marcel Böhme @ 2016-05-06  6:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Bernd Schmidt

Hi,

This patches fixes 
* the stack overflow reported in PR68159 in cplus_demangle_print_callback,
* a potential stack overflow in d_demangle_callback
* a potential stack overflow in is_ctor_or_dtor, and
* six potential buffer overflows (initialise less memory than needed due to integer overflow).

The stack overflow reported in PR68159 occurs due to assigning an array too much memory from the stack (447kb).
Similar stack overflows might occur in the remaining five dynamically allocated arrays in this and the other two functions.

Since the array size is controlled from the mangled string, we better safeguard from integer overflows and thus buffer overflows for these six arrays.

The patch allocates memory from the heap (xmalloc) instead of from the stack (dynamic arrays, alloca), checks for integer overflows, and frees the allocated memory before function return / abort.

Best regards,
- Marcel


Index: ChangeLog
===================================================================
--- ChangeLog	(revision 235941)
+++ ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  <boehme.marcel@gmail.com>
+
+	PR c++/68159
+	* cp-demangle.c: Check for overflow and allocate arrays of user-defined 
+	size on the heap, not on the stack.
+	(CP_DYNAMIC_ARRAYS): Remove redundant definition.
+	(cplus_demangle_print_callback): Check for overflow. Allocate memory 
+	for two arrays on the heap. Free memory before return / exit.
+	(d_demangle_callback): Likewise.
+	(is_ctor_or_dtor): Likewise. 
+
 2016-05-02  Marcel Böhme  <boehme.marcel@gmail.com>
 
 	PR c++/70498
Index: cp-demangle.c
===================================================================
--- cp-demangle.c	(revision 235941)
+++ cp-demangle.c	(working copy)
@@ -186,20 +186,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
    additional unresolved symbols when this code is used in a library.
    FIXME: Is this really a valid reason?  This comes from the original
@@ -4125,26 +4111,26 @@ cplus_demangle_print_callback (int options,
   struct d_print_info dpi;
 
   d_print_init (&dpi, callback, opaque, dc);
+  
+  if (dpi.num_copy_templates > INT_MAX / (int) sizeof (*dpi.copy_templates))
+    xmalloc_failed(INT_MAX);
+  dpi.copy_templates = 
+      (struct d_print_template *) xmalloc (dpi.num_copy_templates 
+			                  * sizeof (*dpi.copy_templates));
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-    __extension__ struct d_print_template temps[dpi.num_copy_templates];
+  if (dpi.num_saved_scopes > INT_MAX / (int) sizeof (*dpi.saved_scopes))
+    xmalloc_failed(INT_MAX);
+  dpi.saved_scopes = 
+      (struct d_saved_scope *) xmalloc (dpi.num_saved_scopes 
+			                * sizeof (*dpi.saved_scopes));
 
-    dpi.saved_scopes = scopes;
-    dpi.copy_templates = temps;
-#else
-    dpi.saved_scopes = alloca (dpi.num_saved_scopes
-			       * sizeof (*dpi.saved_scopes));
-    dpi.copy_templates = alloca (dpi.num_copy_templates
-				 * sizeof (*dpi.copy_templates));
-#endif
+  d_print_comp (&dpi, options, dc);
 
-    d_print_comp (&dpi, options, dc);
-  }
-
   d_print_flush (&dpi);
 
+  free(dpi.copy_templates);
+  free(dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,18 +5931,17 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  if (di.num_comps > INT_MAX / (int) sizeof (*di.comps))
+    xmalloc_failed(INT_MAX);
+  di.comps = (struct demangle_component *) xmalloc (di.num_comps 
+						    * sizeof (*di.comps));
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  if (di.num_subs > INT_MAX / (int) sizeof (*di.subs))
+    xmalloc_failed(INT_MAX);
+  di.subs = (struct demangle_component **) xmalloc (di.num_subs 
+						   * sizeof (*di.subs));
 
+  {
     switch (type)
       {
       case DCT_TYPE:
@@ -5977,6 +5962,8 @@ d_demangle_callback (const char *mangled, int opti
 	d_advance (&di, strlen (d_str (&di)));
 	break;
       default:
+	free (di.comps);
+	free (di.subs);
 	abort (); /* We have listed all the cases.  */
       }
 
@@ -5995,7 +5982,8 @@ d_demangle_callback (const char *mangled, int opti
              ? cplus_demangle_print_callback (options, dc, callback, opaque)
              : 0;
   }
-
+  free (di.comps);
+  free (di.subs);
   return status;
 }
 
@@ -6226,18 +6214,17 @@ is_ctor_or_dtor (const char *mangled,
 
   cplus_demangle_init_info (mangled, DMGL_GNU_V3, strlen (mangled), &di);
 
+  if (di.num_comps > INT_MAX / (int) sizeof (*di.comps))
+    xmalloc_failed(INT_MAX);
+  di.comps = (struct demangle_component *) xmalloc (di.num_comps 
+						    * sizeof (*di.comps));
+
+  if (di.num_subs > INT_MAX / (int) sizeof (*di.subs))
+    xmalloc_failed(INT_MAX);
+  di.subs = (struct demangle_component **) xmalloc (di.num_subs 
+						   * sizeof (*di.subs));
   {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
-
     dc = cplus_demangle_mangled_name (&di, 1);
 
     /* Note that because we did not pass DMGL_PARAMS, we don't expect
@@ -6279,7 +6266,8 @@ is_ctor_or_dtor (const char *mangled,
 	  }
       }
   }
-
+  free (di.comps);
+  free (di.subs);
   return ret;
 }
 
Index: testsuite/demangle-expected
===================================================================
--- testsuite/demangle-expected	(revision 235941)
+++ testsuite/demangle-expected	(working copy)
@@ -4441,3 +4441,8 @@ __vt_90000000000cafebabe
 
 _Z80800000000000000000000
 _Z80800000000000000000000
+#
+# Tests stack overflow
+
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06  6:14 Fix for PR68159 in Libiberty Demangler (6) Marcel Böhme
@ 2016-05-06  7:10 ` Jakub Jelinek
  2016-05-06  9:01   ` Marcel Böhme
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2016-05-06  7:10 UTC (permalink / raw)
  To: Marcel Böhme, Jason Merrill, Jonathan Wakely
  Cc: gcc-patches, Bernd Schmidt

On Fri, May 06, 2016 at 02:14:31PM +0800, Marcel Böhme wrote:
> * the stack overflow reported in PR68159 in cplus_demangle_print_callback,
> * a potential stack overflow in d_demangle_callback
> * a potential stack overflow in is_ctor_or_dtor, and
> * six potential buffer overflows (initialise less memory than needed due to integer overflow).
> 
> The stack overflow reported in PR68159 occurs due to assigning an array too much memory from the stack (447kb).
> Similar stack overflows might occur in the remaining five dynamically allocated arrays in this and the other two functions.
> 
> Since the array size is controlled from the mangled string, we better safeguard from integer overflows and thus buffer overflows for these six arrays.
> 
> The patch allocates memory from the heap (xmalloc) instead of from the stack (dynamic arrays, alloca), checks for integer overflows, and frees the allocated memory before function return / abort.

I think this is very problematic, but I'll let others comment on in detail.

Have you tested the patch at all?

This file is used not just in the various tools like binutils or gdb, but
also in libstdc++, where it used e.g. in the std::terminate handler,
which I think can't just xmalloc_failed, that function can be called already
in out of memory situation, where heap allocation is not possible.

Furthermore, if I apply your patch and rebuild libstdc++, it stops working
altogether:
ldd -d -r ./libstdc++.so.6.0.22 
	linux-vdso.so.1 (0x00007ffe6053c000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f9de23fb000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f9de203a000)
	/lib64/ld-linux-x86-64.so.2 (0x00005585ecc1d000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f9de1e22000)
undefined symbol: xmalloc	(./libstdc++.so.6.0.22)
undefined symbol: xmalloc_failed	(./libstdc++.so.6.0.22)
(which is why I'm asking about what testing you've done).

> @@ -4125,26 +4111,26 @@ cplus_demangle_print_callback (int options,
>    struct d_print_info dpi;
>  
>    d_print_init (&dpi, callback, opaque, dc);
> +  
> +  if (dpi.num_copy_templates > INT_MAX / (int) sizeof (*dpi.copy_templates))
> +    xmalloc_failed(INT_MAX);
> +  dpi.copy_templates = 
> +      (struct d_print_template *) xmalloc (dpi.num_copy_templates 
> +			                  * sizeof (*dpi.copy_templates));

Ignoring the fact that this patch breaks libstdc++, there are also
formatting and other issues.  Missing space in between xmalloc_failed
and (, = should not appear at the end of line, but on the start of
next line and it should be indented by 2, not 4.  Why INT_MAX?
I'd have thought that the allocation size is computed in size_t and
thus it should be SIZE_MAX, (~(size_t) 0) or similar?

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06  7:10 ` Jakub Jelinek
@ 2016-05-06  9:01   ` Marcel Böhme
  2016-05-06  9:52     ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Marcel Böhme @ 2016-05-06  9:01 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, Jonathan Wakely, gcc-patches, Bernd Schmidt

Hi Jakub,

The patch that is attached now is bootstrapped and regression tested on x86_64-pc-linux-gnu.

> 
> This file is used not just in the various tools like binutils or gdb, but
> also in libstdc++, where it used e.g. in the std::terminate handler,
> which I think can't just xmalloc_failed, that function can be called already
> in out of memory situation, where heap allocation is not possible.

Earlier, I was working on libiberty/cplus-dem.c where xmalloc was explicitly available. So, I assumed it would be in libiberty/cp-demangle.c as well.

> Why INT_MAX?
> I'd have thought that the allocation size is computed in size_t and
> thus it should be SIZE_MAX, (~(size_t) 0) or similar?

In two separate patches (the first in cplus-dem.c and the second in cp-demangle.c) it was decided that we should import limit.h and otherwise define INT_MAX, then check against INT_MAX.
However, I removed the overflow check since it is not clear what the behaviour should be when the integer actually overflows. Apparently, it can’t abort. Still, this remains an unresolved security concern if actually inputs can actually be generated that result in overflow.

I also fixed some indentation issues caused by the removal of in-the-middle-of-the-method variable declarations.

-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-    __extension__ struct d_print_template temps[dpi.num_copy_templates];

Let me know if there are more concerns. There might be some more formatting issues lingering.

Best regards,
- Marcel


Index: ChangeLog
===================================================================
--- ChangeLog	(revision 235941)
+++ ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  <boehme.marcel@gmail.com>
+
+	PR c++/68159
+	* cp-demangle.c: Allocate arrays of user-defined size on the heap,
+	not on the stack.
+	(CP_DYNAMIC_ARRAYS): Remove redundant definition.
+	(cplus_demangle_print_callback): Allocate memory for two arrays on
+	the heap. Free memory before return / exit.
+	(d_demangle_callback): Likewise.
+	(is_ctor_or_dtor): Likewise. 
+
 2016-05-02  Marcel Böhme  <boehme.marcel@gmail.com>
 
 	PR c++/70498
Index: cp-demangle.c
===================================================================
--- cp-demangle.c	(revision 235941)
+++ cp-demangle.c	(working copy)
@@ -186,20 +186,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
    additional unresolved symbols when this code is used in a library.
    FIXME: Is this really a valid reason?  This comes from the original
@@ -4125,26 +4111,20 @@ cplus_demangle_print_callback (int options,
   struct d_print_info dpi;
 
   d_print_init (&dpi, callback, opaque, dc);
+  
+  dpi.copy_templates = (struct d_print_template *)
+      malloc (dpi.num_copy_templates * sizeof (*dpi.copy_templates));
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-    __extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.saved_scopes = (struct d_saved_scope *) 
+      malloc (dpi.num_saved_scopes * sizeof (*dpi.saved_scopes));
 
-    dpi.saved_scopes = scopes;
-    dpi.copy_templates = temps;
-#else
-    dpi.saved_scopes = alloca (dpi.num_saved_scopes
-			       * sizeof (*dpi.saved_scopes));
-    dpi.copy_templates = alloca (dpi.num_copy_templates
-				 * sizeof (*dpi.copy_templates));
-#endif
+  d_print_comp (&dpi, options, dc);
 
-    d_print_comp (&dpi, options, dc);
-  }
-
   d_print_flush (&dpi);
 
+  free(dpi.copy_templates);
+  free(dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,57 +5925,54 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (di.num_comps 
+						    * sizeof (*di.comps));
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (di.num_subs 
+						   * sizeof (*di.subs));
 
-    switch (type)
-      {
-      case DCT_TYPE:
-	dc = cplus_demangle_type (&di);
-	break;
-      case DCT_MANGLED:
-	dc = cplus_demangle_mangled_name (&di, 1);
-	break;
-      case DCT_GLOBAL_CTORS:
-      case DCT_GLOBAL_DTORS:
-	d_advance (&di, 11);
-	dc = d_make_comp (&di,
-			  (type == DCT_GLOBAL_CTORS
-			   ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
-			   : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
-			  d_make_demangle_mangled_name (&di, d_str (&di)),
-			  NULL);
-	d_advance (&di, strlen (d_str (&di)));
-	break;
-      default:
-	abort (); /* We have listed all the cases.  */
-      }
+  switch (type)
+    {
+    case DCT_TYPE:
+      dc = cplus_demangle_type (&di);
+      break;
+    case DCT_MANGLED:
+      dc = cplus_demangle_mangled_name (&di, 1);
+      break;
+    case DCT_GLOBAL_CTORS:
+    case DCT_GLOBAL_DTORS:
+      d_advance (&di, 11);
+      dc = d_make_comp (&di,
+			(type == DCT_GLOBAL_CTORS
+			 ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
+			 : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
+			d_make_demangle_mangled_name (&di, d_str (&di)),
+			NULL);
+      d_advance (&di, strlen (d_str (&di)));
+      break;
+    default:
+      free (di.comps);
+      free (di.subs);
+      abort (); /* We have listed all the cases.  */
+    }
 
-    /* If DMGL_PARAMS is set, then if we didn't consume the entire
-       mangled string, then we didn't successfully demangle it.  If
-       DMGL_PARAMS is not set, we didn't look at the trailing
-       parameters.  */
-    if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
-      dc = NULL;
+  /* If DMGL_PARAMS is set, then if we didn't consume the entire
+     mangled string, then we didn't successfully demangle it.  If
+     DMGL_PARAMS is not set, we didn't look at the trailing
+     parameters.  */
+  if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
+    dc = NULL;
 
 #ifdef CP_DEMANGLE_DEBUG
-    d_dump (dc, 0);
+  d_dump (dc, 0);
 #endif
 
-    status = (dc != NULL)
-             ? cplus_demangle_print_callback (options, dc, callback, opaque)
-             : 0;
-  }
-
+  status = (dc != NULL)
+           ? cplus_demangle_print_callback (options, dc, callback, opaque)
+           : 0;
+  
+  free (di.comps);
+  free (di.subs);
   return status;
 }
 
@@ -6226,60 +6203,55 @@ is_ctor_or_dtor (const char *mangled,
 
   cplus_demangle_init_info (mangled, DMGL_GNU_V3, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (di.num_comps 
+						    * sizeof (*di.comps));
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (di.num_subs 
+						   * sizeof (*di.subs));
 
-    dc = cplus_demangle_mangled_name (&di, 1);
+  dc = cplus_demangle_mangled_name (&di, 1);
 
-    /* Note that because we did not pass DMGL_PARAMS, we don't expect
-       to demangle the entire string.  */
+  /* Note that because we did not pass DMGL_PARAMS, we don't expect
+     to demangle the entire string.  */
 
-    ret = 0;
-    while (dc != NULL)
-      {
-	switch (dc->type)
-	  {
-	    /* These cannot appear on a constructor or destructor.  */
-	  case DEMANGLE_COMPONENT_RESTRICT_THIS:
-	  case DEMANGLE_COMPONENT_VOLATILE_THIS:
-	  case DEMANGLE_COMPONENT_CONST_THIS:
-	  case DEMANGLE_COMPONENT_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
-	  default:
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_TYPED_NAME:
-	  case DEMANGLE_COMPONENT_TEMPLATE:
-	    dc = d_left (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_QUAL_NAME:
-	  case DEMANGLE_COMPONENT_LOCAL_NAME:
-	    dc = d_right (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_CTOR:
-	    *ctor_kind = dc->u.s_ctor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_DTOR:
-	    *dtor_kind = dc->u.s_dtor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  }
-      }
-  }
+  ret = 0;
+  while (dc != NULL)
+    {
+      switch (dc->type)
+	{
+	  /* These cannot appear on a constructor or destructor.  */
+	case DEMANGLE_COMPONENT_RESTRICT_THIS:
+	case DEMANGLE_COMPONENT_VOLATILE_THIS:
+	case DEMANGLE_COMPONENT_CONST_THIS:
+	case DEMANGLE_COMPONENT_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
+	default:
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_TYPED_NAME:
+	case DEMANGLE_COMPONENT_TEMPLATE:
+	  dc = d_left (dc);
+	  break;
+	case DEMANGLE_COMPONENT_QUAL_NAME:
+	case DEMANGLE_COMPONENT_LOCAL_NAME:
+	  dc = d_right (dc);
+	  break;
+	case DEMANGLE_COMPONENT_CTOR:
+	  *ctor_kind = dc->u.s_ctor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_DTOR:
+	  *dtor_kind = dc->u.s_dtor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	}
+    }
 
+  free (di.comps);
+  free (di.subs);
   return ret;
 }
 
Index: testsuite/demangle-expected
===================================================================
--- testsuite/demangle-expected	(revision 235941)
+++ testsuite/demangle-expected	(working copy)
@@ -4441,3 +4441,8 @@ __vt_90000000000cafebabe
 
 _Z80800000000000000000000
 _Z80800000000000000000000
+#
+# Tests stack overflow
+
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06  9:01   ` Marcel Böhme
@ 2016-05-06  9:52     ` Jakub Jelinek
  2016-05-06 14:46       ` Marcel Böhme
  2016-05-06 16:17       ` Ian Lance Taylor
  0 siblings, 2 replies; 15+ messages in thread
From: Jakub Jelinek @ 2016-05-06  9:52 UTC (permalink / raw)
  To: Marcel Böhme, Jason Merrill, Ian Lance Taylor
  Cc: Jonathan Wakely, gcc-patches, Bernd Schmidt

On Fri, May 06, 2016 at 05:01:14PM +0800, Marcel Böhme wrote:
> The patch that is attached now is bootstrapped and regression tested on x86_64-pc-linux-gnu.
> 
> > 
> > This file is used not just in the various tools like binutils or gdb, but
> > also in libstdc++, where it used e.g. in the std::terminate handler,
> > which I think can't just xmalloc_failed, that function can be called already
> > in out of memory situation, where heap allocation is not possible.
> 
> Earlier, I was working on libiberty/cplus-dem.c where xmalloc was explicitly available. So, I assumed it would be in libiberty/cp-demangle.c as well.
> 
> > Why INT_MAX?
> > I'd have thought that the allocation size is computed in size_t and
> > thus it should be SIZE_MAX, (~(size_t) 0) or similar?
> 
> In two separate patches (the first in cplus-dem.c and the second in cp-demangle.c) it was decided that we should import limit.h and otherwise define INT_MAX, then check against INT_MAX.
> However, I removed the overflow check since it is not clear what the behaviour should be when the integer actually overflows. Apparently, it can’t abort. Still, this remains an unresolved security concern if actually inputs can actually be generated that result in overflow.

If you just want an array, restricting the size including the sizeof
to fit into int makes no sense, you want to guard it against overflows
during the multiplication.

Anyway, perhaps I'm misremembering, if there is a mode that really can't
fail due to allocation failures or not, we need to deal with that.
Ian or Jason, can all the demangle users allocate heap memory or not?
And, if __cxa_demangle can fail, there is some allocation_failure stuff
in the file.

> @@ -4125,26 +4111,20 @@ cplus_demangle_print_callback (int options,
>    struct d_print_info dpi;
>  
>    d_print_init (&dpi, callback, opaque, dc);
> +  
> +  dpi.copy_templates = (struct d_print_template *)
> +      malloc (dpi.num_copy_templates * sizeof (*dpi.copy_templates));

The indentation is still wrong.  Either malloc would need to be below (struct
or it should be
  dpi.copy_templates
    = (struct d_print_template *) malloc (...)
But much more importantly, you don't handle the allocation failure in
anyway, so if malloc fails, you'll just segfault.

> +  dpi.saved_scopes = (struct d_saved_scope *) 
> +      malloc (dpi.num_saved_scopes * sizeof (*dpi.saved_scopes));

See above.
>  
> +  free(dpi.copy_templates);
> +  free(dpi.saved_scopes);

Formatting, missing space before (.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06  9:52     ` Jakub Jelinek
@ 2016-05-06 14:46       ` Marcel Böhme
  2016-05-06 14:49         ` Jakub Jelinek
  2016-05-06 16:17       ` Ian Lance Taylor
  1 sibling, 1 reply; 15+ messages in thread
From: Marcel Böhme @ 2016-05-06 14:46 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Jason Merrill, Ian Lance Taylor, Jonathan Wakely, gcc-patches,
	Bernd Schmidt

Hi Jakub,

> On 6 May 2016, at 5:51 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> 
> 
> If you just want an array, restricting the size including the sizeof
> to fit into int makes no sense, you want to guard it against overflows
> during the multiplication.
Okay, done. (Someone might want to substitute size_t with unsigned int if the former causes any problems).

> Ian or Jason, can all the demangle users allocate heap memory or not?
This question remains.

> But much more importantly, you don't handle the allocation failure in
> anyway, so if malloc fails, you'll just segfault.
It is handled now. No abort. No overflow.

I also checked: Even if num_saved_scopes or num_copy_templates happen to overflow in d_count_templates_scopes, that integer overflow won’t lead to a buffer overflow because of the checks (and only their uses) in lines 4292 and 4307.
4292:  if (dpi->next_saved_scope >= dpi->num_saved_scopes)
4293:     {
4294:       d_print_error (dpi);
4295:       return;
4296:     }

4307:  if (dpi->next_saved_scope >= dpi->num_saved_scopes)
4307:    {
4307:      d_print_error (dpi);
4307:      return;
4307:    }

As for your previous email:
> On 6 May 2016, at 3:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> Furthermore, if I apply your patch and rebuild libstdc++, it stops working
> altogether:
> ldd -d -r ./libstdc++.so.6.0.22 
> 	linux-vdso.so.1 (0x00007ffe6053c000)
> 	libm.so.6 => /lib64/libm.so.6 (0x00007f9de23fb000)
> 	libc.so.6 => /lib64/libc.so.6 (0x00007f9de203a000)
> 	/lib64/ld-linux-x86-64.so.2 (0x00005585ecc1d000)
> 	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f9de1e22000)
> undefined symbol: xmalloc	(./libstdc++.so.6.0.22)
> undefined symbol: xmalloc_failed	(./libstdc++.so.6.0.22)

Hmm. Just checked, xmalloc should be available through libiberty.h which is imported by cp-demangle.c. 
That earlier patch was successfully bootstrapped and regression tested on on x86_64-pc-linux-gnu from the sources in trunk.

BTW: If I configure libstdc++-v3 directly, I receive an error message:
...
./config.status: creating include/Makefile
./config.status: line 2950: ./../../config-ml.in: No such file or directory                                           

Best regards,
- Marcel


Index: libiberty/ChangeLog
===================================================================
--- libiberty/ChangeLog	(revision 235962)
+++ libiberty/ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  <boehme.marcel@gmail.com>
+
+	PR c++/68159
+	* cp-demangle.c: Allocate arrays of user-defined size on the heap,
+	not on the stack. Do not include <alloca.h>.
+	(CP_DYNAMIC_ARRAYS): Remove definition.
+	(cplus_demangle_print_callback): Allocate memory for two arrays on
+	the heap. Free memory before return / exit.
+	(d_demangle_callback): Likewise.
+	(is_ctor_or_dtor): Likewise. 
+
 2016-05-02  Marcel Böhme  <boehme.marcel@gmail.com>
 
 	PR c++/70498
Index: libiberty/cp-demangle.c
===================================================================
--- libiberty/cp-demangle.c	(revision 235962)
+++ libiberty/cp-demangle.c	(working copy)
@@ -116,18 +116,6 @@
 #include <string.h>
 #endif
 
-#ifdef HAVE_ALLOCA_H
-# include <alloca.h>
-#else
-# ifndef alloca
-#  ifdef __GNUC__
-#   define alloca __builtin_alloca
-#  else
-extern char *alloca ();
-#  endif /* __GNUC__ */
-# endif /* alloca */
-#endif /* HAVE_ALLOCA_H */
-
 #ifdef HAVE_LIMITS_H
 #include <limits.h>
 #endif
@@ -186,20 +174,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
    additional unresolved symbols when this code is used in a library.
    FIXME: Is this really a valid reason?  This comes from the original
@@ -4126,25 +4100,26 @@ cplus_demangle_print_callback (int options,
 
   d_print_init (&dpi, callback, opaque, dc);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-    __extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.copy_templates
+    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
+					  * sizeof (*dpi.copy_templates));
+  dpi.saved_scopes
+    = (struct d_saved_scope *) malloc (((size_t) dpi.num_saved_scopes) 
+				       * sizeof (*dpi.saved_scopes));
+  
+  if (! dpi.copy_templates || ! dpi.saved_scopes)
+    {
+      d_print_error (&dpi);
+      return 0;
+    }
 
-    dpi.saved_scopes = scopes;
-    dpi.copy_templates = temps;
-#else
-    dpi.saved_scopes = alloca (dpi.num_saved_scopes
-			       * sizeof (*dpi.saved_scopes));
-    dpi.copy_templates = alloca (dpi.num_copy_templates
-				 * sizeof (*dpi.copy_templates));
-#endif
+  d_print_comp (&dpi, options, dc);
 
-    d_print_comp (&dpi, options, dc);
-  }
-
   d_print_flush (&dpi);
 
+  free (dpi.copy_templates);
+  free (dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,57 +5920,56 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+						   * sizeof (*di.comps));
+  di.subs = (struct demangle_component **) malloc (((size_t) di.num_subs) 
+						   * sizeof (*di.subs));
+  
+  if (! di.comps || ! di.subs)
+    return 0;
+    
+  switch (type)
+    {
+    case DCT_TYPE:
+      dc = cplus_demangle_type (&di);
+      break;
+    case DCT_MANGLED:
+      dc = cplus_demangle_mangled_name (&di, 1);
+      break;
+    case DCT_GLOBAL_CTORS:
+    case DCT_GLOBAL_DTORS:
+      d_advance (&di, 11);
+      dc = d_make_comp (&di,
+			(type == DCT_GLOBAL_CTORS
+			 ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
+			 : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
+			d_make_demangle_mangled_name (&di, d_str (&di)),
+			NULL);
+      d_advance (&di, strlen (d_str (&di)));
+      break;
+    default:
+      free (di.comps);
+      free (di.subs);
+      abort (); /* We have listed all the cases.  */
+    }
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  /* If DMGL_PARAMS is set, then if we didn't consume the entire
+     mangled string, then we didn't successfully demangle it.  If
+     DMGL_PARAMS is not set, we didn't look at the trailing
+     parameters.  */
+  if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
+    dc = NULL;
 
-    switch (type)
-      {
-      case DCT_TYPE:
-	dc = cplus_demangle_type (&di);
-	break;
-      case DCT_MANGLED:
-	dc = cplus_demangle_mangled_name (&di, 1);
-	break;
-      case DCT_GLOBAL_CTORS:
-      case DCT_GLOBAL_DTORS:
-	d_advance (&di, 11);
-	dc = d_make_comp (&di,
-			  (type == DCT_GLOBAL_CTORS
-			   ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
-			   : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
-			  d_make_demangle_mangled_name (&di, d_str (&di)),
-			  NULL);
-	d_advance (&di, strlen (d_str (&di)));
-	break;
-      default:
-	abort (); /* We have listed all the cases.  */
-      }
-
-    /* If DMGL_PARAMS is set, then if we didn't consume the entire
-       mangled string, then we didn't successfully demangle it.  If
-       DMGL_PARAMS is not set, we didn't look at the trailing
-       parameters.  */
-    if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
-      dc = NULL;
-
 #ifdef CP_DEMANGLE_DEBUG
-    d_dump (dc, 0);
+  d_dump (dc, 0);
 #endif
 
-    status = (dc != NULL)
-             ? cplus_demangle_print_callback (options, dc, callback, opaque)
-             : 0;
-  }
-
+  status = (dc != NULL)
+           ? cplus_demangle_print_callback (options, dc, callback, opaque)
+           : 0;
+  
+  free (di.comps);
+  free (di.subs);
   return status;
 }
 
@@ -6226,60 +6200,57 @@ is_ctor_or_dtor (const char *mangled,
 
   cplus_demangle_init_info (mangled, DMGL_GNU_V3, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+						   * sizeof (*di.comps));
+  di.subs = (struct demangle_component **) malloc (((size_t) di.num_subs) 
+						   * sizeof (*di.subs));
+  
+  if (! di.comps || ! di.subs)
+    return 0;
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  dc = cplus_demangle_mangled_name (&di, 1);
 
-    dc = cplus_demangle_mangled_name (&di, 1);
+  /* Note that because we did not pass DMGL_PARAMS, we don't expect
+     to demangle the entire string.  */
 
-    /* Note that because we did not pass DMGL_PARAMS, we don't expect
-       to demangle the entire string.  */
+  ret = 0;
+  while (dc != NULL)
+    {
+      switch (dc->type)
+	{
+	  /* These cannot appear on a constructor or destructor.  */
+	case DEMANGLE_COMPONENT_RESTRICT_THIS:
+	case DEMANGLE_COMPONENT_VOLATILE_THIS:
+	case DEMANGLE_COMPONENT_CONST_THIS:
+	case DEMANGLE_COMPONENT_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
+	default:
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_TYPED_NAME:
+	case DEMANGLE_COMPONENT_TEMPLATE:
+	  dc = d_left (dc);
+	  break;
+	case DEMANGLE_COMPONENT_QUAL_NAME:
+	case DEMANGLE_COMPONENT_LOCAL_NAME:
+	  dc = d_right (dc);
+	  break;
+	case DEMANGLE_COMPONENT_CTOR:
+	  *ctor_kind = dc->u.s_ctor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_DTOR:
+	  *dtor_kind = dc->u.s_dtor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	}
+    }
 
-    ret = 0;
-    while (dc != NULL)
-      {
-	switch (dc->type)
-	  {
-	    /* These cannot appear on a constructor or destructor.  */
-	  case DEMANGLE_COMPONENT_RESTRICT_THIS:
-	  case DEMANGLE_COMPONENT_VOLATILE_THIS:
-	  case DEMANGLE_COMPONENT_CONST_THIS:
-	  case DEMANGLE_COMPONENT_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
-	  default:
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_TYPED_NAME:
-	  case DEMANGLE_COMPONENT_TEMPLATE:
-	    dc = d_left (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_QUAL_NAME:
-	  case DEMANGLE_COMPONENT_LOCAL_NAME:
-	    dc = d_right (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_CTOR:
-	    *ctor_kind = dc->u.s_ctor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_DTOR:
-	    *dtor_kind = dc->u.s_dtor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  }
-      }
-  }
-
+  free (di.comps);
+  free (di.subs);
   return ret;
 }
 
Index: libiberty/testsuite/demangle-expected
===================================================================
--- libiberty/testsuite/demangle-expected	(revision 235962)
+++ libiberty/testsuite/demangle-expected	(working copy)
@@ -4441,3 +4441,8 @@ __vt_90000000000cafebabe
 
 _Z80800000000000000000000
 _Z80800000000000000000000
+#
+# Tests stack overflow
+
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06 14:46       ` Marcel Böhme
@ 2016-05-06 14:49         ` Jakub Jelinek
  2016-05-06 15:11           ` Marcel Böhme
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2016-05-06 14:49 UTC (permalink / raw)
  To: Marcel Böhme
  Cc: Jason Merrill, Ian Lance Taylor, Jonathan Wakely, gcc-patches,
	Bernd Schmidt

On Fri, May 06, 2016 at 10:46:12PM +0800, Marcel Böhme wrote:
>    d_print_init (&dpi, callback, opaque, dc);
>  
> -  {
> -#ifdef CP_DYNAMIC_ARRAYS
> -    __extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
> -    __extension__ struct d_print_template temps[dpi.num_copy_templates];
> +  dpi.copy_templates
> +    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
> +					  * sizeof (*dpi.copy_templates));
> +  dpi.saved_scopes
> +    = (struct d_saved_scope *) malloc (((size_t) dpi.num_saved_scopes) 
> +				       * sizeof (*dpi.saved_scopes));
> +  
> +  if (! dpi.copy_templates || ! dpi.saved_scopes)
> +    {
> +      d_print_error (&dpi);
> +      return 0;
> +    }

If one malloc succeeds and the other fails, you leak memory.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06 14:49         ` Jakub Jelinek
@ 2016-05-06 15:11           ` Marcel Böhme
  2016-05-06 15:19             ` Jakub Jelinek
  2016-05-06 16:05             ` Marcel Böhme
  0 siblings, 2 replies; 15+ messages in thread
From: Marcel Böhme @ 2016-05-06 15:11 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Jason Merrill, Ian Lance Taylor, Jonathan Wakely, gcc-patches,
	Bernd Schmidt


> If one malloc succeeds and the other fails, you leak memory.
> 
> 	Jakub
Nice catch. Thanks!

Bootstrapped and regression tested on x86_64-pc-linux-gnu.

Best - Marcel

Index: libiberty/ChangeLog
===================================================================
--- libiberty/ChangeLog	(revision 235962)
+++ libiberty/ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  <boehme.marcel@gmail.com>
+
+	PR c++/68159
+	* cp-demangle.c: Allocate arrays of user-defined size on the heap,
+	not on the stack. Do not include <alloca.h>.
+	(CP_DYNAMIC_ARRAYS): Remove definition.
+	(cplus_demangle_print_callback): Allocate memory for two arrays on
+	the heap. Free memory before return / exit.
+	(d_demangle_callback): Likewise.
+	(is_ctor_or_dtor): Likewise.
+	* testsuite/demangle-expected: Add regression test cases.
+
2016-05-02  Marcel Böhme  <boehme.marcel@gmail.com>

	PR c++/70498
Index: libiberty/cp-demangle.c
===================================================================
--- libiberty/cp-demangle.c	(revision 235962)
+++ libiberty/cp-demangle.c	(working copy)
@@ -116,18 +116,6 @@
 #include <string.h>
 #endif
 
-#ifdef HAVE_ALLOCA_H
-# include <alloca.h>
-#else
-# ifndef alloca
-#  ifdef __GNUC__
-#   define alloca __builtin_alloca
-#  else
-extern char *alloca ();
-#  endif /* __GNUC__ */
-# endif /* alloca */
-#endif /* HAVE_ALLOCA_H */
-
 #ifdef HAVE_LIMITS_H
 #include <limits.h>
 #endif
@@ -186,20 +174,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
    additional unresolved symbols when this code is used in a library.
    FIXME: Is this really a valid reason?  This comes from the original
@@ -4126,25 +4100,31 @@ cplus_demangle_print_callback (int options,
 
   d_print_init (&dpi, callback, opaque, dc);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-    __extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.copy_templates
+    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
+					  * sizeof (*dpi.copy_templates));
+  if (! dpi.copy_templates)
+    {
+      d_print_error (&dpi);
+      return 0;
+    }
 
-    dpi.saved_scopes = scopes;
-    dpi.copy_templates = temps;
-#else
-    dpi.saved_scopes = alloca (dpi.num_saved_scopes
-			       * sizeof (*dpi.saved_scopes));
-    dpi.copy_templates = alloca (dpi.num_copy_templates
-				 * sizeof (*dpi.copy_templates));
-#endif
+  dpi.saved_scopes
+    = (struct d_saved_scope *) malloc (((size_t) dpi.num_saved_scopes) 
+				       * sizeof (*dpi.saved_scopes));  
+  if (! dpi.saved_scopes)
+    {
+      d_print_error (&dpi);
+      return 0;
+    }
 
-    d_print_comp (&dpi, options, dc);
-  }
+  d_print_comp (&dpi, options, dc);
 
   d_print_flush (&dpi);
 
+  free (dpi.copy_templates);
+  free (dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,57 +5925,58 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+						   * sizeof (*di.comps));
+  if (! di.comps)
+    return 0;
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (((size_t) di.num_subs) 
+						   * sizeof (*di.subs));  
+  if (! di.subs)
+    return 0;
+    
+  switch (type)
+    {
+    case DCT_TYPE:
+      dc = cplus_demangle_type (&di);
+      break;
+    case DCT_MANGLED:
+      dc = cplus_demangle_mangled_name (&di, 1);
+      break;
+    case DCT_GLOBAL_CTORS:
+    case DCT_GLOBAL_DTORS:
+      d_advance (&di, 11);
+      dc = d_make_comp (&di,
+			(type == DCT_GLOBAL_CTORS
+			 ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
+			 : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
+			d_make_demangle_mangled_name (&di, d_str (&di)),
+			NULL);
+      d_advance (&di, strlen (d_str (&di)));
+      break;
+    default:
+      free (di.comps);
+      free (di.subs);
+      abort (); /* We have listed all the cases.  */
+    }
 
-    switch (type)
-      {
-      case DCT_TYPE:
-	dc = cplus_demangle_type (&di);
-	break;
-      case DCT_MANGLED:
-	dc = cplus_demangle_mangled_name (&di, 1);
-	break;
-      case DCT_GLOBAL_CTORS:
-      case DCT_GLOBAL_DTORS:
-	d_advance (&di, 11);
-	dc = d_make_comp (&di,
-			  (type == DCT_GLOBAL_CTORS
-			   ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
-			   : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
-			  d_make_demangle_mangled_name (&di, d_str (&di)),
-			  NULL);
-	d_advance (&di, strlen (d_str (&di)));
-	break;
-      default:
-	abort (); /* We have listed all the cases.  */
-      }
+  /* If DMGL_PARAMS is set, then if we didn't consume the entire
+     mangled string, then we didn't successfully demangle it.  If
+     DMGL_PARAMS is not set, we didn't look at the trailing
+     parameters.  */
+  if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
+    dc = NULL;
 
-    /* If DMGL_PARAMS is set, then if we didn't consume the entire
-       mangled string, then we didn't successfully demangle it.  If
-       DMGL_PARAMS is not set, we didn't look at the trailing
-       parameters.  */
-    if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
-      dc = NULL;
-
 #ifdef CP_DEMANGLE_DEBUG
-    d_dump (dc, 0);
+  d_dump (dc, 0);
 #endif
 
-    status = (dc != NULL)
-             ? cplus_demangle_print_callback (options, dc, callback, opaque)
-             : 0;
-  }
-
+  status = (dc != NULL)
+           ? cplus_demangle_print_callback (options, dc, callback, opaque)
+           : 0;
+  
+  free (di.comps);
+  free (di.subs);
   return status;
 }
 
@@ -6226,60 +6207,59 @@ is_ctor_or_dtor (const char *mangled,
 
   cplus_demangle_init_info (mangled, DMGL_GNU_V3, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+						   * sizeof (*di.comps));
+  if (! di.comps)
+    return 0;
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (((size_t) di.num_subs) 
+						   * sizeof (*di.subs));
+  if (! di.subs)
+    return 0;  
 
-    dc = cplus_demangle_mangled_name (&di, 1);
+  dc = cplus_demangle_mangled_name (&di, 1);
 
-    /* Note that because we did not pass DMGL_PARAMS, we don't expect
-       to demangle the entire string.  */
+  /* Note that because we did not pass DMGL_PARAMS, we don't expect
+     to demangle the entire string.  */
 
-    ret = 0;
-    while (dc != NULL)
-      {
-	switch (dc->type)
-	  {
-	    /* These cannot appear on a constructor or destructor.  */
-	  case DEMANGLE_COMPONENT_RESTRICT_THIS:
-	  case DEMANGLE_COMPONENT_VOLATILE_THIS:
-	  case DEMANGLE_COMPONENT_CONST_THIS:
-	  case DEMANGLE_COMPONENT_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
-	  default:
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_TYPED_NAME:
-	  case DEMANGLE_COMPONENT_TEMPLATE:
-	    dc = d_left (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_QUAL_NAME:
-	  case DEMANGLE_COMPONENT_LOCAL_NAME:
-	    dc = d_right (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_CTOR:
-	    *ctor_kind = dc->u.s_ctor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_DTOR:
-	    *dtor_kind = dc->u.s_dtor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  }
-      }
-  }
+  ret = 0;
+  while (dc != NULL)
+    {
+      switch (dc->type)
+	{
+	  /* These cannot appear on a constructor or destructor.  */
+	case DEMANGLE_COMPONENT_RESTRICT_THIS:
+	case DEMANGLE_COMPONENT_VOLATILE_THIS:
+	case DEMANGLE_COMPONENT_CONST_THIS:
+	case DEMANGLE_COMPONENT_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
+	default:
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_TYPED_NAME:
+	case DEMANGLE_COMPONENT_TEMPLATE:
+	  dc = d_left (dc);
+	  break;
+	case DEMANGLE_COMPONENT_QUAL_NAME:
+	case DEMANGLE_COMPONENT_LOCAL_NAME:
+	  dc = d_right (dc);
+	  break;
+	case DEMANGLE_COMPONENT_CTOR:
+	  *ctor_kind = dc->u.s_ctor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_DTOR:
+	  *dtor_kind = dc->u.s_dtor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	}
+    }
 
+  free (di.comps);
+  free (di.subs);
   return ret;
 }
 
Index: libiberty/testsuite/demangle-expected
===================================================================
--- libiberty/testsuite/demangle-expected	(revision 235962)
+++ libiberty/testsuite/demangle-expected	(working copy)
@@ -4441,3 +4441,8 @@ __vt_90000000000cafebabe
 
 _Z80800000000000000000000
 _Z80800000000000000000000
+#
+# Tests stack overflow
+
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06 15:11           ` Marcel Böhme
@ 2016-05-06 15:19             ` Jakub Jelinek
  2016-05-16 18:12               ` Jeff Law
  2016-05-06 16:05             ` Marcel Böhme
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2016-05-06 15:19 UTC (permalink / raw)
  To: Marcel Böhme
  Cc: Jason Merrill, Ian Lance Taylor, Jonathan Wakely, gcc-patches,
	Bernd Schmidt

On Fri, May 06, 2016 at 11:11:29PM +0800, Marcel Böhme wrote:
> +  dpi.copy_templates
> +    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
> +					  * sizeof (*dpi.copy_templates));
> +  if (! dpi.copy_templates)
> +    {
> +      d_print_error (&dpi);
> +      return 0;
> +    }

Another thing to consider is if the common values of dpi.num_*
and similarly in the other block are small enough, it might be desirable
to just use an automatic fixed size array (or even alloca) and only
fall back to malloc if it is too large.
Would be nice to say on some distro grab using nm and nm -D all _Z* symbols
from all binaries and shared libraries and run the demangler with some
statistics gathering.  If say dpi.num_saved_scopes is <= 16 in 99.5% cases
(completely random guess), it might be a useful optimization.

Anyway, that is all from me, I'll defer to the demangler maintainers for the
rest.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06 15:11           ` Marcel Böhme
  2016-05-06 15:19             ` Jakub Jelinek
@ 2016-05-06 16:05             ` Marcel Böhme
  2016-05-06 16:17               ` Jakub Jelinek
  1 sibling, 1 reply; 15+ messages in thread
From: Marcel Böhme @ 2016-05-06 16:05 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Jason Merrill, Ian Lance Taylor, Jonathan Wakely, gcc-patches,
	Bernd Schmidt

Hi,

This patch also removes the following part of the comment for method cplus_demangle_print_callback:
"It does not use heap memory to build an output string, so cannot encounter memory allocation failure”.

> On 6 May 2016, at 11:11 PM, Marcel Böhme <boehme.marcel@gmail.com> wrote:
> 
> 
>> If one malloc succeeds and the other fails, you leak memory.
>> 
>> 	Jakub
> Nice catch. Thanks!
> 
> Bootstrapped and regression tested on x86_64-pc-linux-gnu.

Best - Marcel

Index: libiberty/ChangeLog
===================================================================
--- libiberty/ChangeLog	(revision 235962)
+++ libiberty/ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  <boehme.marcel@gmail.com>
+
+	PR c++/68159
+	* cp-demangle.c: Allocate arrays of user-defined size on the heap,
+	not on the stack. Do not include <alloca.h>.
+	(CP_DYNAMIC_ARRAYS): Remove definition.
+	(cplus_demangle_print_callback): Allocate memory for two arrays on
+	the heap. Free memory before return / exit.
+	(d_demangle_callback): Likewise.
+	(is_ctor_or_dtor): Likewise.
+	* testsuite/demangle-expected: Add regression test cases.
+
2016-05-02  Marcel Böhme  <boehme.marcel@gmail.com>

	PR c++/70498
Index: libiberty/cp-demangle.c
===================================================================
--- libiberty/cp-demangle.c	(revision 235962)
+++ libiberty/cp-demangle.c	(working copy)
@@ -116,18 +116,6 @@
 #include <string.h>
 #endif
 
-#ifdef HAVE_ALLOCA_H
-# include <alloca.h>
-#else
-# ifndef alloca
-#  ifdef __GNUC__
-#   define alloca __builtin_alloca
-#  else
-extern char *alloca ();
-#  endif /* __GNUC__ */
-# endif /* alloca */
-#endif /* HAVE_ALLOCA_H */
-
 #ifdef HAVE_LIMITS_H
 #include <limits.h>
 #endif
@@ -186,20 +174,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
    additional unresolved symbols when this code is used in a library.
    FIXME: Is this really a valid reason?  This comes from the original
@@ -4112,9 +4086,7 @@ d_last_char (struct d_print_info *dpi)
    CALLBACK is a function to call to flush demangled string segments
    as they fill the intermediate buffer, and OPAQUE is a generalized
    callback argument.  On success, this returns 1.  On failure,
-   it returns 0, indicating a bad parse.  It does not use heap
-   memory to build an output string, so cannot encounter memory
-   allocation failure.  */
+   it returns 0, indicating a bad parse.  */
 
 CP_STATIC_IF_GLIBCPP_V3
 int
@@ -4126,25 +4098,32 @@ cplus_demangle_print_callback (int options,
 
   d_print_init (&dpi, callback, opaque, dc);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-    __extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.copy_templates
+    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
+					  * sizeof (*dpi.copy_templates));
+  if (! dpi.copy_templates)
+    {
+      d_print_error (&dpi);
+      return 0;
+    }
 
-    dpi.saved_scopes = scopes;
-    dpi.copy_templates = temps;
-#else
-    dpi.saved_scopes = alloca (dpi.num_saved_scopes
-			       * sizeof (*dpi.saved_scopes));
-    dpi.copy_templates = alloca (dpi.num_copy_templates
-				 * sizeof (*dpi.copy_templates));
-#endif
+  dpi.saved_scopes
+    = (struct d_saved_scope *) malloc (((size_t) dpi.num_saved_scopes) 
+				       * sizeof (*dpi.saved_scopes));  
+  if (! dpi.saved_scopes)
+    {
+      free (dpi.copy_templates);
+      d_print_error (&dpi);
+      return 0;
+    }
 
-    d_print_comp (&dpi, options, dc);
-  }
+  d_print_comp (&dpi, options, dc);
 
   d_print_flush (&dpi);
 
+  free (dpi.copy_templates);
+  free (dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,57 +5924,61 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+						   * sizeof (*di.comps));
+  if (! di.comps)
+    return 0;
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (((size_t) di.num_subs) 
+						   * sizeof (*di.subs));  
+  if (! di.subs)
+    {
+      free (di.comps);
+      return 0;
+    }
+    
+  switch (type)
+    {
+    case DCT_TYPE:
+      dc = cplus_demangle_type (&di);
+      break;
+    case DCT_MANGLED:
+      dc = cplus_demangle_mangled_name (&di, 1);
+      break;
+    case DCT_GLOBAL_CTORS:
+    case DCT_GLOBAL_DTORS:
+      d_advance (&di, 11);
+      dc = d_make_comp (&di,
+			(type == DCT_GLOBAL_CTORS
+			 ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
+			 : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
+			d_make_demangle_mangled_name (&di, d_str (&di)),
+			NULL);
+      d_advance (&di, strlen (d_str (&di)));
+      break;
+    default:
+      free (di.comps);
+      free (di.subs);
+      abort (); /* We have listed all the cases.  */
+    }
 
-    switch (type)
-      {
-      case DCT_TYPE:
-	dc = cplus_demangle_type (&di);
-	break;
-      case DCT_MANGLED:
-	dc = cplus_demangle_mangled_name (&di, 1);
-	break;
-      case DCT_GLOBAL_CTORS:
-      case DCT_GLOBAL_DTORS:
-	d_advance (&di, 11);
-	dc = d_make_comp (&di,
-			  (type == DCT_GLOBAL_CTORS
-			   ? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
-			   : DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
-			  d_make_demangle_mangled_name (&di, d_str (&di)),
-			  NULL);
-	d_advance (&di, strlen (d_str (&di)));
-	break;
-      default:
-	abort (); /* We have listed all the cases.  */
-      }
+  /* If DMGL_PARAMS is set, then if we didn't consume the entire
+     mangled string, then we didn't successfully demangle it.  If
+     DMGL_PARAMS is not set, we didn't look at the trailing
+     parameters.  */
+  if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
+    dc = NULL;
 
-    /* If DMGL_PARAMS is set, then if we didn't consume the entire
-       mangled string, then we didn't successfully demangle it.  If
-       DMGL_PARAMS is not set, we didn't look at the trailing
-       parameters.  */
-    if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
-      dc = NULL;
-
 #ifdef CP_DEMANGLE_DEBUG
-    d_dump (dc, 0);
+  d_dump (dc, 0);
 #endif
 
-    status = (dc != NULL)
-             ? cplus_demangle_print_callback (options, dc, callback, opaque)
-             : 0;
-  }
-
+  status = (dc != NULL)
+           ? cplus_demangle_print_callback (options, dc, callback, opaque)
+           : 0;
+  
+  free (di.comps);
+  free (di.subs);
   return status;
 }
 
@@ -6226,60 +6209,62 @@ is_ctor_or_dtor (const char *mangled,
 
   cplus_demangle_init_info (mangled, DMGL_GNU_V3, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-    __extension__ struct demangle_component comps[di.num_comps];
-    __extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+						   * sizeof (*di.comps));
+  if (! di.comps)
+    return 0;
 
-    di.comps = comps;
-    di.subs = subs;
-#else
-    di.comps = alloca (di.num_comps * sizeof (*di.comps));
-    di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (((size_t) di.num_subs) 
+						   * sizeof (*di.subs));
+  if (! di.subs)
+    {
+      free (di.comps);
+      return 0;
+    }  
 
-    dc = cplus_demangle_mangled_name (&di, 1);
+  dc = cplus_demangle_mangled_name (&di, 1);
 
-    /* Note that because we did not pass DMGL_PARAMS, we don't expect
-       to demangle the entire string.  */
+  /* Note that because we did not pass DMGL_PARAMS, we don't expect
+     to demangle the entire string.  */
 
-    ret = 0;
-    while (dc != NULL)
-      {
-	switch (dc->type)
-	  {
-	    /* These cannot appear on a constructor or destructor.  */
-	  case DEMANGLE_COMPONENT_RESTRICT_THIS:
-	  case DEMANGLE_COMPONENT_VOLATILE_THIS:
-	  case DEMANGLE_COMPONENT_CONST_THIS:
-	  case DEMANGLE_COMPONENT_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
-	  case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
-	  default:
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_TYPED_NAME:
-	  case DEMANGLE_COMPONENT_TEMPLATE:
-	    dc = d_left (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_QUAL_NAME:
-	  case DEMANGLE_COMPONENT_LOCAL_NAME:
-	    dc = d_right (dc);
-	    break;
-	  case DEMANGLE_COMPONENT_CTOR:
-	    *ctor_kind = dc->u.s_ctor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  case DEMANGLE_COMPONENT_DTOR:
-	    *dtor_kind = dc->u.s_dtor.kind;
-	    ret = 1;
-	    dc = NULL;
-	    break;
-	  }
-      }
-  }
+  ret = 0;
+  while (dc != NULL)
+    {
+      switch (dc->type)
+	{
+	  /* These cannot appear on a constructor or destructor.  */
+	case DEMANGLE_COMPONENT_RESTRICT_THIS:
+	case DEMANGLE_COMPONENT_VOLATILE_THIS:
+	case DEMANGLE_COMPONENT_CONST_THIS:
+	case DEMANGLE_COMPONENT_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
+	case DEMANGLE_COMPONENT_TRANSACTION_SAFE:
+	default:
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_TYPED_NAME:
+	case DEMANGLE_COMPONENT_TEMPLATE:
+	  dc = d_left (dc);
+	  break;
+	case DEMANGLE_COMPONENT_QUAL_NAME:
+	case DEMANGLE_COMPONENT_LOCAL_NAME:
+	  dc = d_right (dc);
+	  break;
+	case DEMANGLE_COMPONENT_CTOR:
+	  *ctor_kind = dc->u.s_ctor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	case DEMANGLE_COMPONENT_DTOR:
+	  *dtor_kind = dc->u.s_dtor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	}
+    }
 
+  free (di.comps);
+  free (di.subs);
   return ret;
 }
 
Index: libiberty/testsuite/demangle-expected
===================================================================
--- libiberty/testsuite/demangle-expected	(revision 235962)
+++ libiberty/testsuite/demangle-expected	(working copy)
@@ -4441,3 +4441,8 @@ __vt_90000000000cafebabe
 
 _Z80800000000000000000000
 _Z80800000000000000000000
+#
+# Tests stack overflow
+
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__
+_ZZN5Eigen9DenseBaseINS_5BlockINS_6MatrixIdLin1ELin1ELi0ELin1ELin1EEELin1ELin1ELb1EEEE10lazyAssignINS_12CwiseUnaryOpIZZN6netops4expr6sampleINS9_12nn6_combinerIN5boost3mpl9vector2_cIiLi0ELi2EEENS9_11rowwise_addINS9_6matmulINS9_12input_matrixINSE_IiLi0ELi0EEENSC_6fusion7vector3INSK_7vector4IN4nnet8mat_sizeIdLi1EEESP_NSO_IiLi0EEENSO_IdLi0EEEEENSK_7vector7ISP_SP_SP_SP_SP_SP_SP_EENSN_11config_dataIN3nn66detail8nn6_modeILb1EEEEEEEEENS9_13weight_matrixINSD_6v_itemIN4mpl_4int_ILi0EEENSD_9vector1_cIiLi2EEELi0EEENSK_7vector6INSK_7vector2ISP_NSN_8vec_sizeIdLi1EEEEES1F_S1F_S1F_S1F_S1F_EEEEEENS13_INS14_INS16_ILi1EEES19_Li0EEES1G_EEEENS9_16logistic_sigmoidINSG_INSH_INS1N_INSG_INSH_INSI_INSE_IiLi0ELi1EEES11_EENS13_INS14_IS17_NS18_IiLi0EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Q_Li0EEES1G_EEEEEENS13_INS14_IS17_NS18_IiLi1EEELi0EEES1G_EEEENS13_INS14_IS1J_S1Y_Li0EEES1G_EEEEEEEEEclINSL_INSM_INS_12SparseMatrixIdLi1EiEENS2_IdLin1ELin1ELi1ELin1ELin1EEENS2_IiLin1ELi1ELi0ELin1ELi1EEENS2_IdLin1ELi1ELi0ELin1ELi1EEEEENST_IS2A_S2A_S2A_S2A_S2A_S2A_S2A_EESZ_EENS1B_INS1C_INS_3MapIS2B_Li1ENS_6StrideILi0ELi0EEEEENS2H_INS2_IdLi1ELin1ELi1ELi1ELin1EEELi1ES2J_EEEES2N_S2N_S2N_S2N_S2N_EEZZNS9_6concatIJNSI_INSE_IiLi0ELi3EEES11_EENS9_7dropoutILb0ES19_NS1N_INS2P_IJNSG_INSH_INSI_INSE_IiLi1ELi0EEES11_EENS13_INS14_IS17_NS18_IiLi3EEELi0EEES1G_EEEENS13_INS14_IS1J_S2V_Li0EEES1G_EEEENSG_INSH_INSI_INSE_IiLi1ELi1EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi2EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi3EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi4EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi5EEES11_EES2X_EES30_EENSG_INSH_INSI_INSE_IiLi1ELi6EEES11_EES2X_EES30_EEEEEEEEES27_EEclIS2G_S2O_ZNS9_6detail11binary_contIS2G_S2O_S3T_NS13_INS14_IS17_NS18_IiLi4EEELi0EEES1G_EEZNSH_IS3T_S3Z_EclIS2G_S2O_ZNS3W_IS2G_S2O_S40_NS13_INS14_IS1J_S3X_Li0EEES1G_EEZNSG_IS40_S43_EclIS2G_S2O_ZNS1N_IS44_EclIS2G_S2O_ZNS9_7concat2IS2R_S46_EclIS2G_S2O_ZNS3W_IS2G_S2O_S49_NS13_INS14_IS17_NS18_IiLi5EEELi0EEES1G_EEZNSH_IS49_S4D_EclIS2G_S2O_ZNS3W_IS2G_S2O_S4E_NS13_INS14_IS1J_S4B_Li0EEES1G_EEZNSG_IS4E_S4H_EclIS2G_S2O_ZNS9_11concat_zeroIS4I_EclIS2G_S2O_ZNS9_20softmax_crossentropyIS4L_EclIS2G_S2O_ZNKS8_11derived_ptrIS4O_E5fpropIS2G_S2O_EEDaRKT_RKT0_EUlOS4T_OS4W_OT1_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_OT2_E_EEDaRKNS4Q_IS51_EERKNS4Q_IS56_EES4V_S4Y_OKT3_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E0_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_EUlS4Z_S50_S52_S57_E_EEDaS5B_S5E_S4V_S4Y_S5H_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlmRS4T_E0_clINS4Q_IS27_EEEEDamS5R_EUlS4Z_S50_S52_E_EEDaS4V_S4Y_S52_ENKUlS4Z_S50_S52_E_clIRKS2G_RKS2O_KNS_22SparseTimeDenseProductIS2A_S2B_EEEEDaS4Z_S50_S52_EUldE_S64_EEEERS4_RKNS0_IS4T_EEE19__PRETTY_FUNCTION__

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06 16:05             ` Marcel Böhme
@ 2016-05-06 16:17               ` Jakub Jelinek
  0 siblings, 0 replies; 15+ messages in thread
From: Jakub Jelinek @ 2016-05-06 16:17 UTC (permalink / raw)
  To: Marcel Böhme
  Cc: Jason Merrill, Ian Lance Taylor, Jonathan Wakely, gcc-patches,
	Bernd Schmidt

On Sat, May 07, 2016 at 12:05:11AM +0800, Marcel Böhme wrote:
> This patch also removes the following part of the comment for method cplus_demangle_print_callback:
> "It does not use heap memory to build an output string, so cannot encounter memory allocation failure”.

But that exactly is the thing I've talked about.  Removing the comment
doesn't make it right, supposedly it has been done that way for a reason.

The file has lots of different entrypoints, some of them depend on various
macros on what is it built for (libstdc++, libgcc, binutils/gdb/gcc in
libiberty, ...).

And some of them clearly can cope with memory allocation failures, but
they should be turned into the allocation_failure flag setting.

Others don't want any allocations.

E.g. if you read the description of __cxa_demangle, there is
   *STATUS is set to one of the following values:
      0: The demangling operation succeeded.
     -1: A memory allocation failure occurred.
     -2: MANGLED_NAME is not a valid name under the C++ ABI mangling rules.
     -3: One of the arguments is invalid.
and thus, it should be ensured that we end up with *STATUS -1 even for
the cases where malloc failed on those.

But then look at e.g. __gcclibcxx_demangle_callback (but there are various
others).

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06  9:52     ` Jakub Jelinek
  2016-05-06 14:46       ` Marcel Böhme
@ 2016-05-06 16:17       ` Ian Lance Taylor
  2016-05-07  3:22         ` Marcel Böhme
  1 sibling, 1 reply; 15+ messages in thread
From: Ian Lance Taylor @ 2016-05-06 16:17 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Marcel Böhme, Jason Merrill, Ian Lance Taylor,
	Jonathan Wakely, gcc-patches, Bernd Schmidt

On Fri, May 6, 2016 at 2:51 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> Anyway, perhaps I'm misremembering, if there is a mode that really can't
> fail due to allocation failures or not, we need to deal with that.
> Ian or Jason, can all the demangle users allocate heap memory or not?
> And, if __cxa_demangle can fail, there is some allocation_failure stuff
> in the file.

The function cplus_demangle_v3_callback must not call malloc.  The
whole point of that function is to work when nothing else works.  That
is why d_demangle_callback does not, and must not, call malloc.

Ian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06 16:17       ` Ian Lance Taylor
@ 2016-05-07  3:22         ` Marcel Böhme
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Böhme @ 2016-05-07  3:22 UTC (permalink / raw)
  To: Ian Lance Taylor
  Cc: Jakub Jelinek, Jason Merrill, Ian Lance Taylor, Jonathan Wakely,
	gcc-patches, Bernd Schmidt

Hi Ian,

Stack overflows are a security concern and must be addressed. The Libiberty demangler is part of several tools, including binutils, gdb, valgrind, and many other libbfd-based tools that are used by the security community for the analysis of program binaries. Without a patch, the reverse engineering of untrusted binaries as well as determining whether an untrusted binary is malicious could cause serious damage. More details here: http://www.openwall.com/lists/oss-security/2016/05/05/3

> On 7 May 2016, at 12:16 AM, Ian Lance Taylor <iant@google.com> wrote:
> 
> The function cplus_demangle_v3_callback must not call malloc.  The
> whole point of that function is to work when nothing else works.  That
> is why d_demangle_callback does not, and must not, call malloc.

Point taken. In fact, I tracked down the patch submitted by Google's Simon Baldwin and the ensuing discussion from 2007: https://gcc.gnu.org/ml/gcc-patches/2007-01/msg01116.html (committed as revision 121305).

In that thread, Mark Mitchell raised concerns about small stacks and large mangled names and suggested to focus on an allocation interface where the the caller provides "alloc" and "dealloc" functions (i.e., C++ allocators): https://gcc.gnu.org/ml/gcc-patches/2007-01/msg01904.html

In the later patch to libstdc++ which has vterminate use the malloc-less demangler, Benjamin Kosnik raised similar concerns: https://gcc.gnu.org/ml/libstdc++/2007-03/msg00181.html

Perhaps the allocation interface is the way to go?

Best regards,
- Marcel




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-06 15:19             ` Jakub Jelinek
@ 2016-05-16 18:12               ` Jeff Law
  2016-05-16 18:20                 ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Law @ 2016-05-16 18:12 UTC (permalink / raw)
  To: Jakub Jelinek, Marcel Böhme
  Cc: Jason Merrill, Ian Lance Taylor, Jonathan Wakely, gcc-patches,
	Bernd Schmidt

On 05/06/2016 09:19 AM, Jakub Jelinek wrote:
> On Fri, May 06, 2016 at 11:11:29PM +0800, Marcel Böhme wrote:
>> +  dpi.copy_templates
>> +    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates)
>> +					  * sizeof (*dpi.copy_templates));
>> +  if (! dpi.copy_templates)
>> +    {
>> +      d_print_error (&dpi);
>> +      return 0;
>> +    }
>
> Another thing to consider is if the common values of dpi.num_*
> and similarly in the other block are small enough, it might be desirable
> to just use an automatic fixed size array (or even alloca) and only
> fall back to malloc if it is too large.
Please, no, don't fall back to alloca like this.  That coding idiom has 
been the source of numerous security exploits in glibc.  Experience 
shows us that we are not capable of doing that correctly on a consistent 
basis.

Jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-16 18:12               ` Jeff Law
@ 2016-05-16 18:20                 ` Jakub Jelinek
  2016-05-16 18:25                   ` Jeff Law
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2016-05-16 18:20 UTC (permalink / raw)
  To: Jeff Law
  Cc: Marcel Böhme, Jason Merrill, Ian Lance Taylor,
	Jonathan Wakely, gcc-patches, Bernd Schmidt

On Mon, May 16, 2016 at 12:12:38PM -0600, Jeff Law wrote:
> On 05/06/2016 09:19 AM, Jakub Jelinek wrote:
> >On Fri, May 06, 2016 at 11:11:29PM +0800, Marcel Böhme wrote:
> >>+  dpi.copy_templates
> >>+    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates)
> >>+					  * sizeof (*dpi.copy_templates));
> >>+  if (! dpi.copy_templates)
> >>+    {
> >>+      d_print_error (&dpi);
> >>+      return 0;
> >>+    }
> >
> >Another thing to consider is if the common values of dpi.num_*
> >and similarly in the other block are small enough, it might be desirable
> >to just use an automatic fixed size array (or even alloca) and only
> >fall back to malloc if it is too large.
> Please, no, don't fall back to alloca like this.  That coding idiom has been
> the source of numerous security exploits in glibc.  Experience shows us that
> we are not capable of doing that correctly on a consistent basis.

Falling back to fixed size buffer is something we use heavily in gcc, and
are able to get it right, there is nothing hard in it.

For the cases where we can't use malloc at all and we'd need too much memory
that it won't fit into the static buffer, I think all we can do is fall back
into increasing the time complexity in the demangler by processing the
string multiple times.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix for PR68159 in Libiberty Demangler (6)
  2016-05-16 18:20                 ` Jakub Jelinek
@ 2016-05-16 18:25                   ` Jeff Law
  0 siblings, 0 replies; 15+ messages in thread
From: Jeff Law @ 2016-05-16 18:25 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Marcel Böhme, Jason Merrill, Ian Lance Taylor,
	Jonathan Wakely, gcc-patches, Bernd Schmidt

On 05/16/2016 12:19 PM, Jakub Jelinek wrote:
> On Mon, May 16, 2016 at 12:12:38PM -0600, Jeff Law wrote:
>> On 05/06/2016 09:19 AM, Jakub Jelinek wrote:
>>> On Fri, May 06, 2016 at 11:11:29PM +0800, Marcel Böhme wrote:
>>>> +  dpi.copy_templates
>>>> +    = (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates)
>>>> +					  * sizeof (*dpi.copy_templates));
>>>> +  if (! dpi.copy_templates)
>>>> +    {
>>>> +      d_print_error (&dpi);
>>>> +      return 0;
>>>> +    }
>>>
>>> Another thing to consider is if the common values of dpi.num_*
>>> and similarly in the other block are small enough, it might be desirable
>>> to just use an automatic fixed size array (or even alloca) and only
>>> fall back to malloc if it is too large.
>> Please, no, don't fall back to alloca like this.  That coding idiom has been
>> the source of numerous security exploits in glibc.  Experience shows us that
>> we are not capable of doing that correctly on a consistent basis.
>
> Falling back to fixed size buffer is something we use heavily in gcc, and
> are able to get it right, there is nothing hard in it.
Conceptually I agree, it ought not be that hard, in practice, it's been 
an absolute disaster in glibc.

I've often wondered if the right model is to to use escape analysis 
along with the size of the object, loop analysis, etc and let the 
compiler figure this stuff out rather than leaving it to humans.


>
> For the cases where we can't use malloc at all and we'd need too much memory
> that it won't fit into the static buffer, I think all we can do is fall back
> into increasing the time complexity in the demangler by processing the
> string multiple times.
Probably true.

jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-05-16 18:25 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-06  6:14 Fix for PR68159 in Libiberty Demangler (6) Marcel Böhme
2016-05-06  7:10 ` Jakub Jelinek
2016-05-06  9:01   ` Marcel Böhme
2016-05-06  9:52     ` Jakub Jelinek
2016-05-06 14:46       ` Marcel Böhme
2016-05-06 14:49         ` Jakub Jelinek
2016-05-06 15:11           ` Marcel Böhme
2016-05-06 15:19             ` Jakub Jelinek
2016-05-16 18:12               ` Jeff Law
2016-05-16 18:20                 ` Jakub Jelinek
2016-05-16 18:25                   ` Jeff Law
2016-05-06 16:05             ` Marcel Böhme
2016-05-06 16:17               ` Jakub Jelinek
2016-05-06 16:17       ` Ian Lance Taylor
2016-05-07  3:22         ` Marcel Böhme

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 read-only IMAP folder(s) and NNTP newsgroup(s).