fcn errors should * never * be captured in the innerloop of the interpreter. a. failure should/marks the block as erronous during parsing or type resolution. mb->error needs to be set instead.
On 6 mrt. 2011, at 14:21, [email protected] wrote: > Fabian, > > I totally agree with defensive code to prevent segfault and other > "unnecessary errors". > > However, this checkin makes the follwoing tests fail by producing a > different error than before, i.e., they now fail with the newly added > parseError --- and most probably correctly, so, because the tests (try to) > trigger some other error but indeed declare command that reference a > non-existing address to do so. > > monetdb5/mal/Tests/tst102.mal > monetdb5/mal/Tests/tst104.mal > monetdb5/mal/Tests/tst105.mal > monetdb5/mal/Tests/tst105a.mal > monetdb5/mal/Tests/tst106.mal > monetdb5/mal/Tests/tst115.mal > monetdb5/mal/Tests/tst150.mal > monetdb5/mal/Tests/dynamicload.mal > > I'm not sure, whether we should simply approve the new output, as then the > tests would no longer test what they originally intended to test. > > Probably, we should design the tests properly, but I could not come up with > a quick solution. > > In case your newly added parseError defense is a bit too strict, i.e., > required only to prevent calling malAtomProperty() with curInstr->fcn == > NULL, the following patch might be considered: > > ======== > diff -r a4f844538f0c monetdb5/mal/mal_parser.mx > --- a/monetdb5/mal/mal_parser.mx Sun Mar 06 12:59:51 2011 +0100 > +++ b/monetdb5/mal/mal_parser.mx Sun Mar 06 14:01:48 2011 +0100 > @@ -1330,11 +1330,12 @@ > setModuleScope(curInstr, > findModule(cntxt->nspace, > putName(modnme,strlen(modnme))) ); > curInstr->fcn = getAddress(cntxt->srcFile, modnme, nme,TRUE); > - if (curInstr->fcn == NULL) > - return (MalBlkPtr) parseError(cntxt, "<address> not > found\n"); > curBlk->binding = nme; > - if( cntxt->nspace->isAtomModule) > + if( cntxt->nspace->isAtomModule) { > + if (curInstr->fcn == NULL) > + return (MalBlkPtr) xparseError(cntxt, > "<address> not found\n"); > malAtomProperty(curBlk, curInstr); > + } > skipSpace(cntxt); > } else { > return (MalBlkPtr) parseError(cntxt,"'address' expected\n"); > ======== > > This would assume, though, that curInstr->fcn == NULL in other cases is > properly checked and handled elsewhere ... > > Any opinions? > > Stefan > > > On Sat, Mar 05, 2011 at 09:44:10PM +0100, Fabian Groffen wrote: >> Changeset: 8fa1212bfe7c for MonetDB >> URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=8fa1212bfe7c >> Modified Files: >> monetdb5/mal/mal_parser.mx >> Branch: Mar2011 >> Log Message: >> >> parseCommandPattern: return an error when an address wasn't found >> >> On Darwin some more modules than on Linux are missing, which is probably >> the cause why this went unnoticed. If no address was found, fcn will be >> NULL, which in turn will lead to a dereference in malAtomPropery. So >> return an error instead. >> >> >> diffs (12 lines): >> >> diff --git a/monetdb5/mal/mal_parser.mx b/monetdb5/mal/mal_parser.mx >> --- a/monetdb5/mal/mal_parser.mx >> +++ b/monetdb5/mal/mal_parser.mx >> @@ -1330,6 +1330,8 @@ >> setModuleScope(curInstr, >> findModule(cntxt->nspace, putName(modnme,strlen(modnme))) ); >> curInstr->fcn = getAddress(cntxt->srcFile, modnme, nme,TRUE); >> + if (curInstr->fcn == NULL) >> + return (MalBlkPtr) parseError(cntxt, "<address> not found\n"); >> curBlk->binding = nme; >> if( cntxt->nspace->isAtomModule) >> malAtomProperty(curBlk, curInstr); >> _______________________________________________ >> Checkin-list mailing list >> [email protected] >> http://mail.monetdb.org/mailman/listinfo/checkin-list > > -- > | Stefan.Manegold @ CWI.nl | DB Architectures (INS1) | > | http://CWI.nl/~manegold/ | Science Park 123 (L321) | > | Tel.: +31 (0)20 592-4212 | 1098 XG Amsterdam (NL) | > _______________________________________________ > Checkin-list mailing list > [email protected] > http://mail.monetdb.org/mailman/listinfo/checkin-list _______________________________________________ Checkin-list mailing list [email protected] http://mail.monetdb.org/mailman/listinfo/checkin-list
