Fix stack alignment in testcases, which caused libc's puts() stack to
crash in ubuntu systems because libc was using movaps in malloc
initialization.

Triggering build tests again after removing parent revs

In this solution, we don't need to mark any symbols as globals and therefore I've removed the dependency on the parent diff "[BOLT] Local hidden should be global syms."

Reviewer suggestions

Thanks a bunch @lhames , that works!

Thanks for the review!

I'll commit this fix since this seems low-risk enough to unblock buildbots crosscompiling. If it is not an adequate fix, feel free to revert it.

In D155098#4617428, @hzq wrote:

In D155098#4615583, @rafauler wrote:

I'm not sure why runtime/X86/reg-reassign-swap-cold.s is failing. I can't repro the failure. I would suggest submitting your changes on another revision using arc diff --draft, so it is not published, but you can still run the tests to see if they pass, and they debug it from there -- perhaps by removing modifications you did to see if the test pass, getting hints on what is wrong.

The current modifications appear to have resulted in all test cases passing.

I'm not sure why runtime/X86/reg-reassign-swap-cold.s is failing. I can't repro the failure. I would suggest submitting your changes on another revision using arc diff --draft, so it is not published, but you can still run the tests to see if they pass, and they debug it from there -- perhaps by removing modifications you did to see if the test pass, getting hints on what is wrong.

Thanks for working on this. LGTM

Thanks! LGTM

Thanks! LGTM

LGTM. Thanks!

LGTM. Thanks!

LGTM

I think you forgot to delete the now unnecessary CodePointerSize parameter.

Thanks Elvina! LGTM

LGTM

Thanks! LGTM with some modifications I made to the test file available here: https://pastebin.com/3abP1S5G

Thanks! I can start looking into this next week (and the week after it) during my shift.

Thanks! Mostly formatting nits:

In D147544#4497505, @treapster wrote:

In D147544#4477299, @jobnoorman wrote:

I'm not familiar with LongJmpPass but I did notice the following comment:

We pull this pass inside BOLT because here we can do a better job at stub inserting by manipulating the CFG, something linkers can't do.

So wouldn't you lose something if this was moved to JITLink?

You're right, losing CFG will make branches impossible to relax. We'll need MC layer to emit relocs for every branch which is apparently default behavior for RISC-V but not AArch64.
AFAIK currently JITLink doesn't relax branches for RISC-V, but if you intend to implement it, we can see how it works out and consider doing it for AArch64(starting with some MC option to emit R_AARCH64_JUMP26 and R_AARCH64_CONDBR19). As for updating the output values, it seems you mostly solved it in D154604.
@rafauler, do you think it's feasible to move LongJMP to JITLink if we force MC to generate branch relocs, and are there some other reasons to manipulate the actual CFG?

Thanks for the context, the fix looks good to me. Unfortunately, we can't check in the test as is because we can't guarantee the .fdata file will always make sense -- we would typically write this test in assembly language to guarantee offsets remain the same.

😂

Thanks for fixing this. We do need context for the diff and a test case.

I wonder why is this diff not showing up on top of the D151899 diff stack anymore? Isn't it dependent on that stack?

Thanks for working on this @treapster . Can you add to your diff stack a diff that adds a test that uses this improved mechanism?

In D147544#4473532, @treapster wrote:

Hello, @jobnoorman, out team is currently evaluating possible improvements to LongJmp pass, and we're wondering what are the opportunities for doing relaxation/stub insertion on JITLink side? I see currently JITLink can build PLT with TableManager and relax some calls to it and GOT accesses, do you think it's possible/worth it to to extend this functionality further to support a more general replacement of instructions as we do in LongJMP? It seems it already works like that for RISCV because of D149526, does that mean we can implement similar patches for AArch64/x86 and throw LongJmp away? I guess it would be tricky for X86 because of all the encoding mess, but i'm wondering what your thoughts are.

LGTM

LGTM without the test

LGTM

LGTM

LGTM

In D153771#4452461, @treapster wrote:

In D153771#4452452, @treapster wrote:

Turns out, the [unknown] entries in profile are because addresses in indirect call descriptions are not relocated, which makes them meaningless in PIE because of ASLR. When the test is compiled with no-pie, indirect calls are recorded accurately. So, we should either compute base address and add it to stored entries, or produce dynamic relocations.

But since .bolt.instr.tables is not allocatable, the only option is to perform relocation by hand. BTW, why isn't it allocatable?

In D153771#4452452, @treapster wrote:

Turns out, the [unknown] entries in profile are because addresses in indirect call descriptions are not relocated, which makes them meaningless in PIE because of ASLR. When the test is compiled with no-pie, indirect calls are recorded accurately. So, we should either compute base address and add it to stored entries, or produce dynamic relocations.

In D151920#4451480, @treapster wrote:

In D151920#4450871, @rafauler wrote:

We need a test that checks indirect calls are correctly handled, like the one we have in D153771

Or we can make that test work for all platforms.

Thanks @treapster , can you try to rebase on top of current bolt now? I can't test it if the diff doesn't cleanly apply on top of what we have on trunk, and JITLink changed BOLT quite a bit.

We need a test that checks indirect calls are correctly handled, like the one we have in D153771

Thanks for working on fixing these issues, @treapster! I have some suggestions below.

In D153771#4449272, @treapster wrote:

For some reason CHECK-ALL-DAG clause does not always work as expected on the second invocation, what may be happening here? I tried using {{}} regex to match the words exactly, but it didn't help

LGTM

Can we have a small test case showing the bug?

LGTM

Ping. Can't we just remove this assertion? Is it worth making the runtime much slower to check that we don't have a duplicate external symbol?

LGTM

LGTM

Thanks!

Where does this happen?