<div dir="ltr">Hi Akira,<br><br>This is just moving the resetting to a different place. How about we get rid of the resetting completely?<div><br></div><div>Thanks!</div><div><br></div><div>-eric</div></div><br><div class="gmail_quote">On Thu, May 7, 2015 at 12:26 PM Akira Hatanaka <<a href="mailto:ahatanak@gmail.com">ahatanak@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi echristo, dexonsmith,<br>
<br>
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).<br>
<br>
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).<br>
<br>
<a href="http://reviews.llvm.org/D9570" target="_blank">http://reviews.llvm.org/D9570</a><br>
<br>
Files:<br>
include/llvm/CodeGen/AsmPrinter.h<br>
lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp<br>
lib/Target/TargetMachine.cpp<br>
test/Instrumentation/AddressSanitizer/X86/asm_attr.ll<br>
<br>
Index: include/llvm/CodeGen/AsmPrinter.h<br>
===================================================================<br>
--- include/llvm/CodeGen/AsmPrinter.h<br>
+++ include/llvm/CodeGen/AsmPrinter.h<br>
@@ -53,6 +53,7 @@<br>
class MCStreamer;<br>
class MCSubtargetInfo;<br>
class MCSymbol;<br>
+class MCTargetOptions;<br>
class MDNode;<br>
class DwarfDebug;<br>
class Mangler;<br>
@@ -498,6 +499,7 @@<br>
/// Emit a blob of inline asm to the output streamer.<br>
void<br>
EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,<br>
+ const MCTargetOptions &MCOptions,<br>
const MDNode *LocMDNode = nullptr,<br>
InlineAsm::AsmDialect AsmDialect = InlineAsm::AD_ATT) const;<br>
<br>
Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
===================================================================<br>
--- lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
+++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
@@ -225,7 +225,7 @@<br>
TM.getTargetTriple(), TM.getTargetCPU(), TM.getTargetFeatureString()));<br>
OutStreamer->AddComment("Start of file scope inline assembly");<br>
OutStreamer->AddBlankLine();<br>
- EmitInlineAsm(M.getModuleInlineAsm()+"\n", *STI);<br>
+ EmitInlineAsm(M.getModuleInlineAsm()+"\n", *STI, TM.Options.MCOptions);<br>
OutStreamer->AddComment("End of file scope inline assembly");<br>
OutStreamer->AddBlankLine();<br>
}<br>
Index: lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp<br>
===================================================================<br>
--- lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp<br>
+++ lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp<br>
@@ -74,6 +74,7 @@<br>
<br>
/// EmitInlineAsm - Emit a blob of inline asm to the output streamer.<br>
void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,<br>
+ const MCTargetOptions &MCOptions,<br>
const MDNode *LocMDNode,<br>
InlineAsm::AsmDialect Dialect) const {<br>
assert(!Str.empty() && "Can't emit empty inline asm block");<br>
@@ -138,7 +139,7 @@<br>
// because it's not subtarget dependent.<br>
std::unique_ptr<MCInstrInfo> MII(TM.getTarget().createMCInstrInfo());<br>
std::unique_ptr<MCTargetAsmParser> TAP(TM.getTarget().createMCAsmParser(<br>
- TmpSTI, *Parser, *MII, TM.Options.MCOptions));<br>
+ TmpSTI, *Parser, *MII, MCOptions));<br>
if (!TAP)<br>
report_fatal_error("Inline asm not supported by this streamer because"<br>
" we don't have an asm parser for this target\n");<br>
@@ -488,7 +489,13 @@<br>
else<br>
EmitMSInlineAsmStr(AsmStr, MI, MMI, InlineAsmVariant, AP, LocCookie, OS);<br>
<br>
- EmitInlineAsm(OS.str(), getSubtargetInfo(), LocMD, MI->getInlineAsmDialect());<br>
+ // Reset SanitizeAddress based on the function's attribute.<br>
+ MCTargetOptions MCOptions = TM.Options.MCOptions;<br>
+ MCOptions.SanitizeAddress =<br>
+ MF->getFunction()->hasFnAttribute(Attribute::SanitizeAddress);<br>
+<br>
+ EmitInlineAsm(OS.str(), getSubtargetInfo(), MCOptions, LocMD,<br>
+ MI->getInlineAsmDialect());<br>
<br>
// Emit the #NOAPP end marker. This has to happen even if verbose-asm isn't<br>
// enabled, so we use emitRawComment.<br>
Index: lib/Target/TargetMachine.cpp<br>
===================================================================<br>
--- lib/Target/TargetMachine.cpp<br>
+++ lib/Target/TargetMachine.cpp<br>
@@ -73,8 +73,6 @@<br>
RESET_OPTION(NoNaNsFPMath, "no-nans-fp-math");<br>
RESET_OPTION(UseSoftFloat, "use-soft-float");<br>
RESET_OPTION(DisableTailCalls, "disable-tail-calls");<br>
-<br>
- Options.MCOptions.SanitizeAddress = F.hasFnAttribute(Attribute::SanitizeAddress);<br>
}<br>
<br>
/// getRelocationModel - Returns the code generation relocation model. The<br>
Index: test/Instrumentation/AddressSanitizer/X86/asm_attr.ll<br>
===================================================================<br>
--- test/Instrumentation/AddressSanitizer/X86/asm_attr.ll<br>
+++ test/Instrumentation/AddressSanitizer/X86/asm_attr.ll<br>
@@ -4,8 +4,8 @@<br>
target triple = "x86_64-unknown-linux-gnu"<br>
<br>
; CHECK-LABEL: mov_no_attr<br>
-; CHECK-NOT: callq __asan_report_load@PLT<br>
-; CHECK-NOT: callq __asan_report_store@PLT<br>
+; CHECK-NOT: callq __asan_report_load8@PLT<br>
+; CHECK-NOT: callq __asan_report_store8@PLT<br>
define void @mov_no_attr(i64* %dst, i64* %src) {<br>
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)<br>
ret void<br>
<br>
EMAIL PREFERENCES<br>
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</blockquote></div>