[PATCH] D16564: Fix an issue where backend crashes after frontend emits an error message

[Justin Bogner via cfe-commits]

Manman Ren via cfe-commits <cfe-commits at lists.llvm.org> writes:
> manmanren created this revision.
> manmanren added reviewers: echristo, rafael, ahatanak.
> manmanren added a subscriber: cfe-commits.
>
> It can happen that frontend emits error message when releasing the
> builder. When that happens, we emit the error message and continue to
> invoke backend.
> Backend will then crash.
>
> The fix is quite simple, we check for errors after releasing the builder.
>
> http://reviews.llvm.org/D16564
>
> Files:
>   lib/CodeGen/ModuleBuilder.cpp
>   test/CodeGen/target-builtin-error-3.c
>
>
> Index: test/CodeGen/target-builtin-error-3.c
> ===================================================================
> --- test/CodeGen/target-builtin-error-3.c
> +++ test/CodeGen/target-builtin-error-3.c
> @@ -0,0 +1,28 @@
> +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -S -verify -o - -target-feature +avx
> +
> +// RUN: not %clang_cc1 %s -triple=x86_64-apple-darwin -emit-obj -target-feature +avx 2> %t.err
> +// RUN: FileCheck < %t.err %s
> +// CHECK: 1 error generated
> +
> +typedef unsigned short uint16_t;
> +typedef long long __m128i __attribute__((__vector_size__(16)));
> +typedef float __v8sf __attribute__ ((__vector_size__ (32)));
> +typedef float __m256 __attribute__ ((__vector_size__ (32)));
> +typedef uint16_t half;
> +typedef __attribute__ ((ext_vector_type( 8),__aligned__( 16))) half half8;
> +typedef __attribute__ ((ext_vector_type(16),__aligned__( 32))) half half16;
> +typedef __attribute__ ((ext_vector_type(16),__aligned__( 2))) half half16U;
> +typedef __attribute__ ((ext_vector_type( 8),__aligned__( 32))) float float8;
> +typedef __attribute__ ((ext_vector_type(16),__aligned__( 64))) float float16;
> +static inline half8 __attribute__((__overloadable__)) convert_half( float8 a ) {
> +  return __extension__ ({ __m256 __a = (a); (__m128i)__builtin_ia32_vcvtps2ph256((__v8sf)__a, (0x00)); }); // expected-error {{'__builtin_ia32_vcvtps2ph256' needs target feature f16c}}
> +}
> +static inline half16 __attribute__((__overloadable__)) convert_half( float16 a ) {
> +  half16 r; 
> +  r.lo = convert_half( a.lo); 
> +  return r;
> +}
> +void avx_test( uint16_t *destData, float16 argbF)
> +{
> +   ((half16U*)destData)[0] = convert_half(argbF);
> +}
> Index: lib/CodeGen/ModuleBuilder.cpp
> ===================================================================
> --- lib/CodeGen/ModuleBuilder.cpp
> +++ lib/CodeGen/ModuleBuilder.cpp
> @@ -208,6 +208,14 @@
>  
>        if (Builder)
>          Builder->Release();
> +
> +      // Builder->Release can cause diagnostics to be generated.

This comment's a bit confusing. Isn't it more like "If errors occurred
while building the module, we need to stop here before invoking the
backend", or something like that?

> +      if (Diags.hasErrorOccurred()) {
> +        if (Builder)
> +          Builder->clear();
> +        M.reset();
> +        return;
> +      }
>      }
>  
>      void CompleteTentativeDefinition(VarDecl *D) override {
>