-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
s390x: illegal instruction in unit tests #47064
Comments
In mostly default compilation case, looking at the disassembly of the core file, it points to this instruction.
|
cc @nodejs/platform-s390 |
Thank you for reporting the issue, it's related to |
Looks like this was introduced in V8 by https://chromium-review.googlesource.com/c/v8/v8/+/3930898 |
@AdamMajer Hello, would you please verify if this v8 patch work for you? https://chromium-review.googlesource.com/c/v8/v8/+/4334353 |
Sorry for lateness of the reply. Checking now. |
It looks like it still crashes in same unit tests. I will check which code path it's using there. |
Finally, I'm looking at this again. Building now with 20.1.0 where the v8 patch is now included,
This looks like it's coming from src/compiler/backend/s390/code-generator-s390.cc:1706 |
Hi Adam, |
Looks like this fixes the issue. I'll get back tomorrow if something pops up when all the tests run. Thanks! |
Glad to hear, thanks for confirming. |
All tests now pass on z13 and also z15. Thank you for fixing this! |
Not a problem, thanks again for confirming. |
Original commit message: Fix usage of MulHighS64 on <= z13 mgrk is only available with MISC_INSTR_EXT2 installed. A runtime call needs to be made if it's unavailable. We also need to make sure caller saved registers are saved accordingly before making a call. Change-Id: If7ac06eef57cc3db059c2640b77c80de3b16fced Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4521297 Reviewed-by: Junliang Yan <junyan@redhat.com> Commit-Queue: Milad Farazmand <mfarazma@redhat.com> Cr-Commit-Position: refs/heads/main@{#87675} Fixes: nodejs#47064
Original commit message: Fix usage of MulHighS64 on <= z13 mgrk is only available with MISC_INSTR_EXT2 installed. A runtime call needs to be made if it's unavailable. We also need to make sure caller saved registers are saved accordingly before making a call. Change-Id: If7ac06eef57cc3db059c2640b77c80de3b16fced Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4521297 Reviewed-by: Junliang Yan <junyan@redhat.com> Commit-Queue: Milad Farazmand <mfarazma@redhat.com> Cr-Commit-Position: refs/heads/main@{#87675} Fixes: nodejs#47064
Original commit message: Fix usage of MulHighS64 on <= z13 mgrk is only available with MISC_INSTR_EXT2 installed. A runtime call needs to be made if it's unavailable. We also need to make sure caller saved registers are saved accordingly before making a call. Change-Id: If7ac06eef57cc3db059c2640b77c80de3b16fced Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4521297 Reviewed-by: Junliang Yan <junyan@redhat.com> Commit-Queue: Milad Farazmand <mfarazma@redhat.com> Cr-Commit-Position: refs/heads/main@{#87675} Fixes: nodejs#47064
Original commit message: Fix usage of MulHighS64 on <= z13 mgrk is only available with MISC_INSTR_EXT2 installed. A runtime call needs to be made if it's unavailable. We also need to make sure caller saved registers are saved accordingly before making a call. Change-Id: If7ac06eef57cc3db059c2640b77c80de3b16fced Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4521297 Reviewed-by: Junliang Yan <junyan@redhat.com> Commit-Queue: Milad Farazmand <mfarazma@redhat.com> Cr-Commit-Position: refs/heads/main@{#87675} Fixes: #47064 PR-URL: #49401 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Original commit message: Fix usage of MulHighS64 on <= z13 mgrk is only available with MISC_INSTR_EXT2 installed. A runtime call needs to be made if it's unavailable. We also need to make sure caller saved registers are saved accordingly before making a call. Change-Id: If7ac06eef57cc3db059c2640b77c80de3b16fced Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4521297 Reviewed-by: Junliang Yan <junyan@redhat.com> Commit-Queue: Milad Farazmand <mfarazma@redhat.com> Cr-Commit-Position: refs/heads/main@{#87675} Fixes: #47064 PR-URL: #49401 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Original commit message: Fix usage of MulHighS64 on <= z13 mgrk is only available with MISC_INSTR_EXT2 installed. A runtime call needs to be made if it's unavailable. We also need to make sure caller saved registers are saved accordingly before making a call. Change-Id: If7ac06eef57cc3db059c2640b77c80de3b16fced Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4521297 Reviewed-by: Junliang Yan <junyan@redhat.com> Commit-Queue: Milad Farazmand <mfarazma@redhat.com> Cr-Commit-Position: refs/heads/main@{#87675} Fixes: nodejs#47064 PR-URL: nodejs#49401 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Version
19.6.0
Platform
No response
Subsystem
v8
What steps will reproduce the bug?
No response
How often does it reproduce? Is there a required condition?
Every time
What is the expected behavior?
No errors
What do you see instead?
Compiling with mostly default options results in some unit tests failures in file system tests. All these ended up with <Builtins_MulHandler+868> on z13 while running just fine on z15 machine.
For example, when executing
test/parallel/test-fs-cp.mjs
ortest/parallel/test-fs-stat-bigint.js
I've then added
--v8-enable-object-print --v8-non-optimized-debug --v8-with-dchecks
to the configure and rebuilt. During the build, the following error popped upwhich points to,
@export
macro IsHeapNumber(o: HeapObject): bool {
return Is(o);
}
And additionally adding
--v8-enable-short-builtin-calls
to the configure, I get an error in the macro assembler for s390 where unreachable code is apparently reached.Additional information
Looking at the logs, the failures in the unit tests started with 19.3.0 and 19.1.0 was last version where everything worked. This seems to point to v8 update then as possible culprit.
#45230
The text was updated successfully, but these errors were encountered: