[llvm] r175063 - [ms-inline-asm] Use an array_pod_sort, rather than a std:sort.

[Chad Rosier]

On Feb 15, 2013, at 9:31 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:

> OK, here we go.
> 
> The crash happens if you "#include <windows.h>" on a machine with
> VS2010 installed (other versions might be affected too).
> 
> Here's a patch that adds an assert that crashes:
> ---------------------------------------------------------------------------------
> diff --git lib/MC/MCParser/AsmParser.cpp lib/MC/MCParser/AsmParser.cpp
> index 2cce8b0..1cfd5f6 100644
> --- lib/MC/MCParser/AsmParser.cpp
> +++ lib/MC/MCParser/AsmParser.cpp
> @@ -4174,8 +4174,10 @@ bool AsmParser::ParseMSInlineAsm(void *AsmLoc,
> std::string &AsmString,
> 
>     // Emit everything up to the immediate/expression.  If the previous rewrite
>     // was a size directive, then this has already been done.
> -    if (PrevKind != AOK_SizeDirective)
> +    if (PrevKind != AOK_SizeDirective) {
> +      assert(Loc >= Start);
>       OS << StringRef(Start, Loc - Start);
> +    }
>     PrevKind = Kind;
> 
>     // Skip the original expression.
> ---------------------------------------------------------------------------------
> 
> 
> I hope that's enough info to debug :)
> 

Thanks, Timur.  I'm looking into this now.
 
Chad

> FYI The bug was hard to debug as a substantial part of memory was
> corrupted (including EBPs, return addresses etc)
> so I propose something like this to prevent such bugs from re-appearing:
> ---------------------------------------------------------------------------------
> diff --git include/llvm/ADT/StringRef.h include/llvm/ADT/StringRef.h
> index 1e21d92..077c056 100644
> --- include/llvm/ADT/StringRef.h
> +++ include/llvm/ADT/StringRef.h
> @@ -84,6 +84,7 @@ namespace llvm {
>       : Data(data), Length(length) {
>         assert((data || length == 0) &&
>         "StringRef cannot be built from a NULL argument with non-null length");
> +        assert(length < 0x80000000 && "length looks suspicious");
>       }
> 
>     /// Construct a string ref from an std::string.
> ---------------------------------------------------------------------------------
> 
> 2013/2/15 Timur Iskhodzhanov <timurrrr at google.com>:
>> FYI this crashes on my Windows bot.
>> 
>> I'll update with the details when I reproduce locally.
>> 
>> 2013/2/13 Chad Rosier <mcrosier at apple.com>:
>>> Author: mcrosier
>>> Date: Wed Feb 13 12:38:58 2013
>>> New Revision: 175063
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=175063&view=rev
>>> Log:
>>> [ms-inline-asm] Use an array_pod_sort, rather than a std:sort.
>>> 
>>> 
>>> Modified:
>>>    llvm/trunk/lib/MC/MCParser/AsmParser.cpp
>>> 
>>> Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=175063&r1=175062&r2=175063&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
>>> +++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Wed Feb 13 12:38:58 2013
>>> @@ -13,6 +13,7 @@
>>> 
>>> #include "llvm/ADT/APFloat.h"
>>> #include "llvm/ADT/SmallString.h"
>>> +#include "llvm/ADT/STLExtras.h"
>>> #include "llvm/ADT/StringMap.h"
>>> #include "llvm/ADT/Twine.h"
>>> #include "llvm/MC/MCAsmInfo.h"
>>> @@ -4029,8 +4030,14 @@ bool AsmParser::ParseDirectiveMSAlign(SM
>>>   return false;
>>> }
>>> 
>>> -bool AsmStringSort (AsmRewrite A, AsmRewrite B) {
>>> -  return A.Loc.getPointer() < B.Loc.getPointer();
>>> +static int RewritesSort (const void *A, const void *B) {
>>> +  const AsmRewrite *AsmRewriteA = static_cast<const AsmRewrite*>(A);
>>> +  const AsmRewrite *AsmRewriteB = static_cast<const AsmRewrite*>(B);
>>> +  if (AsmRewriteA->Loc.getPointer() < AsmRewriteB->Loc.getPointer())
>>> +    return -1;
>>> +  if (AsmRewriteB->Loc.getPointer() < AsmRewriteA->Loc.getPointer())
>>> +    return 1;
>>> +  return 0;
>>> }
>>> 
>>> bool AsmParser::ParseMSInlineAsm(void *AsmLoc, std::string &AsmString,
>>> @@ -4157,7 +4164,7 @@ bool AsmParser::ParseMSInlineAsm(void *A
>>>   AsmRewriteKind PrevKind = AOK_Imm;
>>>   raw_string_ostream OS(AsmStringIR);
>>>   const char *Start = SrcMgr.getMemoryBuffer(0)->getBufferStart();
>>> -  std::sort (AsmStrRewrites.begin(), AsmStrRewrites.end(), AsmStringSort);
>>> +  array_pod_sort (AsmStrRewrites.begin(), AsmStrRewrites.end(), RewritesSort);
>>>   for (SmallVectorImpl<struct AsmRewrite>::iterator
>>>          I = AsmStrRewrites.begin(), E = AsmStrRewrites.end(); I != E; ++I) {
>>>     const char *Loc = (*I).Loc.getPointer();
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits