[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D141451#4258031 <https://reviews.llvm.org/D141451#4258031>, @aaron.ballman wrote:

> Have you checked to see how the diagnostics work in practice on deep inlining stacks? If it's usually only 1-2 extra notes, that's probably not going to overwhelm anyone. But if it's significantly more, that could be distracting. Do we want any options to control when to emit the notes? e.g., do we want to give users a way to say "only emit the last N inlining decision notes"?

I don't have a good means of aggregating such data, but in my kernel experience we only ever emit about 2-4 notes before we stop inlining.  It is usually much less than the inclusion chain of headers reported.  Example (I've manually added a bad memcmp to kernel sources):

  In file included from drivers/nfc/nfcmrvl/fw_dnld.c:8:
  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:67:
  In file included from ./arch/x86/include/asm/timex.h:5:
  In file included from ./arch/x86/include/asm/processor.h:23:
  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:254:
  ./include/linux/fortify-string.h:661:4: error: call to '__read_overflow2' declared with 'error' attribute: detected read beyond size of object (2nd parameter)
                          __read_overflow2();
                          ^
  note: In function 'fw_dnld_rx_work'
  note:   which inlined function 'process_state_reset'
  note:   which inlined function 'memcmp(void const*, void const* pass_dynamic_object_size0, unsigned long pass_dynamic_object_size0)'
  1 error generated.

Can you construct come "cuckoo-bananas" cases with this though? Yes: https://godbolt.org/z/9eP8vjeb8

> Also, how does this work with LTO when it makes different inlining decisions?

The concern with LTO is less about a change in inlining decisions, and more so about loss of fidelity with corresponding SourceLocations.

Testing the same kernel code same above, but with LTO enabled, we simply get a backend diagnostic from LLD, since the sources and thus SourceLocations are not retained:

  ld.lld: error: call to __read_overflow2 marked "dontcall-error": detected read beyond size of object (2nd parameter)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141451/new/

https://reviews.llvm.org/D141451