Ticket #1237 (new bug)

Opened 5 years ago

Last modified 3 years ago

docs/parrotbyte.pod && docs/pdds/pdd13_bytecode.pod are either incorrect/unclear

Reported by: kjs Owned by:
Priority: normal Milestone:
Component: none Version: 1.7.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

fixup_unpack() in src/packfile.c contains the following code:

for (i = 0; i < self->fixup_count; i++) {
PackFile_FixupEntry * const entry =
self->fixups[i] =
mem_allocate_typed(PackFile_FixupEntry);
entry->type = PF_fetch_opcode(pf, &cursor);
switch (entry->type) {
case enum_fixup_label:
case enum_fixup_sub:
entry->name = PF_fetch_cstring(pf, &cursor);
entry->offset = PF_fetch_opcode(pf, &cursor);
break;
case enum_fixup_none:
break;
default:
PIO_eprintf(interp,
"PackFile_FixupTable_unpack: Unknown fixup type
%d!\n",
entry->type);
return NULL;
}
}

And include/parrot/packfile.h has the following:

typedef enum {
enum_fixup_none,
enum_fixup_label,
enum_fixup_sub
} enum_fixup_t;

When walking through fixup_unpack() in GDB, I get the following:

Breakpoint 4, fixup_unpack (interp=0x805f7e0, seg=0x825c5d8,
cursor=0x9d570120) at src/packfile.c:3043
...
...
3072 entry->type = PF_fetch_opcode(pf, &cursor);
(gdb) n
3073 switch (entry->type) {
(gdb) p entry->type
$13 = 2

However, the docs -- namely docs/pdds/pdd13_bytecode.pod (in the section titled "Fixup Segment") and docs/parrotbyte.pod (in the section "Fixup segment") say that the type for a subroutine fixup should be 0x01, not 0x02 ...

This should either be corrected if wrong, or the docs should be cleared up a bit otherwise to make the meaning more clear.

Thanks, Jesse Taylor

PREVIOUSLY tracked by RT#51634.

Change History

follow-up: ↓ 2   Changed 5 years ago by kjs

This appears to still be an issue. Here is the relevant section from docs/pdds/pdd13_bytecode.pod:

483 =head3 Fixup Segment
484
485 The fixup segment maps names of subs to offsets in the bytecode
stream. It
486 adds one extra field to its header.
487
488 {{ TODO: I think label fixups are no longer used. Check if that is
so. }}
489
490
+--------+--------+--------------------------------------------------------+
491 | Offset | Length | Description
|
492
+--------+--------+--------------------------------------------------------+
493 | 1 | 1 | Number of fixup table entries that follow.
|
494 | | | n
|
495
+--------+--------+--------------------------------------------------------+
496
497 This is followed by n fixup table entries, of variable length, that
take the
498 following form.
499
500
+--------+--------+--------------------------------------------------------+
501 | Offset | Length | Description
|
502
+--------+--------+--------------------------------------------------------+
503 | 0 | 1 | Type of the fixup. Must be:
|
504 | | | 0x01 - Subroutine fixup
|
505
+--------+--------+--------------------------------------------------------+
506 | 1 | 1 | The label that is being fixed up. A string
constant, |
507 | | | stored as an index into the constants table.
|
508
+--------+--------+--------------------------------------------------------+
509 | 2 | 1 | For subroutine fixups, this is an index into
the |
510 | | | constants table for the sub PMC corresponding
to the |
511 | | | label.
|
512
+--------+--------+--------------------------------------------------------+

in reply to: ↑ 1   Changed 3 years ago by jkeenan

Replying to kjs:

This appears to still be an issue. Here is the relevant section from docs/pdds/pdd13_bytecode.pod:

The Fixed Segment code cited is now lines 436-461 of docs/pdds/draft/pdd13_bytecode.pod.

Note: See TracTickets for help on using tickets.