Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix gac to ensure that binaries it creates on Linux can load and run, even if a GAP package with a compiled kernel extension (such as IO) is present #4006

Merged
merged 1 commit into from
May 6, 2020

Conversation

fingolfin
Copy link
Member

Reported by Bill Allombert: compiling a simple test program via, say,
gac catp.g -o catp, and then running it results in an error like this:

[... GAP banner here ...]
Loading the library and packages ...
#W dlopen() error:
SOMEPATH/pkg/io/bin/x86_64-pc-linux-gnu-default64-kv7/io.so:\
undefined symbol: ChangedBags

This was caused by the linker invocation in gac missing the -export-dynamic
flag

This patch also fixes a second issue: While GAP deliberately links using the C
compiler, the linker command in gac used the C++ compiler for linking. Now
both use the C compiler, ensuring consistent behavior.

Note that we quite deliberately use the C compiler to link, despite GAP
containing C++ code: that code that is careful not to use the C++ standard
library and for which RTTI and exceptions are disabled

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them topic: gac GAP to C compiler backport-to-4.11 labels May 3, 2020
@coveralls
Copy link

coveralls commented May 3, 2020

Coverage Status

Coverage decreased (-0.0003%) to 84.371% when pulling 8a228fc on fingolfin:mh/fix-gac into edc5339 on gap-system:master.

Reported by Bill Allombert: compiling a simple test program via, say,
`gac catp.g -o catp`, and then running it results in an error like this:

    [... GAP banner here ...]
    Loading the library and packages ...
    #W dlopen() error:
    SOMEPATH/pkg/io/bin/x86_64-pc-linux-gnu-default64-kv7/io.so:\
    undefined symbol: ChangedBags

This was caused by the linker invocation in gac missing the `-export-dynamic`
flag

This patch also fixes a second issue: While GAP deliberately links using the C
compiler, the linker command in gac used the C++ compiler for linking. Now
both use the C compiler, ensuring consistent behavior.

Note that we quite deliberately use the C compiler to link, despite GAP
containing C++ code: that code that is careful not to use the C++ standard
library and for which RTTI and exceptions are disabled
@fingolfin
Copy link
Member Author

Bill reports that this PR fixes the issue for him (as it does for me).

Copy link
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it fixes the problem, let’s merge.

@fingolfin fingolfin merged commit 28ca9d4 into gap-system:master May 6, 2020
@fingolfin fingolfin deleted the mh/fix-gac branch May 6, 2020 08:12
@fingolfin
Copy link
Member Author

Backported to stable-4.11 via commit 1cc01c9

@PaulaHaehndel PaulaHaehndel added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.11-DONE kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: gac GAP to C compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants