Skip to content

Commit b9f60df

Browse files
committed
[MERGE #5433 @VSadov] Fixing possible overflow in BoundFunction::NewInstance.
Merge pull request #5433 from VSadov:oacr1 Not sure if the possibility is practically reachable, but the static checker complains about a possible overflow. We allocate an extra element in `newValues` based only on `CallFlags_ExtraArg` flag. Therefore we should be consistent when writing to that element. `args.HasExtraArg` looks at more flags and, in theory, could result in writing to an element that does not exist. That seems to be making OACR unhappy. Fixes: OS:#17999665 Fixes: OS:#18010300
2 parents e698825 + 1348ade commit b9f60df

File tree

2 files changed

+13
-4
lines changed

2 files changed

+13
-4
lines changed

lib/Runtime/Base/CallInfo.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace Js
1616
{
1717
AssertOrFailFastMsg(count < Constants::UShortMaxValue - 1, "ArgList too large");
1818
ArgSlot argSlotCount = (ArgSlot)count;
19-
if (flags & CallFlags_ExtraArg)
19+
if (CallInfo::HasExtraArg(flags))
2020
{
2121
argSlotCount++;
2222
}
@@ -25,7 +25,7 @@ namespace Js
2525

2626
uint CallInfo::GetLargeArgCountWithExtraArgs(CallFlags flags, uint count)
2727
{
28-
if (flags & CallFlags_ExtraArg)
28+
if (CallInfo::HasExtraArg(flags))
2929
{
3030
UInt32Math::Inc(count);
3131
}
@@ -35,7 +35,7 @@ namespace Js
3535
ArgSlot CallInfo::GetArgCountWithoutExtraArgs(CallFlags flags, ArgSlot count)
3636
{
3737
ArgSlot newCount = count;
38-
if (flags & Js::CallFlags_ExtraArg)
38+
if (CallInfo::HasExtraArg(flags))
3939
{
4040
if (count == 0)
4141
{

lib/Runtime/Base/CallInfo.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ namespace Js
7777

7878
bool HasExtraArg() const
7979
{
80-
return (this->Flags & CallFlags_ExtraArg) || this->HasNewTarget();
80+
return CallInfo::HasExtraArg(this->Flags);
8181
}
8282

8383
bool HasNewTarget() const
@@ -91,6 +91,15 @@ namespace Js
9191

9292
static ArgSlot GetArgCountWithoutExtraArgs(CallFlags flags, ArgSlot count);
9393

94+
static bool HasExtraArg(CallFlags flags)
95+
{
96+
// Generally HasNewTarget should not be true if CallFlags_ExtraArg is not set.
97+
Assert(!CallInfo::HasNewTarget(flags) || flags & CallFlags_ExtraArg);
98+
99+
// we will still check HasNewTarget to be safe in case if above invariant does not hold.
100+
return (flags & CallFlags_ExtraArg) || CallInfo::HasNewTarget(flags);
101+
}
102+
94103
static bool HasNewTarget(CallFlags flags)
95104
{
96105
return (flags & CallFlags_NewTarget) == CallFlags_NewTarget;

0 commit comments

Comments
 (0)