[PATCH] D39548: [ELF] Set the section size on the section start symbols

Mon Nov 6 10:19:41 PST 2017

arichardson added a comment.

In https://reviews.llvm.org/D39548#916728, @peter.smith wrote:

> Can you share a bit more about how CHERI uses the symbol size? Is it only for known linker generated Sections like in your example above, or to support arbitrary user mysection_start symbols? Is the mysection_start symbol special cased within CHERI or is it just an arbitrary ELF Symbol.

We don't do any special casing based on symbol names, we only look at the information from the symbol table. I think having special cases based on the symbol name is very fragile so I'd rather set a size on all linker defined symbols that are used as dereferenceable pointers.

> I'm just a bit curious about the implications of setting the symbol size, for example if this is for special sections like .dynamic we would only expect the _start and _end, and _DYNAMIC symbols to be defined, in an arbitrary mysection_start there could be multiple other symbols defined as well and having the symbol size set to the section size would mean its [0, size)  range would overlap all other symbols in the section, this could potentially confuse other ELF processing tools that might skip over functions/data based on Symbol Size. I'm also curious as to how the symbol size is used? Does the OS/loader require the static symbol table to be present, what if it has been stripped?

I hadn't thought about the potential to confuse other ELF processing tools. However, wouldn't that also break on code like this?

  .global foo_with_setup
  .ent foo_with_setup
  foo_with_setup:
    ....
    ....
  foo_fast:
   ....
   ....
  .end foo_with_setup

  jal foo_with_setup
  ...
  jal foo_fast

In CHERI each pointer (capability) is architecturally 256 bits (compressed to 128 in memory): 64 bit base address + 64 bit offset + 64 bit length and <64bits attributes (read data/store data/executable/load capability/store capability, etc.). During the initial startup we have to initialize all global capabilities using the information from the ELF symbols.
Currently we generate a special ELF section `__cap_relocs` containing a capability "relocations" for every global pointer.  For historic reasons we don't use ELF relocations but an array of structs that is processed by crt_init_globals() <https://github.com/CTSRD-CHERI/cheribsd/blob/master/lib/csu/cheri/crt1.c#L96> in the initial startup code.
This separate section also has the advantage that we don't need any symbol tables at runtime but can just loop over this array containing addresses resolved at static link time.
We need sizes on every ELF symbol that can be used as a pointer in order to set the correct runtime-bounds on the resulting capability so that we can find all bounds violations.

Say we have the following code:

  char x[20];
  char* y = &x[20];

  int foo(void) {
     char c = y[-1]; // okay
     c = y[0];  // trap at runtime
    return c;
  }

  __attribute__((weak)) extern Elf_Dyn* _DYNAMIC;
  int parse_dynamic(void) {
    Elf_Dyn* dynp = _DYNAMIC;
    for (; dynp->d_tag != DT_NULL; dynp++) {
      // ...
    }
    return 0;
  }

Currently we can generate a `__cap_relocs` section with the following contents (the load address will be added to these addresses before the globals are initialized):

  CAPABILITY RELOCATION RECORDS:
  0x0000000000020000	Base: x (0x0000000000040000)	Offset: 0x0000000000000014	Length: 0x0000000000000014	Permissions: 0x00000000
  0x0000000000030000	Base: y (0x0000000000020000)	Offset: 0x0000000000000000	Length: 0x0000000000000010	Permissions: 0x00000000
  0x0000000000030010	Base: _DYNAMIC (0x00000000000002b0)	Offset: 0x0000000000000000	Length: 0x0000000000000000	Permissions: 0x00000000
  SYMBOL TABLE:
  0000000000000000         *UND*		 00000000 
  00000000000002b0         .dynamic		 00000000 .hidden _DYNAMIC
  0000000000028010         .got		 00000000 .hidden _gp
  0000000000030000 l    d  .cap_table		 00000000 .hidden .cap_table
  0000000000030000 l       .cap_table		 00000010 .hidden y at CAPTABLE
  0000000000030010 l       .cap_table		 00000010 .hidden _DYNAMIC at CAPTABLE
  0000000000010000 g     F .text		 0000001c foo
  0000000000010020 g     F .text		 00000024 parse_dynamic
  0000000000040000 g       .bss		 00000014 x
  0000000000020000 g       .data		 00000010 y

The problem here is because LLD says _DYNAMIC has size 0 we also create a capability at runtime that has a base of _DYNAMIC but with a length of zero so we will crash as soon as we load a single byte from this capability.
In order to not crash while iterating over _DYNAMIC we need the __cap_reloc for _DYNAMIC to be this instead:

  CAPABILITY RELOCATION RECORDS:
  0x0000000000030010	Base: _DYNAMIC (0x00000000000002b0)	Offset: 0x0000000000000000	Length: 0x0000000000000120	Permissions: 0x00000000
  SYMBOL TABLE:
  00000000000002b0         .dynamic		 00000120 .hidden _DYNAMIC

To generate those values we use `Symbol->getSize<ELFT>()` which currently returns zero for `_DYNAMIC` and `__init_array_start`, etc.
So far I have found this to be a problem with `_DYNAMIC`, the init+fini array start symbols as well as sections that have a c indentifier so code assumes it can simply dereference `__start_foo`.

> On the assumption that there is a good reason for CHERI to use the symbol size I have some suggestions for the code.
> 
> - Given that addOptionalRegular is only used for start and end symbols and only uses the (now two) special cases of 0 and -1, I think we should rename the function to make it more specific to the purpose it is being used for.
> - Would it be better to use a similar trick for Size as Value, for example if Size is -1 and symbol is defined with respect to an OutputSection then Size is Size of OutputSection? You'd then not need to track the state of the section symbols and have a potentially fragile one off update.

I like the idea of using Size == -1 as the output section size as this avoids the fragile state update. I will update the patch if you agree that it makes sense include this change in upstream LLD.

I hope my explanation made some sense. I do realize we compared to most architectures CHERI is rather exotic but I feel like having sizes defined on symbols that are generally used as pointers would be a good patch to upstream.

https://reviews.llvm.org/D39548