Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issue 796 - Asserting a null object reference throws AssertError Failure internal\invariant.d(14) or Access Violation #358

Merged
merged 1 commit into from

3 participants

@yebblies
Collaborator

Rewrite assert calls of pointers to structs with invariants and classss references to:
assert(e); -> auto tmp = e, e || assert, e->invariant()

Most of the lines changed are indentation changes due to making some code inside an if {} unconditional.

Add ?w=1 to the url to view the diff without whitespace changes.

http://d.puremagic.com/issues/show_bug.cgi?id=796

@yebblies yebblies Issue 796 - Asserting a null object reference throws AssertError Fail…
…ure internal\invariant.d(14) or Access Violation

Rewrite assert calls of pointers to structs with invariants and classss references to:
`assert(e);` -> `auto tmp = e, e || assert, e->invariant()`
95d6692
@alexrp

PLEASE PLEASE PLEASE merge this. The way assert(obj) works right now is seriously an atrocity and for damn sure does not help people transition to D. Hell, even with the behavior of checking obj's invariant, you'd expect it to make sure the object is actually valid in the first place.

@yebblies
Collaborator

While I appreciate your enthusiasm, I'm the only one who gets notified when you post here. Posting on the newsgroup is likely to get more attention, and voting in bugzilla is always good too.

@WalterBright WalterBright merged commit 7d942b8 into D-Programming-Language:master
@braddr braddr referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 30, 2011
  1. @yebblies

    Issue 796 - Asserting a null object reference throws AssertError Fail…

    yebblies authored
    …ure internal\invariant.d(14) or Access Violation
    
    Rewrite assert calls of pointers to structs with invariants and classss references to:
    `assert(e);` -> `auto tmp = e, e || assert, e->invariant()`
This page is out of date. Refresh to see the latest.
Showing with 81 additions and 59 deletions.
  1. +68 −59 src/e2ir.c
  2. +13 −0 test/runnable/xtest46.d
View
127 src/e2ir.c
@@ -1925,6 +1925,8 @@ elem *AssertExp::toElem(IRState *irs)
if (global.params.useAssert)
{
e = e1->toElem(irs);
+ symbol *ts = NULL;
+ elem *einv = NULL;
InvariantDeclaration *inv = (InvariantDeclaration *)(void *)1;
@@ -1932,10 +1934,11 @@ elem *AssertExp::toElem(IRState *irs)
if (global.params.useInvariants && t1->ty == Tclass &&
!((TypeClass *)t1)->sym->isInterfaceDeclaration())
{
+ ts = symbol_genauto(t1->toCtype());
#if TARGET_LINUX || TARGET_FREEBSD || TARGET_SOLARIS
- e = el_bin(OPcall, TYvoid, el_var(rtlsym[RTLSYM__DINVARIANT]), e);
+ einv = el_bin(OPcall, TYvoid, el_var(rtlsym[RTLSYM__DINVARIANT]), el_var(ts));
#else
- e = el_bin(OPcall, TYvoid, el_var(rtlsym[RTLSYM_DINVARIANT]), e);
+ einv = el_bin(OPcall, TYvoid, el_var(rtlsym[RTLSYM_DINVARIANT]), el_var(ts));
#endif
}
// If e1 is a struct object, call the struct invariant on it
@@ -1944,81 +1947,87 @@ elem *AssertExp::toElem(IRState *irs)
t1->nextOf()->ty == Tstruct &&
(inv = ((TypeStruct *)t1->nextOf())->sym->inv) != NULL)
{
- e = callfunc(loc, irs, 1, inv->type->nextOf(), e, e1->type, inv, inv->type, NULL, NULL);
+ ts = symbol_genauto(t1->toCtype());
+ einv = callfunc(loc, irs, 1, inv->type->nextOf(), el_var(ts), e1->type, inv, inv->type, NULL, NULL);
}
- else
- {
- // Construct: (e1 || ModuleAssert(line))
- Module *m = irs->blx->module;
- char *mname = m->srcfile->toChars();
- //printf("filename = '%s'\n", loc.filename);
- //printf("module = '%s'\n", m->srcfile->toChars());
+ // Construct: (e1 || ModuleAssert(line))
+ Module *m = irs->blx->module;
+ char *mname = m->srcfile->toChars();
- /* Determine if we are in a unittest
- */
- FuncDeclaration *fd = irs->getFunc();
- UnitTestDeclaration *ud = fd ? fd->isUnitTestDeclaration() : NULL;
+ //printf("filename = '%s'\n", loc.filename);
+ //printf("module = '%s'\n", m->srcfile->toChars());
- /* If the source file name has changed, probably due
- * to a #line directive.
- */
- if (loc.filename && (msg || strcmp(loc.filename, mname) != 0))
- { elem *efilename;
+ /* Determine if we are in a unittest
+ */
+ FuncDeclaration *fd = irs->getFunc();
+ UnitTestDeclaration *ud = fd ? fd->isUnitTestDeclaration() : NULL;
- /* Cache values.
- */
- //static Symbol *assertexp_sfilename = NULL;
- //static char *assertexp_name = NULL;
- //static Module *assertexp_mn = NULL;
+ /* If the source file name has changed, probably due
+ * to a #line directive.
+ */
+ if (loc.filename && (msg || strcmp(loc.filename, mname) != 0))
+ { elem *efilename;
- if (!assertexp_sfilename || strcmp(loc.filename, assertexp_name) != 0 || assertexp_mn != m)
- {
- dt_t *dt = NULL;
- const char *id;
- int len;
-
- id = loc.filename;
- len = strlen(id);
- dtsize_t(&dt, len);
- dtabytes(&dt,TYnptr, 0, len + 1, id);
-
- assertexp_sfilename = symbol_generate(SCstatic,type_fake(TYdarray));
- assertexp_sfilename->Sdt = dt;
- assertexp_sfilename->Sfl = FLdata;
+ /* Cache values.
+ */
+ //static Symbol *assertexp_sfilename = NULL;
+ //static char *assertexp_name = NULL;
+ //static Module *assertexp_mn = NULL;
+
+ if (!assertexp_sfilename || strcmp(loc.filename, assertexp_name) != 0 || assertexp_mn != m)
+ {
+ dt_t *dt = NULL;
+ const char *id;
+ int len;
+
+ id = loc.filename;
+ len = strlen(id);
+ dtsize_t(&dt, len);
+ dtabytes(&dt,TYnptr, 0, len + 1, id);
+
+ assertexp_sfilename = symbol_generate(SCstatic,type_fake(TYdarray));
+ assertexp_sfilename->Sdt = dt;
+ assertexp_sfilename->Sfl = FLdata;
#if ELFOBJ
- assertexp_sfilename->Sseg = CDATA;
+ assertexp_sfilename->Sseg = CDATA;
#endif
#if MACHOBJ
- assertexp_sfilename->Sseg = DATA;
+ assertexp_sfilename->Sseg = DATA;
#endif
- outdata(assertexp_sfilename);
+ outdata(assertexp_sfilename);
- assertexp_mn = m;
- assertexp_name = id;
- }
+ assertexp_mn = m;
+ assertexp_name = id;
+ }
- efilename = el_var(assertexp_sfilename);
+ efilename = el_var(assertexp_sfilename);
- if (msg)
- { elem *emsg = msg->toElem(irs);
- ea = el_var(rtlsym[ud ? RTLSYM_DUNITTEST_MSG : RTLSYM_DASSERT_MSG]);
- ea = el_bin(OPcall, TYvoid, ea, el_params(el_long(TYint, loc.linnum), efilename, emsg, NULL));
- }
- else
- {
- ea = el_var(rtlsym[ud ? RTLSYM_DUNITTEST : RTLSYM_DASSERT]);
- ea = el_bin(OPcall, TYvoid, ea, el_param(el_long(TYint, loc.linnum), efilename));
- }
+ if (msg)
+ { elem *emsg = msg->toElem(irs);
+ ea = el_var(rtlsym[ud ? RTLSYM_DUNITTEST_MSG : RTLSYM_DASSERT_MSG]);
+ ea = el_bin(OPcall, TYvoid, ea, el_params(el_long(TYint, loc.linnum), efilename, emsg, NULL));
}
else
{
- Symbol *sassert = ud ? m->toModuleUnittest() : m->toModuleAssert();
- ea = el_bin(OPcall,TYvoid,el_var(sassert),
- el_long(TYint, loc.linnum));
+ ea = el_var(rtlsym[ud ? RTLSYM_DUNITTEST : RTLSYM_DASSERT]);
+ ea = el_bin(OPcall, TYvoid, ea, el_param(el_long(TYint, loc.linnum), efilename));
}
- e = el_bin(OPoror,TYvoid,e,ea);
}
+ else
+ {
+ Symbol *sassert = ud ? m->toModuleUnittest() : m->toModuleAssert();
+ ea = el_bin(OPcall,TYvoid,el_var(sassert),
+ el_long(TYint, loc.linnum));
+ }
+ if (einv)
+ { // tmp = e, e || assert, e->inv
+ elem *eassign = el_bin(OPeq, e->Ety, el_var(ts), e);
+ e = el_combine(eassign, el_bin(OPoror, TYvoid, el_var(ts), ea));
+ e = el_combine(e, einv);
+ }
+ else
+ e = el_bin(OPoror,TYvoid,e,ea);
}
else
{ // BUG: should replace assert(0); with a HLT instruction
View
13 test/runnable/xtest46.d
@@ -2293,6 +2293,18 @@ struct Foo126
/***************************************************/
+void test796()
+{
+ struct S { invariant() { throw new Exception(""); } }
+ S* s;
+ try {
+ assert(s);
+ } catch (Error) {
+ }
+}
+
+/***************************************************/
+
struct Tuple127(S...)
{
S expand;
@@ -3570,6 +3582,7 @@ int main()
test47();
test48();
test49();
+ test796();
test50();
test51();
test52();
Something went wrong with that request. Please try again.