From: Rich Felker <dalias@libc.org>
To: Fangrui Song <i@maskray.me>
Cc: musl@lists.openwall.com
Subject: Re: [musl] [PATCH] Use __builtin_FILE/__builtin_LINE if available
Date: Fri, 17 Feb 2023 21:03:20 -0500 [thread overview]
Message-ID: <20230218020320.GA4163@brightrain.aerifal.cx> (raw)
In-Reply-To: <20230218013333.844224-1-i@maskray.me>
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
next prev parent reply other threads:[~2023-02-18 2:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-18 1:33 Fangrui Song
2023-02-18 2:03 ` Rich Felker [this message]
2023-02-18 2:53 ` Fangrui Song
2023-02-18 12:17 ` Jon Chesterfield
2023-02-21 19:09 ` Fangrui Song
2023-02-27 22:26 ` Szabolcs Nagy
2023-03-02 8:19 ` Fangrui Song
[not found] ` <DS7PR12MB57655F44D45FF7D0B9BB01C5CBB29@DS7PR12MB5765.namprd12.prod.outlook.com>
2023-03-05 3:23 ` Zhihao Yuan
2023-08-30 22:04 ` Fangrui Song
2023-02-21 19:45 ` Jeffrey Walton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230218020320.GA4163@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=i@maskray.me \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).