Просмотр исходного кода

gcc: fix the h8300 insn length that makes branches miss their target

The *addsi_flags pattern lets output_plussi() decide between the two-byte
adds/subs and the six-byte sub.l #imm:32 -- adds/subs do not set the
condition flags, so a value that is branched on needs the long form. The
length attribute never got that distinction: compute_plussi_length()
takes a need_flags argument and returns 6 for exactly this case, but its
only caller passed false unconditionally, so the insn was costed as two
bytes.

Four bytes too few is enough to make gcc pick an 8-bit displacement for a
branch that does not reach. Nothing downstream catches it: gas sets
linkrelax = 1 for h8300 and never resolves a branch itself, and the
linker's relax pass can only delete bytes, never insert them. The
displacement is truncated to a byte and the branch jumps the wrong way.

Found as a hang in uClibc-ng's memcpy(): the unrolled loop in
_wordcopy_fwd_aligned() spans 132 bytes, gcc computed 128, and the
back-edge became a forward jump out of the function. memcpy() and
memmove() are the only two places in the whole library where the
miscompilation triggers; the 4.4 kernel is unaffected.

Introduced by gcc 7da97411b048 ("Use add/sub/neg insns to eliminate
compare/test insns on H8"), which taught output_plussi() about need_flags
and prepared compute_plussi_length() for it but left the length attribute
passing false. Still present in gcc trunk.

Signed-off-by: Ramin Moussavi <ramin.moussavi@yacoub.de>
Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
ramin 4 дней назад
Родитель
Сommit
fe0870b979
1 измененных файлов с 37 добавлено и 0 удалено
  1. 37 0
      toolchain/gcc/patches/13.4.0/0011-h8300-length-of-flag-setting-addsi.patch

+ 37 - 0
toolchain/gcc/patches/13.4.0/0011-h8300-length-of-flag-setting-addsi.patch

@@ -0,0 +1,37 @@
+h8300: length of the flag-setting add/sub must account for the long form
+
+adds/subs #1,#2,#4 are two bytes but do not set the condition flags, so
+when the result is branched on, output_plussi() has to emit the six-byte
+sub.l #imm:32 instead.  compute_plussi_length() knows this and returns 6
+for that case, but its only caller passed need_flags unconditionally as
+false, so the insn was costed as two bytes.
+
+Four bytes too few is enough to make the branch selection pick an 8-bit
+displacement that does not reach.  gas defers every h8300 branch to the
+linker (linkrelax = 1) and the linker may only shrink, never grow, so the
+displacement is silently truncated to a byte and the branch jumps the
+wrong way.  Seen as a hang in uClibc-ng's memcpy(): the loop in
+_wordcopy_fwd_aligned() spans exactly 132 bytes, gcc computed 128.
+
+Introduced by gcc commit 7da97411b048 ("Use add/sub/neg insns to
+eliminate compare/test insns on H8"), which taught output_plussi() the
+need_flags distinction and prepared compute_plussi_length() for it but
+left the length attribute passing false.  Still present in gcc trunk.
+
+Reproducer, miscompiled with -Os -mh -mint32 at exactly 16 assignments:
+
+  extern volatile long v[16];
+  void f (long n) { do { v[0]=n; ... v[15]=n; n -= 4; } while (n != 0); }
+
+diff -Nur gcc-13.4.0.orig/gcc/config/h8300/addsub.md gcc-13.4.0/gcc/config/h8300/addsub.md
+--- gcc-13.4.0.orig/gcc/config/h8300/addsub.md	2026-08-30 15:56:00.012266632 +0200
++++ gcc-13.4.0/gcc/config/h8300/addsub.md	2026-08-30 15:56:00.156816287 +0200
+@@ -132,7 +132,7 @@
+   return output_plussi (operands, GET_CODE (x) != CLOBBER);
+ }
+   [(set (attr "length")
+-	(symbol_ref "compute_plussi_length (operands, false)"))])
++	(symbol_ref "compute_plussi_length (operands, GET_CODE (XVECEXP (PATTERN (insn), 0, 1)) != CLOBBER)"))])
+ 
+ ;; ----------------------------------------------------------------------
+ ;; SUBTRACT INSTRUCTIONS