This is an archive of the discontinued LLVM Phabricator instance.

[libc++] ADL-proof by adding _VSTD:: qualifications to memmove etc. NFCI
ClosedPublic

Authored by Quuxplusone on Dec 10 2020, 12:53 PM.

Details

Summary

Generally these calls aren't vulnerable to ADL because they involve only
primitive types. The ones in <list> and <vector> drag in namespace std
but that's OK; the ones in <fstream> and <strstream> are vulnerable
iff CharT is an enum type, which seems far-fetched.
But absolutely zero of them *need* ADL to happen; so in my opinion
they should all be consistently qualified, just like calls to any
other (non-user-customizable) functions in namespace std.

Also: Include <cstring> and <cwchar> in <__string>.
We seemed to be getting lucky that <memory> included <iterator>
included <iosfwd> included <wchar.h>. That gave us the
global-namespace wmemmove, but not _VSTD::wmemmove.
This is now fixed.

I didn't touch these headers:

<ext/__hash> uses strlen, safely
<support/ibm/locale_mgmt_aix.h> uses memcpy, safely
<string.h> uses memchr and strchr, safely
<wchar.h> uses wcschr, safely
<__bsd_locale_fallbacks.h> uses wcsnrtombs, safely

There's a bit of overlap here with D92875; I'd plan to land that one before this one.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Dec 10 2020, 12:53 PM
Quuxplusone created this revision.
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 10 2020, 12:53 PM
Quuxplusone edited the summary of this revision. (Show Details)Dec 10 2020, 12:53 PM
curdeius accepted this revision.Dec 10 2020, 12:59 PM
ldionne accepted this revision.Dec 10 2020, 1:00 PM
This revision is now accepted and ready to land.Dec 10 2020, 1:00 PM

Rebased on D92875 for the benefit of buildkite. If buildkite is green, I'm going to land both of these.