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
