__builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails (need "-fstrict-flex-array")

[kees]

When bos1 is used on a member who is both an array and at the end of a structure, it fails to correctly resolve. This kind of behavior should only happen for flexible array members:

struct middle_array {
    int a;
    unsigned char c[16];
    int b;
};

struct trailing_array {
    int a;
    int b;
    unsigned char c[16];
};

struct flex_array {
    int a;
    int b;
    unsigned char c[];
};

Both "middle" and "trailing" should see that "c" is 16 bytes. Only "flex" should be "unbounded":

ok:  sizeof(*middle) == 24
ok:  sizeof(middle->c) == 16
ok:  __builtin_object_size(middle, 1) == -1
ok:  __builtin_object_size(middle->c, 1) == 16
ok:  sizeof(*flex) == 8
ok:  __builtin_object_size(flex, 1) == -1
ok:  __builtin_object_size(flex->c, 1) == -1
ok:  sizeof(*trailing) == 24
ok:  sizeof(trailing->c) == 16
ok:  __builtin_object_size(trailing, 1) == -1
WAT: __builtin_object_size(trailing->c, 1) == -1 (expected 16)

https://godbolt.org/z/s9nb4Y7q4

This is likely due to trailing all trailing arrays as historical flexible arrays, but it breaks FORTIFY_SOURCE in that any struct with a fixed size trailing array will receive no sanity checking. Please introduce something like "-fstrict-flex-array".

[kees]

cc @nickdesaulniers @gwelymernans

[kees]

Quoting from bug #55742 :

I think @efriedma-quic covers the reasons behind this well in general.

RE flexible arrays, yeah, clang's frontend gives up in many of those cases. The general pattern of code that it tries to allow for is:

struct string {
  size_t len;
  char data[0];
};

struct string *new_string(const char *s) {
  size_t len = strlen(s);
  struct string *str = malloc(sizeof(*str) + len);
  // <insert *str initialization code here>
  return str;
}

Which is relatively common in C. Clang originally had checks for whether the trailing member of the struct was unsized or had a length <= 1, but that broke in the face of e.g., FreeBSD's struct sockaddr, as @serge-sans-paille notes.

Right, the behavior makes some historical sense, but for code bases that have converted all the ancient "fake" flex-arrays to proper flex arrays, there needs to be a way to gain FORTIFY-style protections over structs with fixed-size trailing arrays. This is currently impossible. Adding -fstrict-flex-array to drop the old work-arounds is needed.

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[kees]

cc @gburgessiv @serge-sans-paille

[tstellar]

@msebor

[serge-sans-paille]

+1 for -fstrict-flex-array, I'll propose a patch

[serge-sans-paille]

Patch proposed at https://reviews.llvm.org/D126864, @kees @gburgessiv I've added you as reviewers.

[llvmbot]

@llvm/issue-subscribers-clang-driver