Skip to content

Commit

Permalink
Auto merge of #3254 - bossmc:flexible-array-members, r=JohnTitor
Browse files Browse the repository at this point in the history
Skip round-trip tests for structs with FAMs

For example:

```
strcut inotify_event {
  int      wd;       /* Watch descriptor */
  uint32_t mask;     /* Mask describing event */
  uint32_t cookie;   /* Unique cookie associating related
                        events (for rename(2)) */
  uint32_t len;      /* Size of name field */
  char     name[];   /* Optional null-terminated name */
};
```

the `name` field at the end of this struct is a Flexible Array Member (FAM - https://en.wikipedia.org/wiki/Flexible_array_member) and represents an inline array of _some_ length (in this case the `len` field gives the length, but FAMs in general are unsized).  These forms cause problems in two ways:

1. There's no rust-equivalent for FAM-containing structs (see rust-lang/rfcs#3396 for details) - the current approach is just to omit the `name` field in the Rust representation.
2. These structs cannot be passed by value (basically the compiler cannot know how many elements of the `name` field should be copied) and different compilers do different things when asked to do so.

This PR focusses on the second of these problems since it was causing test failures on my machine.  If you run the libc-test suite with GCC as your C compiler you get a compiler note saying the following:

```
/out/main.c: In function ‘__test_roundtrip_inotify_event’:
/out/main.c:21011:13: note: the ABI of passing struct with a flexible array member has changed in GCC 4.4
```

and the test suite passes.  OTOH if you build with clang as your compiler you get no compile time warnings/errors but the test suite fails:

```
size of struct inotify_event is 16 in C and 185207048 in Rust
error: test failed, to rerun pass `--test main`

Caused by:
  process didn't exit successfully: `/deps/main-e32ea4d2acb868af` (signal: 11, SIGSEGV: invalid memory reference)
```

(note that 185207048 is `0x08090A0B` (which is what the round-tripped value is initialized with by the test suite) so clearly the Rust and C code are disagreeing about the calling convention/register layout when passing such a struct).

Given that there doesn't seem to be a specification for passing these objects by value, this PR simply omits them from the roundtrip tests and the test suite now passes on GCC _and_ clang without warnings.
  • Loading branch information
bors committed May 26, 2023
2 parents 0d78fe1 + f8e82dc commit e2d90cd
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions libc-test/build.rs
Expand Up @@ -3333,15 +3333,15 @@ fn test_linux(target: &str) {
"Ioctl" if gnu => "unsigned long".to_string(),
"Ioctl" => "int".to_string(),

t if is_union => format!("union {}", t),

t if t.ends_with("_t") => t.to_string(),

// In MUSL `flock64` is a typedef to `flock`.
"flock64" if musl => format!("struct {}", ty),

// typedefs don't need any keywords
t if t.ends_with("_t") => t.to_string(),
// put `struct` in front of all structs:.
t if is_struct => format!("struct {}", t),
// put `union` in front of all unions:
t if is_union => format!("union {}", t),

t => t.to_string(),
}
Expand Down Expand Up @@ -3390,7 +3390,8 @@ fn test_linux(target: &str) {
// on Linux, this is a volatile int
"pthread_spinlock_t" => true,

// For internal use only, to define architecture specific ioctl constants with a libc specific type.
// For internal use only, to define architecture specific ioctl constants with a libc
// specific type.
"Ioctl" => true,

// FIXME: requires >= 5.4.1 kernel headers
Expand Down Expand Up @@ -3964,6 +3965,13 @@ fn test_linux(target: &str) {
true
}

// The `inotify_event` and `cmsghdr` types contain Flexible Array Member fields (the
// `name` and `data` fields respectively) which have unspecified calling convention.
// The roundtripping tests deliberately pass the structs by value to check "by value"
// layout consistency, but this would be UB for the these types.
"inotify_event" => true,
"cmsghdr" => true,

// FIXME: the call ABI of max_align_t is incorrect on these platforms:
"max_align_t" if i686 || mips64 || ppc64 => true,

Expand Down

0 comments on commit e2d90cd

Please sign in to comment.