Thanks @alexfh ! Should be good now.

Address doc + tool-extra issues

As suggested by @aaron.ballman base the detection of top-level-ness on Sema::LookupName to avoid re-implementing the wheel.

ping ? This change looks harmless to me, I'd appreciate an ack though.

Up ?

In D94734#2499865, @yln wrote:

Does the following mean that Python3 is now okay to use for lit?
https://lists.llvm.org/pipermail/llvm-dev/2020-December/147372.html
https://reviews.llvm.org/D93097

First review round. I'm pretty excited by this work, thanks for handling this!

Ignore forward declaration of tagdecl

Update codebase and testbed to reflect recent discussion.

Up?

Address some of the review

Rebased on main.

Rebased on main.

Instead of pretending to be smart, just consider that if the considered decl is at the top level, then it's a top-level decl, otherwise it's not. There may be some interpretation wrt. the standard, but this seems both simple and conservative to me. It gies a few false negative, but it's a warning here, so that should be fine.

LGTM, thanks!

Commited as of 5e31e226b5b2b682607a6578ff5adb33daf4fe39, I totally messed the differential revision :-)

lgtm, thanks!

@siddhesh what anout the following?

Take review into account, some extra tests suggested by @aaron.ballman and the according modification in Decl.cpp

@rnk what do you find of this approach?

Take review into account and add more tests

I do think it would be good for this to somehow handle macros with reserved names, since macros are one of the primary dangers of using reserved names (if I #define _Tp 0 and then later #include <vector>

When __builtin_dynamic_object_size returns a non-constant expression, it cannot be -1 since that is an invalid return value for object size

More test cases and updated reserved isReserved()

As suggested by @rnk, spot the pattern more accurately at clang level, and use a combination of nobuiltin / builtin attributes to flag it at LLVM IR level.
What I like with that approach is that we're not preventing the emission a function Body, but just handling it in a decent way.

Up? I'd like to land that one this week if possible.

@sylvestre.ledru I: double checked and there's nothing in the original advisory against Darwin, but nothing that clearly states it's protected either (unlike Windows-based system). And there's also nothing specific to Darwin in the stack clash protection implementation, I think it's ok to only warn on Windows.

@rnk the new approach doesn't have the compilation time impact the previous had, still passes validation and a new test case has been added.

Updated approach, much less costly: match the pattern of functions forwarding to self, as detecting if they're recursive doesn't match the reality of inline builtins.
The rational could be that if it's doing anything other than forwarding to self, then it's there on purpose and we shouldn't skip it.

Update test case

Improve trivially recursive function detection

Sure! Can you send me an email at sguelton@redhat.com with the email you want to use for authorship? Thanks!

Some survivors!

@nickdesaulniers I added you to the review because you bumped into the non-inling issue at the kernel level at some point.

Update documentation and tests

In D92245#2425817, @emaste wrote:

How do things go wrong on Darwin? I was under the impression that this was implemented in LLVM as strictly inline code, no runtime support required.

That is my impression as well (although it seems that an earlier version might have emitted calls to a stack probe routine?)

Update test

Make the ascii test for isprintable locale-independent

The backend implementation of stack-clash-protection is incompatible with Windows, see https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/X86/X86FrameLowering.cpp#L486

As the state of LastUser are closely tied, it would make sense to encapsulate the two members in a class that would maintain the consistency of these two states, so that the two new operations recordNewUser(Pass, User) and updatelastUser(Pass, User) become explicit ?

If that's okay with @leonardchan in terms of RISCV validation, I'm fine with that one. Any branch removed is less path to cover!

Compiles fine with

I'm going to run a bunch of tests with different configuration before accepting :-)

We already discussed this change on IRC, I'm fine with it, once the suggested renaming is done.

Some history on that particular problem: back to https://reviews.llvm.org/D54472 when we moved away from llvm::isPodLike<> in favor of llvm::is_trivially_copyable<>, the goal was to move to is_trivially_copyable once the minimal compiler requirements reaches a supported state.
So I'm not in favor of a diverging between the llvm and std implementation, but either

Interestingly, gcc and clang have different behavior here: https://godbolt.org/z/hdadhY
And clang already honors the user-defined bcopy why is -D_FORTIFY_SOURCE different from the example here?

In D91461#2402720, @mgorny wrote:

Or to put it the other way, LINK_COMPONENTS is not equivalent to target_link_libraries with the PUBLIC keyword.

! In D91677#2401785, @siddhesh wrote:
Maybe because that D71082 is limited to clang? The bcopy survives clang but llvm forces it into a memmove without this patch.

@sbc100 Can you share your cmake configuration?

The problem you're trying to solve doesn't seem specific to bcopy: any function with an inline definition should probably *not* be replaced, and the inlined definition should be favored. This was the spirit of that patch https://reviews.llvm.org/D71082. i wonder why your test case doesn't fall into that configuration.

In D91641#2401024, @Meinersbur wrote:

There seems to be one set of braces too much, makes me wonder how it ever worked.

@sbc100: This should be fixed by 6795984a47dfd2da4f451ad86a99021d6c9f85b5

Can you give an example of how this would be used to select the gcc-toolchain?

I was thinking of `-Wl,--error-handling-script=/etc/clang/linker-script.sh` or something along the line. The initial goal would be to suggest installing the asan runtime if some asan symbols are missing.