Re: [musl] [PATCH] Use __builtin_FILE/__builtin_LINE if available

[Rich Felker <dalias@libc.org>]

On Fri, Feb 17, 2023 at 05:33:33PM -0800, Fangrui Song wrote:
> C++ inline functions are requred to have exact same sequence of tokens
> in every translation unit, but __FILE__ and __LINE__ may expand to
> different tokens. The ODR violatioin is usually benign, but it can lead
> to errors when C++20 modules are used.
> 
>     echo 'import B; import C; int main() { foo(); }' > A.cc
>     cat > B.ccm <<'eof'
>     module;
>     #include <assert.h>
>     export module B; export inline void foo() { assert(1); }
>     eof
>     cat > C.ccm <<'eof'
>     module;
>     #include <assert.h>
>     export module C; export inline void foo() { assert(1); }
>     eof
>     clang -std=c++20 --precompile B.ccm -o B.pcm
>     clang -std=c++20 --precompile C.ccm -o C.pcm
>     clang -std=c++20 -fprebuilt-module-path=. A.cc B.pcm C.pcm -o A
> 
>     /tmp/d/C.ccm:3:37: error: 'foo' has different definitions in different modules; definition in module 'C' first difference is function body
>     export module C; export inline void foo() { assert(1); }
>                             ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
>     /tmp/d/B.ccm:3:37: note: but in 'B' found a different body
>     export module B; export inline void foo() { assert(1); }
>                             ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
> 
> Fix this by preferring __builtin_FILE/__builtin_LINE which do not need
> preprocessing.
> ---
>  include/assert.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/assert.h b/include/assert.h
> index d14ec94e..b209c2ae 100644
> --- a/include/assert.h
> +++ b/include/assert.h
> @@ -4,6 +4,12 @@
>  
>  #ifdef NDEBUG
>  #define	assert(x) (void)0
> +#elif defined(__has_builtin)
> +#if __has_builtin(__builtin_FILE)
> +#define assert(x) ((void)((x) || (__assert_fail(#x, __builtin_FILE(), __builtin_LINE(), __func__),0)))
> +#else
> +#define assert(x) ((void)((x) || (__assert_fail(#x, __FILE__, __LINE__, __func__),0)))
> +#endif
>  #else
>  #define assert(x) ((void)((x) || (__assert_fail(#x, __FILE__, __LINE__, __func__),0)))
>  #endif
> -- 
> 2.39.GIT

It seems like use of assert here violates the ODR and is thus an
application error, no? In particular, it produces multiple definitions
that have differing behaviors, leaving which one actually gets used up
to the linker. Without the above change, LTO is able to diagnose the
error; with the change; it's silently deferred until runtime (where
the assertion violation message, of produced, will likely indicate the
wrong location).

Rich