GDBJIT: Remove codes when they are garbage collected.

GDBJIT: Remove codes when they are garbage collected.

R=mstarzinger@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=15477

[haitao.feng, 2013-04-28 08:04:38]

Fixed https://code.google.com/p/v8/issues/detail?id=2608.

[haitao.feng, 2013-05-15 08:13:19]

Ping.

[haitao.feng, 2013-06-28 04:43:01]

Rebased with master.

This issue could be re-produced by:
    make ia32.debug gdbjit=on -j8
    cd benchmarks
    ../out/ia32.debug/d8 ./run.js --gdbjit
                             
Richards: 22872                                                                 
DeltaBlue: 17594                                                                
Crypto: 19710                                                                   
RayTrace: 6901                                                                  
                                                                                
#                                                                               
# Fatal error in ../src/gdb-jit.cc, line 2173                                   
# CHECK(e->value == __null) failed                                              
#                                                                               
                                                                                
==== C stack trace ===============================                              
                                                                                
1:V8_Fatal                                                                    
2:v8::internal::GDBJITInterface::RegisterDetailedLineInfo(v8::internal::Code*,v8::internal::GDBJITLineInfo*)
                                                 
3:v8::internal::FullCodeGenerator::MakeCode(v8::internal::CompilationInfo*) 

The gdbjit feature is very useful for the port developers and for anyone who
wants to debug the JavaScript code and C++ codes together. It will be great that
gdbjit compilation and the "d8 ./run.js --gdbjit" could be added into the
trybots so that this feature will not be broken accidentally in the future.

[Michael Starzinger, 2013-07-03 11:18:50]

LGTM. I left a comment about the future direction of GDBJIT but I am fine with
landing this temporary fix.

As for having test coverage for GDBJIT, I couldn't agree more. I actually aim to
enable it (i.e. compilation, not the runtime flag) by default. We should be able
to enable it for standalone builds which would give us plenty of coverage. See
https://codereview.chromium.org/18638002/ about that.

I'll land this.

https://codereview.chromium.org/13880031/diff/6001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/13880031/diff/6001/src/mark-compact.cc#newcod...
src/mark-compact.cc:3120: #ifdef ENABLE_GDB_JIT_INTERFACE
I am not particularly fond of plastering GDBJIT entry hooks throughout the GC.
In the long run this should be unified with the generic eventing mechanism
(v8.h:JitCodeEvent) so that each user of that mechanism observes the same
events. GDBJIT should hence hook into Logger::IssueCodeMovedEvent and friends.
But this is a larger cleanup that we can follup-up upon and I am fine with
landing this as a temporary fix.

[haitao.feng, 2013-07-03 11:44:32]

Thanks for the review.

I have a comment on the Logger::IssueCodeMovedEvent, it might not work for the
current gdbjit implementation. If I understand the codes correctly, the code
removal event is issued when new codes take its place, instead of being marked
as garbage at the current position in this CL. To make gdbjit work and use
friends of Logger::IssueCodeMovedEvent, we might need to issue CodeRemovedEvent
at the positions in this CL.

[Michael Starzinger, 2013-07-03 11:56:35]

Yes, I agree. Switching GDBJIT over to the existing JitCodeEvent mechanism might
require slight adaptations of the GDBJIT interface implementation. But I think
it's worth it in the long run because then only one hooking mechanism needs to
be maintained.

[haitao.feng, 2013-07-03 12:00:32]

Yes, I agree. Thanks for taking care of it :)

[Michael Starzinger, 2013-07-03 15:30:06]

Committed patchset #2 manually as r15477 (presubmit successful).