diff mbox series

[pushed] Objective-C: fix crash with -fobjc-nilcheck

Message ID 64AF42B0-1BD3-41A6-ADF3-45C9483F57BD@sandoe.co.uk
State New
Headers show
Series [pushed] Objective-C: fix crash with -fobjc-nilcheck | expand

Commit Message

Iain Sandoe Aug. 18, 2021, 6:45 p.m. UTC 
Hi,

Thanks to Matt for the PR and the initial draft patch.

=====


When -fobjc-nilcheck is enabled, messages that result in a struct type should
yield a zero-initialized struct when sent to nil.  Currently, the frontend
crashes when it encounters this situation.  This patch fixes the crash by
generating the tree for the `{}` initializer.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
Co-authored-by: Matt Jacobson  <mhjacobson@me.com>

	PR objc/101666

gcc/objc/ChangeLog:

	* objc-act.c (objc_build_constructor): Handle empty constructor
	lists.
	* objc-next-runtime-abi-02.c (build_v2_objc_method_fixup_call):
	Handle nil receivers.
	(build_v2_build_objc_method_call): Likewise.

gcc/testsuite/ChangeLog:

	* obj-c++.dg/pr101666-0.mm: New test.
	* obj-c++.dg/pr101666-1.mm: New test.
	* obj-c++.dg/pr101666.inc: New.
	* objc.dg/pr101666-0.m: New test.
	* objc.dg/pr101666-1.m: New test.
	* objc.dg/pr101666.inc: New.
---
 gcc/objc/objc-act.c                    | 16 +++++++-------
 gcc/objc/objc-next-runtime-abi-02.c    | 22 +++++++------------
 gcc/testsuite/obj-c++.dg/pr101666-0.mm |  7 +++++++
 gcc/testsuite/obj-c++.dg/pr101666-1.mm | 10 +++++++++
 gcc/testsuite/obj-c++.dg/pr101666.inc  | 29 ++++++++++++++++++++++++++
 gcc/testsuite/objc.dg/pr101666-0.m     |  7 +++++++
 gcc/testsuite/objc.dg/pr101666-1.m     | 10 +++++++++
 gcc/testsuite/objc.dg/pr101666.inc     | 29 ++++++++++++++++++++++++++
 8 files changed, 108 insertions(+), 22 deletions(-)
 create mode 100644 gcc/testsuite/obj-c++.dg/pr101666-0.mm
 create mode 100644 gcc/testsuite/obj-c++.dg/pr101666-1.mm
 create mode 100644 gcc/testsuite/obj-c++.dg/pr101666.inc
 create mode 100644 gcc/testsuite/objc.dg/pr101666-0.m
 create mode 100644 gcc/testsuite/objc.dg/pr101666-1.m
 create mode 100644 gcc/testsuite/objc.dg/pr101666.inc

--
diff mbox series

Patch

diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index ec20891152b..6e4fb626936 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -3377,8 +3377,10 @@  objc_build_string_object (tree string)
   return addr;
 }
 
-/* Build a static constant CONSTRUCTOR
-   with type TYPE and elements ELTS.  */
+/* Build a static constant CONSTRUCTOR with type TYPE and elements ELTS.
+   We might be presented with a NULL for ELTS, which means 'empty ctor'
+   which will subsequently be converted into a zero initializer in the
+   middle end.  */
 
 tree
 objc_build_constructor (tree type, vec<constructor_elt, va_gc> *elts)
@@ -3390,12 +3392,10 @@  objc_build_constructor (tree type, vec<constructor_elt, va_gc> *elts)
   TREE_READONLY (constructor) = 1;
 
 #ifdef OBJCPLUS
-  /* Adjust for impedance mismatch.  We should figure out how to build
-     CONSTRUCTORs that consistently please both the C and C++ gods.  */
-  if (!(*elts)[0].index)
+  /* If we know the initializer, then set the type to what C++ expects.  */
+  if (elts && !(*elts)[0].index)
     TREE_TYPE (constructor) = init_list_type_node;
 #endif
-
   return constructor;
 }
 
@@ -9664,7 +9664,9 @@  objc_gimplify_property_ref (tree *expr_p)
       call_exp = TREE_OPERAND (getter, 1);
     }
 #endif
-  gcc_assert (TREE_CODE (call_exp) == CALL_EXPR);
+  gcc_checking_assert ((flag_objc_nilcheck
+			&& TREE_CODE (call_exp) == COND_EXPR)
+		       || TREE_CODE (call_exp) == CALL_EXPR);
 
   *expr_p = call_exp;
 }
diff --git a/gcc/objc/objc-next-runtime-abi-02.c b/gcc/objc/objc-next-runtime-abi-02.c
index c552013ab27..0d963e357c4 100644
--- a/gcc/objc/objc-next-runtime-abi-02.c
+++ b/gcc/objc/objc-next-runtime-abi-02.c
@@ -1675,13 +1675,8 @@  build_v2_objc_method_fixup_call (int super_flag, tree method_prototype,
 
       if (TREE_CODE (ret_type) == RECORD_TYPE
 	  || TREE_CODE (ret_type) == UNION_TYPE)
-	{
-	  vec<constructor_elt, va_gc> *rtt = NULL;
-	  /* ??? CHECKME. hmmm..... think we need something more
-	     here.  */
-	  CONSTRUCTOR_APPEND_ELT (rtt, NULL_TREE, NULL_TREE);
-	  ftree = objc_build_constructor (ret_type, rtt);
-	}
+	/* An empty constructor is zero-filled by the middle end.  */
+	ftree = objc_build_constructor (ret_type, NULL);
       else
 	ftree = fold_convert (ret_type, integer_zero_node);
 
@@ -1694,11 +1689,11 @@  build_v2_objc_method_fixup_call (int super_flag, tree method_prototype,
 					ifexp, ret_val, ftree,
 					tf_warning_or_error);
 #else
-     /* ??? CHECKME.   */
       ret_val = build_conditional_expr (input_location,
-					ifexp, 1,
+					ifexp, 0,
 					ret_val, NULL_TREE, input_location,
 					ftree, NULL_TREE, input_location);
+      ret_val = fold_convert (ret_type, ret_val);
 #endif
     }
   return ret_val;
@@ -1790,11 +1785,8 @@  build_v2_build_objc_method_call (int super, tree method_prototype,
       if (TREE_CODE (ret_type) == RECORD_TYPE
 	  || TREE_CODE (ret_type) == UNION_TYPE)
 	{
-	  vec<constructor_elt, va_gc> *rtt = NULL;
-	  /* ??? CHECKME. hmmm..... think we need something more
-	     here.  */
-	  CONSTRUCTOR_APPEND_ELT (rtt, NULL_TREE, NULL_TREE);
-	  ftree = objc_build_constructor (ret_type, rtt);
+	/* An empty constructor is zero-filled by the middle end.  */
+	  ftree = objc_build_constructor (ret_type, NULL);
 	}
       else
 	ftree = fold_convert (ret_type, integer_zero_node);
@@ -1807,10 +1799,10 @@  build_v2_build_objc_method_call (int super, tree method_prototype,
       ret_val = build_conditional_expr (loc, ifexp, ret_val, ftree,
 					tf_warning_or_error);
 #else
-     /* ??? CHECKME.   */
       ret_val = build_conditional_expr (loc, ifexp, 1,
 					ret_val, NULL_TREE, loc,
 					ftree, NULL_TREE, loc);
+      ret_val = fold_convert (ret_type, ret_val);
 #endif
     }
   return ret_val;
diff --git a/gcc/testsuite/obj-c++.dg/pr101666-0.mm b/gcc/testsuite/obj-c++.dg/pr101666-0.mm
new file mode 100644
index 00000000000..5f87f605ae4
--- /dev/null
+++ b/gcc/testsuite/obj-c++.dg/pr101666-0.mm
@@ -0,0 +1,7 @@ 
+/* { dg-do run } */
+/* { dg-skip-if "NeXT only" { *-*-* } { "-fgnu-runtime" } { "" } } */
+/* { dg-skip-if "ABI 2 only" { *-*-* && { ! objc2 } } { "*" } { "" } } */
+/* { dg-additional-options "-fobjc-nilcheck -Wno-objc-root-class" } */
+
+#include "pr101666.inc"
+
diff --git a/gcc/testsuite/obj-c++.dg/pr101666-1.mm b/gcc/testsuite/obj-c++.dg/pr101666-1.mm
new file mode 100644
index 00000000000..41ef3704bff
--- /dev/null
+++ b/gcc/testsuite/obj-c++.dg/pr101666-1.mm
@@ -0,0 +1,10 @@ 
+/* Later versions of Darwin can compile for 10.5, but cannot link it so we
+   can only run this test up to 10.13.  */
+/* { dg-do compile { target *-*-darwin* } } */
+/* { dg-do run { target *-*-darwin[89]* *-*-darwin1[0-7]* } } */
+/* { dg-skip-if "NeXT only" { *-*-* } { "-fgnu-runtime" } { "" } } */
+/* { dg-skip-if "ABI 2 only" { *-*-* && { ! objc2 } } { "*" } { "" } } */
+/* { dg-additional-options "-fobjc-nilcheck -mmacosx-version-min=10.5 " } */
+/* { dg-additional-options "-Wno-objc-root-class" } */
+
+#include "pr101666.inc"
diff --git a/gcc/testsuite/obj-c++.dg/pr101666.inc b/gcc/testsuite/obj-c++.dg/pr101666.inc
new file mode 100644
index 00000000000..e81e1be2e4c
--- /dev/null
+++ b/gcc/testsuite/obj-c++.dg/pr101666.inc
@@ -0,0 +1,29 @@ 
+#include <cstdlib>
+struct point { double x, y, z; };
+
+@interface Foo
+- (struct point)bar;
+- (struct point)baz;
+@end
+
+@implementation Foo
+- (struct point)bar { struct point q = { 1.0, 2.0, 3.0 }; return q; };
+- (struct point)baz { struct point q = { 4.0, 5.0, 6.0 }; return q; };
+@end
+
+/* Cases where a check for nil should be inserted by the compiler, when
+   -fobjc-nilcheck is in force. We should not attempt the calls, and the
+   result should be 0-filled. */
+
+Foo *f;
+
+int main(void) {
+  struct point p = [f bar];
+  if (p.x != 0.0 || p.y != 0.0 || p.z != 0.0)
+    abort ();
+  id nilobj = (id)0;
+  p = [nilobj baz];
+  if (p.x != 0.0 || p.y != 0.0 || p.z != 0.0)
+    abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/objc.dg/pr101666-0.m b/gcc/testsuite/objc.dg/pr101666-0.m
new file mode 100644
index 00000000000..5f87f605ae4
--- /dev/null
+++ b/gcc/testsuite/objc.dg/pr101666-0.m
@@ -0,0 +1,7 @@ 
+/* { dg-do run } */
+/* { dg-skip-if "NeXT only" { *-*-* } { "-fgnu-runtime" } { "" } } */
+/* { dg-skip-if "ABI 2 only" { *-*-* && { ! objc2 } } { "*" } { "" } } */
+/* { dg-additional-options "-fobjc-nilcheck -Wno-objc-root-class" } */
+
+#include "pr101666.inc"
+
diff --git a/gcc/testsuite/objc.dg/pr101666-1.m b/gcc/testsuite/objc.dg/pr101666-1.m
new file mode 100644
index 00000000000..41ef3704bff
--- /dev/null
+++ b/gcc/testsuite/objc.dg/pr101666-1.m
@@ -0,0 +1,10 @@ 
+/* Later versions of Darwin can compile for 10.5, but cannot link it so we
+   can only run this test up to 10.13.  */
+/* { dg-do compile { target *-*-darwin* } } */
+/* { dg-do run { target *-*-darwin[89]* *-*-darwin1[0-7]* } } */
+/* { dg-skip-if "NeXT only" { *-*-* } { "-fgnu-runtime" } { "" } } */
+/* { dg-skip-if "ABI 2 only" { *-*-* && { ! objc2 } } { "*" } { "" } } */
+/* { dg-additional-options "-fobjc-nilcheck -mmacosx-version-min=10.5 " } */
+/* { dg-additional-options "-Wno-objc-root-class" } */
+
+#include "pr101666.inc"
diff --git a/gcc/testsuite/objc.dg/pr101666.inc b/gcc/testsuite/objc.dg/pr101666.inc
new file mode 100644
index 00000000000..f1dddca6498
--- /dev/null
+++ b/gcc/testsuite/objc.dg/pr101666.inc
@@ -0,0 +1,29 @@ 
+#include <stdlib.h>
+struct point { double x, y, z; };
+
+@interface Foo
+- (struct point)bar;
+- (struct point)baz;
+@end
+
+@implementation Foo
+- (struct point)bar { struct point q = { 1.0, 2.0, 3.0 }; return q; };
+- (struct point)baz { struct point q = { 4.0, 5.0, 6.0 }; return q; };
+@end
+
+/* Cases where a check for nil should be inserted by the compiler, when
+   -fobjc-nilcheck is in force. We should not attempt the calls, and the
+   result should be 0-filled. */
+
+Foo *f;
+
+int main(void) {
+  struct point p = [f bar];
+  if (p.x != 0.0 || p.y != 0.0 || p.z != 0.0)
+    abort ();
+  id nilobj = (id)0;
+  p = [nilobj baz];
+  if (p.x != 0.0 || p.y != 0.0 || p.z != 0.0)
+    abort ();
+  return 0;
+}