This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
RFA: dcache corruption bug
- To: gdb-patches at sourceware dot cygnus dot com
- Subject: RFA: dcache corruption bug
- From: jtc at redback dot com (J.T. Conklin)
- Date: 09 Aug 2000 12:00:30 -0700
- Reply-To: jtc at redback dot com
While adopting dcache for memory region attributes, I discovered what
appears to be a potential cache corruption, plus a nit that the 'refs'
field is never zeroed when a cache line is re-cycled, which makes it
somewhat less than useful.
The comment for dcache_alloc() states its caller should remove the
returned dcache_block from the free list and place it on the valid
list to prevent cache corruption. Good idea, but the code actually
removes a dcache_block from the free list (or recycles one from the
valid list) and inserts it on the valid list. Because none of the
fields are re-initialized, if the caller does not do it the contents
of the cache block could be written to memory.
This was the case for dcache_peek_byte(). If a new block was
allocated, and for some reason the read failed, the block would have
the contents of the previous cache line that used the block.
I don't think that bad values would be ever be written; the line would
have been flushed before the block was re-cycled. But bytes with state
ENTRY_OK would return the old values in subsequent reads.
To fix this problem, I changed dcache_alloc() to initialize the
address and invalidate the cache line before returning a new block.
Other related issues:
* dcache makes no attempt to remove the least-recently-used block when
a cache line is recycled. When a new cache-line is allocated it is
added to the tail of the valid list. If there aren't any cache-lines
in the free list, the one from the head of the valid list is recycled.
Anyone have any reasons why making this a LRU cache is a bad idea?
* dcache has a free list and a valid list. Why not just have one list
and invalidate unused cache lines. When we need a new block, we can
just grab a line from the pool. With a LRU cache, the unused blocks
will be the ones subject for re-cycling.
* Is there a generic set of list manipulation macros in libiberty? I
thought there was, but couldn't find anything.
--jtc
2000-08-09 J.T. Conklin <jtc@redback.com>
* dcache.c (dcache_alloc): Changed to take address of line as an
argument, and to invalidate cache line before returning.
(dcache_peek_byte): Updated.
(dcache_poke_byte): Updated.
Index: dcache.c
===================================================================
RCS file: /cvs/src/src/gdb/dcache.c,v
retrieving revision 1.5
diff -c -r1.5 dcache.c
*** dcache.c 2000/07/30 01:48:25 1.5
--- dcache.c 2000/08/09 18:17:28
***************
*** 157,163 ****
static int dcache_write_line (DCACHE * dcache, struct dcache_block *db);
! static struct dcache_block *dcache_alloc (DCACHE * dcache);
static int dcache_writeback (DCACHE * dcache);
--- 157,163 ----
static int dcache_write_line (DCACHE * dcache, struct dcache_block *db);
! static struct dcache_block *dcache_alloc (DCACHE * dcache, CORE_ADDR addr);
static int dcache_writeback (DCACHE * dcache);
***************
*** 267,281 ****
/* Get a free cache block, put or keep it on the valid list,
! and return its address. The caller should store into the block
! the address and data that it describes, then remque it from the
! free list and insert it into the valid list. This procedure
! prevents errors from creeping in if a memory retrieval is
! interrupted (which used to put garbage blocks in the valid
! list...). */
static struct dcache_block *
! dcache_alloc (DCACHE *dcache)
{
register struct dcache_block *db;
--- 267,276 ----
/* Get a free cache block, put or keep it on the valid list,
! and return its address. */
static struct dcache_block *
! dcache_alloc (DCACHE *dcache, CORE_ADDR addr)
{
register struct dcache_block *db;
***************
*** 297,302 ****
--- 292,302 ----
dcache_write_line (dcache, db);
}
+ db->addr = MASK(addr);
+ db->refs = 0;
+ db->anydirty = 0;
+ memset (db->state, ENTRY_BAD, sizeof (db->data));
+
/* append this line to end of valid list */
if (!dcache->valid_head)
dcache->valid_head = db;
***************
*** 327,335 ****
dcache_write_line (dcache, db);
}
else
! db = dcache_alloc (dcache);
immediate_quit++;
- db->addr = MASK (addr);
while (done < LINE_SIZE)
{
int try =
--- 327,335 ----
dcache_write_line (dcache, db);
}
else
! db = dcache_alloc (dcache, addr);
!
immediate_quit++;
while (done < LINE_SIZE)
{
int try =
***************
*** 379,387 ****
if (!db)
{
! db = dcache_alloc (dcache);
! db->addr = MASK (addr);
! memset (db->state, ENTRY_BAD, sizeof (db->data));
}
db->data[XFORM (addr)] = *ptr;
--- 379,385 ----
if (!db)
{
! db = dcache_alloc (dcache, addr);
}
db->data[XFORM (addr)] = *ptr;
--
J.T. Conklin
RedBack Networks