RSIM Bug Report #3

Version of RSIM	1.0
Bug number	3
Bug class	4
Date	9/25/97
Reported by	authors
Affects	All (No expected impact on simulated performance)
Files	mshr.c

Problem Description

The last portion of the REQUEST case of the function notpres_mshr deals with cases when a request is going to coalesce into an already outstanding MSHR. The if/else clause at line 494 of the release was originally intended to update the writes_present field in cases where a new write request coalesces with an MSHR held only in read state. This writes_present field is used to transition lines from E to M upon reply. Note that this condition cannot actually occur in the cache, because the current MSHR configuration does not allow writes to coalesce into a read MSHR. This piece of code was thus only reserved for future expansion.

However, the writes_present field of the MSHR receiving the coalesced write was not updated; instead the MSHR with the position held in the variable free_mshr was updated. In the case of a coalesced request, free_mshr would equal -1, so this lead to an out-of-bounds array access.

This bug was not detected during testing because the writes_present field is one of the last fields of the MSHR data structure; consequently, the writes_present field of MSHR entry -1 is in the padding space provided by malloc before the dynamically allocated MSHR data structures. Further, the case this segment of code was originally intended to handle -- a MSHR holding only reads converting to an MSHR with both reads and writes -- cannot occur in the simulated system because of the MSHR_STALL_WAR condition. Thus, this bug is not expected to have any impact on simulated performance.

Problem Fix

The else clause of this condition is incorrect even if future modifications allow this situation to arise: an MSHR can never go back to having no writes present once it has writes present to begin with. Thus, the else clause must be removed. In the if clause, the occurrence of free_mshr should be replaced with hit.

Thus, the new code segment for this condition will look like:

if ((captr->cache_level_type != FIRSTLEVEL_WT) && (req->req_type == WRITE || req->req_type == RMW)) captr->mshrs[hit].writes_present = 1; /* transition line to dirty when it comes back */ These changes have been made in the current distribution version of mshr.c .