Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Clarify inner exception and aggregate exception ToString() #27333

Closed
danmoseley wants to merge 9 commits into from
Closed

Clarify inner exception and aggregate exception ToString() #27333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aae660d
Add path to Bad IL exception message
danmoseley Oct 20, 2019
7b57069
Add path to not-absolute-path exception
danmoseley Oct 20, 2019
54336a7
Remove empty lines caused by empty fusion log string
danmoseley Oct 21, 2019
1f7c0e9
Remove 'end stack trace from previous location'
danmoseley Oct 21, 2019
2dc55bd
Add arrows and indenting
danmoseley Oct 21, 2019
ba389ee
Change arrows to ASCII box
danmoseley Oct 21, 2019
c37aab5
Neaten
danmoseley Oct 21, 2019
8392980
Fix chk build
danmoseley Oct 21, 2019
7a6b228
Better
danmoseley Oct 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions src/System.Private.CoreLib/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,6 @@
<data name="AggregateException_DeserializationFailure" xml:space="preserve">
<value>The serialization stream contains no inner exceptions.</value>
</data>
<data name="AggregateException_InnerException" xml:space="preserve">
<value>(Inner Exception #{0}) </value>
<comment>This text is prepended to each inner exception description during aggregate exception formatting</comment>
</data>
<data name="AppDomain_AppBaseNotSet" xml:space="preserve">
<value>The ApplicationBase must be set before retrieving this property.</value>
</data>
Expand Down Expand Up @@ -812,7 +808,7 @@
<value>The value "{0}" is not of type "{1}" and cannot be used in this generic collection.</value>
</data>
<data name="Argument_AbsolutePathRequired" xml:space="preserve">
<value>Absolute path information is required.</value>
<value>Path "{0}" is not an absolute path.</value>
</data>
<data name="Argument_AddingDuplicate" xml:space="preserve">
<value>An item with the same key has already been added.</value>
Expand Down Expand Up @@ -2224,12 +2220,6 @@
<data name="EventSource_VarArgsParameterMismatch" xml:space="preserve">
<value>Event {0} was called with a different type as defined (argument "{1}"). This may cause the event to be displayed incorrectly.</value>
</data>
<data name="Exception_EndOfInnerExceptionStack" xml:space="preserve">
<value>--- End of inner exception stack trace ---</value>
</data>
<data name="Exception_EndStackTraceFromPreviousThrow" xml:space="preserve">
<value>--- End of stack trace from previous location ---</value>
</data>
<data name="Exception_WasThrown" xml:space="preserve">
<value>Exception of type '{0}' was thrown.</value>
</data>
Expand Down
10 changes: 3 additions & 7 deletions src/System.Private.CoreLib/shared/System/AggregateException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,16 +438,12 @@ public override string ToString()
StringBuilder text = new StringBuilder();
text.Append(base.ToString());

for (int i = 0; i < m_innerExceptions.Count; i++)
foreach (Exception ex in m_innerExceptions)
{
if (m_innerExceptions[i] == InnerException)
if (object.ReferenceEquals(ex, InnerException))
Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is == inappropriate here?

Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to filter out the exact same object - figured it was clearer. Also a subclass could have overridden the equals operator.

Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a subclass could have overridden the equals operator.

That's actually not how == works. Unless the strongly-typed variable being used defines an operator == (and Exception doesn't), this will be emitted with the exact same behavior as ReferenceEquals. It doesn't use Equals. e.g. this will output false:

using System;

class Program
{
    static void Main() => Console.WriteLine(new Program() == new Program());

    public override bool Equals(object obj) => true;
}

Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had forgotten this. In the case where this is known to be true (as here), do we prefer to use == for brevity, or O.RE to be explicit about intent? I assume the former as we don't seem to use O.RE that much.

Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It often ends up being personal preference.

Copy link
Copy Markdown
Member

@jkotas jkotas Oct 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roslyn compiles ReferenceEquals into the same IL as what you get from ==, so it is just a style difference. Similar to == null vs. is null.

continue; // Already logged in base.ToString()

text.Append(Environment.NewLineConst + InnerExceptionPrefix);
text.AppendFormat(CultureInfo.InvariantCulture, SR.AggregateException_InnerException, i);
text.Append(m_innerExceptions[i].ToString());
text.Append("<---");
text.AppendLine();
text.Append(InnerExceptionToString(ex));
Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper ends up allocating it's own string builder, formatting to it, getting its string, and then appending that string to this builder. Why not just pass this builder in? If the concern is reusing it in places that don't use a builder, you can make the helper create the builder only if one want passed in, and return a its ToString only if one was.

Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

}

return text.ToString();
Expand Down
5 changes: 2 additions & 3 deletions src/System.Private.CoreLib/shared/System/BadImageFormatException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,10 @@ public override string ToString()
{
string s = GetType().ToString() + ": " + Message;

if (!string.IsNullOrEmpty(_fileName))
if (!string.IsNullOrEmpty(_fileName) && !Message.Contains(_fileName))
Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this all that meaningful? While we discourage parsing, the current format is much more parsable for the filename.

Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the filename is set and not just in the message. Ok, guess I need feedback on this one.

Apart from reducing noise, another possible advantage of reducing line count is that the parts of the output are closer together in a log produced from concurrent Interleaving outputs eg a build log or test run.

s += Environment.NewLineConst + SR.Format(SR.IO_FileName_Name, _fileName);

if (InnerException != null)
s += InnerExceptionPrefix + InnerException.ToString();
s += InnerExceptionToString(InnerException);

if (StackTrace != null)
s += Environment.NewLineConst + StackTrace;
Expand Down
7 changes: 0 additions & 7 deletions src/System.Private.CoreLib/shared/System/Diagnostics/StackTrace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,13 +321,6 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb)
sb.AppendFormat(CultureInfo.InvariantCulture, inFileLineNum, fileName, sf.GetFileLineNumber());
}
}

// Skip EDI boundary for async
if (sf.IsLastFrameFromForeignExceptionStackTrace && !isAsync)
{
sb.AppendLine();
sb.Append(SR.Exception_EndStackTraceFromPreviousThrow);
}
Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this being removed?

Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was that the boundary could be inferred - this was in the previous PR I think.

Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that's easy to do in many/all cases. We explicitly skip this for async because that case generally is easier, but for other cases, you're generally not nearly as linear, and you end up with stack traces that look impossible, which is much more confusing and time consuming to analyze than when they're clearly demarcated.

}
}

Expand Down
36 changes: 30 additions & 6 deletions src/System.Private.CoreLib/shared/System/Exception.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@
using System.Collections;
using System.Diagnostics;
using System.Runtime.Serialization;
using System.Text;

namespace System
{
[Serializable]
[System.Runtime.CompilerServices.TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
public partial class Exception : ISerializable
{
private protected const string InnerExceptionPrefix = " ---> ";

public Exception()
{
_HResult = HResults.COR_E_EXCEPTION;
Expand Down Expand Up @@ -128,10 +127,7 @@ public override string ToString()
s += ": " + message;
}

if (_innerException != null)
{
s += Environment.NewLineConst + InnerExceptionPrefix + _innerException.ToString() + Environment.NewLineConst + " " + SR.Exception_EndOfInnerExceptionStack;
}
s += InnerExceptionToString(InnerException);

string? stackTrace = StackTrace;
if (stackTrace != null)
Expand All @@ -142,6 +138,34 @@ public override string ToString()
return s;
}

// Returns the indented inner exception's ToString() prefixed, but not suffixed, with a newline.
// If there is no inner exception, returns empty string.
private protected string InnerExceptionToString(Exception? inner)
{
if (inner == null)
return "";

/* Format something like this to clearly separate from the containing exception
________________________________________________________________________________________________
Copy link
Copy Markdown
Member

@jkotas jkotas Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the ascii art part of the change has significant risk of breaking tools that analyze exception stacktraces, and it may be controversial (e.g. my personal preference would be a more compact format without the ascii art, but I am happy to be overruled.)

If you would like to pursue it, I think it should have its own issue and PR. The issue should be marked as breaking change and the stakeholders should be included to comment on it:

  • The community projects that are in the business of formatting and understanding exception stacktraces (e.g. Ben.Demystifier)
  • Our diagnostic and TI teams

Copy link
Copy Markdown
Member

@jkotas jkotas Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

breaking tools that analyze exception stacktraces

e.g. some of our own corefx tests are failing because of this problem

Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really just wanted to start a discussion. Any suggestions for an alternative? Maybe the arrows? (Is there general agreement the existing format is hard to correlate?)

Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll close this and open an issue to discuss that...

|_1.Interop+Crypto+OpenSslCryptographicException: error:25070067:DSO support routines:DSO_load:could not load the shared library
| at _1.Program.<>c.<Main>b__0_0() in C:\proj\30\ConsoleApp1\Program.cs:line 31
| at _1.Program.Wrap(Action cb) in C:\proj\30\ConsoleApp1\Program.cs:line 61
\________________________________________________________________________________________________

*/
const string nl = Environment.NewLineConst;
string hundredUnderscores = new string('_', 100);

var sb = new StringBuilder();
sb.Append(nl + " ").Append(hundredUnderscores).Append(nl);
sb.Append(inner.ToString().Trim()); // Remove any trailing newline
sb.Replace("\r\n", "\n", nl.Length, sb.Length - nl.Length); // Normalize newlines except the first
sb.Replace("\n", nl + " |", nl.Length, sb.Length - nl.Length); // Indent each line, after the first
sb.Append(nl + " \\").Append(hundredUnderscores);

return sb.ToString();
}

protected event EventHandler<SafeSerializationEventArgs>? SerializeObjectState
{
add { throw new PlatformNotSupportedException(SR.PlatformNotSupported_SecureBinarySerialization); }
Expand Down
5 changes: 2 additions & 3 deletions src/System.Private.CoreLib/shared/System/IO/FileLoadException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,10 @@ public override string ToString()
{
string s = GetType().ToString() + ": " + Message;

if (!string.IsNullOrEmpty(FileName))
if (!string.IsNullOrEmpty(FileName) && !Message.Contains(FileName))
s += Environment.NewLineConst + SR.Format(SR.IO_FileName_Name, FileName);

if (InnerException != null)
s += Environment.NewLineConst + InnerExceptionPrefix + InnerException.ToString();
s += InnerExceptionToString(InnerException);

if (StackTrace != null)
s += Environment.NewLineConst + StackTrace;
Expand Down
5 changes: 2 additions & 3 deletions src/System.Private.CoreLib/shared/System/IO/FileNotFoundException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,10 @@ public override string ToString()
{
string s = GetType().ToString() + ": " + Message;

if (!string.IsNullOrEmpty(FileName))
if (!string.IsNullOrEmpty(FileName) && !Message.Contains(FileName))
s += Environment.NewLineConst + SR.Format(SR.IO_FileName_Name, FileName);

if (InnerException != null)
s += Environment.NewLineConst + InnerExceptionPrefix + InnerException.ToString();
s += InnerExceptionToString(InnerException);

if (StackTrace != null)
s += Environment.NewLineConst + StackTrace;
Expand Down
2 changes: 1 addition & 1 deletion src/System.Private.CoreLib/shared/System/Reflection/Assembly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ public static Assembly LoadFile(string path)

if (PathInternal.IsPartiallyQualified(path))
{
throw new ArgumentException(SR.Argument_AbsolutePathRequired, nameof(path));
throw new ArgumentException(SR.Format(SR.Argument_AbsolutePathRequired, path), nameof(path));
}

string normalizedPath = Path.GetFullPath(path);
Expand Down
6 changes: 1 addition & 5 deletions src/System.Private.CoreLib/shared/System/Runtime/InteropServices/COMException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ public override string ToString()
s.Append(": ").Append(message);
}

Exception? innerException = InnerException;
if (innerException != null)
{
s.Append(Environment.NewLineConst + InnerExceptionPrefix).Append(innerException.ToString());
}
s.Append(InnerExceptionToString(InnerException));

string? stackTrace = StackTrace;
if (stackTrace != null)
Expand Down
6 changes: 1 addition & 5 deletions src/System.Private.CoreLib/shared/System/Runtime/InteropServices/ExternalException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ public override string ToString()
s += ": " + message;
}

Exception? innerException = InnerException;
if (innerException != null)
{
s += Environment.NewLineConst + InnerExceptionPrefix + innerException.ToString();
}
s += InnerExceptionToString(InnerException);

if (StackTrace != null)
s += Environment.NewLineConst + StackTrace;
Expand Down
8 changes: 4 additions & 4 deletions src/System.Private.CoreLib/shared/System/Runtime/Loader/AssemblyLoadContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ public Assembly LoadFromAssemblyPath(string assemblyPath)

if (PathInternal.IsPartiallyQualified(assemblyPath))
{
throw new ArgumentException(SR.Argument_AbsolutePathRequired, nameof(assemblyPath));
throw new ArgumentException(SR.Format(SR.Argument_AbsolutePathRequired, assemblyPath), nameof(assemblyPath));
}

lock (_unloadLock)
Expand All @@ -319,12 +319,12 @@ public Assembly LoadFromNativeImagePath(string nativeImagePath, string? assembly

if (PathInternal.IsPartiallyQualified(nativeImagePath))
{
throw new ArgumentException(SR.Argument_AbsolutePathRequired, nameof(nativeImagePath));
throw new ArgumentException(SR.Format(SR.Argument_AbsolutePathRequired, nativeImagePath), nameof(nativeImagePath));
}

if (assemblyPath != null && PathInternal.IsPartiallyQualified(assemblyPath))
{
throw new ArgumentException(SR.Argument_AbsolutePathRequired, nameof(assemblyPath));
throw new ArgumentException(SR.Format(SR.Argument_AbsolutePathRequired, assemblyPath), nameof(assemblyPath));
}

lock (_unloadLock)
Expand Down Expand Up @@ -394,7 +394,7 @@ protected IntPtr LoadUnmanagedDllFromPath(string unmanagedDllPath)

if (PathInternal.IsPartiallyQualified(unmanagedDllPath))
{
throw new ArgumentException(SR.Argument_AbsolutePathRequired, nameof(unmanagedDllPath));
throw new ArgumentException(SR.Format(SR.Argument_AbsolutePathRequired, unmanagedDllPath), nameof(unmanagedDllPath));
}

return NativeLibrary.Load(unmanagedDllPath);
Expand Down
1 change: 0 additions & 1 deletion src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,6 @@ internal void SetCurrentStackTrace()
// when it's retrieved.
var sb = new StringBuilder(256);
new StackTrace(fNeedFileInfo: true).ToString(System.Diagnostics.StackTrace.TraceFormat.TrailingNewLine, sb);
sb.AppendLine(SR.Exception_EndStackTraceFromPreviousThrow);
Copy link
Copy Markdown
Member Author

@danmoseley danmoseley Oct 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this line (and thus the string) because elsewhere where remote stack trace and stack trace are combined, they are concatenated without a separator. Eg.,

coreclr/src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs

Line 43 in c37aab5

_remoteStackTraceString += _stackTraceString;

coreclr/src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs

Line 171 in c37aab5

return remoteStackTraceString + stackTraceString;

coreclr/src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs

Line 178 in c37aab5

return remoteStackTraceString + GetStackTrace(this);

Copy link
Copy Markdown
Member

@stephentoub stephentoub Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put it back. Have you looked at what the stack traces look like without it?

_remoteStackTraceString = sb.ToString();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/vm/appdomain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3801,7 +3801,7 @@ DomainAssembly* AppDomain::LoadDomainAssembly(AssemblySpec* pSpec,
{
StackSString name;
pSpec->GetFileOrDisplayName(0, name);
pEx=new EEFileLoadException(name, pEx->GetHR(), NULL, pEx);
pEx=new EEFileLoadException(name, pEx->GetHR(), pEx);
AddExceptionToCache(pSpec, pEx);
PAL_CPP_THROW(Exception *, pEx);
}
Expand Down
8 changes: 4 additions & 4 deletions src/vm/assemblynative.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,13 @@ void QCALLTYPE AssemblyNative::LoadFromPath(INT_PTR ptrNativeAssemblyLoadContext

// Need to verify that this is a valid CLR assembly.
if (!pILImage->CheckILFormat())
ThrowHR(COR_E_BADIMAGEFORMAT, BFA_BAD_IL);
THROW_BAD_FORMAT(BFA_BAD_IL, pILImage.GetValue());

LoaderAllocator* pLoaderAllocator = NULL;
if (SUCCEEDED(pBinderContext->GetLoaderAllocator((LPVOID*)&pLoaderAllocator)) && pLoaderAllocator->IsCollectible() && !pILImage->IsILOnly())
{
// Loading IJW assemblies into a collectible AssemblyLoadContext is not allowed
ThrowHR(COR_E_BADIMAGEFORMAT, BFA_IJW_IN_COLLECTIBLE_ALC);
THROW_BAD_FORMAT(BFA_IJW_IN_COLLECTIBLE_ALC, pILImage.GetValue());
}
}

Expand All @@ -270,15 +270,15 @@ void QCALLTYPE AssemblyNative::LoadFromPath(INT_PTR ptrNativeAssemblyLoadContext
{
// ReadyToRun images are treated as IL images by the rest of the system
if (!pNIImage->CheckILFormat())
ThrowHR(COR_E_BADIMAGEFORMAT);
THROW_BAD_FORMAT(COR_E_BADIMAGEFORMAT, pNIImage.GetValue());

pILImage = pNIImage.Extract();
pNIImage = NULL;
}
else
{
if (!pNIImage->CheckNativeFormat())
ThrowHR(COR_E_BADIMAGEFORMAT);
THROW_BAD_FORMAT(COR_E_BADIMAGEFORMAT, pNIImage.GetValue());
}
}
#endif // FEATURE_PREJIT
Expand Down
9 changes: 2 additions & 7 deletions src/vm/clrex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1675,10 +1675,9 @@ OBJECTREF EETypeLoadException::CreateThrowable()
// EEFileLoadException is an EE exception subclass representing a file loading
// error
// ---------------------------------------------------------------------------
EEFileLoadException::EEFileLoadException(const SString &name, HRESULT hr, void *pFusionLog, Exception *pInnerException/* = NULL*/)
EEFileLoadException::EEFileLoadException(const SString &name, HRESULT hr, Exception *pInnerException/* = NULL*/)
: EEException(GetFileLoadKind(hr)),
m_name(name),
m_pFusionLog(pFusionLog),
m_hr(hr)
{
CONTRACTL
Expand Down Expand Up @@ -1822,18 +1821,14 @@ OBJECTREF EEFileLoadException::CreateThrowable()
}
CONTRACTL_END;

// Fetch any log info from the fusion log
SString logText;
struct _gc {
OBJECTREF pNewException;
STRINGREF pNewFileString;
STRINGREF pFusLogString;
} gc;
ZeroMemory(&gc, sizeof(gc));
GCPROTECT_BEGIN(gc);

gc.pNewFileString = StringObject::NewString(m_name);
gc.pFusLogString = StringObject::NewString(logText);
gc.pNewException = AllocateObject(MscorlibBinder::GetException(m_kind));

MethodDesc* pMD = MemberLoader::FindMethod(gc.pNewException->GetMethodTable(),
Expand All @@ -1850,7 +1845,7 @@ OBJECTREF EEFileLoadException::CreateThrowable()
ARG_SLOT args[] = {
ObjToArgSlot(gc.pNewException),
ObjToArgSlot(gc.pNewFileString),
ObjToArgSlot(gc.pFusLogString),
NULL,
(ARG_SLOT) m_hr
};

Expand Down
8 changes: 3 additions & 5 deletions src/vm/clrex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,13 +668,11 @@ class EEFileLoadException : public EEException

private:
SString m_name;
void *m_pFusionLog;
HRESULT m_hr;


public:

EEFileLoadException(const SString &name, HRESULT hr, void *pFusionLog = NULL, Exception *pInnerException = NULL);
EEFileLoadException(const SString &name, HRESULT hr, Exception *pInnerException = NULL);
~EEFileLoadException();

// virtual overrides
Expand All @@ -698,12 +696,12 @@ class EEFileLoadException : public EEException
virtual Exception *CloneHelper()
{
WRAPPER_NO_CONTRACT;
return new EEFileLoadException(m_name, m_hr, m_pFusionLog);
return new EEFileLoadException(m_name, m_hr);
}

private:
#ifdef _DEBUG
EEFileLoadException() : m_pFusionLog(NULL)
EEFileLoadException()
{
// Used only for DebugIsEECxxExceptionPointer to get the vtable pointer.
// We need a variant which does not allocate memory.
Expand Down
21 changes: 13 additions & 8 deletions src/vm/excep.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12380,20 +12380,25 @@ VOID ThrowBadFormatWorker(UINT resID, LPCWSTR imageName DEBUGARG(__in_z const ch
#ifndef DACCESS_COMPILE
SString msgStr;

if ((imageName != NULL) && (imageName[0] != 0))
{
msgStr += W("[");
msgStr += imageName;
msgStr += W("] ");
}

SString resStr;
if (resID == 0 || !resStr.LoadResource(CCompRC::Optional, resID))
{
resStr.LoadResource(CCompRC::Error, MSG_FOR_URT_HR(COR_E_BADIMAGEFORMAT));
resStr.LoadResource(CCompRC::Error, MSG_FOR_URT_HR(BFA_BAD_IL)); // "Bad IL format."
}
msgStr += resStr;

if ((imageName != NULL) && (imageName[0] != 0))
{
SString suffixResStr;
if (suffixResStr.LoadResource(CCompRC::Optional, COR_E_BADIMAGEFORMAT)) // "The format of the file '%1' is invalid."
{
SString suffixMsgStr;
suffixMsgStr.FormatMessage(FORMAT_MESSAGE_FROM_STRING, (LPCWSTR)suffixResStr, 0, 0, imageName);
msgStr.AppendASCII(" ");
msgStr += suffixMsgStr;
}
}

#ifdef _DEBUG
if (0 != strcmp(cond, "FALSE"))
{
Expand Down