From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 22913 invoked from network); 18 Feb 2023 02:03:37 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 18 Feb 2023 02:03:37 -0000 Received: (qmail 31880 invoked by uid 550); 18 Feb 2023 02:03:33 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 31848 invoked from network); 18 Feb 2023 02:03:33 -0000 Date: Fri, 17 Feb 2023 21:03:20 -0500 From: Rich Felker To: Fangrui Song Cc: musl@lists.openwall.com Message-ID: <20230218020320.GA4163@brightrain.aerifal.cx> References: <20230218013333.844224-1-i@maskray.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230218013333.844224-1-i@maskray.me> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH] Use __builtin_FILE/__builtin_LINE if available 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 > export module B; export inline void foo() { assert(1); } > eof > cat > C.ccm <<'eof' > module; > #include > 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