diff mbox

Fix up emitting VAR_DECLs in ubsan

Message ID 20131126232608.GA31608@redhat.com
State New
Headers show

Commit Message

Marek Polacek Nov. 26, 2013, 11:26 p.m. UTC 
This patch does two things:

1) The hash table should consume VAR_DECLs rather than ADDR_EXPRs; the
   latter allegedly cannot be shared between different locations.
2) Sometimes it can happen that some earlier instrumentation (e.g.
   -fsanitize=shift) creates an .Lubsan_type, but later on
   varpool_finalize_decl determines that this type is currently unused
   and doesn't emit the type into the assembly.  However, the
   reference to it stays in the hashtab, so if some later instrumentation
   tries to use the decl from the hashtab, we got a problem.
   Fixed by checking whether we already assembled that decl, if yes,
   we can reuse it, otherwise just create the decl.

Ran the testsuite on x86_64-linux.  Ok for trunk?

2013-11-27  Marek Polacek  <polacek@redhat.com>

	* ubsan.c (ubsan_type_descriptor): If varpool_get_node returns NULL
	for a decl, recreate that decl.  Save into the hash table VAR_DECLs
	rather than ADDR_EXPRs.
testsuite/
	* c-c++-common/ubsan/undefined-1.c: New test.


	Marek

Comments

Jakub Jelinek Nov. 26, 2013, 11:31 p.m. UTC | #1 
On Wed, Nov 27, 2013 at 12:26:08AM +0100, Marek Polacek wrote:
> Ran the testsuite on x86_64-linux.  Ok for trunk?

Ok, with a minor nit.

> --- gcc/testsuite/c-c++-common/ubsan/undefined-1.c.mp3	2013-11-26 23:56:42.151624262 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/undefined-1.c	2013-11-27 00:12:46.053851104 +0100
> @@ -0,0 +1,25 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=undefined" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> +
> +int
> +foo (int x, int y)
> +{
> +  if (2 & 1)

Please use here:
  int z = 2;
  if (z & 1)
or similar instead, to make sure it is only early
GIMPLE optimization passes that optimize it away, if already the
front end knows it isn't needed, perhaps in the future it could
just avoid instrumenting it.

	Jakub
diff mbox

Patch

--- gcc/ubsan.c.mp3	2013-11-26 18:07:46.225509763 +0100
+++ gcc/ubsan.c	2013-11-27 00:03:35.718418339 +0100
@@ -269,8 +269,12 @@  ubsan_type_descriptor (tree type, bool w
   type = TYPE_MAIN_VARIANT (type);
 
   tree decl = decl_for_type_lookup (type);
-  if (decl != NULL_TREE)
-    return decl;
+  /* It is possible that some of the earlier created DECLs were found
+     unused, in that case they weren't emitted and varpool_get_node
+     returns NULL node on them.  But now we really need them.  Thus,
+     renew them here.  */
+  if (decl != NULL_TREE && varpool_get_node (decl))
+    return build_fold_addr_expr (decl);
 
   tree dtype = ubsan_type_descriptor_type ();
   tree type2 = type;
@@ -369,11 +373,10 @@  ubsan_type_descriptor (tree type, bool w
   DECL_INITIAL (decl) = ctor;
   rest_of_decl_compilation (decl, 1, 0);
 
-  /* Save the address of the VAR_DECL into the hash table.  */
-  decl = build_fold_addr_expr (decl);
+  /* Save the VAR_DECL into the hash table.  */
   decl_for_type_insert (type, decl);
 
-  return decl;
+  return build_fold_addr_expr (decl);
 }
 
 /* Create a structure for the ubsan library.  NAME is a name of the new
--- gcc/testsuite/c-c++-common/ubsan/undefined-1.c.mp3	2013-11-26 23:56:42.151624262 +0100
+++ gcc/testsuite/c-c++-common/ubsan/undefined-1.c	2013-11-27 00:12:46.053851104 +0100
@@ -0,0 +1,25 @@ 
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
+
+int
+foo (int x, int y)
+{
+  if (2 & 1)
+    return x << y;
+  return 0;
+}
+
+int
+bar (int x, int y)
+{
+  return x + y;
+}
+
+int
+main (void)
+{
+  foo (3, 2);
+  bar (12, 42);
+  return 0;
+}