Christian, Do you mind adding a couple words explaining why you reverted next time?
Thanks, -Ivan On Fri, Mar 6, 2009 at 12:55, <[email protected]> wrote: > > Reviewers: Mads Ager, > > Description: > Reverted r1434 > > Please review this at http://codereview.chromium.org/40216 > > Affected files: > M src/codegen-ia32.cc > M test/mjsunit/regress/regress-259.js > > > Index: src/codegen-ia32.cc > diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc > index > 649c9e5d45f3a7d6a6304602e0963e9de739a301..b5ac20078f319ca61fdbab88e75b586d1eb1cfd3 > 100644 > --- a/src/codegen-ia32.cc > +++ b/src/codegen-ia32.cc > @@ -2830,29 +2830,22 @@ void CodeGenerator::VisitTryFinally(TryFinally* > node) { > } > > // Unlink from try chain; be careful not to destroy the TOS. > - if (unlink.is_linked()) { > - unlink.Bind(); > - } > - > - // Control can reach here via a jump to unlink or by falling off the > - // end of the try block (with no unlinks). > - if (has_valid_frame()) { > - // Reload sp from the top handler, because some statements that we > - // break from (eg, for...in) may have left stuff on the stack. > - // Preserve the TOS in a register across stack manipulation. > - frame_->EmitPop(eax); > - ExternalReference handler_address(Top::k_handler_address); > - __ mov(edx, Operand::StaticVariable(handler_address)); > - const int kNextOffset = StackHandlerConstants::kNextOffset + > - StackHandlerConstants::kAddressDisplacement; > - __ lea(esp, Operand(edx, kNextOffset)); > - frame_->Forget(frame_->height() - handler_height); > - > - frame_->EmitPop(Operand::StaticVariable(handler_address)); > - frame_->Drop(StackHandlerConstants::kSize / kPointerSize - 1); > - // Next_sp popped. > - frame_->EmitPush(eax); > - } > + unlink.Bind(); > + // Reload sp from the top handler, because some statements that we > + // break from (eg, for...in) may have left stuff on the stack. > + // Preserve the TOS in a register across stack manipulation. > + frame_->EmitPop(eax); > + ExternalReference handler_address(Top::k_handler_address); > + __ mov(edx, Operand::StaticVariable(handler_address)); > + const int kNextOffset = StackHandlerConstants::kNextOffset + > + StackHandlerConstants::kAddressDisplacement; > + __ lea(esp, Operand(edx, kNextOffset)); > + frame_->Forget(frame_->height() - handler_height); > + > + frame_->EmitPop(Operand::StaticVariable(handler_address)); > + frame_->Drop(StackHandlerConstants::kSize / kPointerSize - 1); > + // Next_sp popped. > + frame_->EmitPush(eax); > > // --- Finally block --- > finally_block.Bind(); > Index: test/mjsunit/regress/regress-259.js > diff --git a/test/mjsunit/regress/regress-259.js > b/test/mjsunit/regress/regress-259.js > deleted file mode 100644 > index > f0476ffc375f0b7a0f1b53d3ea7667db4bad884d..0000000000000000000000000000000000000000 > --- a/test/mjsunit/regress/regress-259.js > +++ /dev/null > @@ -1,33 +0,0 @@ > -// Copyright 2009 the V8 project authors. All rights reserved. > -// Redistribution and use in source and binary forms, with or without > -// modification, are permitted provided that the following conditions are > -// met: > -// > -// * Redistributions of source code must retain the above copyright > -// notice, this list of conditions and the following disclaimer. > -// * Redistributions in binary form must reproduce the above > -// copyright notice, this list of conditions and the following > -// disclaimer in the documentation and/or other materials provided > -// with the distribution. > -// * Neither the name of Google Inc. nor the names of its > -// contributors may be used to endorse or promote products derived > -// from this software without specific prior written permission. > -// > -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > - > -// Test that we do not crash when compiling a try/finally with an > -// infinite loop (with no normal exits) in the try block. > - > -// See http://code.google.com/p/v8/issues/detail?id=259 > - > -assertThrows("try { while (true) { throw 0; }} finally {}"); > > > > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
