lld/mach-o x86_64 asserts

On LLD mach/o x86_64 LLD calls dataInCodeTransitionStart and dataInCodeTransitionEnd, which aren't implemented (and thus return the defautl value 0), later in applyFixupFinal it reaches the default: unreachable causing miscompiles depending on the optimizer.

Attached is a simple patch that fixes this.

It also fixes the read8 method, as object files inside .a files aren't 8 byte aligned when memory mapped.

ldd_relocation_fixes.txt (1.96 KB)

I confirm that this passes the Zig test suite. This solves the 1 outstanding patch that Zig has against LLD, and if this is merged, we can go back to using LLD releases instead of a fork.

Carlo,

Thank you for your contribution! We don’t usually commit any feature without a test, so can you write a test for us? Then I think I can commit this for you.

That sounds quite reasaonable; how does one usually go about doing that? a repro zip that hits both asserts?

You can take inspiration from anything in lld/test, but basically
either an assembly source (or multiple) passed through llvm-mc and
then lld, or a YAML file passed to yaml2obj (and then lld). From what
I see the Mach-O backend prefers YAML-style testing, but I don't think
that's a strict requirement.

Thanks,

Doesn't look like the test system works on windows?

C:\p\llvm\llvm\tools\lld\test>python c:\p\llvm\llvm-bin32\relwithdebinfo\bin\llvm-lit.py darwin\darwin-asserts-in-x86_64.ll
llvm-lit.py: C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py:101: fatal: unable to parse config file 'C:\\p\\llvm\\llvm\\tools\\lld\\test\\lit.cfg.py', traceback: Traceback (most recent call last):
  File "C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py", line 88, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "C:\p\llvm\llvm\tools\lld\test\lit.cfg.py", line 23, in <module>
    config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
AttributeError: 'NoneType' object has no attribute 'use_lit_shell'

darwin-asserts-in-x86_64.ll (3.52 KB)

Got it.

Attached are both the testcase & the fix.

darwin-asserts-in-x86_64.ll (2.13 KB)

ldd_relocation_fixes.txt (1.96 KB)

Ping Rui. Is there anything else that needs to be done on this patch?

Sorry, I was thinking to review the test but didn’t.

Is this test complete? It does invoke lld, but it didn’t verify its output.

When I wrote the test, it failed lld (assertion) in debug mode. I can’t get it to fail for non debug but afaik the test system should fail on lld asserts?

Will this patch make it into LLD 7.0.0 release?

It’s currently the only patch that zig’s LLD fork has on top of upstream.

I'd suggest adding a comment to the test explaining what it's checking
for: "; Check that lld doesn't assert when bla bla...". Checking for
the expected output is nicer, but sometimes that's hard.

In the test run lines, you should replace
"darwin-asserts-in-x86_64.ll" with %s and darwin-asserts-in-x86_64.o
with %t.o and the lit will expand those to the source file and to a
temporary name, respectively.

Rui, do you have any other concerns?