Re: [PATCH][RFC] API extension for binutils (type of symbols).

["Martin Liška" <mliska@suse.cz>]

On 3/11/20 11:22 AM, Richard Biener wrote:
> On Wed, Mar 11, 2020 at 10:19 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 3/10/20 1:07 PM, Martin Liška wrote:
>>> On 3/10/20 12:24 PM, Richard Biener wrote:
>>>> Not sure how symtab is encoded right now but we also could have
>>>
>>> Ok, right now I don't see symtab entry much extensible.
>>>
>>> But what am I suggesting is to parse LTO bytecode version and then
>>> process conditional parsing of lto_symtab section.
>>>
>>> Thoughts?
>>> Martin
>>
>> So as H.J. correctly pointed I can't add new symbol_type into ld_plugin_symbol struct.
>> It would make ABI change as correctly identified by abidiff:
>>
>> abidiff /tmp/before.o /tmp/after.o
>> Functions changes summary: 0 Removed, 1 Changed, 0 Added function
>> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>> Function symbols changes summary: 0 Removed, 0 Added function symbol not referenced by debug info
>> Variable symbols changes summary: 0 Removed, 1 Added variable symbol not referenced by debug info
>>
>> 1 function with some indirect sub-type change:
>>
>>     [C]'function ld_plugin_status onload(ld_plugin_tv*)' at lto-plugin.c:1275:1 has some indirect sub-type changes:
>>       parameter 1 of type 'ld_plugin_tv*' has sub-type changes:
>>         in pointed to type 'struct ld_plugin_tv' at plugin-api.h:451:1:
>>           type size hasn't changed
>>           1 data member changes (1 filtered):
>>            type of 'union {int tv_val; const char* tv_string; ld_plugin_register_claim_file tv_register_claim_file; ld_plugin_register_all_symbols_read tv_register_all_symbols_read; ld_plugin_register_cleanup tv_register_cleanup; ld_plugin_add_symbols tv_add_symbols; ld_plugin_get_symbols tv_get_symbols; ld_plugin_add_input_file tv_add_input_file; ld_plugin_message tv_message; ld_plugin_get_input_file tv_get_input_file; ld_plugin_get_view tv_get_view; ld_plugin_release_input_file tv_release_input_file; ld_plugin_add_input_library tv_add_input_library; ld_plugin_set_extra_library_path tv_set_extra_library_path; ld_plugin_get_input_section_count tv_get_input_section_count; ld_plugin_get_input_section_type tv_get_input_section_type; ld_plugin_get_input_section_name tv_get_input_section_name; ld_plugin_get_input_section_contents tv_get_input_section_contents; ld_plugin_update_section_order tv_update_section_order; ld_plugin_allow_section_ordering tv_allow_section_ordering; ld_plugin_allow_unique_segment_for_sections tv_allow_unique_segment_for_sections; ld_plugin_unique_segment_for_sections tv_unique_segment_for_sections; ld_plugin_get_input_section_alignment tv_get_input_section_alignment; ld_plugin_get_input_section_size tv_get_input_section_size; ld_plugin_register_new_input tv_register_new_input; ld_plugin_get_wrap_symbols tv_get_wrap_symbols;} ld_plugin_tv::tv_u' changed:
>>              type size hasn't changed
>>              1 data member changes (1 filtered):
>>               type of 'ld_plugin_add_symbols tv_add_symbols' changed:
>>                 underlying type 'enum ld_plugin_status (void*, int, const ld_plugin_symbol*)*' changed:
>>                   in pointed to type 'function type enum ld_plugin_status (void*, int, const ld_plugin_symbol*)':
>>                     parameter 3 of type 'const ld_plugin_symbol*' has sub-type changes:
>>                       in pointed to type 'const ld_plugin_symbol':
>>                         in unqualified underlying type 'struct ld_plugin_symbol' at plugin-api.h:86:1:
>>                           type size changed from 256 to 288 (in bits)
>>                           1 data member insertion:
>>                             'int ld_plugin_symbol::symbol_type', at offset 256 (in bits) at plugin-api.h:95:1
>>
>> So that I need to come up with ld_plugin_symbol_v2. It brings more challenges: one has 2 parallel symbol
>> tables:
>>
>> struct plugin_symtab
>> {
>> ...
>>     struct ld_plugin_symbol_v2 *syms;
>>     struct ld_plugin_symbol *syms_v1;
>> ...
>> };
>>
>> and the information of these should by aligned.
>>
>> The patch can survive lto.exp and I would like to ask H.J. to write bintuils counterpart that will
>> utilize the new LDPT_GET_SYMBOLS_V4, LDPT_ADD_SYMBOLS_V2.
>>
>> Thoughts?
> 
> Can't we simply have _V4/V2 use the upper half of
> ld_plugin_symbol::def?  If the linker
> then requests _V4 but the plugin cannot cope it could still "use" the
> data but get
> LDST_UNKNOWN (zero) there.

Can be possible, but it's hack a bit. The plugin has a mechanisms for versioning
and this change does not align with the idea.

> 
> IMHO LDST_VARIABLE_BSS is "misplaced"?  "BSS" is the section of the variable.

Yes.

> If we want to encode more of ELF it should be LDST_OBJECT and LDST_FUNC.
> Note there's also rodata vs data info that would be missing in case
> we'd want tools
> like readelf -s dump the symbol table of the IL part of an object.  It
> looks like
> nm can also distinguish rodata from data ("R", "r" vs "d") and "small object"
> data sections (not sure what's that about).  It seems nm cannot distinguish
> symbols in mergeable string sections (it dumps "R" for me there).  So intead
> of mangling everything into enum ld_plugin_symbol_type should we instead
> add a
> 
> enum ld_plugin_symbol_special_section
> {
>    LDSSS_DEFAULT,
>    LDSSS_BSS,
>    LDSSS_RODATA,
>    ...
> }

Which maps to what we have for:

/* Information that is provided by all instances of the section type.  */
struct GTY(()) section_common {
   /* The set of SECTION_* flags that apply to this section.  */
   unsigned int flags;
};

#define SECTION_CODE	 0x00100	/* contains code */
...
#define SECTION_BSS	 0x02000	/* contains zeros only */
...
#define SECTION_TLS	 0x40000	/* contains thread-local storage */
...
#define SECTION_COMMON   0x800000	/* contains common data */
#define SECTION_RELRO	 0x1000000	/* data is readonly after relocation processing */
...

Anyway, that would be another type which we need in ld_plugin_symbol.

Martin

> 
> where LDSSS_DEFAULT means .text for FUNC and .data for OBJECT?
> LDSSS_COMDAT might also apply but there's already the comdat_key
> member which makes this info implicitely available.
> 
> Richard.
> 
>> Martin
>>
>>