[llvm] r175063 - [ms-inline-asm] Use an array_pod_sort, rather than a std:sort.
Timur Iskhodzhanov
timurrrr at google.com
Fri Feb 15 10:37:14 PST 2013
I can do better than that =P
Here's a minimized repro:
---------
int f(int a, int b) {
__asm{
mov ecx, b;
mov eax, dword ptr [a];
mov edx, dword ptr [a+4];
shld edx, eax, cl;
shl eax, cl;
}
}
---------
clang -Xclang -cxx-abi -Xclang microsoft source.cpp
2013/2/15 Chad Rosier <mcrosier at apple.com>:
> 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