Skip to content

Commit

Permalink
[MERGE #4641 @MSLaguana] Correcting memory allocation in LiteralStrin…
Browse files Browse the repository at this point in the history 
…gWithPropertyStringPtr::NewFromCString

Merge pull request #4641 from MSLaguana:fixJsrtStringCreation

A confusion between bytes and character count meant that JsCreateString
was creating strings that used twice as much space as they needed.

While I was at it, I also tried to clean up the types in `Utf8Helper.h` to make it more clear what the values referred to.
  • Loading branch information
@MSLaguana
MSLaguana committed Feb 6, 2018
2 parents 86e6e5c + 1492900 commit e176a29
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 28 deletions.
36 changes: 18 additions & 18 deletions lib/Common/Codex/Utf8Helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ namespace utf8
}

inline HRESULT NarrowStringToWideNoAlloc(_In_ LPCSTR sourceString, size_t sourceCount,
__out_ecount(destBufferCount) LPWSTR destString, size_t destBufferCount, _Out_ size_t* destCount)
__out_ecount(destBufferCount) LPWSTR destString, size_t destBufferCount, _Out_ charcount_t* destCount)
{
size_t sourceStart = 0;
size_t cbSourceString = sourceCount;
Expand Down Expand Up @@ -123,7 +123,7 @@ namespace utf8

if (sourceStart == sourceCount)
{
*destCount = sourceCount;
*destCount = static_cast<charcount_t>(sourceCount);
destString[sourceCount] = WCHAR(0);
}
else
Expand Down Expand Up @@ -160,7 +160,7 @@ namespace utf8
///
template <typename AllocatorFunction>
HRESULT NarrowStringToWide(_In_ AllocatorFunction allocator,_In_ LPCSTR sourceString,
size_t sourceCount, _Out_ LPWSTR* destStringPtr, _Out_ size_t* destCount, size_t* allocateCount = nullptr)
size_t sourceCount, _Out_ LPWSTR* destStringPtr, _Out_ charcount_t* destCount, size_t* allocateCount = nullptr)
{
size_t cbDestString = (sourceCount + 1) * sizeof(WCHAR);
if (cbDestString < sourceCount) // overflow ?
Expand All @@ -184,7 +184,7 @@ namespace utf8
}

template <class Allocator>
HRESULT NarrowStringToWide(_In_ LPCSTR sourceString, size_t sourceCount, _Out_ LPWSTR* destStringPtr, _Out_ size_t* destCount, size_t* allocateCount = nullptr)
HRESULT NarrowStringToWide(_In_ LPCSTR sourceString, size_t sourceCount, _Out_ LPWSTR* destStringPtr, _Out_ charcount_t* destCount, size_t* allocateCount = nullptr)
{
return NarrowStringToWide(Allocator::allocate, sourceString, sourceCount, destStringPtr, destCount, allocateCount);
}
Expand All @@ -205,30 +205,30 @@ namespace utf8

inline HRESULT NarrowStringToWideDynamic(_In_ LPCSTR sourceString, _Out_ LPWSTR* destStringPtr)
{
size_t unused;
charcount_t unused;
return NarrowStringToWide<malloc_allocator>(
sourceString, strlen(sourceString), destStringPtr, &unused);
}

inline HRESULT NarrowStringToWideDynamicGetLength(_In_ LPCSTR sourceString, _Out_ LPWSTR* destStringPtr, _Out_ size_t* destLength)
inline HRESULT NarrowStringToWideDynamicGetLength(_In_ LPCSTR sourceString, _Out_ LPWSTR* destStringPtr, _Out_ charcount_t* destLength)
{
return NarrowStringToWide<malloc_allocator>(
sourceString, strlen(sourceString), destStringPtr, destLength);
}

template <class Allocator, class SrcType, class DstType>
template <class Allocator, class SrcType, class DstType, class CountType>
class NarrowWideStringConverter
{
public:
static size_t Length(const SrcType& src);
static HRESULT Convert(
SrcType src, size_t srcCount, DstType* dst, size_t* dstCount, size_t* allocateCount = nullptr);
SrcType src, size_t srcCount, DstType* dst, CountType* dstCount, size_t* allocateCount = nullptr);
static HRESULT ConvertNoAlloc(
SrcType src, size_t srcCount, DstType dst, size_t dstCount, size_t* written);
SrcType src, size_t srcCount, DstType dst, CountType dstCount, CountType* written);
};

template <class Allocator>
class NarrowWideStringConverter<Allocator, LPCSTR, LPWSTR>
class NarrowWideStringConverter<Allocator, LPCSTR, LPWSTR, charcount_t>
{
public:
// Note: Typically caller should pass in Utf8 string length. Following
Expand All @@ -240,23 +240,23 @@ namespace utf8

static HRESULT Convert(
LPCSTR sourceString, size_t sourceCount,
LPWSTR* destStringPtr, size_t* destCount, size_t* allocateCount = nullptr)
LPWSTR* destStringPtr, charcount_t * destCount, size_t* allocateCount = nullptr)
{
return NarrowStringToWide<Allocator>(
sourceString, sourceCount, destStringPtr, destCount, allocateCount);
}

static HRESULT ConvertNoAlloc(
LPCSTR sourceString, size_t sourceCount,
LPWSTR destStringPtr, size_t destCount, size_t* written)
LPWSTR destStringPtr, charcount_t destCount, charcount_t* written)
{
return NarrowStringToWideNoAlloc(
sourceString, sourceCount, destStringPtr, destCount, written);
}
};

template <class Allocator>
class NarrowWideStringConverter<Allocator, LPCWSTR, LPSTR>
class NarrowWideStringConverter<Allocator, LPCWSTR, LPSTR, size_t>
{
public:
// Note: Typically caller should pass in WCHAR string length. Following
Expand All @@ -283,14 +283,14 @@ namespace utf8
}
};

template <class Allocator, class SrcType, class DstType>
template <class Allocator, class SrcType, class DstType, class CountType>
class NarrowWideConverter
{
typedef NarrowWideStringConverter<Allocator, SrcType, DstType>
typedef NarrowWideStringConverter<Allocator, SrcType, DstType, CountType>
StringConverter;
private:
DstType dst;
size_t dstCount;
CountType dstCount;
size_t allocateCount;
bool freeDst;

Expand Down Expand Up @@ -347,6 +347,6 @@ namespace utf8
}
};

typedef NarrowWideConverter<malloc_allocator, LPCSTR, LPWSTR> NarrowToWide;
typedef NarrowWideConverter<malloc_allocator, LPCWSTR, LPSTR> WideToNarrow;
typedef NarrowWideConverter<malloc_allocator, LPCSTR, LPWSTR, charcount_t> NarrowToWide;
typedef NarrowWideConverter<malloc_allocator, LPCWSTR, LPSTR, size_t> WideToNarrow;
}
2 changes: 1 addition & 1 deletion lib/Jsrt/Jsrt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4662,7 +4662,7 @@ CHAKRA_API JsCreateString(
length = strlen(content);
}

if (length > static_cast<CharCount>(-1))
if (length > MaxCharCount)
{
return JsErrorOutOfMemory;
}
Expand Down
10 changes: 5 additions & 5 deletions lib/Runtime/Library/ConcatString.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,24 +99,24 @@ namespace Js
}

ScriptContext * scriptContext = library->GetScriptContext();
size_t cbDestString = (charCount + 1) * sizeof(WCHAR);
if ((CharCount)cbDestString < charCount) // overflow
if (charCount > MaxCharCount)
{
Js::JavascriptError::ThrowOutOfMemoryError(scriptContext);
}

Recycler * recycler = library->GetRecycler();
char16* destString = RecyclerNewArrayLeaf(recycler, WCHAR, cbDestString);
char16* destString = RecyclerNewArrayLeaf(recycler, WCHAR, charCount + 1);
if (destString == nullptr)
{
Js::JavascriptError::ThrowOutOfMemoryError(scriptContext);
}

HRESULT result = utf8::NarrowStringToWideNoAlloc(cString, charCount, destString, charCount + 1, &cbDestString);
charcount_t cchDestString = 0;
HRESULT result = utf8::NarrowStringToWideNoAlloc(cString, charCount, destString, charCount + 1, &cchDestString);

if (result == S_OK)
{
return (JavascriptString*) RecyclerNew(library->GetRecycler(), LiteralStringWithPropertyStringPtr, destString, (CharCount)cbDestString, library);
return (JavascriptString*) RecyclerNew(library->GetRecycler(), LiteralStringWithPropertyStringPtr, destString, cchDestString, library);
}

Js::JavascriptError::ThrowOutOfMemoryError(scriptContext);
Expand Down
8 changes: 4 additions & 4 deletions lib/Runtime/Library/WabtInterface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ struct Context
ScriptContext* scriptContext;
};

char16* NarrowStringToWide(Context* ctx, const char* src, const size_t* srcSize = nullptr, size_t* dstSize = nullptr)
char16* NarrowStringToWide(Context* ctx, const char* src, const size_t* srcSize = nullptr, charcount_t* dstSize = nullptr)
{
auto allocator = [&ctx](size_t size) {return (char16*)AnewArray(ctx->allocator, char16, size); };
char16* dst = nullptr;
size_t size;
charcount_t size;
HRESULT hr = utf8::NarrowStringToWide(allocator, src, srcSize ? *srcSize : strlen(src), &dst, &size);
if (hr != S_OK)
{
Expand Down Expand Up @@ -86,11 +86,11 @@ Js::Var Int64ToVar(int64 value, void* user_data)
Js::Var StringToVar(const char* src, uint length, void* user_data)
{
Context* ctx = (Context*)user_data;
size_t bufSize = 0;
charcount_t bufSize = 0;
size_t slength = (size_t)length;
char16* buf = NarrowStringToWide(ctx, src, &slength, &bufSize);
Assert(bufSize < UINT32_MAX);
return JavascriptString::NewCopyBuffer(buf, (charcount_t)bufSize, ctx->scriptContext);
return JavascriptString::NewCopyBuffer(buf, bufSize, ctx->scriptContext);
}

Js::Var CreateBuffer(const uint8* buf, uint size, void* user_data)
Expand Down

0 comments on commit e176a29

Please sign in to comment.