Thread overview | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
January 24, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. Since this druntime change, the win32 dmd test34.d test has failed: https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c51272c5804de8df27af75099b977 http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441 Before test 3: object.Error at src\rt\deh.d(631): Access Violation I did a test build at the previous commit and the test passes. So that's definitely the one that introduces the problem. It might well be just exposing a dormant problem, but either way, it stopped working at that point. |
January 24, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Brad Roberts | I hadn't looked at that diff yet. It doesnt look right to me. Can __FILE__ really be used in that way?
Sent from my iPhone
On Jan 24, 2011, at 8:00 PM, Brad Roberts <braddr at puremagic.com> wrote:
> Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. Since this druntime change, the win32 dmd test34.d test has failed:
>
> https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c51272c5804de8df27af75099b977
>
> http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441
>
> Before test 3:
> object.Error at src\rt\deh.d(631): Access Violation
>
> I did a test build at the previous commit and the test passes. So that's definitely the one that introduces the problem. It might well be just exposing a dormant problem, but either way, it stopped working at that point.
>
> _______________________________________________
> D-runtime mailing list
> D-runtime at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/d-runtime
|
January 24, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Sean Kelly | I scanned the diff and didn't see anything we're not doing elsewhere already. The default arg idiom has worked for a while. The access violation is coming from inside deh.d itself, though I haven't gone to see what line 631 is yet.
On 1/24/2011 8:26 PM, Sean Kelly wrote:
> I hadn't looked at that diff yet. It doesnt look right to me. Can __FILE__ really be used in that way?
>
> Sent from my iPhone
>
> On Jan 24, 2011, at 8:00 PM, Brad Roberts <braddr at puremagic.com> wrote:
>
>> Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. Since this druntime change, the win32 dmd test34.d test has failed:
>>
>> https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c51272c5804de8df27af75099b977
>>
>> http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441
>>
>> Before test 3:
>> object.Error at src\rt\deh.d(631): Access Violation
>>
>> I did a test build at the previous commit and the test passes. So that's definitely the one that introduces the problem. It might well be just exposing a dormant problem, but either way, it stopped working at that point.
>>
|
January 24, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Brad Roberts | On Monday 24 January 2011 20:55:42 Brad Roberts wrote:
> I scanned the diff and didn't see anything we're not doing elsewhere already. The default arg idiom has worked for a while. The access violation is coming from inside deh.d itself, though I haven't gone to see what line 631 is yet.
All the tests passed for Linux and Windows after I checked it in. It shouldn't have caused any breaking changes as far as APIs go. As for using __FILE__ and __LINE__ as default arguments, I do it all the time, and it works just fine. I even verified that it worked for the extern(C) functions before I gave them the default arguments as well. I don't know what the issue with deh.d is, but that commit _was_ working, and the basic idiom definitely works.
- Jonathan M Davis
|
January 26, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Brad Roberts | On 1/24/2011 8:00 PM, Brad Roberts wrote: > Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. Since this druntime change, the win32 dmd test34.d test has failed: > > https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c51272c5804de8df27af75099b977 > > http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441 > > Before test 3: > object.Error at src\rt\deh.d(631): Access Violation > > I did a test build at the previous commit and the test passes. So that's definitely the one that introduces the problem. It might well be just exposing a dormant problem, but either way, it stopped working at that point. Ok.. I finally took some time to dive into this one. It's failing on a catch (DerivedFromError) situation. >From test34.d: class B58 { long x; void set(long i) { writeln("B58::set(long)"); x = i; } void set(int i) { writeln("B58::set(int)"); x = i; } long squareIt() { writeln("B58::squareit()"); return x * x; } } class D58 : B58 { long square; void set(long i) { writeln("D58::set(long)"); B58.set(i); square = x * x; } long squareIt() { writeln("D58::squareit()"); return square; } } long foo58(B58 b) { int i; try { b.set(3); } catch (HiddenFuncError o) { i = 1; } assert(i == 1); return b.squareIt(); } void test58() { writeln(foo58(new D58)); } Catching Error derived classes is illegal-ish now, right? I haven't narrowed it down to exactly where the segv is coming from (the line number above is the part of the eh mechanism that's constructing the error object in the SEH receiving code, not the point of the segv.. unless that line of code is segving somehow. Don, do you have the time to dig a little since you've been all over that part of the code recently? Thanks, Brad |
January 26, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Brad Roberts | On Wednesday 26 January 2011 00:25:34 Brad Roberts wrote:
> On 1/24/2011 8:00 PM, Brad Roberts wrote:
> > Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. Since this druntime change, the win32 dmd test34.d test has failed:
> >
> > https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c512 72c5804de8df27af75099b977
> >
> > http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441
> >
> > Before test 3:
> > object.Error at src\rt\deh.d(631): Access Violation
> >
> > I did a test build at the previous commit and the test passes. So that's definitely the one that introduces the problem. It might well be just exposing a dormant problem, but either way, it stopped working at that point.
>
> Ok.. I finally took some time to dive into this one. It's failing on a catch (DerivedFromError) situation.
>
> From test34.d:
>
> class B58
> { long x;
> void set(long i) { writeln("B58::set(long)"); x = i; }
> void set(int i) { writeln("B58::set(int)"); x = i; }
> long squareIt() { writeln("B58::squareit()"); return x * x; }
> }
> class D58 : B58
> {
> long square;
> void set(long i) { writeln("D58::set(long)"); B58.set(i); square = x *
> x; } long squareIt() { writeln("D58::squareit()"); return square; } }
>
> long foo58(B58 b)
> { int i;
> try
> {
> b.set(3);
> }
> catch (HiddenFuncError o)
> {
> i = 1;
> }
> assert(i == 1);
> return b.squareIt();
> }
>
> void test58()
> {
> writeln(foo58(new D58));
> }
>
> Catching Error derived classes is illegal-ish now, right? I haven't narrowed it down to exactly where the segv is coming from (the line number above is the part of the eh mechanism that's constructing the error object in the SEH receiving code, not the point of the segv.. unless that line of code is segving somehow.
>
> Don, do you have the time to dig a little since you've been all over that part of the code recently?
As I understand it, catching Errors is perfectly legal, but because none of the proper cleanup is going to have been done between the point that Error was thrown from and the point that it was caught it, the state of things might be a bit messed up. The unit testing functions that I wrote which are currently under review catch Errors, and they'd be somewhat crippled if they couldn't.
Catching Errors is not generally a good idea, but as far as I understand it, it's supposed to be legal.
- Jonathan M Davis
|
January 26, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jonathan M Davis | On 26 January 2011 09:33, Jonathan M Davis <jmdavisProg at gmx.com> wrote: > On Wednesday 26 January 2011 00:25:34 Brad Roberts wrote: >> On 1/24/2011 8:00 PM, Brad Roberts wrote: >> > Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. ?Since this druntime change, the win32 dmd test34.d test has failed: >> > >> > https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c512 72c5804de8df27af75099b977 >> > >> > http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441 >> > >> > ? Before test 3: >> > ? object.Error at src\rt\deh.d(631): Access Violation >> > >> > I did a test build at the previous commit and the test passes. ?So that's definitely the one that introduces the problem. ?It might well be just exposing a dormant problem, but either way, it stopped working at that point. >> >> Ok.. I finally took some time to dive into this one. ?It's failing on a catch (DerivedFromError) situation. >> >> From test34.d: >> >> class B58 >> { ? ?long x; >> ? ? ?void set(long i) { writeln("B58::set(long)"); x = i; } >> ? ? ?void set(int i) ?{ writeln("B58::set(int)"); x = i; } >> ? ? ?long squareIt() ?{ writeln("B58::squareit()"); return x * x; } >> } >> class D58 : B58 >> { >> ? ? ?long square; >> ? ? ?void set(long i) { writeln("D58::set(long)"); B58.set(i); square = x * >> x; } long squareIt() ?{ writeln("D58::squareit()"); return square; } } >> >> long foo58(B58 b) >> { ? int i; >> ? ? try >> ? ? { >> ? ? ? ? b.set(3); >> ? ? } >> ? ? catch (HiddenFuncError o) >> ? ? { >> ? ? ? ? i = 1; >> ? ? } >> ? ? assert(i == 1); >> ? ? return b.squareIt(); >> } >> >> void test58() >> { >> ? ? writeln(foo58(new D58)); >> } >> >> Catching Error derived classes is illegal-ish now, right? ?I haven't narrowed it down to exactly where the segv is coming from (the line number above is the part of the eh mechanism that's constructing the error object in the SEH receiving code, not the point of the segv.. unless that line of code is segving somehow. >> >> Don, do you have the time to dig a little since you've been all over that part of the code recently? > > As I understand it, catching Errors is perfectly legal, but because none of the proper cleanup is going to have been done between the point that Error was thrown from and the point that it was caught it, the state of things might be a bit messed up. The unit testing functions that I wrote which are currently under review catch Errors, and they'd be somewhat crippled if they couldn't. > > Catching Errors is not generally a good idea, but as far as I understand it, it's supposed to be legal. Yes, that's correct, it's legal to catch errors. There is an obvious disastrous problem with that commit: the inclusion of __FILE__ and __LINE__ in the constructors for Error and Throwable is completely wrong. They should not be in there. That assumes that the exceptions are thrown via a throw statement, which is not correct, and will cause serious problems. Dunno if that's the only problem, though. Sorry, don't have any more time right now. |
January 26, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Don Clugston | On Wednesday 26 January 2011 01:11:10 Don Clugston wrote:
> On 26 January 2011 09:33, Jonathan M Davis <jmdavisProg at gmx.com> wrote:
> > On Wednesday 26 January 2011 00:25:34 Brad Roberts wrote:
> >> On 1/24/2011 8:00 PM, Brad Roberts wrote:
> >> > Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. Since this druntime change, the win32 dmd test34.d test has failed:
> >> >
> >> > https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c 512 72c5804de8df27af75099b977
> >> >
> >> > http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441
> >> >
> >> > Before test 3:
> >> > object.Error at src\rt\deh.d(631): Access Violation
> >> >
> >> > I did a test build at the previous commit and the test passes. So that's definitely the one that introduces the problem. It might well be just exposing a dormant problem, but either way, it stopped working at that point.
> >>
> >> Ok.. I finally took some time to dive into this one. It's failing on a catch (DerivedFromError) situation.
> >>
> >> From test34.d:
> >>
> >> class B58
> >> { long x;
> >> void set(long i) { writeln("B58::set(long)"); x = i; }
> >> void set(int i) { writeln("B58::set(int)"); x = i; }
> >> long squareIt() { writeln("B58::squareit()"); return x * x; }
> >> }
> >> class D58 : B58
> >> {
> >> long square;
> >> void set(long i) { writeln("D58::set(long)"); B58.set(i); square =
> >> x * x; } long squareIt() { writeln("D58::squareit()"); return square;
> >> } }
> >>
> >> long foo58(B58 b)
> >> { int i;
> >> try
> >> {
> >> b.set(3);
> >> }
> >> catch (HiddenFuncError o)
> >> {
> >> i = 1;
> >> }
> >> assert(i == 1);
> >> return b.squareIt();
> >> }
> >>
> >> void test58()
> >> {
> >> writeln(foo58(new D58));
> >> }
> >>
> >> Catching Error derived classes is illegal-ish now, right? I haven't narrowed it down to exactly where the segv is coming from (the line number above is the part of the eh mechanism that's constructing the error object in the SEH receiving code, not the point of the segv.. unless that line of code is segving somehow.
> >>
> >> Don, do you have the time to dig a little since you've been all over that part of the code recently?
> >
> > As I understand it, catching Errors is perfectly legal, but because none of the proper cleanup is going to have been done between the point that Error was thrown from and the point that it was caught it, the state of things might be a bit messed up. The unit testing functions that I wrote which are currently under review catch Errors, and they'd be somewhat crippled if they couldn't.
> >
> >
> >
> > Catching Errors is not generally a good idea, but as far as I understand it, it's supposed to be legal.
>
> Yes, that's correct, it's legal to catch errors.
> There is an obvious disastrous problem with that commit: the inclusion
> of __FILE__ and __LINE__ in the constructors for Error and Throwable
> is completely wrong. They should not be in there. That assumes that
> the exceptions are thrown via a throw statement, which is not correct,
> and will cause serious problems. Dunno if that's the only problem,
> though.
> Sorry, don't have any more time right now.
How else would an error be thrown? I was suprised that they weren't there in the first place. Any exception types that I've ever written do that. Theoretically, exceptions are supposed to always give the file and line number that they came from, so it makes perfect sense to me that they'de have default paramaters for it.
If it's really a problem, then it should be changed. But obviously I'm misunderstanding something here, because I would have thought that that would be the desired behavior, and I have no idea how you'd ever get an exception thrown without a throw statement.
- Jonathan M Davis
|
January 26, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jonathan M Davis | On 26 January 2011 10:32, Jonathan M Davis <jmdavisProg at gmx.com> wrote:
> On Wednesday 26 January 2011 01:11:10 Don Clugston wrote:
>> On 26 January 2011 09:33, Jonathan M Davis <jmdavisProg at gmx.com> wrote:
>> > On Wednesday 26 January 2011 00:25:34 Brad Roberts wrote:
>> >> On 1/24/2011 8:00 PM, Brad Roberts wrote:
>> >> > Now that the dust of the svn -> git migration has settled some, there's a failure that's stuck with us. ?Since this druntime change, the win32 dmd test34.d test has failed:
>> >> >
>> >> > https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c 512 72c5804de8df27af75099b977
>> >> >
>> >> > http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441
>> >> >
>> >> > ? Before test 3:
>> >> > ? object.Error at src\rt\deh.d(631): Access Violation
>> >> >
>> >> > I did a test build at the previous commit and the test passes. ?So that's definitely the one that introduces the problem. ?It might well be just exposing a dormant problem, but either way, it stopped working at that point.
>> >>
>> >> Ok.. I finally took some time to dive into this one. ?It's failing on a catch (DerivedFromError) situation.
>> >>
>> >> From test34.d:
>> >>
>> >> class B58
>> >> { ? ?long x;
>> >> ? ? ?void set(long i) { writeln("B58::set(long)"); x = i; }
>> >> ? ? ?void set(int i) ?{ writeln("B58::set(int)"); x = i; }
>> >> ? ? ?long squareIt() ?{ writeln("B58::squareit()"); return x * x; }
>> >> }
>> >> class D58 : B58
>> >> {
>> >> ? ? ?long square;
>> >> ? ? ?void set(long i) { writeln("D58::set(long)"); B58.set(i); square =
>> >> x * x; } long squareIt() ?{ writeln("D58::squareit()"); return square;
>> >> } }
>> >>
>> >> long foo58(B58 b)
>> >> { ? int i;
>> >> ? ? try
>> >> ? ? {
>> >> ? ? ? ? b.set(3);
>> >> ? ? }
>> >> ? ? catch (HiddenFuncError o)
>> >> ? ? {
>> >> ? ? ? ? i = 1;
>> >> ? ? }
>> >> ? ? assert(i == 1);
>> >> ? ? return b.squareIt();
>> >> }
>> >>
>> >> void test58()
>> >> {
>> >> ? ? writeln(foo58(new D58));
>> >> }
>> >>
>> >> Catching Error derived classes is illegal-ish now, right? ?I haven't narrowed it down to exactly where the segv is coming from (the line number above is the part of the eh mechanism that's constructing the error object in the SEH receiving code, not the point of the segv.. unless that line of code is segving somehow.
>> >>
>> >> Don, do you have the time to dig a little since you've been all over that part of the code recently?
>> >
>> > As I understand it, catching Errors is perfectly legal, but because none of the proper cleanup is going to have been done between the point that Error was thrown from and the point that it was caught it, the state of things might be a bit messed up. The unit testing functions that I wrote which are currently under review catch Errors, and they'd be somewhat crippled if they couldn't.
>> >
>> >
>> >
>> > Catching Errors is not generally a good idea, but as far as I understand it, it's supposed to be legal.
>>
>> Yes, that's correct, it's legal to catch errors.
>> There is an obvious disastrous problem with that commit: the inclusion
>> of __FILE__ and __LINE__ in the constructors for Error and Throwable
>> is completely wrong. They should not be in there. That assumes that
>> the exceptions are thrown via a throw statement, which is not correct,
>> and will cause serious problems. Dunno if that's the only problem,
>> though.
>> Sorry, don't have any more time right now.
>
> How else would an error be thrown?
In many cases, it's thrown by hardware. The exception object is
created long after the throw occurred.
There are also 'foreign' exceptions, which are thrown by (say) C++.
(I suspect that Linux DMD can't handle any of those -- but Windows DMD can).
|
January 26, 2011 [D-runtime] win32 regression in exception handling | ||||
---|---|---|---|---|
| ||||
Posted in reply to Don Clugston | On Wednesday 26 January 2011 01:58:50 Don Clugston wrote:
> On 26 January 2011 10:32, Jonathan M Davis <jmdavisProg at gmx.com> wrote:
> > How else would an error be thrown?
>
> In many cases, it's thrown by hardware. The exception object is
> created long after the throw occurred.
> There are also 'foreign' exceptions, which are thrown by (say) C++.
>
> (I suspect that Linux DMD can't handle any of those -- but Windows DMD
> can).
So, what exactly about that makes it a problem that the file and line number have default arguments? If it gives the arguments (like it pretty much has to be doing), then it doesn't use the defaults, so they don't cause any problems there. And is it only file and line number which cause problems if they have defaults, or is it everything (e.g. Throwable next)? And does this to apply to all Throwables?
- Jonathan M Davis
|
Copyright © 1999-2021 by the D Language Foundation