[PATCH] Fix an ICE when compiling Go with LTO (PR go/89019)

[Richard Biener]

On Thu, Jan 24, 2019 at 2:18 AM Nikhil Benesch <nikhil.benesch@gmail.com> wrote:
>
> This patch fixes an ICE I reported earlier today as PR go/89019, which
> occurs when compiling sufficiently complicated Go code with link-time
> optimization (i.e., -flto) enabled.
>
> Both of these simple test programs are sufficient to trigger the ICE:
>
>     $ cat crash1.go
>     package main
>
>     type fcanary func()
>
>     $ cat crash2.go
>     package main
>
>     func f() {
>         var canary func()
>         func() {
>             canary()
>         }()
>     }
>
> The cause appears to be that the propagation of structural equality
> requirements is mishandled during the handoff from the Go frontend to
> the middle-end. Pointers to types that required structural equality
> were not always marked as requiring structural equality themselves.
> Remarkably, the only code that notices the mistake is the free_lang_data
> IPA pass, which is only active when LTO is enabled.
>
> The enclosed patch fixes the issue and provides test coverage by adding
> a subtest to the Go torture test suite that compiles with -flto.
> Bootstrapped and ran Go test suite on x86_64-pc-linux-gnu.
>
> 2018-01-23  Nikhil Benesch  <nikhil.benesch@gmail.com>
>
>         PR go/89019
>         * go-gcc.cc (Gcc_backend::placeholder_struct_type): Mark
>         placeholder structs as requiring structural equality.
>         (Gcc_backend::set_placeholder_pointer_type): Propagate
>         the canonical type from the referenced type to the pointer type.
>         * lib/go-torture.exp: Test compiling with -flto.
>
> diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc
> index 7fbdd074119..4e9e0e3026a 100644
> --- a/gcc/go/go-gcc.cc
> +++ b/gcc/go/go-gcc.cc
> @@ -1049,6 +1049,7 @@ Gcc_backend::set_placeholder_pointer_type(Btype* placeholder,
>      }
>    gcc_assert(TREE_CODE(tt) == POINTER_TYPE);
>    TREE_TYPE(pt) = TREE_TYPE(tt);
> +  TYPE_CANONICAL(pt) = TYPE_CANONICAL(tt);

This doesn't make sense - it says to the middle-end that
T* is equal to T?

>    if (TYPE_NAME(pt) != NULL_TREE)
>      {
>        // Build the data structure gcc wants to see for a typedef.
> @@ -1080,6 +1081,12 @@ Gcc_backend::placeholder_struct_type(const std::string& name,
>                              get_identifier_from_string(name),
>                              ret);
>        TYPE_NAME(ret) = decl;
> +
> +      // The struct type that eventually replaces this placeholder will require
> +      // structural equality. The placeholder must too, so that the requirement
> +      // for structural equality propagates to references that are constructed
> +      // before the replacement occurs.
> +      SET_TYPE_STRUCTURAL_EQUALITY(ret);
>      }
>    return this->make_type(ret);
>  }
> diff --git a/gcc/testsuite/lib/go-torture.exp b/gcc/testsuite/lib/go-torture.exp
> index 213711e41df..a7eca184416 100644
> --- a/gcc/testsuite/lib/go-torture.exp
> +++ b/gcc/testsuite/lib/go-torture.exp
> @@ -34,7 +34,8 @@ if ![info exists TORTURE_OPTIONS] {
>         { -O2 -fomit-frame-pointer -finline-functions -funroll-loops } \
>         { -O2 -fbounds-check } \
>         { -O3 -g } \
> -       { -Os }]
> +       { -Os } \
> +       { -flto }]
>  }
>
>
>