[CERT C Review Batch 2/5] Review proposed Rust categorization

[PLeVasseur]

Context

Thanks for taking a look at this batch.

This issue is part of the follow-on work from #336, where Félix finished a first pass mapping CERT C rules into Rust-oriented categories.

The goal here is to get a focused review on one slice of the 103 rules without making anyone wade through the whole set at once.

If you notice something that cuts across batches, or you want to leave a more global comment, please put that on #336.

If you want to read the original CERT wording before reviewing a mapping, the rule IDs in the table below link directly to the CMU SEI Confluence page for each rule. You may also find the top-level SEI CERT C Coding Standard useful for broader context.

Reviewer note

Please feel free to leave working notes here as you go.

When you feel done with this batch, please leave a short final summary on #336 and link back to this issue so the parent thread stays the main record of the outcome.

Review goal

For each rule below, please check whether the current proposed bucket still feels right for Rust.

If you disagree, are unsure, or think the rule should be merged with another concern, that is useful feedback too. A short rationale is enough.

Current buckets:

• Does not map to Rust
• Definitely maps to Rust
• Maybe
• Maps directly to Unsafe Rust

Rules in this batch

Please do not feel boxed in by the current buckets. If one of these looks miscategorized, that is exactly the kind of feedback we want.

ID	CERT C rule	Current proposed bucket	Notes	Feedback
PRE30	Do not create a universal character name through concatenation	Does not map to Rust	Rationale: it's just very specific to C	feedback
DCL38	Use the correct syntax when declaring a flexible array member	Does not map to Rust	Why not? The rule is about "using the standardized syntax for this feature, instead of relying on Undefined Behavior and a compiler-specific, non-standardized syntax". Such a suggestion makes no sense in Rust, since there is only one compiler and its syntax is the only one available. Correct syntax can compile; incorrect syntax cannot.	1, 2, 3, 4
EXP35	Do not modify objects with temporary lifetime	Does not map to Rust	Rationale: the borrow checker eliminates this problem	1, 2
EXP43	Avoid Undefined Behavior when using restrict-qualified pointers	Does not map to Rust	Rationale The borrow checker gives us correct-by-construction restrict qualifications in codegen. Basically, "restrict" is part of the type system, and type checking (in particular borrow checking) enforces that its rules are always abided by.	1, 2, 3
ARR32	Ensure size arguments for variable length arrays are in a valid range	Does not map to Rust	Rationale: VLAs are not a thing in Rust	agree
ARR38	Guarantee that library functions do not form invalid pointers	Does not map to Rust	Rationale The rule basically asks the user that, whenever they call a function that receives a pointer to an array and a length, that said length doesn't go outside the bounds of the array (which would form an invalid pointer) In Safe Rust, this is handled by bounds checks, and in the mapping of ARR30 we're already handling that. In Unsafe Rust, doing this is to break the Safety Contract of the API we're calling, which is getting its own rule. and is outside the mapping of this ARR38 (this rule) Recently moved out of the applicable bucket	1, 2, 3
STR32	Do not pass a non-null-terminated character sequence to a library function that expects a string	Does not map to Rust	Rationale: strings are not null-terminated in Rust	1, 2, 3, 4
FIO34	Distinguish between characters read from a file and EOF or WEOF	Does not map to Rust	Why not? See the Read trait. EOF / WEOF cannot be acquired from them. As long as file reading operations are done through the Read trait, this should not be a concern.	feedback
FIO37	Do not assume that fgets() or fgetws() returns a nonempty string when successful	Does not map to Rust	Rationale This rule comes from the need to properly handle errors found during the execution of fgets() and fgetws(). Errors in those functions are reported in a way whose proper handling cannot be enforced by the compiler. Fallible APIs in Rust return values of the enum Result<T, E>, whose error case must be properly handled, in order for the program to compile. This feature completely eliminates the need for this kind of rule in Rust.	feedback
FIO39	Do not alternately input and output from a stream without an intervening flush or positioning call	Does not map to Rust	Rationale This is UB in C, due to implementation details of the stream handling APIs. The implementation of those APIs in Rust is different, so the guideline doesn't really map to it. For more information, see this comment and this issue.	NA
FIO44	Only use values for fsetpos() that are returned from fgetpos()	Does not map to Rust	Why not? The closest APIs in Rust's std to C's fsetpos and fgetpos are the ones provided by the std::io::Seek trait. Their main differences can be summarized by two things: - The foundational method of the trait, Seek::seek, seeks by the number of bytes given to it. It doesn't require the "multibyte parser state" that's sometimes encoded in the fpos_t parameter for fsetpos, since the trait has no notion of wide-oriented streams. - There is no Undefined Behavior in Seek's APIs. Thus, there is no reason to restrict the provenance of the offsets used there.
ERR33	Detect and handle standard library errors	Does not map to Rust	Why not? Functions in std that may fail, wrap their result in the Result<T, E> type, which forces the caller to handle the error case whenever it happens. Thus, this rule is already enforced by the compiler.	feedback
MSC37	Ensure that control never reaches the end of a non-void function	Does not map to Rust	Rationale: this is already enforced by the type system
INT30	Ensure that integer operations do not wrap	Definitely maps to Rust	Already implemented; still review mapping/category
INT33	Ensure that division and remainder operations do not result in divide-by-zero errors	Definitely maps to Rust	Already implemented; still review mapping/category	agree
ARR30	Do not form or use out-of-bounds pointers or array subscripts	Definitely maps to Rust	How would it map? In Safe Rust: - Direct addressing will panic if OOB - arr.get(index) will return None if OOB In Unsafe Rust: - arr.get_unchecked(index) is UB if OOB. So, I'd say out-of-bound indexing should be either prevented or handled properly. If using Unsafe indexing, it should be formally validated.	agree
FIO45	Avoid TOCTOU race conditions while accessing files	Definitely maps to Rust	Rationale: std::fs explicitly talks about TOCTOU and how to avoid it.	agree
FLP30	Do not use floating-point variables as loop counters	Maybe	Current consensus The portability issue isn't a problem in Rust; not for basic arithmetic, at least. The Zulip thread remained inconclusive. It's a very hard topic, and it's hard to make actionable rules out of this. Feel free to peruse and continue that conversation. Zulip thread: Are floats ever used as loop counters?	1, 2, 3
ARR36	Do not subtract or compare two pointers that do not refer to the same array	Maps directly to Unsafe Rust	How would it map? This maps to a series of guidelines. Pointers cannot be subtracted or compared in Rust. However, the same idea here applies - pointer arithmetic between pointers must only happen inside the same allocation: std::ptr->allocation I would say that every Safety rule in the ptr module probably merits its own Guideline. One rule that must be definitely be in there is: our offset methods must never take us to a point outside of the bounds of the object our pointer is attached to.	1, 2, 3
MEM30	Do not access freed memory	Maps directly to Unsafe Rust	How would it map? This is impossible in Safe Rust. It is possible in Unsafe. There is nuance, however, so I strongly recommend reading the Zulip thread below. Zulip thread: Can I read something after dropping it?	agree
EXP34	Do not dereference null pointers	Maps directly to Unsafe Rust	Maps with modifications: 1. Zero-sized accesses are Defined Behavior std::ptr safety. 2. Volatile access to the address 0x0 is permitted std::ptr::read_volatile documentation.	feedback

What would help

• Each rule is either confirmed, challenged, or marked as needing more discussion.
• Any merged rules or overlapping concerns are called out explicitly.
• Any rule that looks like a good follow-up coding guideline issue is noted.
• A short summary is posted on #336 with a link back to this issue.

After opening this issue

Post this comment to request a reviewer from the Producer queue:

@guidelines-bot /r? producers

[PLeVasseur]

@guidelines-bot /r? producers

[github-actions]

👋 Hey @vjonaswolf! You've been assigned to review this coding guideline issue.

Your Role as Reviewer

As outlined in our contribution guide, please:

1. Provide initial feedback within 14 days

2. Work with @PLeVasseur to flesh out the concept and ensure the guideline is well-prepared for a Pull Request

3. Check the prerequisites before the issue is ready to become a PR:

   • The new rule isn't already covered by another rule
   • All sections contain some content
   • Content written may be incomplete, but must not be incorrect
   • The 🧪 Code Example Test Results section shows all example code compiles

4. When ready, add the sign-off: create pr label to signal the contributor should create a PR

Bot Commands

If you need to pass this review:

• @guidelines-bot /pass [reason] - Pass just this issue to the next reviewer
• @guidelines-bot /away YYYY-MM-DD [reason] - Step away from the queue until a date
• @guidelines-bot /release [@username] [reason] - Release assignment (yours or someone else's with triage+ permission)

To assign someone else:

• @guidelines-bot /r? @username - Assign a specific reviewer
• @guidelines-bot /r? producers - Request the next reviewer from the queue

Other commands:

• @guidelines-bot /claim - Claim this review for yourself
• @guidelines-bot /rectify - Reconcile this issue/PR review state from GitHub
• @guidelines-bot /label +label-name - Add a label
• @guidelines-bot /label -label-name - Remove a label
• @guidelines-bot /queue - Show reviewer queue
• @guidelines-bot /commands - Show all available commands

[github-actions]

✅ @vjonaswolf (next in queue) has been assigned as reviewer.

[bjorn3]

For ARR36 these operations are UB in C, but comparisons are safe in Rust and subtractions are safe if you use wrapping_sub and UB if you use the unsafe sub.

[vjonaswolf]

@bjorn3 thanks for the input.

[kpreid]

Unsolicited input:

DCL38 Use the correct syntax when declaring a flexible array member (Does not map to Rust)

Rust does have an analogous problem and an analogous official syntax for dynamically sized structs:

struct FlexStruct {
    data: [i32],
}

This syntax is applicable to fewer cases than C flexible array members (because it currently requires the length to be stored out-of-line in a wide pointer to the struct, whereas many uses of such structs want the length to be stored only in-line as a field), but, similarly, it should be used when using it is an option, and, similarly, uncareful use of a struct type that does not acknowledge being dynamically sized can lead to out-of-bounds access UB. Therefore, this rule should be considered to map to Unsafe Rust.

EXP43 Avoid Undefined Behavior when using restrict-qualified pointers (Does not map to Rust)

This rule maps to Unsafe Rust. While the rules of &mut and restrict are not identical, unsafe code which produces &mut T must take care to to avoid aliasing with other &mut T or with unsafe writes to the same memory.

STR32 Do not pass a non-null-terminated character sequence to a library function that expects a string (Does not map to Rust)

This is not an issue “natively” in Rust, but Unsafe Rust code performing FFI must be aware that Rust strs are not null-terminated unless constructed to be.

[riking]

FLP30 is Definitely Maps to Safe Rust. It's a bit harder to do by accident:

    for x in (2.0..6.0) {
    // ^~ ERROR: `std::ops::Range<{float}>` is not an iterator
        println!("{}", x);
    }

EXP34: Correct, with the caveat that volatile access to address 0 is permitted

[felix91gr]

@riking

FLP30 is Definitely Maps to Safe Rust.

Sorry, since this one is a Maybe, then perhaps it should've been accompanied by the currently accrued context. I didn't ask Pete to do that, so this one is on me 😆

For the official source on the Maybe table, please check here.

Here is a copy of that cell:

FLP30: Why "Maybe"

The portability issue isn't a problem in Rust; not for basic arithmetic, at least.

The Zulip thread remained inconclusive. It's a very hard topic, and it's hard to make actionable rules out of this. Feel free to peruse and continue that conversation.

Zulip thread: Are floats ever used as loop counters?

[felix91gr]

Update: I've added the Rationale notes to the table n.n

[felix91gr]

Re: DCL38-C

As @kpreid pointed out to me today, a data structure very similar to C's Flexible Array Members is possible in Rust (using unsafe and other such things).

Thus, my original justification for why the rule does not map is incorrect.

However, the spirit of that rule is a bit funny. Some history on DCL38-C:

It has nothing to do with how to avoid pitfalls in the usage of Flexible Array Members. Rather, it has got to do with how they used to be a "non-official official" language construct, where every compiler had a slightly different syntax for declaring them. Of course, that syntax depended on Undefined Behavior, in order to not clash with otherwise valid (and semantically different) C constructs that were part of the C Standard.

All of that changed when Flexible Array Members got standardized: a specific, official syntax was picked for them, but the other compilers still had their "rogue" implementations of the construct that had to still compile. Thus, non-official (and actually UB) ways to declare FAMs kept being available to users, even though they were technically UB.

Enter DCL38-C: "Use the correct syntax when declaring a flexible array member". That's what it's all about: "just write the dang thing the official way. Don't declare your FAMs using UB!"

All of that is to say that the rule still would not apply to Rust.

Here's my new justification for it:

The rule is about "using the standardized syntax for this feature, instead of relying on Undefined Behavior and a compiler-specific, non-standardized syntax".

Such a suggestion makes no sense in Rust, since there is only one compiler and its syntax is the only one available. Correct syntax can compile; incorrect syntax cannot.

I've updated the table with this new Rationale. Please everyone, feel free to give me your thoughts on it.