AaronBallman wrote: > _Every_ `va_list` stores pointers; otherwise, it wouldn't be able to support > an arbitrary number of arguments. Copying just copies the pointers, and > that's fine because the memory they point to is immutable and always outlives > the `va_list`. You can imagine a `va_list` implementation that doesn't have > these properties, but it's hard.
But do separate va_list objects within the same function always have the same pointer *values*? I was thinking the "saved state" pointers might actually point to different memory regions to support different restoration points, but I could be way off base there. > > I'm not super happy about "this is well-defined if you happen to be on a > > target that makes it well-defined" because there's basically no reasonable > > way for a user to find that information out themselves. > > Well, if we're going to document that this is allowed, we should document > what targets it's allowed on. I'm not arguing that we shouldn't document it. I had the unpleasant experience of trying to figure out what targets it's allowed on. We have five links to ABI resources regarding va_list in https://github.com/llvm/llvm-project/blob/746f5726158d31aeeb1fd12b226dc869834a7ad2/clang/include/clang/Basic/TargetInfo.h#L319; four of the links are dead and one of the targets is unsupported as of 2022 (thus has no documentation whatsoever) but its va_list type continues to be used outside of that target (https://github.com/llvm/llvm-project/blob/a1d73ace13a20ed122a66d3d59f0cbae58d0f669/clang/lib/Basic/Targets/Le64.h#L41). Given the ease of calling `va_copy`, the fact that C had implicit UB here for ~40 years, and it's going to be a slog to try to nail down every single target's behavior, I still think documenting it as explicit UB is reasonable. If users or target owners want a different behavior, then they could give a use case for why and we could re-assess at that point. All we're effectively saying with that documentation is "if you do it, we don't promise it will work" and we already effectively say that with our existing documentation when we say `It is undefined behavior to call this function with a list that has not been initialized by either __builtin_va_start or __builtin_va_copy.` Alternatively, we could document that it's supported on all targets, test whatever codegen Clang emits, assume all the targets support this, and then document any unsupported targets discovered in the wild as explicitly unsupported. I'm a bit less comfortable with that, but when both the codegen code owners tell me they it's very unlikely this *won't* work on any targets we care about, I can certainly trust that expertise! https://github.com/llvm/llvm-project/pull/98146 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
