Don't generate exception handling code if `Throwable` is not declared by JinShil · Pull Request #7786 · dlang/dmd

JinShil · 2018-01-26T14:05:23Z

This PR is a follow up to #7395 and #7768. It is to Throwable what #7395 and #7768 were to ModuleInfo.

In D, anything that can be thrown must inherit from object.Throwable. Therefore, if object.Throwable does not exist, no exception handling code should be generated, and any attempt to throw should emit compilation error.

With #7395, #7768, and this PR, we can now generate a minimal executable with almost no boilerplate and produce very small resulting binaries.

Example 1

object.d

module object;

private alias extern(C) int function(char[][] args) MainFunc;
private extern (C) int _d_run_main(int argc, char** argv, MainFunc mainFunc)
{
    return mainFunc(null);
}

main.d

void main() { }

Compile

dmd -conf= -defaultlib= main.d object.d -of=main

Result

size main
   text	   data	    bss	    dec	    hex	filename
   1403	    584	     16	   2003	    7d3	main

Example 2

object.d

module object;

extern(C) void __d_sys_exit(long arg1)
{
    asm
    {
        mov RAX, 60;
        mov RDI, arg1;
        syscall;
    }
}

extern void main();
private extern(C) void _start()
{
    main();
    __d_sys_exit(0);
}

main.d

void main() { }

Compile and link

dmd -c -lib main.d object.d -of=main.o
ld main.o -o main

Result

size main
   text	   data	    bss	    dec	    hex	filename
     56	      0	      0	     56	     38	main

Again, this will be of interest to users wishing to incrementally or partially port D to new platforms, users hoping to target resource constrained platforms, and users wishing to use D as a library from other languages without druntime.

dlang-bot · 2018-01-26T14:05:24Z

Thanks for your pull request, @JinShil!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

wilzbach

How about adding the second example to the testsuite too?

wilzbach · 2018-01-26T14:16:27Z

src/dmd/dclass.d

 extern (C++) class ClassDeclaration : AggregateDeclaration
 {
-    extern (C++) __gshared
+    extern (C++) static __gshared


It's already __gshared. DMD should emit a warning when both are used as this doesn't make any sense.

I was following the example here:

dmd/src/dmd/dmodule.d

Line 312 in e17992f

extern (C++) static __gshared AggregateDeclaration moduleinfo;

But, I'll change it.

JinShil · 2018-01-27T06:25:05Z

How about adding the second example to the testsuite too?

There's a few concerns I have about that:

It requires compiling and linking separately due to all the hard-coded linking logic in DMD. I'm not sure how to do that in the test suite.
It's very Linux 64-bit specific.
I think verifying the size will be quite brittle and too sensitive to small insignificant changes.

But I'll add it if it's desired, and I can figure out how to compile and link separately in the test suite.

WalterBright · 2018-01-27T09:03:30Z

src/dmd/backend/mscoffobj.c

-    SegData[seg]->SDbuf->write(name, strlen(name));
-    SegData[seg]->SDbuf->writeByte('"');
-    return true;
+    if (name && strlen(name) > 0)


This isn't checked in cgobj.c, why here? What does it have to do with Throwable?

It's because the tests are compiled with defaultlib=, and the mircosoft linker rejects empty arguments to /DEFAULTLIB:. It's just necessary to get the tests to pass on Windows.

BTW, in glue.d where this function is called:

if (global.params.mscrtlib && global.params.mscrtlib[0]) objmod.includelib(global.params.mscrtlib);

so now the check is being done twice. Also, the strlen can be safely replaced with [0].

WalterBright · 2018-01-27T09:04:36Z

src/dmd/link.d

+                    cmdbuf.writeByte(' ');
+                    cmdbuf.writestring("/DEFAULTLIB:");
+                    writeFilename(&cmdbuf, f);
+                }


This looks like it belongs in a separate PR?

I could do that, but I don't have a test to verify it. This PR has the logic necessary to create a test that actually benefits from defaultlib= (i.e. no defaultlib). So, I think they need to go together.

Should clearly set up the code so that no default lib has defaultlib set to null, not a 0 length string.

WalterBright · 2018-01-27T09:06:17Z

src/dmd/statementsem.d


-        if (!global.params.useExceptions)       // if not worrying about exceptions
+        // if not worrying about exceptions
+        if (!global.params.useExceptions || !ClassDeclaration.throwable)


Elsewhere is written useExceptions && throwable, so I suggest rewriting this to be consistent:

if (!(global.params.useExceptions && ClassDeclaration.throwable))

WalterBright · 2018-01-27T09:12:48Z

src/dmd/link.d

-                writeFilename(&cmdbuf, global.params.libfiles[i]);
+                // The `/DEFAULTLIB:` argument cannot be empty
+                const f = global.params.libfiles[i];
+                if (f && strlen(f) > 0)


replace strlen(f) > 0 with f[0]

also, perhaps this check should be done where libfiles[] is filled, rather than where it is used.

WalterBright · 2018-01-27T09:14:09Z

Good idea!

JinShil · 2018-01-27T16:43:38Z

The tests in this PR require defaultlib= (i.e. no defaultlib). Prior to this PR, that parameter set defaultlibname to an empty string, which was then passed to the /DEFAULTLIB: argument of Microsoft's linker on Windows. However, Microsoft's linker throws an error if the argument to /DEFAULTLIB: is empty.

Per @WalterBright's suggestion in prior comments in this PR, defaultlibname is now set to null in mars.d. However, that means logic in link.d and glue.d now needs to check defaultlibname for null to ensure there are no segfaults and that the /DEFAULTLIB: argument is not empty.

@WalterBright, this is ready for another look. Thanks.

wilzbach reviewed Jan 26, 2018

View reviewed changes

JinShil added Review:WIP Work In Progress - not ready for review or pulling and removed Review:WIP Work In Progress - not ready for review or pulling labels Jan 26, 2018

WalterBright reviewed Jan 27, 2018

View reviewed changes

JinShil requested review from MartinNowak, mathias-lang-sociomantic and rainers as code owners January 27, 2018 09:50

Don't generate exception handling code if Throwable is not declared

8f16880

WalterBright added the Merge:auto-merge label Jan 28, 2018

dlang-bot merged commit ec4534e into dlang:master Jan 28, 2018

This was referenced Jan 28, 2018

Don't require TypeInfo in a minimal runtime if it's not needed #7799

Merged

Don't require object.d for -lib if code does not utilize runtime features #7825

Closed

Add changelog for minimal runtime implementation PRs #7829

Closed

This was referenced Feb 13, 2018

Fix no_Throwable test so it doesn't import druntime's object.d #7883

Merged

Add changelog for minimal runtime implementation PRs #7887

Merged

wilzbach mentioned this pull request Mar 18, 2018

Fix issue 18242 - Warn if object.Exception is missing #8050

Merged

JinShil mentioned this pull request Apr 24, 2018

Add ability to use interfaces and classes without the runtime if they only contain static members #8204

Merged

JinShil deleted the optional_throwable branch April 24, 2018 10:26

JinShil mentioned this pull request May 18, 2019

Add -revert=noautoimports to begin transitioning to a pay-as-you-go opt-in langauge #9814

Closed

JinShil mentioned this pull request Aug 26, 2019

Move hard-coded logic to generate C main out of the compiler and into druntime #10351

Merged

Uh oh!

Comments

Conversation

JinShil commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example 1

Example 2

Uh oh!

dlang-bot commented Jan 26, 2018

Bugzilla references

Uh oh!

wilzbach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JinShil commented Jan 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JinShil Jan 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WalterBright commented Jan 27, 2018

Uh oh!

JinShil commented Jan 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JinShil commented Jan 26, 2018 •

edited

Loading

JinShil commented Jan 27, 2018 •

edited

Loading

JinShil Jan 27, 2018 •

edited

Loading