ctest: add tests for field size, offset, and field ptr

[mbyx]

Description

Adds tests for checking if the size and offset of a field is the same in Rust and C, as well as the pointer to that field.

Sources

Checklist

• Relevant tests in libc-test/semver have been updated
• No placeholder or unstable values like *LAST or *MAX are
  included (see #3131)
• Tested locally (cd libc-test && cargo test --target mytarget);
  especially relevant for platforms that may not be checked in CI

[tgross35]

Nit, I think these should go after TestConst since they're used later in the templates. Same for the new field_* functions.

[tgross35]

Missing \n

[tgross35]

Can the args and return type be split to separate fields so this is directly visible rather than a comment? So

{{ item.volatile_keyword }} {{ item.c_ret_ty }} ctest_field_ptr__{{ item.id }}__{{ item.field.ident() }}({{ item.c_args }})

[tgross35]

Oh, I see the problem, function pointer syntax is horrid.

I'd suggest eliminating this with a typedef when you're dealing with function pointers:

typedef ret_ty (*some_name)(argty1, argty2);
some_name ctest_field_ptr__{{ item.id }}__{{ item.field.ident() }}(...) { /* ... */ }

some_name can be something like ctest_field_ty__{{ item.id }}__{{ item.field.ident() }}

[tgross35]

This block can be split a bit so we can see what unsafe is handling. Something like:

1. let field_ptr = &raw const ((*ty_ptr).{{ item.field.ident() }}) should get its own unsafe block that says it doesn't actually read ty_ptr (AIUI requiring unsafe for this op is a limitation of the unsafe check, can't cause UB)
2. Getting the size and offset from C should each get their own unsafe block assigned to a variable
3. The read_unaligned function should get its own block
4. The rest should be able to exist outside of unsafe

[tgross35]

Similar to the above, everything except let field_ptr = ... and let c_field_ptr = ctest_field_ptr__...(...) doesn't need to be in unsafe

[tgross35]

Shouldn't this be allow rather than expect? Because when there are field tests, it's not actually unused.

[tgross35]

With these two small changes, the size+offset changes look good to me. I do have some more feedback for the field pointer tests though; maybe want to split that off to another PR so the rest of this can merge now?

[tgross35]

Since structs and unions use the same field types, I think this could be collapsed to a single iterator. Something like:

let should_skip = |map_input| helper
    .generator
    .skips
    .iter()
    .any(|f| f(map_input));

let struct_fields = helper.ffi_items.structs().iter()
    .flat_map(|st| st.fields().map(|field| (st, field))
    .filter(|(st, field)| should_skip(&MapInput::StructField(st, field))
    .map(|(st, field)| field);
let union_fields = helper.ffi_items.unions().iter()
    .flat_map(|un| un.fields().map(|field| (un, field))
    .filter(|(un, field)| should_skip(&MapInput::UnionField(un, field))
    .map(|(un, field)| field);

for field in struct_fields.chain(union_fields) {
    // ...
}

Something similar for populate_field_ptr_tests

[tgross35]

Safety comments

Suggested change

	let ty_ptr = unsafe { &raw const (*uninit_ty).{{ item.field.ident() }} };
	let val = unsafe { ty_ptr.read_unaligned() };
	let ctest_field_offset = unsafe { ctest_offset_of__{{ item.id }}__{{ item.field.ident() }}() };
	check_same(offset_of!({{ item.id }}, {{ item.field.ident() }}) as u64, ctest_field_offset,
	"field offset {{ item.field.ident() }} of {{ item.id }}");
	let ctest_field_size = unsafe { ctest_size_of__{{ item.id }}__{{ item.field.ident() }}() };
	// SAFETY: we assume the field access doesn't wrap
	let ty_ptr = unsafe { &raw const (*uninit_ty).{{ item.field.ident() }} };
	// SAFETY: we assume that all zeros is a valid bitpattern for `ty_ptr`, otherwise the
	// test should be skipped.
	let val = unsafe { ty_ptr.read_unaligned() };
	// SAFETY: FFI call with no preconditions
	let ctest_field_offset = unsafe { ctest_offset_of__{{ item.id }}__{{ item.field.ident() }}() };
	check_same(offset_of!({{ item.id }}, {{ item.field.ident() }}) as u64, ctest_field_offset,
	"field offset {{ item.field.ident() }} of {{ item.id }}");
	// SAFETY: FFI call with no preconditions
	let ctest_field_size = unsafe { ctest_size_of__{{ item.id }}__{{ item.field.ident() }}() };

[mbyx]

It's fine to review the field ptr checks now, splitting it would be harder.