libunwind: Remove OS requirements from tests to make them run on more OSes #167642

arrowd · 2025-11-12T05:54:11Z

There might be a cleaner way to enable these tests running on FreeBSD, I'm open to suggestions.

llvmbot · 2025-11-12T05:54:41Z

@llvm/pr-subscribers-libunwind

Author: Gleb Popov (arrowd)

Changes

There might be a cleaner way to enable these tests running on FreeBSD, I'm open to suggestions.

Full diff: https://github.com/llvm/llvm-project/pull/167642.diff

6 Files Affected:

(modified) libunwind/test/bad_unwind_info.pass.cpp (+1-1)
(modified) libunwind/test/eh_frame_fde_pc_range.pass.cpp (+2-2)
(modified) libunwind/test/forceunwind.pass.cpp (+1-1)
(modified) libunwind/test/remember_state_leak.pass.sh.s (+1-1)
(modified) libunwind/test/signal_unwind.pass.cpp (+1-1)
(modified) libunwind/test/unwind_leaffunction.pass.cpp (+1-1)

diff --git a/libunwind/test/bad_unwind_info.pass.cpp b/libunwind/test/bad_unwind_info.pass.cpp
index 272a83f64a611..2fadcfefdecc3 100644
--- a/libunwind/test/bad_unwind_info.pass.cpp
+++ b/libunwind/test/bad_unwind_info.pass.cpp
@@ -10,7 +10,7 @@
 // Ensure that libunwind doesn't crash on invalid info; the Linux aarch64
 // sigreturn frame check would previously attempt to access invalid memory in
 // this scenario.
-// REQUIRES: target={{(aarch64|s390x|x86_64)-.+linux.*}}
+// REQUIRES: target={{(aarch64|s390x|x86_64)-.+(linux|freebsd).*}}
 
 // GCC doesn't support __attribute__((naked)) on AArch64.
 // UNSUPPORTED: gcc
diff --git a/libunwind/test/eh_frame_fde_pc_range.pass.cpp b/libunwind/test/eh_frame_fde_pc_range.pass.cpp
index 795ce66806f28..e86f80839a655 100644
--- a/libunwind/test/eh_frame_fde_pc_range.pass.cpp
+++ b/libunwind/test/eh_frame_fde_pc_range.pass.cpp
@@ -13,7 +13,7 @@
 
 // clang-format off
 
-// REQUIRES: target={{x86_64-.+-linux-gnu}}
+// REQUIRES: target={{x86_64-.+-(linux-gnu|freebsd.*)}}
 // REQUIRES: objcopy-available
 
 // TODO: Figure out why this fails with Memory Sanitizer.
@@ -21,7 +21,7 @@
 
 // RUN: %{build}
 // RUN: %{objcopy} --dump-section .eh_frame_hdr=%t_ehf_hdr.bin %t.exe
-// RUN: echo -ne '\xFF' | dd of=%t_ehf_hdr.bin bs=1 seek=2 count=2 conv=notrunc status=none
+// RUN: printf '\377' | dd of=%t_ehf_hdr.bin bs=1 seek=2 count=2 conv=notrunc status=none
 // RUN: %{objcopy} --update-section .eh_frame_hdr=%t_ehf_hdr.bin %t.exe
 // RUN: %{exec} %t.exe
 
diff --git a/libunwind/test/forceunwind.pass.cpp b/libunwind/test/forceunwind.pass.cpp
index 9e032fc680806..f3ad08c2cf693 100644
--- a/libunwind/test/forceunwind.pass.cpp
+++ b/libunwind/test/forceunwind.pass.cpp
@@ -7,7 +7,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// REQUIRES: linux
+// REQUIRES: linux || freebsd
 
 // TODO: Figure out why this fails with Memory Sanitizer.
 // XFAIL: msan
diff --git a/libunwind/test/remember_state_leak.pass.sh.s b/libunwind/test/remember_state_leak.pass.sh.s
index d3335cf82290b..0e0723f66d3c8 100644
--- a/libunwind/test/remember_state_leak.pass.sh.s
+++ b/libunwind/test/remember_state_leak.pass.sh.s
@@ -6,7 +6,7 @@
 #
 #===------------------------------------------------------------------------===#
 
-# REQUIRES: target={{x86_64-.+-linux-gnu}}
+# REQUIRES: target={{x86_64-.+-(linux-gnu|freebsd.*)}}
 
 # Inline assembly isn't supported by Memory Sanitizer
 # UNSUPPORTED: msan
diff --git a/libunwind/test/signal_unwind.pass.cpp b/libunwind/test/signal_unwind.pass.cpp
index 4de271ecb886b..dd2bd9f2f1d5a 100644
--- a/libunwind/test/signal_unwind.pass.cpp
+++ b/libunwind/test/signal_unwind.pass.cpp
@@ -8,7 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 // Ensure that the unwinder can cope with the signal handler.
-// REQUIRES: target={{(aarch64|loongarch64|riscv64|s390x|x86_64)-.+linux.*}}
+// REQUIRES: target={{(aarch64|loongarch64|riscv64|s390x|x86_64)-.+(linux|freebsd).*}}
 
 // TODO: Figure out why this fails with Memory Sanitizer.
 // XFAIL: msan
diff --git a/libunwind/test/unwind_leaffunction.pass.cpp b/libunwind/test/unwind_leaffunction.pass.cpp
index d336c159c131b..1c796b3433973 100644
--- a/libunwind/test/unwind_leaffunction.pass.cpp
+++ b/libunwind/test/unwind_leaffunction.pass.cpp
@@ -8,7 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 // Ensure that leaf function can be unwund.
-// REQUIRES: target={{(aarch64|loongarch64|riscv64|s390x|x86_64)-.+linux.*}}
+// REQUIRES: target={{(aarch64|loongarch64|riscv64|s390x|x86_64)-.+(linux|freebsd).*}}
 
 // TODO: Figure out why this fails with Memory Sanitizer.
 // XFAIL: msan

github-actions · 2025-11-12T05:55:51Z

✅ With the latest revision this PR passed the C/C++ code formatter.

arichardson

I feel like a lot of these should not have a requires Linux clause but instead list the unsupported targets.

It seems like a lot of the time they are actually checking for non-macos/non-windows. I wonder what happens when we drop the OS part of the triple from the test. Hopefully pre-commit CI will complain if this breaks something.

arichardson · 2025-11-12T06:23:45Z

libunwind/test/eh_frame_fde_pc_range.pass.cpp

 // clang-format off

-// REQUIRES: target={{x86_64-.+-linux-gnu}}
+// REQUIRES: target={{x86_64-.+-(linux-gnu|freebsd.*)}}


Not sure why this was restricted to Linux in the first place. Maybe because of the echo issue you fixed here?

arichardson · 2025-11-12T06:25:14Z

libunwind/test/eh_frame_fde_pc_range.pass.cpp


 // RUN: %{build}
 // RUN: %{objcopy} --dump-section .eh_frame_hdr=%t_ehf_hdr.bin %t.exe
-// RUN: echo -ne '\xFF' | dd of=%t_ehf_hdr.bin bs=1 seek=2 count=2 conv=notrunc status=none


I'm surprised this is still failing, echo should be a python built-in by default now. But fixing this to be more portable is good anyway (@boomanaiden154)

This test is still failing on x86_64 mingw:

# .---command stdout------------ # | 123 # | fde_pc = 00007FF64AE46030 # | fde_fpc = 0000000000000000 # | fde_fpc1 = 00007FF64AE46024 # `----------------------------- # .---command stderr------------ # | libunwind: pc not in table, pc=0x7FF64AE413E0 # | Assertion failed: fde_fpc != NULL, file D:\a\llvm-project\llvm-project\libunwind\test\eh_frame_fde_pc_range.pass.cpp, line 51 # `----------------------------- # error: command failed with exit status: 0xffffffff

So this probably still needs an exclusion for that. (And it's probably relevant to make it generic for all of windows again - libunwind isn't normally built and tested in msvc environments, so whatever issue there is is probably specific to all of windows, not windows-gnu in particular.)

arrowd · 2025-11-18T08:31:04Z

I have removed all OS requirements from tests to see the CI fallout. I'll add UNSUPPORTED clauses based on the build results.

arichardson

Thanks! This new approach looks good to me assuming CI is happy.

rorth · 2025-11-18T17:09:50Z

I haven't even tried building libunwind on Solaris in ages. I had it somewhat working years ago as part of a libc++ port that was never merged, so I can't help/comment here.

libunwind/test/aarch64_vg_unwind.pass.cpp

arichardson

Relaxing the windows regexes sounds good and it looks like there is just one macos test failure so this should almost be ready to land.

libunwind/test/forceunwind.pass.cpp

mstorsjo · 2025-11-21T08:25:40Z

libunwind/test/bad_unwind_info.pass.cpp

-// REQUIRES: target={{(aarch64|s390x|x86_64)-.+linux.*}}
+// REQUIRES: target={{(aarch64|s390x|x86_64)-.+}}
+// UNSUPPORTED: system-windows
+// UNSUPPORTED: target={{.*-windows-gnu}}


This seems redundant, system-windows should cover windows-gnu as well, if system-windows is set at all? If not, just relax the regex to {{.*-windows.*}} as suggested before.

Looks like the system-* features are not set anywhere. Here is the output from one of the windows builders. I think we should just use the target regex.

All available features: 32-bit-pointer, buildhost=win32, buildhost=windows, c++26, can-create-symlinks, clang, clang-20, clang-20.1, clang-20.1.8, diagnose-if-support, enable-benchmarks=run, executor-has-no-bash, gcc-style-warnings, glibc-old-ru_RU-decimal-point, has-fblocks, has-fconstexpr-steps, has-no-zdump, host-has-gdb-with-python, large_tests, libcpp-has-no-experimental-hardening-observe-semantic, libcpp-has-no-experimental-syncstream, libcpp-has-no-experimental-tzdb, libcpp-has-no-incomplete-pstl, locale.en_US.UTF-8, locale.fr_CA.ISO8859-1, locale.fr_FR.UTF-8, locale.ja_JP.UTF-8, locale.ru_RU.UTF-8, locale.zh_CN.UTF-8, long_tests, objcopy-available, objective-c++, optimization=none, std-at-least-c++03, std-at-least-c++11, std-at-least-c++14, std-at-least-c++17, std-at-least-c++20, std-at-least-c++23, std-at-least-c++26, stdlib=libc++, stdlib=llvm-libc++, target=i686-w64-windows-gnu, verify-support, win32-broken-printf-a-precision, win32-broken-utf8-wchar-ctype, windows

arichardson

I think it should be good to land once my suggestions have been applied. Thanks for working on this.

libunwind/test/aarch64_vg_unwind.pass.cpp

libunwind/test/aarch64_za_unwind.pass.cpp

libunwind/test/bad_unwind_info.pass.cpp

libunwind/test/eh_frame_fde_pc_range.pass.cpp

libunwind/test/forceunwind.pass.cpp

libunwind/test/remember_state_leak.pass.sh.s

libunwind/test/signal_unwind.pass.cpp

libunwind/test/unwind_scalable_vectors.pass.cpp

libunwind/test/unwind_leaffunction.pass.cpp

arichardson · 2025-11-21T23:29:07Z

It looks like there are also shorter feature names that can be used for the UNSUPPORTED: cases: https://github.com/llvm/llvm-project/blob/main/libcxx/utils/libcxx/test/features/platform.py

darwin and windows work (without the system- prefix.)

… OSes While at it, replace "echo -ne" with "printf" for more compatibility. Co-authored-by: Alexander Richardson <[email protected]>

arichardson

LGTM, will merge once CI passes.

arrowd · 2025-11-28T08:39:00Z

I have just re-reviewed my changes once more to be sure. Can we merge this?

mstorsjo

LGTM too

mstorsjo · 2025-11-28T21:15:32Z

I have just re-reviewed my changes once more to be sure. Can we merge this?

Feel free to. Or do you need someone else to merge it for you?

arrowd · 2025-11-29T04:09:42Z

Yes please, I don't have rights on this repo.

arrowd requested a review from a team as a code owner November 12, 2025 05:54

llvmbot added the libunwind label Nov 12, 2025

arichardson reviewed Nov 12, 2025

View reviewed changes

arrowd force-pushed the libunwind-freebsd-tests branch from ac87d2d to 087dad9 Compare November 18, 2025 08:30

arrowd force-pushed the libunwind-freebsd-tests branch from 087dad9 to 936acb2 Compare November 18, 2025 16:45

arrowd changed the title ~~libunwind: Enable more tests to run on FreeBSD~~ libunwind: Remove OS requirements from tests to make them run on more OSes Nov 18, 2025

arichardson approved these changes Nov 18, 2025

View reviewed changes

arichardson requested review from mstorsjo and rorth November 18, 2025 17:05

mstorsjo reviewed Nov 18, 2025

View reviewed changes

libunwind/test/aarch64_vg_unwind.pass.cpp Outdated Show resolved Hide resolved

arichardson approved these changes Nov 18, 2025

View reviewed changes

libunwind/test/forceunwind.pass.cpp Show resolved Hide resolved

arrowd force-pushed the libunwind-freebsd-tests branch 4 times, most recently from 12e9693 to 369be95 Compare November 20, 2025 17:48

mstorsjo reviewed Nov 21, 2025

View reviewed changes

arichardson requested changes Nov 21, 2025

View reviewed changes

arrowd force-pushed the libunwind-freebsd-tests branch from 1f0d8ac to 7cbe4c0 Compare November 22, 2025 07:51

libunwind: Remove OS requirements from tests to make them run on more…

f992d45

… OSes While at it, replace "echo -ne" with "printf" for more compatibility. Co-authored-by: Alexander Richardson <[email protected]>

arrowd force-pushed the libunwind-freebsd-tests branch from 7cbe4c0 to f992d45 Compare November 22, 2025 15:05

arichardson approved these changes Nov 22, 2025

View reviewed changes

mstorsjo approved these changes Nov 28, 2025

View reviewed changes

arichardson merged commit 33bcde0 into llvm:main Dec 1, 2025
79 checks passed

libunwind: Remove OS requirements from tests to make them run on more OSes #167642

libunwind: Remove OS requirements from tests to make them run on more OSes #167642

Uh oh!

Conversation

arrowd commented Nov 12, 2025

Uh oh!

llvmbot commented Nov 12, 2025

Uh oh!

github-actions bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

arichardson Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

arichardson Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

mstorsjo Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arrowd commented Nov 18, 2025

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

rorth commented Nov 18, 2025

Uh oh!

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mstorsjo Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

arichardson Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arichardson commented Nov 21, 2025

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

arrowd commented Nov 28, 2025

Uh oh!

mstorsjo left a comment

Choose a reason for hiding this comment

Uh oh!

mstorsjo commented Nov 28, 2025

Uh oh!

arrowd commented Nov 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Nov 12, 2025 •

edited

Loading

mstorsjo Nov 22, 2025 •

edited

Loading