nickdesaulniers planned changes to this revision. nickdesaulniers added a comment.
In D74094#4643439 <https://reviews.llvm.org/D74094#4643439>, @rjmccall wrote: > Under [expr.call]p6 (https://eel.is/c++draft/expr.call#6), it is > implementation-defined whether the lifetime of a parameter ends at the end of > the containing full expression or at the end of the function. If the > implementation chooses the latter, a reference to the parameter will dangle > after the return. In your example, on your chosen target (probably x64-64?), > we are actually forced by the psABI to end those lifetimes at the end of the > function, because the contents of the parameters are passed in registers > (rather than by implicit reference) and reconstituted in the stack frame of > the callee. Therefore your `min` function has undefined behavior, and not > just in a formal sense but actually in a very directly dangerous sense. That > should not be surprising; intuitively, you should think of parameters as > local variables, and therefore this code as returning a reference to a local > variable, which is highly suspect even if it is sometimes — only on certain > targets and for certain types — formally allowed. > >> That makes me think we should probably do this lifetime annotation in the >> caller of CodeGenFunction::EmitCallArg since it will have information about >> the return type of the caller. If a function returns a reference to a type, >> we probably don't want to emit the lifetime markers for any parameter of >> that type. I wonder if strict aliasing needs to be considered, too? > > This is definitely barking up the wrong tree. The language specifies the > lifetime of an object, and any access to the object during its lifetime is > valid. If the parameter's lifetime *was* guaranteed to be the full > expression here, we would be obliged to keep it alive for that full duration. > > In particular, we cannot do anything with reasoning like "the return value > cannot be a reference to the parameter". The right way to understand that is > as an effort to invoke the as-if rule, arguing that it is impossible to > reference the parameter after the call if it is not part of the return value, > and therefore we can be more aggressive than the language would formally > allow. But it is not true that the only way to reference a parameter after a > call is if it is part of the return value. For example, the function could > store a pointer to the parameter into a global variable, and as long as all > accesses through that global happen before the parameter's lifetime ends, > that's totally fine — it's obviously a very confusing and error-prone way to > write code, but it's legal. (Also, even if the address of the parameter > *could* only escape into the return value, the type analysis would have to be > much more complicated than just whether it's a reference to the type of a > parameter.) Hmm...ok thanks for the feedback. In that case then, is it your opinion that there was also a "very directly dangerous sense" of UB then in the test case <https://reviews.llvm.org/D74094#4633799> for which the previous revision of this patch was reverted? It's currently unclear to me how to go about relanding this; is there an explicit bug in e698695fbbf62e6676f8907665187f2d2c4d814b <https://reviews.llvm.org/rGe698695fbbf62e6676f8907665187f2d2c4d814b> or not? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74094/new/ https://reviews.llvm.org/D74094 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
