[gcc r15-3891] pretty-print: Fix up allocate_object

[Jakub Jelinek]

https://gcc.gnu.org/g:9dc1ab9062e93ae178830d66d5850406777a477d

commit r15-3891-g9dc1ab9062e93ae178830d66d5850406777a477d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Sep 26 11:55:13 2024 +0200

    pretty-print: Fix up allocate_object
    
    On Thu, Aug 29, 2024 at 06:58:12PM -0400, David Malcolm wrote:
    > The following patch rewrites the internals of pp_format.
    
    > The tokens and token lists are allocated on the chunk_obstack, and so
    > there's no additional heap activity required, with the memory reclaimed
    > when the chunk_obstack is freed after phase 3 of formatting.
    
    > +static void *
    > +allocate_object (size_t sz, obstack &s)
    > +{
    > +  /* We must not be half-way through an object.  */
    > +  gcc_assert (obstack_base (&s) == obstack_next_free (&s));
    > +
    > +  obstack_grow (&s, obstack_base (&s), sz);
    > +  void *buf = obstack_finish (&s);
    > +  return buf;
    >  }
    
    I think this is wrong.  I hoped it would be the reason of the
    unexpected libstdc++ warnings on certain architectures after
    seeing
    ==4027220== Source and destination overlap in memcpy(0x4627154, 0x4627154, 12)
    ==4027220==    at 0x404B93E: memcpy (vg_replace_strmem.c:1123)
    ==4027220==    by 0xAAD5618: allocate_object(unsigned int, obstack&) (pretty-print.cc:1183)
    ==4027220==    by 0xAAD8C0E: operator new (pretty-print.cc:1210)
    ==4027220==    by 0xAAD8C0E: make (pretty-print-format-impl.h:305)
    ==4027220==    by 0xAAD8C0E: format_phase_1 (pretty-print.cc:1659)
    ==4027220==    by 0xAAD8C0E: pretty_printer::format(text_info&) (pretty-print.cc:1618)
    ==4027220==    by 0xAAA840E: pp_format (pretty-print.h:583)
    ==4027220==    by 0xAAA840E: diagnostic_context::report_diagnostic(diagnostic_info*) (diagnostic.cc:1260)
    ==4027220==    by 0xAAA8703: diagnostic_context::diagnostic_impl(rich_location*, diagnostic_metadata const*, diagnostic_option_id, char const*, char**, diagnostic_t) (diagnostic.cc:1404)
    ==4027220==    by 0xAAB8682: warning(diagnostic_option_id, char const*, ...) (diagnostic-global-context.cc:166)
    ==4027220==    by 0x97725F5: warn_deprecated_use(tree_node*, tree_node*) (tree.cc:12485)
    ==4027220==    by 0x8B6694B: mark_used(tree_node*, int) (decl2.cc:6121)
    ==4027220==    by 0x8C9E25E: tsubst_expr(tree_node*, tree_node*, int, tree_node*) [clone .part.0] (pt.cc:21626)
    ==4027220==    by 0x8C9E5E6: tsubst_expr(tree_node*, tree_node*, int, tree_node*) [clone .part.0] (pt.cc:20935)
    ==4027220==    by 0x8C9E1D7: tsubst_expr(tree_node*, tree_node*, int, tree_node*) [clone .part.0] (pt.cc:20424)
    ==4027220==    by 0x8C9DF2E: tsubst_expr(tree_node*, tree_node*, int, tree_node*) [clone .part.0] (pt.cc:20496)
    ==4027220==
    etc. valgrind warnings, unfortunately it is not, but still
    I think this is a bug.
    If the obstack has enough space in it, i.e. if obstack_room (&s) >= sz,
    then obstack_grow from obstack_base will copy uninitialized bytes
    through memcpy (obstack_base (&s), obstack_base (&s), sz);
    (which pedantically isn't valid due to the overlap, and so
    the reason why valgrind complains, but in reality I think most
    implementations can handle it fine, after all, we also use it for
    structure assignments which could have full or no overlap but never
    partial).
    If obstack_room (&s) < sz, then obstack_grow will first
    _obstack_newchunk (&s, sz); which will allocate new memory and
    copy the existing data of the object (but the above assertion
    guartantees it will copy 0 bytes) and then the memcpy copies
    sz bytes from the old base to the new (if unlucky, that could crash
    as there could be end of page and unmapped next page in between).
    
    I think we should use obstack_blank instead of obstack_grow, which
    does everything obstack_grow does, except for the memcpy of the
    uninitialized data.
    
    2024-09-25  Jakub Jelinek  <jakub@redhat.com>
    
            * pretty-print.cc (allocate_object): Use obstack_blank rather than
            obstack_grow.

Diff:
---
 gcc/pretty-print.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/pretty-print.cc b/gcc/pretty-print.cc
index 0cd9b4d0a416..68c145e2d532 100644
--- a/gcc/pretty-print.cc
+++ b/gcc/pretty-print.cc
@@ -1180,7 +1180,7 @@ allocate_object (size_t sz, obstack &s)
   /* We must not be half-way through an object.  */
   gcc_assert (obstack_base (&s) == obstack_next_free (&s));
 
-  obstack_grow (&s, obstack_base (&s), sz);
+  obstack_blank (&s, sz);
   void *buf = obstack_finish (&s);
   return buf;
 }