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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to