diff mbox

Fix -fsanitizer=undefined ICE (PR sanitizer/59258)

Message ID 20131126201415.GK892@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 26, 2013, 8:14 p.m. UTC
Hi!

The problem here is that ubsan_create_data was called
with location_t that includes both location and BLOCK, and that
location was sticked into an ADDR_EXPR in a static var's constructor.
As the BLOCk wasn't live in any of the functions, it was removed as unused
and the GCmemory was reused for something different during GC,
but then the BLOCK was rediscovered again from the ADDR_EXPR.

Fixed thusly (in fact either the loc = LOCATION_LOCUS (loc); line
or build_fold_addr_expr line should fix this).  IMHO we want both
changes, because we compare in ubsan_create_data loc to UNKNOWN_LOCATION,
which without LOCATION_LOCUS doesn't work reliably (that is why I've earlier
added the xloc.file == NULL handling which isn't needed anymore),
and setting locus of ADDR_EXPR in a static var's constructor doesn't
really make any sense.

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

The testcase was huge and GC dependent, so it wasn't really reducible.

2013-11-25  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/59258
	* ubsan.c (ubsan_source_location): Don't add any location
	to ADDR_EXPR in the ctor.  Revert 2013-11-22 change.
	(ubsan_create_data): Strip block info from LOC.


	Jakub

Comments

Jeff Law Nov. 26, 2013, 9:20 p.m. UTC | #1
On 11/26/13 13:14, Jakub Jelinek wrote:
> Hi!
>
> The problem here is that ubsan_create_data was called
> with location_t that includes both location and BLOCK, and that
> location was sticked into an ADDR_EXPR in a static var's constructor.
> As the BLOCk wasn't live in any of the functions, it was removed as unused
> and the GCmemory was reused for something different during GC,
> but then the BLOCK was rediscovered again from the ADDR_EXPR.
>
> Fixed thusly (in fact either the loc = LOCATION_LOCUS (loc); line
> or build_fold_addr_expr line should fix this).  IMHO we want both
> changes, because we compare in ubsan_create_data loc to UNKNOWN_LOCATION,
> which without LOCATION_LOCUS doesn't work reliably (that is why I've earlier
> added the xloc.file == NULL handling which isn't needed anymore),
> and setting locus of ADDR_EXPR in a static var's constructor doesn't
> really make any sense.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> The testcase was huge and GC dependent, so it wasn't really reducible.
>
> 2013-11-25  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR sanitizer/59258
> 	* ubsan.c (ubsan_source_location): Don't add any location
> 	to ADDR_EXPR in the ctor.  Revert 2013-11-22 change.
> 	(ubsan_create_data): Strip block info from LOC.
OK.
jeff
diff mbox

Patch

--- gcc/ubsan.c.jj	2013-11-25 18:30:19.000000000 +0100
+++ gcc/ubsan.c	2013-11-26 08:41:20.987761054 +0100
@@ -229,13 +229,13 @@  ubsan_source_location (location_t loc)
   xloc = expand_location (loc);
 
   /* Fill in the values from LOC.  */
-  size_t len = xloc.file ? strlen (xloc.file) : 0;
-  tree str = build_string (len + 1, xloc.file ? xloc.file : "");
+  size_t len = strlen (xloc.file);
+  tree str = build_string (len + 1, xloc.file);
   TREE_TYPE (str) = build_array_type (char_type_node,
 				      build_index_type (size_int (len)));
   TREE_READONLY (str) = 1;
   TREE_STATIC (str) = 1;
-  str = build_fold_addr_expr_loc (loc, str);
+  str = build_fold_addr_expr (str);
   tree ctor = build_constructor_va (type, 3, NULL_TREE, str, NULL_TREE,
 				    build_int_cst (unsigned_type_node,
 						   xloc.line), NULL_TREE,
@@ -398,6 +398,7 @@  ubsan_create_data (const char *name, loc
   tree td_type = ubsan_type_descriptor_type ();
   TYPE_READONLY (td_type) = 1;
   td_type = build_pointer_type (td_type);
+  loc = LOCATION_LOCUS (loc);
 
   /* Create the structure type.  */
   ret = make_node (RECORD_TYPE);