[llvm-commits] [PATCH] Start of support for __attribute__((ms_hook_prologue))

[Charles Davis]

Committed in r117264.

Chip

On 10/25/10 5:03 AM, Michael Spencer wrote:
> On Mon, Oct 25, 2010 at 6:42 AM, Charles Davis <cdavis at mymail.mines.edu> wrote:
>> On 10/25/10 12:16 AM, Michael Spencer wrote:
>>> On Sun, Oct 24, 2010 at 9:43 PM, Charles Davis <cdavis at mymail.mines.edu> wrote:
>>>> Hi,
>>>>
>>>> I thought that, while I wait for a response to my latest clang "patch"
>>>> on cfe-dev, I'd work on something else Wine-related.
>>>>
>>>> Microsoft's compiler has a switch, '/hotpatch', that inserts a magic
>>>> sequence in the beginning of every compiled function. This sequence lets
>>>> other DLLs hook these functions (assuming they were exported from a DLL)
>>>> to do interesting things when they're called. In response, GCC added an
>>>> attribute, 'ms_hook_prologue', that does the same in GCC (partially at
>>>> the behest of the Wine developers). And now I intend to do the same for
>>>> Clang.
>>>>
>>>> Of course, this needs backend support. So, as with my
>>>> 'force_align_arg_pointer' patches, I'm starting with the basics. This
>>>> patch adds support for the attribute in LLVM IR. OK to commit?
>>>>
>>>> Chip
>>>
>>> I'm not convinced that a new attribute is needed.
>> All right then, let me convince you.
>>> We already have
>>> dllexport linkage, so why not just add an option that makes all
>>> dllexport functions hot patchable? It doesn't change the calling
>>> convention at all.
>> Several reasons:
>> - This adds some noop instruction(s) to the beginning of the function.
>> (On x86, it adds a 'mov %edi,%edi'.) This could potentially affect
>> performance if the function is called over and over again. Are you sure
>> you want all dllexport functions to be hotpatchable?
> 
> It actually only requires that the first instruction be two bytes.
> 
>> - Not everyone uses dllexport. Some people still use .def files. Wine is
>> one notable example (though the .def files are autogenerated). They
>> declare some of their functions with this attribute.
> 
> OK.
> 
>> - GCC already has this attribute. It's too late.
> 
> We don't have to implement all gcc features.
> 
>>>
>>> Also, this doesn't seem to include the lexing or parsing code for the keyword.
>> It was only missing the lexing code. Patch reattached.
>>>
>>> - Michael Spencer
> 
> Individually choosing functions to apply this to independent of
> dllexport is a good enough reason for me. I have no objections.