Re: [PATCH v2 1/5] symbols: add minimal self-test

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.08.2025 16:24, Roger Pau Monné wrote:
> On Wed, Apr 02, 2025 at 03:57:57PM +0200, Jan Beulich wrote:
>> ... before making changes to the involved logic.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> While Andrew validly suggests cf_check isn't a requirement for selecting
>> which function(s) to use (with the non-upstream gcc patch that we're
>> using in CI), that's only because of how the non-upstream patch works.
>> Going function-pointer -> unsigned long -> function-pointer without it
>> being diagnosed that the cf_check is missing is a shortcoming there, and
>> might conceivably be fixed at some point. (Imo any address-taking on a
>> function should require it to be cf_check.) Hence I'd like to stick to
>> using cf_check functions only for passing to test_lookup().
>>
>> With this FAST_SYMBOL_LOOKUP may make sense to permit enabling even
>> when LIVEPATCH=n. Thoughts? (In this case "symbols: centralize and re-
>> arrange $(all_symbols) calculation" would want pulling ahead.)
>>
>> --- a/xen/common/symbols.c
>> +++ b/xen/common/symbols.c
>> @@ -260,6 +260,41 @@ unsigned long symbols_lookup_by_name(con
>>      return 0;
>>  }
>>  
>> +#ifdef CONFIG_SELF_TESTS
>> +
>> +static void __init test_lookup(unsigned long addr, const char *expected)
>> +{
>> +    char buf[KSYM_NAME_LEN + 1];
>> +    const char *name, *symname;
>> +    unsigned long size, offs;
>> +
>> +    name = symbols_lookup(addr, &size, &offs, buf);
>> +    if ( !name )
>> +        panic("%s: address not found\n", expected);
>> +    if ( offs )
>> +        panic("%s: non-zero offset (%#lx) unexpected\n", expected, offs);
> 
> If there's a non-zero offset returned, could you also print the
> returned name? (so use %s+%#lx) there's a change the returned name
> doesn't match what we expect if there's a non-zero offset.

Hmm, perhaps we could, even if that's not the main goal of the test. Note
though that the patch has gone in already, with Jason's R-b.

> The rest LGTM:
> 
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Nevertheless, thanks.

Jan