Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SString ANSI representation. #71186

Merged
merged 8 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 1 addition & 2 deletions src/coreclr/debug/ee/rcthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -548,13 +548,12 @@ static LONG _debugFilter(LPEXCEPTION_POINTERS ep, PVOID pv)
// We can't really use SStrings on the helper thread; though if we're at this point, we've already died.
// So go ahead and risk it and use them anyways.
SString sStack;
StackScratchBuffer buffer;
GetStackTraceAtContext(sStack, ep->ContextRecord);
const CHAR *string = NULL;

EX_TRY
{
string = sStack.GetANSI(buffer);
string = sStack.GetUTF8();
}
EX_CATCH
{
Expand Down
78 changes: 2 additions & 76 deletions src/coreclr/inc/sstring.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ class EMPTY_BASES_DECL SString : private SBuffer
REPRESENTATION_UNICODE = 0x04, // 100
REPRESENTATION_ASCII = 0x01, // 001
REPRESENTATION_UTF8 = 0x03, // 011
REPRESENTATION_ANSI = 0x07, // 111

REPRESENTATION_VARIABLE_MASK = 0x02,
REPRESENTATION_SINGLE_MASK = 0x01,
Expand Down Expand Up @@ -118,8 +117,7 @@ class EMPTY_BASES_DECL SString : private SBuffer
enum tagUTF8Literal { Utf8Literal };
enum tagLiteral { Literal };
enum tagUTF8 { Utf8 };
enum tagANSI { Ansi };
enum tagASCII {Ascii };
enum tagASCII { Ascii };

static void Startup();
static CHECK CheckStartup();
Expand All @@ -141,8 +139,6 @@ class EMPTY_BASES_DECL SString : private SBuffer
SString(enum tagASCII dummyTag, const ASCII *string, COUNT_T count);
SString(enum tagUTF8 dummytag, const UTF8 *string);
SString(enum tagUTF8 dummytag, const UTF8 *string, COUNT_T count);
SString(enum tagANSI dummytag, const ANSI *string);
SString(enum tagANSI dummytag, const ANSI *string, COUNT_T count);
SString(WCHAR character);

// NOTE: Literals MUST be read-only never-freed strings.
Expand All @@ -167,7 +163,6 @@ class EMPTY_BASES_DECL SString : private SBuffer
void Set(const WCHAR *string);
void SetASCII(const ASCII *string);
void SetUTF8(const UTF8 *string);
void SetANSI(const ANSI *string);
void SetAndConvertToUTF8(const WCHAR* string);

// Set this string to a copy of the first count chars of the given string
Expand All @@ -180,7 +175,6 @@ class EMPTY_BASES_DECL SString : private SBuffer
void SetASCII(const ASCII *string, COUNT_T count);

void SetUTF8(const UTF8 *string, COUNT_T count);
void SetANSI(const ANSI *string, COUNT_T count);

// Set this string to the unicode character
void Set(WCHAR character);
Expand Down Expand Up @@ -476,36 +470,12 @@ class EMPTY_BASES_DECL SString : private SBuffer
// Helper function to convert string in-place to lower-case (no allocation overhead for SString instance)
static void LowerCase(__inout_z LPWSTR wszString);

// These routines will use the given scratch string if necessary
// to perform a conversion to the desired representation

// Use a local declaration of InlineScratchBuffer or StackScratchBuffer for parameters of
// AbstractScratchBuffer.
class AbstractScratchBuffer;

// These routines will use the given scratch buffer if necessary
// to perform a conversion to the desired representation. Note that
// the lifetime of the pointer return is limited by BOTH the
// scratch string and the source (this) string.
//
// Typical usage:
//
// SString *s = ...;
// {
// StackScratchBuffer buffer;
// const ANSI *ansi = s->GetANSI(buffer);
// CallFoo(ansi);
// }
// // No more pointers to returned buffer allowed.
const ANSI *GetANSI(AbstractScratchBuffer &scratch) const;

// You can always get a UTF8 string. This will force a conversion
// if necessary.
const UTF8 *GetUTF8() const;

// Converts/copies into the given output string
void ConvertToUnicode(SString &dest) const;
void ConvertToANSI(SString &dest) const;
COUNT_T ConvertToUTF8(SString &dest) const;

//-------------------------------------------------------------------
Expand All @@ -522,7 +492,7 @@ class EMPTY_BASES_DECL SString : private SBuffer

// example usage:
// void GetName(SString & str) {
// char * p = str.OpenANSIBuffer(3);
// char * p = str.OpenUTF8Buffer(3);
// strcpy(p, "Cat");
// str.CloseBuffer();
// }
Expand All @@ -544,7 +514,6 @@ class EMPTY_BASES_DECL SString : private SBuffer
// Open the raw buffer for writing countChars characters (not including the null).
WCHAR *OpenUnicodeBuffer(COUNT_T maxCharCount);
UTF8 *OpenUTF8Buffer(COUNT_T maxSingleCharCount);
ANSI *OpenANSIBuffer(COUNT_T maxSingleCharCount);

//Returns the unicode string, the caller is reponsible for lifetime of the string
WCHAR *GetCopyOfUnicodeString();
Expand Down Expand Up @@ -843,20 +812,6 @@ class EMPTY_BASES_DECL InlineSString : public SString
SetUTF8(string, count);
}

FORCEINLINE InlineSString(enum tagANSI dummytag, const ANSI *string)
: SString(m_inline, SBUFFER_PADDED_SIZE(MEMSIZE))
{
WRAPPER_NO_CONTRACT;
SetANSI(string);
}

FORCEINLINE InlineSString(enum tagANSI dummytag, const ANSI *string, COUNT_T count)
: SString(m_inline, SBUFFER_PADDED_SIZE(MEMSIZE))
{
WRAPPER_NO_CONTRACT;
SetANSI(string, count);
}

FORCEINLINE InlineSString(WCHAR character)
: SString(m_inline, SBUFFER_PADDED_SIZE(MEMSIZE))
{
Expand Down Expand Up @@ -916,35 +871,6 @@ typedef InlineSString<2 * 260> LongPathString;

#define SL(_literal) SString(SString::Literal, _literal)

// ================================================================================
// ScratchBuffer classes are used by the GetXXX() routines to allocate scratch space in.
// ================================================================================

class EMPTY_BASES_DECL SString::AbstractScratchBuffer : private SString
{
protected:
// Do not use this class directly - use
// ScratchBuffer or StackScratchBuffer.
AbstractScratchBuffer(void *buffer, COUNT_T size);
};

template <COUNT_T MEMSIZE>
class EMPTY_BASES_DECL ScratchBuffer : public SString::AbstractScratchBuffer
{
private:
DAC_ALIGNAS(::SString::AbstractScratchBuffer)
BYTE m_inline[MEMSIZE];

public:
ScratchBuffer()
: AbstractScratchBuffer((void *)m_inline, MEMSIZE)
{
WRAPPER_NO_CONTRACT;
}
};

typedef ScratchBuffer<256> StackScratchBuffer;

// ================================================================================
// Special contract definition - THROWS_UNLESS_NORMALIZED
// this is used for operations which might fail for generalized strings but
Expand Down
81 changes: 1 addition & 80 deletions src/coreclr/inc/sstring.inl
Original file line number Diff line number Diff line change
Expand Up @@ -327,41 +327,6 @@ inline SString::SString(tagUTF8 dummytag, const UTF8 *string, COUNT_T count)
SS_RETURN;
}

inline SString::SString(tagANSI dummytag, const ANSI *string)
: SBuffer(Immutable, s_EmptyBuffer, sizeof(s_EmptyBuffer))
{
SS_CONTRACT_VOID
{
SS_CONSTRUCTOR_CHECK;
PRECONDITION(CheckPointer(string, NULL_OK));
THROWS;
GC_NOTRIGGER;
}
SS_CONTRACT_END;

SetANSI(string);

SS_RETURN;
}

inline SString::SString(tagANSI dummytag, const ANSI *string, COUNT_T count)
: SBuffer(Immutable, s_EmptyBuffer, sizeof(s_EmptyBuffer))
{
SS_CONTRACT_VOID
{
SS_CONSTRUCTOR_CHECK;
PRECONDITION(CheckPointer(string, NULL_OK));
PRECONDITION(CheckCount(count));
THROWS;
GC_NOTRIGGER;
}
SS_CONTRACT_END;

SetANSI(string, count);

SS_RETURN;
}

inline SString::SString(WCHAR character)
: SBuffer(Immutable, s_EmptyBuffer, sizeof(s_EmptyBuffer))
{
Expand Down Expand Up @@ -1574,8 +1539,7 @@ inline CHECK SString::CheckRepresentation(int representation)
CHECK(representation == REPRESENTATION_EMPTY
|| representation == REPRESENTATION_UNICODE
|| representation == REPRESENTATION_ASCII
|| representation == REPRESENTATION_UTF8
|| representation == REPRESENTATION_ANSI);
|| representation == REPRESENTATION_UTF8);
CHECK((representation & REPRESENTATION_MASK) == representation);

CHECK_OK;
Expand Down Expand Up @@ -1684,31 +1648,6 @@ inline WCHAR *SString::GetCopyOfUnicodeString()
SS_RETURN buffer.Extract();
}

//----------------------------------------------------------------------------
// Return a writeable buffer that can store 'countChars'+1 ansi characters.
// Call CloseBuffer when done.
//----------------------------------------------------------------------------
inline ANSI *SString::OpenANSIBuffer(COUNT_T countChars)
{
SS_CONTRACT(ANSI*)
{
GC_NOTRIGGER;
PRECONDITION(CheckPointer(this));
PRECONDITION(CheckCount(countChars));
#if _DEBUG
SS_POSTCONDITION(IsBufferOpen());
#endif
SS_POSTCONDITION(GetRawCount() == countChars);
SS_POSTCONDITION(GetRepresentation() == REPRESENTATION_ANSI || countChars == 0);
SS_POSTCONDITION(CheckPointer(RETVAL));
THROWS;
}
SS_CONTRACT_END;

OpenBuffer(REPRESENTATION_ANSI, countChars);
SS_RETURN GetRawANSI();
}

//----------------------------------------------------------------------------
// Return a writeable buffer that can store 'countChars'+1 ansi characters.
// Call CloseBuffer when done.
Expand Down Expand Up @@ -2116,24 +2055,6 @@ inline WCHAR SString::Index::operator[](int index) const
return *(WCHAR*)&GetAt(index);
}

//-----------------------------------------------------------------------------
// Opaque scratch buffer class routines
//-----------------------------------------------------------------------------
inline SString::AbstractScratchBuffer::AbstractScratchBuffer(void *buffer, COUNT_T size)
: SString(buffer, size)
{
SS_CONTRACT_VOID
{
GC_NOTRIGGER;
PRECONDITION(CheckPointer(buffer));
PRECONDITION(CheckCount(size));
NOTHROW;
}
SS_CONTRACT_END;

SS_RETURN;
}

#ifdef _MSC_VER
#pragma warning(pop)
#endif // _MSC_VER
Expand Down
10 changes: 3 additions & 7 deletions src/coreclr/utilcode/check.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,11 @@ void CHECK::Trigger(LPCSTR reason)
STATIC_CONTRACT_GC_NOTRIGGER;

const char *messageString = NULL;
NewHolder<StackScratchBuffer> pScratch(NULL);
NewHolder<StackSString> pMessage(NULL);

EX_TRY
{
FAULT_NOT_FATAL();

pScratch = new StackScratchBuffer();
pMessage = new StackSString();

pMessage->AppendASCII(reason);
Expand All @@ -127,7 +124,7 @@ void CHECK::Trigger(LPCSTR reason)
pMessage->AppendASCII(m_condition);
#endif

messageString = pMessage->GetANSI(*pScratch);
messageString = pMessage->GetUTF8();
Copy link
Member

Choose a reason for hiding this comment

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

If the message string is Utf8 now, we should use OutputDebugStringUtf8 below to print it.

Copy link
Member

Choose a reason for hiding this comment

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

Also, fix DbgAssertDialog to expect Utf8. It ends up hitting OutputDebugStringA(szExpr) that has similar problem.

}
EX_CATCH
{
Expand All @@ -138,7 +135,7 @@ void CHECK::Trigger(LPCSTR reason)
#if _DEBUG
DbgAssertDialog((char*)m_file, m_line, (char *)messageString);
#else
OutputDebugStringA(messageString);
OutputDebugStringUtf8(messageString);
DebugBreak();
#endif

Expand Down Expand Up @@ -260,8 +257,7 @@ LPCSTR CHECK::AllocateDynamicMessage(const SString &s)
STATIC_CONTRACT_GC_NOTRIGGER;

// Make a copy of it.
StackScratchBuffer buffer;
const char * pMsg = s.GetANSI(buffer);
const char * pMsg = s.GetUTF8();

// Must copy that into our own field.
size_t len = strlen(pMsg) + 1;
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/utilcode/debug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ static const char * szLowMemoryAssertMessage = "Assert failure (unable to format
bool _DbgBreakCheck(
LPCSTR szFile,
int iLine,
LPCSTR szExpr,
LPCUTF8 szExpr,
BOOL fConstrained)
{
STATIC_CONTRACT_THROWS;
Expand Down Expand Up @@ -387,7 +387,7 @@ bool _DbgBreakCheck(
OutputDebugStringA("\n");
OutputDebugStringA(szFile);
OutputDebugStringA("\n");
OutputDebugStringA(szExpr);
OutputDebugStringUtf8(szExpr);
OutputDebugStringA("\n");
printf(szLowMemoryAssertMessage);
printf("\n");
Expand Down Expand Up @@ -624,7 +624,7 @@ bool GetStackTraceAtContext(SString & s, CONTEXT * pContext)
// If we have a supplied context, then don't skip any frames. Else we'll
// be using the current context, so skip this frame.
const int cSkip = (pContext == NULL) ? 1 : 0;
char * szString = s.OpenANSIBuffer(cchMaxAssertStackLevelStringLen * cTotal);
char * szString = s.OpenUTF8Buffer(cchMaxAssertStackLevelStringLen * cTotal);
GetStringFromStackLevels(cSkip, cTotal, szString, pContext);
s.CloseBuffer((COUNT_T) strlen(szString));

Expand Down
6 changes: 2 additions & 4 deletions src/coreclr/utilcode/loaderheap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1617,8 +1617,7 @@ void UnlockedLoaderHeap::UnlockedBackoutMem(void *pMem,
LoaderHeapSniffer::PitchSniffer(&message);
}

StackScratchBuffer scratch;
DbgAssertDialog(szFile, lineNum, (char*) message.GetANSI(scratch));
DbgAssertDialog(szFile, lineNum, (char*) message.GetUTF8());

}
}
Expand Down Expand Up @@ -2231,8 +2230,7 @@ void LoaderHeapSniffer::ValidateFreeList(UnlockedLoaderHeap *pHeap)

}

StackScratchBuffer scratch;
DbgAssertDialog(__FILE__, __LINE__, (char*) message.GetANSI(scratch));
DbgAssertDialog(__FILE__, __LINE__, (char*) message.GetUTF8());

}

Expand Down
Loading