Re: xsrc/58133: X server crashes; radeon 5450; modesetting

[Rhialto <rhialto%falu.nl@localhost>]

The following reply was made to PR xsrc/58133; it has been noted by GNATS.

From: Rhialto <rhialto%falu.nl@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: xsrc-manager%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost,
	Rhialto <rhialto%falu.nl@localhost>
Subject: Re: xsrc/58133: X server crashes; radeon 5450; modesetting
Date: Sun, 14 Apr 2024 21:24:06 +0200

 I noticed that in the cases where I could check, vbo_offset == 0.
 That means that the flow through glamor_get_vbo_space() must have gone
 like so:
 
 glamor_get_vbo_space(ScreenPtr screen, unsigned size, char **vbo_offset)
 {
     glamor_screen_private *glamor_priv = glamor_get_screen_private(screen);
     void *data;
 
     glamor_make_current(glamor_priv);
 
     glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
 
     if (glamor_priv->has_buffer_storage) {	// YES
       if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
            if (glamor_priv->vbo_size)
                 glUnmapBuffer(GL_ARRAY_BUFFER);
 
             if (size > glamor_priv->vbo_size) { // NO
 		// allocate a bigger buffer,
 		// glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
             }
 
             glamor_priv->vbo_offset = 0;
             glamor_priv->vb = glMapBufferRange(GL_ARRAY_BUFFER,
                                                0, glamor_priv->vbo_size,
                                                GL_MAP_WRITE_BIT |
                                                GL_MAP_INVALIDATE_BUFFER_BIT |
                                                GL_MAP_PERSISTENT_BIT |
                                                GL_MAP_COHERENT_BIT);
         }
         *vbo_offset = (void *)(uintptr_t)glamor_priv->vbo_offset;
         data = glamor_priv->vb + glamor_priv->vbo_offset;
         glamor_priv->vbo_offset += size;
     } else // IRRELEVANT
     return data;
 }
 
 This is consistent with the case where the crash was in
 glamor_composite_glyphs(), where `glamor_screen_private *glamor_priv` is
 available, and I could check for has_buffer_storage etc.
 
 I couldn't help wondering if glMapBufferRange() (documented at
 https://registry.khronos.org/OpenGL-Refpages/gl4/html/glMapBufferRange.xhtml
 I think) might perhaps work (slightly) async, so that by the time
 `*data` is written to, the mapping is not yet complete. gdb had no
 trouble to read v[0] in the core dump.
 
 Also I wonder why GL_MAP_READ_BIT is not included. I only see writing to
 the buffer, but who trusts the compiler not to read from it...
 
 I also wonder why the unmapping and remapping of the buffer isn't moved
 into the inner condition (the one which is false for our case, where a
 bigger buffer is allocated). That ought to be more efficient.
 
 It is unlikely that this is the first time through
 glamor_get_vbo_space(); many characters have been printed before.
 
 In case it matters, in most of my xterms I use bitmap fonts,
 lucidasanstypewriter-12, which is as far as I can tell indeed a 7x13
 font (width and height as seen in `info locals` before).