Bug 66896 - ipa-prop.c:2479 runtime error: member call on null pointer of type 'struct ipa_polymorphic_call_context'
Summary: ipa-prop.c:2479 runtime error: member call on null pointer of type 'struct ip...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ipa (show other bugs)
Version: 5.1.0
: P3 normal
Target Milestone: ---
Assignee: Martin Liška
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-16 08:11 UTC by Vittorio Zecca
Modified: 2015-12-07 15:38 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Suggested patch (865 bytes, patch)
2015-07-16 09:36 UTC, Martin Liška
Details | Diff
To be compiled with -O2 (502 bytes, text/plain)
2015-07-24 14:01 UTC, Vittorio Zecca
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vittorio Zecca 2015-07-16 08:11:32 UTC
//must be compiled with -O2 , or "-O[1] -fdevirtualize"
//ipa-prop.c:2479:30: runtime error: member call on null pointer of type 'struct ipa_polymorphic_call_context'
//gcc ipa-prop.c source line "dst_ctx->combine_with (ctx);"
//because dst_ctx is NULL
//double checked with "gcc_assert(dst_ctx);"
void f2 (void *);
void f3 ();

struct A
{
  int *a;
  A ();
  ~A () { a3 (); }
  int a1 (int * p) { if (!p) f3 (); f2 (p); }
  void a3 () { if (*a) a1 (a); }
};

struct B : A {~B () { a3 ();}};

struct F {};

struct G : F {B g;};

void foo () {G g;}
Comment 1 Martin Liška 2015-07-16 09:36:59 UTC
Created attachment 35994 [details]
Suggested patch
Comment 2 Martin Liška 2015-07-16 14:48:50 UTC
Author: marxin
Date: Thu Jul 16 14:48:18 2015
New Revision: 225887

URL: https://gcc.gnu.org/viewcvs?rev=225887&root=gcc&view=rev
Log:
Fix PR ipa/66896.

	* g++.dg/ipa/pr66896.c: New test.
	PR ipa/66896.
	* ipa-prop.c (update_jump_functions_after_inlining): Create properly
	dst_ctx if it does not exist.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-prop.c
    trunk/gcc/testsuite/ChangeLog
Comment 3 Martin Liška 2015-07-16 14:53:40 UTC
Author: marxin
Date: Thu Jul 16 14:53:08 2015
New Revision: 225888

URL: https://gcc.gnu.org/viewcvs?rev=225888&root=gcc&view=rev
Log:
Fix PR ipa/66896.

	* g++.dg/ipa/pr66896.c: New test.
	PR ipa/66896.
	* ipa-prop.c (update_jump_functions_after_inlining): Create properly
	dst_ctx if it does not exist.

Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/ipa-prop.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
Comment 4 Martin Liška 2015-07-16 14:55:06 UTC
Fixed in both trunk and gcc-5 branch.
Comment 5 Vittorio Zecca 2015-07-17 06:36:16 UTC
Yes I applied the fix and it now works on all the gcc testsuite.
Comment 6 Vittorio Zecca 2015-07-23 08:28:29 UTC
I just found the same issue at line 2479:
dst_ctx->combine_with (ctx);
dst_ctx is again NULL
Maybe the same patch should be applied here? Namely:
if (!dst_ctx)
                        {
                          vec_safe_grow_cleared
(args->polymorphic_call_contexts,
                                                 count);
                          dst_ctx =
ipa_get_ith_polymorhic_call_context (args, i);
                        }
immediately before?
Do you need a reproducer?
This is compiling pythia software.
Comment 7 Martin Jambor 2015-07-24 11:34:18 UTC
(In reply to Vittorio Zecca from comment #6)
> I just found the same issue at line 2479:

As even the the gcc-5 branch is changing slightly, line numbers on
their own are not are not the best indicator of the position of the
code you refer to.

> dst_ctx->combine_with (ctx);
> dst_ctx is again NULL
> Maybe the same patch should be applied here? Namely:
> if (!dst_ctx)
>                         {
>                           vec_safe_grow_cleared
> (args->polymorphic_call_contexts,
>                                                  count);
>                           dst_ctx =
> ipa_get_ith_polymorhic_call_context (args, i);
>                         }
> immediately before?

I have looked at both occurrences of dst_ctx->combine_with (ctx) in
function update_jump_functions_after_inlining and they are all
guarded with the code just as you describe.

> Do you need a reproducer?

Yes please, I cannot see the error you describe.  Thanks.
Comment 8 Vittorio Zecca 2015-07-24 14:01:57 UTC
Created attachment 36052 [details]
To be compiled with -O2
Comment 9 Vittorio Zecca 2015-07-24 14:03:24 UTC
At line 2473 of ipa-prop.c I have

if (!ctx.useless_p ())
I changed it into 

if (!ctx.useless_p () || !dst_ctx)

Now the sanitizer runtime error message disappears.
I am attaching another source, gccerr20-bis.C, that used to force update_jump_functions_after_inlining to reference a NULL pointer dst_ctx
Comment 10 Martin Jambor 2015-07-24 16:08:32 UTC
(In reply to Vittorio Zecca from comment #8)
> Created attachment 36052 [details]
> To be compiled with -O2

This compiles fine for me (with -O2) both with the current trunk and
the current gcc 5 branch.

(In reply to Vittorio Zecca from comment #9)
> At line 2473 of ipa-prop.c I have
> 
> if (!ctx.useless_p ())
> I changed it into 
> 
> if (!ctx.useless_p () || !dst_ctx)
> 
> Now the sanitizer runtime error message disappears.

What sanitizer error?
Comment 11 Vittorio Zecca 2015-07-24 17:24:26 UTC
I have a version of gcc 5.2.0 compiled with the -fsanitize=undefined option.
This sanitized version gave me a runtime error due to dereferencing
the pointer dst_ctx
which was NULL. After the change I suggested the message disappeared.

You may double check my finding, as I did myself, by putting

assert(dst_ctx)

before its dereferencing in ipa-prop.c as in

assert(dst_ctx),dst_ctx->combine_with (ctx);

It happens twice in isa-prop.c

Then try it with the two C codes I sent, with option -O2
Comment 12 Martin Liška 2015-08-05 15:56:40 UTC
(In reply to Vittorio Zecca from comment #11)
> I have a version of gcc 5.2.0 compiled with the -fsanitize=undefined option.
> This sanitized version gave me a runtime error due to dereferencing
> the pointer dst_ctx
> which was NULL. After the change I suggested the message disappeared.
> 
> You may double check my finding, as I did myself, by putting
> 
> assert(dst_ctx)
> 
> before its dereferencing in ipa-prop.c as in
> 
> assert(dst_ctx),dst_ctx->combine_with (ctx);
> 
> It happens twice in isa-prop.c
> 
> Then try it with the two C codes I sent, with option -O2

Hello.

I double-checked that there's no other NULL dereferencing in ipa-prop.c.

Can you please confirm that gcc-5-branch works correctly?

Thanks,
Martin
Comment 13 Vittorio Zecca 2015-08-06 06:24:13 UTC
I see only two NULL dereferencing in ipa-prop.c my lines 2479 and 2545
same statement

dst_ctx->combine_with (ctx);

Did you take care of both of them?
Comment 14 Martin Liška 2015-08-06 08:39:39 UTC
(In reply to Vittorio Zecca from comment #13)
> I see only two NULL dereferencing in ipa-prop.c my lines 2479 and 2545
> same statement
> 
> dst_ctx->combine_with (ctx);
> 
> Did you take care of both of them?

Hi.

Both cases are fixed on both trunk and gcc-5 branch in function
update_jump_functions_after_inlining.

Thanks,
Martin
Comment 15 Jakub Jelinek 2015-12-07 14:49:51 UTC
The commits mentioned a test being added to the testsuite, but it doesn't seem to have been added.  Also, the ChangeLog entry mentioned wrong extension (in g++.dg/ipa/ it should be *.C instead of *.c).
Comment 16 Martin Liška 2015-12-07 15:38:09 UTC
(In reply to Jakub Jelinek from comment #15)
> The commits mentioned a test being added to the testsuite, but it doesn't
> seem to have been added.  Also, the ChangeLog entry mentioned wrong
> extension (in g++.dg/ipa/ it should be *.C instead of *.c).

Thanks for pointing out, fixed both for gcc-5-branch and trunk.

Martin