Stop resetting SanitizeAddress in TargetMachine::resetTargetOptions

[ahatanak]

Summary

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).

[echristo]

Hi Akira,

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

Thanks!

-eric

[ahatanak]

In D9570#169482, @echristo wrote:

Hi Akira,

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

Hi Eric,

Wouldn't getting rid of the resetting completely make it impossible to turn instrumentation on and off on a per-function basis? I don't know how important it is to have that capability, but it looks like r206971 was committed to enable it.

[echristo]

That's not quite what I meant. I meant just have the places that need to
know whether or not we're sanitizing look at the function attribute rather
than resetting a value on MCOptions.

Thanks!

-eric

[ahatanak]

MCTargetOptions::SanitizeAddress is used in CreateX86AsmInstrumentation, which is called by X86AsmParser's constructor. Those functions don't have access to Function and therefore cannot look at the function attribute (they can be called when llvm-mc is used with -asm-instrumentation=address).

[echristo]

Ah, this is terrible. I think this needs to be rewritten as an MI level
pass at which point it'll have access to the MachineFunction etc.

Kostya?

-eric

[kcc]

+eugenis

[echristo]

Yeah, this is all terrible. I see why we can't get around this right now
without rewriting all of this and that's pretty bad.

I think Akira's solution is probably the best for the moment, but we might
want to think about rewriting this work as a late MI level pass... or
something in the future.

(Yes, consider this as an LGTM Akira). :)

-eric

[ahatanak]

Thanks, I'll commit this patch shortly.

llvm/trunk/include/llvm/CodeGen/AsmPrinter.h

 class MCCFIInstruction;
 class MCContext;
 class MCExpr;
 class MCInst;
 class MCSection;
 class MCStreamer;
 class MCSubtargetInfo;
 class MCSymbol;
+class MCTargetOptions;
 class MDNode;
 class DwarfDebug;
 class Mangler;
 class TargetLoweringObjectFile;
 class DataLayout;
 class TargetMachine;
 
 /// This class is intended to be used as a driving class for all asm writers.
private:
   mutable unsigned Counter;
 
   /// This method emits the header for the current function.
   void EmitFunctionHeader();
 
   /// 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;
 
   /// This method formats and emits the specified machine instruction that is an
   /// inline asm.
   void EmitInlineAsm(const MachineInstr *MI) const;
 
   //===------------------------------------------------------------------===//

llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

bool AsmPrinter::doInitialization(Module &M) {
   // Emit module-level inline asm if it exists.
   if (!M.getModuleInlineAsm().empty()) {
     // We're at the module level. Construct MCSubtarget from the default CPU
     // and target triple.
     std::unique_ptr<MCSubtargetInfo> STI(TM.getTarget().createMCSubtargetInfo(
         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();
   }
 
   if (MAI->doesSupportDebugInformation()) {
     bool skip_dwarf = false;
     if (Triple(TM.getTargetTriple()).isKnownWindowsMSVCEnvironment()) {
       Handlers.push_back(HandlerInfo(new WinCodeViewLineTables(this),

llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

if (LocInfo->getNumOperands() != 0)
         LocCookie = CI->getZExtValue();
   }
 
   DiagInfo->DiagHandler(Diag, DiagInfo->DiagContext, LocCookie);
 }
 
 /// 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");
 
   // Remember if the buffer is nul terminated or not so we can avoid a copy.
   bool isNullTerminated = Str.back() == 0;
   if (isNullTerminated)
     Str = Str.substr(0, Str.size()-1);
void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
   MCSubtargetInfo TmpSTI = STI;
 
   // We create a new MCInstrInfo here since we might be at the module level
   // and not have a MachineFunction to initialize the TargetInstrInfo from and
   // we only need MCInstrInfo for asm parsing. We create one unconditionally
   // 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");
   Parser->setAssemblerDialect(Dialect);
   Parser->setTargetParser(*TAP.get());
   if (MF) {
     const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
     TAP->SetFrameRegister(TRI->getFrameRegister(*MF));
void AsmPrinter::EmitInlineAsm(const MachineInstr *MI) const {
   InlineAsm::AsmDialect InlineAsmVariant = MI->getInlineAsmDialect();
   AsmPrinter *AP = const_cast<AsmPrinter*>(this);
   if (InlineAsmVariant == InlineAsm::AD_ATT)
     EmitGCCInlineAsmStr(AsmStr, MI, MMI, InlineAsmVariant, AsmPrinterVariant,
                         AP, LocCookie, OS);
   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.
   OutStreamer->emitRawComment(MAI->getInlineAsmEnd());
 }
 
 
 /// PrintSpecial - Print information related to the specified machine instr

llvm/trunk/lib/Target/TargetMachine.cpp

#define RESET_OPTION(X, Y)                                                     \
   } while (0)
 
   RESET_OPTION(NoFramePointerElim, "no-frame-pointer-elim");
   RESET_OPTION(LessPreciseFPMADOption, "less-precise-fpmad");
   RESET_OPTION(UnsafeFPMath, "unsafe-fp-math");
   RESET_OPTION(NoInfsFPMath, "no-infs-fp-math");
   RESET_OPTION(NoNaNsFPMath, "no-nans-fp-math");
   RESET_OPTION(DisableTailCalls, "disable-tail-calls");
-
-  Options.MCOptions.SanitizeAddress = F.hasFnAttribute(Attribute::SanitizeAddress);
 }
 
 /// getRelocationModel - Returns the code generation relocation model. The
 /// choices are static, PIC, and dynamic-no-pic, and target default.
 Reloc::Model TargetMachine::getRelocationModel() const {
   if (!CodeGenInfo)
     return Reloc::Default;
   return CodeGenInfo->getRelocationModel();