qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: "Alex Bennée" <alex.bennee@linaro.org>,
	"Taylor Simpson" <tsimpson@quicinc.com>
Cc: "Brian Cain" <bcain@quicinc.com>,
	"peter.maydell@linaro.org" <peter.maydell@linaro.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Hexagon toolchain update vs linux-user signals
Date: Wed, 3 Nov 2021 11:22:03 -0400	[thread overview]
Message-ID: <4f65337b-b9ed-dc4c-ac09-025bef5eaa4c@linaro.org> (raw)
In-Reply-To: <87ee7xjqlv.fsf@linaro.org>

On 11/3/21 9:31 AM, Alex Bennée wrote:
>>> Could it be a toolchain thing?
>>
>> Not likely a toolchain problem.  If I can access both of the signals
>> binaries, I can confirm.
> 
> Testing against two signals binaries I see a 4-7% failure rate against the
> new binary versus the original pre-toolchain change one. That's not to
> say the binary is broken - it could be a subtle change that exacerbated
> our existing poor signals support.
> 
>    https://transfer.sh/xA2ejk/signals.old (pre-toolchain change)
>    https://transfer.sh/vSsn5s/signals
> 
> something in the CI ensures it fails much more reliably as U can't get
> it to pass on a retry.

I've had a closer look at the signals failure, and it really could be a toolchain problem.

The sigsegv is at

#0  0x00005555557a387f in stb_p (ptr=0x10000, v=0 '\000')
     at /home/richard.henderson/qemu/src/include/qemu/bswap.h:326
#1  0x00005555557a4bc5 in cpu_stb_mmu (env=0x555555e4eb50, addr=0,
     val=0 '\000', oi=0, ra=93824992935986) at ../src/accel/tcg/user-exec.c:359
#2  0x00005555557a5396 in cpu_stb_mmuidx_ra (env=0x555555e4eb50, addr=0,
     val=0, mmu_idx=0, ra=93824992935986)
     at ../src/accel/tcg/ldst_common.c.inc:83
#3  0x00005555557a57e6 in cpu_stb_data_ra (env=0x555555e4eb50, addr=0, val=0,
     ra=93824992935986) at ../src/accel/tcg/ldst_common.c.inc:183
#4  0x00005555555ff6f0 in helper_commit_store (env=0x555555e4eb50, slot_num=1)
     at ../src/target/hexagon/op_helper.c:151
#5  0x0000555555600032 in check_noshuf (env=0x555555e4eb50, slot=0)
     at ../src/target/hexagon/op_helper.c:407
#6  0x00005555556000e4 in mem_load4 (env=0x555555e4eb50, slot=0, vaddr=305000)
     at ../src/target/hexagon/op_helper.c:431
#7  0x00005555556063c0 in helper_L2_loadri_io (env=0x555555e4eb50, RsV=305000,
     siV=0, slot=0) at target/hexagon/helper_funcs_generated.c.inc:1013
#8  0x00007fffe8034f5a in code_gen_buffer ()

which is a store to address 0, which obviously should fail.

This comes from

IN: nontrivial_free
0x000224c4:  0x78004003 {       R3 = #0x0
0x000224c8:  0xf204d001         P1 = cmp.eq(R4,R16) }
0x000224cc:  0x5c00413e {       if (P1) jump:nt PC+124
0x000224d0:  0x38034000         if (P0) memb(R3+#0x0) = #0x0
0x000224d4:  0x9180c002         R2 = memw(R0+#0x0) }

which is part of the new toolchain's libc.  This is quite obviously a store to address 0 
if P0 is true.  Which looks pretty questionable.  Presumably P0 is not always set, which 
is why the program does not always crash.  But there doesn't appear to be anything wrong 
with the qemu translation.

I'm suspicious of the new compiler.  This looks like some sort of code scheduling bug, 
where R3=0 got moved ahead of the final use of the previous value in R3.

In the short term, I recommend dropping the hexagon toolchain update and that Taylor 
generate a new HVX pull request with the new tests present but disabled in the makefile.


r~


  reply	other threads:[~2021-11-03 15:37 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-31 16:42 [PULL 00/30] Hexagon HVX (target/hexagon) patch series Taylor Simpson
2021-10-31 16:42 ` [PULL 01/30] Hexagon HVX (target/hexagon) README Taylor Simpson
2021-10-31 16:42 ` [PULL 02/30] Hexagon HVX (target/hexagon) add Hexagon Vector eXtensions (HVX) to core Taylor Simpson
2021-10-31 16:42 ` [PULL 03/30] Hexagon HVX (target/hexagon) register names Taylor Simpson
2021-10-31 16:42 ` [PULL 04/30] Hexagon HVX (target/hexagon) instruction attributes Taylor Simpson
2021-10-31 16:42 ` [PULL 05/30] Hexagon HVX (target/hexagon) macros Taylor Simpson
2021-10-31 16:42 ` [PULL 06/30] Hexagon HVX (target/hexagon) import macro definitions Taylor Simpson
2021-10-31 16:42 ` [PULL 07/30] Hexagon HVX (target/hexagon) semantics generator Taylor Simpson
2021-10-31 16:42 ` [PULL 08/30] Hexagon HVX (target/hexagon) semantics generator - part 2 Taylor Simpson
2021-10-31 16:42 ` [PULL 09/30] Hexagon HVX (target/hexagon) C preprocessor for decode tree Taylor Simpson
2021-10-31 16:42 ` [PULL 10/30] Hexagon HVX (target/hexagon) instruction utility functions Taylor Simpson
2021-10-31 16:42 ` [PULL 11/30] Hexagon HVX (target/hexagon) helper functions Taylor Simpson
2021-10-31 16:42 ` [PULL 12/30] Hexagon HVX (target/hexagon) TCG generation Taylor Simpson
2021-10-31 16:42 ` [PULL 13/30] Hexagon HVX (target/hexagon) helper overrides infrastructure Taylor Simpson
2021-10-31 16:42 ` [PULL 14/30] Hexagon HVX (target/hexagon) helper overrides for histogram instructions Taylor Simpson
2021-10-31 16:42 ` [PULL 15/30] Hexagon HVX (target/hexagon) helper overrides - vector assign & cmov Taylor Simpson
2021-10-31 16:42 ` [PULL 16/30] Hexagon HVX (target/hexagon) helper overrides - vector add & sub Taylor Simpson
2021-10-31 16:42 ` [PULL 17/30] Hexagon HVX (target/hexagon) helper overrides - vector shifts Taylor Simpson
2021-10-31 16:42 ` [PULL 18/30] Hexagon HVX (target/hexagon) helper overrides - vector max/min Taylor Simpson
2021-10-31 16:42 ` [PULL 19/30] Hexagon HVX (target/hexagon) helper overrides - vector logical ops Taylor Simpson
2021-10-31 16:42 ` [PULL 20/30] Hexagon HVX (target/hexagon) helper overrides - vector compares Taylor Simpson
2021-10-31 16:43 ` [PULL 21/30] Hexagon HVX (target/hexagon) helper overrides - vector splat and abs Taylor Simpson
2021-10-31 16:43 ` [PULL 22/30] Hexagon HVX (target/hexagon) helper overrides - vector loads Taylor Simpson
2021-10-31 16:43 ` [PULL 23/30] Hexagon HVX (target/hexagon) helper overrides - vector stores Taylor Simpson
2021-10-31 16:43 ` [PULL 24/30] Hexagon HVX (target/hexagon) import semantics Taylor Simpson
2021-10-31 16:43 ` [PULL 25/30] Hexagon HVX (target/hexagon) instruction decoding Taylor Simpson
2021-10-31 16:43 ` [PULL 26/30] Hexagon HVX (target/hexagon) import instruction encodings Taylor Simpson
2021-10-31 16:43 ` [PULL 27/30] Hexagon HVX (tests/tcg/hexagon) vector_add_int test Taylor Simpson
2021-10-31 16:43 ` [PULL 28/30] Hexagon HVX (tests/tcg/hexagon) hvx_misc test Taylor Simpson
2021-11-01 10:33   ` Philippe Mathieu-Daudé
2021-11-01 13:43     ` Richard Henderson
2021-11-01 14:09       ` Taylor Simpson
2021-11-01 14:17         ` Philippe Mathieu-Daudé
2021-11-01 15:02           ` Richard Henderson
2021-11-02 16:05             ` Taylor Simpson
2021-11-02 16:41               ` Alex Bennée
2021-11-02 16:53                 ` Taylor Simpson
2021-11-03 13:31                   ` Alex Bennée
2021-11-03 15:22                     ` Richard Henderson [this message]
2021-10-31 16:43 ` [PULL 29/30] Hexagon HVX (tests/tcg/hexagon) scatter_gather test Taylor Simpson
2021-10-31 16:43 ` [PULL 30/30] Hexagon HVX (tests/tcg/hexagon) histogram test Taylor Simpson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4f65337b-b9ed-dc4c-ac09-025bef5eaa4c@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=bcain@quicinc.com \
    --cc=f4bug@amsat.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=tsimpson@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).