494817 - nanojit: pageBottom is incorrect

Status: RESOLVED DUPLICATE of bug 494639

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

nanojit/nanojit.h has these macros:

#define pageTop(x)          ( (int*)alignTo(x,NJ_PAGE_SIZE) )
#define pageDataStart(x)    ( (int*)(alignTo(x,NJ_PAGE_SIZE) + sizeof(PageHeader)) )
#define pageBottom(x)       ( (int*)(alignTo(x,NJ_PAGE_SIZE)+NJ_PAGE_SIZE)-1 )
#define samepage(x,y)       (pageTop(x) == pageTop(y))

pageBottom is incorrect -- the cast binds tighter than the '-', so it returns the address of the 4th last byte on the page, not the address of the last byte.

It's only used in bytesToBottom in Assembler.cpp, and bytesToBottom is only used in profiling code, so the worst effect is slight miscounting of code sizes, but it seems worth fixing to avoid the counting errors and prevent any future errors.

There are a couple of options for fixing:

1. Just shift the -1 inside the parentheses.  But casting a non-aligned address to (int*) seems like a bad idea.

2. Do (1) and change the cast to something like (int8_t*) or (uintptr_t).  This allows byte-based accounting which is less error-prone.  But then the type is different to pageTop and pageDataStart.

3. Do (1) and (2) and change the types of pageTop and pageDataStart as well.  (Almost all callers of them immediately cast the result to something else anyway, so there wouldn't be much change as a result.)


I favour (3).

Comment 1

[Nicholas Nethercote [inactive]]

This bug is likely to be subsumed by bug 494639.  I'll close this one once 494639 has been committed to TM.

Comment 2

[Nicholas Nethercote [inactive]]

Actually, I'll mark it as a dup of bug 494639.