This revision was automatically updated to reflect the committed changes.
Closed by commit rL278050: [Attr] Add support for the `ms_hook_prologue`
attribute. (authored by cdavis).
Changed prior to commit:
https://reviews.llvm.org/D19909?vs=66729=67224#toc
Repository:
rL LLVM
cdavis5x marked an inline comment as done.
Comment at: include/clang/Basic/AttrDocs.td:560
@@ +559,3 @@
+
+This attribute cannot be used in conjunction with the ``naked``,
+``always_inline``, or ``__forceinline`` attributes.
Done.
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM (when Sanjoy's patch goes in), with two minor nits.
Can you also add a test that the attribute fails on one of the target
architectures Windows does not support?
cdavis5x updated this revision to Diff 57853.
cdavis5x marked an inline comment as done.
cdavis5x added a comment.
- Add Sema tests for the `ms_hook_prologue` attribute.
- Disallow `ms_hook_prologue` on architectures that Windows doesn't support.
- Disallow `ms_hook_prologue` with the `naked` and
cdavis5x added a comment.
For now, I've disallowed it with `naked` and `always_inline`/`__forceinline`
attributes. Do any other attributes affect prologue generation in a way that
might interfere with `ms_hook_prologue`? AFAICT, GCC only disallows
`ms_hook_prologue` on a) nested functions and
aaron.ballman added a comment.
The patch is missing semantic test cases. I would like to see tests of the
attribute being accepted, as well as rejected because the subject was
incorrect, args were provided, etc.
Also, I believe there are attributes which this attribute should not be
combined
cdavis5x marked an inline comment as done.
Comment at: include/clang/Basic/Attr.td:2032
@@ -2031,1 +2031,3 @@
+def MSHookPrologue : InheritableAttr {
+ let Spellings = [GCC<"ms_hook_prologue">];
aaron.ballman wrote:
> Does this attribute appertain to all
cdavis5x updated this revision to Diff 57389.
cdavis5x added a comment.
- Use Sanjoy's `patchable-function` attribute.
- Add documentation for this new attribute.
http://reviews.llvm.org/D19909
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/CodeGen/TargetInfo.cpp
aaron.ballman added a comment.
Generally, I think this should make use of the patchable function functionality
from http://reviews.llvm.org/D19046 when lowering, but I do have some comments
on the attribute itself as well.
Comment at: include/clang/Basic/Attr.td:2032
@@
cdavis5x created this revision.
cdavis5x added reviewers: aaron.ballman, rnk.
cdavis5x added a subscriber: cfe-commits.
cdavis5x added a dependency: D19908: [X86] Support the "ms-hotpatch" attribute..
Based on a patch by Michael Mueller.
This attribute specifies that a function can be hooked or
10 matches
Mail list logo
