gdb on OSX is really touchy!
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc
File src/gdb-jit.cc (right):
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode47
src/gdb-jit.cc:47: #endif
Done (though I have kept the __MACH_O/__ELF defines for purposes of an
initializer down below; if this is still not a great idea I can try to
change it further).
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
I don't think we are using macros for anything like this in the code
base.
Can we use typedef here instead?
typedef ELF* DebugObject;
or something similar.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode59
src/gdb-jit.cc:59: mach_o_(0),
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
Instead of duplicating code and members replace elf with a member of
type
DebugObject (OBJ_FORMAT in your code).
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode263
src/gdb-jit.cc:263: virtual void WriteBody(Writer::Slot<Header> header,
Writer* writer) {
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
Seems to be shared between ELFSection and MachOSection. Extract to a
base class
for both sections?
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode273
src/gdb-jit.cc:273: virtual bool WriteBody(Writer* writer) {
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
Ditto.
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode547
src/gdb-jit.cc:547: ASSERT(w->position() == 0);
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
Can you split this method into several like it is done for ELF?
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1019
src/gdb-jit.cc:1019: : MachOSection("__debug_info", "__DWARF", 1,
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
one argument per line.
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1129
src/gdb-jit.cc:1129: for (int context_slot = 0; context_slot <
context_slots;
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
for(x;
y;
z)
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1189
src/gdb-jit.cc:1189: : MachOSection("__debug_abbrev", "__DWARF", 1,
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
Argument per line
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1293
src/gdb-jit.cc:1293: int current_abbreviation = 4;
On 2011/06/23 11:28:37, Vyacheslav Egorov wrote:
Move it to the beginning of the function and use instead of 1, 2, 3
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1318
src/gdb-jit.cc:1318: for (int context_slot = 0; context_slot <
context_slots;
On 2011/06/23 11:28:37, Vyacheslav Egorov wrote:
for(x;
y;
z)
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1329
src/gdb-jit.cc:1329: for (int internal_slot = 0; internal_slot <
internal_slots;
On 2011/06/23 11:28:37, Vyacheslav Egorov wrote:
for(x;
y;
z)
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1832
src/gdb-jit.cc:1832: void __gdb_print_v8_object(MaybeObject* object) {
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
Frankly speaking I don't see any reason to add this instead of just
calling
Print directly from gdb prompt
gdb was having some trouble finding the symbol for Print; exposing it as
a plain cdecl makes it easier.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1861
src/gdb-jit.cc:1861: bool dump_if_enabled,
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
something with indentation.
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.cc#newcode1871
src/gdb-jit.cc:1871: OS::SNPrintF(Vector<char>(file_name,
kMaxFileNameSize), "%s%s%d%s",
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
One argument per line.
(somehow I missed this place in the previous commits, sorry).
Done.
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.h
File src/gdb-jit.h (right):
http://codereview.chromium.org/6995161/diff/1/src/gdb-jit.h#newcode118
src/gdb-jit.h:118: Script* script = NULL,
No problem; I think I got the other callers.
On 2011/06/23 11:14:47, Vyacheslav Egorov wrote:
C++ Style Guide does not allow default arguments. I am sorry I've
missed this in
case of script.
Can you change all callers to pass NULL explicitly?
http://codereview.chromium.org/6995161/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
