1

I'm trying to replace the global new operator in Visual Studio and C++. This is my code (only one new operator shown for simplicity):

void* operator new(size_t _Size)
{
    // Do something
}

It works fine, however Visual Studio is giving me a warning when running Code-Analysis for my project:

warning C28251: Inconsistent annotation for 'new': this instance has no annotations. The first user-provided annotation on this built-in function is at line vcruntime_new.h(48).

Using the annotations from vcruntime_new.h for my operator new replacement, as suggested in the warning from IntelliSense, resolves the warning:

_NODISCARD _Ret_notnull_ _Post_writable_byte_size_(_Size) _VCRT_ALLOCATOR
void* __cdecl operator new(size_t _Size)
{
    // Do something
}
  1. Is it safe to use the annotations inside vcruntime_new.h for my own replacement code like shown above?
  2. What are the consequences of this change?
  3. Are there special use cases were the "new operator" cannot be used anymore like before because of the annotations?
  4. And why is that change necessary?

EDIT:

  1. Am I correct, that the annotations won't change anything in the resulting binary, and are simply for static code analysis? (Except __cdecl, which changes the assembler, but it should be standard anyway I guess?)
Marc
  • 338
  • 2
  • 15
  • You are getting this diagnostic because you aren't actually trying to overload anything. This is an attempt at overriding the global `operator new`. – IInspectable Jul 29 '21 at 06:39
  • Yes I meant overriding, or more accurate "replacing". Just a wording error. – Marc Jul 29 '21 at 07:15
  • There are two types of annotations here: [SAL](https://learn.microsoft.com/en-us/cpp/code-quality/understanding-sal) and the calling convention. The calling convention (`__cdecl`) is mandatory for proper functioning. The SAL annotations are used by static code analysis only. Instead of copying them all over, you could assign the [`_Use_decl_annotations_`](https://stackoverflow.com/q/24426875/1889329) attribute instead. – IInspectable Jul 29 '21 at 08:14

1 Answers1

0

This applies to _Ret_notnull_ and _Post_writable_byte_size_(_Size):

Is it safe to use the annotations inside vcruntime_new.h for my own replacement code like shown above?

Yes, as long as your operator new actually follow these rules in annotation. It might not follow _Ret_notnull_ (for example non-throwing new returns nullptr), tough normally will. But it should follow _Post_writable_byte_size_(_Size), since operator new is required so.

What are the consequences of this change?

Annotations will probably help Visual Studio Code Analysis pinpoint errors, but will make you using non-standard stuff, making your program less portable and less clear.

Are there special use cases were the "new operator" cannot be used anymore like before because of the annotations?

No.

And why is that change necessary?

Because you want to avoid that warning instead of silencing it.

Am I correct, that the annotations won't change anything in the resulting binary, and are simply for static code analysis? (Except __cdecl, which changes the assembler, but it should be standard anyway I guess?)

Correct. Not even optimizer would use that (optimizer would use __assume, __restrict, and other annotations separate from static analysis).

Alex Guteniev
  • 12,039
  • 2
  • 34
  • 79
  • Thanks. I added another question to my post (5). I think your last sentence answered that too, it's simply for the static code analyzer and wont change the resulting binary? – Marc Jul 29 '21 at 11:43
  • @MarcS , I'm not sure about `_VCRT_ALLOCATOR`. It resloves to `__declspec(allocator)`, see here https://learn.microsoft.com/en-us/cpp/cpp/allocator?view=msvc-160 – Alex Guteniev Jul 29 '21 at 12:05
  • *"as long as your operator `new` actually follow these rules in annotation"* - SAL annotations work both ways. They help static code analyzers verify correctness of client code using an interface, but they also apply to the implementation. If the implementation fails to establish the promises made, then that will be flagged as well. – IInspectable Jul 29 '21 at 21:52