littlekernel / lk

LK embedded kernel
MIT License
3.11k stars 613 forks source link

Add -fcheck-new to c++ flags #326

Open travisg opened 2 years ago

travisg commented 2 years ago

Depending on when it was added to gcc and/or if clang supports it, should add this to the c++ flags since its more realistic that heap allocations can fail in an embedded environment.

heatd commented 2 years ago

FWIW the latest LLVM release, 14.0.0, does not support it. Although someone should really add it to clang so that standard new could be easier used in embedded programming.

travisg commented 2 years ago

Agreed. Bummer they don't. As it is, there are a handful of places in LK that use new and then test for null and indeed it does not actually result in any real test. Probably worth adding even if it's not properly supported in clang.

heatd commented 7 months ago

Supported since LLVM 17 (https://github.com/llvm/llvm-project/commit/52c8f0bb20eb9e7b1b54ffdddf6da77b53caeb3a)

travisg commented 3 months ago

Probably worth conditionally adding, now that somewhere along the line some build system support was added for checking an individual flag.

heatd commented 3 months ago

Probably worth conditionally adding, now that somewhere along the line some build system support was added for checking an individual flag.

FWIW, conditionally adding this is tricky (and not really needed?). clang "supported" this flag for ages (aka -fcheck-new was silently ignored, as are a bunch of other GCC-specific flags). You should be able to just add -fcheck-new and hope the compiler does implement it