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 Issue 17521 - -betterC programs should not link in Phobos by default #6918

Merged
merged 1 commit into from
Jun 26, 2017

Conversation

WalterBright
Copy link
Member

No description provided.

@WalterBright WalterBright changed the title -betterC programs should not link in Phobos by default fix Issue 17521 - -betterC programs should not link in Phobos by default Jun 19, 2017
@WalterBright WalterBright force-pushed the link-betterC branch 4 times, most recently from b8bb995 to 26ce0c7 Compare June 19, 2017 06:20
@WalterBright
Copy link
Member Author

Consider the program:

import core.stdc.stdio;

extern (C) int main(char** argv, int argc) {
    printf("hello world\n");
    return 0;
}

compile on Win32 with -betterC, and the resulting executable is 23Kb! The non-better-C version is 194Kb.

@CyberShadow
Copy link
Member

Is there a document which enumerates the effects of -betterC? May be worth starting keeping track of them.

@WalterBright
Copy link
Member Author

I've been writing one. You're right, it is needed. But as betterC is a work in progress, I don't know yet what all it will need to do.

@JinShil
Copy link
Contributor

JinShil commented Jun 20, 2017

I don't know yet what all it will need to do.

It's not necessary to document what it will do; only what it currently does.

@JinShil
Copy link
Contributor

JinShil commented Jun 20, 2017

After pondering this a little, it occurred to me that DMD should never link in Phobos or druntime automatically. Rather, I think such dependencies should be specified on a platform-by-platform basis using a dmd.conf, linker script, or some other configuration file that is distributed with the toolchain's package.

@jpf91
Copy link
Contributor

jpf91 commented Jun 20, 2017

After pondering this a little, it occurred to me that DMD should never link in Phobos or druntime automatically. Rather, I think such dependencies should be specified on a platform-by-platform basis using a dmd.conf, linker script, or some other configuration file that is distributed with the toolchain's package.

👍

You need something like this anyway to handle druntime/phobos dependencies: Depending on the OS when linking an application with static libphobos/druntime you'll also have to explicitly link druntime or phobos dependencies (libdl, libz, on windows all the import libraries). Hardcoding this in the compiler is not a good idea.

In GDC we create a libgphobos.spec file when compiling phobos which contains all required dependencies. If libphobos is installed and the compiler can find this file it will link dependencies automatically:

cat /opt/gdc/lib64/libgphobos.spec 
#
# This spec file is read by gdc when linking.
# It is used to specify the libraries we need to link in, in the right
# order.
#

%rename lib liborig_gdc_renamed
*lib: %(liborig_gdc_renamed) -latomic -lm -lpthread  -ldl

phobos and druntime are still hardcoded here though. This is mostly to support simple shared/static library switching using --static-libphobos and --shared-libphobos. This requires bracketing -lphobos with special linker opts which seems difficult to do in GCC spec files.

@@ -98,7 +98,8 @@ bool doUnwindEhFrame()
* g++ on FreeBSD does not generate mixed frames, while g++ on OSX and Linux does.
*/
assert(!(usednteh & ~(EHtry | EHcleanup)));
return (usednteh & (EHtry | EHcleanup)) || (config.exe & (EX_FREEBSD | EX_FREEBSD64));
return (usednteh & (EHtry | EHcleanup)) ||
(config.exe & (EX_FREEBSD | EX_FREEBSD64)) && !config.betterC;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that exceptions will NEVER work if you use the -betterC switch?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. Just that the frame handler isn't by default on on FreeBSD.

@braddr
Copy link
Member

braddr commented Jun 21, 2017

IMHO, this is a good short term fix, but it's papering over the larger issue: way too much w/in phobos and druntime is dragged in when not needed or used. Andrei's work to kill off static this()'s will help a bunch. More investigation and effort needs to be put into making linking those libraries to be essentially free when no parts of them are used. IE, needing this flag is a sign that something somewhere is wrong and not addressing that head on is a problem.

@WalterBright
Copy link
Member Author

I agree with you, @braddr , but this issue has been twisting around for 15 years with no progress and little interest. With the switch, we find out if this is a real thing or not, and we find out exactly what needs to be done.

@MartinNowak
Copy link
Member

@WalterBright, you need to put the fix Issue 12345 reference into the commit message, not on github.
This also seems to fix most of Issue 11881 – -betterC switch suffers from bit rot, please add the issue as well.

@adamdruppe
Copy link
Contributor

adamdruppe commented Jun 26, 2017 via email

@MartinNowak
Copy link
Member

Right now, it is generated, but not used, right?

I've updated the issue (https://issues.dlang.org/show_bug.cgi?id=11881#c18), just use an extern(C) main and the wrapper won't be generated.

@adamdruppe
Copy link
Contributor

adamdruppe commented Jun 26, 2017 via email

@dlang-bot
Copy link
Contributor

dlang-bot commented Jun 26, 2017

Thanks for your pull request, @WalterBright!

Bugzilla references

Fix Bugzilla Description
11881 -betterC switch suffers from bit rot
17521 -betterC programs should not link in Phobos runtime library by default

@WalterBright
Copy link
Member Author

@MartinNowak seems like dlang-bot removed the auto-merge. We should immediately suppress insubordination by bots!

@MartinNowak
Copy link
Member

@MartinNowak seems like dlang-bot removed the auto-merge. We should immediately suppress insubordination by bots!

GH had some delays processing push events (https://status.github.com/messages/2017-06-26), hence the bot got triggered after I added the label.

@wilzbach
Copy link
Member

We should immediately suppress insubordination by bots!

image

In general, this is a feature to avoid allowing contributors without merge rights to sneak unapproved code into dlang repos. Our bot removes auto-merge if a new commit(s) gets added to a PR or a force-push to the PR branch has happened as these changes haven't been reviewed & approved yet. Unfortunately as @MartinNowak mentioned the GH API lead to this accident.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants