Page MenuHomePhabricator

[X86][test] Add tests for -fpatchable-function-entry=N,M (where M>0) and its interaction with -fcf-protection=branch
ClosedPublic

Authored by MaskRay on Jan 20 2020, 4:43 PM.

Diff Detail

Event Timeline

MaskRay created this revision.Jan 20 2020, 4:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2020, 4:43 PM

Unit tests: fail. 62040 tests passed, 2 failed and 783 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_class/try_lock.pass.cpp
failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/lock.pass.cpp

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

llvm/lib/Target/X86/X86MCInstLower.cpp
2646 ↗(On Diff #239212)

See comments on D73070 about making nop emission generic.

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
39

Need to know where .Lfunc_end2 is placed, please add a CHECK for it.

llvm/test/CodeGen/X86/patchable-function-entry.ll
89

Ditto.

MaskRay updated this revision to Diff 239394.Jan 21 2020, 11:37 AM
MaskRay retitled this revision from [X86] Support -fpatchable-function-entry=N,M where M>0 to [X86][test] Add tests for -fpatchable-function-entry=N,M (where M>0) and its interaction with -fcf-protection=branch.

Delete code change.
This is a pure test patch now.

MaskRay marked 3 inline comments as done.Jan 21 2020, 11:39 AM

Unit tests: fail. 62081 tests passed, 1 failed and 784 were skipped.

failed: Clang.Driver/cc-print-options.c

clang-tidy: unknown.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
28

Should the filename of this added test be s/ibt.ll/bti.ll?

llvm/test/CodeGen/X86/patchable-function-entry.ll
95

The comment about -ffunction-sections added to the aarch64 test in https://reviews.llvm.org/D73070 would be helpful here, too.

MaskRay updated this revision to Diff 239432.Jan 21 2020, 2:00 PM

Improve tests

MaskRay updated this revision to Diff 239433.Jan 21 2020, 2:01 PM
MaskRay marked an inline comment as done.

Fix a typo

-mbranch-protection=bti -> -fcf-protection=branch

MaskRay marked an inline comment as done.Jan 21 2020, 2:02 PM
MaskRay added inline comments.
llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
28

Sorry, I made a typo.

It should be -fcf-protection=branch (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html)

This is the Intel technology which resembles to AArch64's IBT.

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
28

Yes, and I think the name of the test is still misspelled, I think.

Consider the test modified in https://reviews.llvm.org/D73070 named llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll, but this file is named llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll. bti vs ibt. Not only is the order different, but bti is only (?) aarch64 specific, so maybe the filename of this test should have fcf, not bti (or ibt).

Unit tests: pass. 62082 tests passed, 0 failed and 784 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Unit tests: pass. 62083 tests passed, 0 failed and 784 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

MaskRay marked an inline comment as done.Jan 21 2020, 2:38 PM
MaskRay added inline comments.
llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll
28

AArch64: Branch Target Identification (bti)
x86: Indirect Branch Tracking (ibt) (I recently added an lld option -z force-ibt, so I am confident here...)

I think the filename is correct, but my comment above made a typo...

This is the Intel technology which resembles to AArch64's IBT.

IBT->BTI.

nickdesaulniers accepted this revision.Jan 22 2020, 9:11 AM

Ah, sorry then for my misunderstanding.

This revision is now accepted and ready to land.Jan 22 2020, 9:11 AM
This revision was automatically updated to reflect the committed changes.