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

[Daniel Berlin <dberlin at dberlin dot org>]

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;
> +}
>