[lldb][windows] Fix crash on getting nested exception

[alvinhochun]

Summary

LLDB tries to follow EXCEPTION_RECORD::ExceptionRecord to follow the
nested exception chain. In practice this code just causes Access
Violation whenever there is a nested exception. Since there does not
appear to be any code in LLDB that is actually using the nested
exceptions, this change just removes the crashing code and adds a
comment for future reference.

Fixes https://github.com/mstorsjo/llvm-mingw/issues/292

[DavidSpickett]

Any reason to #if 0 this instead of just removing it and maybe adding a one line comment like "nested exceptions are not supported"? So that someone can git blame that and find this commit with the removal.

If this is not used at all this is academic, but what situation would it be describing if it did? Is it something like if you had an exception in a signal handler, that was handling an initial exception?

[alvinhochun]

Any reason to #if 0 this instead of just removing it and maybe adding a one line comment like "nested exceptions are not supported"? So that someone can git blame that and find this commit with the removal.

I want to leave a clear note explaining what not to do with record.ExceptionRecord. In case someone want to implement handling of nested exceptions in the future, they will be more likely to see it and not repeat the mistake. Though I suppose just having the comment is fine and we don't really need to keep all the code in #if 0.

If this is not used at all this is academic, but what situation would it be describing if it did? Is it something like if you had an exception in a signal handler, that was handling an initial exception?

I am not familiar with the inner workings of exception handling on Windows and there aren't any references I could find, so this is just speculation.

In the sample program from https://github.com/mstorsjo/llvm-mingw/issues/292#issuecomment-1160239522, I am triggering an Access Violation (segfault) inside WindowProc. An Access Violation will produce the exception 0xc0000005, Debuggers will get a first chance exception at that point. (GDB and WinDbg both break execution on this, but LLDB decides to let the debuggee continue for some reason.) After this, the exception is passed to an exception handler if there is one.

Because WindowProc is called by Windows, there seems to have an exception handler internally. For whatever reason it then decides to throw its own exception 0xc000041d (it seems undocumented but does have a message: "An unhandled exception was encountered during a user callback."). I suspect here the nested exception would contain info about the original exception (Access Violation 0xc0000005).

There seems to be an undocumented NtRaiseException@12 function that is used to raise the exception with a EXCEPTION_RECORD directly (which can contain a nested exception). The normal RaiseException API in kernel32 only allows specifying the exception code, flag and arguments.

Update: Actually it seems the documented RtlRaiseException also take an EXCEPTION_RECORD.

[alvinhochun]

Remove old code instead of #if 0.

[DavidSpickett]

LGTM

[labath]

How can one trigger this behavior? Adding a test would be preferable to a long comment (we can have both, but the need for a (long) comment diminishes if you have a test which will break if someone implements it the wrong way.

lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h

 
 // ExceptionRecord
 //
 // ExceptionRecord defines an interface which allows implementors to receive
 // notification of events that happen in a debugged process.
 class ExceptionRecord {
 public:
   ExceptionRecord(const EXCEPTION_RECORD &record, lldb::tid_t thread_id) {
+    // Notes about the `record.ExceptionRecord` field:
+    // In the past, some code tried to parse the nested exception with it, but
+    // in practice, that code just causes Access Violation. I suspect
+    // `ExceptionRecord` here actually points to the address space of the
+    // debuggee process. However, I did not manage to find any official or
+    // unofficial reference that clarifies this point. If anyone would like to
+    // reimplement this, please also keep in mind to check how this behaves when
+    // debugging a WOW64 process. I suspect you may have to use the explicit
+    // `EXCEPTION_RECORD32` and `EXCEPTION_RECORD64` structs.
     m_code = record.ExceptionCode;
     m_continuable = (record.ExceptionFlags == 0);
-    if (record.ExceptionRecord)
-      m_next_exception.reset(
-          new ExceptionRecord(*record.ExceptionRecord, thread_id));
     m_exception_addr = reinterpret_cast<lldb::addr_t>(record.ExceptionAddress);
     m_thread_id = thread_id;
     m_arguments.assign(record.ExceptionInformation,
                        record.ExceptionInformation + record.NumberParameters);
   }
 
   // MINIDUMP_EXCEPTIONs are almost identical to EXCEPTION_RECORDs.
   ExceptionRecord(const MINIDUMP_EXCEPTION &record, lldb::tid_t thread_id)
       : m_code(record.ExceptionCode), m_continuable(record.ExceptionFlags == 0),
-        m_next_exception(nullptr),
         m_exception_addr(static_cast<lldb::addr_t>(record.ExceptionAddress)),
         m_thread_id(thread_id),
         m_arguments(record.ExceptionInformation,
-                    record.ExceptionInformation + record.NumberParameters) {
-    // Set up link to nested exception.
-    if (record.ExceptionRecord) {
-      m_next_exception.reset(new ExceptionRecord(
-          *reinterpret_cast<const MINIDUMP_EXCEPTION *>(record.ExceptionRecord),
-          thread_id));
-    }
-  }
+                    record.ExceptionInformation + record.NumberParameters) {}
 
   virtual ~ExceptionRecord() {}
 
   DWORD
   GetExceptionCode() const { return m_code; }
   bool IsContinuable() const { return m_continuable; }
-  const ExceptionRecord *GetNextException() const {
-    return m_next_exception.get();
-  }
   lldb::addr_t GetExceptionAddress() const { return m_exception_addr; }
 
   lldb::tid_t GetThreadID() const { return m_thread_id; }
 
   const std::vector<ULONG_PTR>& GetExceptionArguments() const { return m_arguments; }
 
 private:
   DWORD m_code;
   bool m_continuable;
-  std::shared_ptr<ExceptionRecord> m_next_exception;
   lldb::addr_t m_exception_addr;
   lldb::tid_t m_thread_id;
   std::vector<ULONG_PTR> m_arguments;
 };
 }
 
 #endif