On 6 October 2017 at 13:53, Jiri Slaby <jslaby@xxxxxxx> wrote:
> Hi,
>
> On 10/04/2017, 09:33 AM, Ard Biesheuvel wrote:
>> On 4 October 2017 at 08:22, Jiri Slaby <jslaby@xxxxxxx> wrote:
>>> On 10/02/2017, 02:48 PM, Ard Biesheuvel wrote:
>>>> On 2 October 2017 at 10:12, Jiri Slaby <jslaby@xxxxxxx> wrote:
>>>>> There is a couple of assembly functions, which are invoked only locally
>>>>> in the file they are defined. In C, we mark them "static". In assembly,
>>>>> annotate them using SYM_{FUNC,CODE}_START_LOCAL (and switch their
>>>>> ENDPROC to SYM_{FUNC,CODE}_END too). Whether FUNC or CODE depends on
>>>>> ENDPROC/END for a particular function (C or non-C).
>>>>>
>>>>
>>>> I wasn't cc'ed on the cover letter, so I am missing the rationale of
>>>> replacing ENTRY/ENDPROC with other macros.
>>>
>>> There was no cover letter. I am attaching what is in PATCH 1/27 instead:
>>> Introduce new C macros for annotations of functions and data in
>>> assembly. There is a long-standing mess in macros like ENTRY, END,
>>> ENDPROC and similar. They are used in different manners and sometimes
>>> incorrectly.
>>>
>>
>> I must say, I don't share this sentiment.
>>
>> In arm64, we use ENTRY/ENDPROC for functions with external linkage,
>> and the bare symbol name/ENDPROC for functions with local linkage. I
>> guess we could add ENDOBJECT if we wanted to, but we never really felt
>> the need.
>
> Yes and this is exactly the reason for the new macros. Now, it's a
> complete mess. One arch does this, another does that. And we are in a
> state to have reliable stacktraces, let objtool check functions, let
> objtool generate annotations (e.g. for ORC unwinder), etc.
>
You are implying that ENTRY/ENDPROC and 'bare symbol name'/ENDPROC
prevent you from doing any of this, but this is simply not true.
> Without knowing what is start, where is its end, what is function, what
> is object (data) etc., it can barely check or even generate anything
> automatically. Not speaking about impaired macros like your name/ENDPROC
> above.
>
What is the problem with impaired macros?
> There was a cleanup in x86 done by Josh and others that we have at least
> correct ENTRY+END and ENTRY+ENDPROC annotations in most cases now. Even
> though it was concluded the names are weird (leftover from the past). So
> yes, there was a discussion about the cleanup, naming and such. And I
> came up with the names in this e-mail.
>
> http://lkml.kernel.org/r/%3C20170217104757.28588-1-jslaby@xxxxxxx%3E
>
OK, but wrapping asm directives in macros will not suddenly make the
assembler care about whether you use them or, whether they are paired,
etc.
>>> So introduce macros with clear use to annotate assembly as follows:
>>>
>>> a) Support macros for the ones below
>>> SYM_T_FUNC -- type used by assembler to mark functions
>>> SYM_T_OBJECT -- type used by assembler to mark data
>>> SYM_T_NONE -- type used by assembler to mark entries of unknown type
>>>
>>
>> Is is necessary to mark an entry as having no type? What is the
>> default type for an unmarked entry?
>
> The default is indeed T_NONE. The thing is that most macros use
> SYM_START and SYM_END which requires the type as argument. So for
> convenience, we define also SYM_T_NONE used e.g. to define SYM_CODE_END:
> #define SYM_CODE_END(name) \
> SYM_END(CODE, name, SYM_T_NONE)
>
> Despite it needs not be there. But users of the macros should not care.
>
>>> They are defined as STT_FUNC, STT_OBJECT, and STT_NOTYPE
>>> respectively. According to the gas manual, this is the most portable
>>> way. I am not sure about other assemblers, so we can switch this back
>>> to %function and %object if this turns into a problem. Architectures
>>> can also override them by something like ", @function" if they need.
>>>
>>> SYM_A_ALIGN, SYM_A_NONE -- align the symbol?
>>> SYM_V_GLOBAL, SYM_V_WEAK, SYM_V_LOCAL -- visibility of symbols
>>>
>>
>> Linkage != visibility
>
> OK, I can fix this.
>
>>> d) For data
>>> SYM_DATA_START -- global data symbol
>>> SYM_DATA_END -- the end of the SYM_DATA_START symbol
>>> SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol
>>> SYM_DATA_SIMPLE -- start+end wrapper around simple global data
>>> SYM_DATA_SIMPLE_LOCAL -- start+end wrapper around simple local data
>>>
>>
>> I am sorry but I think this is terrible. Do we really need 20+ new
>> macros to wrap every single assembler directive involved in defining
>> symbols and setting their attributes?
>
> Basically, most code uses SYM_FUNC_START/END and SYM_DATA_START/END (or
> SYM_DATA_SIMPLE). The rest is special cases that _have_ to be annotated
> as such anyway (by e.g. SYM_CODE_START/END). Objtool cannot check the
> code without this reliably and it is exactly the same as using either
> END or ENDPROC for a particular function except people use these in a
> wrong way as they are undocumented.
>
So what is preventing people from using these new macros in the wrong way?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
