[clang] [llvm] [Clang] Protect ObjCMethodList assignment operator against self-assignment (PR #97933)

Tom Honermann via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 6 14:02:15 PDT 2024


tahonermann wrote:

I'm sorry, @smanna12, I misread the code earlier and misled you. That `PunnedPointer` assignment operator I directed you too isn't a copy assignment operator (it takes a `intptr_t`, not a reference or value of `PunnedPointer`). Changing that assignment operator won't affect this issue since it isn't used by the (compiler generated) copy assignment operators of `PointerIntPair` and `ObjCMethodList`.

In practice, I think the existing code is fine as is. From the perspective of the C++ standard, I'm genuinely unsure whether there is an issue or not. Consider the following example. https://godbolt.org/z/WsYsYr6ob.
```
struct X {
  char d[257];
};
X& f(X &x) {
  x = x;
  return x;
}
```
Under optimization, all four compilers (presumably) recognize the self assignment and optimize away the copy of `x.d`. Without optimization, Clang implements the copy via a call to `memcpy()`, MSVC emits a `rep movsb` instruction, and gcc and icc both omit the copy. According to the C++ standard (via deference to the C standard), `memcpy()` requires non-overlapping source and destination regions. The use of `memcpy()` by Clang therefore results in undefined behavior unless the implementation of `memcpy()` that is called handles overlapping memory (which it likely does in this case).

The C++ standard does not state whether the compiler generated copy assignment operator is required to handle self assignment for data members of array type. All it says is ([\[class.copy.assign\]p(12.2)](http://eel.is/c++draft/class.copy.assign#12.2):
> if the subobject is an array, each element is assigned, in the manner appropriate to the element type;

I think we should triage the static analysis issue as a false positive and abandon this PR for now. I'll follow up with the WG21 core working group to see if there is a core issue here.

https://github.com/llvm/llvm-project/pull/97933


More information about the cfe-commits mailing list