RFA: Handle unsigned characters from string constants in store-ccp

RFA: Handle unsigned characters from string constants in store-ccp

[Daniel Jacobowitz]

The attached test fails on all platforms on various hosts, because
tree-ssa-ccp.c has this bit of code:

	    return build_int_cst (TREE_TYPE (t), (TREE_STRING_POINTER (ctor)
					          [TREE_INT_CST_LOW (idx)]));

TREE_TYPE (t) here is the type of the desired value, unsigned char.
TREE_STRING_POINTER points to a const char *.  So if the character we
load has its high bit set, and char is signed, then we create an
INTEGER_CST whose value is -99 but whose type is unsigned.  VRP later
decides, quite reasonably, that no unsigned char could be equal to -99.

Is this patch OK?

-- 
Daniel Jacobowitz
CodeSourcery

2007-07-24  Daniel Jacobowitz  <dan@codesourcery.com>

	* tree-ssa-ccp.c (fold_const_aggregate_ref): Use fold_convert.

2007-07-24  Daniel Jacobowitz  <dan@codesourcery.com>

	* gcc.c-torture/execute/20070724-1.c: New.

Index: testsuite/gcc.c-torture/execute/20070724-1.c
===================================================================
--- testsuite/gcc.c-torture/execute/20070724-1.c	(revision 0)
+++ testsuite/gcc.c-torture/execute/20070724-1.c	(revision 0)
@@ -0,0 +1,11 @@
+void abort (void);
+
+static unsigned char magic[] = "\235";
+static unsigned char value = '\235';
+
+int main()
+{
+  if (value != magic[0])
+    abort ();
+  return 0;
+}
Index: tree-ssa-ccp.c
===================================================================
--- tree-ssa-ccp.c	(revision 126877)
+++ tree-ssa-ccp.c	(working copy)
@@ -1053,8 +1053,10 @@ fold_const_aggregate_ref (tree t)
 	          == MODE_INT)
 	      && GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor)))) == 1
 	      && compare_tree_int (idx, TREE_STRING_LENGTH (ctor)) < 0)
-	    return build_int_cst (TREE_TYPE (t), (TREE_STRING_POINTER (ctor)
-					          [TREE_INT_CST_LOW (idx)]));
+	    return fold_convert (TREE_TYPE (t),
+				 build_int_cst (NULL,
+						(TREE_STRING_POINTER (ctor)
+						 [TREE_INT_CST_LOW (idx)])));
 	  return NULL_TREE;
 	}
 

Re: RFA: Handle unsigned characters from string constants in store-ccp

[Diego Novillo]

On 7/24/07 1:00 PM, Daniel Jacobowitz wrote:

> Is this patch OK?

Yes.

Re: RFA: Handle unsigned characters from string constants in store-ccp

[Richard Guenther]

On 7/24/07, Diego Novillo <dnovillo@google.com> wrote:
> On 7/24/07 1:00 PM, Daniel Jacobowitz wrote:
>
> > Is this patch OK?
>
> Yes.

No, you should use build_int_cst_type (TREE_TYPE (t), (unsigned
char)...) instead.  (Definitely not use build_int_cst with a NULL type
argument)

Richard.

Re: RFA: Handle unsigned characters from string constants in store-ccp

[Daniel Jacobowitz]

On Tue, Jul 24, 2007 at 10:30:50PM +0200, Richard Guenther wrote:
> On 7/24/07, Diego Novillo <dnovillo@google.com> wrote:
> > On 7/24/07 1:00 PM, Daniel Jacobowitz wrote:
> >
> > > Is this patch OK?
> >
> > Yes.
> 
> No, you should use build_int_cst_type (TREE_TYPE (t), (unsigned
> char)...) instead.  (Definitely not use build_int_cst with a NULL type
> argument)

Sorry, I'd already committed it.  Should the place this code is based
on change too?  That's fold-convert.c:fold_read_from_constant_string.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: Handle unsigned characters from string constants in store-ccp

[Richard Guenther]

On 7/25/07, Daniel Jacobowitz <drow@false.org> wrote:
> On Tue, Jul 24, 2007 at 10:30:50PM +0200, Richard Guenther wrote:
> > On 7/24/07, Diego Novillo <dnovillo@google.com> wrote:
> > > On 7/24/07 1:00 PM, Daniel Jacobowitz wrote:
> > >
> > > > Is this patch OK?
> > >
> > > Yes.
> >
> > No, you should use build_int_cst_type (TREE_TYPE (t), (unsigned
> > char)...) instead.  (Definitely not use build_int_cst with a NULL type
> > argument)
>
> Sorry, I'd already committed it.  Should the place this code is based
> on change too?  That's fold-convert.c:fold_read_from_constant_string.

Yes, that would be nice.

Thanks,
Richard.

Re: RFA: Handle unsigned characters from string constants in store-ccp

[Daniel Jacobowitz]

On Wed, Jul 25, 2007 at 02:11:28PM +0200, Richard Guenther wrote:
> On 7/25/07, Daniel Jacobowitz <drow@false.org> wrote:
> > On Tue, Jul 24, 2007 at 10:30:50PM +0200, Richard Guenther wrote:
> > > On 7/24/07, Diego Novillo <dnovillo@google.com> wrote:
> > > > On 7/24/07 1:00 PM, Daniel Jacobowitz wrote:
> > > >
> > > > > Is this patch OK?
> > > >
> > > > Yes.
> > >
> > > No, you should use build_int_cst_type (TREE_TYPE (t), (unsigned
> > > char)...) instead.  (Definitely not use build_int_cst with a NULL type
> > > argument)
> >
> > Sorry, I'd already committed it.  Should the place this code is based
> > on change too?  That's fold-convert.c:fold_read_from_constant_string.
> 
> Yes, that would be nice.

Checked in like so, bootstrapped and tested on x86_64-linux.

-- 
Daniel Jacobowitz
CodeSourcery

2007-07-26  Daniel Jacobowitz  <dan@codesourcery.com>

	* fold-const.c (fold_read_from_constant_string): Use
	build_int_cst_type.
	* tree-ssa-ccp.c (fold_const_aggregate_ref): Likewise.

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 126911)
+++ gcc/fold-const.c	(working copy)
@@ -14063,10 +14063,9 @@ fold_read_from_constant_string (tree exp
 	  && (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (TREE_TYPE (string))))
 	      == MODE_INT)
 	  && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (string)))) == 1))
-	return fold_convert (TREE_TYPE (exp),
-			     build_int_cst (NULL_TREE,
-					    (TREE_STRING_POINTER (string)
-					     [TREE_INT_CST_LOW (index)])));
+	return build_int_cst_type (TREE_TYPE (exp),
+				   (TREE_STRING_POINTER (string)
+				    [TREE_INT_CST_LOW (index)]));
     }
   return NULL;
 }
Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c	(revision 126911)
+++ gcc/tree-ssa-ccp.c	(working copy)
@@ -1053,10 +1053,9 @@ fold_const_aggregate_ref (tree t)
 	          == MODE_INT)
 	      && GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor)))) == 1
 	      && compare_tree_int (idx, TREE_STRING_LENGTH (ctor)) < 0)
-	    return fold_convert (TREE_TYPE (t),
-				 build_int_cst (NULL,
-						(TREE_STRING_POINTER (ctor)
-						 [TREE_INT_CST_LOW (idx)])));
+	    return build_int_cst_type (TREE_TYPE (t),
+				       (TREE_STRING_POINTER (ctor)
+					[TREE_INT_CST_LOW (idx)]));
 	  return NULL_TREE;
 	}