[PATCH]Save/restore crtl->is_thunk for emitting thunk code (PR/debug 39378)

[Daniel Berlin]

On Wed, Mar 11, 2009 at 4:09 PM, Jing Yu <jingyuuiuc@gmail.com> wrote:
>  This patch is to fix the bug PR/debug 39378 that crtl is not reset
>  after emitting thunk functions. The thunk is emitted with crtl->is_thunk=1,
> and the thunk target function is also emitted
>  with crtl->is_thunk=1. The bug exposes if compiled with -mthumb flag
>  on arm-eabi backend, where the generated binary causes segmentation
>  fault, because the thunk target function is coded in 32 bit mode, but
>  the thunk calls the target via arm-to-thunk approach.
>
> The fix is that save crtl->is_thunk before the call to lang_hooks.callgraph.emit
> _associated_thunks() in cgraph_expand_function(), and restore crtl->is_thunk
> after the call.
> I also add a test case to make sure the bug will not happen again.

Shouldn't the fix be to just move the flag to the per-function
structure and use it directly, instead of keeping it in the shared
structure?

>
> DejaGNU test:
> I built the toolchain with GNU trunk GCC (gcc version
> 4.4.0 20090305) and newlib, and tested it on the arm emulator.
> This patch does not trigger more regression errors or warnings.
>
> Target: arm-eabi
> Configured with: /home/jingyu/projects/gcc/gcc_src/configure
> --prefix=/home/jingyu/local/gcc_arm_eabi --target=arm-eabi
> --build=i686-linux --host=i686-linux --enable-multilib --with-newlib
> --with-gnu-as --with-gnu-ld --enable-languages=c,c++
> Thread model: single
> gcc version 4.4.0 20090305 (experimental) (GCC)
>
> Thanks,
> Jing
>
>
> Index: gcc/cgraphunit.c
> ===================================================================
> --- gcc/cgraphunit.c    (revision 144771)
> +++ gcc/cgraphunit.c    (working copy)
> @@ -1043,7 +1043,11 @@
>
>    /* Generate RTL for the body of DECL.  */
>    if (lang_hooks.callgraph.emit_associated_thunks)
> -    lang_hooks.callgraph.emit_associated_thunks (decl);
> +    {
> +      bool is_thunk = crtl->is_thunk;
> +      lang_hooks.callgraph.emit_associated_thunks (decl);
> +      crtl->is_thunk = is_thunk;
> +    }
>    tree_rest_of_compilation (decl);
>
>    /* Make sure that BE didn't give up on compiling.  */
>
> Index: gcc/testsuite/g++.dg/inherit/thunk10.C
> ===================================================================
> --- gcc/testsuite/g++.dg/inherit/thunk10.C      (revision 0)
> +++ gcc/testsuite/g++.dg/inherit/thunk10.C      (revision 0)
> @@ -0,0 +1,61 @@
> +/* { dg-options "-mthumb" { target arm*-*-* } } */
> +/* { dg-do run } */
> +/* { dg-timeout 10 } */
> +
> +/* PR target/39378 */
> +/* Check if the thunk target function is emitted correctly. */
> +class B1
> +{
> +public:
> +  virtual int foo1(void);
> +  int b1;
> +};
> +
> +
> +class B2
> +{
> +public:
> +  virtual int foo2 (void);
> +  int b2;
> +};
> +
> +class D : public B1, public B2
> +{
> +  int foo1(void);
> +  int foo2(void);
> +  int d;
> +};
> +
> +int B1::foo1(void)
> +{
> +  return 3;
> +}
> +
> +int B2::foo2(void)
> +{
> +  return 4;
> +}
> +
> +int D::foo1(void)
> +{
> +  return 1;
> +}
> +
> +int D::foo2(void)
> +{
> +  return 2;
> +}
> +
> +__attribute__((noinline)) int test(B2* bp)
> +{
> +  return bp->foo2();
> +}
> +
> +int main()
> +{
> +  B2 *bp = new D();
> +  if (test(bp) == 2)
> +    return 0;
> +  else
> +    return 1;
> +}
>