> -----Original Message-----
> From: Jakub Jelinek [mailto:ja...@redhat.com]
> Sent: Wednesday, January 31, 2018 9:57 PM
> To: Uros Bizjak <ubiz...@gmail.com>; Kirill Yukhin
> <kirill.yuk...@gmail.com>
> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
> <igor.v.tsimbal...@intel.com>
> Subject: [PATCH] Fix ICE with CET and -g (PR target/84146)
>
> Hi!
>
> We ICE on the following test because rest_of_insert_endbranch
> separates a setjmp call from the following
> NOTE_INSN_CALL_ARG_LOCATION
> that must always immediately follow the call.
> No other note or debug insn (which aren't around after var-tracking anyway)
> needs to follow the call, so the loop it was doing is unnecessary, on the
> other side, we are already at a late state where bb boundaries are fuzzy and
> we are going to throw away the cfg before doing final.
>
> So, all we need is just check if the call is followed by this note and
> if yes, emit the endbr after it.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Just in case, ok from CET viewpoint.
Thanks,
Igor
> 2018-01-31 Jakub Jelinek <ja...@redhat.com>
>
> PR target/84146
> * config/i386/i386.c (rest_of_insert_endbranch): Only skip
> NOTE_INSN_CALL_ARG_LOCATION after a call, not anything else,
> and skip it regardless of bb boundaries. Use CALL_P macro,
> don't test INSN_P (insn) together with CALL_P or JUMP_P check
> unnecessarily, formatting fix.
>
> * gcc.target/i386/pr84146.c: New test.
>
> --- gcc/config/i386/i386.c.jj 2018-01-31 09:26:18.341505667 +0100
> +++ gcc/config/i386/i386.c 2018-01-31 14:13:33.815243832 +0100
> @@ -2609,31 +2609,27 @@ rest_of_insert_endbranch (void)
> for (insn = BB_HEAD (bb); insn != NEXT_INSN (BB_END (bb));
> insn = NEXT_INSN (insn))
> {
> - if (INSN_P (insn) && GET_CODE (insn) == CALL_INSN)
> + if (CALL_P (insn))
> {
> if (find_reg_note (insn, REG_SETJMP, NULL) == NULL)
> continue;
> /* Generate ENDBRANCH after CALL, which can return more
> than
> twice, setjmp-like functions. */
>
> - /* Skip notes and debug insns that must be next to the
> - call insn. ??? This might skip a lot more than
> - that... ??? Skipping barriers and emitting code
> - after them surely looks like a mistake; we probably
> - won't ever hit it, for we'll hit BB_END first. */
> + /* Skip notes that must immediately follow the call insn. */
> rtx_insn *next_insn = insn;
> - while ((next_insn != BB_END (bb))
> - && (DEBUG_INSN_P (NEXT_INSN (next_insn))
> - || NOTE_P (NEXT_INSN (next_insn))
> - || BARRIER_P (NEXT_INSN (next_insn))))
> - next_insn = NEXT_INSN (next_insn);
> + if (NEXT_INSN (insn)
> + && NOTE_P (NEXT_INSN (insn))
> + && (NOTE_KIND (NEXT_INSN (insn))
> + == NOTE_INSN_CALL_ARG_LOCATION))
> + next_insn = NEXT_INSN (insn);
>
> cet_eb = gen_nop_endbr ();
> emit_insn_after_setloc (cet_eb, next_insn, INSN_LOCATION
> (insn));
> continue;
> }
>
> - if (INSN_P (insn) && JUMP_P (insn) && flag_cet_switch)
> + if (JUMP_P (insn) && flag_cet_switch)
> {
> rtx target = JUMP_LABEL (insn);
> if (target == NULL_RTX || ANY_RETURN_P (target))
> @@ -2668,7 +2664,7 @@ rest_of_insert_endbranch (void)
> if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
> || (NOTE_P (insn)
> && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
> -/* TODO. Check /s bit also. */
> + /* TODO. Check /s bit also. */
> {
> cet_eb = gen_nop_endbr ();
> emit_insn_after (cet_eb, insn);
> --- gcc/testsuite/gcc.target/i386/pr84146.c.jj 2018-01-31
> 16:32:28.099929916 +0100
> +++ gcc/testsuite/gcc.target/i386/pr84146.c 2018-01-31
> 14:04:17.796122397 +0100
> @@ -0,0 +1,14 @@
> +/* PR target/84146 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g -mcet -fcf-protection=full" } */
> +
> +int __setjmp (void **);
> +void *buf[64];
> +
> +void
> +foo (void)
> +{
> + __setjmp (buf);
> + for (;;)
> + ;
> +}
>
> Jakub
