Custom assert handler to point to bugzilla #6103

mihails-strasuns · 2016-09-02T17:42:49Z

Makes it more obvious that any failed assertion is a compiler
bug to be reported. Inspired by http://forum.dlang.org/thread/[email protected]

quickfur · 2016-09-02T18:04:29Z

Strange, tested this locally but asserts don't seem to trigger the custom error message?

quickfur · 2016-09-02T18:07:30Z

Ah, nevermind, the problem is that the backend code uses a different assert function (the one from C/C++). Any way to redirect those asserts as well?

mihails-strasuns · 2016-09-02T18:09:48Z

Will have to look into that, my C has got a bit rusty :)

PetarKirov · 2016-09-02T18:10:11Z

I was just about to ask if it works in practice (obviously can't be unittested reliably). IIRC, dmd is built with -release, so calls to the assert handler would be replaced with halt instructions?

mihails-strasuns · 2016-09-02T18:14:10Z

IIRC, dmd is built with -release

That shouldn't be the case, otherwise one wouldn't see the assertion message, just the immediate segfault.

EDIT: ah, forgot that C code is compiled differently, yes, this needs to be taken into account

quickfur · 2016-09-02T18:14:31Z

I'm testing a debug build, but it seems that every ICE I can find on bugzilla either comes from the backend, or involves a segfault (so doesn't go through this code). I'm having trouble actually seeing the new error message appear. :-P

mihails-strasuns · 2016-09-02T18:15:39Z

@quickfur looks like with C assert it is tricky. Would exposing extern(C) wrapper from D and switching C sources to use it instead be a viable option?

quickfur · 2016-09-02T18:17:46Z

haha, finally found an ICE that triggers this error: https://issues.dlang.org/show_bug.cgi?id=16284

It seems that you need a \n at the end of the message. :-)

quickfur · 2016-09-02T18:18:51Z

@Dicebot Is it because the C assert is a macro?

quickfur · 2016-09-02T18:22:20Z

There's also util_assert in backend/util2.c, which might be a bit easier to work with. In fact, why isn't util_assert used consistently?

quickfur · 2016-09-02T18:27:53Z

Another idea: assert.h is explicitly #include'd by several dmd source files; possibly we can hack it by redirecting it to a custom header file that forwards assert to the D assert handler?

jacob-carlborg · 2016-09-03T11:02:47Z

It would be nice to get a stack trace for the assertions in the C code. Using the platform supplied assert, at least on OS X, will print a stack trace.

AndrejMitrovic · 2016-09-03T11:24:22Z

I was going to suggest adding a test-case, but we want to get rid of compiler bugs that trigger these. 😄

codecov-io · 2016-09-03T12:26:36Z

Current coverage is 87.41% (diff: 14.28%)

No coverage report found for master at fb2fa43.

Powered by Codecov. Last update fb2fa43...d4b055c

mihails-strasuns · 2016-09-03T12:27:55Z

Ok, I decided initially to simply go for ensuring built-in assert is never used and adding same bugzilla reference to custom assert function. Using D assert feels tempting but I am not sure what will happen if one attempts to throw an exception/error from C code.

Makes it more obvious that any failed assertion is a compiler bug to be reported.

mihails-strasuns · 2016-09-03T17:17:34Z

Green apart from code coverage checks.

wilzbach · 2016-09-03T17:52:29Z

src/backend/util2.c

    fflush(stdout);
    printf("Internal error: %s %d\n",file,line);
+    printf("This is a compiler bug, please report it via "
+        "https://issues.dlang.org/enter_bug.cgi\n");


How about pre-filling some selections?
e.g. https://issues.dlang.org/enter_bug.cgi?component=dmd&bug_severity=critical&keywords=ice

(we could make a short URL or short domain (e.g. https://bugs.dlang.io/ice out of this, if that's too long)

As someone on the NG proposed: What about a link to the DWiki instead which describes further steps? (e.g. how to dustmite, what to mention in the bug report, ...)

Just an empty bug report page can be quite intimidating for a user.

We don't seem to have such a page as of now, but maybe we can take a bit of information from Get involved and start a new one?

the issue writing guidelines seems to be a generic page from Bugzilla

Just an empty bug report page can be quite intimidating for a user.

If they start typing out the title they might find a duplicate one listed which could show a way to work around the bug. Even if they end up submitting a duplicate report this would get attention from bugzilla lurkers and they would get feedback pretty soon.

It would really suck if bugs like this went unreported. Bugzilla seems like a good first choice, but an ICE should be rare enough that adding two links to the diagnostics isn't such an overwhelming idea. Something like:

This is an unexpected internal compiler bug, please submit a bug report via https://issues.dlang.org/enter_bug.cgi? component=dmd&bug_severity=critical&keywords=ice For more info on internal compiler errors see <this wiki page>

should be major, not critical.

I wonder if having this link would encourage spammers to post on bugzilla by raising the visibility of it

Because the URL will be in the source code? Perhaps obfuscate it using base64 encoding or similar.

Bugzilla URLs are already in various parts of our website (and therefore on github), so I'm not sure how adding it to the source code makes it different?

I guess we'll see! So far we haven't had any trouble with abusive bugzilla postings.

But still, it should be major, not critical. Critical would be things like silently generating bad code, which is much worse than an assert.

mihails-strasuns · 2016-09-05T02:09:48Z

FYI: I will be travelling for next 1.5 weeks and will only get back to this PR after. Feel free to take it over if you want to merge it faster.

mathias-lang-sociomantic · 2016-09-27T09:43:40Z

I was just about to ask if it works in practice (obviously can't be unittested reliably). IIRC, dmd is built with -release, so calls to the assert handler would be replaced with halt instructions?

It isn't built with -release locally, but shipped with -release, so the assertHandler will never be used.
See the release script enable those lines.

It's the case since the switch DDMD, I'll open a discussion on dmd-internals.

mihails-strasuns · 2016-09-28T12:21:56Z

It's the case since the switch DDMD, I'll open a discussion on dmd-internals.

Yes, this is a blocker. I'll wait for the thread to resolve before updating this.

wilzbach · 2016-12-21T08:05:37Z

It isn't built with -release locally, but shipped with -release, so the assertHandler will never be used.
It's the case since the switch DDMD, I'll open a discussion on dmd-internals.
Yes, this is a blocker. I'll wait for the thread to resolve before updating this.

So is it possible to release dmd with asserts enabled or are the better options?

wilzbach · 2017-03-22T20:49:20Z

So is it possible to release dmd with asserts enabled or are the better options?

Sounds to me like DIP1006 would solve this problem (compiling dmd in release mode, but with asserts enabled).

dnadlinger · 2017-03-22T20:59:38Z

Or we could use a signal handler to detect the assert(0)s. (LDC already supports selective toggling of asserts and other contract-type features, if that is where the holdup is.)

wilzbach · 2018-04-05T03:13:11Z

Revived this in #8138

Dicebot added 3 commits September 3, 2016 15:29

Custom assert handler to point to bugzilla

2635964

Makes it more obvious that any failed assertion is a compiler bug to be reported.

Mention bugzilla in backend assert handler

92c1430

Always use tassert.h instead of assert.h

027083f

wilzbach reviewed Sep 3, 2016
View reviewed changes

wilzbach added the Merge:Blocked label Sep 28, 2016

wilzbach mentioned this pull request Mar 22, 2017

[Memory Corruption] add toTypeFunction() to catch bad casts #6643

Merged

mihails-strasuns closed this Sep 14, 2017

wilzbach mentioned this pull request Apr 4, 2018

Automatically print the segtrace for segfaulting tests #8132

Merged

wilzbach mentioned this pull request Apr 5, 2018

Point users to Bugzilla on ICEs #8138

Merged

Uh oh!

Custom assert handler to point to bugzilla #6103

Custom assert handler to point to bugzilla #6103

Uh oh!

Conversation

mihails-strasuns commented Sep 2, 2016

Uh oh!

quickfur commented Sep 2, 2016

Uh oh!

quickfur commented Sep 2, 2016

Uh oh!

mihails-strasuns commented Sep 2, 2016

Uh oh!

PetarKirov commented Sep 2, 2016

Uh oh!

mihails-strasuns commented Sep 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quickfur commented Sep 2, 2016

Uh oh!

mihails-strasuns commented Sep 2, 2016

Uh oh!

quickfur commented Sep 2, 2016

Uh oh!

quickfur commented Sep 2, 2016

Uh oh!

quickfur commented Sep 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quickfur commented Sep 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacob-carlborg commented Sep 3, 2016

Uh oh!

AndrejMitrovic commented Sep 3, 2016

Uh oh!

codecov-io commented Sep 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 87.41% (diff: 14.28%)

Uh oh!

mihails-strasuns commented Sep 3, 2016

Uh oh!

mihails-strasuns commented Sep 3, 2016

Uh oh!

wilzbach Sep 3, 2016

Choose a reason for hiding this comment

Uh oh!

AndrejMitrovic Sep 3, 2016 • 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!

WalterBright Sep 4, 2016

Choose a reason for hiding this comment

Uh oh!

jacob-carlborg Sep 4, 2016

Choose a reason for hiding this comment

Uh oh!

AndrejMitrovic Sep 4, 2016

Choose a reason for hiding this comment

Uh oh!

WalterBright Sep 5, 2016

Choose a reason for hiding this comment

Uh oh!

WalterBright Sep 5, 2016

Choose a reason for hiding this comment

Uh oh!

mihails-strasuns commented Sep 5, 2016

Uh oh!

mathias-lang-sociomantic commented Sep 27, 2016

Uh oh!

mihails-strasuns commented Sep 28, 2016

Uh oh!

wilzbach commented Dec 21, 2016

Uh oh!

wilzbach commented Mar 22, 2017

Uh oh!

dnadlinger commented Mar 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Apr 5, 2018

mihails-strasuns commented Sep 2, 2016 •

edited

Loading

quickfur commented Sep 2, 2016 •

edited

Loading

quickfur commented Sep 2, 2016 •

edited

Loading

codecov-io commented Sep 3, 2016 •

edited

Loading

AndrejMitrovic Sep 3, 2016 •

edited

Loading

dnadlinger commented Mar 22, 2017 •

edited

Loading