[PATCH] [asan-asm-instrumentation] CFI directives are generated for .S files.

Yuri Gorshenin ygorshenin at chromium.org
Tue Sep 30 10:45:38 PDT 2014


PTAL

================
Comment at: lib/MC/MCStreamer.cpp:278
@@ -277,2 +277,3 @@
   CurFrame->Instructions.push_back(Instruction);
+  CurFrame->CfaRegister = static_cast<unsigned>(Register);
 }
----------------
eugenis wrote:
> What about MCStreamer::EmitCFIDefCfa?
Done.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:579
@@ +578,3 @@
+    const MCRegisterInfo *MRI = Ctx.getRegisterInfo();
+    unsigned FrameReg = GetFrameReg(Ctx, Out);
+    if (MRI && FrameReg != X86::NoRegister) {
----------------
eugenis wrote:
> This is shadowing X86AsmInstrumentation::FrameReg. Confusing.
> Perhaps rename the other one to smth like "InitialFrameReg"?
That's why I moved FrameReg to the private part of the X86AsmInstrumentation and there're no shadowing. But if you still insisting I'll rename X86AsmInstrumentation::FrameReg to something like InitialFrameReg.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:850
@@ -837,1 +849,3 @@
 
+unsigned X86AsmInstrumentation::GetFrameRegGeneric(const MCContext &Ctx,
+                                                   MCStreamer &Out) {
----------------
eugenis wrote:
> Please check (and test) that all this stuff works with -fno-unwind-tables.
Sure, but could you please explain what -fno-unwind-tables does?

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:868
@@ +867,3 @@
+  // If CFA register is not specified explicitly, let's assume that it's SP.
+  if (!Frame.CfaRegister)
+    return X86::RSP;
----------------
eugenis wrote:
> Is it even possible? Can we CHECK instead?
> Is it the same situation as "No active dwarf frame" above?
MCDwarfFrameInfo::CfaRegister is set when something like .cfi_def_cfa_register was emitted. Otherwise it won't be set (even between .cfi_startproc and .cfi_endproc).

================
Comment at: test/Instrumentation/AddressSanitizer/X86/asm_cfi.s:1
@@ +1,2 @@
+# RUN: llvm-mc %s -triple=i386-unknown-linux-gnu -asm-instrumentation=address -asan-instrument-assembly | FileCheck %s
+
----------------
eugenis wrote:
> Please add at least a one-line comment on what this test is about.
> 
Done.

http://reviews.llvm.org/D5520






More information about the llvm-commits mailing list