Re: [PATCH] make ggc pick up comp_dir_string() cache value. (dwarf2out.c)

["Otto, Thomas" <thomas dot otto at pdv-fs dot de>]

> > > > "ggc_collect() discarding/reusing remap_debug_filename() output,
> > > > thus producing invalid objects"
> > >
> > > Hmm, but AFAICS it can end up on the heap if plain get_src_pwd ()
> > > result survives.  I vaguely remember GC being happy with heap
> > > strings (due to identifiers?), but not sure.  Otherwise the patch looks
> > > obvious enough.
> >
> > Good point, remap_filename can return the original filename pointer
> > (which can point to env["PWD"] or to an allocated string) or a ggc string.
> >
> > Since not freeing the string pointed to by a static variable is ok (as
> > getpwd does it), what about checking if remap_filename just returns
> > the input filename ptr again, and if not copy the ggc string into an allocated
> > buffer.
> 
> I think we always want GC allocated storage here since the whole dwarf2out
> data structures live in GC memory.  That means unconditionally copying there
> as is done inside the DWARF2_DIR_SHOULD_END_WITH_SEPARATOR path.

Explicitly requesting gc storage (A or B below) doesn't seem to work. The mapped 
wd string is not modified this time but is placed in a very different part of the 
assembly, in .section .debug_str.dwo, and after assembling it does not even make it 
into object, not even in a corrupted form as previously.

Placing the static cached_wd on function or file level makes no difference, only the 
GTY(()) marker fixes it again. Or of course allocating a buffer myself (C).

And while DWARF2_DIR_SHOULD_END_WITH_SEPARATOR is only true on VMS, if I 
enter this if block and do not specify -fdebug-prefix-map then the final object does 
not even contain the working directory, probably because it is stored in 
the ggc_vec_alloc data. Tested with 8.3 built with 4.9 and now also 8.3 itself. Maybe 
gc and static storage do not play well together.

That leaves classic allocation of memory and pointing the cached_wd there. It 
really shouldn't matter since it is just a cache of an otherwise side effect free 
function return value. It would outlive all of its callers anyhow, gc or not.


  static const char *cached_wd = NULL;
  // [...]
  cached_wd = remap_debug_filename (wd);
  if (cached_wd != wd)
    {
      size_t cached_wd_len = strlen (cached_wd);
A:        char *buf = ggc_vec_alloc<char> (cached_wd_len);
B:        char *buf = (char*)ggc_alloc_atomic (cached_wd_len);
C:        char *buf = (char*)xmalloc (cached_wd_len);
      memcpy (buf, cached_wd, cached_wd_len);
      cached_wd = buf;
    }