[PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Marek Polacek]

In r270550, Jakub fixed classify_insn to handle asm goto: if the asm can
jump to a label, the insn should be a JUMP_INSN.

However, as the following testcase shows, non-null ASM_OPERANDS_LABEL_VEC
doesn't guarantee that the rtx has any actual labels it can branch to.
Here, the rtvec has 0 elements because of the __thread variable: we perform
ix86_rewrite_tls_address which calls copy_isns and that allocates the rtvec:

    XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));

This causes an ICE in update_br_prob_note: BRANCH_EDGE (bb) crashes
because there's no branch edge.  I think we can fix this by checking
that there is at least one label the asm can jump to before wrapping
the ASM_OPERANDS in a JUMP_INSN.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/11?

	PR rtl-optimization/104777

gcc/ChangeLog:

	* rtl.cc (classify_insn): For ASM_OPERANDS, return JUMP_INSN only if
	ASM_OPERANDS_LABEL_VEC has at least one element.

gcc/testsuite/ChangeLog:

	* gcc.dg/torture/tls/pr104777.c: New test.
---
 gcc/rtl.cc                                  |  7 +++--
 gcc/testsuite/gcc.dg/torture/tls/pr104777.c | 30 +++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/tls/pr104777.c

diff --git a/gcc/rtl.cc b/gcc/rtl.cc
index f17474bfee1..a29d5e1371e 100644
--- a/gcc/rtl.cc
+++ b/gcc/rtl.cc
@@ -765,7 +765,9 @@ classify_insn (rtx x)
     return CALL_INSN;
   if (ANY_RETURN_P (x))
     return JUMP_INSN;
-  if (GET_CODE (x) == ASM_OPERANDS && ASM_OPERANDS_LABEL_VEC (x))
+  if (GET_CODE (x) == ASM_OPERANDS
+      && ASM_OPERANDS_LABEL_VEC (x)
+      && GET_NUM_ELEM (ASM_OPERANDS_LABEL_VEC (x)) > 0)
     return JUMP_INSN;
   if (GET_CODE (x) == SET)
     {
@@ -794,7 +796,8 @@ classify_insn (rtx x)
       if (has_return_p)
 	return JUMP_INSN;
       if (GET_CODE (XVECEXP (x, 0, 0)) == ASM_OPERANDS
-	  && ASM_OPERANDS_LABEL_VEC (XVECEXP (x, 0, 0)))
+	  && ASM_OPERANDS_LABEL_VEC (XVECEXP (x, 0, 0))
+	  && GET_NUM_ELEM (ASM_OPERANDS_LABEL_VEC (XVECEXP (x, 0, 0))) > 0)
 	return JUMP_INSN;
     }
 #ifdef GENERATOR_FILE
diff --git a/gcc/testsuite/gcc.dg/torture/tls/pr104777.c b/gcc/testsuite/gcc.dg/torture/tls/pr104777.c
new file mode 100644
index 00000000000..abaf59731fc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/tls/pr104777.c
@@ -0,0 +1,30 @@
+/* PR rtl-optimization/104777 */
+/* { dg-do compile } */
+/* { dg-require-effective-target tls } */
+ 
+int savestate_r;
+int savestate_ssb;
+extern void abort();
+__thread int  loop;
+void f (void)
+{
+  int savestate_r0_5;
+  int savestate_r1_6;
+
+  __asm__("" : "=m" (savestate_ssb), "=r" (savestate_r));
+  savestate_r0_5 = savestate_r;
+  if (savestate_r0_5 == 0)
+  {
+    __asm__ __volatile__("" :  : "m" (loop));
+    abort ();
+  }
+
+  __asm__("" : "=m" (savestate_ssb), "=r" (savestate_r));
+  savestate_r1_6 = savestate_r;
+  if (savestate_r1_6 != 0)
+    return;
+
+  __asm__ __volatile__("" :  : "m" (loop));
+  abort ();
+
+}

base-commit: b7dbe870bc0193c76345f893d94c5bf6c99a6afe
-- 
2.35.1

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Segher Boessenkool]

On Mon, Mar 07, 2022 at 07:03:17PM -0500, Marek Polacek via Gcc-patches wrote:
> In r270550, Jakub fixed classify_insn to handle asm goto: if the asm can
> jump to a label, the insn should be a JUMP_INSN.
> 
> However, as the following testcase shows, non-null ASM_OPERANDS_LABEL_VEC
> doesn't guarantee that the rtx has any actual labels it can branch to.

But it should.

> Here, the rtvec has 0 elements because of the __thread variable: we perform
> ix86_rewrite_tls_address which calls copy_isns and that allocates the rtvec:
> 
>     XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));

So fix *that* instead?  Everywhere else does not use length zero RTL
vectors.  copy_rtx makes sure to do the right thing here, for example.

We do not have notation to create zero-length vectors in RTL source
code either, btw.:
    case 'V':
      /* 'V' is an optional vector: if a closeparen follows,
         just store NULL for this element.  */
(optional vectors are at the end of an RTX), and if you write [] you
will hit
          fatal_with_file_and_line ("vector must have at least one element");


Segher

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Marek Polacek]

On Mon, Mar 07, 2022 at 07:19:09PM -0600, Segher Boessenkool wrote:
> On Mon, Mar 07, 2022 at 07:03:17PM -0500, Marek Polacek via Gcc-patches wrote:
> > In r270550, Jakub fixed classify_insn to handle asm goto: if the asm can
> > jump to a label, the insn should be a JUMP_INSN.
> > 
> > However, as the following testcase shows, non-null ASM_OPERANDS_LABEL_VEC
> > doesn't guarantee that the rtx has any actual labels it can branch to.
> 
> But it should.

Ok, that would make sense.  However...
 
> > Here, the rtvec has 0 elements because of the __thread variable: we perform
> > ix86_rewrite_tls_address which calls copy_isns and that allocates the rtvec:
> > 
> >     XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));
> 
> So fix *that* instead?  Everywhere else does not use length zero RTL
> vectors.  copy_rtx makes sure to do the right thing here, for example.

...I don't see that.  In fact copy_rtx does the same thing as
copy_insn:

       case 'V':
         if (XVEC (orig, i) != NULL)
           {
             XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));

which will copy a zero-length vector too, right?  But even if it didn't
we'd still ICE on the original rtx as I found out.  The zero-length
rtvec is originally created in expand_asm_stmt:

  rtvec labelvec = rtvec_alloc (nlabels);

where nlabels is 0 but using NULL_RTVEC instead just means crashes everywhere.
  
So I'm afraid I don't have a better fix (except that I should have used
ASM_OPERANDS_LABEL_LENGTH).

> We do not have notation to create zero-length vectors in RTL source
> code either, btw.:
>     case 'V':
>       /* 'V' is an optional vector: if a closeparen follows,
>          just store NULL for this element.  */
> (optional vectors are at the end of an RTX), and if you write [] you
> will hit
>           fatal_with_file_and_line ("vector must have at least one element");

Marek

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Segher Boessenkool]

Hi!

On Tue, Mar 08, 2022 at 10:08:25AM -0500, Marek Polacek wrote:
> On Mon, Mar 07, 2022 at 07:19:09PM -0600, Segher Boessenkool wrote:
> > On Mon, Mar 07, 2022 at 07:03:17PM -0500, Marek Polacek via Gcc-patches wrote:
> > > In r270550, Jakub fixed classify_insn to handle asm goto: if the asm can
> > > jump to a label, the insn should be a JUMP_INSN.
> > > 
> > > However, as the following testcase shows, non-null ASM_OPERANDS_LABEL_VEC
> > > doesn't guarantee that the rtx has any actual labels it can branch to.
> > 
> > But it should.
> 
> Ok, that would make sense.  However...
>  
> > > Here, the rtvec has 0 elements because of the __thread variable: we perform
> > > ix86_rewrite_tls_address which calls copy_isns and that allocates the rtvec:
> > > 
> > >     XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));
> > 
> > So fix *that* instead?  Everywhere else does not use length zero RTL
> > vectors.  copy_rtx makes sure to do the right thing here, for example.
> 
> ...I don't see that.  In fact copy_rtx does the same thing as
> copy_insn:
> 
>        case 'V':
>          if (XVEC (orig, i) != NULL)
>            {
>              XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));
> 
> which will copy a zero-length vector too, right?

It doesn't.  It copies NULL as NULL.  That is what that "if" is for.
You can do similar in copy_insn_1?


Segher

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Marek Polacek]

On Tue, Mar 08, 2022 at 09:14:56AM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Mar 08, 2022 at 10:08:25AM -0500, Marek Polacek wrote:
> > On Mon, Mar 07, 2022 at 07:19:09PM -0600, Segher Boessenkool wrote:
> > > On Mon, Mar 07, 2022 at 07:03:17PM -0500, Marek Polacek via Gcc-patches wrote:
> > > > In r270550, Jakub fixed classify_insn to handle asm goto: if the asm can
> > > > jump to a label, the insn should be a JUMP_INSN.
> > > > 
> > > > However, as the following testcase shows, non-null ASM_OPERANDS_LABEL_VEC
> > > > doesn't guarantee that the rtx has any actual labels it can branch to.
> > > 
> > > But it should.
> > 
> > Ok, that would make sense.  However...
> >  
> > > > Here, the rtvec has 0 elements because of the __thread variable: we perform
> > > > ix86_rewrite_tls_address which calls copy_isns and that allocates the rtvec:
> > > > 
> > > >     XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));
> > > 
> > > So fix *that* instead?  Everywhere else does not use length zero RTL
> > > vectors.  copy_rtx makes sure to do the right thing here, for example.
> > 
> > ...I don't see that.  In fact copy_rtx does the same thing as
> > copy_insn:
> > 
> >        case 'V':
> >          if (XVEC (orig, i) != NULL)
> >            {
> >              XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));
> > 
> > which will copy a zero-length vector too, right?
> 
> It doesn't.  It copies NULL as NULL.  That is what that "if" is for.

But XVEC (orig, i) is not null, it just has XVECLEN 0.

> You can do similar in copy_insn_1?

You mean copy_rtx?  It already has the same XVEC (orig, i) != NULL check.

But like I said above, even if we didn't copy these XVECLEN 0 rtvecs,
the crash would not go away.

Marek

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Segher Boessenkool]

On Tue, Mar 08, 2022 at 10:25:45AM -0500, Marek Polacek wrote:
> On Tue, Mar 08, 2022 at 09:14:56AM -0600, Segher Boessenkool wrote:
> > On Tue, Mar 08, 2022 at 10:08:25AM -0500, Marek Polacek wrote:
> > > ...I don't see that.  In fact copy_rtx does the same thing as
> > > copy_insn:
> > > 
> > >        case 'V':
> > >          if (XVEC (orig, i) != NULL)
> > >            {
> > >              XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));
> > > 
> > > which will copy a zero-length vector too, right?
> > 
> > It doesn't.  It copies NULL as NULL.  That is what that "if" is for.
> 
> But XVEC (orig, i) is not null, it just has XVECLEN 0.

So where did *that* come from?  This isn't correct RTL.

> > You can do similar in copy_insn_1?
> 
> You mean copy_rtx?  It already has the same XVEC (orig, i) != NULL check.

No, I mean do similar in copy_insn_1 as what copy_rtx already
(correctly) does.

> But like I said above, even if we didn't copy these XVECLEN 0 rtvecs,
> the crash would not go away.

An rtvec should never have length 0.  Look at gen_rtvec for another
example.

You can get rid of the crash, sure.  But it is a much better plan to try
and get rid of the actual problem!  (And then add some more checking to
make sure this doesn't happen in the future.)


Segher

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Marek Polacek]

On Tue, Mar 08, 2022 at 09:49:15AM -0600, Segher Boessenkool wrote:
> On Tue, Mar 08, 2022 at 10:25:45AM -0500, Marek Polacek wrote:
> > On Tue, Mar 08, 2022 at 09:14:56AM -0600, Segher Boessenkool wrote:
> > > On Tue, Mar 08, 2022 at 10:08:25AM -0500, Marek Polacek wrote:
> > > > ...I don't see that.  In fact copy_rtx does the same thing as
> > > > copy_insn:
> > > > 
> > > >        case 'V':
> > > >          if (XVEC (orig, i) != NULL)
> > > >            {
> > > >              XVEC (copy, i) = rtvec_alloc (XVECLEN (orig, i));
> > > > 
> > > > which will copy a zero-length vector too, right?
> > > 
> > > It doesn't.  It copies NULL as NULL.  That is what that "if" is for.
> > 
> > But XVEC (orig, i) is not null, it just has XVECLEN 0.
> 
> So where did *that* come from?  This isn't correct RTL.

I already said it before:

The zero-length rtvec is originally created in expand_asm_stmt:

  rtvec labelvec = rtvec_alloc (nlabels);

where nlabels is 0 but using NULL_RTVEC instead just means crashes everywhere.

> > > You can do similar in copy_insn_1?
> > 
> > You mean copy_rtx?  It already has the same XVEC (orig, i) != NULL check.
> 
> No, I mean do similar in copy_insn_1 as what copy_rtx already
> (correctly) does.

Do similar what?  They already do the same thing with XVECs as I've said
twice.  If you mean something other than the 'V' case, please be explicit.
 
> > But like I said above, even if we didn't copy these XVECLEN 0 rtvecs,
> > the crash would not go away.
> 
> An rtvec should never have length 0.  Look at gen_rtvec for another
> example.
> 
> You can get rid of the crash, sure.  But it is a much better plan to try
> and get rid of the actual problem!  (And then add some more checking to
> make sure this doesn't happen in the future.)

Yes, I realize that.  That's why I've tried using NULL_RTVEC in expand_asm_stmt
rather than using a zero-length rtvec.  It resulted in crashes in, for instance,
jump.cc.  So I'm not sure if such a fix is suitable for stage4.  I may try again.

Marek

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Jakub Jelinek]

On Tue, Mar 08, 2022 at 09:49:15AM -0600, Segher Boessenkool wrote:
> > But like I said above, even if we didn't copy these XVECLEN 0 rtvecs,
> > the crash would not go away.
> 
> An rtvec should never have length 0.  Look at gen_rtvec for another
> example.

That is not true.  In case of ASM_OPERANDS, lots of code relies that it
can use ASM_OPERANDS_{INPUT,LABEL}_LENGTH without checking if
ASM_OPERANDS_{INPUT,LABEL}_VEC is non-NULL.  Those ASM*LENGTH macros are
defined as XVECLEN which I believe will just segfault if the vec is NULL:
#define XVECLEN(RTX, N)         GET_NUM_ELEM (XVEC (RTX, N))
#define GET_NUM_ELEM(RTVEC)             ((RTVEC)->num_elem)
#define XVEC(RTX, N)    (RTL_CHECK2 (RTX, N, 'E', 'V').rt_rtvec)
cfgexpand.cc as Marek said will allocate even zero length vectors using
rtvec_alloc (0):
  rtvec argvec = rtvec_alloc (ninputs);
  rtvec constraintvec = rtvec_alloc (ninputs);
  rtvec labelvec = rtvec_alloc (nlabels);
or e.g. in
  PATTERN (insn) = gen_rtx_ASM_OPERANDS (VOIDmode, ggc_strdup (""), "", 0,
                                         rtvec_alloc (0),
                                         rtvec_alloc (0),
                                         ASM_OPERANDS_LABEL_VEC (tmp),
                                         ASM_OPERANDS_SOURCE_LOCATION(tmp));

	Jakub

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Marek Polacek]

On Tue, Mar 08, 2022 at 05:12:43PM +0100, Jakub Jelinek wrote:
> On Tue, Mar 08, 2022 at 09:49:15AM -0600, Segher Boessenkool wrote:
> > > But like I said above, even if we didn't copy these XVECLEN 0 rtvecs,
> > > the crash would not go away.
> > 
> > An rtvec should never have length 0.  Look at gen_rtvec for another
> > example.
> 
> That is not true.  In case of ASM_OPERANDS, lots of code relies that it
> can use ASM_OPERANDS_{INPUT,LABEL}_LENGTH without checking if
> ASM_OPERANDS_{INPUT,LABEL}_VEC is non-NULL.  Those ASM*LENGTH macros are
> defined as XVECLEN which I believe will just segfault if the vec is NULL:

Yup, they will segv.  I've guarded a few spots with ASM_OPERANDS_LABEL_VEC
before using _LENGTH but there were just more and more crashes so I gave up.

> #define XVECLEN(RTX, N)         GET_NUM_ELEM (XVEC (RTX, N))
> #define GET_NUM_ELEM(RTVEC)             ((RTVEC)->num_elem)
> #define XVEC(RTX, N)    (RTL_CHECK2 (RTX, N, 'E', 'V').rt_rtvec)
> cfgexpand.cc as Marek said will allocate even zero length vectors using
> rtvec_alloc (0):
>   rtvec argvec = rtvec_alloc (ninputs);
>   rtvec constraintvec = rtvec_alloc (ninputs);
>   rtvec labelvec = rtvec_alloc (nlabels);
> or e.g. in
>   PATTERN (insn) = gen_rtx_ASM_OPERANDS (VOIDmode, ggc_strdup (""), "", 0,
>                                          rtvec_alloc (0),
>                                          rtvec_alloc (0),
>                                          ASM_OPERANDS_LABEL_VEC (tmp),
>                                          ASM_OPERANDS_SOURCE_LOCATION(tmp));

I didn't see the latter, but I wouldn't be surprised if there were more.

Marek

Re: [PATCH] rtl: ICE with thread_local and inline asm [PR104777]

[Segher Boessenkool]

On Tue, Mar 08, 2022 at 05:12:43PM +0100, Jakub Jelinek wrote:
> On Tue, Mar 08, 2022 at 09:49:15AM -0600, Segher Boessenkool wrote:
> > > But like I said above, even if we didn't copy these XVECLEN 0 rtvecs,
> > > the crash would not go away.
> > 
> > An rtvec should never have length 0.  Look at gen_rtvec for another
> > example.
> 
> That is not true.  In case of ASM_OPERANDS, lots of code relies that it
> can use ASM_OPERANDS_{INPUT,LABEL}_LENGTH without checking if
> ASM_OPERANDS_{INPUT,LABEL}_VEC is non-NULL.  Those ASM*LENGTH macros are
> defined as XVECLEN which I believe will just segfault if the vec is NULL:
> #define XVECLEN(RTX, N)         GET_NUM_ELEM (XVEC (RTX, N))
> #define GET_NUM_ELEM(RTVEC)             ((RTVEC)->num_elem)
> #define XVEC(RTX, N)    (RTL_CHECK2 (RTX, N, 'E', 'V').rt_rtvec)
> cfgexpand.cc as Marek said will allocate even zero length vectors using
> rtvec_alloc (0):
>   rtvec argvec = rtvec_alloc (ninputs);
>   rtvec constraintvec = rtvec_alloc (ninputs);
>   rtvec labelvec = rtvec_alloc (nlabels);
> or e.g. in
>   PATTERN (insn) = gen_rtx_ASM_OPERANDS (VOIDmode, ggc_strdup (""), "", 0,
>                                          rtvec_alloc (0),
>                                          rtvec_alloc (0),
>                                          ASM_OPERANDS_LABEL_VEC (tmp),
>                                          ASM_OPERANDS_SOURCE_LOCATION(tmp));

Wow, what a mess.  And this part is completely undocumented even :-(
It seems unintentional (and wrong) to me, but yes we are in stage 4, if
we want to clean this up one way or the other, now is not the time.

In that case: your patch looks good to me Marek.


Segher

[PATCH v2] rtl: ICE with thread_local and inline asm [PR104777]

[Marek Polacek]

On Tue, Mar 08, 2022 at 10:24:50AM -0600, Segher Boessenkool wrote:
> On Tue, Mar 08, 2022 at 05:12:43PM +0100, Jakub Jelinek wrote:
> > On Tue, Mar 08, 2022 at 09:49:15AM -0600, Segher Boessenkool wrote:
> > > > But like I said above, even if we didn't copy these XVECLEN 0 rtvecs,
> > > > the crash would not go away.
> > > 
> > > An rtvec should never have length 0.  Look at gen_rtvec for another
> > > example.
> > 
> > That is not true.  In case of ASM_OPERANDS, lots of code relies that it
> > can use ASM_OPERANDS_{INPUT,LABEL}_LENGTH without checking if
> > ASM_OPERANDS_{INPUT,LABEL}_VEC is non-NULL.  Those ASM*LENGTH macros are
> > defined as XVECLEN which I believe will just segfault if the vec is NULL:
> > #define XVECLEN(RTX, N)         GET_NUM_ELEM (XVEC (RTX, N))
> > #define GET_NUM_ELEM(RTVEC)             ((RTVEC)->num_elem)
> > #define XVEC(RTX, N)    (RTL_CHECK2 (RTX, N, 'E', 'V').rt_rtvec)
> > cfgexpand.cc as Marek said will allocate even zero length vectors using
> > rtvec_alloc (0):
> >   rtvec argvec = rtvec_alloc (ninputs);
> >   rtvec constraintvec = rtvec_alloc (ninputs);
> >   rtvec labelvec = rtvec_alloc (nlabels);
> > or e.g. in
> >   PATTERN (insn) = gen_rtx_ASM_OPERANDS (VOIDmode, ggc_strdup (""), "", 0,
> >                                          rtvec_alloc (0),
> >                                          rtvec_alloc (0),
> >                                          ASM_OPERANDS_LABEL_VEC (tmp),
> >                                          ASM_OPERANDS_SOURCE_LOCATION(tmp));
> 
> Wow, what a mess.  And this part is completely undocumented even :-(
> It seems unintentional (and wrong) to me, but yes we are in stage 4, if
> we want to clean this up one way or the other, now is not the time.

Yeah.  I guess rtvec_alloc should either assert n > 0 or return NULL_RTVEC
when n == 0.

> In that case: your patch looks good to me Marek.

Thanks.  I've tweaked the patch to use ASM_OPERANDS_LABEL_LENGTH rather than
GET_NUM_ELEM.  I'll push it if it passes testing on x86_64-pc-linux-gnu.

-- >8 --
In r270550, Jakub fixed classify_insn to handle asm goto: if the asm can
jump to a label, the insn should be a JUMP_INSN.

However, as the following testcase shows, non-null ASM_OPERANDS_LABEL_VEC
doesn't guarantee that the rtx has any actual labels it can branch to.
Here, the rtvec has 0 elements because expand_asm_stmt created it:

  rtvec labelvec = rtvec_alloc (nlabels); // nlabels == 0

This causes an ICE in update_br_prob_note: BRANCH_EDGE (bb) crashes
because there's no branch edge.  I think we can fix this by checking
that there is at least one label the asm can jump to before wrapping
the ASM_OPERANDS in a JUMP_INSN.

	PR rtl-optimization/104777

gcc/ChangeLog:

	* rtl.cc (classify_insn): For ASM_OPERANDS, return JUMP_INSN only if
	ASM_OPERANDS_LABEL_VEC has at least one element.

gcc/testsuite/ChangeLog:

	* gcc.dg/torture/tls/pr104777.c: New test.
---
 gcc/rtl.cc                                  |  4 +--
 gcc/testsuite/gcc.dg/torture/tls/pr104777.c | 30 +++++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/tls/pr104777.c

diff --git a/gcc/rtl.cc b/gcc/rtl.cc
index f17474bfee1..d383ae9c099 100644
--- a/gcc/rtl.cc
+++ b/gcc/rtl.cc
@@ -765,7 +765,7 @@ classify_insn (rtx x)
     return CALL_INSN;
   if (ANY_RETURN_P (x))
     return JUMP_INSN;
-  if (GET_CODE (x) == ASM_OPERANDS && ASM_OPERANDS_LABEL_VEC (x))
+  if (GET_CODE (x) == ASM_OPERANDS && ASM_OPERANDS_LABEL_LENGTH (x) > 0)
     return JUMP_INSN;
   if (GET_CODE (x) == SET)
     {
@@ -794,7 +794,7 @@ classify_insn (rtx x)
       if (has_return_p)
 	return JUMP_INSN;
       if (GET_CODE (XVECEXP (x, 0, 0)) == ASM_OPERANDS
-	  && ASM_OPERANDS_LABEL_VEC (XVECEXP (x, 0, 0)))
+	  && ASM_OPERANDS_LABEL_LENGTH (XVECEXP (x, 0, 0)) > 0)
 	return JUMP_INSN;
     }
 #ifdef GENERATOR_FILE
diff --git a/gcc/testsuite/gcc.dg/torture/tls/pr104777.c b/gcc/testsuite/gcc.dg/torture/tls/pr104777.c
new file mode 100644
index 00000000000..abaf59731fc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/tls/pr104777.c
@@ -0,0 +1,30 @@
+/* PR rtl-optimization/104777 */
+/* { dg-do compile } */
+/* { dg-require-effective-target tls } */
+ 
+int savestate_r;
+int savestate_ssb;
+extern void abort();
+__thread int  loop;
+void f (void)
+{
+  int savestate_r0_5;
+  int savestate_r1_6;
+
+  __asm__("" : "=m" (savestate_ssb), "=r" (savestate_r));
+  savestate_r0_5 = savestate_r;
+  if (savestate_r0_5 == 0)
+  {
+    __asm__ __volatile__("" :  : "m" (loop));
+    abort ();
+  }
+
+  __asm__("" : "=m" (savestate_ssb), "=r" (savestate_r));
+  savestate_r1_6 = savestate_r;
+  if (savestate_r1_6 != 0)
+    return;
+
+  __asm__ __volatile__("" :  : "m" (loop));
+  abort ();
+
+}

base-commit: 058d19b42ad4c4c22635f70db6913a80884aedec
-- 
2.35.1

Re: [PATCH v2] rtl: ICE with thread_local and inline asm [PR104777]

[Jakub Jelinek]

On Tue, Mar 08, 2022 at 11:33:59AM -0500, Marek Polacek wrote:
> 	PR rtl-optimization/104777
> 
> gcc/ChangeLog:
> 
> 	* rtl.cc (classify_insn): For ASM_OPERANDS, return JUMP_INSN only if
> 	ASM_OPERANDS_LABEL_VEC has at least one element.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/torture/tls/pr104777.c: New test.
> ---
>  gcc/rtl.cc                                  |  4 +--
>  gcc/testsuite/gcc.dg/torture/tls/pr104777.c | 30 +++++++++++++++++++++
>  2 files changed, 32 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/tls/pr104777.c
> 
> diff --git a/gcc/rtl.cc b/gcc/rtl.cc
> index f17474bfee1..d383ae9c099 100644
> --- a/gcc/rtl.cc
> +++ b/gcc/rtl.cc
> @@ -765,7 +765,7 @@ classify_insn (rtx x)
>      return CALL_INSN;
>    if (ANY_RETURN_P (x))
>      return JUMP_INSN;
> -  if (GET_CODE (x) == ASM_OPERANDS && ASM_OPERANDS_LABEL_VEC (x))
> +  if (GET_CODE (x) == ASM_OPERANDS && ASM_OPERANDS_LABEL_LENGTH (x) > 0)
>      return JUMP_INSN;
>    if (GET_CODE (x) == SET)
>      {
> @@ -794,7 +794,7 @@ classify_insn (rtx x)
>        if (has_return_p)
>  	return JUMP_INSN;
>        if (GET_CODE (XVECEXP (x, 0, 0)) == ASM_OPERANDS
> -	  && ASM_OPERANDS_LABEL_VEC (XVECEXP (x, 0, 0)))
> +	  && ASM_OPERANDS_LABEL_LENGTH (XVECEXP (x, 0, 0)) > 0)

I think the > 0 in there is unnecessary, negative XVECLEN would be invalid
RTL.
Ok for trunk either way if it passes testing.

	Jakub