This is an archive of the discontinued LLVM Phabricator instance.

[llvm] Fix string copy confusion
ClosedPublic

Authored by urnathan on Mar 24 2022, 6:01 AM.

Details

Summary

The microsoft demangler makes copies of the demangled strings, but has some confusion between StringView representation (sans NUL), and C-strings (with NUL). Here we also have a use of strcpy, which happens to work because the incoming string view happens to have a trailing NUL. But a simple memcpy excluding the NUL is sufficient.

This is broken out of D120990 and is a prerequisite of that patch.

Diff Detail

Event Timeline

urnathan created this revision.Mar 24 2022, 6:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2022, 6:01 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
urnathan requested review of this revision.Mar 24 2022, 6:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2022, 6:01 AM
erichkeane accepted this revision.Mar 24 2022, 6:32 AM

This looks logical to me. That said, we're deep into files that I don't claim any knowledge/ownership over, so please give this a day or two before committing for others to have a chance to take a look.

This revision is now accepted and ready to land.Mar 24 2022, 6:32 AM
dblaikie accepted this revision.Mar 24 2022, 9:27 AM

Yeah, it wouldn't /entirely/ surprise me (knowing little about the code) if something, somewhere, depended on the StringView actually pointing to a null terminated string - but if tests are passing, etc (especially with something like asan), then this sounds OK to me.

I did look for consumers of the cache of copied names, and AFAICT they handled StringViews, not C-strings. I'll wait a couple of days anyway.

This revision was landed with ongoing or failed builds.Mar 28 2022, 9:38 AM
This revision was automatically updated to reflect the committed changes.