[cfe-dev] Can indirect class parameters be noalias?
David Rector via cfe-dev
cfe-dev at lists.llvm.org
Wed Aug 26 18:28:05 PDT 2020
> On Aug 26, 2020, at 12:45 PM, Florian Hahn <florian_hahn at apple.com> wrote:
>
> Hi,
>
>> On Aug 4, 2020, at 18:32, David Rector via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>>
>>
>>
>>> On Aug 3, 2020, at 11:28 PM, Hal Finkel via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>>>
>>>
>>>
>>> On 8/3/20 8:45 PM, John McCall wrote:
>>>> On 31 Jul 2020, at 19:50, Hal Finkel wrote:
>>>>
>>>> On 7/31/20 5:59 PM, James Y Knight wrote:
>>>>
>>>> This discussion reminds me of an example I ran into a couple weeks ago, where the execution of the program is dependent precisely upon whether the ABI calls for the object to be passed indirectly, or in a register
>>>>
>>>> In the case where NVRO is triggered, the class member foo_ is fully-constructed on the first line of CreateFoo (despite appearing as if that's only constructing a local variable). In the case where the struct is small enough to fit in a register, NVRO does not apply, and in that case, foo_ isn't constructed until after CreateFoo returns.
>>>>
>>>> Therefore, I believe it's implementation-defined whether the following program has undefined behavior.
>>>>
>>>> https://godbolt.org/z/YT9zsz <https://godbolt.org/z/YT9zsz> <https://godbolt.org/z/YT9zsz <https://godbolt.org/z/YT9zsz>>
>>>>
>>>> #include <assert.h>
>>>>
>>>> struct Foo {
>>>> int x;
>>>> * // assert fails if you comment out these unused fields!
>>>> * int dummy[4];
>>>> };
>>>>
>>>> struct Bar {
>>>> Bar() : foo_(CreateFoo()) {}
>>>>
>>>> Foo CreateFoo() {
>>>> Foo f;
>>>> f.x = 55;
>>>> assert(foo_.x == 55);
>>>> return f;
>>>> }
>>>> Foo foo_;
>>>> };
>>>>
>>>> int main() {
>>>> Bar b;
>>>> }
>>>>
>>>> Looks that way to me too. The example in 11.10.5p2 sort of makes this point as well (by pointing out that you can directly initialize a global this way).
>>>>
>>>> It does seem hard to argue that this is invalid under the specification. To me it seems like it clearly ought to be invalid, though. Note that Clang has always emitted return address arguments as noalias, so this has immediate significance.
>>>>
>>>> If I were writing the specification, I would rewrite the restriction in [class.cdtor]p2 to say that pointers derived by naming a returned/constructed object do not formally point to the object until the function actually returns, even if the copy is elided. That would make James’s example undefined behavior.
>>>>
>>>> John.
>>>>
>>>
>>> I agree. It seems like we should be able to make a sanitizer detect this kind of mistake as well (although the general case will require some msan-like propagation scheme).
>>>
>>> -Hal
>>>
>>
>> On second thought I agree as well — i.e. allow both the NRVO and the noalias optimizations to be applied as they currently are, and adjust the standard so that any code which would break due to those optimizations should be considered undefined.
>>
>> That would be consistent with the proposal of making escaping pointers within a constructor of a trivially-copyable class undefined behavior.
>>
>> And more generally: if there are any other instances in which the compiler is currently forced to be pessimistic, just to handle a few weird usages which are technically allowed, I think by all means we should ask the standard to explicitly address those, via the same approach: the typical, more-optimizable cases should be allowed to fully optimize, by making the rare, less-optimizable cases undefined behavior.
>>
>
> It would be great to make some progress on the issue, but unfortunately I am unclear what the next steps should be?
>
> As mentioned earlier, I added a patch to add noalias under a flag (https://reviews.llvm.org/D85473 <https://reviews.llvm.org/D85473>). Are there any concerns/objections on this as a first step?
>
> Whether to enable it by default can then be discussed separately. IIUC from the discussion, we would need an adjustment to the standard along to lines of John’s proposal to making escaping pointers within a constructor of a trivially-copyable class undefined behavior. Are there any pointers/suggestions on how to best go about that?
I earlier inferred this was John’s proposal without him saying as much, and in fact now I think it wouldn’t go far enough, given the fact that NRVO basically lets certain functions act as a glorified constructors, giving further opportunities to escape a pointer. Consider this addition to Richard’s example:
```
#include <stdio.h>
struct A {
A(A **where) : data{"hello world 1"} {
*where = this; //Escaped pointer 1 (proposed UB?)
}
A() : data{"hello world 2"} {}
char data[65536];
};
A *p;
[[gnu::noinline]]
void f(A a) {
for (int i = 0; i != sizeof(A::data) - 2; ++i)
p->data[i+1] = a.data[i];
puts(a.data);
}
A CreateA(A **where) {
A justlikethis;
*where = &justlikethis; //Escaped pointer 2 (should also be UB, then)
return justlikethis;
}
// elsewhere, perhaps compiled by a smarter compiler that doesn't make a copy here
int main() {
f({&p}); // 1
f(CreateA(&p)); // 2
```
I think John was on to the right path in addressing James’s NRVO example; that example and John’s response reproduced here:
> On Aug 3, 2020, at 9:45 PM, John McCall via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>
> On 31 Jul 2020, at 19:50, Hal Finkel wrote:
>
> On 7/31/20 5:59 PM, James Y Knight wrote:
>
> This discussion reminds me of an example I ran into a couple weeks ago, where the execution of the program is dependent precisely upon whether the ABI calls for the object to be passed indirectly, or in a register
>
> In the case where NVRO is triggered, the class member foo_ is fully-constructed on the first line of CreateFoo (despite appearing as if that's only constructing a local variable). In the case where the struct is small enough to fit in a register, NVRO does not apply, and in that case, foo_ isn't constructed until after CreateFoo returns.
>
> Therefore, I believe it's implementation-defined whether the following program has undefined behavior.
>
> https://godbolt.org/z/YT9zsz <https://godbolt.org/z/YT9zsz> <https://godbolt.org/z/YT9zsz <https://godbolt.org/z/YT9zsz>>
>
> #include <assert.h>
>
> struct Foo {
> int x;
> * // assert fails if you comment out these unused fields!
> * int dummy[4];
> };
>
> struct Bar {
> Bar() : foo_(CreateFoo()) {}
>
> Foo CreateFoo() {
> Foo f;
> f.x = 55;
> assert(foo_.x == 55);
> return f;
> }
> Foo foo_;
> };
>
> int main() {
> Bar b;
> }
>
> Looks that way to me too. The example in 11.10.5p2 sort of makes this point as well (by pointing out that you can directly initialize a global this way).
>
> It does seem hard to argue that this is invalid under the specification. To me it seems like it clearly ought to be invalid, though. Note that Clang has always emitted return address arguments as noalias, so this has immediate significance.
>
> If I were writing the specification, I would rewrite the restriction in [class.cdtor]p2 to say that pointers derived by naming a returned/constructed object do not formally point to the object until the function actually returns, even if the copy is elided. That would make James’s example undefined behavior.
>
> John.
>
I think the goal should be to start with the above wording and add wording which also prevents escaping any pointer to the returned/constructed object until the function returns/construction finishes, for trivially copyable classes anyway...or something like that.
I have wandered way out of my depth, so I’ll leave it to you guys from here! Thanks Florian for doing the legwork/testing, so helpful!
Dave
>
>> But in tandem, the standard should also introduce attributes that allow advanced users to explicitly opt out of those optimizations (e.g. "forcecopy" or "maybealias" or whatever) to allow their weird cases to remain standard compliant. Nobody loses.
>>
>> People write C and C++ because they want the fastest possible compiled code — the standard should get out of the way wherever it is obstructing optimization.
>>
>
> Agreed, although this change seems to have the potential to break a very small subset of code out there in a very subtle manner, without an easy way to detect that. But maybe we can detect and warn about the most common/obvious violations at least and that is enough?
>
> Cheers,
> Florian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200826/12dcdc02/attachment-0001.html>
More information about the cfe-dev
mailing list