diff options
author | Robert Kovacsics <rmk35@cl.cam.ac.uk> | 2018-07-13 17:28:11 +0100 |
---|---|---|
committer | Kovacsics RĂ³bert <kovirobi@gmail.com> | 2018-07-19 13:43:35 +0000 |
commit | cbfe914f885cc54a3c8763c0ed6aed8e425d6357 (patch) | |
tree | 0c187b8c43cf1b2f148f509bdebcca9ecafc9357 /src/mem | |
parent | ed427a3fcdf49a0ea0afa212c96240a04d4678d7 (diff) | |
download | gem5-cbfe914f885cc54a3c8763c0ed6aed8e425d6357.tar.xz |
mem: Fix off-by-one error in checkFunctional, and simplify it
There was an off-by-one error in the isRead() case, as `val_end` and
`func_end` pointed to the last byte to write to (not one past the last
byte), and thus `*_end - *_start` was not the length of the data to
memcpy.
This was correct in the case of
val_start >= func_start && val_end <= func_end
where `overlap_size = size`, but if it were (as the other cases
suggest) `overlap_size = val_end - val_start`, then it would also be
off by one.
Also, the isWrite() case catered for this.
I simplified the four ifs into one case which uses min/max (this is
how I spotted the inconsistency).
Change-Id: Ib5c5da084652e752f6baf1eec56b51b4f0f5c95c
Reviewed-on: https://gem5-review.googlesource.com/11750
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>
Diffstat (limited to 'src/mem')
-rw-r--r-- | src/mem/packet.cc | 116 |
1 files changed, 34 insertions, 82 deletions
diff --git a/src/mem/packet.cc b/src/mem/packet.cc index 7a81cdbb7..f2f7fd9b8 100644 --- a/src/mem/packet.cc +++ b/src/mem/packet.cc @@ -50,6 +50,7 @@ #include "mem/packet.hh" +#include <algorithm> #include <cstring> #include <iostream> @@ -227,10 +228,10 @@ bool Packet::checkFunctional(Printable *obj, Addr addr, bool is_secure, int size, uint8_t *_data) { - Addr func_start = getAddr(); - Addr func_end = getAddr() + getSize() - 1; - Addr val_start = addr; - Addr val_end = val_start + size - 1; + const Addr func_start = getAddr(); + const Addr func_end = getAddr() + getSize() - 1; + const Addr val_start = addr; + const Addr val_end = val_start + size - 1; if (is_secure != _isSecure || func_start > val_end || val_start > func_end) { @@ -251,94 +252,45 @@ Packet::checkFunctional(Printable *obj, Addr addr, bool is_secure, int size, return false; } - // offset of functional request into supplied value (could be - // negative if partial overlap) - int offset = func_start - val_start; + const Addr val_offset = func_start > val_start ? + func_start - val_start : 0; + const Addr func_offset = func_start < val_start ? + val_start - func_start : 0; + const Addr overlap_size = std::min(val_end, func_end)+1 - + std::max(val_start, func_start); if (isRead()) { - if (func_start >= val_start && func_end <= val_end) { - memcpy(getPtr<uint8_t>(), _data + offset, getSize()); - if (bytesValid.empty()) - bytesValid.resize(getSize(), true); - // complete overlap, and as the current packet is a read - // we are done - return true; - } else { - // Offsets and sizes to copy in case of partial overlap - int func_offset; - int val_offset; - int overlap_size; - - // calculate offsets and copy sizes for the two byte arrays - if (val_start < func_start && val_end <= func_end) { - // the one we are checking against starts before and - // ends before or the same - val_offset = func_start - val_start; - func_offset = 0; - overlap_size = val_end - func_start; - } else if (val_start >= func_start && val_end > func_end) { - // the one we are checking against starts after or the - // same, and ends after - val_offset = 0; - func_offset = val_start - func_start; - overlap_size = func_end - val_start; - } else if (val_start >= func_start && val_end <= func_end) { - // the one we are checking against is completely - // subsumed in the current packet, possibly starting - // and ending at the same address - val_offset = 0; - func_offset = val_start - func_start; - overlap_size = size; - } else if (val_start < func_start && val_end > func_end) { - // the current packet is completely subsumed in the - // one we are checking against - val_offset = func_start - val_start; - func_offset = 0; - overlap_size = func_end - func_start; - } else { - panic("Missed a case for checkFunctional with " - " %s 0x%x size %d, against 0x%x size %d\n", - cmdString(), getAddr(), getSize(), addr, size); - } - - // copy partial data into the packet's data array - uint8_t *dest = getPtr<uint8_t>() + func_offset; - uint8_t *src = _data + val_offset; - memcpy(dest, src, overlap_size); + memcpy(getPtr<uint8_t>() + func_offset, + _data + val_offset, + overlap_size); - // initialise the tracking of valid bytes if we have not - // used it already - if (bytesValid.empty()) - bytesValid.resize(getSize(), false); + // initialise the tracking of valid bytes if we have not + // used it already + if (bytesValid.empty()) + bytesValid.resize(getSize(), false); - // track if we are done filling the functional access - bool all_bytes_valid = true; + // track if we are done filling the functional access + bool all_bytes_valid = true; - int i = 0; + int i = 0; - // check up to func_offset - for (; all_bytes_valid && i < func_offset; ++i) - all_bytes_valid &= bytesValid[i]; + // check up to func_offset + for (; all_bytes_valid && i < func_offset; ++i) + all_bytes_valid &= bytesValid[i]; - // update the valid bytes - for (i = func_offset; i < func_offset + overlap_size; ++i) - bytesValid[i] = true; + // update the valid bytes + for (i = func_offset; i < func_offset + overlap_size; ++i) + bytesValid[i] = true; - // check the bit after the update we just made - for (; all_bytes_valid && i < getSize(); ++i) - all_bytes_valid &= bytesValid[i]; + // check the bit after the update we just made + for (; all_bytes_valid && i < getSize(); ++i) + all_bytes_valid &= bytesValid[i]; - return all_bytes_valid; - } + return all_bytes_valid; } else if (isWrite()) { - if (offset >= 0) { - memcpy(_data + offset, getConstPtr<uint8_t>(), - (min(func_end, val_end) - func_start) + 1); - } else { - // val_start > func_start - memcpy(_data, getConstPtr<uint8_t>() - offset, - (min(func_end, val_end) - val_start) + 1); - } + memcpy(_data + val_offset, + getConstPtr<uint8_t>() + func_offset, + overlap_size); } else { panic("Don't know how to handle command %s\n", cmdString()); } |