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

Address deref issue #1530

Merged
merged 1 commit into from
Sep 2, 2016
Merged

Address deref issue #1530

merged 1 commit into from
Sep 2, 2016

Conversation

akroshg
Copy link
Contributor

@akroshg akroshg commented Sep 2, 2016

During the forward global optimizer pass, given a property store that causes an object layout to go from object-header-inlined to
non-object-header-inlined, kill all type syms with object-header-inlined types to protect against aliasing.

During the forward global optimizer pass, given a property store that causes an object layout to go from object-header-inlined to
non-object-header-inlined,  kill all type syms with object-header-inlined types to protect against aliasing.
@akroshg
Copy link
Contributor Author

akroshg commented Sep 2, 2016

@pleath @LouisLaf @curtisman @rajatd - any of you take a look at this? thanks.

@akroshg
Copy link
Contributor Author

akroshg commented Sep 2, 2016

@dotnet-bot test this please.

@dilijev
Copy link
Contributor

dilijev commented Sep 2, 2016

@dotnet-bot test this please

@@ -2170,7 +2170,7 @@ GlobOpt::FinishOptPropOp(IR::Instr *instr, IR::PropertySymOpnd *opnd, BasicBlock
isObjTypeSpecialized = ProcessPropOpInTypeCheckSeq<true>(instr, opnd, block, updateExistingValue, emitsTypeCheckOut, changesTypeValueOut, &isObjTypeChecked);
}

if (opnd == instr->GetDst() && this->objectTypeSyms && !isObjTypeChecked)
if (opnd == instr->GetDst() && this->objectTypeSyms)
Copy link
Contributor

Choose a reason for hiding this comment

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

So the change you're making here is that we kill the current type sym even if its type has been checked upstream? But if the type has been checked, and if the store causes us to transition to a type with a different layout, then we can update the type sym's value in the value table, and that should cause downstream dereferences to be correct. Is that not happening?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait, I am porting the change you have made. Was there a problem in the porting?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's pretty funny. I obviously have vacation brain. I'll look again later.

-- Paul


From: Akrosh Gandhimailto:notifications@github.com
Sent: ‎9/‎2/‎2016 10:04 AM
To: Microsoft/ChakraCoremailto:ChakraCore@noreply.github.com
Cc: Paul Leathersmailto:pleath@microsoft.com; Mentionmailto:mention@noreply.github.com
Subject: Re: [Microsoft/ChakraCore] Address deref issue (#1530)

In lib/Backend/GlobOptFields.cpphttps://github.com//pull/1530#discussion_r77379371:

@@ -2170,7 +2170,7 @@ GlobOpt::FinishOptPropOp(IR::Instr *instr, IR::PropertySymOpnd *opnd, BasicBlock
isObjTypeSpecialized = ProcessPropOpInTypeCheckSeq(instr, opnd, block, updateExistingValue, emitsTypeCheckOut, changesTypeValueOut, &isObjTypeChecked);
}

  • if (opnd == instr->GetDst() && this->objectTypeSyms && !isObjTypeChecked)
  • if (opnd == instr->GetDst() && this->objectTypeSyms)

Wait, I am porting the change you have made. Was there a problem in the porting?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/1530/files/5ec2d8f6dd3e67e8aa85002dbad152a614f92eeb#r77379371, or mute the threadhttps://github.com/notifications/unsubscribe-auth/APF8RCyVA8Ey0Rwuy34S_lZV5yLHz5mDks5qmFcvgaJpZM4JzSqF.

@chakrabot chakrabot merged commit 5ec2d8f into chakra-core:release/1.2 Sep 2, 2016
chakrabot pushed a commit that referenced this pull request Sep 2, 2016
Merge pull request #1530 from akroshg:deref

During the forward global optimizer pass, given a property store that causes an object layout to go from object-header-inlined to
non-object-header-inlined,  kill all type syms with object-header-inlined types to protect against aliasing.
@akroshg
Copy link
Contributor Author

akroshg commented Sep 2, 2016

Since it was just a porting I went ahead and merged it. You can refine this later.

chakrabot pushed a commit that referenced this pull request Sep 2, 2016
Merge pull request #1530 from akroshg:deref

During the forward global optimizer pass, given a property store that causes an object layout to go from object-header-inlined to
non-object-header-inlined,  kill all type syms with object-header-inlined types to protect against aliasing.
chakrabot pushed a commit that referenced this pull request Sep 2, 2016
Merge pull request #1530 from akroshg:deref

During the forward global optimizer pass, given a property store that causes an object layout to go from object-header-inlined to
non-object-header-inlined,  kill all type syms with object-header-inlined types to protect against aliasing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants