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

Evgeniy Stepanov eugenis at google.com
Wed Oct 1 01:39:52 PDT 2014


D5547 is good, but I'd also like a line in asm_cfi.ll test that checks that we don't add any CFI instructions if there is no .cfi_startproc (i.e. unwind tables are off).

================
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) {
----------------
ygorshenin wrote:
> 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.
Yes, please rename.


================
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;
----------------
ygorshenin wrote:
> 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).
And this makes MCDwarfFrameInfo::CfaRegister unreliable, which is not so good.
There are target-dependent initial CFI instructions, see MCAsmInfo::addInitialFrameState. Could you make CfaRegister take them into account?

http://reviews.llvm.org/D5520






More information about the llvm-commits mailing list