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

Issue 21488 - Always compile dmd with -fPIC on POSIX targets #12798

Merged
merged 1 commit into from Aug 6, 2021

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Jul 1, 2021

Segmentation faults or linker errors occur on 32-bit PIE platforms if any object is not compiled with -fPIC.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jul 1, 2021

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
21488 normal Bundled 32-bit dlang tools (ddemangle, dustmite, rdmd) segfault on startup

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + dmd#12798"

@ibuclaw
Copy link
Member Author

ibuclaw commented Jul 2, 2021

@WalterBright seems that there's something not right with string literals and -fPIC on 32-bit. Looks like a missing null terminator?

https://auto-tester.puremagic.com/show-run.ghtml?projectid=14&runid=4884149&isPull=true

@Geod24
Copy link
Member

Geod24 commented Jul 2, 2021

Looks like a missing null terminator?

Really ? What I see is:

 ... compilable/testheader2i.d      -o- -H -Hfgenerated/compilable/testheader2i.di -inline ()==============================
Test 'compilable/testheader2i.d' failed. The logged output:
Executing post-test script: tools/postscript.sh compilable/extra-files/header-postscript.sh compilable testheader2i generated/compilable/testheader2i.d.out0
+ source compilable/extra-files/header-postscript.sh
++ source tools/common_funcs.sh
++ grep -v 'D import file generated from' generated/compilable/testheader2i.di
++ test_name=header2i
++ diff -pu --strip-trailing-cr compilable/extra-files/header2i.di generated/compilable/testheader2i.di.2
--- compilable/extra-files/header2i.di	2021-07-01 19:51:26.072319059 -0700
+++ generated/compilable/testheader2i.di.2	2021-07-01 20:03:29.439094997 -0700
@@ -117,7 +117,7 @@ void test13275()
 	if (auto n = 1)
 	{
 	}
-	if (const n = 1)
+	if (const @trusted n = 1)
 	{
 	}
 	if (immutable n = 1)
@@ -126,7 +126,7 @@ void test13275()
 	if (shared n = 1)
 	{
 	}
-	if (const shared n = 1)
+	if (const shared @trusted n = 1)
 	{
 	}
 	if (int n = 1)
/home/braddr/sandbox/at-client/pull-4884281-Linux_32/dmd/generated/linux/release/32/dmd -conf= -m32 -Icompilable -o- -H -Hfgenerated/compilable/testheader2i.di -inline  -odgenerated/compilable -ofgenerated/compilable/testheader2i_0.o  -c compilable/testheader2i.d compilable/extra-files/header2.d 

==============================
Test 'compilable/testheader2i.d' failed: Expected rc == 0, but exited with rc == 1

Looks like the assignment ends up with an extra @trusted in its STC ? That would then be emitted here.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jul 2, 2021

Looks like a missing null terminator?

I shouldn't be attempting to triage at 3am in the morning, what I thought I saw was

+	if (const @trusted n = 1)
+	if (const @trusted shared n = 1)

Backend is still guilty until proven innocent.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jul 2, 2021

Both druntime and phobos are now built with shared (dlang/phobos#8154, dlang/druntime#3505), let's see if errors were caused by mixing PIC and non-PIC...

@Geod24
Copy link
Member

Geod24 commented Jul 7, 2021

I think you need a rebase

@ibuclaw
Copy link
Member Author

ibuclaw commented Jul 7, 2021

I rebased against the wrong branch. :D

@RazvanN7 RazvanN7 closed this Jul 8, 2021
@RazvanN7 RazvanN7 reopened this Jul 8, 2021
@WalterBright
Copy link
Member

Backend is still guilty until proven innocent.

I need something more specific than that. If there's a code gen bug, please file a bug report.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jul 8, 2021

Backend is still guilty until proven innocent.

I need something more specific than that. If there's a code gen bug, please file a bug report.

There would be something more specific if it were reproducible in a i386/ubuntu:16.04 container.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jul 8, 2021

Also, bug from last year. https://issues.dlang.org/show_bug.cgi?id=21374

@WalterBright
Copy link
Member

21374

Thank you. An assert fail in the code generator is clearly a codegen bug.

But there's nothing anyone can do about at one time, something went wrong somewhere.

@ibuclaw
Copy link
Member Author

ibuclaw commented Aug 2, 2021

@WalterBright looks like the EBX has an address which is not pointing to GOT when calling into thunks.

Enabling this disabled code fixes the dll test on 32-bit here locally.
https://github.com/dlang/dmd/blob/master/src/dmd/backend/cod3.d#L4879-L4888

@ibuclaw
Copy link
Member Author

ibuclaw commented Aug 2, 2021

@WalterBright looks like the EBX has an address which is not pointing to GOT when calling into thunks.

Enabling this disabled code fixes the dll test on 32-bit here locally.
https://github.com/dlang/dmd/blob/master/src/dmd/backend/cod3.d#L4879-L4888

Which means #2278 introduced a regression.

@ibuclaw
Copy link
Member Author

ibuclaw commented Aug 3, 2021

Remaining issue in pipeline is due a PIC bug in the host compiler (2.079.0), fixed in 2.090 by #10696.

@ibuclaw ibuclaw force-pushed the issue21488 branch 2 times, most recently from 18448e6 to 1dfe589 Compare August 3, 2021 23:21
src/build.d Outdated Show resolved Hide resolved
@ibuclaw ibuclaw force-pushed the issue21488 branch 2 times, most recently from 3de65cc to 468ef58 Compare August 4, 2021 11:25
@ibuclaw
Copy link
Member Author

ibuclaw commented Aug 6, 2021

This should really be pulled as soon as, because the longer it's left, the harder/longer it'll be to wean off ubuntu 14.04/16.04. DMD has already shot itself by not doing this 2-3 years ago with 64bit Linux.

@dlang-bot dlang-bot merged commit ffcc851 into dlang:stable Aug 6, 2021
@ibuclaw ibuclaw deleted the issue21488 branch August 6, 2021 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants