[PATCH] D28791: [compiler-rt][crt] Simple crtbegin and crtend implementation

Sun Feb 24 14:57:09 PST 2019

phosek added a comment.

In D28791#1357948 <https://reviews.llvm.org/D28791#1357948>, @joerg wrote:

> For the clang side, I don't understand why Driver::GetFilePath is not good enough. This shouldn't need all toolchain changes at all.

I split the driver changes from this patch to make it smaller.

The reason why `Driver::GetFilePath` is not enough is naming. These files are installed to `lib/clang/<version>/lib/linux` and there can be versions for multiple architectures side-by-side so I cannot use just `crtbegin.o`. compiler-rt CMake's build would derive e.g. `clang_rt.crtbegin-x86_64.o` as the file name, and `ToolChain::getCompilerRT` contains the logic to construct that name based on the target.

> For the compiler-rt side:
>  (1) I would drop the const for .ctor/dtor, since the content has to be writeable anyway for function pointer relocations.

Done

> (2) I'm trying to remember what the alignment rules for .eh_frame are. There seem to be different approaches here in the wild.

I went with with `sizeof(void *)` which is what libgcc's implementation seem to be using.

> (3) .ctor/.dtor should be using pointer types, not long. sizeof(void (*)(*)) == sizeof(long) is a very questionable assumption.

Done

> (4) __CTOR_END__ and __DTOR_END__ are unused and will just be dropped.

They're used now.

> (5) The logic around CRT_HAS_INITFINI_ARRAY is strange. If it is not set, __do_init and __do_fini must be called from .init/.fini with appropiate assembler code. Setting .init_array/.fini_array can't work in that case.

Done

> (6) Putting read-only content into .eh_frame can be a problem on MIPS for many GNU ld versions. It used to bitch about mixing read-only and read-write sections and the default DWARF encoding required writable .eh_frame.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D28791/new/

https://reviews.llvm.org/D28791