diff mbox

[RFH,/] PR 20140

Message ID 4F01B054.6070809@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Jan. 2, 2012, 1:25 p.m. UTC 
Hi,
> On Mon, Jan 2, 2012 at 3:24 AM, Paolo Carlini<paolo.carlini@oracle.com>  wrote:
>> On 01/02/2012 02:49 AM, Jason Merrill wrote:
>>> On 01/01/2012 08:10 PM, Paolo Carlini wrote:
>>>> The analysis is confirmed by the fact that the rather heavy handed
>>>> patchlet which I'm attaching, which uses copy_node to avoid the
>>>> corruption, "works". How do we normally handle this kind of situation?
>>> In most cases, trees are unshared at instantiation time, but that doesn't
>>> apply to constants like this.  The patch is OK.
>> Oh I see, thanks. Then I'm going to properly test it, etc, and if everything
>> goes well, I'll post what I will actually commit.
> I think it's enough to call copy_node for CONSTANT_CLASS_P objects.
In this branch we are always dealing with STRING_CST as init, I don't 
think we are going to discriminate much by using that predicate. Or 
maybe I'm not getting your point.

Anyway, in the meanwhile I investigated the issue somewhat more, and my 
tentative fix indeed works but it's definitely a too big hammer in terms 
of memory management and memcpy for no reason: we would copy_node very 
often, also when a plain char array is initialized with a plain char 
string literal, or a wchar_t from a wchar_t. Bad, because currently many 
circumstances in which we do TREE_TYPE (init) = type are in fact benign: 
char - char and wchar_t - wchar_t definitely work unconditionally (and 
within an individual instantiation we don't have problems, only between 
instantiations). Thus I'm thinking the below would be better (and safe), 
copy_node should trigger much less often, essentially, only when an 
explicitly unsigned / signed char array is involved.

Tested x86_64-linux.

Thanks,
Paolo.

/////////////////////////
/cp
2012-01-02  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/20140
	* typeck2.c (digest_init_r): Use copy_init when initializing
	unsigned and signed char arrays.

/testsuite
2012-01-02  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/20140
	* g++.dg/template/init9.C: New.

Comments

Jason Merrill Jan. 2, 2012, 3:06 p.m. UTC | #1 
On 01/02/2012 08:25 AM, Paolo Carlini wrote:
> Bad, because currently many
> circumstances in which we do TREE_TYPE (init) = type are in fact benign:

I don't think they are benign.  Even when types are equivalent, we need 
to convert between them, as several things in the back end rely on 
pointer equality, and we don't want different instantiations changing 
the type in subtly different ways.

Jason
diff mbox

Patch

Index: testsuite/g++.dg/template/init9.C
===================================================================
--- testsuite/g++.dg/template/init9.C	(revision 0)
+++ testsuite/g++.dg/template/init9.C	(revision 0)
@@ -0,0 +1,12 @@ 
+// PR c++/20140
+
+template<typename T> void foo()
+{
+  unsigned char s[] = "";
+}
+
+int main()
+{
+  foo<char>();
+  foo<int>();
+}
Index: cp/typeck2.c
===================================================================
--- cp/typeck2.c	(revision 182784)
+++ cp/typeck2.c	(working copy)
@@ -902,6 +902,8 @@  digest_init_r (tree type, tree init, bool nested,
 		}
 	    }
 
+	  if (char_type != typ1)
+	    init = copy_node (init);
 	  TREE_TYPE (init) = type;
 	  if (TYPE_DOMAIN (type) != 0 && TREE_CONSTANT (TYPE_SIZE (type)))
 	    {