[llvm-dev] Change in -Wnonnull in gcc-11

[Luke Benes via llvm-dev]

In gcc-11, -Wnonnull considers the implicit this argument of every C++ nonstatic member function to have been implicitly declared with attribute nonnull and triggers warnings for calls where the pointer is null. [1]

Should clang also do this? This change helped webkit identify and fix some issues. [2][3][4][5]

Building clang trunk with gcc-11 produces 13 new warnings related to this enhancement. They are in MachineModuleInfo.cpp and RangeAdapterTest.cpp.[6] Do these look like valid issues?

-Luke

[1] https://gcc.gnu.org/gcc-11/changes.html\
[2] https://bugs.webkit.org/show_bug.cgi?id=224838
[3] https://bugs.webkit.org/show_bug.cgi?id=224826
[4] https://trac.webkit.org/changeset/276343/webkit
[5] https://bugs.webkit.org/show_bug.cgi?id=224452
[6] https://pastebin.com/EJTZphGm

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[David Blaikie via llvm-dev]

Sure, those things seem legit - I think we have some sanitizer mode (& some optimizations, when the sanitizer checks aren't enabled) for checking non-null-ness in various cases, which probably would hit these cases (though given that I think various sanitizer self host builds are checked regularly, I wonder if there's an interesting explanation for why the cases in [6] were not found already by runtime analysis like that) - probably makes sense to catch them at compile time where we can. Though clang's warnings aren't interprocedural like it looks like some of these warnings in [6] are, so clang might still not catch all of them.

[James Y Knight via llvm-dev]

The first warning looks valid, although it doesn't happen at runtime.

1. In constructor ‘llvm::MachineModuleInfo::MachineModuleInfo(const llvm::LLVMTargetMachine*)’,
2. inlined from ‘llvm::MachineModuleInfoWrapperPass::MachineModuleInfoWrapperPass(const llvm::LLVMTargetMachine*)’ at /llvm/lib/CodeGen/MachineModuleInfo.cpp:357:26,
3. inlined from ‘llvm::Pass* llvm::callDefaultCtor() [with PassName = llvm::MachineModuleInfoWrapperPass]’ at /llvm/include/llvm/PassSupport.h:80:76:
4. /llvm/lib/CodeGen/MachineModuleInfo.cpp:240:51: warning: ‘this’ pointer is null [-Wnonnull]
5. 240 | Context.setObjectFileInfo(TM->getObjFileLowering());

I believe it should be a simple matter of deleting the zero-arg constructor. I'll try that and push it.

I note that the GCC warning points to an odd place, though -- it probably should be pointing to the construction TM(*TM), which initializes a reference with null, which is already UB.

MachineModuleInfo::MachineModuleInfo(const LLVMTargetMachine *TM)

    : TM(*TM), ....

However, the rest of the warnings come from:

  decltype(static_cast<const TypeParam *>(nullptr)->rbegin())>::value

which I think is a false-positive in GCC's warning.

TTBOMK, calling a method on nullptr in an unevaluated context (e.g. decltype, non-vla-sizeof, etc) is not UB, and therefore should not emit a warning.

[David Blaikie via llvm-dev]

On Wed, Sep 1, 2021 at 12:58 PM James Y Knight via llvm-dev <llvm...@lists.llvm.org> wrote:

The first warning looks valid, although it doesn't happen at runtime.

1. In constructor ‘llvm::MachineModuleInfo::MachineModuleInfo(const llvm::LLVMTargetMachine*)’,
2. inlined from ‘llvm::MachineModuleInfoWrapperPass::MachineModuleInfoWrapperPass(const llvm::LLVMTargetMachine*)’ at /llvm/lib/CodeGen/MachineModuleInfo.cpp:357:26,
3. inlined from ‘llvm::Pass* llvm::callDefaultCtor() [with PassName = llvm::MachineModuleInfoWrapperPass]’ at /llvm/include/llvm/PassSupport.h:80:76:
4. /llvm/lib/CodeGen/MachineModuleInfo.cpp:240:51: warning: ‘this’ pointer is null [-Wnonnull]
5. 240 | Context.setObjectFileInfo(TM->getObjFileLowering());

I believe it should be a simple matter of deleting the zero-arg constructor. I'll try that and push it.

I note that the GCC warning points to an odd place, though -- it probably should be pointing to the construction TM(*TM), which initializes a reference with null, which is already UB.

MachineModuleInfo::MachineModuleInfo(const LLVMTargetMachine *TM)

    : TM(*TM), ....

However, the rest of the warnings come from:

  decltype(static_cast<const TypeParam *>(nullptr)->rbegin())>::value

which I think is a false-positive in GCC's warning.

Oh, yeah, if it's in an unevaluated context - agreed, that's a false positive. (though we could probably also just make that code nicer to read by using std::declval<const TypeParam&>().rbegin(), I think?