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

[Eric Christopher via cfe-commits]

I'm also just curious how we got all the way to here without having the
error emitted and compilation stopped?

-eric

On Thu, Jan 28, 2016 at 9:53 AM Justin Bogner <mail at justinbogner.com> wrote:

> 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 {
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160128/d4508cac/attachment.html>