This is an archive of the discontinued LLVM Phabricator instance.

[asan-assembly-instrumentation] Added CFI directives to the generated instrumentation code.
ClosedPublic

Authored by ygorshenin on Sep 4 2014, 5:20 AM.

Details

Summary

[asan-assembly-instrumentation] Added CFI directives to the generated instrumentation code.

Diff Detail

Event Timeline

ygorshenin updated this revision to Diff 13250.Sep 4 2014, 5:20 AM
ygorshenin retitled this revision from to [asan-assembly-instrumentation] Added CFI directives to the generated instrumentation code..
ygorshenin updated this object.
ygorshenin edited the test plan for this revision. (Show Details)
ygorshenin added a reviewer: eugenis.
ygorshenin added a subscriber: Unknown Object (MLST).
eugenis edited edge metadata.Sep 4 2014, 5:31 AM

Tests please.

eugenis added inline comments.Sep 4 2014, 5:44 AM
include/llvm/MC/MCTargetAsmParser.h
126

Please add a comment.
How does it behave when parsing top-level asm?

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp
314

Don't forget end-to-end tests for both cases (esp and non-esp), i.e. omit-frame-pointer and no-omit-frame-pointer.

554

Is it the right way to obtain MCRegisterInfo?
Direct calls to TheXXXTarget seem to be rather uncommon.

ygorshenin updated this revision to Diff 13446.Sep 9 2014, 3:07 AM
ygorshenin edited edge metadata.

Fixes.

PTAL

End-to-end test is added in http://reviews.llvm.org/D5260.

include/llvm/MC/MCTargetAsmParser.h
126

Done.

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp
554

It seems that it's possible to get MCRegisterInfo from MCContext, thanks!

eugenis added inline comments.Sep 9 2014, 3:51 AM
include/llvm/MC/MCTargetAsmParser.h
127

It's better to document what the function does (or does not do), rather then when it is called.
For example, it looks like FrameReg stays uninitialized if it is not called, resulting in undefined behavior for top-level asm.

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll
46

attribute 1 and 3 seem to be unused
maybe remove all the fp-math stuff from 2 and 4, too

50

remove this, and all !srcloc.

PTAL

include/llvm/MC/MCTargetAsmParser.h
127

FrameReg is a field in X86AsmInstrumentation and initialized to X86::NoRegister in the ctor.

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll
46

Done.

50

Done.

eugenis accepted this revision.Sep 9 2014, 5:21 AM
eugenis edited edge metadata.

LGTM

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll
46

Please remove "stack-protector-buffer-size"="8", too.

This revision is now accepted and ready to land.Sep 9 2014, 5:21 AM
ygorshenin updated this revision to Diff 13456.Sep 9 2014, 6:11 AM
ygorshenin edited edge metadata.

Fixes.

Many thanks!

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll
46

Done.

ygorshenin closed this revision.Sep 10 2014, 2:55 AM
ygorshenin updated this revision to Diff 13522.

Closed by commit rL217482 (authored by @ygorshenin).