Re: [GIT PULL] ext4 changes for the 6.4 merge window

[Nick Desaulniers]

On Wed, Apr 26, 2023 at 11:33 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Apr 26, 2023 at 11:22 AM Nick Desaulniers
> <ndesaulniers@xxxxxxxxxx> wrote:
> >
> > Ah, it does do something in the callee, not the caller:
>
> Ack, it does seem to have _some_ meaning for the return case, just not
> the one we'd be looking for as a way to find mistakes in the
> error-pointer case.

Is this what you had in mind?
```
$ cat linus.c
#define NULL ((void*)0)

void * _Nonnull foo (void) {
    return &foo;
}

void bar (void) {
    if (foo() == NULL) // maybe should warn that foo() returns _Nonnull?
        bar();
}
$ clang linus.c -fsyntax-only
linus.c:8:15: warning: comparison of _Nonnull function call 'foo'
equal to a null pointer is always false
[-Wtautological-pointer-compare]
    if (foo() == NULL) // maybe should warn that foo() returns _Nonnull?
              ^
linus.c:3:1: note: return type has '_Nonnull' nullability attribute
void * _Nonnull foo (void) {
^
1 warning generated.
```

Quick PoC, obviously incomplete.
```
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 18a0154b0041..10e405b1cf65 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3975,8 +3975,9 @@ def note_xor_used_as_pow_silence : Note<
   "replace expression with '%0' %select{|or use 'xor' instead of '^'
}1to silence this warning">;

 def warn_null_pointer_compare : Warning<
-    "comparison of %select{address of|function|array}0 '%1' %select{not |}2"
-    "equal to a null pointer is always %select{true|false}2">,
+    "comparison of %select{address of|function|array|_Nonnull function call}0 "
+    "'%1' %select{not |}2equal to a null pointer is always "
+    "%select{true|false}2">,
     InGroup<TautologicalPointerCompare>;
 def warn_nonnull_expr_compare : Warning<
     "comparison of nonnull %select{function call|parameter}0 '%1' "
@@ -3992,6 +3993,8 @@ def warn_address_of_reference_null_compare : Warning<
   "code; comparison may be assumed to always evaluate to "
   "%select{true|false}0">,
   InGroup<TautologicalUndefinedCompare>;
+def note_return_type_nonnull :
+  Note<"return type has '_Nonnull' nullability attribute">;
 def note_reference_is_return_value : Note<"%0 returns a reference">;

 def note_pointer_declared_here : Note<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index f66eb9fcf13d..9f6d326f5b72 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13176,6 +13176,22 @@ static void AnalyzeImpConvsInComparison(Sema
&S, BinaryOperator *E) {
 ///
 /// \param E the binary operator to check for warnings
 static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
+  if (auto Call = dyn_cast<CallExpr>(E->getLHS())) {
+    QualType RetType = Call->getCallReturnType(S.Context);
+    if (std::optional<NullabilityKind> NK = RetType->getNullability()) {
+      if (*NK == NullabilityKind::NonNull &&
+        E->getRHS()->isNullPointerConstant(S.Context,
+
Expr::NPC_ValueDependentIsNotNull)) {
+        std::string result;
+        llvm::raw_string_ostream os(result);
+        Call->getDirectCallee()->getNameForDiagnostic(os,
S.getLangOpts(), true);
+        S.Diag(E->getExprLoc(), diag::warn_null_pointer_compare) << 3 <<
+          result << true;
+        S.Diag(Call->getDirectCallee()->getReturnTypeSourceRange().getBegin(),
+               diag::note_return_type_nonnull);
+      }
+    }
+  }
   // The type the comparison is being performed in.
   QualType T = E->getLHS()->getType();


```


-- 
Thanks,
~Nick Desaulniers