diff mbox

Fixup T_U_D reading

Message ID alpine.LNX.2.00.1009171612410.28912@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Sept. 17, 2010, 2:13 p.m. UTC 
I forgot to copy the string - we only get a reference to the
string section.  Oops.

Bootstrapped and tested on x86_64-unknown-linux-gnu, committed.

Richard.

2010-09-17  Richard Guenther  <rguenther@suse.de>

	* lto-streamer-in.c (lto_input_ts_translation_unit_decl_tree_pointers):
	Properly copy the read string.

Comments

Steven Bosscher Sept. 17, 2010, 2:19 p.m. UTC | #1 
On Fri, Sep 17, 2010 at 4:13 PM, Richard Guenther <rguenther@suse.de> wrote:
>
> I forgot to copy the string - we only get a reference to the
> string section.  Oops.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, committed.
>
> Richard.
>
> 2010-09-17  Richard Guenther  <rguenther@suse.de>
>
>        * lto-streamer-in.c (lto_input_ts_translation_unit_decl_tree_pointers):
>        Properly copy the read string.
>
> Index: gcc/lto-streamer-in.c
> ===================================================================
> --- gcc/lto-streamer-in.c       (revision 164368)
> +++ gcc/lto-streamer-in.c       (working copy)
> @@ -2241,7 +2271,7 @@ lto_input_ts_translation_unit_decl_tree_
>                                                  struct data_in *data_in,
>                                                  tree expr)
>  {
> -  TRANSLATION_UNIT_LANGUAGE (expr) = input_string (data_in, ib);
> +  TRANSLATION_UNIT_LANGUAGE (expr) = xstrdup (input_string (data_in, ib));
>   VEC_safe_push (tree, gc, all_translation_units, expr);
>  }

I guess it's not worth sharing the language strings somehow??

Ciao!
Steven
Richard Biener Sept. 17, 2010, 2:20 p.m. UTC | #2 
On Fri, 17 Sep 2010, Steven Bosscher wrote:

> On Fri, Sep 17, 2010 at 4:13 PM, Richard Guenther <rguenther@suse.de> wrote:
> >
> > I forgot to copy the string - we only get a reference to the
> > string section.  Oops.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, committed.
> >
> > Richard.
> >
> > 2010-09-17  Richard Guenther  <rguenther@suse.de>
> >
> >        * lto-streamer-in.c (lto_input_ts_translation_unit_decl_tree_pointers):
> >        Properly copy the read string.
> >
> > Index: gcc/lto-streamer-in.c
> > ===================================================================
> > --- gcc/lto-streamer-in.c       (revision 164368)
> > +++ gcc/lto-streamer-in.c       (working copy)
> > @@ -2241,7 +2271,7 @@ lto_input_ts_translation_unit_decl_tree_
> >                                                  struct data_in *data_in,
> >                                                  tree expr)
> >  {
> > -  TRANSLATION_UNIT_LANGUAGE (expr) = input_string (data_in, ib);
> > +  TRANSLATION_UNIT_LANGUAGE (expr) = xstrdup (input_string (data_in, ib));
> >   VEC_safe_push (tree, gc, all_translation_units, expr);
> >  }
> 
> I guess it's not worth sharing the language strings somehow??

Well, it's going to be O(number of TUs) strings, so - no.  It might
be worth transitioning LANG_NAME from a string to an enum maybe ;)

Richard.
Jakub Jelinek Sept. 17, 2010, 2:26 p.m. UTC | #3 
On Fri, Sep 17, 2010 at 04:20:33PM +0200, Richard Guenther wrote:
> > >  {
> > > -  TRANSLATION_UNIT_LANGUAGE (expr) = input_string (data_in, ib);
> > > +  TRANSLATION_UNIT_LANGUAGE (expr) = xstrdup (input_string (data_in, ib));
> > >   VEC_safe_push (tree, gc, all_translation_units, expr);
> > >  }
> > 
> > I guess it's not worth sharing the language strings somehow??
> 
> Well, it's going to be O(number of TUs) strings, so - no.  It might
> be worth transitioning LANG_NAME from a string to an enum maybe ;)

Perhaps putting there not a const char *, but IDENTIFIER and use
get_identifier ("GNU C") etc.?

	Jakub
Richard Biener Sept. 17, 2010, 2:32 p.m. UTC | #4 
On Fri, 17 Sep 2010, Jakub Jelinek wrote:

> On Fri, Sep 17, 2010 at 04:20:33PM +0200, Richard Guenther wrote:
> > > >  {
> > > > -  TRANSLATION_UNIT_LANGUAGE (expr) = input_string (data_in, ib);
> > > > +  TRANSLATION_UNIT_LANGUAGE (expr) = xstrdup (input_string (data_in, ib));
> > > >   VEC_safe_push (tree, gc, all_translation_units, expr);
> > > >  }
> > > 
> > > I guess it's not worth sharing the language strings somehow??
> > 
> > Well, it's going to be O(number of TUs) strings, so - no.  It might
> > be worth transitioning LANG_NAME from a string to an enum maybe ;)
> 
> Perhaps putting there not a const char *, but IDENTIFIER and use
> get_identifier ("GNU C") etc.?

Well, as I said, if changing anything changing it to an enum makes
more sense than trying to save a few bytes via string sharing
(in fact LANG_NAME is also not an identifier).

Richard.
Richard Henderson Sept. 17, 2010, 8:35 p.m. UTC | #5 
On 09/17/2010 07:32 AM, Richard Guenther wrote:
> On Fri, 17 Sep 2010, Jakub Jelinek wrote:
> 
>> On Fri, Sep 17, 2010 at 04:20:33PM +0200, Richard Guenther wrote:
>>>>>  {
>>>>> -  TRANSLATION_UNIT_LANGUAGE (expr) = input_string (data_in, ib);
>>>>> +  TRANSLATION_UNIT_LANGUAGE (expr) = xstrdup (input_string (data_in, ib));
>>>>>   VEC_safe_push (tree, gc, all_translation_units, expr);
>>>>>  }
>>>>
>>>> I guess it's not worth sharing the language strings somehow??
>>>
>>> Well, it's going to be O(number of TUs) strings, so - no.  It might
>>> be worth transitioning LANG_NAME from a string to an enum maybe ;)
>>
>> Perhaps putting there not a const char *, but IDENTIFIER and use
>> get_identifier ("GNU C") etc.?
> 
> Well, as I said, if changing anything changing it to an enum makes
> more sense than trying to save a few bytes via string sharing
> (in fact LANG_NAME is also not an identifier).

For the enum, use DW_LANG_* ?


r~
Richard Biener Sept. 20, 2010, 12:58 p.m. UTC | #6 
On Fri, 17 Sep 2010, Richard Henderson wrote:

> On 09/17/2010 07:32 AM, Richard Guenther wrote:
> > On Fri, 17 Sep 2010, Jakub Jelinek wrote:
> > 
> >> On Fri, Sep 17, 2010 at 04:20:33PM +0200, Richard Guenther wrote:
> >>>>>  {
> >>>>> -  TRANSLATION_UNIT_LANGUAGE (expr) = input_string (data_in, ib);
> >>>>> +  TRANSLATION_UNIT_LANGUAGE (expr) = xstrdup (input_string (data_in, ib));
> >>>>>   VEC_safe_push (tree, gc, all_translation_units, expr);
> >>>>>  }
> >>>>
> >>>> I guess it's not worth sharing the language strings somehow??
> >>>
> >>> Well, it's going to be O(number of TUs) strings, so - no.  It might
> >>> be worth transitioning LANG_NAME from a string to an enum maybe ;)
> >>
> >> Perhaps putting there not a const char *, but IDENTIFIER and use
> >> get_identifier ("GNU C") etc.?
> > 
> > Well, as I said, if changing anything changing it to an enum makes
> > more sense than trying to save a few bytes via string sharing
> > (in fact LANG_NAME is also not an identifier).
> 
> For the enum, use DW_LANG_* ?

Yes, certainly one possibility.  I've placed it on my list of things
to do (replace LANG_NAME with that as well - I love the strcmp in
fold-const.c ...).

Richard.
diff mbox

Patch

Index: gcc/lto-streamer-in.c
===================================================================
--- gcc/lto-streamer-in.c	(revision 164368)
+++ gcc/lto-streamer-in.c	(working copy)
@@ -2241,7 +2271,7 @@  lto_input_ts_translation_unit_decl_tree_
 						  struct data_in *data_in,
 						  tree expr)
 {
-  TRANSLATION_UNIT_LANGUAGE (expr) = input_string (data_in, ib);
+  TRANSLATION_UNIT_LANGUAGE (expr) = xstrdup (input_string (data_in, ib));
   VEC_safe_push (tree, gc, all_translation_units, expr);
 }