Re: [PATCH v1] xen/arm: align *(.proc.info) in the linker script

[Julien Grall <julien@xen.org>]

Hi,

On 02/03/2023 09:45, Jan Beulich wrote:
> On 01.03.2023 21:38, Julien Grall wrote:
>> On 01/03/2023 17:50, Julien Grall wrote:
>>> I got the answer. The problem now is gitlab only keep the artifact for
>>> the latest build and it only provide a zImage (having the ELF would be
>>> easier).
>>>
>>> I will try to reproduce the error on my end.
>>
>> I managed to reproduce it. It looks like that after your bug patch,
>> *(.rodata.*) will not be end on a 4-byte boundary. Before your patch,
>> all the messages will be in .rodata.str. Now they are in .bug_frames.*,
>> so there some difference in .rodata.*.
> 
> Strings in .bug_frames.*? This sounds like a mistake, which - if indeed
> the case - will want investigating before the conversion series is
> actually considered for committing.

No. I misread the code. But there are definitely a difference in size:

Before:

Section Headers:
   [Nr] Name              Type            Addr     Off    Size   ES Flg 
Lk Inf Al
   [ 0]                   NULL            00000000 000000 000000 00 
0   0  0
   [ 1] .text             PROGBITS        00200000 008000 07e7a8 00 WAX 
0   0 32
   [ 2] .rodata           PROGBITS        0027f000 087000 02acc8 00   A 
0   0 16
   [ 3] .note.gnu.build-i NOTE            002a9cc8 0b1cc8 000024 00   A 
0   0  4
   [ 4] .data.ro_after_in PROGBITS        002aa000 0b2000 001000 00  WA 
0   0  8
   [ 5] .data.read_mostly PROGBITS        002ab000 0b3000 001118 00  WA 
0   0  8
   [ 6] .data             PROGBITS        002ad000 0b5000 006ca8 00  WA 
0   0 4096
   [ 7] .arch.info        PROGBITS        002b3ca8 0bbca8 000208 00   A 
0   0  4
   [ 8] .dev.info         PROGBITS        002b3eb0 0bbeb0 000070 00   A 
0   0  4
   [ 9] .init.text        PROGBITS        002b4000 0bc000 016054 00  AX 
0   0  8
   [10] .init.data        PROGBITS        002d0000 0d8000 030008 00  WA 
0   0 32768
   [11] .bss              NOBITS          00308000 108008 0394c0 00  WA 
0   0 4096
   [12] .debug_abbrev     PROGBITS        00000000 108008 03006e 00 
0   0  1
   [13] .debug_info       PROGBITS        00000000 138076 27b6b5 00 
0   0  1
   [14] .debug_str        PROGBITS        00000000 3b372b 01a123 01  MS 
0   0  1
   [15] .debug_line       PROGBITS        00000000 3cd84e 0a59e6 00 
0   0  1
   [16] .debug_frame      PROGBITS        00000000 473234 018cc8 00 
0   0  4
   [17] .debug_loc        PROGBITS        00000000 48befc 108473 00 
0   0  1
   [18] .debug_ranges     PROGBITS        00000000 594370 031450 00 
0   0  8
   [19] .debug_aranges    PROGBITS        00000000 5c57c0 004dd0 00 
0   0  8
   [20] .comment          PROGBITS        00000000 5ca590 00005d 01  MS 
0   0  1
   [21] .ARM.attributes   ARM_ATTRIBUTES  00000000 5ca5ed 000037 00 
0   0  1
   [22] .symtab           SYMTAB          00000000 5ca624 022d60 10 
23 7573  4
   [23] .strtab           STRTAB          00000000 5ed384 00c631 00 
0   0  1
   [24] .shstrtab         STRTAB          00000000 5f99b5 00010b 00 
0   0  1

After:

   [Nr] Name              Type            Addr     Off    Size   ES Flg 
Lk Inf Al
   [ 0]                   NULL            00000000 000000 000000 00 
0   0  0
   [ 1] .text             PROGBITS        00200000 008000 07e670 00 WAX 
0   0 32
   [ 2] .rodata           PROGBITS        0027f000 087000 02b3e8 00   A 
0   0 16
   [ 3] .note.gnu.build-i NOTE            002aa3e8 0b23e8 000024 00   A 
0   0  4
   [ 4] .data.ro_after_in PROGBITS        002ab000 0b3000 001000 00  WA 
0   0  8
   [ 5] .data.read_mostly PROGBITS        002ac000 0b4000 001118 00  WA 
0   0  8
   [ 6] .data             PROGBITS        002ae000 0b6000 006ca8 00  WA 
0   0 4096
   [ 7] .arch.info        PROGBITS        002b4ca8 0bcca8 000208 00   A 
0   0  4
   [ 8] .dev.info         PROGBITS        002b4eb0 0bceb0 000070 00   A 
0   0  4
   [ 9] .init.text        PROGBITS        002b5000 0bd000 016054 00  AX 
0   0  8
   [10] .init.data        PROGBITS        002d0000 0d8000 030008 00  WA 
0   0 32768
   [11] .bss              NOBITS          00308000 108008 0394c0 00  WA 
0   0 4096
   [12] .debug_abbrev     PROGBITS        00000000 108008 02fe48 00 
0   0  1
   [13] .debug_info       PROGBITS        00000000 137e50 27ac8f 00 
0   0  1
   [14] .debug_str        PROGBITS        00000000 3b2adf 01a107 01  MS 
0   0  1
   [15] .debug_line       PROGBITS        00000000 3ccbe6 0a590e 00 
0   0  1
   [16] .debug_frame      PROGBITS        00000000 4724f4 018da0 00 
0   0  4
   [17] .debug_loc        PROGBITS        00000000 48b294 10822c 00 
0   0  1
   [18] .debug_ranges     PROGBITS        00000000 5934c0 031028 00 
0   0  8
   [19] .debug_aranges    PROGBITS        00000000 5c44e8 004dd8 00 
0   0  8
   [20] .comment          PROGBITS        00000000 5c92c0 00005d 01  MS 
0   0  1
   [21] .ARM.attributes   ARM_ATTRIBUTES  00000000 5c931d 000037 00 
0   0  1
   [22] .symtab           SYMTAB          00000000 5c9354 0220f0 10 
23 7374  4
   [23] .strtab           STRTAB          00000000 5eb444 00c677 00 
0   0  1
   [24] .shstrtab         STRTAB          00000000 5f7abb 00010b 00 
0   0  1

It is not entirely clear why. Anyway, it doesn't matter too much.

Note that the size of Xen grew a little bit on Arm (+0.03%). I am not 
too concerned though as we now consolidated the BUG infrastructure. So 
that's a plus.

> 
>> That said, it is not entirely clear why we never seen the issue before
>> because my guessing there are no guarantee that .rodata.* will be
>> suitably aligned.
>>
>> Anyway, here a proposal for the commit message:
>>
>> "
>> xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned
>>
>> The entries in *(.proc.info) are expected to be 4-byte aligned and the
>> compiler will access them using 4-byte load instructions. On Arm32, the
>> alignment is strictly enforced by the processor and will result to a
>> data abort if it is not correct.
>>
>> However, the linker script doesn't encode this requirement. So we are at
>> the mercy of the compiler/linker to have aligned the previous sections
>> suitably.
> 
> May I suggest "aligned/padded", because it's really the tail of the
> previous section which matters?

Sure.

Cheers,

-- 
Julien Grall