[cairo] crash copying recording surface to PDF surface with tags

[Uli Schlachter]

Hi,

Am 26.12.20 um 19:46 schrieb Ben Pfaff:
> On Sat, Dec 26, 2020 at 10:25 AM Uli Schlachter <psychon at znc.in> wrote:
>> Am 26.12.20 um 19:12 schrieb Ben Pfaff:
>> [...]> I do still see the following use of an uninitialized value (I forgot
>>> to report this before, sorry!):
>>>     Conditional jump or move depends on uninitialised value(s)
>>>        at 0x494E67B: _cairo_pdf_surface_emit_recording_surface
>>> (cairo-pdf-surface.c:3375)

According to "git show 1.16.0:src/cairo-pdf-surface.c", line 3375 is the
second line of this multi-line statement:

  transparency_group = pdf_source->hash_entry->need_transp_group ||
    !(pdf_source->hash_entry->operator == CAIRO_OPERATOR_OVER &&
      _cairo_recording_surface_has_only_bilevel_alpha (recording) &&
      _cairo_recording_surface_has_only_op_over (recording));

[...]
> I do still see the use of an uninitialized value, though.  I added
> --track-origins to the valgrind command line and that gave the following
> additional information:
> 
>     Uninitialised value was created by a heap allocation
>       at 0x483877F: malloc (vg_replace_malloc.c:307)
>       by 0x48CF16E: _cairo_recording_surface_snapshot (cairo-recording-surface.c:1563)
[...]

so.... in the above line, one of the two statements with "recording"
must be accessing an uninitialised value. The two functions just return
recording->has_bilevel_alpha and recording->has_only_op_over,
respectively. In fact, those are the only two members that
cairo_recording_surface_create() initialises, but
_cairo_recording_surface_snapshot() does not.

Since I still cannot reproduce this (for whatever reason...), could you
test the following patch for me?

diff --git a/src/cairo-recording-surface.c b/src/cairo-recording-surface.c
index 6df8b0821..74823105a 100644
--- a/src/cairo-recording-surface.c
+++ b/src/cairo-recording-surface.c
@@ -1582,6 +1582,8 @@ _cairo_recording_surface_snapshot (void
*abstract_other)
     surface->indices = NULL;
     surface->num_indices = 0;
     surface->optimize_clears = TRUE;
+    surface->has_bilevel_alpha = other->has_bilevel_alpha;
+    surface->has_only_op_over = other->has_only_op_over;

     _cairo_array_init (&surface->commands, sizeof (cairo_command_t *));
     status = _cairo_recording_surface_copy (surface, other);

Thanks,
Uli
-- 
This can be a, a little complicated. Listen, my advice is... ask
somebody else for advice, at least someone who's... got more experience
at...  giving advice.