[clang] Add -fcheck-new support

[heatd]

Summary

Add -fcheck-new and -fno-check-new, from GCC, which make the compiler
not assume pointers returned from operator new are non-null.
Fixes #16931.

[jrtc27]

clang/test/CodeGenCXX/fcheck-new.cpp

3

Do you really want -O2 or do you just want to run mem2reg to eliminate all the alloca noise?

[heatd]

clang/test/CodeGenCXX/fcheck-new.cpp

3

I added -O2 because I was testing with it, since if -fcheck-new doesn't work, it's way more noticeable, as the nullptr check gets optimized out; if it works, the branching is pretty visible and shows exactly what the option does. Also, to eliminate all the noise :) I can definitely remove -O2 though, if you want to.

[jrtc27]

clang/test/CodeGenCXX/fcheck-new.cpp

3

Thinking about this more, I think the branching complicates matters. All that optimisation happens in LLVM IR land, but you're just touching Clang CodeGen. Is int *foo() { return new int; } checking the attributes on the call to _Znwm not enough (and with mem2reg just to clean it up so the function body is trivial)?

Also you should specify a target triple as this will just be LLVM_DEFAULT_TARGET_TRIPLE and change both the type for new (i32 vs i64) and its mangling (j vs m for unsigned int vs unsigned long, and *-windows-msvc is totally different), right?

10

Another advantage of not using -O2 is you lose the TBAA noise

clang/test/Driver/clang_f_opts.c

73

Hmmm

[heatd]

Re-did the test, deleted redundant minus line diff.

[jrtc27]

clang/test/CodeGenCXX/fcheck-new.cpp

2

Not sure why they were checked before, but you'll need this to actually get the attributes checked

[heatd]

clang/test/Driver/clang_f_opts.c

73

Huh, running clang-format on clang_f_opts.c makes it do that. I fixed that up in the new revision but that should possibly be fixed.

[jrtc27]

clang/test/CodeGenCXX/fcheck-new.cpp

3

Not sure how this doesn't need -Xclang -disable-O0-optnone to work?..

[MaskRay]

clang/lib/Driver/ToolChains/Clang.cpp

5984

See addOptInFlag in this file.

clang/test/CodeGenCXX/fcheck-new.cpp

2

We generally only use %clang_cc1 for clang code gen tests. %clang generally is only used for test/Driver tests.

[urnathan]

clang/lib/Sema/SemaDecl.cpp

15440

'invalidates' or 'negates' are better words here.

[heatd]

Adjusted the driver code to use addOptInFlag, adjusted the test, fixed the comment.

[heatd]

In D125272#3504113, @heatd wrote:

Adjusted the driver code to use addOptInFlag, adjusted the test, fixed the comment.

Ping.

[heatd]

In D125272#3515874, @heatd wrote:

In D125272#3504113, @heatd wrote:

Adjusted the driver code to use addOptInFlag, adjusted the test, fixed the comment.

Ping.

Ping.

[heatd]

In D125272#3531836, @heatd wrote:

In D125272#3515874, @heatd wrote:

In D125272#3504113, @heatd wrote:

Adjusted the driver code to use addOptInFlag, adjusted the test, fixed the comment.

Ping.

Ping.

Could someone please review it or tell me what needs to be done before pushing this? Thanks.

[MaskRay]

clang/lib/Frontend/CompilerInvocation.cpp

4117

Use CodeGenOpts<"CheckNew"> and avoid change to this file. CodeGenOpts seems more suitable than LangOpts.

clang/test/CodeGenCXX/fcheck-new.cpp

3

Please remove opt. Optimizations should be tested in the llvm/ layer, not in clang codegen.

If utils/update_cc_test_checks.py does not generate good looking IR, write it by hand.

[jrtc27]

clang/test/CodeGenCXX/fcheck-new.cpp

3

Disagree. This is a pattern that is extremely useful for testing Clang's IR generation whilst cleaning up the alloca so the IR is readable. This is not about the generated checks, this is about alloca/load/store everywhere being a pain to inspect, whereas a simple mem2reg cleans this up.

[heatd]

clang/test/CodeGenCXX/fcheck-new.cpp

3

How should I proceed about this?

Sorry, I haven't thought about this pending change-set for a few months now, oops!

[lebedev.ri]

This review may be stuck/dead, consider abandoning if no longer relevant.
Removing myself as reviewer in attempt to clean dashboard.