[llvm] [BOLT] Support instrumentation hook via DT_FINI_ARRAY (PR #67348)

Job Noorman via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 2 02:36:27 PDT 2023


mtvec wrote:

> What about DT_INIT, what does the ABI say? Usually there is symmetry between INIT and FINI, INITARRAY and FINIARRAY.

At least on RISC-V, `DT_INIT` is also not used. However, BOLT [patches the ELF entry point](https://github.com/llvm/llvm-project/blob/32521bb37caad512fd539d95bda26170a91e8a4b/bolt/lib/Rewrite/RewriteInstance.cpp#L4233-L4234) for runtime initialization in executables. I see it [also patches `DT_INIT`](https://github.com/llvm/llvm-project/blob/32521bb37caad512fd539d95bda26170a91e8a4b/bolt/lib/Rewrite/RewriteInstance.cpp#L5044-L5050) for, iiuc, libraries, so we might want to support `DT_INIT_ARRAY` for this in the future.

> BTW ideally we would like to add a new entry to dynamic table and insert ourselves into DT_PREINIT_ARRAY, but this is outside the scope of this patch. We don't currently rewrite the dynamic table so we are not doing that for this reason. We need to run our code before every constructor, that's why PREINIT is more adequate.
> 
> EDIT: PREINIT_ARRAY is only for binaries, they're not used for DSOs. The context on this issue is that we currently might crash with instrumentation if your binary runs constructors in a dependent lib, and that dependent lib runs instrumented code from your binary. PREINIT_ARRAY will guarantee that we run before any constructors in DSOs linked in DT_NEEDED.

This indeed makes sense for a future enhancement.

> If we're being pedantic, I think we need to patch the last entry of FINI_ARRAY, not the first. If you patch the first entry, you will still run destructors after your runtime lib. But I don't think we're going to crash because of this (at least for instrumentation), since instrumentation finalization code will just dump profile. No need to implement this, but we should at least have a comment documenting this behavior, so nobody is surprised in the future if they try to implement some runtime lib that expects to run its finalization code after application code.

The entries in `DT_FINI_ARRAY` are run in reverse order so I think using the first entry is actually the correct thing to do (although admittedly, I didn't think about this before you mentioned it :)).

https://github.com/llvm/llvm-project/pull/67348


More information about the llvm-commits mailing list