ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.19k stars 16.77k forks source link

clang fix needed for the equivalent gcc fix here: https://github.com/ArduPilot/ardupilot/pull/22614 #22615

Open davidbuzz opened 1 year ago

davidbuzz commented 1 year ago

Bug report

new() expects exceptions, and we compile without them, and so when any compiler optimisation level above -O0 is used, godbolt shows that some did-new-return-nullptr checks are elided by the optimisations that shouldn't be.

see the gcc fix here https://github.com/ArduPilot/ardupilot/pull/22614

Please describe the problem

Version What version was the issue encountered with

Platform [ X] All [ ] AntennaTracker [ ] Copter [ ] Plane [ ] Rover [ ] Submarine

Airframe type What type of airframe (flying wing, glider, hex, Y6, octa etc)

Hardware type What autopilot hardware was used? (Pixhawk, Cube, Pixracer, Navio2, etc)

Logs Please provide a link to any relevant logs that show the issue

davidbuzz commented 1 year ago

clang understands check-new but doesn't use it.

davidbuzz commented 1 year ago

the linux boards are the ones usually compiled with clang, so this is mostly gonna afffect them.

davidbuzz commented 1 year ago

... above my paygrade to fix clang. :-)

khancyr commented 1 year ago

why not use C++ "smart pointer" like unique_ptr, this is the normal replacement for new() and is noexcept ?

davidbuzz commented 1 year ago

@khancyr - thanks , but what this issue needs is an actual fix, not random unimplemented suggestion/s. if you have a PR that fixes this issue using 'smart pointer' that would be awesome, but there's probably a number of other ways to fix it too.

peterbarker commented 1 year ago

@khancyr - thanks , but what this issue needs is an actual fix, not random unimplemented suggestion/s. if you have a PR that fixes this issue using 'smart pointer' that would be awesome, but there's probably a number of other ways to fix it too.

No, that wouldn't be awesome.

I loathe the smart pointer stuff.

See https://github.com/ArduPilot/ardupilot/pull/16138

kknives commented 1 year ago

I tried using Clang's new -fno-new-infallible flag, hoping that it would add the null checks for new back again. But no luck. -fno-new-infallible does nothing by itself. -fcheck-new is still a FIXME on clang . I came up with a small test to examine the effects these flags have. Maybe it can be useful in the future.