[Mesa-stable] [Mesa-dev][PATCH v2] Fixing an x86 FPU bug.

marius marius.predut at intel.com
Thu Jan 22 14:00:28 PST 2015


From: Marius Predut <marius.predut at intel.com>

On 32-bit, for floating point operations is used x86 FPU registers instead SSE,
reason for  when reinterprets an integer as a float result is unexpected
(modify floats when they are written to memory).
The defect was checked with and without -O3 compiler flag.

CC: <mesa-stable at lists.freedesktop.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668
Signed-off-by: Marius Predut <marius.predut at intel.com>
---
 src/mesa/main/context.c       |    3 ++-
 src/mesa/main/macros.h        |   47 ++++++++++++++++++++++++-----------------
 src/mesa/vbo/vbo_attrib_tmp.h |   20 ++++++++++++++----
 src/mesa/vbo/vbo_exec.h       |    3 ++-
 src/mesa/vbo/vbo_exec_api.c   |   31 +++++++++++++--------------
 src/mesa/vbo/vbo_exec_eval.c  |   22 ++++++++++++++-----
 src/mesa/vbo/vbo_save_api.c   |   16 +++++++-------
 src/mesa/vbo/vbo_save_draw.c  |    4 ++--
 8 files changed, 90 insertions(+), 56 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 400c158..11ab8a9 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -134,6 +134,7 @@
 #include "math/m_matrix.h"
 #include "main/dispatch.h" /* for _gloffset_COUNT */
 #include "uniforms.h"
+#include "macros.h"
 
 #ifdef USE_SPARC_ASM
 #include "sparc/sparc.h"
@@ -656,7 +657,7 @@ _mesa_init_constants(struct gl_constants *consts, gl_api api)
    consts->MaxSamples = 0;
 
    /* GLSL default if NativeIntegers == FALSE */
-   consts->UniformBooleanTrue = FLT_AS_UINT(1.0f);
+   consts->UniformBooleanTrue = FLOAT_AS_UNION(1.0f).u;
 
    /* GL_ARB_sync */
    consts->MaxServerWaitTimeout = 0x1fff7fffffffULL;
diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
index cd5f2d6..2651ffc 100644
--- a/src/mesa/main/macros.h
+++ b/src/mesa/main/macros.h
@@ -32,6 +32,7 @@
 #define MACROS_H
 
 #include "imports.h"
+#include "program/prog_parameter.h"
 
 
 /**
@@ -170,27 +171,26 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
 	ub = ((GLubyte) F_TO_I((f) * 255.0F))
 #endif
 
-static inline GLfloat INT_AS_FLT(GLint i)
+static union gl_constant_value UINT_AS_UNION(GLuint u)
 {
-   fi_type tmp;
-   tmp.i = i;
-   return tmp.f;
+   union gl_constant_value tmp;
+   tmp.u = u;
+   return tmp;
 }
 
-static inline GLfloat UINT_AS_FLT(GLuint u)
+static inline union gl_constant_value INT_AS_UNION(GLint i)
 {
-   fi_type tmp;
-   tmp.u = u;
-   return tmp.f;
+   union gl_constant_value tmp;
+   tmp.i = i;
+   return tmp;
 }
 
-static inline unsigned FLT_AS_UINT(float f)
+static inline union gl_constant_value FLOAT_AS_UNION(GLfloat f)
 {
-   fi_type tmp;
+   union gl_constant_value tmp;
    tmp.f = f;
-   return tmp.u;
+   return tmp;
 }
-
 /**
  * Convert a floating point value to an unsigned fixed point value.
  *
@@ -382,6 +382,15 @@ do {                                    \
     V[3] = V3;                          \
 } while(0)
 
+/** Assignment union*/
+#define ASSIGN_4V_UNION( V, V0, V1, V2, V3 )  \
+do {                                    \
+    V[0].f = V0;                          \
+    V[1].f = V1;                          \
+    V[2].f = V2;                          \
+    V[3].f = V3;                          \
+} while(0)
+
 /*@}*/
 
 
@@ -620,23 +629,23 @@ do {				\
  * The default values are chosen based on \p type.
  */
 static inline void
-COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz, const GLfloat src[4],
+COPY_CLEAN_4V_TYPE_AS_FLOAT(gl_constant_value dst[4], int sz, const gl_constant_value src[4],
                             GLenum type)
 {
    switch (type) {
    case GL_FLOAT:
-      ASSIGN_4V(dst, 0, 0, 0, 1);
+      ASSIGN_4V_UNION(dst, 0, 0, 0, 1);
       break;
    case GL_INT:
-      ASSIGN_4V(dst, INT_AS_FLT(0), INT_AS_FLT(0),
-                     INT_AS_FLT(0), INT_AS_FLT(1));
+      ASSIGN_4V_UNION(dst, INT_AS_UNION(0).f, INT_AS_UNION(0).f,
+                INT_AS_UNION(0).f, INT_AS_UNION(1).f);
       break;
    case GL_UNSIGNED_INT:
-      ASSIGN_4V(dst, UINT_AS_FLT(0), UINT_AS_FLT(0),
-                     UINT_AS_FLT(0), UINT_AS_FLT(1));
+      ASSIGN_4V_UNION(dst, UINT_AS_UNION(0).f, UINT_AS_UNION(0).f,
+                UINT_AS_UNION(0).f, UINT_AS_UNION(1).f);
       break;
    default:
-      ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
+      ASSIGN_4V_UNION(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
       ASSERT(!"Unexpected type in COPY_CLEAN_4V_TYPE_AS_FLOAT macro");
    }
    COPY_SZ_4V(dst, sz, src);
diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
index ec66934..17a0a10 100644
--- a/src/mesa/vbo/vbo_attrib_tmp.h
+++ b/src/mesa/vbo/vbo_attrib_tmp.h
@@ -28,6 +28,18 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "util/u_format_r11g11b10f.h"
 #include "main/varray.h"
 
+
+/* ATTR */
+#define ATTR( A, N, T, V0, V1, V2, V3 )	  ATTR_##T((A), (N), (T), (V0), (V1), (V2), (V3))
+
+#define ATTR_GL_UNSIGNED_INT( A, N, T, V0, V1, V2, V3 ) \
+    ATTR_UNION(A, N, T, UINT_AS_UNION(V0), UINT_AS_UNION(V1), UINT_AS_UNION(V2), UINT_AS_UNION(V3))
+#define ATTR_GL_INT( A, N, T, V0, V1, V2, V3 ) \
+    ATTR_UNION(A, N, T, INT_AS_UNION(V0), INT_AS_UNION(V1), INT_AS_UNION(V2), INT_AS_UNION(V3))
+#define ATTR_GL_FLOAT( A, N, T, V0, V1, V2, V3 )	\
+    ATTR_UNION(A, N, T, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1), FLOAT_AS_UNION(V2), FLOAT_AS_UNION(V3))
+
+
 /* float */
 #define ATTR1FV( A, V ) ATTR( A, 1, GL_FLOAT, (V)[0], 0, 0, 1 )
 #define ATTR2FV( A, V ) ATTR( A, 2, GL_FLOAT, (V)[0], (V)[1], 0, 1 )
@@ -41,8 +53,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 /* int */
 #define ATTRI( A, N, X, Y, Z, W) ATTR( A, N, GL_INT, \
-                                       INT_AS_FLT(X), INT_AS_FLT(Y), \
-                                       INT_AS_FLT(Z), INT_AS_FLT(W) )
+                                       X, Y, \
+                                       Z, W )
 
 #define ATTR2IV( A, V ) ATTRI( A, 2, (V)[0], (V)[1], 0, 1 )
 #define ATTR3IV( A, V ) ATTRI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
@@ -56,8 +68,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 /* uint */
 #define ATTRUI( A, N, X, Y, Z, W) ATTR( A, N, GL_UNSIGNED_INT, \
-                                        UINT_AS_FLT(X), UINT_AS_FLT(Y), \
-                                        UINT_AS_FLT(Z), UINT_AS_FLT(W) )
+                                        X, Y, \
+                                        Z, W )
 
 #define ATTR2UIV( A, V ) ATTRUI( A, 2, (V)[0], (V)[1], 0, 1 )
 #define ATTR3UIV( A, V ) ATTRUI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
diff --git a/src/mesa/vbo/vbo_exec.h b/src/mesa/vbo/vbo_exec.h
index bb265de..7db6b6b 100644
--- a/src/mesa/vbo/vbo_exec.h
+++ b/src/mesa/vbo/vbo_exec.h
@@ -38,6 +38,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "vbo.h"
 #include "vbo_attrib.h"
 
+#include "program/prog_parameter.h"
 
 /**
  * Max number of primitives (number of glBegin/End pairs) per VBO.
@@ -104,7 +105,7 @@ struct vbo_exec_context
       GLenum attrtype[VBO_ATTRIB_MAX];
       GLubyte active_sz[VBO_ATTRIB_MAX];
 
-      GLfloat *attrptr[VBO_ATTRIB_MAX]; 
+      gl_constant_value *attrptr[VBO_ATTRIB_MAX];
       struct gl_client_array arrays[VERT_ATTRIB_MAX];
 
       /* According to program mode, the values above plus current
diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
index 5f8250e..a61efde 100644
--- a/src/mesa/vbo/vbo_exec_api.c
+++ b/src/mesa/vbo/vbo_exec_api.c
@@ -46,7 +46,6 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "vbo_context.h"
 #include "vbo_noop.h"
 
-
 #ifdef ERROR
 #undef ERROR
 #endif
@@ -159,7 +158,7 @@ static void vbo_exec_copy_to_current( struct vbo_exec_context *exec )
           * ctx->Current.Attrib and ctx->Light.Material.Attrib arrays.
           */
 	 GLfloat *current = (GLfloat *)vbo->currval[i].Ptr;
-         GLfloat tmp[4];
+         gl_constant_value tmp[4];
 
          COPY_CLEAN_4V_TYPE_AS_FLOAT(tmp,
                                      exec->vtx.attrsz[i],
@@ -216,10 +215,10 @@ vbo_exec_copy_from_current(struct vbo_exec_context *exec)
    for (i = VBO_ATTRIB_POS + 1; i < VBO_ATTRIB_MAX; i++) {
       const GLfloat *current = (GLfloat *) vbo->currval[i].Ptr;
       switch (exec->vtx.attrsz[i]) {
-      case 4: exec->vtx.attrptr[i][3] = current[3];
-      case 3: exec->vtx.attrptr[i][2] = current[2];
-      case 2: exec->vtx.attrptr[i][1] = current[1];
-      case 1: exec->vtx.attrptr[i][0] = current[0];
+      case 4: exec->vtx.attrptr[i][3].f = current[3];
+      case 3: exec->vtx.attrptr[i][2].f = current[2];
+      case 2: exec->vtx.attrptr[i][1].f = current[1];
+      case 1: exec->vtx.attrptr[i][0].f = current[0];
 	 break;
       }
    }
@@ -291,7 +290,7 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context *exec,
 
       for (i = 0 ; i < VBO_ATTRIB_MAX ; i++) {
 	 if (exec->vtx.attrsz[i]) {
-	    exec->vtx.attrptr[i] = tmp;
+	    exec->vtx.attrptr[i] = (gl_constant_value*) tmp;
 	    tmp += exec->vtx.attrsz[i];
 	 }
 	 else
@@ -305,8 +304,8 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context *exec,
    }
    else {
       /* Just have to append the new attribute at the end */
-      exec->vtx.attrptr[attr] = exec->vtx.vertex +
-	 exec->vtx.vertex_size - newSize;
+      exec->vtx.attrptr[attr] = (gl_constant_value*)(exec->vtx.vertex +
+        exec->vtx.vertex_size - newSize);
    }
 
    /* Replay stored vertices to translate them
@@ -327,15 +326,15 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context *exec,
 
 	    if (sz) {
 	       GLint old_offset = old_attrptr[j] - exec->vtx.vertex;
-	       GLint new_offset = exec->vtx.attrptr[j] - exec->vtx.vertex;
+	       GLint new_offset = (GLfloat*)exec->vtx.attrptr[j] - exec->vtx.vertex;
 
 	       if (j == attr) {
 		  if (oldSize) {
-		     GLfloat tmp[4];
+             gl_constant_value tmp[4];
                      COPY_CLEAN_4V_TYPE_AS_FLOAT(tmp, oldSize,
-                                                 data + old_offset,
+                                                 (gl_constant_value*)(data + old_offset),
                                                  exec->vtx.attrtype[j]);
-		     COPY_SZ_4V(dest + new_offset, newSize, tmp);
+             COPY_SZ_4V(dest + new_offset, newSize, (GLfloat*)tmp);
 		  } else {
 		     GLfloat *current = (GLfloat *)vbo->currval[j].Ptr;
 		     COPY_SZ_4V(dest + new_offset, sz, current);
@@ -383,7 +382,7 @@ vbo_exec_fixup_vertex(struct gl_context *ctx, GLuint attr, GLuint newSize)
        * zeros.  Don't need to flush or wrap.
        */
       for (i = newSize; i <= exec->vtx.attrsz[attr]; i++)
-	 exec->vtx.attrptr[attr][i-1] = id[i-1];
+        exec->vtx.attrptr[attr][i-1].f = id[i-1];
    }
 
    exec->vtx.active_sz[attr] = newSize;
@@ -401,7 +400,7 @@ vbo_exec_fixup_vertex(struct gl_context *ctx, GLuint attr, GLuint newSize)
  * This macro is used to implement all the glVertex, glColor, glTexCoord,
  * glVertexAttrib, etc functions.
  */
-#define ATTR( A, N, T, V0, V1, V2, V3 )					\
+#define ATTR_UNION( A, N, T, V0, V1, V2, V3 )					\
 do {									\
    struct vbo_exec_context *exec = &vbo_context(ctx)->exec;		\
 									\
@@ -412,7 +411,7 @@ do {									\
       vbo_exec_fixup_vertex(ctx, A, N);					\
    									\
    {									\
-      GLfloat *dest = exec->vtx.attrptr[A];				\
+      gl_constant_value *dest = exec->vtx.attrptr[A];				\
       if (N>0) dest[0] = V0;						\
       if (N>1) dest[1] = V1;						\
       if (N>2) dest[2] = V2;						\
diff --git a/src/mesa/vbo/vbo_exec_eval.c b/src/mesa/vbo/vbo_exec_eval.c
index 82f89b9..670e763 100644
--- a/src/mesa/vbo/vbo_exec_eval.c
+++ b/src/mesa/vbo/vbo_exec_eval.c
@@ -33,6 +33,18 @@
 #include "vbo_exec.h"
 
 
+/** Copy \p SZ elements into a 4-element vector */
+#define COPY_UNION_SZ_4V(DST, SZ, SRC)  \
+do {                              \
+   switch (SZ) {                  \
+   case 4: (DST)[3].f = (SRC)[3];   \
+   case 3: (DST)[2].f = (SRC)[2];   \
+   case 2: (DST)[1].f = (SRC)[1];   \
+   case 1: (DST)[0].f = (SRC)[0];   \
+   }                              \
+} while(0)
+
+
 static void clear_active_eval1( struct vbo_exec_context *exec, GLuint attr ) 
 {
    assert(attr < Elements(exec->eval.map1));
@@ -138,9 +150,9 @@ void vbo_exec_do_EvalCoord1f(struct vbo_exec_context *exec, GLfloat u)
 				   exec->eval.map1[attr].sz, 
 				   map->Order);
 
-	 COPY_SZ_4V( exec->vtx.attrptr[attr],
-		     exec->vtx.attrsz[attr],
-		     data );
+	 COPY_UNION_SZ_4V( exec->vtx.attrptr[attr],
+             exec->vtx.attrsz[attr],
+             data );
       }
    }
 
@@ -186,7 +198,7 @@ void vbo_exec_do_EvalCoord2f( struct vbo_exec_context *exec,
 				  exec->eval.map2[attr].sz, 
 				  map->Uorder, map->Vorder);
 
-	 COPY_SZ_4V( exec->vtx.attrptr[attr],
+	 COPY_UNION_SZ_4V( exec->vtx.attrptr[attr],
 		     exec->vtx.attrsz[attr],
 		     data );
       }
@@ -225,7 +237,7 @@ void vbo_exec_do_EvalCoord2f( struct vbo_exec_context *exec,
          NORMALIZE_3FV(normal);
 	 normal[3] = 1.0;
 
-	 COPY_SZ_4V( exec->vtx.attrptr[VBO_ATTRIB_NORMAL],
+ 	 COPY_UNION_SZ_4V( exec->vtx.attrptr[VBO_ATTRIB_NORMAL],
 		     exec->vtx.attrsz[VBO_ATTRIB_NORMAL],
 		     normal );
 
diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
index 5055c22..c24adde 100644
--- a/src/mesa/vbo/vbo_save_api.c
+++ b/src/mesa/vbo/vbo_save_api.c
@@ -575,8 +575,8 @@ _save_copy_to_current(struct gl_context *ctx)
    for (i = VBO_ATTRIB_POS + 1; i < VBO_ATTRIB_MAX; i++) {
       if (save->attrsz[i]) {
          save->currentsz[i][0] = save->attrsz[i];
-         COPY_CLEAN_4V_TYPE_AS_FLOAT(save->current[i], save->attrsz[i],
-                                     save->attrptr[i], save->attrtype[i]);
+         COPY_CLEAN_4V_TYPE_AS_FLOAT((gl_constant_value*)save->current[i], save->attrsz[i],
+                                     (gl_constant_value*)save->attrptr[i], save->attrtype[i]);
       }
    }
 }
@@ -682,7 +682,7 @@ _save_upgrade_vertex(struct gl_context *ctx, GLuint attr, GLuint newsz)
             if (save->attrsz[j]) {
                if (j == attr) {
                   if (oldsz) {
-                     COPY_CLEAN_4V_TYPE_AS_FLOAT(dest, oldsz, data,
+                     COPY_CLEAN_4V_TYPE_AS_FLOAT((gl_constant_value*)dest, oldsz, (gl_constant_value*)data,
                                                  save->attrtype[j]);
                      data += oldsz;
                      dest += newsz;
@@ -769,7 +769,7 @@ _save_reset_vertex(struct gl_context *ctx)
  * 3f version won't otherwise set color[3] to 1.0 -- this is the job
  * of the chooser function when switching between Color4f and Color3f.
  */
-#define ATTR(A, N, T, V0, V1, V2, V3)				\
+#define ATTR_UNION(A, N, T, V0, V1, V2, V3)				\
 do {								\
    struct vbo_save_context *save = &vbo_context(ctx)->save;	\
 								\
@@ -778,10 +778,10 @@ do {								\
 								\
    {								\
       GLfloat *dest = save->attrptr[A];				\
-      if (N>0) dest[0] = V0;					\
-      if (N>1) dest[1] = V1;					\
-      if (N>2) dest[2] = V2;					\
-      if (N>3) dest[3] = V3;					\
+      if (N>0) dest[0] = V0.f;					\
+      if (N>1) dest[1] = V1.f;					\
+      if (N>2) dest[2] = V2.f;					\
+      if (N>3) dest[3] = V3.f;					\
       save->attrtype[A] = T;                                    \
    }								\
 								\
diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
index d0521d7..d923403 100644
--- a/src/mesa/vbo/vbo_save_draw.c
+++ b/src/mesa/vbo/vbo_save_draw.c
@@ -76,11 +76,11 @@ _playback_copy_to_current(struct gl_context *ctx,
    for (i = VBO_ATTRIB_POS+1 ; i < VBO_ATTRIB_MAX ; i++) {
       if (node->attrsz[i]) {
 	 GLfloat *current = (GLfloat *)vbo->currval[i].Ptr;
-         GLfloat tmp[4];
+         gl_constant_value tmp[4];
 
          COPY_CLEAN_4V_TYPE_AS_FLOAT(tmp,
                                      node->attrsz[i],
-                                     data,
+                                     (gl_constant_value*)data,
                                      node->attrtype[i]);
          
          if (node->attrtype[i] != vbo->currval[i].Type ||
-- 
1.7.9.5



More information about the mesa-stable mailing list