Hey everyone,
a few days ago, I started investigating a report in Bugzilla
containing an application that raises
an exception that goes unhandled, and the stack trace that gets
printed is clearly corrupt.
The report is the following:
http://www.reactos.org/bugzilla/show_bug.cgi?id=5684
At first I thought the context wasn't being recorded properly, since
both EBP and ESP are reported
as being 0 at the time the exception was raised. However, there are
applications for which we show
a correct stack trace in case of an unhandled exception, so this most
likely wasn't the case.
My next thought was that we were somehow corrupting the context, but
this wasn't the case either.
The exception is also being dispatched nicely: exception handlers get
the correct details.
Lastly, the UnhandledExceptionFilter seemed to be passing the context
on to the stack trace printing
function without altering it, so the problem isn't there either. So
where else could something be
happening? That's right, inside one of the exception handler functions.
Some of you may recognize the exception code (0xeedfade) in the bug
report as a Delphi exception.
I did too, so I fired up my copy of Delphi 6 and went digging in its
exception handler functions.
Every occurrence of UnhandledExceptionFilter in its exception handler
functions showed the context
wasn't being passed to it, but instead the registration frame was.
This happens in a very predictive
way however, so creating a patch (read: filthy hack) that detects this
and fixes up the context record
was easy. It is heavily tied to exactly this kind of defect, no general case.
Running the application still triggers the exception of course, but
now we get a reliable stack trace.
A naive search in Bugzilla indicates at least the following reports
are affected by this problem:
Bug 3602, bug 3927, bug 5123 and bug 5684.
I haven't tested them all, but the tell-tale signs are there.
What does one do when encountering such a bug in compiler? Check if
it still exists in the latest
version and file a bug report of course. I got confirmation that the
bug is present in the recently
released Delphi XE with the help of Marco van de Voort of the
FreePascal project (shameless plug, I know).
A bug report has been filed with what I consider an extreme amount of
detail and how to solve it in
their product. Did I mention the report also contains a shameless
plug to the ReactOS project?
Today, to my astonishment, I found another report on their site
which.. you guessed it.. mentions
this problem. This report appears to have been made in 2007, so
guessing when (newly compiled)
Delphi applications will finally start calling
UnhandledExceptionFilter correctly is an exercise
I leave to the reader.
Now to get to the question that some of you may already be asking, and
is the purpose of this mail:
Are we putting this hack into our codebase so we get a decent stack
trace when an application messes up in
the manner this mail is about, or not? Applications can mess this up
in a lot of different ways,
so it won't work on those.
I warmly invite everyone to weigh in on this attempt of a discussion
(which I hope will be productive),
and hope people will see (again) that not everything that goes wrong
in ReactOS is the fault of the OS.
References for those interested in the nasty details:
Bug report: http://qc.embarcadero.com/wc/qcmain.aspx?d=89377
Patch:
Index: dll/win32/kernel32/except/except.c
===================================================================
--- dll/win32/kernel32/except/except.c (revision 49448)
+++ dll/win32/kernel32/except/except.c (working copy)
@@ -218,6 +218,13 @@
PEXCEPTION_RECORD ExceptionRecord = ExceptionInfo->ExceptionRecord;
PCONTEXT ContextRecord = ExceptionInfo->ContextRecord;
+ /* Attempt to hack around bad calls of UnhandledExceptionFilter */
+ if ((ULONG_PTR) (ExceptionRecord->ExceptionAddress) !=
(ULONG_PTR) (ContextRecord->Eip))
+ {
+ DPRINT1("Bogus context detected, using context record HACK!!\n");
+ ContextRecord = (PCONTEXT) ExceptionInfo[1].ExceptionRecord;
+ }
+
/* Print a stack trace. */
DbgPrint("Unhandled exception\n");
DbgPrint("ExceptionCode: %8x\n", ExceptionRecord->ExceptionCode);
@@ -415,11 +422,17 @@
if (dwExceptionCode == 0xeedface)
{
- DPRINT1("Delphi Exception at address: %p\n",
ExceptionRecord.ExceptionInformation[0]);
+ DPRINT1("Delphi 2 Exception at address: %p\n",
ExceptionRecord.ExceptionInformation[0]);
DPRINT1("Exception-Object: %p\n",
ExceptionRecord.ExceptionInformation[1]);
DPRINT1("Exception text: %s\n",
ExceptionRecord.ExceptionInformation[2]);
}
+ if (dwExceptionCode == 0xeedfade)
+ {
+ DPRINT1("Delphi Exception at address: %p\n",
ExceptionRecord.ExceptionInformation[0]);
+ DPRINT1("Exception-Object: %p\n",
ExceptionRecord.ExceptionInformation[1]);
+ }
+
/* Trace the wine special error and show the modulename and functionname */
if (dwExceptionCode == 0x80000100 /*EXCEPTION_WINE_STUB*/)
{
WBR,
Roel Messiant
Hi,
when commiting translation changes in a patch, please modify every language resources file.
Think about our translators :).
Best regards,
P. Schweitzer
Exactly.
The current explorer is self contained and incorrect according to true win32 shell architecture.
Our shell libraries aren’t complete enough to support the new explorer.
Andrew Hill was working on these libraries along with a basic ATL implementation some time ago.
I haven’t heard from him for a while now though.
Ged.
From: Adam Kachwalla [mailto:geekdundee@gmail.com]
Sent: 03 November 2010 08:51
To: Ged Murphy
Subject: Re: [ros-dev] explorer_new
Interesting... so I guess the old explorer.exe implemented that from scratch - I guess the one in shell32 is incomplete then right?
On Wed, 03 Nov 2010 19:50:18 +1100, Ged Murphy <gedmurphy(a)gmail.com> wrote:
It’s implemented in shell32 as an undocumented COM object.
From: Adam Kachwalla [mailto:geekdundee@gmail.com]
Sent: 03 November 2010 08:34
To: Ged Murphy
Subject: Re: [ros-dev] explorer_new
It is part of the shell though isn't it? If start menu isn't part of explorer then what is it part of?
On Wed, 03 Nov 2010 19:32:53 +1100, Ged Murphy <gedmurphy(a)gmail.com> wrote:
That’s because the start menu isn’t part of explorer
From: Adam Kachwalla [mailto:geekdundee@gmail.com]
Sent: 03 November 2010 08:28
To: 'ReactOS Development List'; Ged Murphy
Subject: Re: [ros-dev] explorer_new
Although it seems completely non-functional in the latest trunk builds. Start menu doesn't work, etc.
On Wed, 03 Nov 2010 19:23:29 +1100, Ged Murphy <gedmurphy(a)gmail.com> wrote:
It’s relatively complete, it’s the support libraries which are incomplete.
The only people with then necessary COM and shell skills (which is only really 2 of the ros devs) are either busy with other things or on sabbatical.
Ged.
From: ros-dev-bounces(a)reactos.org [mailto:ros-dev-bounces@reactos.org] On Behalf Of Adam Kachwalla
Sent: 03 November 2010 01:19
To: ReactOS Development List
Subject: [ros-dev] explorer_new
Can anybody tell me what explorer_new is meant to be there for?
I understand people used to be working on it before, but it seems abandoned.
--
Give a man a fish and he will eat for a day.
Teach a man to fish and he will eat for a lifetime.
Give a man religion and he will die praying for a fish.
--
Give a man a fish and he will eat for a day.
Teach a man to fish and he will eat for a lifetime.
Give a man religion and he will die praying for a fish.
--
Give a man a fish and he will eat for a day.
Teach a man to fish and he will eat for a lifetime.
Give a man religion and he will die praying for a fish.
Hi guys,
I'm reading win32k.sys right now. I found KeyboardThreadMain thread
only use KeyboardClass0 device. I want to know how to communicate
KeyboardClass1,KeyboardClass2, and so on?
Fan Zhang
Hi guys,
I'm reading win32k.sys right now. I found KeyboardThreadMain thread only use
KeyboardClass0 device. I want to know how to communicate
KeyboardClass1,KeyboardClass2, and so on?
Fan Zhang
I've just tried to build the cmake branch for the first time and hit the following error :
-- xmllite has no base address
-- win32csr has no base address
-- Configuring done
CMake Error in dll/ntdll/CMakeLists.txt:
Cannot find source file "ntdll.def". Tried extensions .c .C .c++ .cc .cpp
.cxx .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx
Reverting the changes in this revision fixed it.
I know nothing about cmake, but this change appears to have broken the build?
Ged.
-----Original Message-----
From: ros-diffs-bounces(a)reactos.org [mailto:ros-diffs-bounces@reactos.org] On Behalf Of jgardou(a)svn.reactos.org
Sent: 01 November 2010 09:32
To: ros-diffs(a)reactos.org
Subject: [ros-diffs] [jgardou] 49396: [CMAKE] - put ntdll.def in source files
Author: jgardou
Date: Mon Nov 1 09:32:04 2010
New Revision: 49396
URL: http://svn.reactos.org/svn/reactos?rev=49396&view=rev
Log:
[CMAKE]
- put ntdll.def in source files
Modified:
branches/cmake-bringup/dll/ntdll/CMakeLists.txt
Modified: branches/cmake-bringup/dll/ntdll/CMakeLists.txt
URL: http://svn.reactos.org/svn/reactos/branches/cmake-bringup/dll/ntdll/CMakeLi…
==============================================================================
--- branches/cmake-bringup/dll/ntdll/CMakeLists.txt [iso-8859-1] (original)
+++ branches/cmake-bringup/dll/ntdll/CMakeLists.txt [iso-8859-1] Mon Nov 1 09:32:04 2010
@@ -21,10 +21,8 @@
rtl/libsupp.c
rtl/version.c
def/ntdll.rc
- def/ntdll.def)
+ ${CMAKE_CURRENT_BINARY_DIR}/def/ntdll.def)
-set_source_files_properties(def/ntdll.def PROPERTIES EXTERNAL_OBJECT TRUE)
-
if(ARCH MATCHES i386)
list(APPEND SOURCE dispatch/i386/dispatch.S)
elseif(ARCH MATCHES amd64)
@@ -48,7 +46,6 @@
endif()
target_link_libraries(ntdll
- ${CMAKE_CURRENT_BINARY_DIR}/def/ntdll.def
ntdllsys
libcntpr
pseh)
Hello,
The jgardou changes add thousands of whitespace changes which make it extremely hard to understand what is going on. Please to not arbitrarily change whitespace across 18 files in a patch!
Also, there is nothing wrong with the way ClassPNP was defined before. If you think "How on Earth did this work in trunk?", as quoted from the commit log, maybe you should ask others "Hey, how did this work?".
Please revert the changes and come ask how it worked, I'll be happy to help.
-r