Adding ignore fuchsia tests for signal interpretation cases #102032

andrewpollack · 2022-09-19T21:04:10Z

Tests where Signal interpreting is required. Since Fuchsia currently does not return signals of type libc::SIGSEGV etc., instead, use generalized !status.success() case.

cc. @djkoloski

r? @tmandry

src/test/ui/runtime/out-of-stack.rs

src/test/ui/abi/segfault-no-out-of-stack.rs

src/test/ui/abi/stack-probes-lto.rs

src/test/ui/process/signal-exit-status.rs

tmandry · 2022-09-20T22:48:25Z

src/test/ui/process/signal-exit-status.rs

@@ -2,6 +2,7 @@
 // ignore-emscripten no processes
 // ignore-sgx no processes
 // ignore-windows
+// ignore-fuchsia code returned as ZX_TASK_RETCODE_EXCEPTION_KILL, FIXME (#109976)


There's no Fuchsia issue here; the issue is on the standard library exposing these concepts correctly, so it belongs on this Github repo. Probably you can point directly to #58590. (Short of that, which is a major change, it's possible we could map this std::os::unix::process::ExitStatusExt API partly onto Fuchsia contexts.)

Btw, to link to the issue you created you want http://fxbug.dev/109976. A number starting with # refers to an issue in this Github repo.

tmandry · 2022-09-20T22:48:39Z

src/test/ui/runtime/out-of-stack.rs

@@ -5,6 +5,7 @@
 // ignore-android: FIXME (#20004)
 // ignore-emscripten no processes
 // ignore-sgx no processes
+// ignore-fuchsia must translate zircon signal to SIGABRT, FIXME (#109976)


same as above

tmandry · 2022-09-20T22:48:54Z

src/test/ui/abi/segfault-no-out-of-stack.rs

@@ -3,6 +3,7 @@
 #![allow(unused_imports)]
 // ignore-emscripten can't run commands
 // ignore-sgx no processes
+// ignore-fuchsia must translate zircon signal to SIGSEGV/SIGBUS, FIXME (#109976)


This test has a handler for not(unix); we can use that one for Fuchsia without disabling the test entirely.

A previous version changed those lines into:
#[cfg(all(unix, not(target_os = "fuchsia")))]
#[cfg(any(not(unix), target_os = "fuchsia"))]
Is this how I should approach the change?

Yeah I think that's right and you could also leave a FIXME with a link to the same issue you're using for the other tests.

I think that manually adding Fuchsia to the #[cfg(not(unix))] case may cause us to forget that this test does not actually pass for Fuchsia. We have to either support this functionality or remove Fuchsia from #[cfg(unix)], upon which this test will pass with no modifications.

To clarify, the test "passes" in the sense that it does not have a successful status !status.success(), but "fails" in the sense that the signal is not what is expected (SIGSEGV/SIGBUS). I agree that this comes down to whether this is expected behavior or not for Posix-lite

tmandry · 2022-09-21T17:01:06Z

@bors r+ rollup

bors · 2022-09-21T17:01:08Z

📌 Commit 6c29716 has been approved by tmandry

It is now in the queue for this repository.

Rollup of 12 pull requests Successful merges: - rust-lang#101952 (Avoid panicking on missing fallback) - rust-lang#102030 (Don't crate-locally reexport walk functions in tidy) - rust-lang#102032 (Adding ignore fuchsia tests for signal interpretation cases) - rust-lang#102033 (Adding needs-unwind to nicer-assert-messages compiler ui tests) - rust-lang#102054 (Unify "all items" page's sidebar with other pages) - rust-lang#102071 (Adding needs-unwind for tests testing memory size of Futures/Closures) - rust-lang#102073 (Adding ignore fuchsia tests for execvp) - rust-lang#102075 (rustdoc: remove no-op CSS `.content > .methods > .method`) - rust-lang#102079 (Update books) - rust-lang#102084 (Adding needs-unwind for test using panic::catch_unwind) - rust-lang#102100 (Prevent usage of .stab elements to create scrollable areas in doc blocks) - rust-lang#102102 (Add doc aliases on Sized trait) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…r=tmandry Fixup failing fuchsia tests The Fuchsia platform passes all tests with these changes. Two tests are ignored because they rely on Fuchsia not returning a status code upon a process aborting. See rust-lang#102032 and rust-lang#58590 for more details on that topic. Many formatting changes are also included in this PR. r? tmandry r? erickt

Rollup merge of rust-lang#127461 - c6c7:fixup-failing-fuchsia-tests, r=tmandry Fixup failing fuchsia tests The Fuchsia platform passes all tests with these changes. Two tests are ignored because they rely on Fuchsia not returning a status code upon a process aborting. See rust-lang#102032 and rust-lang#58590 for more details on that topic. Many formatting changes are also included in this PR. r? tmandry r? erickt

rust-highfive assigned tmandry Sep 19, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 19, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2022

andrewpollack force-pushed the bad-signal-compiler-tests branch 2 times, most recently from 6a8bdb5 to 45e258e Compare September 20, 2022 19:20

djkoloski reviewed Sep 20, 2022

View reviewed changes

src/test/ui/runtime/out-of-stack.rs Outdated Show resolved Hide resolved

src/test/ui/abi/segfault-no-out-of-stack.rs Outdated Show resolved Hide resolved

andrewpollack force-pushed the bad-signal-compiler-tests branch 2 times, most recently from e6bb6d3 to 812db90 Compare September 20, 2022 21:04

djkoloski reviewed Sep 20, 2022

View reviewed changes

src/test/ui/abi/stack-probes-lto.rs Outdated Show resolved Hide resolved

src/test/ui/process/signal-exit-status.rs Outdated Show resolved Hide resolved

andrewpollack force-pushed the bad-signal-compiler-tests branch 3 times, most recently from 1fbad5d to b4383f5 Compare September 20, 2022 22:40

tmandry reviewed Sep 20, 2022

View reviewed changes

andrewpollack requested a review from tmandry September 20, 2022 22:55

andrewpollack force-pushed the bad-signal-compiler-tests branch from b4383f5 to 9a2b5ba Compare September 21, 2022 00:19

djkoloski approved these changes Sep 21, 2022

View reviewed changes

Adding ignore fuchsia tests for signal cases

6c29716

andrewpollack force-pushed the bad-signal-compiler-tests branch from 9a2b5ba to 6c29716 Compare September 21, 2022 15:57

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 21, 2022

JohnTitor mentioned this pull request Sep 22, 2022

Rollup of 12 pull requests #102121

Merged

bors merged commit 5542e50 into rust-lang:master Sep 22, 2022

rustbot added this to the 1.66.0 milestone Sep 22, 2022

tmandry added the O-fuchsia Operating system: Fuchsia label May 24, 2023

c6c7 mentioned this pull request Jul 7, 2024

Fixup failing fuchsia tests #127461

Merged

c6c7 mentioned this pull request Jul 11, 2024

Fix panic=abort tests on fuchsia #127595

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding ignore fuchsia tests for signal interpretation cases #102032

Adding ignore fuchsia tests for signal interpretation cases #102032

andrewpollack commented Sep 19, 2022

tmandry Sep 20, 2022

tmandry Sep 20, 2022

tmandry Sep 20, 2022

andrewpollack Sep 20, 2022

tmandry Sep 20, 2022 •

edited

Loading

djkoloski Sep 20, 2022

andrewpollack Sep 20, 2022

tmandry commented Sep 21, 2022

bors commented Sep 21, 2022

Adding ignore fuchsia tests for signal interpretation cases #102032

Adding ignore fuchsia tests for signal interpretation cases #102032

Conversation

andrewpollack commented Sep 19, 2022

tmandry Sep 20, 2022

Choose a reason for hiding this comment

tmandry Sep 20, 2022

Choose a reason for hiding this comment

tmandry Sep 20, 2022

Choose a reason for hiding this comment

andrewpollack Sep 20, 2022

Choose a reason for hiding this comment

tmandry Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

djkoloski Sep 20, 2022

Choose a reason for hiding this comment

andrewpollack Sep 20, 2022

Choose a reason for hiding this comment

tmandry commented Sep 21, 2022

bors commented Sep 21, 2022

tmandry Sep 20, 2022 •

edited

Loading