[Mesa-dev] [PATCH 3/4] vbo: replace assert(0) with unreachable()

[Brian Paul]

On 01/18/2018 06:11 AM, Ilia Mirkin wrote:
> unreachable is very different from assert(0). Unreachable means "go
> into an infinite loop, or other undefined behavior". I haven't checked
> the code you're changing, but if it's a complete impossibility for
> that case to be hit, then unreachable is fine. Otherwise assert should
> be used with a fallback that lets the code limp along instead of
> sending the unwitting program into a cpu hang.

I think I've said before that I'm not 100% clear on assert() vs. 
unreachable(), at least with respect to some of the discussion here in 
the past.

My understanding has been that unreachable() declares a point in the 
code which should never be reached/executed whereas assert is for 
declaring a logical condition should be true.

If I've got that right, I believe my use of unreachable() here is 
correct.  It seems to be about the same as other instances of 
unreachable() in Mesa.  But I doubt all those instances are declaring 
"complete impossibility" as you said.

The "undefined behavior" part does bother me.  I'd rather have Mesa try 
to recover and limp along than terminate or infinite loop when something 
unexpected happens in a release build.

-Brian


> On Wed, Jan 17, 2018 at 5:57 PM, Brian Paul <brianp at vmware.com> wrote:
>> ---
>>   src/mesa/vbo/vbo_context.h    | 8 ++++----
>>   src/mesa/vbo/vbo_exec_array.c | 2 +-
>>   src/mesa/vbo/vbo_exec_draw.c  | 4 ++--
>>   src/mesa/vbo/vbo_save_api.c   | 5 ++---
>>   src/mesa/vbo/vbo_save_draw.c  | 2 +-
>>   5 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/vbo/vbo_context.h b/src/mesa/vbo/vbo_context.h
>> index 04079b7..cd1cbd9 100644
>> --- a/src/mesa/vbo/vbo_context.h
>> +++ b/src/mesa/vbo/vbo_context.h
>> @@ -154,7 +154,7 @@ vbo_draw_method(struct vbo_context *vbo, gl_draw_method method)
>>            ctx->Array._DrawArrays = vbo->save.inputs;
>>            break;
>>         default:
>> -         assert(0);
>> +         unreachable("Bad VBO drawing method");
>>         }
>>
>>         ctx->NewDriverState |= ctx->DriverFlags.NewArray;
>> @@ -178,7 +178,7 @@ vbo_attrtype_to_integer_flag(GLenum format)
>>      case GL_UNSIGNED_INT64_ARB:
>>         return GL_TRUE;
>>      default:
>> -      assert(0);
>> +      unreachable("Bad vertex attribute type");
>>         return GL_FALSE;
>>      }
>>   }
>> @@ -195,7 +195,7 @@ vbo_attrtype_to_double_flag(GLenum format)
>>      case GL_DOUBLE:
>>         return GL_TRUE;
>>      default:
>> -      assert(0);
>> +      unreachable("Bad vertex attribute type");
>>         return GL_FALSE;
>>      }
>>   }
>> @@ -218,7 +218,7 @@ vbo_get_default_vals_as_union(GLenum format)
>>      case GL_UNSIGNED_INT:
>>         return (fi_type *)default_int;
>>      default:
>> -      assert(0);
>> +      unreachable("Bad vertex format");
>>         return NULL;
>>      }
>>   }
>> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
>> index 024d4d6..16521ff 100644
>> --- a/src/mesa/vbo/vbo_exec_array.c
>> +++ b/src/mesa/vbo/vbo_exec_array.c
>> @@ -150,7 +150,7 @@ check_draw_elements_data(struct gl_context *ctx, GLsizei count,
>>            j = ((const GLuint *) elements)[i];
>>            break;
>>         default:
>> -         assert(0);
>> +         unreachable("Unexpected index buffer type");
>>         }
>>
>>         /* check element j of each enabled array */
>> diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c
>> index 3aff97e..080d50c 100644
>> --- a/src/mesa/vbo/vbo_exec_draw.c
>> +++ b/src/mesa/vbo/vbo_exec_draw.c
>> @@ -159,7 +159,7 @@ vbo_copy_vertices(struct vbo_exec_context *exec)
>>      case PRIM_OUTSIDE_BEGIN_END:
>>         return 0;
>>      default:
>> -      assert(0);
>> +      unreachable("Unexpected primitive type");
>>         return 0;
>>      }
>>   }
>> @@ -220,7 +220,7 @@ vbo_exec_bind_arrays(struct gl_context *ctx)
>>         }
>>         break;
>>      default:
>> -      assert(0);
>> +      unreachable("Bad vertex program mode");
>>      }
>>
>>      for (attr = 0; attr < VERT_ATTRIB_MAX ; attr++) {
>> diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
>> index 1c57544..49939ed 100644
>> --- a/src/mesa/vbo/vbo_save_api.c
>> +++ b/src/mesa/vbo/vbo_save_api.c
>> @@ -174,7 +174,7 @@ copy_vertices(struct gl_context *ctx,
>>                   sz * sizeof(GLfloat));
>>         return i;
>>      default:
>> -      assert(0);
>> +      unreachable("Unexpected primitive type");
>>         return 0;
>>      }
>>   }
>> @@ -675,8 +675,7 @@ copy_from_current(struct gl_context *ctx)
>>            save->attrptr[i][0] = save->current[i][0];
>>            break;
>>         case 0:
>> -         assert(0);
>> -         break;
>> +         unreachable("Unexpected vertex attribute size");
>>         }
>>      }
>>   }
>> diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
>> index f5c4a90..6bccc85 100644
>> --- a/src/mesa/vbo/vbo_save_draw.c
>> +++ b/src/mesa/vbo/vbo_save_draw.c
>> @@ -194,7 +194,7 @@ bind_vertex_list(struct gl_context *ctx,
>>         }
>>         break;
>>      default:
>> -      assert(0);
>> +      unreachable("Bad vertex program mode");
>>      }
>>
>>      for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) {
>> --
>> 2.7.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwIBaQ&c=uilaK90D4TOVoH58JNXRgQ&r=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA&m=SJVaLFD8_QCAi6uc4VUr2wFcKOrJamuh3iJsMoff4Bo&s=3wBYGFWHc1R5-3vDy0mELtBG7JrGMsBHsHHci0bNerM&e=