RFC: C++ Buffer Hardening

[DenisYaroshevskiy]

Here is Alexander Stepanov arguing against doing this kind of checks.

Efficient Programming with Components: Lecture 11 Part 2

[jankorous]

I’ll respond to the individual points below but let me start with another way how to phrase our intentions.

We are closing a class of security vulnerabilities in C++ codebases for which security is critical-enough that adopting a significantly restrictive programming model together with libc++ with extra runtime checks at a significant adoption and runtime performance costs is worth it. We do not expect this to be true for every C++ project.

While we plan to use warnings and fixits, in our view these are just vehicles to adopt the programming model and verify that the source code is compliant. Under this programming model the compiler will make sure that the only pointer arithmetic in the code is done by libc++ which combined with its hardened mode will guarantee bounds-checks before memory access. For non-compliant code we’ll emit warnings.

I’m uncomfortable with this approach without further demonstration that it’s viable in terms of finding true positive issues vs swamping the user with diagnostics.

Let me repeat that the approach we’re taking is not necessarily suitable for each and every project.
Our perspective represented by the strict programming model is arguably rather extreme - pointer arithmetic in user code is considered unsafe and our adopters will be security critical components where this approach is justified. Under this programming model there is no notion of false positives.

The two exceptions where pointer arithmetic is allowed will be - libc++ headers and code explicitly labeled as “unsafe” by macros/pragmas we’ll provide.

Will this diagnostic be triggered by code in system headers?

We need to support adoption of the programming model on per-project basis. Warnings for code in system headers can’t typically be addressed by changes in the client code. Because of that we don’t plan to emit diagnostics for system headers.

Have you tried this approach out yet on real world code bases – how many diagnostics do you get?

We have done experiments with manual changes in several internal codebases. The changes required by the programming model can be massive. That is why we see providing adoption tooling to automatically transform the source code as critical. Again, the adoption cost will be non-zero and it is likely that it won’t make sense for every project to pay this (and other) cost.

And you mentioned this is potentially useful for C – are you intending to enable the diagnostic for C as well as C++? (How do C users eliminate pointer arithmetic to silence the warnings?)

While our primary use-case are C++ codebases a similar model can be in theory applied to C codebases by using the pragmas to allow a small (well tested and audited) part of the codebase to do pointer arithmetic and restrict that for the rest of the codebase.

I’m especially wary of introducing anything like the C++ Core Guideline rules into the compiler proper. The C++ Core Guidelines are pretty reasonable for folks starting a new project from scratch, but effectively zero effort has been put into their enforcement recommendations and my experience with trying to support the coding standard in clang-tidy is that it’s not worth the effort (we frequently go back to the C++ Core Guidelines authors with questions and the responses are usually along the lines of “please tell us what enforcement should look like” which is far too much effort for reviewers to have to put into a review).

Our approach is different. While coding styles and such certainly have their use we don’t believe that’s a reliable way to prevent security vulnerabilities. That is why we aim to have a compiler-enforced programming model.

It almost certainly will be outside of -Wall unless the implementation is drastically different from what I’m imagining in terms of diagnostic behavior. However, we have historically had quite a bit of evidence that off-by-default warnings are a lot of effort for very little gain because the vast majority of users will never enable them. Typically, we ask “what can be done to enable this diagnostic by default?” and I wonder the same thing here.

We essentially plan to trade adoption cost and some runtime performance for improved security. The diagnostics could be generally useful as a tool for auditing purposes but it is not very likely that the programming model sees universal adoption.

This also makes me a little uncomfortable; our typical policy for fix-its is that they have to actually fix the code, not break it differently. And in this case, it may take working code and break it rather than taking broken code and breaking it differently, which is a bit worse. This matters because fix-it hints can be automatically applied when they’re associated with a warning and there’s only one hint for that line of code – so it would be very easy for a user to do significant damage to their code base unless we attach these fix-its to a note. But if we attach them to a note, we now potentially double the number of diagnostics we emit for something I’m already worried is going to be too chatty.

We plan to use fix-its for code transformations that are possibly both more complex and more extensive than typical warning fix-its. There are limits to how much tools can infer from the code and in some cases we might have to get the user involved which we plan to do via placeholders in the code. An example would be a situation where we detect that an int * variable needs to be transformed to std::span<int>. We might be able to produce all the necessary code transformations but maybe we won’t be able to infer the correct size to use for construction of the span. In such case, we’d transform the code to something like std::span<int> myVar(buffer, #size#); and ask user to fill-in the size. Having the placeholder breaking the build and forcing user to not ignore it would be preferable to the user having the option to silently ignore the issue.

[NoQ]

To add to Jan’s reply, yeah, so we start with two seemingly valid assumptions:

• The problem is really damn important (like folks have pointed out, massive source of CVEs);
• It can’t seem to be solved without intrusive code changes (you might eliminate quality problems this way, but not vulnerabilities).

And it immediately follows that traditional solutions, like very quiet high-true-positive-rate warnings or runtime sanitizers, will never be enough. So we ask ourselves, How else can we achieve practicality?

Like, I completely agree that cppcoreguidelines are typically impractical and unenforcable, I rejected such checks from static analyzer myself in the past. If the problem wasn’t that important, we’d totally go with the Ostrich algorithm and call it a day.

So our solution is, we think that we can achieve practicality by building this fixit machine. If we can’t avoid rewriting code, let’s at least make it easy enough to do so.

For most problems this is worse than doing nothing. And even for this problem, in a not-insignificant amount of cases that’s going to be still worse than doing nothing.

But if any problem deserves unique consideration, that’s probably the one.

So while we’re not absolutely hard-bent on putting it in the compiler, we think that this problem is uniquely important enough to have us question our usual intuition and treat it specially. Another thing we can try to do is treat it as a language mode, i.e. say, have an -f... flag instead of (or in combination with) a -W... flag, if this helps avoid user confusion.

[bcardosolopes]

@jankorous

C++ Safe Buffers Programming Model together with Hardened libc++ provide runtime mitigation of out-of-bounds memory access

This is pretty cool idea, looking forward to see it happening.

@AaronBallman

However, we have historically had quite a bit of evidence that off-by-default warnings are a lot of effort for very little gain because the vast majority of users will never enable them. Typically, we ask “what can be done to enable this diagnostic by default?” and I wonder the same thing here.

This is a fair point, it’s always better if we can enable things by default. I’ve got and given patch reviews around this point in the past, but I wonder whether sticking to this logic (potential cargo cult stuff on lots of effort + very little gain + no users) might just prevent the community from getting simple switches that could provide goodness in your easily-accessible-every-day-compiler (no need to go chase a specific tool or setup a check).

Examples where this has been useful are existing warnings that help understanding Clang Modules builds. IMO the use case of this RFC totally justifies an off-by-default warning.

@NoQ

So our solution is, we think that we can achieve practicality by building this fixit machine

I like the fixit machine idea. What kind of limitations you currently face and want to improve?

[supercat]

There have been innumerable exploits from out of bounds accesses over the years, and those are just the ones we know about as they were used in actual attacks rather than misc crashes/incorrect behaviour potentially long after the error.

Yes, and all of these come from kernel-level software/firmware and not from user-space code where the vast majority of C++ is used.

Nonetheless, if some driver inadvertently accessed the off-by-1 address, do you think we have the benefit of bringing down the whole system just because? I don’t think so. It isn’t practical although it seems the right thing ™ to do.

The environments where termination isn’t considered acceptable, the alternative of “keep going with corrupted memory” is even less acceptable.

And hardening the library literally makes no difference to those situations. It does not apply.

So that brings us to the rest of the cases where it arguably may make a difference and which is a user-space code that is not exploitable and not critical by any means. If the error happens, it will be restarted. I think it’s simple as that. These types of applications can live with that.

Does hardening the library bring much, if any, value in such cases? I personally don’t see the big difference between (1) segfaulting, (2) continuing to run because you’re lucky, and (3) terminating through the hardened library. The only value I see is in the increased test coverage, as I already mentioned, and maybe a tiny bit of better debugging experience.

Does hardening the library have the potential to introduce downsides, such as the ones in performance? I think it has. Does it matter? It depends on the context.

We have a lot of evidence to show that “zero bugs” is not feasible (and even safe languages like rust, etc will crash on bounds errors).

So only because “safe” Rust, which btw is far from the truth [1], is doing it C++ should do it as well regardless of the absence of good arguments for doing so? Let’s please not jump on the annoying Rust train just because. I’d rather have good reasons for it.

[1] CVE - Search Results

[tschuett]

The value of a hardened libc++ is catching out of bounds accesses and pin pointing at the issue. It is up to you what your crash handler is doing.

From a brief scan of the Rust CVEs, it looks like these are business logic issues and not out of bound accesses.

[AaronBallman]

@ldionne

So far, the only semi-formal criteria I’ve been able to come up with is that no implementation of a check should require blocking thread synchronization (basically taking a global lock). Apart from that, we also won’t do anything that would change the complexity of algorithms, but that’s a byproduct of conforming to the Standard in most cases.

I think those are both excellent things to avoid.

What about impacts on constexpr evaluation? Do you think it’s reasonable to have a constexpr STL interface which fails in one mode but not another? (I think it seems like a feature rather than a bug, but I’m curious if I’m alone in that thinking.)

How about impacts on template instantiation? Would it be reasonable for different modes to instantiate templates at different points?

I suppose another question is how compile time performance should be considered. Are you going to attempt to avoid introducing significant compile-time overhead between modes? I’d expect some amount of variation, of course, but would it be acceptable for a hardened mode to take 30% longer at compile time for non-pathological code?

@ldionne

IMO, anywhere in the C++ Standard Library where the behavior is undefined is a place where we could have a check. In practice, I expect that we’ll need to carefully consider each proposed check individually. And perhaps we’ll begin to develop more formal criteria as we gain experience with this.

Thank you, I love this answer.

[DenisYaroshevskiy]

@ldionne

If your compiler team makes a decision that you disagree with w.r.t. adding checks, you should take it up to them.

You know it is not that simple. If the check exists - there will be a holy war around it. And if you change jobs, you’d have to fight it again. All the while you get more and more of custom rewrites and blog posts around the internet about how standard library is slow and you should not use it.

It is also important what the industry does. If @ kcc gets his way and Google enables this everywhere - the fight against enabling this will escalate another two levels.

On top of it - if you put bounds checks everywhere in std::sort or std::find - how long do you think it will take people to write their own sort or find? Including inside the projects that care about security. Is that what we want - people rewriting standard library?

[AaronBallman]

@jankorous

I’ll respond to the individual points below but let me start with another way how to phrase our intentions.

Thank you for the extra details!

My current thinking (subject to change) is that this sounds like a better fit for clang-tidy than for the compiler proper. It’s not that this isn’t an important problem to solve, so let me be clear on that. I’ve written a few books on secure coding in C and C++, I’m aware of the importance and the difficulties with what you’re trying to do and I very much appreciate the efforts in this space. It’s more a matter of finding a good home for the idea.

Concretely, my questions and concerns are:

• Let me make sure we’re both on the same page about the kind of code being diagnosed. This will not only warn about code like int *ip = foo(); int *next = ip + 1; it will also be warning about code like int array[10]; int &val = array[2]; (because array access is pointer arithmetic and you want people to use STL interfaces rather than C arrays) and code like (const char *)(this + 1) (for accessing a tail-allocated buffer in C++)? I’ve been assuming you intend to warn on all of these.
• What you are proposing is experimental and we want to avoid experimenting in the compiler. Are you aware of any other production compiler or static analyzer which implements what you’ve proposed? I’m not aware of any myself, which is why I claim this is experimental.
• You (correctly, IMO) point out that the changes to existing code under this model have a reasonable likelihood of requiring massive effort to address. There’s no way these diagnostics will be enabled by default under the model described, and off-by-default warnings have a higher bar for inclusion in Clang to some extent (we need evidence they’re worth the cost). If we’re going to need to make users take special steps to enable the behavior, is putting it in a different tool a problem?
• Are you expecting to propose something to WG14 or WG21? (Or MISRA?) I don’t see a path forward towards standardization of this idea, so I was assuming you weren’t looking in that direction, but if you are, that would be good to know more details about.
• We try to avoid creating language variants and this sort of bumps up against that line. It’s not a nonconforming extension (the proposal is to add warnings not errors, if I understand correctly), but when it’s enabled it requires the user to make adjustments to well-defined C and C++ code that is pretty fundamental to the way the language works. So this isn’t like adding fallthrough warnings where we expect the user to annotate branches with explicit fallthrough. This is more like adding an implicit nodiscard to every non-void returning function unless the user adds a special marking – it fundamentally changes the way in which you can use the language. So it’s sort of a variant and sort of not a variant. FWIW, I think those special markings are reasonable for C++ and not for C. For C++, the user has other tools in the toolbox they can use to get improved security. For C, the user has no such tools – there’s no way to escape pointer arithmetic in C so I don’t see anyone enabling this feature for C code in practice (they’d have to add a bunch of pragmas just to get their already-well-defined code back to compiling without warnings, and there’s no benefit to the diagnostic being enabled at that point).

Currently, it sounds like the safe buffers part of the proposal isn’t meeting the usual expectations for adding an extension to Clang (Clang - Get Involved), which is why I am thinking it may be a more reasonable feature for clang-tidy instead. We have more wiggle room to add disruptive (in a good way!) checks in that tool, and the tool still supports what I think you’ll need for the feature (AST matchers should allow you to locate pointer arithmetic you want to flag, it has built in support for fix-it hints, and it integrates nicely into CI pipelines so it shouldn’t be overly onerous for users to enable when they want to opt into it).

Regardless of where it lives, I think it’s good to keep discussing the behavior of what you have in mind. A few observations:

• Does it make sense to enable safe buffers in older C++ standards that may not have the tools required for a replacement, like C++98/C++03?
• What constitutes an “unsafe operation on a raw pointer”? Is this + 1 unsafe? Is AnyKindOfPointer + 0 unsafe? Are pointer operations with constant offsets that are known to be within bounds of a constant sized array unsafe? How do you plan to handle interfaces the user has no control over, like accessing argv from main()?
• You mention pragmas and that sets off my spidey senses – what do you have in mind for how those pragmas work in practice? Are there a pair of pragmas so there’s an explicit range of code covered by it? Or is it a pragma that is bounded to a particular scope (compound blocks)? File-level only?
• Are you planning on diagnosing code in unevaluated contexts as well, or are those fine because there’s no actual pointer arithmetic happening? e.g., decltype(some_int_ptr + 1)
• What is your plan for macros? e.g.,

#define MACRO(x) + x
#define IS_THIS_BAD(x, y) (x + y)

int *ptr = foo();
int *next = ptr MACRO(1);

int *other = IS_THIS_BAD(ptr, 1);
int but_then_again = IS_THIS_BAD(1, 1);

[jankorous]

Thank you for the detailed response!

The crucial difference between having the diagnostics emitted by clang itself compared to any other tool is the ability to guarantee the security properties we need. External tools might be a perfect fit for opportunistic bug-finding but are insufficient for a strong security model that is practically auditable. Security of a project (represented by conformance to our programming model) must be inferable from the codebase itself and we can’t include external tools (including their configuration and infrastructure) in this model.

Let me make sure we’re both on the same page about the kind of code being diagnosed. This will not only warn about code like int *ip = foo(); int *next = ip + 1; it will also be warning about code like int array[10]; int &val = array[2]; (because array access is pointer arithmetic and you want people to use STL interfaces rather than C arrays) and code like (const char *)(this + 1) (for accessing a tail-allocated buffer in C++)? I’ve been assuming you intend to warn on all of these.

Yes, we want the bounds to be checked for all of these.

What you are proposing is experimental and we want to avoid experimenting in the compiler. Are you aware of any other production compiler or static analyzer which implements what you’ve proposed? I’m not aware of any myself, which is why I claim this is experimental.

Yes, this is a novel idea but not an open-ended experiment. We have a direction and while not all of the individual steps are clear at this point we know where we are going. We have done extensive amounts of experimentation with this concept internally and for this approach in particular we’ve verified its sanity against multiple different internal codebases.

You (correctly, IMO) point out that the changes to existing code under this model have a reasonable likelihood of requiring massive effort to address. There’s no way these diagnostics will be enabled by default under the model described, and off-by-default warnings have a higher bar for inclusion in Clang to some extent (we need evidence they’re worth the cost). If we’re going to need to make users take special steps to enable the behavior, is putting it in a different tool a problem?

Having the diagnostics directly in the compiler allows the programming model to be enforced during the build. It also avoids the AST to be constructed twice (once by clang, once by another tool) which would significantly EDIT:“+regress performance and” impact user experience.

There are many other warnings in clang that are off-by-default. Moving the verification in another tool would significantly impact the security guarantees our model can provide while presumably the maintenance cost is the same. To be clear - we are fully committed to maintain this code.

Are you expecting to propose something to WG14 or WG21? (Or MISRA?) I don’t see a path forward towards standardization of this idea, so I was assuming you weren’t looking in that direction, but if you are, that would be good to know more details about.

We view our proposal as a particular way to use C++ language. Since it is completely within the bounds of C++ language we didn’t think there’s a case for proposal on language level. Could you maybe elaborate more on what your thoughts are here?

We try to avoid creating language variants and this sort of bumps up against that line. It’s not a nonconforming extension (the proposal is to add warnings not errors, if I understand correctly), but when it’s enabled it requires the user to make adjustments to well-defined C and C++ code that is pretty fundamental to the way the language works. So this isn’t like adding fallthrough warnings where we expect the user to annotate branches with explicit fallthrough. This is more like adding an implicit nodiscard to every non-void returning function unless the user adds a special marking – it fundamentally changes the way in which you can use the language. So it’s sort of a variant and sort of not a variant.

We are not changing any aspect of the language itself. Any C++ code conforming to the proposed programming model and relying on hardened libc++ is still trivially compliant with C++ standard.
Regardless of that - the nature of our programming model will not change whether implemented in clang or another tool.

FWIW, I think those special markings are reasonable for C++ and not for C. For C++, the user has other tools in the toolbox they can use to get improved security. For C, the user has no such tools – there’s no way to escape pointer arithmetic in C so I don’t see anyone enabling this feature for C code in practice (they’d have to add a bunch of pragmas just to get their already-well-defined code back to compiling without warnings, and there’s no benefit to the diagnostic being enabled at that point).

While our main focus really is C++ the warnings model is indeed valuable and extendable for C. Vendors do ship higher level C abstractions for buffer management (for example CoreFoundation on Apple platforms) and a model that allows projects to ensure that these are used rather than raw pointer arithmetic could potentially be quite useful.

Currently, it sounds like the safe buffers part of the proposal isn’t meeting the usual expectations for adding an extension to Clang (Clang - Get Involved (Clang - Get Involved)), which is why I am thinking it may be a more reasonable feature for clang-tidy instead. We have more wiggle room to add disruptive (in a good way!) checks in that tool, and the tool still supports what I think you’ll need for the feature (AST matchers should allow you to locate pointer arithmetic you want to flag, it has built in support for fix-it hints, and it integrates nicely into CI pipelines so it shouldn’t be overly onerous for users to enable when they want to opt into it).

Keeping the functionality in clang enables security guarantees, unlocks integration into certain existing workflows and also allows much wider adoption by the virtue of clang having many more users than other tools.

Examples of some other work from the same bucket as our proposal that has been integrated into clang is Nullability, Objective-C modernizer or Objective-C ARC migrator.

What changes to our proposal would make it more acceptable for you?

(I will address the rest of the questions about design details separately.)
EDIT: typos + clarification

[jankorous]

Does it make sense to enable safe buffers in older C++ standards that may not have the tools required for a replacement, like C++98/C++03?

Our primary use-case are C++ codebases that have either adopted C++20 or able to do so. At the same time we do believe that the programming model (without the code transformations) still makes sense not only for older C++ standards but also for C.

What constitutes an “unsafe operation on a raw pointer”? Is this + 1 unsafe? Is AnyKindOfPointer + 0 unsafe? Are pointer operations with constant offsets that are known to be within bounds of a constant sized array unsafe?

Generally we prefer reliability at the expense of being more intrusive. Specifically, pointer + 0 doesn’t look harmful but this + 1 and constant offsets to constant sized arrays are more interesting and I imagine we can handle this on case-by-case basis.

How do you plan to handle interfaces the user has no control over, like accessing argv from main()?

The overall idea is to make sure every buffer access is guarded by a bounds-check. What we might possibly do for example for argv[i] is to transform the code like this (ignoring the inner pointer for now):

std::span<char*> argv_sp(argv, argc);
// ...
argv_sp[i]; // bounds-check is supplied by hardened libc++

You mention pragmas and that sets off my spidey senses – what do you have in mind for how those pragmas work in practice? Are there a pair of pragmas so there’s an explicit range of code covered by it? Or is it a pragma that is bounded to a particular scope (compound blocks)? File-level only?

We are tentatively planning to go with the former - pair of pragmas wrapped in macros for nicer syntax.
We’ve have discussed this quite a bit internally and our current thinking is a pair of pragmas to opt-out of the programming model (escape hatch) and a pair of pragmas to opt-in (to enable incremental adoption).

Possibly UNSAFE_BUFFERS_USAGE_BEGIN, UNSAFE_BUFFERS_USAGE_END and ONLY_SAFE_BUFFERS_BEGIN, ONLY_SAFE_BUFFERS_END. We should very likely implement additional diagnostics for these - for example to prevent nesting or leaving open scope at the end of a header file.

I do feel your spidey senses and would appreciate all your thoughts on this. It would help us design this mechanism in a safe and ergonomic way.

Are you planning on diagnosing code in unevaluated contexts as well, or are those fine because there’s no actual pointer arithmetic happening? e.g., decltype(some_int_ptr + 1)

That’s a really good point - I don’t think we should emit diagnostics if we can prove that the expression is not evaluated.

What is your plan for macros?

Our analysis will work on the AST so it will see macros as already expanded. It makes sense to emit warnings for such code but it is unlikely we would be able to provide fixits.

[fhahn]

@DenisYaroshevskiy

I don’t see why would this be a good idea.
The performance overhead is real and is easily measured: Quick C++ Benchmarks

Apparently optimising it out is not something current compilers can do.
While I appreciate the security concern - introducing a 20% overhead everywhere for security goes against the language.

I wouldn’t focus too much on performance for now. Yes, LLVM/Clang currently doesn’t optimize this example, but there are improvements in the works to improve optimizations in that direction and all the information needed is available in the IR. Once hardened APIs become more widely used I expect optimizations in LLVM/Clang will catch up for the most important cases. Especially if libc++ code can also be adjusted to make it more optimization friendly.

It would be good to collect missed optimizations as issues on Github.

[AaronBallman]

@jankorous

Yes, this is a novel idea but not an open-ended experiment. We have a direction and while not all of the individual steps are clear at this point we know where we are going. We have done extensive amounts of experimentation with this concept internally and for this approach in particular we’ve verified its sanity against multiple different internal codebases.

Please share concrete details about the extension experimentation you’ve done, if you can. Specifically, I’d like to know:

• how much code was analyzed?

  • was the code predominately existing code, new code, or a mixture of both?
  • were the projects varied in terms of pointer arithmetic usage (e.g., did you test video games and word processors and compilers, or did you test only networking code?)
  • how many diagnostics were issued per project (roughly)? Any idea of what % of overall code was diagnosed?

• how did mitigation work in practice?

  • what percentage of diagnosed code was using dangerous pointer arithmetic?
  • what percentage of mitigation attempts ended up introducing new vulnerabilities?
  • what was user reaction to the diagnostics?

The reason why I still think this is experimental is because I’m not seeing details about what’s worked well, what didn’t work well and needed to be changed, and why this approach is the right approach.

@jankorous

We view our proposal as a particular way to use C++ language. Since it is completely within the bounds of C++ language we didn’t think there’s a case for proposal on language level. Could you maybe elaborate more on what your thoughts are here?

You are subsetting both C and C++ and creating a new language variant in which pointer arithmetic does not exist (at least in terms of the programming model you want developers to use in this mode). What body of folks are defining what that means and how it works? (Basically, where’s the specification for how this feature works? Is that specification your own invention of a Clang-specific feature, or is this work expected to span compilers/tools as some sort of portable idea?)

@jankorous

We are not changing any aspect of the language itself. Any C++ code conforming to the proposed programming model and relying on hardened libc++ is still trivially compliant with C++ standard.
Regardless of that - the nature of our programming model will not change whether implemented in clang or another tool.

You’re proposed model is subsetting the language. Conforming C++ code will be diagnosed as not conforming to your model. That is a kind of language variant.

@jankorous

Examples of some other work from the same bucket as our proposal that has been integrated into clang is Nullability, Objective-C modernizer or Objective-C ARC migrator.

Objective-C is its own language with its own kind of governing body, so the ObjC extensions don’t really persuade me. (FWIW, I also think ARC rewriting is an example of something that should perhaps be considered for removal from in tree because it’s causing maintenance issues as C evolves: https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTContext.cpp#L4285).

I think nullability qualifiers are a similar extension to what you’re proposing, but they’re a feature which can be incrementally adopted by a code base through addition of the qualifiers to existing code and reacting to the resulting diagnostics. In this proposal, there’s no incremental adoption possible – you enable the diagnostics, you get all of the warnings across the entire code base. Also, there was a path towards standardization for the nullability qualifiers that I don’t think exists for this proposal.

@jankorous

What changes to our proposal would make it more acceptable for you?

It needs to meet the bar for adding an extension to Clang, and I don’t believe it does currently. Specifically:

1. Without usage details and with it being an off-by-default warning, it fails to show evidence of a significant user community that will use the functionality.
2. I understand that it would be better from your perspective to have the check in the compiler because separate tools come with problems and being in the compiler means wide availability, but that doesn’t really address #2 as to why those problems are insurmountable. Also, one of the authors of the proposal mentioned it wasn’t a hard requirement that this functionality live within the compiler.
3. There is no specification for this, so #3 is missing entirely.
4. #4 may or may not matter; you don’t have to have this in front of a standards body for it to be included in Clang (as an example, CUDA has no actual standard but we still support it). However, there are security standards for C and C++ both in terms of ISO standards and industry de facto standards, so there are organizations that could be weighing in on the design of this feature.
5. I think #5 is reasonably covered in terms of a long-term support plan (as covered as it can be).
6. In terms of implementability within Clang itself, I don’t think #6 is an issue. The code to do what you want should be somewhat straightforward (at least at a high level). However, #6 is also about the user experience and that might still be a problem. I don’t see any way to incrementally use this feature, it seems like enabling it for an existing code base will overwhelm the user with diagnostics that aren’t actual issues in the code (except that the code doesn’t match your model), automatic fix-its aren’t guaranteed to leave the user’s code in a working state, etc.
7. I have confidence in your ability to meet the requirements for #7, so I don’t think that’s an issue.

If you think I’ve misunderstood something about your proposal or there’s something about my concerns you’re not understanding, I’m happy to meet with you to discuss (Zoom, Meet, WebEx, whatever works for you) if you think that’d be fruitful.

[tschuett]

Maybe the out-of-range checks could be builtins and lower to llvm intrinsics in the libc++ namespace.
llvm.libcxx.outofrange...to give the optimiser more information.

[thesamesam]

I don’t think this really applies in terms of concerns about splits because libstdc++ has had _GLIBCXX_ASSERTIONS (which is distinct from _GLIBCXX_DEBUG) for years which Fedora uses for almost all package builds, but nobody is required to use it for their own development or projects.

Gentoo is evaluating its use for hardened builds too.

I don’t see a reason why libcxx shouldn’t explore adding similar behaviour under a toggle.

Those concerned about fragmentation are proving that it’s in fact not an issue, as they hadn’t realised it existed for libstdc++ all these years, and it didn’t lead to C++'s ruin.

[aaronpuchert]

@jankorous

Under this programming model the compiler will make sure that the only pointer arithmetic in the code is done by libc++ which combined with its hardened mode will guarantee bounds-checks before memory access.

That seems to be saying that the C++ standard library has all the containers that one might ever need, which not even we as LLVM community can seem to agree on, given that we have several low-level data structures (like SmallVector) that can’t be expressed in terms of the C++ standard library. I think you shouldn’t be drawing a distinction on the library level, but rather allow pointer arithmetic based on the existence of some markers similar to Rust’s unsafe. I have a hard time believing that anything but small pet projects are able to exclusively rely on the standard library’s abstractions without any of their own low-level data structures.

@jankorous

External tools might be a perfect fit for opportunistic bug-finding but are insufficient for a strong security model that is practically auditable. Security of a project (represented by conformance to our programming model) must be inferable from the codebase itself and we can’t include external tools (including their configuration and infrastructure) in this model.

What does this have to do with clang vs clang-tidy? Why can you run clang for an audit but are not able to run clang-tidy? What makes one an internal and the other an external tool?

I have to say that I fully agree with @AaronBallman here: pointer arithmetic isn’t per se dangerous and cannot really be avoided. You can only encapsulate it into libraries. So using or not using it is a question of idiom and dialect, which is exactly what clang-tidy exists for. It’s true that there are plenty of off-by-default warnings, but this is one that can by definition never be enabled globally because at some places you always have to do pointer arithmetic. Contrast this with the lifetime annotations that the authors want to put into clang-tidy despite the fact that it might actually be enabled globally some day (including low-level libraries).

[aaronpuchert]

@waffles_the_dog

Where are you getting “really scarce”?

There have been innumerable exploits from out of bounds accesses over the years, and those are just the ones we know about as they were used in actual attacks rather than misc crashes/incorrect behaviour potentially long after the error.

Are there actual statistics for (reasonably modern) C++? Of course there have been plenty of CVEs with buffer overflows, but the vast majority of them seem to have been in C code. We’ve been starting to categorize bugs on our pretty large C++ code base a while ago, and have around 2.5% of crash/UB bugs due to out-of-bounds accesses. (We don’t separately count security issues.) Most of them have been in older code that is still closer to C than C++.

It seems totally legitimate to me to question whether a number of abstractions enabled by modern C++ aren’t already good enough. Do we have to completely forbid any kind of pointer arithmetic (and thus custom abstractions)? Does every memory access have to be bounds-checked (in production no less), even if it’s obvious from the code that everything is fine?

@waffles_the_dog

We have a lot of evidence to show that “zero bugs” is not feasible

Of course you can prove formal correctness of a program. It’s certainly not feasible for every kind of software, but especially in the area of security-critical software there are a number of projects out there that went through the hoops. Of course there are things outside of the model (think side channel attacks), but they are certainly also out of scope for boundary checks.

[aaronpuchert]

@fhahn

I wouldn’t focus too much on performance for now. Yes, LLVM/Clang currently doesn’t optimize this example, but there are improvements in the works to improve optimizations in that direction and all the information needed is available in the IR.

Some issues are not so trivial as this:

• Index vectors (i.e. vectors that contains indices to another vector) are problematic, because we’d need the compiler to realize that all integers in some arbitrarily large array are bounded.
• Because char can alias everything, things like std::vector<char> (and I think also std::string) can make it hard to hoist boundary checks, because the character buffer can alias with everything, including, sadly, the fields of std::vector/std::string itself that store the size or end pointer. This might get tricky.

[aaronpuchert]

@thesamesam

I don’t think this really applies in terms of concerns about splits because libstdc++ has had _GLIBCXX_ASSERTIONS (which is distinct from _GLIBCXX_DEBUG) for years which Fedora uses for almost all package builds, but nobody is required to use it for their own development or projects.

That’s not hardcoded into libstdc++ though, every user can decide for themselves whether to enable it. That’s different from what is being proposed here:

@jankorous

The intent is that a vendor shipping libc++ on their platform can decide to pick which checks to enable in the library that they ship

So a given binary version of libc++ will either always have the checks on or not. @ldionne went into some details later. I guess the difficulty arises from wanting to have everything boundary checked, versus libstdc++'s “best effort” assertions that don’t guarantee the standard library to be fully safe against overruns.

[ldionne]

@aaronpuchert

I guess the difficulty arises from wanting to have everything boundary checked, versus libstdc++'s “best effort” assertions that don’t guarantee the standard library to be fully safe against overruns.

The main difficulty in fact is that some of the checks we want to implement require an ABI change. ABI-affecting properties need to be pinned down by vendors at configuration time because all the software in the stack needs to be built using the same ABI. Any check that is not ABI-affecting in our proposal will still be controllable by users regardless of how libc++ has been configured in the first place.