[PATCH for-4.21] symbols: avoid emitting "end" symbols for data items

[PATCH for-4.21] symbols: avoid emitting "end" symbols for data items

[Jan Beulich]

symbols-dummy.c and the generated .xen-syms.?.S may place their symbols in
different sections: Like for all C files, -fdata-sections may be in effect
there. As a result, besides moving these symbols may then also have
different amounts of "end" symbols inserted between them. While the
movement is likely not problematic, the change in table size is - linking
passes 2 and 3 want no address (and hence no size) changes between them.

As, at least right now, the "end" symbols are useful only for code, limit
their emission accordingly. When data symbols are emitted (i.e. when
LIVEPATCH=y), this obviously also has a positive effect on overall table
size (I'm seeing almost 600 entries going away in the build I'm looking
at).

Fixes: d3b637fba31b ("symbols: arrange to know where functions end")
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Furthermore at least some gcc versions emit the (read-only) data there into
.bss.symbols_* rather than the expected (but still potentially problematic)
.rodata.symbols_*.

--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -176,10 +176,9 @@ static int read_symbol(FILE *in, struct
 		*sym++ = '#';
 	}
 	strcpy(sym, str);
-	if (sort_by_name || map_only) {
+	if (sort_by_name || map_only)
 		s->orig_symbol = strdup(SYMBOL_NAME(s));
-		s->type = stype; /* As s->sym[0] ends mangled. */
-	}
+	s->type = stype; /* As s->sym[0] may end up mangled. */
 	s->sym[0] = stype;
 	s->typed = strcmp(type, "FUNC") == 0 ||
 	           strcmp(type, "OBJECT") == 0 ||
@@ -313,6 +312,7 @@ static int compare_name_orig(const void
 static bool want_symbol_end(unsigned int idx)
 {
 	return table[idx].size &&
+	       toupper(table[idx].type) == 'T' &&
 	       (idx + 1 == table_cnt ||
 	        table[idx].addr + table[idx].size < table[idx + 1].addr);
 }

Re: [PATCH for-4.21] symbols: avoid emitting "end" symbols for data items

[Andrew Cooper]

On 29/10/2025 1:34 pm, Jan Beulich wrote:
> symbols-dummy.c and the generated .xen-syms.?.S may place their symbols in
> different sections: Like for all C files, -fdata-sections may be in effect
> there. As a result, besides moving these symbols may then also have
> different amounts of "end" symbols inserted between them.

Sorry, I can't parse this sentence.  Do you mean "these symbols, there
may also be" ?

>  While the
> movement is likely not problematic, the change in table size is - linking
> passes 2 and 3 want no address (and hence no size) changes between them.
>
> As, at least right now, the "end" symbols are useful only for code, limit
> their emission accordingly. When data symbols are emitted (i.e. when
> LIVEPATCH=y), this obviously also has a positive effect on overall table
> size (I'm seeing almost 600 entries going away in the build I'm looking
> at).

Xen-crashdump-analyser needs end in System.map, and I expect so does
`crash`.

As this patch only adjusts the embedded symbol table, I think that's all
fine?

~Andrew

Re: [PATCH for-4.21] symbols: avoid emitting "end" symbols for data items

[Jan Beulich]

On 29.10.2025 16:13, Andrew Cooper wrote:
> On 29/10/2025 1:34 pm, Jan Beulich wrote:
>> symbols-dummy.c and the generated .xen-syms.?.S may place their symbols in
>> different sections: Like for all C files, -fdata-sections may be in effect
>> there. As a result, besides moving these symbols may then also have
>> different amounts of "end" symbols inserted between them.
> 
> Sorry, I can't parse this sentence.  Do you mean "these symbols, there
> may also be" ?

Oh, yes, I screwed up there. (I think it read half-way sensible to me when
inserting a mental comma after "moving".) Really I'm intending to go with
"... besides these symbols moving, there may then also be ..."

>>  While the
>> movement is likely not problematic, the change in table size is - linking
>> passes 2 and 3 want no address (and hence no size) changes between them.
>>
>> As, at least right now, the "end" symbols are useful only for code, limit
>> their emission accordingly. When data symbols are emitted (i.e. when
>> LIVEPATCH=y), this obviously also has a positive effect on overall table
>> size (I'm seeing almost 600 entries going away in the build I'm looking
>> at).
> 
> Xen-crashdump-analyser needs end in System.map, and I expect so does
> `crash`.
> 
> As this patch only adjusts the embedded symbol table, I think that's all
> fine?

I think so. With "end" (quoted) I never mean symbols with the name 'end'
here, but rather those that tools/symbols injects (as unnamed ones). No
symbols with names (including ones named 'end') will be affected / removed.
(I think though that it's '_end' anyway that you mean.)

Jan

Re: [PATCH for-4.21] symbols: avoid emitting "end" symbols for data items

[Roger Pau Monné]

On Wed, Oct 29, 2025 at 02:34:29PM +0100, Jan Beulich wrote:
> symbols-dummy.c and the generated .xen-syms.?.S may place their symbols in
> different sections: Like for all C files, -fdata-sections may be in effect
> there. As a result, besides moving these symbols may then also have
> different amounts of "end" symbols inserted between them. While the
> movement is likely not problematic, the change in table size is - linking
> passes 2 and 3 want no address (and hence no size) changes between them.
> 
> As, at least right now, the "end" symbols are useful only for code, limit
> their emission accordingly. When data symbols are emitted (i.e. when
> LIVEPATCH=y), this obviously also has a positive effect on overall table
> size (I'm seeing almost 600 entries going away in the build I'm looking
> at).
> 
> Fixes: d3b637fba31b ("symbols: arrange to know where functions end")
> Reported-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Thanks, this does seem to solve the issue I was seeing with clang +
LLD.

Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Roger Pau Monné <roger.pau@citrix.com>

With the commit message adjustment that you discussed with Andrew.

Roger.

Re: [PATCH for-4.21] symbols: avoid emitting "end" symbols for data items

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 2068 bytes --]


On 10/29/25 2:34 PM, Jan Beulich wrote:
> symbols-dummy.c and the generated .xen-syms.?.S may place their symbols in
> different sections: Like for all C files, -fdata-sections may be in effect
> there. As a result, besides moving these symbols may then also have
> different amounts of "end" symbols inserted between them. While the
> movement is likely not problematic, the change in table size is - linking
> passes 2 and 3 want no address (and hence no size) changes between them.
>
> As, at least right now, the "end" symbols are useful only for code, limit
> their emission accordingly. When data symbols are emitted (i.e. when
> LIVEPATCH=y), this obviously also has a positive effect on overall table
> size (I'm seeing almost 600 entries going away in the build I'm looking
> at).
>
> Fixes: d3b637fba31b ("symbols: arrange to know where functions end")
> Reported-by: Roger Pau Monné<roger.pau@citrix.com>
> Signed-off-by: Jan Beulich<jbeulich@suse.com>

Release-Acked-By: Oleksii Kurochko<oleksii.kurochko@gmail.com>

Thanks.

~ Oleskii


> ---
> Furthermore at least some gcc versions emit the (read-only) data there into
> .bss.symbols_* rather than the expected (but still potentially problematic)
> .rodata.symbols_*.
>
> --- a/xen/tools/symbols.c
> +++ b/xen/tools/symbols.c
> @@ -176,10 +176,9 @@ static int read_symbol(FILE *in, struct
>   		*sym++ = '#';
>   	}
>   	strcpy(sym, str);
> -	if (sort_by_name || map_only) {
> +	if (sort_by_name || map_only)
>   		s->orig_symbol = strdup(SYMBOL_NAME(s));
> -		s->type = stype; /* As s->sym[0] ends mangled. */
> -	}
> +	s->type = stype; /* As s->sym[0] may end up mangled. */
>   	s->sym[0] = stype;
>   	s->typed = strcmp(type, "FUNC") == 0 ||
>   	           strcmp(type, "OBJECT") == 0 ||
> @@ -313,6 +312,7 @@ static int compare_name_orig(const void
>   static bool want_symbol_end(unsigned int idx)
>   {
>   	return table[idx].size &&
> +	       toupper(table[idx].type) == 'T' &&
>   	       (idx + 1 == table_cnt ||
>   	        table[idx].addr + table[idx].size < table[idx + 1].addr);
>   }

[-- Attachment #2: Type: text/html, Size: 2812 bytes --]