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: Wed, 10 Apr 2024 19:46:03 +0200

 So far I have tried to avoid reproducing it, but I can try :)
 I will add some logging, and since the callers^2 seem to have a fallback
 path, I will propagate the error condition a bit better so that that
 path should be taken. Something like this:
 
 --- glamor_text.c.orig	2024-04-10 19:22:22.286045752 +0200
 +++ glamor_text.c	2024-04-10 19:21:41.815261092 +0200
 @@ -120,6 +120,12 @@
      /* Set up the vertex buffers for the font and destination */
  
      v = glamor_get_vbo_space(drawable->pScreen, count * (6 * sizeof (GLshort)), &vbo_offset);
 +    if (v == NULL) {
 +	LogMessage(X_ERROR,
 +		   "glamor_text: glamor_get_vbo_space failed (%d)\n",
 +		   count * (6 * sizeof (GLshort)));
 +	return -1;
 +    }
  
      glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
      glVertexAttribDivisor(GLAMOR_VERTEX_POS, 1);
 @@ -293,10 +299,13 @@
      if (!prog)
          goto bail;
  
 -    x = glamor_text(drawable, gc, glamor_font, prog,
 +    int new_x = glamor_text(drawable, gc, glamor_font, prog,
                      x, y, count, chars, charinfo, sixteen);
  
 -    *final_pos = x;
 +    if (new_x != -1) {
 +	*final_pos = new_x;
 +	return TRUE;
 +    }
  
  bail:
      return FALSE;
 @@ -463,10 +472,12 @@
      if (!glamor_use_program(pixmap, gc, prog, NULL))
          goto bail;
  
 -    (void) glamor_text(drawable, gc, glamor_font, prog,
 +    int new_x =  glamor_text(drawable, gc, glamor_font, prog,
                         x, y, count, chars, charinfo, sixteen);
  
 -    return TRUE;
 +    if (new_x != -1) {
 +	return TRUE;
 +    }
  
  bail:
      return FALSE;
 
 I noticed that none of the other callers of glamor_get_vbo_space() check
 for NULL, though.
 
 I do have the core dump available and bzip2-compressed it is only 7.1M
 (64M uncompressed). If you want it I can send it off-gnats. Maybe you
 can examine that radeon_buffer_object that way (I haven't searched hard
 enough to find it yet).
 
 -Olaf.