Hi Sanjoy,

We are almost at the landing point.

I just need you to check a couple of questions and style nitpicks.


http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc
File src/gdb-jit.cc (left):

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#oldcode1091
src/gdb-jit.cc:1091: if (!FLAG_gdbjit_full &&
!code_desc.is_line_info_available()) {
Why did you kill FLAG_gdbjit_full support?

It is an optimization: without it simple node.js application takes
several minutes to start under GDB.

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc
File src/gdb-jit.cc (right):

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode657
src/gdb-jit.cc:657: GDBJITInterface::CodeTag type)
nitpick: type -> tag

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode662
src/gdb-jit.cc:662: type_(type)
nitpick: type -> tag

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode690
src/gdb-jit.cc:690: ASSERT(ofs < OFFSET_TYPE_MAX);
will not compile on ia32

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode695
src/gdb-jit.cc:695: ASSERT(ofs < OFFSET_TYPE_MAX);
will not compile on ia32

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode709
src/gdb-jit.cc:709: GDBJITInterface::CodeTag get_type() const {
nitpick: type -> tag

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode710
src/gdb-jit.cc:710: return type_;
nitpick: type -> tag

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode718
src/gdb-jit.cc:718: GDBJITInterface::CodeTag type_;
nitpick: type -> tag

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode1164
src/gdb-jit.cc:1164: elf->AddSection(new UnwindInfoSection(desc));
I don't think it will compile on ia32

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode1332
src/gdb-jit.cc:1332: #define RBP_PUSH_OFFSET 1
We prefer constants to macroses:

static const int kRBPPushOffset = 1;

etc.

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode1351
src/gdb-jit.cc:1351: #define AddUnwindInfo(x) ((void) x)
Move #ifdef into function body to avoid defining macros.

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.cc#newcode1365
src/gdb-jit.cc:1365: if (e->value != NULL &&
!IsLineInfoTagged(e->value))
Is there a reason for this change?

If we are registering several code object twice we will leak memory
here.

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.h
File src/gdb-jit.h (right):

http://codereview.chromium.org/6371011/diff/35001/src/gdb-jit.h#newcode113
src/gdb-jit.h:113: CodeTag type,
nitpick: type -> tag

http://codereview.chromium.org/6371011/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to