This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Avoid ICEs with throwing calls with -fprintf-return-value (PR middle-end/78901)


On 01/02/2017 12:54 PM, Jakub Jelinek wrote:
Hi!

As my earlier patch to adjust cfns.gperf for C99/C11 builtins has been
committed already, the non-__*_chk builtins (and when using __builtin__*_chk
form even those) should be all noexcept, and when using fortification
the glibc headers declare those properly throw() and ssp includes just
use builtins directly.  Therefore I think it is just a waste of time to
add code to handle something that will not really happen in real-world code,
only when somebody declares those incorrectly by hand.

The following patch therefore just punts on stmt_ends_bb_p (info.callstmt)
rather than purging dead edges and scheduling cfg cleanups on replacement
of call with constant if the compiler thinks it might throw, or when
inserting after the call (also invalid, would need to be inserted on
fallthrough edge instead).

Thanks for taking care of this bug!  Punting on this case sounds
fine.

I was not familiar with the stmt_ends_bb_p() function or how it
relates to exceptions so I looked it up.  It looks to be a wrapper
around is_ctrl_stmt() and is_ctrl_altering_stmt().  Of the two, it
looks to me like the first one will always return false here because
a function call is not a control statement, and the second will end
up returning the result of g gimple_call_ctrl_altering_p() because
the argument is a GIMPLE_CALL.  Would it be appropriate to call
gimple_call_ctrl_altering_p() here directly instead? (I ask mainly
because to me, unlike the former, the latter clearly conveys that
it's being called to determine whether the function call may alter
control flow [such as by throwing].)

Martin


Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-01-02  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/78901
	* gimple-ssa-sprintf.c (try_substitute_return_value): Don't change
	possibly throwing calls.

	* g++.dg/opt/pr78901.C: New test.

--- gcc/gimple-ssa-sprintf.c.jj	2017-01-02 12:09:29.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c	2017-01-02 13:05:08.400755201 +0100
@@ -2699,9 +2699,15 @@ try_substitute_return_value (gimple_stmt
      the output overflows the destination object (but leave it enabled
      when the function is bounded because then the behavior is well-
      defined).  */
-  if (lhs && res.bounded && res.under4k
+  if (lhs
+      && res.bounded
+      && res.under4k
       && (info.bounded || res.number_chars <= info.objsize)
-      && res.number_chars - 1 <= target_int_max ())
+      && res.number_chars - 1 <= target_int_max ()
+      /* Not prepared to handle possibly throwing calls here; they shouldn't
+	 appear in non-artificial testcases, except when the __*_chk routines
+	 are badly declared.  */
+      && !stmt_ends_bb_p (info.callstmt))
     {
       tree cst = build_int_cst (integer_type_node, res.number_chars - 1);

--- gcc/testsuite/g++.dg/opt/pr78901.C.jj	2017-01-02 13:04:24.271329153 +0100
+++ gcc/testsuite/g++.dg/opt/pr78901.C	2017-01-02 13:03:19.000000000 +0100
@@ -0,0 +1,18 @@
+// PR middle-end/78901
+// { dg-do compile }
+// { dg-options "-O2 -Wno-stringop-overflow" }
+
+extern "C" int __snprintf_chk (char *, __SIZE_TYPE__, int, __SIZE_TYPE__, const char *, ...);
+
+int
+foo (char *c)
+{
+  try
+    {
+      return __snprintf_chk (c, 64, 0, 32, "%s", "abcdefghijklmnopq");
+    }
+  catch (...)
+    {
+      return -1;
+    }
+}

	Jakub



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]