[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 17 Feb 2023 18:53:38 -0800
From: Fangrui Song <i@...kray.me>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Use __builtin_FILE/__builtin_LINE if available
On Fri, Feb 17, 2023 at 6:03 PM Rich Felker <dalias@...c.org> wrote:
>
> 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
I am unsure whether it is an application error in the above example.
If it is not a good example, here is another one
where the inline function using assert is in a header:
echo 'import B; import C; int main() { foo(); }' > A.cc
cat > a.h <<'eof'
#include <assert.h>
inline void fn() { assert(1); }
eof
cat > B.ccm <<'eof'
module;
#include "a.h"
export module B; export inline void foo() { fn(); }
eof
mkdir -p ./d
cat > d/C.ccm <<'eof'
module;
#include "../a.h"
export module C; export inline void foo() { fn(); }
eof
sed 's/^ /\t/' > Makefile <<'eof'
C := clang
all:
$C -std=c++20 --precompile B.ccm -o B.pcm
$C -std=c++20 --precompile d/C.ccm -o d/C.pcm
$C -std=c++20 -fprebuilt-module-path=. -fprebuilt-module-path=d A.cc
B.pcm d/C.pcm -o A
eof
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
