Drop the Error output parameter and return an Expected instead

The follow-up change is D94690. It also moves the instantiation of ObjLinkingLayer from the initialization list to the ctor body.

Thanks! Eventually, I closed https://llvm.org/PR36209

I'd like to land this before release 12 branches. If you have any objections, please speak up before the end of the week. Thanks.

Friendly ping. @lhames Can you confirm this fix? I couldn't find a test for this piece of code.

Does anyone have objections to this documentation update? Preview is here still:
https://weliveindetail.github.io/docs-pub/html/DebuggingJITedCode.html

Ok, I grabbed your latest version and checked again for the EH frame registration on Linux. This works very well now! Thanks

One issue with the orc-lazy JIT will be symbol renaming for lazy reexports: Upfront we only emit stubs for our function symbols and they trigger compilation of the actual code in a separate logical unit once they are called. The debug object we hand over to LLDB doesn't take that into account. (Maybe we are just inserting the wrong load address? Don't know.) Additionally, I am not sure how debug data is handled in the partitioning logic of the CompileOnDemandLayer.

Thanks for having a look.

Last issue: EH-frame registration isn't ready for ELF right? That would be great to fix (see the unticked inline comment).
I can offer to check the mangling prefix details once this had landed.

That's right, but I don't see how even the static ones could be resolved. Wouldn't it hard-wire the objects together? In the next session we want to load some object files from cache while others have changed.. Maybe one day I should check how ccache and the likes handle it.

Thanks for having a look!

LLDB regression https://bugs.llvm.org/show_bug.cgi?id=36209 fixed with D61611 and D90769

Preview for the update: https://weliveindetail.github.io/docs-pub/html/DebuggingJITedCode.html

Two issues when trying to build llvm-jitlink-executor with the libs separated:

In D90132#2370918, @sgraenitz wrote:

I'm curious to get your take on the library breakdown and corresponding headers too.

There's a number of includes still pointing to #include "llvm/ExecutionEngine/Orc/TargetProcess/XY". Otherwise, yes LGTM.

I'm curious to get your take on the library breakdown and corresponding headers too.

Such a oneshot temporary label can usually be omitted:

Polish comments and variable names in test

Great that there is so fast progress! Haven't reviewed in detail (yet), but it looks like there are no conceptual changes compared to the proposal we know? A few notes inline.

One note on the optimizeELF_x86_64_GOTAndStubs pass that had to be considered for this patch: it currently only relaxes edges for MOVQ instructions to LEA and we avoid that in the test by using a MOVL. If I understand correctly, LLD doesn't have this restriction. It relaxes MOVQ to LEAQ and MOVL to LEAL. For reference see https://reviews.llvm.org/D15779#change-f8vl5AmubNpC

With movl we have to target eax

Prevent the optimizer from relaxing the edge by using a movl instruction (anyway it's been only 4 bytes all the time)

I think it's not a big deal, but you may want to revert the named_data change. See inline comment.

Skipping the optimization pass, I had expected to get something like movq 0x1ff9(%rip), %rax but instead it's:

dis -s 0x00007ffff7ff5000 -e 0x00007ffff7ff5010
    0x7ffff7ff5000: movq   (%rip), %rax

I didn't get the test to work yet. Neither with a R_X86_64_REX_GOTPCRELX relocation for common_data (TOT clang and llvm-mc emit that, llvm-mc-10 too btw) nor with R_X86_64_GOTPCREL (clang-10). Some observations in the following.

Add test ELF_x86-64_common

Yes, that brings up the issue with relaxed R_X86_64_REX_GOTPCRELX (42) relocations you mentioned in Discord: JIT session error: Unsupported x86-64 relocation:42 I will check whether we can map it to PCRel32GOTLoad or otherwise what's the difference.

Thanks for coming up with a test case. I haven't looked into llvm-jitlink's check code yet. That serves a an excellent starting point!

The ELF spec has an extra paragraph for special handling in dynamic linkers. I am not sure how much it's relevant here:

When the dynamic linker encounters a reference to a symbol that resolves to a definition of type STT_COMMON, it may (but is not required to) change its symbol resolution rules as follows: instead of binding the reference to the first symbol found with the given name, the dynamic linker searches for the first symbol with that name with type other than STT_COMMON. If no such symbol is found, it looks for the STT_COMMON definition of that name that has the largest size.

Polish comments

Drop unrelated switch case R_X86_64_32S

Of course, I must compile with -fPIC to avoid relocation types like R_X86_64_32S in the first place. Looking into the test case now.

It turned out the SIGSEGV: invalid address is caused by not handling the R_X86_64_32S type correctly. This is somewhat unrelated, but must be fixed in order to come up with a test case.

Relocations to the common section seem to be calculated correctly now, but at runtime I see a SIGSEGV: invalid address. Maybe memory for the section isn't allocated?

Fix NULL entry in symbol table

FYI The issue came up when I was trying to jitlink gzip: https://people.csail.mit.edu/smcc/projects/single-file-programs/gzip.c Next step would be an isolated test case.

Looks good. Two bots failed, both unrelated to my change.
http://green.lab.llvm.org/green/job/lldb-cmake/23457/
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/17430/

Back to the review: I'd like to keep the test for the example and see how the build servers behave. Generally it might be useful to have tests for all the "LLJITWith..." examples right?
Do you think it makes sense to land the patch on the weekend in order to keep the number of people getting annoyed by me breaking their builds at a minimum? :)

I will get back to performance evaluation maybe in a few weeks and sure I am happy to share my progress.

Fix clang-tidy warnings

Fix clang-format issue and rebase

In D85974#2218405, @lhames wrote:

What performance issues did you run in to with threading and performance? I haven't had a chance to look in to that yet.

Test discovery should ignore subdirectories that contain test inputs.

I would like to remove the ThinLtoJIT example. It needs a more decent threading library to speed-up multithreaded compile times and that's easier to do out-of-tree.
This might be a useful (minimal) portion to keep in-tree. What do you think?

Fixed with commit 28e1015e327ec56ac4bf93967d3aa2915b215e36