ldc-developers / ldc2.snap

Snap package definition for LDC, the LLVM-based D compiler
11 stars 4 forks source link

--no-warn-mismatch linker option clashes with --linker=lld #102

Open WebDrake opened 4 years ago

WebDrake commented 4 years ago

The current ldc2.conf uses the --no-warn-mismatch linker flag. This matches what is found in the current 1.18.0 upstream binary package, but has been dropped in more recent releases.

Unfortunately this clashes with the use of lld as linker, since it does not support that flag.

WebDrake commented 4 years ago

One question here is whether it is still appropriate for the snap package to define its own ldc2.conf: changes have been made upstream that may obviate the need to do so. This should be tested to ensure that we are not creating unnecessary divergences.

WebDrake commented 4 years ago

The requirement for --no-warn-mismatch has been dropped as of LDC 1.20, so we can fix the LLD issue there.

w.r.t whether we can drop defining our own ldc2.conf, the issue here (which I'd forgotten until retrying it just now) is that the snap build process results in incorrect paths in the resulting file. So for example with the 1.18.0 package the autogenerated file is:

// See comments in driver/config.d in ldc source tree for grammar description of
// this config file.

// For cross-compilation, you can add sections for specific target triples by
// naming the sections as (quoted) regex patterns. See LDC's `-v` output
// (`config` line) to figure out your normalized triple, depending on the used
// `-mtriple`, `-m32` etc. E.g.:
//
//     "^arm.*-linux-gnueabihf$": { … };
//     "86(_64)?-.*-linux": { … };
//     "i[3-6]86-.*-windows-msvc": { … };
//
// Later sections take precedence and override settings from previous matching
// sections while inheriting unspecified settings from previous sections.
// A `default` section always matches (treated as ".*") and is therefore usually
// the first section.
default:
{
    // default switches injected before all explicit command-line switches
    switches = [
        "-defaultlib=phobos2-ldc,druntime-ldc",
        "-L--no-warn-search-mismatch",
    ];
    // default switches appended after all explicit command-line switches
    post-switches = [
        "-I/include/d",
    ];
    // default directories to be searched for libraries when linking
    lib-dirs = [
        "/lib64",
        "/lib32",
    ];
    // default rpath when linking against the shared default libs
    rpath = "/lib64:/lib32";
};

"^wasm(32|64)-":
{
    switches = [
        "-defaultlib=",
        "-link-internally",
        "-L--export-dynamic",
    ];
    lib-dirs = [];
};

... where it should be clear that the post-switches, lib-dirs and rpath should have paths relative to the snap root, or to the location of the LDC binary.

However, we should update the ldc2.conf file to closer match upstream. It's a pity that it is not possible to autogenerate a config file whose paths are relative to the binary location. Even upstream has to manually work around this with a manual search-and-replace: https://github.com/ldc-developers/ldc/blob/dc2c8546554ec43960d81acdb43e875e0d947fec/.azure-pipelines/posix.yml#L218 https://github.com/ldc-developers/ldc/blob/dc2c8546554ec43960d81acdb43e875e0d947fec/.azure-pipelines/posix.yml#L232

WebDrake commented 4 years ago

The requirement for --no-warn-mismatch has been dropped as of LDC 1.20, so we can fix the LLD issue there.

Having dived into this a little deeper, it looks like 1.20's solution has been to separate out the 32-bit requirements into a separate ldc2.conf section:

"i686-.*-linux-gnu":
{
    lib-dirs = [
        "%%ldcbinarypath%%/../lib32",
    ];
    rpath = "%%ldcbinarypath%%/../lib32";
};

However, this approach seems to be predicated on the assumption that the package will only ever run on 64-bit systems. The snap package is (for now at least) maintaining i386 builds as well as x86_64.

So, I wonder if this is really the right approach for the snap package, or, assuming that it is, maybe it would be preferable to have both i386 and x86_64 specialized sections. @kinke @JohanEngelen any thoughts/advice?

Finally: is this approach also possible for earlier compiler versions (e.g. 1.18, 1.19) or is it only possible for 1.20+?

kinke commented 4 years ago

However, this approach seems to be predicated on the assumption that the package will only ever run on 64-bit systems.

Yes, and rightfully so, as it features 64-bit executables only.

You could add a section for the native target too (not using i386 though, that's i686), but IIRC there should be a default section nonetheless, as the compiler otherwise complains if the config file doesn't feature any matching section when cross-compiling. This approach is possible for older versions too. The little tools/add-multilib-section.sh tool used to add a section has been introduced with v1.20 though (and should be suited to add the other explicit section too).

WebDrake commented 4 years ago

Yes, and rightfully so, as it features 64-bit executables only.

Quite so. The point is that the snap package's situation is different (one build contains 64-bit exes, the other 32-bit), so it may need different solutions.

I see two different options here:

The second is possibly more natural/intuitive for a basic setup that's designed to support builds for multiple different architectures, not all 64-bit, but it may run into the missing-defaults concerns you cite (I don't know what the effect of empty, rather than missing, defaults would be?).

I am open to the possibility of just dropping support for the 32-bit snap packages, but it's caught LDC bugs in the past, so I would prefer not to if possible.

(not using i386 though, that's i686)

If I say i386 odds are I'm referring not to something in LDC config, but to the snap package architectures (which are referred to as amd64 and i386 respectively).

But when setting up ldc2.conf, should it be specifically i686 or is it worth using i[3-6]86 as suggested in the comments in ldc2.conf.in ... ?

kinke commented 4 years ago

I haven't tested a 32-bit multilib build, but the installed conf for v1.20 should handle that gracefully too - default section is for 32-bit, and the multilib is the 64-bit one (x86_64-.*-linux-gnu). [It also takes care of using i686 for 64-bit multilib builds.]

So if you switched to the default installed .conf with v1.20 and replaced /lib by %%ldcbinarypath%%/../lib and /include by %%ldcbinarypath%%/../include (as the CMAKE_INSTALL_PREFIX seems to be / and thus very search&replace-unfriendly), you should be fine for a while.