Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 27 Feb 2023 23:26:53 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: Fangrui Song <i@...kray.me>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] Use __builtin_FILE/__builtin_LINE if available

* Fangrui Song <i@...kray.me> [2023-02-21 11:09:14 -0800]:

> On Sat, Feb 18, 2023 at 4:17 AM Jon Chesterfield
> <jonathanchesterfield@...il.com> wrote:
> >
> > On Sat, 18 Feb 2023, 02:54 Fangrui Song, <i@...kray.me> wrote:
> >
> > 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.
> >
> >
> >
> > It is sad that C++ modules broke 'assert' but not surprising. Modules were largely created out of aversion to macros. This isn't something libc can fix though, I suggest a defect report against C++ instead.
> >
> > Changing the semantics of assert in C seems like a bad thing to do.
> >
> > Thanks
> 
> I disagree. This is a footgun where the right fix (or workaround, if
> you prefer) is on the libc side. It is fairly reasonable for a header
> to use assert and not expect two includes using different paths to not
> cause C++ module problems.
> 
> The current module behavior regarding macros is a reasonable
> compromise. It can be evolved (e.g.
> https://gracicot.github.io/modules/2018/05/14/modules-macro.html).

i dont see how that solves the fundamental problem:

the *behavior* of assert changes depending on which include path is
used and thus inline functions that are supposed to be equivalent
aren't. (__builtin_FILE makes the pp-token sequence the same across
the instances, but the actual code will have different paths, which
while not an odr violation per the literal words of the spec, it
clearly violates the reason the rule is there in the first place.)

libc can avoid printing the file path in the assert fail message for
c++. this makes assert less useful but it solves the conformance issue.
if c++ does not specify which path assert should print (or allow it to
be unpredictable) then it is difficult to do better than this.

it would have been more useful to have a __builtin_canonical_FILE()
or similar that gives a path that is somehow independent of include
path, but we don't have that now.

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.