[llvm] r304123 - Don't capture a temporary std::string in a StringRef.

[Zachary Turner via llvm-commits]

looks like MSVC's implementation writes a 0 into the first character on
destruction.

basic_string::~basic_string() calls a function named _Tidy_deallocate(),
which looks like this:

void _Tidy_deallocate()
{
                // bunch of extra stuff snipped out
_Traits::assign(_My_data._Bx._Buf[0], _Elem());
}

I'd be curious whether ASAN catches this on a non-MSVC implementation, if
not it seems like a bug they would probably want to fix

On Sun, May 28, 2017 at 7:45 PM Craig Topper <craig.topper at gmail.com> wrote:

> Hmm the one I fixed earlier was a similar getValueAsString being captured
> in to a StringRef. I unable to reproduce it with debug symbols enabled, but
> valgrind was able to flag it as an invalid access. But valgrind wasn't
> flagging the one you fixed. Does MSVC's implementation of std::string null
> terminates the array before destructing?
>
> ~Craig
>
> On Sun, May 28, 2017 at 7:38 PM, Zachary Turner <zturner at google.com>
> wrote:
>
>> Also it wasn't actually "crashing", it was hitting the llvm_unreachable
>> at the bottom of this function.
>>
>> On Sun, May 28, 2017 at 7:38 PM Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> Just let it crash under the debugger, and when I looked at Name it was
>>> "\0em16".  But when I stepped into getValueAsString it looked like it was
>>> indeed returning "Mem16", which should have been correct.  So there was
>>> obviously some memory corruption, but since it happened in such a narrow
>>> time frame (between the return statement and assigning the value to the
>>> StringRef, there weren't really a lot of possibilities.  So when I checked
>>> the function signature, sure enough it was returning a std::string.
>>>
>>> On Sun, May 28, 2017 at 7:33 PM Craig Topper <craig.topper at gmail.com>
>>> wrote:
>>>
>>>> How did you narrow it down to that?
>>>>
>>>> ~Craig
>>>>
>>>> On Sun, May 28, 2017 at 7:20 PM, Zachary Turner via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: zturner
>>>>> Date: Sun May 28 21:20:12 2017
>>>>> New Revision: 304123
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=304123&view=rev
>>>>> Log:
>>>>> Don't capture a temporary std::string in a StringRef.
>>>>>
>>>>> This fixes the breakages in llvm-tblgen.
>>>>>
>>>>> Modified:
>>>>>     llvm/trunk/utils/TableGen/X86FoldTablesEmitter.cpp
>>>>>
>>>>> Modified: llvm/trunk/utils/TableGen/X86FoldTablesEmitter.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/X86FoldTablesEmitter.cpp?rev=304123&r1=304122&r2=304123&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/utils/TableGen/X86FoldTablesEmitter.cpp (original)
>>>>> +++ llvm/trunk/utils/TableGen/X86FoldTablesEmitter.cpp Sun May 28
>>>>> 21:20:12 2017
>>>>> @@ -285,7 +285,7 @@ getMemOperandSize(const Record *MemRec,
>>>>>          (MemRec->getName() == "sdmem" || MemRec->getName() ==
>>>>> "ssmem"))
>>>>>        return 128;
>>>>>
>>>>> -    StringRef Name =
>>>>> +    std::string Name =
>>>>>
>>>>>  MemRec->getValueAsDef("ParserMatchClass")->getValueAsString("Name");
>>>>>      if (Name == "Mem8")
>>>>>        return 8;
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170529/b98c8a86/attachment.html>