mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 13:12:46 +00:00 
			
		
		
		
	LibJS/Bytecode: Keep saved return value in call frame register
This fixes an issue where returning inside a `try` block and then calling a function inside `finally` would clobber the saved return value from the `try` block. Note that we didn't need to change the base of register allocation, since it was already 1 too high. With this fixed, https://microsoft.com/edge loads in bytecode mode. :^) Thanks to Luke for reducing the issue!
This commit is contained in:
		
							parent
							
								
									475f1b6083
								
							
						
					
					
						commit
						9f06e130a2
					
				
					 4 changed files with 26 additions and 10 deletions
				
			
		|  | @ -46,8 +46,6 @@ void Interpreter::visit_edges(Cell::Visitor& visitor) | |||
| { | ||||
|     if (m_return_value.has_value()) | ||||
|         visitor.visit(*m_return_value); | ||||
|     if (m_saved_return_value.has_value()) | ||||
|         visitor.visit(*m_saved_return_value); | ||||
|     if (m_saved_exception.has_value()) | ||||
|         visitor.visit(*m_saved_exception); | ||||
|     for (auto& frame : m_call_frames) { | ||||
|  | @ -277,8 +275,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu | |||
|         if (!unwind_contexts().is_empty() && !will_yield) { | ||||
|             auto& unwind_context = unwind_contexts().last(); | ||||
|             if (unwind_context.executable == m_current_executable && unwind_context.finalizer) { | ||||
|                 m_saved_return_value = m_return_value; | ||||
|                 m_return_value = {}; | ||||
|                 reg(Register::saved_return_value()) = m_return_value.release_value(); | ||||
|                 m_current_block = unwind_context.finalizer; | ||||
|                 // the unwind_context will be pop'ed when entering the finally block
 | ||||
|                 continue; | ||||
|  | @ -308,13 +305,15 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu | |||
|         } | ||||
|     } | ||||
| 
 | ||||
|     auto saved_return_value = reg(Register::saved_return_value()); | ||||
| 
 | ||||
|     auto frame = pop_call_frame(); | ||||
| 
 | ||||
|     Value return_value = js_undefined(); | ||||
|     if (m_return_value.has_value()) { | ||||
|         return_value = m_return_value.release_value(); | ||||
|     } else if (m_saved_return_value.has_value() && !m_saved_exception.has_value()) { | ||||
|         return_value = m_saved_return_value.release_value(); | ||||
|     } else if (!saved_return_value.is_empty()) { | ||||
|         return_value = saved_return_value; | ||||
|     } | ||||
| 
 | ||||
|     // NOTE: The return value from a called function is put into $0 in the caller context.
 | ||||
|  | @ -335,7 +334,6 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu | |||
|     if (m_saved_exception.has_value()) { | ||||
|         Value thrown_value = m_saved_exception.value(); | ||||
|         m_saved_exception = {}; | ||||
|         m_saved_return_value = {}; | ||||
|         if (auto* call_frame = frame.get_pointer<NonnullOwnPtr<CallFrame>>()) | ||||
|             return { throw_completion(thrown_value), move(*call_frame) }; | ||||
|         return { throw_completion(thrown_value), nullptr }; | ||||
|  | @ -366,8 +364,8 @@ ThrowCompletionOr<void> Interpreter::continue_pending_unwind(Label const& resume | |||
|         return throw_completion(m_saved_exception.release_value()); | ||||
|     } | ||||
| 
 | ||||
|     if (m_saved_return_value.has_value()) { | ||||
|         do_return(m_saved_return_value.release_value()); | ||||
|     if (!saved_return_value().is_empty()) { | ||||
|         do_return(saved_return_value()); | ||||
|         return {}; | ||||
|     } | ||||
| 
 | ||||
|  |  | |||
|  | @ -61,6 +61,7 @@ public: | |||
|     ValueAndFrame run_and_return_frame(Realm&, Bytecode::Executable&, Bytecode::BasicBlock const* entry_point, CallFrame* = nullptr); | ||||
| 
 | ||||
|     ALWAYS_INLINE Value& accumulator() { return reg(Register::accumulator()); } | ||||
|     ALWAYS_INLINE Value& saved_return_value() { return reg(Register::saved_return_value()); } | ||||
|     Value& reg(Register const& r) { return registers()[r.index()]; } | ||||
| 
 | ||||
|     auto& saved_lexical_environment_stack() { return call_frame().saved_lexical_environments; } | ||||
|  | @ -119,7 +120,6 @@ private: | |||
|     Optional<BasicBlock const*> m_pending_jump; | ||||
|     BasicBlock const* m_scheduled_jump { nullptr }; | ||||
|     Optional<Value> m_return_value; | ||||
|     Optional<Value> m_saved_return_value; | ||||
|     Optional<Value> m_saved_exception; | ||||
|     Executable* m_current_executable { nullptr }; | ||||
|     OwnPtr<JS::Interpreter> m_ast_interpreter; | ||||
|  |  | |||
|  | @ -19,6 +19,13 @@ public: | |||
|         return Register(accumulator_index); | ||||
|     } | ||||
| 
 | ||||
|     constexpr static u32 saved_return_value_index = 1; | ||||
| 
 | ||||
|     static constexpr Register saved_return_value() | ||||
|     { | ||||
|         return Register(saved_return_value_index); | ||||
|     } | ||||
| 
 | ||||
|     constexpr explicit Register(u32 index) | ||||
|         : m_index(index) | ||||
|     { | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling