RFC: Enforcing Bounds Safety in C (-fbounds-safety)

Clang Frontend
62

Allow me to clarify further. The example aims to discuss type attributes added to nested pointers (or pointer to pointers) within the same declaration. Thus, the remaining question for __sized_by(Buf, Len) is to determine to which level of the nested pointer in the same declaration (Buf ) the attribute pertains. For example, void **buf has a nested pointer type. Its inner-most pointer type (void * ) and the outer-most pointer type (void ** ) may require separate type attributes. If the type attributes in this case are _Nullable and _Nonnull , they are represented as void *_Nullable *_Nonnull today.

Also, it might be worth reiterating that, according to WG14 N2335, C2x supports C++ -style attributes, such as [[attr]] , where type attributes must be positioned immediately after the type, as demonstrated in void *[[attr1]] *[[attr2]] buf. Type attributes of -fbounds-safety (such as __sized_by , __indexable , __single , etc.) aim to adhere to this standard syntactic rule.

2 Likes
63

Good point about nested pointers, though in my experience having different qualifiers for the internal and external types on a multi-dimensional pointer is merely an academic thing that doesn’t exist in practice, but my experience is limited.

As for your attribute comment, I asked Aaron Ballman about that for my _Overload idea, and the problem with attributes is they need to be completely optional information for a compiler, which wouldn’t hold for operator overloading, and if I’m reading this proposal correctly it wouldn’t work for this either.

64

Thank you for this thorough and well-considered RFC!

In general, I am strongly in favor of the overall feature. However, given the amount of discussion on the RFC, I’d ask that you update the RFC document with any changes you intend to make to the design based on feedback so that it’s clear what the final shape of the proposal is in. (Ultimately, the interesting bits will likely wind up in public documentation anyway, so this hopefully won’t be busywork.)

I tend to share the concerns about requiring late parsing for code like void foo(int * __counted_by(N), int N); as this can be fragile for reasons pointed out by others, but also because late parsing can be somewhat expensive. However, I’m not convinced that the GCC extension allowing you to forward declare a parameter is a good alternative. While we do have a feature request for adding this (GNU extension: Support forward declaring function parameters · Issue #47617 · llvm/llvm-project · GitHub), this extension was explicitly rejected by WG14 in recent history (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2780.pdf) and so it doesn’t meet our usual requirements for adding as an extension to Clang (Clang - Get Involved) without some further effort.

Note, WG14 is in the process of going through another round of national body comments in about two weeks. An updated paper on forward declaring parameters has been published but has not yet been considered by the committee, https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3121.pdf, and there is a national body comment asking to consider this for C2x still. There’s a separate national body comment asking WG14 to consider late parsing parameter names similar to what you’re proposing here. We should not make any final decisions on this aspect of the design until we’ve heard what WG14 has to say as that may provide us with some insights.

To solve this sort of design issue, we have historically supported trailing attributes that can refer to positional arguments, which typically look like: void foo(int *ptr, int n) __attribute__((counted_by(1, 2))); (positional arguments are one-based). This has ergonomic issues though, especially in C++ where it’s not always clear whether this in a member function counts as an argument or not, so it would be reasonable to consider other alternatives if a WG14 position is inconclusive.

One question I had was whether the expression passed to the annotation is required to be a simple identifier or whether it can be a more complex expression. e.g., can you do things like:

// Counted by one or the other
void func(int n, int m, bool b, int * __counted_by(b ? n : m) ptr);

struct S {
  int n;
  // Counted by a member variable
  void func(int *__counted_by(n) ptr);
};

struct Base {
  int n;
};
struct Derived : Base {
  // Counted by fully-qualified member variable
  void func(int * __counted_by(Base::n) ptr);
};

// Counted by an array element
void func(int n, int array[static 2], int * __counted_by(array[n]) ptr);

Another question I had is what the redeclaration behavior will be. Do the annotations have to show up on the first declaration, all declarations, etc? And what happens if the annotations differ on a redeclaration? e.g.,

void foo(int n, int *ptr);
void foo(int n, int * __counted_by(n)) {} // Is this OK?

void bar(int n, int * __counted_by(n) ptr);
void bar(int n, int *ptr) {} // Is this OK?

// In File1.c
void baz(int n, int *ptr) {}

// In File2.c
extern void baz(int n, int * __counted_by(n) ptr); // ?

void quux(int n, int m, int * __counted_by(n) ptr);
void quux(int n, int m, int * __counted_by(m) ptr); // Do we reject this?

void barf(int n, int m, int * __counted_by(n) ptr1, int *ptr2);
void barf(int n, int m, int *ptr1, int * __counted_by(m) ptr2); // Do we merge this into ptr1 being counted by n and ptr2 being counted by m?
3 Likes
65

Absolutely! We will update the document to incorporate the details discussed in the comments.

Thanks for providing the context! We’ll be waiting and keeping an eye on WG14’s conclusion on the related proposals.

Right. Another ergonomic issue with using trailing attributes relates to how to express attributes on nested pointers, e.g., void foo(int *__counted_by(n) *out_buf, int n);. I can imagine taking the nested level as an additional argument for the attribute, as in void foo(int **out_buf, int n) __attribute__((counted_by(1, 2, 1)));. However, this approach could lead to confusion about which argument corresponds to what and which number represents which nested level.

Also, the count expression can be an arithmetic expression without side effects, e.g., __counted_by(10 * tens + ones), which is another reason why trailing attributes may not be suitable.

Another issue is that supporting structs like the one below also requires late parsing, and we were not aware of good alternatives for this.

struct foo {
  int *__counted_by(n) buf;
  int n;
};

So, in case WG14 does not arrive at a conclusion this time, I’m wondering if you’d be open to us experimentally implementing late parsing for attributes on parameters and struct fields, under our experimental flag. This would allow us to collect data and provide a basis to propose to WG14 eventually. Also, my team is committed to modifying the implementation to align with the C standard as it evolves, and supporting the long-term maintenance of this feature.

1 Like
66

As we only briefly touched on this here, the expression can be a simple reference to a declaration, a constant (including calls to constant functions), or an arithmetic expression that does not have side effects. Therefore, the following examples will work:

// Single arithmetic expression without side effect
void foo(int tens, int ones, int *__counted_by(10 * tens + ones) ptr);

// Call to a const function
void const_foo(void) __attribute__((const));
int *__counted_by(const_foo()) var;

However, the current implementation doesn’t allow ternary operators, and we were planning to extend our specification to support ternary operators.

I’ve slightly adjusted the examples for C since the current proposal doesn’t support C++. An expression passed to a bounds annotation can refer to other members of the same struct, so the following example is allowed.

struct S {
  int n;
  // Allowed: Counted by a member variable
  int *__counted_by(n) ptr;
};

However, referring to a member of another struct, including nested structs (or the base struct) is not allowed. This is because of complications to maintain the soundness of the count value in a different struct. In the following example, bp , which is a pointer to Base , is used to update n . This could potentially break the correctness of the bounds information if the dynamic type of bp is Derived (or aliased with a pointer to Derived ) but Sema won’t be able to identify this.

struct Base {
  int n;
};
struct Derived {
  struct Base header;
  // Not allowed: Counted by a member of other struct member
  int * __counted_by(header.n) ptr;
};

void foo(struct Base *bp, struct Derived *dp) {
    // Potentially unsafe and compiler doesn't know if `bp` and `dp` alias 
    bp->n++;
}

Currently, the extension does not support the patten of “counted by an array element” to support the example below, but we are open to hearing about use cases and suggestions to extend the model.

// Not allowed: Counted by an array element
void func(int n, int array[static 2], int * __counted_by(array[n]) ptr);

-fbounds-safety rejects any mismatched annotations for function redeclarations. As a result, the compiler reports errors for most of the examples provided below.

void foo(int n, int *ptr);
void foo(int n, int * __counted_by(n)) {} // error due to a bounds annotation mismatch

void bar(int n, int * __counted_by(n) ptr);
void bar(int n, int *ptr) {} // error due to a bounds annotation mismatch

// In File1.c
void baz(int n, int *ptr) {}

// In File2.c
void baz(int n, int * __counted_by(n) ptr); // missing an error as the compiler cannot see the other decl in File1.c 

void quux(int n, int m, int * __counted_by(n) ptr);
void quux(int n, int m, int * __counted_by(m) ptr); // error due to a bounds annotation mismatch

void barf(int n, int m, int * __counted_by(n) ptr1, int *ptr2);
void barf(int n, int m, int *ptr1, int * __counted_by(m) ptr2); // error due to bounds annotation mismatches
67

I’ll report back with what I hear from the committee.

Also a good point! (I really don’t like attributes which refer to positional arguments for so many reasons, so finding a way to associate this information with the parameter more directly is my preference as well.)

Structures in C don’t have the notion of a “this” pointer, so members cannot refer to other members even when the order would otherwise allow for it without any late parsing: Compiler Explorer This has caused us problems in the past: Thread Safety Analysis "guarded_by" attribute doesn't work for struct fields in C code · Issue #20777 · llvm/llvm-project · GitHub and might be more involved than it seems at first (it certainly forges a new path forward for C).

We’ll know whether the committee has strong feelings by Jun 28 (the meetings end on the 27th and we effectively have to process all the national body comments), so we should have at least an idea of the committee sentiment reasonably soon.

I would be okay with experimenting in this space under a feature flag once we know what direction WG14 seems most interested in. I would be especially curious to see what problems arise from the approach, as declarations can be arbitrarily complex. (e.g., how often does existing code run into problems where the identifier is already declared as a global name and is later reused as a parameter name, how often do we run into things like void func(int array[n], void (*fp)(int n), struct S { int n; } s);, what should happen with something like: void func(int array[({n;})], int n);, etc).

I would like to keep the two extensions under separate flags as they are orthogonal features (even if related). The late parsing feature means we could support thread safety annotations in C with less awkward syntax and we could also support some more complex declarations involving multi-dimensional arrays that are bounded by later parameters, etc. So it stands on its own as an extension outside of bounds safety (which could also make use of it).

Ah, good to know, thank you! By “simple reference to a declaration”, will that involve member accesses? e.g.,

struct S { int n; };
void func(struct S s, int * __counted_by(s.n) array);

or just id expressions?

I didn’t have a particular use case in mind, mostly just trying to understand the shape of the feature.

Excellent! I think it’s fine for us to document that it’s UB if the signatures do not match across TU boundaries as in the baz example.

2 Likes
68

Right, that is a great insight. In the implementation, we’ve added a name lookup for struct members referred to by attributes within the struct’s scope. The name lookup is performed after parsing the struct body, ensuring the field declarations of the same struct are always seen first (not a global declaration of the same name). Another challenge was creating count expressions from struct fields (e.g., __counted_by(n+1) in the example below) without instantiation and without the notion of this in C. This means that the expressions won’t have a base required to create MemberExpr . Therefore, we had to create a DeclRef directly referring to the struct field to represent the expression passed to the bounds annotations, which isn’t something allowed in C. I envision this as an extended behavior limited to expressions, passed to attributes only. It should be generally beneficial for other attributes that require more expressiveness from C, such as the guarded_by attribute you mentioned.

struct S {
  int n;
  // Allowed: Counted by a member variable
  int *__counted_by(n+1) ptr;
};

Thank you! We will wait until we will have better understanding on the direction.

Yes, this makes a lot of sense. We will keep the late parsing logic in a separate flag.

This will only involve id expressions. Member accesses will not be allowed.

69

Overall I’m very excited. I look forward to deploying these mitigations in the Linux kernel and helping review the patches to LLVM when they are available; this RFC makes it crystal clear to me how we might adopt such an approach in yet another large commercial C codebas. As such, I have many questions and comments in good faith. Thank you to the work you and your team did to trailblaze here.

  • the examples show the use of the attribute on function parameters. Can it be used on struct definitions? Looks like yes, but it’s not mentioned until much later in the RFC. Official docs should have examples up front. In particular, for incremental adoption, I think that it will be more common perhaps to annotate struct definitions than function parameters? What has been your experience in your production codebase? More parameter annotations or struct definition annotations?
  • “The annotation cannot apply to pointers to incomplete types or types without size such as void *.” It’s common for function interfaces in C to accept void* and do casts locally to a more specific pointed-to type. Is there any hope for that pattern of code to use __counted_by/__sized_by?
  • __ended_by documentation should perhaps use wording specific to “the end of a valid object” and provide some information about what could happen if used incorrectly. (UB?)
  • “To avoid this, the compiler produces either a compile-time error or run-time trap.” well, which is it?
  • If there are multiple null checks inserted (perhaps by __counted_by_or_null or the other _or_null) variants, do they get coalesced? If so, how does that interact with -fno-delete-null-pointer-checks?
  • “__bidi_indexable is the default annotation for non-ABI visible pointers, such as local pointer variables” what about an array of local pointers? Can we expect that entire array to triple in size?
  • “using the intrinsic function __unsafe_terminated_by_to_indexable” why are these not _builtin prefixed? Same for __unsafe_forge_bidi_indexable and __unsafe_forge_single.
  • Is the single parameter for __terminated_by the terminal value? If so, then is __null_terminated just __terminated_by(NULL)? If so, seems redundant. Or is the T parameter a count?
  • Is -fbounds-safety necessary for the non-ABI changes? It seems like __counted_by/__sized_by could be used without that command line flag? Could -fbounds-safety just opt into the ABI changing behaviors? That might allow for finer grain incremental adoption. Or use in C++.
  • __ptrcheck_abi_assume_unsafe_indexable is mentioned once, and I’m unsure of what it does.
  • “All const char pointers are __null_terminated by default.” What annotation should be used when that’s NOT the case? Later replies make it sound like __counted_by might be used for this, but it’s unclear to me how that might look in practice. Or __unsafe_indexable perhaps? I guess we could wrap that in a preprocessor macro __nonnull_terminated or __maybe_non_null_terminated.
  • What is the lower bounds used for? Tracking __bidi_indexable?
  • Can __counted_by/__sized_by be posted separate/first? I’m curious how much of the rest of the machinery presented in this RFC do they depend on?
  • Happy to have seen some consideration for toolchain portability, as that is critical for the Linux kernel. I’m disappointed though that this uses __has_feature rather than __has_attribute or __has_builtin. I’m also slightly disappointed these attributes don’t use GNU C style attributes, especially since __counted_by/__sized_by have no bearing on ABI. This isn’t a sticking point though for me personally.
  • Can users play with these attributes today in a published/publicly accessible version of Apple’s distribution of Clang?
  • I don’t particularly understand the section on sugar types, but maybe that’s referring to an implementation detail in Clang that I’m simply unfamiliar with. There’s a comment though “__counted_by and friends not participate in function overloading” which makes me curious if perhaps we’ll need to update the Linux kernel’s implementation of FORTIFY_SOURCE for these. Hopefully not; i.e. we only need to provide one definition for memcpy and string.h friends even though we use __attribute__((overload)) on these IIRC (@kees knows more).
  • The late parsing seems cool; that way the member with the count doesn’t need to come first which would otherwise inhibit incremental adoption. This is a deficiency in the guarded_by attributes. I’m happy to see some WG14 folks trying to figure out the best way to refer to something before you’ve seen it; the trailing attribute syntax (that’s not used here, but some folks have advocated for in this thread) is IMO developer hostile.
  • For optimizations, if I access an array multiple times (let’s say I manually unrolled a loop), can I expect LLVM to reduce the number of bounds checks?
  • It might be nice to have a command line flag that accepts a symbol name for a function to be called in place of __builtin_trap().
  • __builtin_dynamic_object_size() should take into account __counted_by/__sized_by annotations.
  • The RFC lacks discussion about flexible array members and whether access to the can be protected by __counted_by/__sized_by. The Linux kernel has many many flexible array members whose accesses have lead to many bugs in the past. Flexible array members are discussed in the comments, but IMO the RFC MUST be amended to discuss them.
  • Has Apple been able to use these attributes in their libc? I’m particularly curious about FORTIFY_SOURCE like implementations for string.h.
  • The syntax for __counted_by in the flexible array member square brackets is surprising. I would have expected the attribute to be outside the square brackets. (Bikeshedding) Was it really easier to parse attributes there in clang?
  • Let’s say for an initial patch set, I’d like to use __counted_by. Please consider how you’d factor your patches to get that feature working as a minimum.
  • What is the ETA to see any such code accompanying this RFC?
1 Like
70

GCC is adding __has_feature at long last, FWIW. Not that that helps portability to MSVC, but you probably don’t care about that.

1 Like
71

Let me summarize my arguments here why I think late parsing would be a mistake.

  • In C, everything so far has forward declarations. Breaking this rule will make language less coherent and will also be an argument against standardization of this feature later.
  • Late parsing adds substantial complexity to compilers. Clang might be fine with this, but adding this to other compilers has some cost.
  • Late parsing makes code parsing slower. In C you can now parse, type check and do constant fold in one simple forward parse. This property would be lost.
  • Late parsing makes code reading harder to a human, for exactly the same reason: You would then need to look ahead to understand things. The syntax might look simpler, but will be less convenient to read and more difficult to understand. Reading is more important than saving key strokes.
  • Late parsing is problematic for other tools that need to understand partial code, such as an IDE. How do you deal with code which is edited at some point? With forward declarations that never affects code before this point. If this code is correct, all analysis can be done up to this point. With late parsing, this might change semantics also for earlier code, this needs special stuff.
  • You need to find rules to avoid cyclic dependencies. This may not be a problem for attributes, but once something like this is properly integrated into a type system, this will be a problem. One either has to add arbitrary or annoying restrictions or add another level of complexity with cycles.

In summary, I think it would be a language design mistake to do it this way. In contrast, the GCC extension is rather simple, fits well with the rules of the C language, is very flexible and powerful without introducing semantic problems with cycles. Just implement this. It may look ugly at first, but technically it is far better in so many ways.

1 Like
72

Thanks @nickdesaulniers for your valuable feedback!

Yes, all the annotations we discussed in the context of function parameters are also available in structures. Typically, even incremental adoption tends to involve more annotations for function parameters because functions vastly outnumber structures in most programs. That said, it is generally required to annotate both.

That only refers to __counted_by. One of the purposes of __sized_by is to serve that role for void * . Additionally, void *__single casts to any other __single pointer.

There is a bounds check (with a trap on the failure path) at all locations assigning a new value to a __counted_by, __sized_by or __ended_by pair. Given an example int foo[10] , the start (Start ) of an __ended_by pair can be any value inclusively between &foo[0] and &foo[10] (one past the end), and the end can be any value inclusively between Start and &foo[10] (also one past the end). Of course, if both are the same (such as {&foo[10], &foo[10]} ), no elements can be dereferenced.

-fbounds-safety essentially inserts run-time checks and traps to maintain the invariant, while we strive to include as many compile-time diagnostics as possible to identify potential run-time traps early on. So whether a run-time trap will be reported at compile time depends on the codebase and the implementation. If the compiler knows if an operation will always lead to a trap at run time, it reports an error. An example of this is the assignment of a __counted_by pointer with a __single pointer, and a count variable with a constant value greater than 1. The compiler can report an error in this case because it knows this will result in a run-time trap. Moreover, as we become aware of certain patterns likely to cause a run-time trap, we incrementally incorporate compiler warnings.

Generally, yes, but it really depends on whether the optimizer can identify these checks to be redundant since -fbounds-safety relies on the LLVM optimizer to optimize redundant checks. For instance, in the example below, the compiler may or may not remove the redundant null check, depending, for example, on whether it’s beneficial to keep the result of the null check in the register while performing other operations (assuming these don’t update the local variable p). The same applies to null checks inserted by -fbounds-safety.

void foo(int *p) {
    if (!p) {
        // do something
    } 
    // ...
    // some other operations
    // ...
    if (!p) {
        // do something
    }  
}

-fno-delete-null-pointer-checks doesn’t interfere with the optimizer removing redundant checks. This flag is all about treating null pointer dereferences as non-UB. As such, it prevents the compiler from removing null checks solely because they occur after the pointer dereference. However, the compiler can still eliminate redundant checks.

Another possible point of interaction could be the behavior related to the nonnull attribute because -fno-delete-null-pointer-checks strips nonnull from function parameters. However, this doesn’t interfere with -fbounds-safety because whether -fno-delete-null-pointer-checks is enabled or not, -fbounds-safety reports an error if a pointer has both __counted_by_or_null and the nonnull attribute. This is consistent with the compiler implementation that maintains nonnull -related warnings even when -fno-delete-null-pointer-checks is enabled.

This only applies to “outermost” pointers of local variables. All nested pointers and array element types are considered as ABI-visible so they are __single by default. The programmers need to explicitly add __bidi_indexable or __indexable in order to make them wide pointers.

Could you clarify what you mean by “the non-ABI changes” and using __counted_by/__sized_by without -fbounds-safety ? Are you referring to merely integrating some bounds checks when possible (i.e., emitting checks on a pointer dereference if it has __counted_by)? If that’s the case, while that would be out of scope of our work, I can imagine such a task could be accomplished by extending sanitizers, relevant builtins such as __builtin_dynamic_object_size , and/or compiler warnings to utilize the attributes once we provide the parsing logic.

But if you mean more comprehensive bounds safety support, it becomes quite complicated because -fbounds-safety is about the language model and the type system to ensure that all pointers always have the correct bounds information and all pointers are secured by default. For this to function properly, the “ABI changing behaviors” - the aspect that transforms local variables into implicitly wide pointers, is critical for propagating the necessary bounds information without the need for manual bounds annotations.

Yes. In case the pointer is involved in pointer arithmetic, it needs a separate lower bound to perform the necessary bounds check, as shown in the example below:

int foo(int *__counted_by(size) buf, int size, int offs) {
    int *local = buf; // implicitly bidi_indexable
    local += offs;
    return *local; // checks both the lower and upper bounds
}

Thanks for bringing this up! We’d love to learn more about the ways to improve the portability.

Thanks @jrtc27! Sounds like using __has_feature wouldn’t cause the portability issue with GCC anymore?

We are planning to release the preview in Apple’s fork of llvm-project in near future.

Oh interesting. Is there a reason why your memcpy and string.h friends have the overloadable attributes?

To clarify the phrase “not participating in function overloading”, I meant that functions cannot be overloaded based solely on differences in __counted_by and similar attributes, just as they can’t be overloaded based on any other type attributes. So you still only need to provide a single definition for these functions.

Yes, this should work with -ftrap-function= . We have been using it to log the traps and continue the execution. Do you have any other use cases in mind?

Right. We only briefly mention __counted_by can be used to annotate incomplete arrays and the arrays decay into a pointer with the size. We will add a detailed programming model for flexible array members.

__counted_by is conceptually a type qualifier, so we’ve decided to place it inside array brackets, similar to other type qualifiers such as restrict and static which can also be put inside array brackets. There is already a pathway in Clang to parse qualifiers (and attributes) inside array brackets so it didn’t require us too much additional work to allow the parsing of __counted_by .

We don’t have a concrete ETA yet but the plan is to start working on the initial patch after we hear about WG14’s position on proposals that may affect the late-parsing related behavior (@AaronBallman mentioned it’s expected in next week). And the initial patch is going to include the implementation of the parsing logic for __counted_by under another experimental flag , without -fbounds-safety , for attribute-only use cases. This is so that other relevant features in Clang can begin experimenting with the attributes. I envision that @Kees’s work on using a count attribute with flexible array members for __builtin_get_dynamic_object_size() could be one of the first clients. (I thought I saw @Kees’s RFC in the discourse but I couldn’t find it anymore).

1 Like
73

“using the intrinsic function __unsafe_terminated_by_to_indexable” why are these not _builtin prefixed? Same for __unsafe_forge_bidi_indexable and __unsafe_forge_single.

The actual intrinsics are named __builtin_unsafe_terminated_by_to_indexable (and so on), but we ship a header with macros that rename them to make them a bit more convenient, like so:

#define __unsafe_terminated_by_to_indexable(P) __builtin_unsafe_terminated_by_to_indexable(P)

Is the single parameter for __terminated_by the terminal value? If so, then is __null_terminated just __terminated_by(NULL)? If so, seems redundant. Or is the T parameter a count?

Indeed, it’s just macro-defined syntax sugar:

#define __null_terminated __terminated_by(0)

They are completely interchangeable, so you’re free to use one or the other. In our experience it’s very rare to use a terminator other than NULL so having a dedicated alias for that seems reasonable.

__ptrcheck_abi_assume_unsafe_indexable is mentioned once, and I’m unsure of what it does.

“For instance, __ptrcheck_abi_assume_unsafe_indexable() will make all ABI-visible pointers be __unsafe_indexable. ” means that unannotated function parameters (and the inner type for nested pointers) are __unsafe_indexable instead of __single. It’s syntactic sugar for a file-scope pragma.

“All const char pointers are __null_terminated by default.” What annotation should be used when that’s NOT the case? Later replies make it sound like __counted_by might be used for this, but it’s unclear to me how that might look in practice. Or __unsafe_indexable perhaps? I guess we could wrap that in a preprocessor macro __nonnull_terminated or __maybe_non_null_terminated.

Default bounds are only applied when no explicit annotation is added. Which annotation should be used depends on the use case for you const char pointer, but annotating it with __single/__bidi_indexable/__counted_by/__ended_by/__unsafe_indexable will all disable the default annotation. While you could have a __nonnull_terminated, that would only fall back to the other pointer defaults (__single for ABI visible pointers, __bidi_indexable for non-ABI visible pointers, __unsafe_indexable in system headers), so you may as well type out the actual annotation.

1 Like
74

WG14 discussed GCC’s forward parameter declarations and finally decided that it is too late to get the specification correct for C23. It was rejected with this comment:

“We would like to pursue something along these lines for a future edition of the standard.”

Of course, if there is a good alternative one could consider that instead for the next revision of the standard. But I am personally very skeptical about late parsing for the reasons outlined in the comment above. That it is possible to implement this clang does not make it a good fit for C and/or other compilers, and this is the prerequisite for standardization.

1 Like
75

GCC’s extension with forward declared parameters is also suboptimal, which is why WG14 keeps having the discussion about what a decent approach looks like. Forward declared parameters has been rejected multiple times at this point because of the concerns with it as well as concerns with timing of making changes to the standard. For example:

  • In all other aspects of the language, type ident; introduces an object declaration, except for forward declarations of parameters. That inconsistency is hard to explain, especially given that related languages like C++ already let you do type ident; in places where you cannot traditionally do so in C but do introduce declarations (e.g., if (int x; foo(x)) {}).
  • Forward declared parameters run into all sorts of unpleasant edge cases due to the way C works:
    • void func(struct S { int a; } x; int whatever, struct S s); // Is this okay?
    • void func(struct S { int a; } x; int whatever, struct S { int a; } x); // What about this?
    • void func(struct S { int a; } x; int whatever, struct S { int b; } x); // This?
    • void func(int oh_no[g()]; int whatever, int oh_no[g()]); // How many times is g() called?
    • void func(int no, int oh_no[no = 12]; int whatever, int no, int oh_no[no]); // What is the value of no on entry to the function?
  • There’s challenges with recovering from incorrect syntax with forward declared parameters in order to give good QoI. e.g., void f(int a, int b, int a); was this a typo with the third parameter or was this a missing semicolon after the first? Or void f(int a; int b, int c); was this a typo with using a semicolon when a comma was intended, or did the user try to rename one identifier and forgot the other?
  • Forward declared parameters don’t generalize at all into solving the same problem elsewhere in the language. As this RFC points out, there is a need to refer to structure member declarations the same as referring to parameter declarations. We can’t reuse these facilities to solve the structure problem because the syntax picked already means “declare a member subobject” today.

Don’t get me wrong, late parsing isn’t perfect either. e.g., the idea of “let me find any identifier that would be in scope once the scope is finally closed” applies anywhere in the TU (file-scope declarations, local variables, etc) and we don’t want to turn C into a language needing two-pass facilities everywhere. However, late parsing does work reasonably well for the cases where declaration order matters and cannot be changed due to ABI (function signatures and structure layouts) which is the hard problem to solve. Users can rearrange declarations more easily in other circumstances, but there’s still a circular reference issue where you want to support something like type foo [[attr(bar)]]; type bar [[attr(foo)]]; Late parsing does not add significant parse time overhead because it’s a pay-for-what-you-use implementation detail. When parsing a context where this can occur, if you see a reference to an identifier that cannot be found, you only have to late parse that one declaration otherwise declaration parsing occurs as usual. Given that the vast majority of uses of function declarations will not involve forward declared parameters, the hit to compile times is negligible. You are definitely right that it adds implementation complexity though.

I don’t agree that if fits well within the rules of the C language, it definitely is not flexible (it’s only plausible in parameter declarations), and it introduces new kinds of problems to be concerned about. This is not a situation where “just implement this” is a good way forward – we’ve not implemented this feature despite otherwise going for GCC compatibility specifically because the feature has these undesirable properties and the extension is not particularly popular in the wild: context:global (?m:^(\w+… - Sourcegraph (my ability to write Re2 regexes is pretty limited, so there may be a better way to do that – but spot-checking those results shows almost no usage of the feature; I’d be curious to know if GCC folks have more details on how frequently they see the extension used).

Given the lack of clarity coming out of the WG14 meetings last week (no approach looked to be a “slam dunk” within the committee) and that the committee won’t be considering new features for a while now, I think this proposal should proceed with whatever approach the authors think best so long as it can be justified to the code reviewers.

4 Likes
76

Am Samstag, dem 01.07.2023 um 14:40 +0000 schrieb Aaron Ballman via LLVM Discussion Forums:

AaronBallman
July 1

GCC’s extension with forward declared parameters is also suboptimal, which is why WG14 keeps having the discussion about what a decent approach looks like. Forward declared parameters has been rejected multiple times at this point because of the concerns with it as well as concerns with timing of making changes to the standard.

There were now several positive votes that we want to have this feature. It was rejected
for wording and timing issues. If not for timing, we would have this now in C23.

For example:

  • In all other aspects of the language, type ident; introduces an object declaration, except for forward declarations of parameters

I do not understand this. A parameter (forward) declaration is also an object decleration? Where do
you see any inconsistency?

  • That inconsistency is hard to explain, especially given that related languages like C++ already let you do type ident; in places where you cannot traditionally do so in C but do introduce declarations (e.g., if (int x; foo(x)) {}).

I think this is nicely consistent. This is also a declaration which allows you to refer to an
object ‘x’ of type ‘int’. (And I would be in favor of adding this C++ feature also to C.)

  • Forward declared parameters run into all sorts of unpleasant edge cases due to the way C works:

I think the nice thing about forward declaration is that they do not really
introduce new edge cases. This implementation in GCC is quite simply.

  • void func(struct S { int a; } x; int whatever, struct S s); // Is this okay?

This would not be allowed because there is no parameter declaration for ‘x’.
This is potentially the only special rule: That we allow forward declarations
only for later parameter declarations. It would also be possible to just allow
arbitrary object declarations and corresponding to paramter and this would
also not cause any fundamental problems, but with this rules I think we can
give better diagnostics.

  • void func(struct S { int a; } x; int whatever, struct S { int a; } x); // What about this?

In C23 this would be allowed because you can redeclare ‘struct S’
with content. But this is not a a special thing about parameter forward
declarations, this is exactly how it works everywhere else:

// allowed before C23
static struct S { int a; } x;
static struct S x;

// allowed with C23:
static struct S { int a; } x;
static struct S { int a; } x;

  • void func(struct S { int a; } x; int whatever, struct S { int b; } x); // This?

With C23 we require redeclaration of structs to have identical content. Again, the same
rules also apply everywhere else, e.g,

static struct S { int a; } x;
static struct S { int b; } x; // invalid in C23

  • void func(int oh_no[g()]; int whatever, int oh_no[g()]); // How many times is g() called?

For simplicity and consistency, I think it should be called two times
because I do not see why there should be an exceptional rule for parameter
forward declarations that would have its size expression not evaluated when
it is evaluated for other declarations. This also how it works in GCC so there is
existing practice.

I think that size expressions with side effects are generally confusing and
a compiler should warn about those in general. This would affect parameter
declarations, type names in casts and typeof, and potentially VLAs.

We currently do not have the exact same situation for other redeclarations in
the standard (I think), because we have a constraint in other cases that avoids
external or internal linkage (where redeclaration is allowed) with variably
modified types. So adding a similar constraintsimply forbidding VM types
here would also be an option.

We would have to answer the same question if we allowed (as discussed) redeclarations
for compatible typedefs:

typedef int a[g()];
typedef int a[g()];

And I think we should do this to avoid “same type” mess and I think then for
consistency g() should also be called twice.

  • void func(int no, int oh_no[no = 12]; int whatever, int no, int oh_no[no]); // What is the value of no on entry to the function?

I think it should have the value 12 to be consistent with
the following example without forward declaration:

void foo(int n, int a[n = 12])
{
// n has value 12
}

  • There’s challenges with recovering from incorrect syntax with forward declared parameters in order to give good QoI. e.g., void f(int a, int b, int a); was this a typo with the third parameter or was this a missing semicolon after the first? Or void f(int a; int b, int c); was this a typo with using a semicolon when a comma was intended, or did the user try to rename one identifier and forgot the other?

We do not really have a negative experience in GCC with this feature being
activated by default for many decades. Both your examples would violate a
constraint and there would be a corresponding error message that would tell
the user what is wrong:

https://godbolt.org/z/TqW8nrGex

In fact, - after studying different options - I believe that with delayed parsing such
questions will become more difficult and it will be much harder to give good
diagnostics to users. (Also compilers will have to add a lot of infrastructure that
clang may have for C++ but other compilers currently do not need.)

  • Forward declared parameters don’t generalize at all into solving the same problem elsewhere in the language. As this RFC points out, there is a need to refer to structure member declarations the same as referring to parameter declarations.

We could also consider some king of forward declarations in structs.
But I think the situation there is a bit different. The size expressions in
structs would not be evaluated when the declaration of the struct is
encountered, but when a member is referenced later.

So semantically, this is very different and it may make sense to keep the
syntax and semantics different:

In GCC, we have

void foo(int n)
{
struct foo { int (*x)[g(n)]; } a ;
}

where the size expression is evaluated when the declaration is encountered
similar to:

void foo(int n)
{
int vla[g(n)];
}

For structs if allowed

int n;
struct foo {
char (*buf)[g(n)];
int n;
} s;

then there is an ambiguity about what this means. The same is true
for attributes:

int n;
struct foo {
char *buf attribute((bounds(g(n)));
int n;
} s;

When we later access ‘s->buf’ we need to know the value of the size
expression. I am not sure we want to trigger arbitrary (re-)computations
of complex size expressions simply when accessing a member of a struct.

So I think we should have different and new syntax for this feature,
because it will have different semantics from the simply declarations that
we already have in C (in contrast to parameters which I think should
work exactly like other declarations).

A potential solution is to add new syntax with constraints that avoid side effects

struct foo {
char (*buf)[.n];
int n;
};

  • We can’t reuse these facilities to solve the structure problem because the syntax picked already means “declare a member subobject” today.

Yes, just using a ‘;’ would not work for structs.

Don’t get me wrong, late parsing isn’t perfect either. e.g., the idea of “let me find any identifier that would be in scope once the scope is finally closed” applies anywhere in the TU (file-scope declarations, local variables, etc) and we don’t want to turn C into a language needing two-pass facilities everywhere.

Yes, this is my concern. The problem is that we already allow basically all
of the language in prototypes via size expressions. So once you allow it
there, you already have all the complexity.

I think this was a bad idea not to contrain size expressions to very simple
expressions (e.g. just identifiers), but this is what we have now.

However, late parsing does work reasonably well for the cases where declaration order matters and cannot be changed due to ABI (function signatures and structure layouts) which is the hard problem to solve.

It is the problem forward declarations also solve.

Users can rearrange declarations more easily in other circumstances, but there’s still a circular reference issue where you want to support something like type foo [[attr(bar)]]; type bar [[attr(foo)]]; Late parsing does not add significant parse time overhead because it’s a pay-for-what-you-use implementation detail.

It adds time when it is used. We want this feature to be used.

It also adds substantial complexity we currently do not require from
compilers. Clang has a philosophy of just accepting this complexity. I assume
because it has this because it is a C++ compiler which accepts C in the same FE,
so it has to accept this cost anyway. But this is not at all true for other compilers.

When parsing a context where this can occur, if you see a reference to an identifier that cannot be found, you only have to late parse that one declaration otherwise declaration parsing occurs as usual.

Sure, but this late parsing can add arbitrary cost to the sitations you have to do it.
And it adds a huge complexity to implement it in the first place. You need to be
able to store the full generic AST before type checking for arbitrary compex expressions
and revisit it later.

This is not how many C compiler works because in C you can do type checking already
when parsing. Smaller C compilers (and also GCC) do this and at that time also emit
diagnostics. So other C compiler may not currently have the support to do late
parsing and this is a substantial burding (essentially a complete FE rewrite) and you
would imposethis to everyone by changing this fundamental property the language.

Given that the vast majority of uses of function declarations will not involve forward declared parameters, the hit to compile times is negligible. You are definitely right that it adds implementation complexity though.

Sch costs tend to accumulate. I guess you could say this also for most C++ features.
Still, in practice, compile time for largers C++ projects is bad.

uecker:

In contrast, the GCC extension is rather simple, fits well with the rules of the C language, is very flexible and powerful without introducing semantic problems with cycles. Just implement this. It may look ugly at first, but technically it is far better in so many ways.

I don’t agree that if fits well within the rules of the C language, it definitely is not flexible (it’s only plausible in parameter declarations),

It is very flexible. You mean it can not be applied to structs? This is true, but I think this
needs to have a semantically different solution anyway. So I am fine with new syntax
there.

and it introduces new kinds of problems to be concerned about This is not a situation

where “just implement this” is a good way forward – we’ve not implemented this feature despite otherwise going for GCC compatibility specifically because the feature has these undesirable properties and the extension is not

I do not see any undesirable properties. The GCC experience is that it is simple to
implement and does not cause problems. It also solves the problem we have now
just fine for GCC. If clang would support it, we could move on and annotate a
lot of existing interfaces, which would be a huge step forward for safety.

particularly popular in the wild: context:global (?m:^(\w+… - Sourcegraph (my ability to write Re2 regexes is pretty limited, so there may be a better way to do that – but spot-checking those results shows almost no usage of the feature; I’d be curious to know if GCC folks have more details on how frequently they see the extension used).

I think this argument is irrelevant. Any other new solution you want to invent also
has no current users. The reason this is not used is that a) it was useless until
recently (this changed now with better static analysis in GCC) and b) it is not
portable (which would change if clang implements it).

Given the lack of clarity coming out of the WG14 meetings last week (no approach looked to be a “slam dunk” within the committee) and that the committee won’t be considering new features for a while now, I think this proposal should proceed with whatever approach the authors think best so long as it can be justified to the code reviewers.

I think the authors should consider the technical arguments I gave.

Martin.

77

Update: I actually think this is not only technical but also societal. Clang unilaterally pushing forward with a solution that changes a fundamental property of the language (one-pass parsing + type checking) which may hurt other smaller compilers, analysis tools, and code editing tools when there is an existing solution that is known to work and does not have such problems, just because clang can do this easily with its extensive infrastructure inherited from C++, seems a questionable move to me.

Update 2: Note, that my concern specifically about clang using late-parsing as a solution that will cause problems for others. If clang does not implement GCC’s extension (although it seems a good solution to me and I do not really understand the problem with it), but instead would implement something else which is also simply to implement for others without causing all the problems, then I would be much less concerned. For example, you could consider new syntax with enough constraints that parsing in one pass is still possible.

1 Like
78

That is not how I interpret the situation in WG14, but it’s still moot – WG14 has not formed a consensus position on what to do here so the design space is still open to the industry to find solutions we’re happy with.

void func1(int x; int x); // Fine
void func2(int x; int x; int x); // Fine
void func3(int x, x, x; int x); // Weirdly, not fine
void func4(void) {
  int x;
  int x; // Not fine
}
struct S {
  int x;
  int x; // Not fine
};

int x;
int x; // Not fine

I don’t see the consistency argument there; in my example, the formal parameter defining no comes after the forward declaration with the side effect. In my mental model, arguments at the call site are assigned into the formal parameters used by the local function. So the no = 12 side effect happens first (no call site argument to assign to it because it’s not a “real” parameter), then the assignment from the call site into no happens second, so int oh_no[no = 12] gives an int[12] while int oh_no[no] gives an int[no] and no doesn’t necessarily have to be 12 within the function. But thinking on it a bit more — shouldn’t this be rejected as the forward declaration doesn’t match the final declaration?

I disagree strongly – I do not want ad hoc solutions for each situation where we need this functionality. I want one solution that works in all the situations we need to look forward for an identifier. As another example that’s not handled by forward declared parameters:

[[vendor::attr(n)]] void func(int n);

No amount of forward declared parameters will help this situation, but it’s a real problem I wish we had a solution for today because we’re stuck using GNU attributes for some declaration attributes due to this: Compiler Explorer (moving the [[]] attribute to after the parameter list changes the attribute to apply to the function type rather than the function declaration).

Note: I don’t care whether the solution is late parsing or some alternative syntax. I care that I can apply the same solution to all of these situations instead of having independent solutions for each. I also don’t agree that forward declarations in structures are any different than forward declarations of parameters; in all these cases, what’s needed is a way to say “lookup might not find this identifier yet, but it needs to be able to find it to complete the declaration”. (I think it’s reasonable to argue that it’s better to have the name and the type information up front, but really all that we need is the name because the type can be looked up later.)

This sort of thing would be lovely to generalize so it works everywhere. The . makes that awkward for the case of parameters, but perhaps we could find some consistent syntax that works? void func(int array[<<<n>>>], int n); struct S { int array[<<<n>>>]; int n; }; [[vendor::attr(<<<n>>>)]] void frob(int n); (Not a serious syntax suggestion, just more to point out that having a consistent syntactic way to say “will be found later within the same full declaration” is what I’d strongly prefer aiming for.)

My point is that no matter how much this feature will be used, it will always be a minority of the time compared to the total number of function declarations parsed. The number of times you need a forward declared parameter (or struct field) is tiny compared to the number of times you don’t.

100% agreed – I’m not making a fiat decision here, just giving my opinion on the direction. Ultimately, it’s up to the proposal authors to come forward with a solution that works to meet their needs and that the community at large finds acceptable. I just wanted it to be clear that I don’t think the proposal authors need to wait longer for WG14 opinions as we didn’t come to a consensus position.

If the authors move forward with a late parsing approach, I’d be curious what the actual overhead costs are when using the feature because I do agree that those costs could add up. Or they could be entirely negligible until you use the facilities with millions of declarations, etc.

This is how extensions are developed and there’s nothing novel about this situation in that regard. WG14 has no consensus on a feature in this area; if they did, we would follow that. There is an existing extension supported by another popular compiler, so we should certainly be evaluating whether we want to adopt that extension as well. If we decide not to go that route, that’s our choice to make. We already have extensions that change fundamental properties of the language (we allow function overloading in C, we allow VLA usage in C++, etc); if it’s a non-conforming extension, it should be behind a feature flag, but this proposal already is (-fbounds-safety).

I don’t want Clang to go down a path that will cause problems for future standardization, but we also cannot predict what the committee will or won’t do in the future. So it’s a balancing act between finding a solution we’re happy with and finding a solution we think the committee might be interested in or at least won’t be frustrated by, but what I think ultimately drives decisions when there’s not a clear path forward is what provides the best user experience for our users (which includes things like portability).

5 Likes
79

Thanks for your insights! To be clear, this RFC does not intend to compete with the parameter forward declaration approach that may possibly be included in the standard in the future. -fbounds-safety should also work nicely with functions that use the parameter forward declaration once it’s adopted in Clang.

In general, we are more interested in what this model can do “after” parsing, i.e., enforcing bounds safety using the bounds information. Therefore, we are open to any syntax suggestions as long as they serve our purposes.

That said, there are several reasons why we are interested in exploring late parsing, specifically for “attributes”, for now, by putting it under a separate experimental flag:

  • Firstly, this approach generally works with struct fields and other contexts without breaking portability. As discussed earlier, the GCC’s forward declaration extension doesn’t work for struct fields. In the long term, introducing a new syntax for late-parsed struct members, e.g., char (*buf)[.n]; or int array[<<<n>>>], might make sense, but it would break the portability of the code adopting the model at this point until we can push it into the standard.
  • Secondly, late parsing for attributes is interesting on its own as it solves some of the existing issues with C attributes. For example, late parsing for attributes would allow use cases like struct guarded_int { int value __attribute__((guarded_by(m)); struct mutex m; } and [[vendor::decl_attr(n)]] void func(int n); which isn’t currently supported.
  • Additionally, by confining this to attributes only, we should be able to avoid some of the potential issues related to late parsing. For example, cyclic dependencies may not be a problem for attributes. Other tools that need to understand partial code, such as IDEs, can ignore unsupported attributes; or at least this doesn’t introduce a new problem as Clang C++ already supports late-parsed attributes.
  • Lastly, implementing it under a separate feature flag will allow us to collect data on the performance impact of the approach, while making it adaptable to any new direction as we will learn more.
2 Likes
80

Am Donnerstag, dem 06.07.2023 um 03:22 +0000 schrieb Yeoul Na via LLVM Discussion Forums:

rapidsna
July 6

Thanks for your insights! To be clear, this RFC does not intend to compete with the parameter forward declaration approach that may possibly be included in the standard in the future. -fbounds-safety should also work nicely with functions that use the parameter forward declaration once it’s adopted in Clang.

Yes, I understand this. This was not my point. My point is that any feature based on
delayed parsing is problematic from a language design point of view for the reasons
I spelled out. In particular, it might also problems for others implementing it.

Parameter forward declarations are a simple one possible alternative which does not
have all those issues of delayed parsing

In general, we are more interested in what this model can do “after” parsing, i.e., enforcing bounds safety using the bounds information. Therefore, we are open to any syntax suggestions as long as they serve our purposes.

Yes, this is also what I am interesting. But I am also intested in having solutions
that can be adopted by other without causing problems.

That said, there are several reasons why we are interested in exploring late parsing, specifically for “attributes”, for now, by putting it under a separate experimental flag:

I can’t stop you from “exploring late parsing”, I just think you shouldn’t.

  • Firstly, this approach generally works with struct fields and other contexts without breaking portability. As discussed earlier, the GCC’s forward declaration extension doesn’t work for struct fields. In the long term, introducing a new syntax for late-parsed struct members, e.g., char (*buf)[.n]; or int array[<<<n>>>], might make sense, but it would break the portability of the code adopting the model at this point until we can push it into the standard.

If you use late parsing for new attributes then this obviously does not cause
backwards compatibility problems. But it means that you introduce an attribute
that follow unusual rules regarding identifier lookup that are not coherent with
the rest of the language. I simply do not think this is sound language design,
even for a non-standard attribute.

  • Secondly, late parsing for attributes is interesting on its own as it solves some of the existing issues with C attributes. For example, late parsing for attributes would allow use cases like struct guarded_int { int value __attribute__((guarded_by(m)); struct mutex m; } and [[vendor::decl_attr(n)]] void func(int n); which isn’t currently supported.

Yes, it solves those problems, but at a cost.

  • Additionally, by confining this to attributes only, we should be able to avoid some of the potential issues related to late parsing. For example, cyclic dependencies may not be a problem for attributes. Other tools that need to understand partial code, such as IDEs, can ignore unsupported attributes; or at least this doesn’t introduce a new problem as Clang C++ already supports late-parsed attributes.

It does not cause problems for clang. It will cause problems for others.
And I do think it is a bit short sighted to ignore the long-term need
of IDEs, tools , or other compilers, even for a non-standard attribute.

Martin

  • Lastly, implementing it under a separate feature flag will allow us to collect data on the performance impact of the approach, while making it adaptable to any new direction as we will learn more.

Visit Topic or reply to this email to respond.

To unsubscribe from these emails, click here.

1 Like
81

I couldn’t agree more. We simply couldn’t find other solutions that also work for struct fields without causing a portability problem at this point. In practice, ensuring portability is an indispensable requirement for this model to be adopted in open-source projects like the Linux kernel.

I hear you, but again, I don’t see this to introduce a “new” problem for IDEs, given that late-parsed attributes already exist in C++, but I’d love to learn more. In the short term, this won’t be a problem for other compilers, as I expect that they must have been designed to ignore new attributes that they do not recognize. Additionally, the annotations are macro-defined and expand to empty without the feature flag. In the long term, depending on how this goes, we might want to consider proposing to WG14 a new forward declaration syntax for struct fields that can replace late-parsed attributes. In this regard, I believe experimenting with late parsing for attributes in Clang will still help make more convincing points as to why we will need the new syntax.