[llvm] r194575 - Remove AllowQuotesInName and friends from MCAsmInfo.
Kevin Modzelewski
kmod at dropbox.com
Wed Nov 13 19:09:03 PST 2013
Did a little more debugging, and it seems like this is related to the fact
that I'm creating global variables with odd names: specifically, I get all
sorts of weird behavior if I put any nul bytes into variable names.
On Wed, Nov 13, 2013 at 6:43 PM, Kevin Modzelewski <kmod at dropbox.com> wrote:
> Hi, this commit seems to cause some infrequent issue with mcjit. I'm not
> 100% sure what the exact problem is or how it is related to this patch (I
> bisected to find this), but the result is that sometimes the jit'd code
> memory comes up with the wrong permissions. Even though it's
> deterministic, it's infrequent so unfortunately I don't have a good test
> case at the moment; I can work on reducing it but I thought I'd flag the
> issue in the meantime. It's also definitely possible that I'm using mcjit
> is some brittle way and that this patch just exposes it, but I don't think
> that that would cause the type of issue I'm seeing.
>
> Kevin
>
>
> On Wed, Nov 13, 2013 at 6:01 AM, Rafael Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> Author: rafael
>> Date: Wed Nov 13 08:01:59 2013
>> New Revision: 194575
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=194575&view=rev
>> Log:
>> Remove AllowQuotesInName and friends from MCAsmInfo.
>>
>> Accepting quotes is a property of an assembler, not of an object file. For
>> example, ELF can support any names for sections and symbols, but the gnu
>> assembler only accepts quotes in some contexts and llvm-mc in a few more.
>>
>> LLVM should not produce different symbols based on a guess about which
>> assembler
>> will be reading the code it is printing.
>>
>> Modified:
>> llvm/trunk/include/llvm/MC/MCAsmInfo.h
>> llvm/trunk/lib/MC/MCAsmInfo.cpp
>> llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
>> llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp
>> llvm/trunk/lib/MC/MCSectionELF.cpp
>> llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
>> llvm/trunk/lib/Target/Mangler.cpp
>> llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp
>> llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp
>> llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll
>> llvm/trunk/test/CodeGen/X86/global-sections.ll
>>
>> Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)
>> +++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Wed Nov 13 08:01:59 2013
>> @@ -144,18 +144,6 @@ namespace llvm {
>> /// AssemblerDialect - Which dialect of an assembler variant to use.
>> unsigned AssemblerDialect; // Defaults to 0
>>
>> - /// AllowQuotesInName - This is true if the assembler allows for
>> complex
>> - /// symbol names to be surrounded in quotes. This defaults to false.
>> - bool AllowQuotesInName;
>> -
>> - /// AllowNameToStartWithDigit - This is true if the assembler allows
>> symbol
>> - /// names to start with a digit (e.g., "0x0021"). This defaults to
>> false.
>> - bool AllowNameToStartWithDigit;
>> -
>> - /// AllowPeriodsInName - This is true if the assembler allows
>> periods in
>> - /// symbol names. This defaults to true.
>> - bool AllowPeriodsInName;
>> -
>> /// \brief This is true if the assembler allows @ characters in
>> symbol
>> /// names. Defaults to false.
>> bool AllowAtInName;
>> @@ -467,15 +455,6 @@ namespace llvm {
>> unsigned getAssemblerDialect() const {
>> return AssemblerDialect;
>> }
>> - bool doesAllowQuotesInName() const {
>> - return AllowQuotesInName;
>> - }
>> - bool doesAllowNameToStartWithDigit() const {
>> - return AllowNameToStartWithDigit;
>> - }
>> - bool doesAllowPeriodsInName() const {
>> - return AllowPeriodsInName;
>> - }
>> bool doesAllowAtInName() const {
>> return AllowAtInName;
>> }
>>
>> Modified: llvm/trunk/lib/MC/MCAsmInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCAsmInfo.cpp (original)
>> +++ llvm/trunk/lib/MC/MCAsmInfo.cpp Wed Nov 13 08:01:59 2013
>> @@ -50,9 +50,6 @@ MCAsmInfo::MCAsmInfo() {
>> Code32Directive = ".code32";
>> Code64Directive = ".code64";
>> AssemblerDialect = 0;
>> - AllowQuotesInName = false;
>> - AllowNameToStartWithDigit = false;
>> - AllowPeriodsInName = true;
>> AllowAtInName = false;
>> UseDataRegionDirectives = false;
>> ZeroDirective = "\t.zero\t";
>>
>> Modified: llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp (original)
>> +++ llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp Wed Nov 13 08:01:59 2013
>> @@ -43,7 +43,6 @@ MCAsmInfoCOFF::MCAsmInfoCOFF() {
>> void MCAsmInfoMicrosoft::anchor() { }
>>
>> MCAsmInfoMicrosoft::MCAsmInfoMicrosoft() {
>> - AllowQuotesInName = true;
>> }
>>
>> void MCAsmInfoGNUCOFF::anchor() { }
>>
>> Modified: llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp (original)
>> +++ llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp Wed Nov 13 08:01:59 2013
>> @@ -26,7 +26,6 @@ MCAsmInfoDarwin::MCAsmInfoDarwin() {
>> GlobalPrefix = "_";
>> PrivateGlobalPrefix = "L";
>> LinkerPrivateGlobalPrefix = "l";
>> - AllowQuotesInName = true;
>> HasSingleParameterDotFile = false;
>> HasSubsectionsViaSymbols = true;
>>
>>
>> Modified: llvm/trunk/lib/MC/MCSectionELF.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCSectionELF.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCSectionELF.cpp (original)
>> +++ llvm/trunk/lib/MC/MCSectionELF.cpp Wed Nov 13 08:01:59 2013
>> @@ -32,6 +32,29 @@ bool MCSectionELF::ShouldOmitSectionDire
>> return false;
>> }
>>
>> +static void printName(raw_ostream &OS, StringRef Name) {
>> + if (Name.find_first_not_of("0123456789_."
>> + "abcdefghijklmnopqrstuvwxyz"
>> + "ABCDEFGHIJKLMNOPQRSTUVWXYZ") == Name.npos)
>> {
>> + OS << Name;
>> + return;
>> + }
>> + OS << '"';
>> + for (const char *B = Name.begin(), *E = Name.end(); B < E; ++B) {
>> + if (*B == '"') // Unquoted "
>> + OS << "\\\"";
>> + else if (*B != '\\') // Neither " or backslash
>> + OS << *B;
>> + else if (B + 1 == E) // Trailing backslash
>> + OS << "\\\\";
>> + else {
>> + OS << B[0] << B[1]; // Quoted character
>> + ++B;
>> + }
>> + }
>> + OS << '"';
>> +}
>> +
>> void MCSectionELF::PrintSwitchToSection(const MCAsmInfo &MAI,
>> raw_ostream &OS,
>> const MCExpr *Subsection) const {
>> @@ -44,27 +67,8 @@ void MCSectionELF::PrintSwitchToSection(
>> return;
>> }
>>
>> - StringRef name = getSectionName();
>> - if (name.find_first_not_of("0123456789_."
>> - "abcdefghijklmnopqrstuvwxyz"
>> - "ABCDEFGHIJKLMNOPQRSTUVWXYZ") == name.npos)
>> {
>> - OS << "\t.section\t" << name;
>> - } else {
>> - OS << "\t.section\t\"";
>> - for (const char *b = name.begin(), *e = name.end(); b < e; ++b) {
>> - if (*b == '"') // Unquoted "
>> - OS << "\\\"";
>> - else if (*b != '\\') // Neither " or backslash
>> - OS << *b;
>> - else if (b + 1 == e) // Trailing backslash
>> - OS << "\\\\";
>> - else {
>> - OS << b[0] << b[1]; // Quoted character
>> - ++b;
>> - }
>> - }
>> - OS << '"';
>> - }
>> + OS << "\t.section\t";
>> + printName(OS, getSectionName());
>>
>> // Handle the weird solaris syntax if desired.
>> if (MAI.usesSunStyleELFSectionSwitchSyntax() &&
>> @@ -135,8 +139,11 @@ void MCSectionELF::PrintSwitchToSection(
>> OS << "," << EntrySize;
>> }
>>
>> - if (Flags & ELF::SHF_GROUP)
>> - OS << "," << Group->getName() << ",comdat";
>> + if (Flags & ELF::SHF_GROUP) {
>> + OS << ",";
>> + printName(OS, Group->getName());
>> + OS << ",comdat";
>> + }
>> OS << '\n';
>>
>> if (Subsection)
>>
>> Modified: llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
>> (original)
>> +++ llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp Wed Nov
>> 13 08:01:59 2013
>> @@ -24,6 +24,5 @@ MSP430MCAsmInfo::MSP430MCAsmInfo(StringR
>> CommentString = ";";
>>
>> AlignmentIsInBytes = false;
>> - AllowNameToStartWithDigit = true;
>> UsesELFSectionDirectiveForBSS = true;
>> }
>>
>> Modified: llvm/trunk/lib/Target/Mangler.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mangler.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/Mangler.cpp (original)
>> +++ llvm/trunk/lib/Target/Mangler.cpp Wed Nov 13 08:01:59 2013
>> @@ -23,17 +23,6 @@
>> #include "llvm/Support/raw_ostream.h"
>> using namespace llvm;
>>
>> -static bool isAcceptableChar(char C, bool AllowPeriod) {
>> - if ((C < 'a' || C > 'z') &&
>> - (C < 'A' || C > 'Z') &&
>> - (C < '0' || C > '9') &&
>> - C != '_' && C != '$' && C != '@' &&
>> - !(AllowPeriod && C == '.') &&
>> - !(C & 0x80))
>> - return false;
>> - return true;
>> -}
>> -
>> static char HexDigit(int V) {
>> return V < 10 ? V+'0' : V+'A'-10;
>> }
>> @@ -45,46 +34,6 @@ static void MangleLetter(SmallVectorImpl
>> OutName.push_back('_');
>> }
>>
>> -/// NameNeedsEscaping - Return true if the identifier \p Str needs quotes
>> -/// for this assembler.
>> -static bool NameNeedsEscaping(StringRef Str, const MCAsmInfo *MAI) {
>> - assert(!Str.empty() && "Cannot create an empty MCSymbol");
>> -
>> - // If the first character is a number and the target does not allow
>> this, we
>> - // need quotes.
>> - if (!MAI->doesAllowNameToStartWithDigit() && Str[0] >= '0' && Str[0]
>> <= '9')
>> - return true;
>> -
>> - // If any of the characters in the string is an unacceptable
>> character, force
>> - // quotes.
>> - bool AllowPeriod = MAI->doesAllowPeriodsInName();
>> - for (unsigned i = 0, e = Str.size(); i != e; ++i)
>> - if (!isAcceptableChar(Str[i], AllowPeriod))
>> - return true;
>> - return false;
>> -}
>> -
>> -/// appendMangledName - Add the specified string in mangled form if it
>> uses
>> -/// any unusual characters.
>> -static void appendMangledName(SmallVectorImpl<char> &OutName, StringRef
>> Str,
>> - const MCAsmInfo *MAI) {
>> - // The first character is not allowed to be a number unless the target
>> - // explicitly allows it.
>> - if (!MAI->doesAllowNameToStartWithDigit() && Str[0] >= '0' && Str[0]
>> <= '9') {
>> - MangleLetter(OutName, Str[0]);
>> - Str = Str.substr(1);
>> - }
>> -
>> - bool AllowPeriod = MAI->doesAllowPeriodsInName();
>> - for (unsigned i = 0, e = Str.size(); i != e; ++i) {
>> - if (!isAcceptableChar(Str[i], AllowPeriod))
>> - MangleLetter(OutName, Str[i]);
>> - else
>> - OutName.push_back(Str[i]);
>> - }
>> -}
>> -
>> -
>> /// appendMangledQuotedName - On systems that support quoted symbols, we
>> still
>> /// have to escape some (obscure) characters like " and \n which would
>> break the
>> /// assembler's lexing.
>> @@ -134,22 +83,14 @@ void Mangler::getNameWithPrefix(SmallVec
>> OutName.append(Prefix, Prefix+strlen(Prefix));
>> }
>> }
>> -
>> +
>> // If this is a simple string that doesn't need escaping, just append
>> it.
>> - if (!NameNeedsEscaping(Name, MAI) ||
>> - // If quotes are supported, they can be used unless the string
>> contains
>> - // a quote or newline.
>> - (MAI->doesAllowQuotesInName() &&
>> - Name.find_first_of("\n\"") == StringRef::npos)) {
>> + // Quotes can be used unless the string contains a quote or newline.
>> + if (Name.find_first_of("\n\"") == StringRef::npos) {
>> OutName.append(Name.begin(), Name.end());
>> return;
>> }
>> -
>> - // On systems that do not allow quoted names, we need to mangle most
>> - // strange characters.
>> - if (!MAI->doesAllowQuotesInName())
>> - return appendMangledName(OutName, Name, MAI);
>> -
>> +
>> // Okay, the system allows quoted strings. We can quote most
>> anything, the
>> // only characters that need escaping are " and \n.
>> assert(Name.find_first_of("\n\"") != StringRef::npos);
>>
>> Modified: llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp (original)
>> +++ llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp Wed Nov
>> 13 08:01:59 2013
>> @@ -35,8 +35,6 @@ NVPTXMCAsmInfo::NVPTXMCAsmInfo(const Str
>>
>> PrivateGlobalPrefix = "$L__";
>>
>> - AllowPeriodsInName = false;
>> -
>> HasSetDirective = false;
>>
>> HasSingleParameterDotFile = false;
>>
>> Modified: llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp (original)
>> +++ llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp Wed Nov
>> 13 08:01:59 2013
>> @@ -31,9 +31,6 @@ AMDGPUMCAsmInfo::AMDGPUMCAsmInfo(StringR
>> InlineAsmStart = ";#ASMSTART";
>> InlineAsmEnd = ";#ASMEND";
>> AssemblerDialect = 0;
>> - AllowQuotesInName = false;
>> - AllowNameToStartWithDigit = false;
>> - AllowPeriodsInName = false;
>>
>> //===--- Data Emission Directives
>> -------------------------------------===//
>> ZeroDirective = ".zero";
>>
>> Modified: llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll Wed Nov 13 08:01:59 2013
>> @@ -2,23 +2,23 @@
>>
>> define i32 @main(i32 %x) nounwind gc "ocaml" {
>> ; CHECK: .text
>> -; CHECK-NEXT: .globl caml_3C_stdin_3E___code_begin
>> -; CHECK-NEXT: caml_3C_stdin_3E___code_begin:
>> +; CHECK-NEXT: .globl "caml<stdin>__code_begin"
>> +; CHECK-NEXT: "caml<stdin>__code_begin":
>> ; CHECK-NEXT: .data
>> -; CHECK-NEXT: .globl caml_3C_stdin_3E___data_begin
>> -; CHECK-NEXT: caml_3C_stdin_3E___data_begin:
>> +; CHECK-NEXT: .globl "caml<stdin>__data_begin"
>> +; CHECK-NEXT: "caml<stdin>__data_begin":
>>
>> %puts = tail call i32 @foo(i32 %x)
>> ret i32 0
>>
>> -; CHECK: .globl caml_3C_stdin_3E___code_end
>> -; CHECK-NEXT: caml_3C_stdin_3E___code_end:
>> +; CHECK: .globl "caml<stdin>__code_end"
>> +; CHECK-NEXT: "caml<stdin>__code_end":
>> ; CHECK-NEXT: .data
>> -; CHECK-NEXT: .globl caml_3C_stdin_3E___data_end
>> -; CHECK-NEXT: caml_3C_stdin_3E___data_end:
>> +; CHECK-NEXT: .globl "caml<stdin>__data_end"
>> +; CHECK-NEXT: "caml<stdin>__data_end":
>> ; CHECK-NEXT: .quad 0
>> -; CHECK-NEXT: .globl caml_3C_stdin_3E___frametable
>> -; CHECK-NEXT: caml_3C_stdin_3E___frametable:
>> +; CHECK-NEXT: .globl "caml<stdin>__frametable"
>> +; CHECK-NEXT: "caml<stdin>__frametable":
>> ; CHECK-NEXT: .short 1
>> ; CHECK-NEXT: .align 8
>> ; CHECK-NEXT: # live roots for main
>>
>> Modified: llvm/trunk/test/CodeGen/X86/global-sections.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/global-sections.ll?rev=194575&r1=194574&r2=194575&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/global-sections.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/global-sections.ll Wed Nov 13 08:01:59
>> 2013
>> @@ -65,10 +65,10 @@
>> ; PR4584
>> @"foo bar" = linkonce global i32 42
>>
>> -; LINUX: .type foo_20_bar, at object
>> -; LINUX: .section .data.foo_20_bar,"aGw", at progbits,foo_20_bar,comdat
>> -; LINUX: .weak foo_20_bar
>> -; LINUX: foo_20_bar:
>> +; LINUX: .type "foo bar", at object
>> +; LINUX: .section ".data.foo bar","aGw", at progbits,"foo bar",comdat
>> +; LINUX: .weak "foo bar"
>> +; LINUX: "foo bar":
>>
>> ; DARWIN: .section __DATA,__datacoal_nt,coalesced
>> ; DARWIN: .globl "_foo bar"
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131113/9923bf6d/attachment.html>
More information about the llvm-commits
mailing list