[PATCH] Stop resetting SanitizeAddress in TargetMachine::resetTargetOptions

[Eric Christopher]

Hi Akira,

This is just moving the resetting to a different place. How about we get
rid of the resetting completely?

Thanks!

-eric

On Thu, May 7, 2015 at 12:26 PM Akira Hatanaka <ahatanak at gmail.com> wrote:

> Hi echristo, dexonsmith,
>
> This is part of the work to remove TargetMachine::resetTargetOptions (the
> FIXME added to TargetMachine.cpp in r236009 explains why this function has
> to be removed).
>
> In this patch, a temporary copy of MCTargetOptions is created and its
> SanitizeAddress field is reset based on the function's attribute every time
> an InlineAsm instruction is emitted in AsmPrinter::EmitInlineAsm. This is
> an NFC fix. Also, fixes the changes made to asm_attr.ll in r212455 which
> seems to have unintentionally dropped the "8" in the function name (I
> believe this test is supposed to fail if the line resetting SanitizeAddress
> in resetTargetOptions is removed, but it doesn't).
>
> http://reviews.llvm.org/D9570
>
> Files:
>   include/llvm/CodeGen/AsmPrinter.h
>   lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>   lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
>   lib/Target/TargetMachine.cpp
>   test/Instrumentation/AddressSanitizer/X86/asm_attr.ll
>
> Index: include/llvm/CodeGen/AsmPrinter.h
> ===================================================================
> --- include/llvm/CodeGen/AsmPrinter.h
> +++ include/llvm/CodeGen/AsmPrinter.h
> @@ -53,6 +53,7 @@
>  class MCStreamer;
>  class MCSubtargetInfo;
>  class MCSymbol;
> +class MCTargetOptions;
>  class MDNode;
>  class DwarfDebug;
>  class Mangler;
> @@ -498,6 +499,7 @@
>    /// Emit a blob of inline asm to the output streamer.
>    void
>    EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
> +                const MCTargetOptions &MCOptions,
>                  const MDNode *LocMDNode = nullptr,
>                  InlineAsm::AsmDialect AsmDialect = InlineAsm::AD_ATT)
> const;
>
> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> @@ -225,7 +225,7 @@
>          TM.getTargetTriple(), TM.getTargetCPU(),
> TM.getTargetFeatureString()));
>      OutStreamer->AddComment("Start of file scope inline assembly");
>      OutStreamer->AddBlankLine();
> -    EmitInlineAsm(M.getModuleInlineAsm()+"\n", *STI);
> +    EmitInlineAsm(M.getModuleInlineAsm()+"\n", *STI,
> TM.Options.MCOptions);
>      OutStreamer->AddComment("End of file scope inline assembly");
>      OutStreamer->AddBlankLine();
>    }
> Index: lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
> +++ lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
> @@ -74,6 +74,7 @@
>
>  /// EmitInlineAsm - Emit a blob of inline asm to the output streamer.
>  void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
> +                               const MCTargetOptions &MCOptions,
>                                 const MDNode *LocMDNode,
>                                 InlineAsm::AsmDialect Dialect) const {
>    assert(!Str.empty() && "Can't emit empty inline asm block");
> @@ -138,7 +139,7 @@
>    // because it's not subtarget dependent.
>    std::unique_ptr<MCInstrInfo> MII(TM.getTarget().createMCInstrInfo());
>    std::unique_ptr<MCTargetAsmParser> TAP(TM.getTarget().createMCAsmParser(
> -      TmpSTI, *Parser, *MII, TM.Options.MCOptions));
> +      TmpSTI, *Parser, *MII, MCOptions));
>    if (!TAP)
>      report_fatal_error("Inline asm not supported by this streamer because"
>                         " we don't have an asm parser for this target\n");
> @@ -488,7 +489,13 @@
>    else
>      EmitMSInlineAsmStr(AsmStr, MI, MMI, InlineAsmVariant, AP, LocCookie,
> OS);
>
> -  EmitInlineAsm(OS.str(), getSubtargetInfo(), LocMD,
> MI->getInlineAsmDialect());
> +  // Reset SanitizeAddress based on the function's attribute.
> +  MCTargetOptions MCOptions = TM.Options.MCOptions;
> +  MCOptions.SanitizeAddress =
> +      MF->getFunction()->hasFnAttribute(Attribute::SanitizeAddress);
> +
> +  EmitInlineAsm(OS.str(), getSubtargetInfo(), MCOptions, LocMD,
> +                MI->getInlineAsmDialect());
>
>    // Emit the #NOAPP end marker.  This has to happen even if verbose-asm
> isn't
>    // enabled, so we use emitRawComment.
> Index: lib/Target/TargetMachine.cpp
> ===================================================================
> --- lib/Target/TargetMachine.cpp
> +++ lib/Target/TargetMachine.cpp
> @@ -73,8 +73,6 @@
>    RESET_OPTION(NoNaNsFPMath, "no-nans-fp-math");
>    RESET_OPTION(UseSoftFloat, "use-soft-float");
>    RESET_OPTION(DisableTailCalls, "disable-tail-calls");
> -
> -  Options.MCOptions.SanitizeAddress =
> F.hasFnAttribute(Attribute::SanitizeAddress);
>  }
>
>  /// getRelocationModel - Returns the code generation relocation model. The
> Index: test/Instrumentation/AddressSanitizer/X86/asm_attr.ll
> ===================================================================
> --- test/Instrumentation/AddressSanitizer/X86/asm_attr.ll
> +++ test/Instrumentation/AddressSanitizer/X86/asm_attr.ll
> @@ -4,8 +4,8 @@
>  target triple = "x86_64-unknown-linux-gnu"
>
>  ; CHECK-LABEL: mov_no_attr
> -; CHECK-NOT: callq __asan_report_load at PLT
> -; CHECK-NOT: callq __asan_report_store at PLT
> +; CHECK-NOT: callq __asan_report_load8 at PLT
> +; CHECK-NOT: callq __asan_report_store8 at PLT
>  define void @mov_no_attr(i64* %dst, i64* %src) {
>    tail call void asm sideeffect "movq ($1), %rax  \0A\09movq %rax, ($0)
> \0A\09", "r,r,~{memory},~{rax},~{dirflag},~{fpsr},~{flags}"(i64* %dst, i64*
> %src)
>    ret void
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150508/fe3185d7/attachment.html>