Skip to content

Custom assert handler to point to bugzilla #6103

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Custom assert handler to point to bugzilla #6103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2635964
Custom assert handler to point to bugzilla
Sep 2, 2016
92c1430
Mention bugzilla in backend assert handler
Sep 3, 2016
027083f
Always use tassert.h instead of assert.h
Sep 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/backend/aa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

#include "tinfo.h"
#include "aa.h"

static char __file__[] = __FILE__; /* for tassert.h */
#include "tassert.h"

// Implementation of associative array
// Auto-rehash and pre-allocate - Dave Fladebo

Expand Down
4 changes: 3 additions & 1 deletion src/backend/divcoeff.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ Source: https://github.com/dlang/dmd/blob/master/src/backend/divcoeff.c
*/

#include <stdio.h>
#include <assert.h>

static char __file__[] = __FILE__; /* for tassert.h */
#include "tassert.h"

typedef unsigned long long ullong;

Expand Down
2 changes: 2 additions & 0 deletions src/backend/util2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ void util_assert(const char *file, int line)
{
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");
Copy link
Contributor

@wilzbach wilzbach Sep 3, 2016

Choose a reason for hiding this comment

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

  1. 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)

  1. 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

Copy link
Contributor

@AndrejMitrovic AndrejMitrovic Sep 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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>

Copy link
Member

@WalterBright WalterBright Sep 4, 2016

Choose a reason for hiding this comment

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

  1. should be major, not critical.
  2. I wonder if having this link would encourage spammers to post on bugzilla by raising the visibility of it

Copy link
Contributor

@jacob-carlborg jacob-carlborg Sep 4, 2016

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@AndrejMitrovic AndrejMitrovic Sep 4, 2016

Choose a reason for hiding this comment

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

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?

Copy link
Member

@WalterBright WalterBright Sep 5, 2016

Choose a reason for hiding this comment

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

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

Copy link
Member

@WalterBright WalterBright Sep 5, 2016

Choose a reason for hiding this comment

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

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

err_exit();
#if __clang__
__builtin_unreachable();
Expand Down
19 changes: 19 additions & 0 deletions src/mars.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,7 @@ int main()
{
import core.memory;
import core.runtime;
import core.exception;

version (GC)
{
Expand Down Expand Up @@ -1604,6 +1605,24 @@ int main()
dmd_coverSetMerge(true);
}

assertHandler = (string file, size_t line, string msg) nothrow {
import ddmd.errors;
import core.stdc.stdio;

fprintf(stderr, "Fatal failure on line %d in file '%s'",
line, file.ptr);

if (msg.length)
fprintf(stderr, ": %s\n", msg.ptr);
else
fprintf(stderr, "\n");

fprintf(stderr, "This is a compiler bug, please report it via " ~
"https://issues.dlang.org/enter_bug.cgi\n");

exit(EXIT_FAILURE);
};

auto args = Runtime.cArgs();
return tryMain(args.argc, cast(const(char)**)args.argv);
}
Expand Down