[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

Tue May 2 09:20:59 PDT 2023

serge-sans-paille added a comment.

In D148723#4296111 <https://reviews.llvm.org/D148723#4296111>, @efriedma wrote:

> I'm a little concerned that this will explode in unexpected ways... in particular, it'll fail to link if the function doesn't actually exist externally.  Which it probably doesn't if it would otherwise be linkonce_odr.

Let me summarize current approach and the problem it tries to solve in order to find a solution (I'm not claiming current solution is the best).

Some code define an always inline version of existing builtins. This has happened in the past in the case of Fortified source, and also for in some math.h header. Previous behavior of clang was to ignore the actual definition because it knew the builtin behavior and directly replaced it by a builtin call. This was not satisfying because this by-passed fortified implementation of these builtins.

What we were doing before extending the scope of what is an inline builtin was to detect inline, always-inline, gnu-inline functions and rename them into `<builtin-name>.inline`, updating all call site (except the trivially recursive ones) to call this version. The original definition remained but got stripped of its body (so that the `<builtin-name>.inline` version could call it in a non-recursive manner), turning it into a declaration. This declaration kept its original attribute, which made it fall into a COMDAT section which is invalid. What this patch is trying to achieve is to massage the original definition in such a way that it's compatible with the other assumptions.

Writing this I realize that I could also remove its inline / always_inline attribute when it's stripped of its body, that maybe enough.

What this p

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

https://reviews.llvm.org/D148723