Report inlining location when using __attribute__((__warning__(msg)))

[kees]

When __warning__(msg) reports from usage within an inline, there's no information about where the inline was used. For example:

gcc says inlined from 'main' at /app/example.c

In file included from /app/example.c:5:
In function 'indirection',
    inlined from 'main' at /app/example.c:9:5:
example.h:6:9: error: call to '__write_overflow' declared with attribute warning: eek [-Werror=attribute-warning]
    6 |         __write_overflow();
      |         ^~~~~~~~~~~~~~~~~~

Clang doesn't include that hint:

In file included from /app/example.c:5:
example.h:6:9: error: call to __write_overflow declared with 'warning' attribute: eek [-Werror,-Wattribute-warning]
        __write_overflow();
        ^

https://godbolt.org/z/W9PjxsE6z

Which makes FORTIFY_SOURCE warnings much harder to find. :)

In file included from net/mptcp/protocol.c:10:
In file included from ./include/linux/module.h:13:
In file included from ./include/linux/stat.h:19:
In file included from ./include/linux/time.h:60:
In file included from ./include/linux/time32.h:13:
In file included from ./include/linux/timex.h:65:
In file included from ./arch/x86/include/asm/timex.h:5:
In file included from ./arch/x86/include/asm/processor.h:22:
In file included from ./arch/x86/include/asm/msr.h:11:
In file included from ./arch/x86/include/asm/cpumask.h:5:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:11:
In file included from ./include/linux/string.h:253:
./include/linux/fortify-string.h:230:4: warning: call to __write_overflow_field declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
                        __write_overflow_field(p_size_field, size);
                        ^

[nickdesaulniers]

I think this is what diagnose_as_builtin fn attr is meant to solve.
https://reviews.llvm.org/D112024
cc @gburgessiv

[kees]

I think this is what diagnose_as_builtin fn attr is meant to solve.

In what way?

[nickdesaulniers]

Hmm...I guess not.

A similar problem exists with inline asm:

// $(CC) -O2
void __write_overflow(void) __attribute__((warning("eek")));

static inline void indirection(int x) {
  if (__builtin_constant_p(x) && x > 10)
    __write_overflow();
  asm("# %0"::"i"(x));
}

void foo (int y) {
  indirection(42);
  indirection(y);
}

in this case, the call to indirection with an integer constant expression is valid (and only due to inlining via optimizations being enabled), but the call without is not. GCC doesn't give precise info about which statement is problematic, but seems to have the infrastructure to do so. If we can implement similar behavior for the warnings attribute, then we likely can improve the case for asm, too.

[kees]

Until this is solved correctly in Clang, Nick has suggested a work-around can be used to help identify the inlining location for tracking down specific warnings with KCFLAGS=-Rpass=inline

[nickdesaulniers]

Reduced test case showing ambiguity between possible call sites:

__attribute__((__warning__("foo")))
void foo(void);

static inline void bar(int x) {
    if (x == 10)
        foo();
}

void baz() {
    bar(10);
}

void quux() {
    bar(9);
}

Clang:

<source>:6:9: warning: call to foo declared with 'warning' attribute: foo [-Wattribute-warning]
        foo();
        ^

GCC:

In function 'bar',
    inlined from 'baz' at <source>:10:5:
<source>:6:9: warning: call to 'foo' declared with attribute warning: foo [-Wattribute-warning]
    6 |         foo();
      |         ^~~~~

[nickdesaulniers]

Even without inlining info, I think we can at least print/encode the "In function 'bar'" part. (Though the inlined from part is perhaps the more necessary part)

[nickdesaulniers]

I think we could add an IR Fn Attr to track "calls a dontwarn function" which is propagated during inline to the caller, at which point we record in the metadata the inlining chain. Though if we DCE the call, then we would have to remove the caller's fn attr.

[nickdesaulniers]

PropagateCallSiteMetadata in /llvm-project/llvm/lib/Transforms/Utils/InlineFunction.cpp might be a good place. Or more generally in llvm::InlineFunction

[nickdesaulniers]

InlineFunction.cpp tracks struct ClonedCodeInfo.ContainsCalls. If that's set, we can scan the calls for calls to "dontcall-warn" attributed callees.

[nickdesaulniers]

diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 6aaaa33cebde..ad6cc5b6232d 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2463,6 +2463,13 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
         // inlining, commonly when the callee is an intrinsic.
         if (MarkNoUnwind && !CI->doesNotThrow())
           CI->setDoesNotThrow();
+
+        if (CI->getCalledFunction()->hasFnAttribute("dontcall-error") ||
+            CI->getCalledFunction()->hasFnAttribute("dontcall-warn")) {
+          MDString *VAM = MDString::get(CI->getContext(), CalledFunc->getName());
+          MDTuple *MDT = MDNode::get(CI->getContext(), {VAM});
+          CI->setMetadata("inlined-from", MDT);
+        }
       }
     }
   }

will record in the IR when we inline a call that is to one of the attributed functions:

define dso_local void @baz() #0 {
entry:
  call void @foo() #2, !srcloc !5, !inlined-from !6
  ret void
}
...
!6 = !{!"bar"}

though the metadata should be a list, since we might have multiple levels of inlining. We should probably first check if such metadata exists, and update it if so.

[nickdesaulniers]

ok, this produces a metadata of the mangled fn name, or appends a new value to the list if there's such existing metadata:

diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 6aaaa33cebde..56a92f57ebee 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2463,6 +2463,18 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
         // inlining, commonly when the callee is an intrinsic.
         if (MarkNoUnwind && !CI->doesNotThrow())
           CI->setDoesNotThrow();
+
+        if (CI->getCalledFunction()->hasFnAttribute("dontcall-error") ||
+            CI->getCalledFunction()->hasFnAttribute("dontcall-warn")) {
+          Metadata *MD = MDString::get(CI->getContext(), CalledFunc->getName());
+          if (MDNode *N = CI->getMetadata("inlined-from")) {
+            TempMDTuple Temp = cast<MDTuple>(N)->clone();
+            Temp->push_back(MD);
+            MD = MDNode::replaceWithUniqued(std::move(Temp));
+          }
+          MDTuple *MDT = MDNode::get(CI->getContext(), {MD});
+          CI->setMetadata("inlined-from", MDT);
+        }
       }
     }
   }

[nickdesaulniers]

https://reviews.llvm.org/D141451

[kees]

D141451 doesn't seem to cross compilation units?

$ cat wat.h
extern void __real_foo(void) __attribute__((__warning__("__real_foo")));

static inline void foo(void)
{
        __real_foo();
}

$ cat wat.c
#include "wat.h"

void bar() {
    foo();
}

$ clang -Wall -c -o wat.o wat.c
In file included from wat.c:1:
./wat.h:6:2: warning: call to __real_foo declared with 'warning' attribute: __real_foo [-Wattribute-warning]
        __real_foo();
        ^
1 warning generated.