[llvm] r175063 - [ms-inline-asm] Use an array_pod_sort, rather than a std:sort.
Chad Rosier
mcrosier at apple.com
Fri Feb 15 09:45:33 PST 2013
Would you mind attaching the preprocessed source and crash script?
clang: note: diagnostic msg: C:/Users/timurrrr/AppData/Local/Temp/windowsh_pass-036380.cpp
clang: note: diagnostic msg: C:/Users/timurrrr/AppData/Local/Temp/windowsh_pass-036380.sh
Chad
On Feb 15, 2013, at 9:39 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> Attached is the stacktrace just in case
>
> 2013/2/15 Timur Iskhodzhanov <timurrrr at google.com>:
>> 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 :)
>>
>>
>> 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
>
> 2013/2/15 Timur Iskhodzhanov <timurrrr at google.com>:
>> 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 :)
>>
>>
>> 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
> <crashstack.txt>
More information about the llvm-commits
mailing list