libc: port freebsd to use ctest-next #4648
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
All checks passed... except that the freebsd checks didn't even run. @tgross35 is there a way to only run those checks and not the others just to make sure there isn't any minor bug? |
|
I have no idea, that CI can act different since it's not GHA. There are some issues with FreeBSD15 #4645 but not the others. Let's try a restart |
tgross35
reviewed
Aug 13, 2025
libc-test/build.rs
Outdated
Comment on lines
2446
to
2449
| | "devstat_priority" => ty.to_string(), | ||
| | "devstat_priority" => ty.to_string().into(), | ||
|
|
||
| // FIXME(freebsd): https://github.com/rust-lang/libc/issues/1273 | ||
| "sighandler_t" => "sig_t".to_string(), | ||
| "sighandler_t" => "sig_t".to_string().into(), |
There was a problem hiding this comment.
Wrap in Some rather than using .into() for clarity
Comment on lines
-2867
to
+2872
| cfg.volatile_item(|i| { | ||
| use ctest::VolatileItemKind::*; | ||
| match i { | ||
| // aio_buf is a volatile void* but since we cannot express that in | ||
| // Rust types, we have to explicitly tell the checker about it here: | ||
| StructField(ref n, ref f) if n == "aiocb" && f == "aio_buf" => true, | ||
| _ => false, | ||
| } | ||
| }); | ||
| // aio_buf is a volatile void* but since we cannot express that in | ||
| // Rust types, we have to explicitly tell the checker about it here: | ||
| cfg.volatile_struct_field(|s, f| s.ident() == "aiocb" && f.ident() == "aio_buf"); |
There was a problem hiding this comment.
Small nit: it's not exactly canonical Rust, but keeping it match does make it easier to add/remove items. No strong need to update this one, though.
(Sorry if I said this in another PR already, can't remember if I did)
mbyx
force-pushed
the
libc-test-freebsd-port
branch
4 times, most recently
from
4d6c7b8 to
d56110e
Compare
|
I'll try to keep fixing it, but it does seem that |
mbyx
force-pushed
the
libc-test-freebsd-port
branch
from
d56110e to
a1006fd
Compare
|
Almost there. It seems only Digging around, https://cgit.freebsd.org/src/tree/sys/i386/include/reg.h?h=releng/13.1 just includes https://cgit.freebsd.org/src/tree/sys/x86/include/reg.h?h=releng/13.1, and here |
tgross35
approved these changes
Aug 13, 2025
|
It feels strange to finally see a successful freebsd run. Could the difference in naming be a bug that slipped by when freebsd mostly failed? It doesn't look like |
|
I don't really know; there is some investigation in #4641
You know what's funny: it's been broken for months. But I was looking at the CI log just now, and apparently 3d93bf5 from two days ago made ctest stop complaining that it can't understand macros, so FreeBSD passes (except for broken 13). So it actually decided to start working slightly before this PR 🙃 I don't at all trust that the problems wouldn't come back though so I'm still thrilled to have this fix |
|
FWIW, this breaks libc-test on freebsd15/ppc64 (EDIT: tested on big-endian, not little-endian), which was previously working. |
|
Any chance you would be able to run the file in GDB to get a backtrace? |
(analysis based on version 3f2a3ab) There is a coredump, but GDB's jemalloc support code is very unhappy about itIf I load the coredump without the program, GDB can at least show a raw backtraceFunnily enough, running ... which I can then decode with addr2lineRelevant code excerptsmcontext_t ctest_roundtrip__mcontext_t(
mcontext_t value,
const uint8_t is_padding_byte[sizeof(mcontext_t)],
uint8_t value_bytes[sizeof(mcontext_t)]
) {
int size = (int)sizeof(mcontext_t);
// Mark `p` as volatile so that the C compiler does not optimize away the pattern we create.
// Otherwise the Rust side would not be able to see it.
volatile uint8_t* p = (volatile uint8_t*)&value;
int i = 0;
for (i = 0; i < size; ++i) {
// We skip padding bytes in both Rust and C because writing to it is undefined.
// Instead we just make sure the the placement of the padding bytes remains the same.
if (is_padding_byte[i]) { continue; } // <--- 57475 --------------
value_bytes[i] = p[i];
// After we check that the pattern remained unchanged from Rust to C, we invert the pattern
// and send it back to Rust to make sure that it remains unchanged from C to Rust.
uint8_t d = (uint8_t)(255) - (uint8_t)(i % 256);
d = d == 0 ? 42: d;
p[i] = d;
}
return value;
} pub fn ctest_roundtrip_mcontext_t() {
type U = mcontext_t;
extern "C" {
fn ctest_size_of__mcontext_t() -> u64;
fn ctest_roundtrip__mcontext_t(
input: MaybeUninit<U>, is_padding_byte: *const bool, value_bytes: *mut u8
) -> U;
}
const SIZE: usize = size_of::<U>();
let is_padding_byte = roundtrip_padding__mcontext_t();
let mut expected = vec![0u8; SIZE];
let mut input = MaybeUninit::<U>::zeroed();
let input_ptr = input.as_mut_ptr().cast::<u8>();
// Fill the uninitialized memory with a deterministic pattern.
// From Rust to C: every byte will be labelled from 1 to 255, with 0 turning into 42.
// From C to Rust: every byte will be inverted from before (254 -> 1), but 0 is still 42.
for i in 0..SIZE {
let c: u8 = (i % 256) as u8;
let c = if c == 0 { 42 } else { c };
let d: u8 = 255_u8 - (i % 256) as u8;
let d = if d == 0 { 42 } else { d };
unsafe {
input_ptr.add(i).write_volatile(c);
expected[i] = d;
}
}
let c_size = unsafe { ctest_size_of__mcontext_t() } as usize;
if SIZE != c_size {
FAILED.store(true, Ordering::Relaxed);
eprintln!(
"size of mcontext_t is {c_size} in C and {SIZE} in Rust\n",
);
return;
}
let mut c_value_bytes = vec![0; size_of::<mcontext_t>()];
let r: U = unsafe {
ctest_roundtrip__mcontext_t(input, is_padding_byte.as_ptr(), c_value_bytes.as_mut_ptr()) // <-- 180279
};
// Check that the value bytes as read from C match the byte we sent from Rust.
for (i, is_padding_byte) in is_padding_byte.iter().enumerate() {
if *is_padding_byte { continue; }
let rust = unsafe { *input_ptr.add(i) };
let c = c_value_bytes[i];
if rust != c {
eprintln!("rust[{}] = {} != {} (C): Rust \"mcontext_t\" -> C", i, rust, c);
FAILED.store(true, Ordering::Relaxed);
[...]Assembly code and register state at the crash siteASCII letters!? Ah, just a counting pattern, but it shouldn't be in the pointer. breaking at entry of `ctest_roundtrip__mcontext_t`So, this bogus pointer value is indeed present at function entry. A short patch later, I can see that this was not the intention on the Rust side: $ git diff
diff --git a/ctest/templates/test.rs b/ctest/templates/test.rs
index 4e33fb07c..6055d3124 100644
--- a/ctest/templates/test.rs
+++ b/ctest/templates/test.rs
@@ -285,6 +285,9 @@ mod generated_tests {
}
let mut c_value_bytes = vec![0; size_of::<{{ item.id }}>()];
+ println!("{{ item.id }} size: {SIZE}");
+ println!("{{ item.id }} is_padding_byte: {:08x}", is_padding_byte.as_ptr() as usize);
+ println!("{{ item.id }} c_value_bytes: {:08x}", c_value_bytes.as_ptr() as usize);
let r: U = unsafe {
ctest_roundtrip__{{ item.id }}(input, is_padding_byte.as_ptr(), c_value_bytes.as_mut_ptr())
};Assembly code of the caller, `generated_tests::ctest_roundtrip_mcontext_t`I suspect an ABI difference between Rust and C (compiled with FreeBSD clang version 19.1.7) in how structs are passed as parameters, but I'm not sure. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Sources
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI