Page MenuHomePhabricator

[clang] Fix libclang linking on Solaris
ClosedPublic

Authored by ro on Aug 5 2021, 5:04 AM.

Details

Reviewers
tstellar
MaskRay
Group Reviewers
Restricted Project
Commits
rGa382a746275b: [clang] Fix libclang linking on Solaris
Summary

Linking libclang.so is currently broken on Solaris:

ld: fatal: option --version-script requires option -z gnu-version-script-compat to be specified

While Solaris ld supports a considerable subset of --version-script, there are some
elements of the syntax that aren't.

The fix is equivalent to D78510. Additionally, use C-style comments is a GNU extension
that can easily be avoided by using # as comment character.

Tested on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11.

Diff Detail

Event Timeline

ro created this revision.Aug 5 2021, 5:04 AM
ro requested review of this revision.Aug 5 2021, 5:04 AM
MaskRay added inline comments.Aug 5 2021, 8:54 AM
clang/tools/libclang/libclang.map
1–5

GNU ld doesn't support # comments. ld.lld supports it.

Is there any kind of comment that is supported by all linkers?

ld.lld, GNU ld, and gold support /* */. I am not sure about Solaris ld -z gnu-version-script-compat.

ro added a comment.Aug 5 2021, 10:00 AM

Is there any kind of comment that is supported by all linkers?

Seems there isn't. In that case, we could either move the comments to a separate file (libclang.README?), strip the comments when using
Solaris ld (a bit ugly), or have two separate version scripts (terrible, bound to diverge).

ld.lld, GNU ld, and gold support /* */. I am not sure about Solaris ld -z gnu-version-script-compat.

Not sure in which way? Support for /* */ comments (no) or need for the option? The latter is explained in ld(1):

-z gnu-version-script=mapfile
-z gnu-version-script-compat
--version-script mapfile

    Provides partial support for the GNU version script style  of  map-
    file.  Version  scripts are based on the original Solaris version 1
    symbol definition syntax, with some  extensions.  ld  supports  the
    most  common  such extension, the use of wildcard characters in the
    specified symbol names. Other GNU-specific extensions  may  not  be
    supported.  ld  will  issue  an appropriate error if an unsupported
    extension is encountered.

    For convenience in building software  developed  with  GNU  version
    scripts,  the native GNU  --version-script option is accepted as an
    alias for -z gnu-version-script. Due to the partial nature  of  the
    support  for  GNU version scripts, the use of --version-script must
    be explicitly enabled by specifying -z gnu-version-script-compat.
clang/tools/libclang/libclang.map
1–5

Drats, I'd have sworn they supported `#' comments, too,
especially since the version script syntax originated with
Sun and was later adopted by GNU ld.

In D107559#2928999, @ro wrote:

Is there any kind of comment that is supported by all linkers?

Seems there isn't. In that case, we could either move the comments to a separate file (libclang.README?), strip the comments when using
Solaris ld (a bit ugly), or have two separate version scripts (terrible, bound to diverge).

I think it would be better to move the comments to libclang.README.

MaskRay added inline comments.Aug 5 2021, 11:57 AM
clang/tools/libclang/libclang.map
1–5

Confirmed that GNU ld and gold support # in version scripts, but not in linker scripts.

ld.lld supports both.

Using # is fine to me if Solaris ld doesn't support /* in GNU version scripts.

MaskRay added inline comments.Aug 5 2021, 12:02 PM
clang/tools/libclang/libclang.map
1–5

Correction: only GNU ld doesn't support # in linker scripts.

ld.lld and gold support # everywhere.

Filed a GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=28198

ro added inline comments.Aug 6 2021, 1:03 AM
clang/tools/libclang/libclang.map
1–5

I'd only checked ld.texi and didn't even think of trying it when it wasn't documented there.

I can ask about C-style comments being added to Solaris ld, but I've never encountered the need before.

So the original patch is ok after all?

ro added a comment.Aug 6 2021, 1:05 AM

Ah, I forgot: I've meanwhile tested the patch on x86-64-pc-linux-gnu (Ubuntu 20.04) where /usr/bin/ld is from binutils 2.34: libclang.so was versioned just the same without and with the patch.

MaskRay accepted this revision.Aug 6 2021, 8:06 PM

# LG

This revision is now accepted and ready to land.Aug 6 2021, 8:06 PM
This revision was automatically updated to reflect the committed changes.