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

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Wed May 31 15:55:20 PDT 2017


> On May 28, 2017, at 7:59 PM, Zachary Turner via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 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

I think it gets it. Hopefully this captures the essence of the bug:

// Compile with: clang++ /tmp/x.cc -o /tmp/x -fsanitize=address -fsanitize-address-use-after-scope
#include <string>

struct StringRef {
  const char *data;
  StringRef(const std::string &s) : data(s.data()) {}
};

std::string foo(const std::string &s) {
  return s + s;
}

int main() {
  StringRef s = foo("bar");
  return s.data[0];
}
// ==7094==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fff5ccc3781 at pc 0x000102f3d821 bp 0x7fff5ccc3730 sp 0x7fff5ccc3728

vedant

> 
> 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
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list