Re: [Minios-devel] [UNIKRAFT PATCH 3/4] plat/*: Provide section for library constructors

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 13.02.2018 09:04, Florian Schmidt wrote:
> I'm not sure this is doing exactly the right thing. I'll take the x86 
> linuxu code as example, but it should apply to the other platforms, too:
> On 02/12/2018 05:01 PM, Simon Kuenzer wrote:
> > diff --git a/plat/linuxu/x86/link64.ld b/plat/linuxu/x86/link64.ld
> > index a7d642d..3f78b81 100644
> > --- a/plat/linuxu/x86/link64.ld
> > +++ b/plat/linuxu/x86/link64.ld
> > @@ -56,6 +56,30 @@ SECTIONS {
> >           *(.rodata)
> >           *(.rodata.*)
> >       }
> > +
> > +    /* Constructor table */
> > +    ukplat_ctortab = .;
> > +    .ctortab : {
> > +        *(.ukplat_ctortab0)
> > +        *(.ukplat_ctortab0.*)
> > +        *(.ukplat_ctortab1)
> > +        *(.ukplat_ctortab1.*)
> > +        *(.ukplat_ctortab2)
> > +        *(.ukplat_ctortab2.*)
> > +        *(.ukplat_ctortab3)
> > +        *(.ukplat_ctortab3.*)
> > +        *(.ukplat_ctortab4)
> > +        *(.ukplat_ctortab4.*)
> > +        *(.ukplat_ctortab5)
> > +        *(.ukplat_ctortab5.*)
> > +        *(.ukplat_ctortab6)
> > +        *(.ukplat_ctortab6.*)
> > +        *(.ukplat_ctortab7)
> > +        *(.ukplat_ctortab7.*)
> > +        LONG(0);
> > +    }
> > +    ukplat_ectortab = .;
> > +
> >       . = ALIGN(0x1000);
> >       _erodata = .;
> So you put the constructor before the end symbol signifying the end of 
> read-only data. That works as expected:
> $ nm -n build/build/unikraft_linuxu-x86_64
> [...]
> 0000000000405000 R _rodata
> 00000000004052c0 r hex2ascii_data
> 0000000000405300 R _nolibc_ctype
> 0000000000405c60 R ukplat_ctortab
> 0000000000405c64 D ukplat_ectortab
> 0000000000406000 D _data
> 0000000000406000 D _erodata
> [...]
>
> However, when I look at the sections, ctortab is writable:
>
> $ readelf -S build/unikraft_linuxu-x86_64
> There are 10 section headers, starting at offset 0x74e8:
>
> Section Headers:
>    [Nr] Name              Type             Address           Offset
>         Size              EntSize          Flags  Link  Info  Align
> [...]
>    [ 2] .rodata           PROGBITS         0000000000405000  00005000
>         0000000000000c60  0000000000000000   A       0     0     32
>    [ 3] .ctortab          PROGBITS         0000000000405c60  00005c60
>         0000000000000004  0000000000000000  WA       0     0     1
>    [ 4] .data             PROGBITS         0000000000406000  00006000
>         00000000000000bc  0000000000000000  WA       0     0     32
> [...]
>
> That leaves me with two questions: First, is .ctortab supposed to be 
> writable? My gut feeling is no, because it is completely populated at 
> compile/link time, so no dynamicity there. Second, is _erodata designed 
> to point at the end of .ctortab instead of .rodata? Even if .ctortab is 
> supposed to be non-writable, I'm not sure about this one. Also, third: 
> is the alignment of 1 correct?
(1) You are right, it is supposed to be non-writable. I will do a v2.
(2) For now this is for simplicity: by placing _erodata after the 
ctortab, there is no special handling in existing code necessary: (1) 
platform code that marks a page region as read-only (see Xen), (2) the 
memory region API does not need to distinguish between rodata and ctortab.
(3) The alignment could be optimized since we have only function 
pointers here.
> Oh, and one more minor question unrelated to this: why did you not 
> follow the example of the other pointers and started the name with an 
> underscore (and name it the same as the section)?
Because ukplat_ctortab is a used symbol, defined as an extern pointer 
array (see /include/uk/plat/ctors.h). I thought it would confuse the 
API. Indeed ukplat_ectortab is not used for now and it is probably 
better to remove it (-> v2).
> Cheers,
> Florian
Thanks,

Simon

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel