Skip to content

Commit

Permalink
memory corruption in fixup handling caused a crash in jwasmr.exe
Browse files Browse the repository at this point in the history
  • Loading branch information
Baron-von-Riedesel committed Oct 12, 2020
1 parent 72a4de0 commit bb4de13
Show file tree
Hide file tree
Showing 13 changed files with 55 additions and 15 deletions.
5 changes: 4 additions & 1 deletion History.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
!= ES wasn't rejected; see string5.asm.
- string operations with symbolic memory operands did ignore assumes, thus
no segment prefix was created; see string8.asm.
- comment behind MACRO directive was appended to all lines inside the macro.
- comment behind MACRO directive was appended to all lines inside the macro;
see lst2.asm.
- jwasmr.exe: memory corruption may have caused a system crash if pass one
ended with an error.

03/14/2020, v2.13:

Expand Down
11 changes: 9 additions & 2 deletions Html/Manual.html
Original file line number Diff line number Diff line change
Expand Up @@ -5162,8 +5162,8 @@ <H1 ID="AC"> Appendix C. Errors and Warnings </H1>

<TR BGCOLOR="#F0F0F0">
<TD><b>x211</b></TD>
<TD></TD>
<TD></TD>
<TD>Model must be FLAT</TD>
<TD>In 64-bit mode, or if output format is -pe, there's no segmented model available.</TD>
</TR>

<TR BGCOLOR="#F0F0F0">
Expand Down Expand Up @@ -5574,6 +5574,13 @@ <H1 ID="AC"> Appendix C. Errors and Warnings </H1>
<TD>A 16-bit procedure that is to be exported must be declared with the FAR distance attribute.</TD>
</TR>

<TR BGCOLOR="#F0F0F0">
<TD><b>x277</b></TD>
<TD>Start label not in a 16-bit segment</TD>
<TD>MZ format only: the start label is in a 32- or 64-bit segment.
The resulting MZ binary won't run without further modification, i.e. adding a DOS-extender stub.</TD>
</TR>

</TABLE>

<H1 ID="AD"> Appendix D. Differences between Masm 6 and Masm 8 </H1>
Expand Down
Binary file removed bin/JWasm_v214pre5_dos.zip
Binary file not shown.
Binary file added bin/JWasm_v214pre6_dos.zip
Binary file not shown.
Binary file not shown.
Binary file added bin/JWasmr.exe
Binary file not shown.
3 changes: 3 additions & 0 deletions src/H/fixup.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ struct fixup {
#endif
unsigned char loader_resolved:1; /* operator LROFFSET */
unsigned char orgoccured:1; /* v2.04 ORG occured behind this fix */
#if FASTMEM==0
unsigned char count:2; /* v2.14 ref count */
#endif
};
};
union {
Expand Down
2 changes: 1 addition & 1 deletion src/backptch.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ ret_code BackPatch( struct asym *sym )
uint_32 oldofs = sym->offset;
#endif

DebugMsg1(("BackPatch(%s): location=%s:%X, bp_fixup=%p\n", sym->name, sym->segment ? sym->segment->name : "!NULL!", sym->offset, sym->bp_fixup ));
DebugMsg1(("BackPatch(%s): location=%s:%" I32_SPEC "X, bp_fixup=%p\n", sym->name, sym->segment ? sym->segment->name : "!NULL!", sym->offset, sym->bp_fixup ));

for( fixup = sym->bp_fixup; fixup; fixup = next ) {
next = fixup->nextbp;
Expand Down
2 changes: 1 addition & 1 deletion src/expreval.c
Original file line number Diff line number Diff line change
Expand Up @@ -3189,7 +3189,7 @@ static ret_code evaluate( struct expr *opnd1, int *i, struct asm_tok tokenarray[
struct expr opnd2;

curr_operator = *i;
DebugMsg1(("%u evaluate loop, operator=>%s< opnd1->sym=%X, type=%s\n",
DebugMsg1(("%u evaluate loop, operator=>%s< opnd1->sym=%p, type=%s\n",
evallvl, tokenarray[curr_operator].string_ptr, opnd1->sym, (opnd1->type ? opnd1->type->name : "NULL") ));

if ( opnd1->kind != EXPR_EMPTY ) {
Expand Down
23 changes: 21 additions & 2 deletions src/fixup.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ struct fixup *CreateFixup( struct asym *sym, enum fixup_types type, enum fixup_o
fixup->marker = 'XF';
DebugMsg1(("CreateFixup, pass=%u: fix=%p sym=%s\n", Parse_Pass+1, fixup, sym ? sym->name : "NULL" ));
#endif
fixup->flags = 0; /* v2.14: moved up here so count won't be overwritten */

/* add the fixup to the symbol's linked list (used for backpatch)
* this is done for pass 1 only.
Expand All @@ -80,6 +81,9 @@ struct fixup *CreateFixup( struct asym *sym, enum fixup_types type, enum fixup_o
if ( sym ) { /* changed v1.96 */
fixup->nextbp = sym->bp_fixup;
sym->bp_fixup = fixup;
#if FASTMEM==0
fixup->count++;
#endif
}
/* v2.03: in pass one, create a linked list of
* fixup locations for a segment. This is to improve
Expand All @@ -92,6 +96,9 @@ struct fixup *CreateFixup( struct asym *sym, enum fixup_types type, enum fixup_o
if ( CurrSeg ) {
fixup->nextrlc = CurrSeg->e.seginfo->FixupList.head;
CurrSeg->e.seginfo->FixupList.head = fixup;
#if FASTMEM==0
fixup->count++;
#endif
}
}
/* initialize locofs member with current offset.
Expand All @@ -101,13 +108,12 @@ struct fixup *CreateFixup( struct asym *sym, enum fixup_types type, enum fixup_o
fixup->offset = 0;
fixup->type = type;
fixup->option = option;
fixup->flags = 0;
fixup->frame_type = Frame_Type; /* this is just a guess */
fixup->frame_datum = Frame_Datum;
fixup->def_seg = CurrSeg; /* may be NULL (END directive) */
fixup->sym = sym;

DebugMsg1(("CreateFixup(sym=%s type=%u, opt=%u) cnt=%" I32_SPEC "X, loc=%" I32_SPEC "Xh frame_type/datum=%u/%u\n",
DebugMsg1(("CreateFixup(sym=%s type=%u, opt=%u) cnt=%" I32_SPEC "u, loc=%" I32_SPEC "Xh frame_type/datum=%u/%u\n",
sym ? sym->name : "NULL", type, option, ++cnt, fixup->locofs, fixup->frame_type, fixup->frame_datum ));
return( fixup );
}
Expand All @@ -125,17 +131,30 @@ void FreeFixup( struct fixup *fixup )
if ( dir ) {
if ( fixup == dir->e.seginfo->FixupList.head ) {
dir->e.seginfo->FixupList.head = fixup->nextrlc;
#if FASTMEM==0
fixup->count--;
#endif
} else {
for ( fixup2 = dir->e.seginfo->FixupList.head; fixup2; fixup2 = fixup2->nextrlc ) {
if ( fixup2->nextrlc == fixup ) {
fixup2->nextrlc = fixup->nextrlc;
#if FASTMEM==0
fixup->count--;
#endif
break;
}
}
}
}
}
#if FASTMEM==0
DebugMsg1(("FreeFixup( %p ), count=%u, def_seg=%p\n", fixup, fixup->count, fixup->def_seg ));
if ( !fixup->count ) {
LclFree( fixup );
}
#else
LclFree( fixup );
#endif
}

/*
Expand Down
2 changes: 1 addition & 1 deletion src/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ static ret_code memory_operand( struct code_info *CodeInfo, unsigned CurrOpnd, s
uint_8 Ofssize;
enum fixup_types fixup_type;

DebugMsg1(("memory_operand(opndx.value=%X / sym=%s / memtype=%Xh, with_fixup=%u) enter, [CodeInfo->memtype=%Xh, Ofssize=%u, adrsiz=%u]\n",
DebugMsg1(("memory_operand(opndx.value=%" I32_SPEC "X / sym=%s / memtype=%Xh, with_fixup=%u) enter, [CodeInfo->memtype=%Xh, Ofssize=%u, adrsiz=%u]\n",
opndx->value, opndx->sym ? opndx->sym->name : "NULL", opndx->mem_type, with_fixup, CodeInfo->mem_type, CodeInfo->Ofssize, CodeInfo->prefix.adrsiz ));

/* v211: use full 64-bit value */
Expand Down
9 changes: 7 additions & 2 deletions src/segment.c
Original file line number Diff line number Diff line change
Expand Up @@ -1505,8 +1505,13 @@ void SegmentFini( void )
DebugMsg(("SegmentFini: segment %s\n", curr->sym.name ));
for ( fix = curr->e.seginfo->FixupList.head; fix ; ) {
struct fixup *next = fix->nextrlc;
DebugMsg(("SegmentFini: free fixup [sym=%s, loc=%" I32_SPEC "X]\n", fix->sym ? fix->sym->name : "NULL", fix->location ));
LclFree( fix );
DebugMsg(("SegmentFini: free fixup %p [sym=%s, count=%u]\n", fix, fix->sym ? fix->sym->name : "NULL", fix->count ));
/* v2.14: problem is that a fixup may be in two linked lists after step 1;
* hence only free the fixup if nextbp is NULL.
*/
fix->count--;
if ( !fix->count )
LclFree( fix );
fix = next;
}
}
Expand Down
13 changes: 8 additions & 5 deletions src/symbols.c
Original file line number Diff line number Diff line change
Expand Up @@ -410,23 +410,26 @@ static void free_ext( struct asym *sym )
}

/* free a symbol.
* the symbol is no unlinked from hash tables chains,
* the symbol is not unlinked from hash table chains,
* hence it is assumed that this is either not needed
* or done by the caller.
*/

void SymFree( struct asym *sym )
/******************************/
{
//DebugMsg(("SymFree: free %X, name=%s, state=%X\n", sym, sym->name, sym->state));
//DebugMsg(("SymFree: free %p, name=%s, state=%u\n", sym, sym->name, sym->state));
free_ext( sym );
#if FASTMEM==0
if ( sym->state != SYM_EXTERNAL ) {
if ( sym->state != SYM_EXTERNAL ) { /* external backpatches are cleared in PassOneChecks() */
struct fixup *fix;
for( fix = sym->bp_fixup ; fix; ) {
struct fixup *next = fix->nextbp;
DebugMsg(("SymFree: free bp fixup %p\n", fix ));
LclFree( fix );
DebugMsg(("SymFree: free backpatch fixup %p [count=%u]\n", fix, fix->count ));
/* v2.14: free fixup only if not referenced anymore */
fix->count--;
if ( !fix->count )
LclFree( fix );
fix = next;
}
}
Expand Down

0 comments on commit bb4de13

Please sign in to comment.