[gem5-dev] [S] Change in gem5/gem5[develop]: arch-riscv: Add VS field to the STATUS CSR

2022-11-03 Thread Hoa Nguyen (Gerrit) via gem5-dev 
Hoa Nguyen has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/65274?usp=email )



Change subject: arch-riscv: Add VS field to the STATUS CSR
..

arch-riscv: Add VS field to the STATUS CSR

Per RISC-V ISA Manual, vol II, section 3.1.6, page 20, the VS field
is located at bits 10..9 of mstatus. Per section 4.1.1, page 63,
the VS field is located at the same bits of sstatus.

Change-Id: Ifda1c551a23ed892fb8ac7ef31fa98f0b6db
Signed-off-by: Hoa Nguyen 
---
M src/arch/riscv/regs/misc.hh
1 file changed, 26 insertions(+), 10 deletions(-)



diff --git a/src/arch/riscv/regs/misc.hh b/src/arch/riscv/regs/misc.hh
index cb8c907..5f07447 100644
--- a/src/arch/riscv/regs/misc.hh
+++ b/src/arch/riscv/regs/misc.hh
@@ -562,6 +562,7 @@
 Bitfield<16, 15> xs;
 Bitfield<14, 13> fs;
 Bitfield<12, 11> mpp;
+Bitfield<10, 9> vs;
 Bitfield<8> spp;
 Bitfield<7> mpie;
 Bitfield<5> spie;
@@ -612,6 +613,7 @@
 const RegVal STATUS_XS_MASK = 3ULL << 15;
 const RegVal STATUS_FS_MASK = 3ULL << FS_OFFSET;
 const RegVal STATUS_MPP_MASK = 3ULL << 11;
+const RegVal STATUS_VS_MASK = 3ULL << 9;
 const RegVal STATUS_SPP_MASK = 1ULL << 8;
 const RegVal STATUS_MPIE_MASK = 1ULL << 7;
 const RegVal STATUS_SPIE_MASK = 1ULL << 5;
@@ -624,21 +626,21 @@
 STATUS_TW_MASK | STATUS_TVM_MASK |
 STATUS_MXR_MASK | STATUS_SUM_MASK |
 STATUS_MPRV_MASK | STATUS_XS_MASK |
-STATUS_FS_MASK | STATUS_MPP_MASK |
-STATUS_SPP_MASK | STATUS_MPIE_MASK |
-STATUS_SPIE_MASK | STATUS_UPIE_MASK |
-STATUS_MIE_MASK | STATUS_SIE_MASK |
-STATUS_UIE_MASK;
+STATUS_FS_MASK | STATUS_VS_MASK |
+STATUS_MPP_MASK | STATUS_SPP_MASK |
+STATUS_MPIE_MASK | STATUS_SPIE_MASK |
+STATUS_UPIE_MASK | STATUS_MIE_MASK |
+STATUS_SIE_MASK | STATUS_UIE_MASK;
 const RegVal SSTATUS_MASK = STATUS_SD_MASK | STATUS_UXL_MASK |
 STATUS_MXR_MASK | STATUS_SUM_MASK |
 STATUS_XS_MASK | STATUS_FS_MASK |
-STATUS_SPP_MASK | STATUS_SPIE_MASK |
-STATUS_UPIE_MASK | STATUS_SIE_MASK |
-STATUS_UIE_MASK;
+STATUS_VS_MASK | STATUS_SPP_MASK |
+STATUS_SPIE_MASK | STATUS_UPIE_MASK |
+STATUS_SIE_MASK | STATUS_UIE_MASK;
 const RegVal USTATUS_MASK = STATUS_SD_MASK | STATUS_MXR_MASK |
 STATUS_SUM_MASK | STATUS_XS_MASK |
-STATUS_FS_MASK | STATUS_UPIE_MASK |
-STATUS_UIE_MASK;
+STATUS_FS_MASK | STATUS_VS_MASK |
+STATUS_UPIE_MASK | STATUS_UIE_MASK;

 const RegVal MEI_MASK = 1ULL << 11;
 const RegVal SEI_MASK = 1ULL << 9;

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/65274?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ifda1c551a23ed892fb8ac7ef31fa98f0b6db
Gerrit-Change-Number: 65274
Gerrit-PatchSet: 1
Gerrit-Owner: Hoa Nguyen 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [M] Change in gem5/gem5[develop]: arch-riscv: Updating the SD bit of mstatus upon the register read

2022-11-03 Thread Hoa Nguyen (Gerrit) via gem5-dev 
Hoa Nguyen has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/65273?usp=email )



Change subject: arch-riscv: Updating the SD bit of mstatus upon the  
register read

..

arch-riscv: Updating the SD bit of mstatus upon the register read

Per RISC-V ISA Manual, vol II, section 3.1.6.6, page 26, the SD bit is
a read-only bit indicating whether any of FS, VS, and XS fields being
in the respective dirty state.

Per section 3.1.6, page 20, the SD bit is the most significant bit of
the mstatus register for both RV32 and RV64.

Per section 3.1.6.6, page 29, the explicit formula for updating the SD is,
SD = ((FS==DIRTY) | (XS==DIRTY) | (VS==DIRTY))

Previously in gem5, this bit is not updated anywhere in the gem5
implementation. This cause an issue of incorrectly saving the context
before entering the system call and consequently, incorecttly restoring
the context after a system call as described here [1].

Ideally, we want to update the SD after every relevant instruction;
however, lazily updating the Status register upon its read produces
the same effect.

[1] https://gem5-review.googlesource.com/c/public/gem5/+/65272/

Change-Id: I1db0cc619d43bc5bacb1d03f6f214345d9d90e28
Signed-off-by: Hoa Nguyen 
---
M src/arch/riscv/isa.cc
1 file changed, 57 insertions(+), 0 deletions(-)



diff --git a/src/arch/riscv/isa.cc b/src/arch/riscv/isa.cc
index e215e24..15a0beb 100644
--- a/src/arch/riscv/isa.cc
+++ b/src/arch/riscv/isa.cc
@@ -348,6 +348,32 @@
 else
 return mbits(val, 63, 1);
 }
+  case MISCREG_STATUS:
+{
+// Updating the SD bit.
+// . Per RISC-V ISA Manual, vol II, section 3.1.6.6, page 26,
+// the SD bit is a read-only bit indicating whether any of
+// FS, VS, and XS fields being in the respective dirty state.
+// . Per section 3.1.6, page 20, the SD bit is the most
+// significant bit of the MSTATUS CSR for both RV32 and RV64.
+// . Per section 3.1.6.6, page 29, the explicit formula for
+// updating the SD is,
+//   SD = ((FS==DIRTY) | (XS==DIRTY) | (VS==DIRTY))
+// . Ideally, we want to update the SD after every relevant
+// instruction, however, lazily updating the Status register
+// upon its read produces the same effect as well.
+auto status = readMiscRegNoEffect(idx);
+uint64_t xs_bits = bits(status, 16, 15);
+uint64_t fs_bits = bits(status, 14, 13);
+uint64_t vs_bits = bits(status, 10, 9);
+uint64_t sd_bit = \
+(xs_bits == 3) | (fs_bits == 3) | (vs_bits == 3);
+// We assume RV64 here, updating the SD bit at index 63.
+replaceBits(status, 63, 63, sd_bit);
+setMiscReg(idx, status)
+
+return readMiscRegNoEffect(idx);
+}
   default:
 // Try reading HPM counters
 // As a placeholder, all HPM counters are just cycle counters

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/65273?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I1db0cc619d43bc5bacb1d03f6f214345d9d90e28
Gerrit-Change-Number: 65273
Gerrit-PatchSet: 1
Gerrit-Owner: Hoa Nguyen 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [S] Change in gem5/gem5[develop]: arch-riscv: Update FS field of mstatus register where approriate.

2022-11-03 Thread Hoa Nguyen (Gerrit) via gem5-dev 
Hoa Nguyen has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/65272?usp=email )



Change subject: arch-riscv: Update FS field of mstatus register where  
approriate.

..

arch-riscv: Update FS field of mstatus register where approriate.

Per RISC-V ISA Manual, vol II, section 3.1.6.6, page 25, the
FS field of the mstatus register encodes the status of the floating
point unit, including the floating point registers. Per page 27,
microarchitecture can choose to set the FS field to Dirty even if
the floating point unit has not been modified.

Per section 3.1.6, page 20, the FS field is located at bits 14..13
of the mstatus register.

Per section 3.1.6.6, page 27, the FS field is used for saving
context.

Upon a system call, the Linux kernel relies on mstatus for
choosing registers to save for switching to kernel code.
In particular, if the SD bit (updating this bit is also a bug
in gem5 and will be explained in the next commit) is not set
properly due to the FS field is incorrect, the process of saving
the context and restoring the context result in the floating
point registers being zeroed out. I.e., upon the saving context
function call, the floating point registers are not saved, while
in restore context function call, the floating point registers
are overwritten with zero bits.

Previously, in gem5 RISC-V ISA, the FS field is not updated upon
floating point instruction execution. This caused issue on context
saving described above.

This change conservatively updates the FS field to Dirty on
the execution of any floating point instruction.

Change-Id: I8b3b4922e8da483cff3a2210ee80c163cace182a
Signed-off-by: Hoa Nguyen 
---
M src/arch/riscv/isa/formats/fp.isa
1 file changed, 43 insertions(+), 0 deletions(-)



diff --git a/src/arch/riscv/isa/formats/fp.isa  
b/src/arch/riscv/isa/formats/fp.isa

index 65e81cd..d0bd245 100644
--- a/src/arch/riscv/isa/formats/fp.isa
+++ b/src/arch/riscv/isa/formats/fp.isa
@@ -40,6 +40,9 @@
 if (status.fs == FPUStatus::OFF)
 return std::make_shared("FPU is off",  
machInst);


+status.fs = FPUStatus::DIRTY;
+xc->setMiscReg(MISCREG_STATUS, status);
+
 %(op_decl)s;
 %(op_rd)s;


--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/65272?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I8b3b4922e8da483cff3a2210ee80c163cace182a
Gerrit-Change-Number: 65272
Gerrit-PatchSet: 1
Gerrit-Owner: Hoa Nguyen 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [S] Change in gem5/gem5[develop]: stdlib: Add __repr__ to pystats

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/63271?usp=email )


 (

9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: stdlib: Add __repr__ to pystats
..

stdlib: Add __repr__ to pystats

For Statistics the value is returned. E.g.:

```
print(simstats.board.core.some_integer)

5

```

For Groups the names of the stats in that group are listed.
E.g.:

```
print(stats.board.core)

[Group: [some_integer, another_stat, another_group]]

```

Change-Id: I94cea907608fba622f4fc141d5b22ac95d8cde40
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/63271
Tested-by: kokoro 
Maintainer: Bobby Bruce 
Reviewed-by: Bobby Bruce 
---
M src/python/m5/ext/pystats/group.py
M src/python/m5/ext/pystats/statistic.py
2 files changed, 44 insertions(+), 1 deletion(-)

Approvals:
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/python/m5/ext/pystats/group.py  
b/src/python/m5/ext/pystats/group.py

index 6e2da87..680a5d5 100644
--- a/src/python/m5/ext/pystats/group.py
+++ b/src/python/m5/ext/pystats/group.py
@@ -56,7 +56,7 @@
 time_conversion: Optional[TimeConversion] = None,
 **kwargs: Dict[
 str, Union["Group", Statistic, List["Group"],  
List["Statistic"]]

-]
+],
 ):
 if type is None:
 self.type = "Group"
@@ -140,6 +140,15 @@
 pattern = regex
 yield from self.children(lambda _name: bool(pattern.search(_name)))

+def _repr_name(self) -> str:
+return "Group"
+
+def __repr__(self) -> str:
+stats_list = []
+for key in self.__dict__:
+stats_list.append(key)
+return f"{self._repr_name()}: {stats_list}"
+

 class Vector(Group):
 """
@@ -152,3 +161,6 @@

 def __init__(self, scalar_map: Mapping[str, Scalar]):
 super().__init__(type="Vector", time_conversion=None, **scalar_map)
+
+def _repr_name(self) -> str:
+return "Vector"
diff --git a/src/python/m5/ext/pystats/statistic.py  
b/src/python/m5/ext/pystats/statistic.py

index 446c9ba..b018060 100644
--- a/src/python/m5/ext/pystats/statistic.py
+++ b/src/python/m5/ext/pystats/statistic.py
@@ -56,6 +56,9 @@
 self.description = description
 self.datatype = datatype

+def __repr__(self):
+return str(self.value)
+

 class Scalar(Statistic):
 """

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/63271?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I94cea907608fba622f4fc141d5b22ac95d8cde40
Gerrit-Change-Number: 63271
Gerrit-PatchSet: 14
Gerrit-Owner: Bobby Bruce 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [M] Change in gem5/gem5[develop]: python: Move find from group to AbstractStat

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/63692?usp=email )


Change subject: python: Move find from group to AbstractStat
..

python: Move find from group to AbstractStat

This expands the 'find' feature to be recursive and find all the
stats/groups of stats of that regex all the way down the SimStats tree.

Change-Id: Id888911a6189e0440d2537f9720aa594353e00c7
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/63692
Maintainer: Bobby Bruce 
Reviewed-by: Bobby Bruce 
Tested-by: kokoro 
---
M src/python/m5/ext/pystats/abstract_stat.py
M src/python/m5/ext/pystats/group.py
2 files changed, 77 insertions(+), 86 deletions(-)

Approvals:
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/python/m5/ext/pystats/abstract_stat.py  
b/src/python/m5/ext/pystats/abstract_stat.py

index 511ee36..f2a75fc 100644
--- a/src/python/m5/ext/pystats/abstract_stat.py
+++ b/src/python/m5/ext/pystats/abstract_stat.py
@@ -26,6 +26,15 @@

 from .serializable_stat import SerializableStat

+import re
+from typing import (
+Callable,
+List,
+Optional,
+Pattern,
+Union,
+)
+

 class AbstractStat(SerializableStat):
 """
@@ -34,4 +43,55 @@
 All PyStats are JsonSerializable.
 """

-pass
+def children(
+self,
+predicate: Optional[Callable[[str], bool]] = None,
+recursive: bool = False,
+) -> List["AbstractStat"]:
+"""Iterate through all of the children, optionally with a predicate
+
+```
+>>> system.children(lambda _name: 'cpu' in name)
+[cpu0, cpu1, cpu2]
+```
+
+:param: predicate(str) -> bool: Optional. Each child's name is  
passed

+to this function. If it returns true, then the child is
+yielded. Otherwise, the child is skipped.
+If not provided then all children are returned.
+"""
+
+to_return = []
+for attr in self.__dict__:
+obj = getattr(self, attr)
+if isinstance(obj, AbstractStat):
+if (predicate and predicate(attr)) or not predicate:
+to_return.append(obj)
+if recursive:
+to_return = to_return + obj.children(
+predicate=predicate, recursive=True
+)
+
+return to_return
+
+def find(self, regex: Union[str, Pattern]) -> List["AbstractStat"]:
+"""Find all stats that match the name, recursively through all the
+SimStats.
+
+
+```
+>>> system.find('cpu[0-9]')
+[cpu0, cpu1, cpu2]
+```
+Note: The above will not match `cpu_other`.
+
+:param: regex: The regular expression used to search. Can be a
+precompiled regex or a string in regex format
+"""
+if isinstance(regex, str):
+pattern = re.compile(regex)
+else:
+pattern = regex
+return self.children(
+lambda _name: re.match(pattern, _name), recursive=True
+)
diff --git a/src/python/m5/ext/pystats/group.py  
b/src/python/m5/ext/pystats/group.py

index 7fcd665..0b71663 100644
--- a/src/python/m5/ext/pystats/group.py
+++ b/src/python/m5/ext/pystats/group.py
@@ -24,15 +24,11 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-import re
 from typing import (
-Callable,
 Dict,
-Iterator,
 List,
 Mapping,
 Optional,
-Pattern,
 Union,
 )

@@ -68,87 +64,6 @@
 for key, value in kwargs.items():
 setattr(self, key, value)

-def children(
-self, predicate: Optional[Callable[[str], bool]] = None
-) -> Iterator[Union["Group", Statistic]]:
-"""Iterate through all of the children, optionally with a predicate
-
-```
->>> system.children(lambda _name: 'cpu' in name)
-[cpu0, cpu1, cpu2]
-```
-
-:param: predicate(str) -> bool: Optional. Each child's name is  
passed

-to this function. If it returns true, then the child is
-yielded. Otherwise, the child is skipped.
-If not provided then all children are returned.
-"""
-for attr in self.__dict__:
-# Check the provided predicate. If not a match, skip this child
-if predicate and not predicate(attr):
-continue
-obj = getattr(self, attr)
-if isinstance(obj, Group) or isinstance(obj, Statistic):
-yield obj
-
-def find(self, name: str) -> Iterator[Union["Group", Statistic]]:
-"""Find all stats that match the name
-
-This function searches all of the "children" in this group. It  
yields
-the set of

[gem5-dev] [S] Change in gem5/gem5[develop]: misc: Update black to process src/python/m5/ext/pystats

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/63711?usp=email )


 (

9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: misc: Update black to process src/python/m5/ext/pystats
..

misc: Update black to process src/python/m5/ext/pystats

The exclusion in .pre-commit-config.yaml covered all files in
src/python/m5/ext. This excludes src/python/m5/exit/pystats, which we
want covered by black. This commit updates .pre-commit-config.yaml to
only exclude src/python/m5/ext/pyfdt.

This change also runs black on these files.

Change-Id: Iecff45ea2a27a37fc0d00b867d41300aad911c7a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/63711
Tested-by: kokoro 
Reviewed-by: Bobby Bruce 
Maintainer: Bobby Bruce 
---
M .pre-commit-config.yaml
M src/python/m5/ext/pystats/group.py
M src/python/m5/ext/pystats/serializable_stat.py
M src/python/m5/ext/pystats/simstat.py
4 files changed, 27 insertions(+), 4 deletions(-)

Approvals:
  kokoro: Regressions pass
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved




diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 9fcca88..8cbc6af 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -44,7 +44,7 @@
 build/.*|
 src/systemc/ext/.*|
 src/systemc/tests/.*/.*|
-src/python/m5/ext/.*|
+src/python/m5/ext/pyfdt/.*|
 tests/.*/ref/.*
   )$

diff --git a/src/python/m5/ext/pystats/group.py  
b/src/python/m5/ext/pystats/group.py

index 55ec4b2..6e2da87 100644
--- a/src/python/m5/ext/pystats/group.py
+++ b/src/python/m5/ext/pystats/group.py
@@ -40,6 +40,7 @@
 from .statistic import Scalar, Statistic
 from .timeconversion import TimeConversion

+
 class Group(SerializableStat):
 """
 Used to create the heirarchical stats structure. A Group object  
contains a

@@ -70,7 +71,7 @@
 def children(
 self, predicate: Optional[Callable[[str], bool]] = None
 ) -> Iterator[Union["Group", Statistic]]:
-""" Iterate through all of the children, optionally with a  
predicate

+"""Iterate through all of the children, optionally with a predicate

 ```
 >>> system.children(lambda _name: 'cpu' in name)
@@ -91,7 +92,7 @@
 yield obj

 def find(self, name: str) -> Iterator[Union["Group", Statistic]]:
-""" Find all stats that match the name
+"""Find all stats that match the name

 This function searches all of the "children" in this group. It  
yields
 the set of attributes (children) that have the `name` as a  
substring.

@@ -117,7 +118,7 @@
 def find_re(
 self, regex: Union[str, Pattern]
 ) -> Iterator[Union["Group", Statistic]]:
-""" Find all stats that match the name
+"""Find all stats that match the name

 This function searches all of the "children" in this group. It  
yields

 the set of attributes (children) that have the `name` mathing the
diff --git a/src/python/m5/ext/pystats/serializable_stat.py  
b/src/python/m5/ext/pystats/serializable_stat.py

index 3ad9b50..c4de181 100644
--- a/src/python/m5/ext/pystats/serializable_stat.py
+++ b/src/python/m5/ext/pystats/serializable_stat.py
@@ -30,6 +30,7 @@

 from .storagetype import StorageType

+
 class SerializableStat:
 """
 Classes which inherit from SerializableStat can be serialized as JSON
diff --git a/src/python/m5/ext/pystats/simstat.py  
b/src/python/m5/ext/pystats/simstat.py

index 3cea133..bab47cc 100644
--- a/src/python/m5/ext/pystats/simstat.py
+++ b/src/python/m5/ext/pystats/simstat.py
@@ -32,6 +32,7 @@
 from .statistic import Statistic
 from .timeconversion import TimeConversion

+
 class SimStat(SerializableStat):
 """
 Contains all the statistics for a given simulation.

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/63711?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Iecff45ea2a27a37fc0d00b867d41300aad911c7a
Gerrit-Change-Number: 63711
Gerrit-PatchSet: 11
Gerrit-Owner: Bobby Bruce 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [S] Change in gem5/gem5[develop]: stdlib: Add 'get_simstats' function to simulator

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/63272?usp=email )


 (

7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: stdlib: Add 'get_simstats' function to simulator
..

stdlib: Add 'get_simstats' function to simulator

Change-Id: Iedf937a66f33c5a5feada4ffbf550540f65680d1
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/63272
Tested-by: kokoro 
Maintainer: Jason Lowe-Power 
Reviewed-by: Jason Lowe-Power 
---
M src/python/gem5/simulate/simulator.py
1 file changed, 29 insertions(+), 4 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/python/gem5/simulate/simulator.py  
b/src/python/gem5/simulate/simulator.py

index 5a4ab17..8be915e 100644
--- a/src/python/gem5/simulate/simulator.py
+++ b/src/python/gem5/simulate/simulator.py
@@ -27,7 +27,7 @@
 import m5
 import m5.ticks
 from m5.stats import addStatVisitor
-from m5.stats.gem5stats import get_simstat
+from m5.ext.pystats.simstat import SimStat
 from m5.objects import Root
 from m5.util import warn

@@ -275,8 +275,20 @@
 Obtain the current simulation statistics as a Dictionary,  
conforming

 to a JSON-style schema.

-**Warning:** Will throw an Exception if called before `run()`. The
-board must be initialized before obtaining statistics
+:raises Exception: An exception is raised if this function is  
called

+before `run()`. The board must be initialized before obtaining
+statistics.
+"""
+
+return self.get_simstats().to_json()
+
+def get_simstats(self) -> SimStat:
+"""
+Obtains the SimStat of the current simulation.
+
+:raises Exception: An exception is raised if this function is  
called

+before `run()`. The board must be initialized before obtaining
+statistics.
 """

 if not self._instantiated:
@@ -284,7 +296,7 @@
 "Cannot obtain simulation statistics prior to  
inialization."

 )

-return get_simstat(self._root).to_json()
+return m5.stats.gem5stats.get_simstat(self._root)

 def add_text_stats_output(self, path: str) -> None:
 """

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/63272?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Iedf937a66f33c5a5feada4ffbf550540f65680d1
Gerrit-Change-Number: 63272
Gerrit-PatchSet: 15
Gerrit-Owner: Bobby Bruce 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [M] Change in gem5/gem5[develop]: tests,stdlib: Add a test for JsonSerializable

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/58729?usp=email )


 (

4 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: tests,stdlib: Add a test for JsonSerializable
..

tests,stdlib: Add a test for JsonSerializable

Change-Id: I9e1fa6ee67f5d73d41fa9972bdb9a3da7dda8957
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/58729
Maintainer: Jason Lowe-Power 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
A tests/pyunit/pyunit_jsonserializable_check.py
1 file changed, 84 insertions(+), 0 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/tests/pyunit/pyunit_jsonserializable_check.py  
b/tests/pyunit/pyunit_jsonserializable_check.py

new file mode 100644
index 000..97642a4
--- /dev/null
+++ b/tests/pyunit/pyunit_jsonserializable_check.py
@@ -0,0 +1,71 @@
+# Copyright (c) 2022 The Regents of The University of California
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met: redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer;
+# redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution;
+# neither the name of the copyright holders nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest
+from m5.ext.pystats.jsonserializable import JsonSerializable
+
+
+class MockSerializable(JsonSerializable):
+def __init__(self):
+self.child_1 = MockSerializableChild()
+self.child_1.stat1 = 2
+self.child_1.stat2 = "3"
+self.child_list = []
+
+child_list_1 = MockSerializableChild()
+child_list_1.stat1 = "hello"
+self.child_list.append(child_list_1)
+child_list_2 = MockSerializableChild()
+child_list_2.list_stat2 = ["1", 2, "3", 4, 5.2, None]
+self.child_list.append(child_list_2)
+
+
+class MockSerializableChild(JsonSerializable):
+def __init__(self):
+pass
+
+
+class JsonSerializableTestSuite(unittest.TestCase):
+def test_to_json(self):
+obj = MockSerializable()
+obj_json = obj.to_json()
+self.assertTrue("child_1" in obj_json)
+self.assertTrue("stat1" in obj_json["child_1"])
+self.assertEquals(2, obj_json["child_1"]["stat1"])
+self.assertTrue("stat2" in obj_json["child_1"])
+self.assertEquals("3", obj_json["child_1"]["stat2"])
+self.assertTrue("child_list" in obj_json)
+self.assertEquals(2, len(obj_json["child_list"]))
+self.assertTrue("stat1" in obj_json["child_list"][0])
+self.assertEqual("hello", obj_json["child_list"][0]["stat1"])
+self.assertTrue("list_stat2" in obj_json["child_list"][1])
+self.assertEquals(6, len(obj_json["child_list"][1]["list_stat2"]))
+self.assertEquals("1", obj_json["child_list"][1]["list_stat2"][0])
+self.assertEquals(2, obj_json["child_list"][1]["list_stat2"][1])
+self.assertEquals("3", obj_json["child_list"][1]["list_stat2"][2])
+self.assertEquals(4, obj_json["child_list"][1]["list_stat2"][3])
+self.assertEquals(5.2, obj_json["child_list"][1]["list_stat2"][4])
+self.assertEquals(None, obj_json["child_list"][1]["list_stat2"][5])

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/58729?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I9e1fa6ee67f5d73d41fa9972bdb9a3da7dda8957
Gerrit-Change-Number: 58729
Gerrit-PatchSet: 7
Gerrit-Owner: Bobby Bruce

[gem5-dev] [M] Change in gem5/gem5[develop]: stdlib: Rename JsonSerializable to SerializableStat

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/59273?usp=email )


 (

10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: stdlib: Rename JsonSerializable to SerializableStat
..

stdlib: Rename JsonSerializable to SerializableStat

As this abstract class now allows the output of text stats, it's more
appropriate to rename it. It no longer handles processing just for JSON
output

Change-Id: Ia9a1e3ef4029de45a11ac261fb14c9bdfa412cdd
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/59273
Reviewed-by: Bobby Bruce 
Tested-by: kokoro 
Maintainer: Bobby Bruce 
---
M src/python/SConscript
M src/python/m5/ext/pystats/__init__.py
M src/python/m5/ext/pystats/group.py
R src/python/m5/ext/pystats/serializable_stat.py
M src/python/m5/ext/pystats/simstat.py
M src/python/m5/ext/pystats/statistic.py
M tests/pyunit/pyunit_jsonserializable_check.py
7 files changed, 33 insertions(+), 19 deletions(-)

Approvals:
  kokoro: Regressions pass
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved




diff --git a/src/python/SConscript b/src/python/SConscript
index 6e0f6d7..f08752a 100644
--- a/src/python/SConscript
+++ b/src/python/SConscript
@@ -283,7 +283,7 @@
 PySource('m5.ext.pyfdt', 'm5/ext/pyfdt/__init__.py')

 PySource('m5.ext.pystats', 'm5/ext/pystats/__init__.py')
-PySource('m5.ext.pystats', 'm5/ext/pystats/jsonserializable.py')
+PySource('m5.ext.pystats', 'm5/ext/pystats/serializable_stat.py')
 PySource('m5.ext.pystats', 'm5/ext/pystats/group.py')
 PySource('m5.ext.pystats', 'm5/ext/pystats/simstat.py')
 PySource('m5.ext.pystats', 'm5/ext/pystats/statistic.py')
diff --git a/src/python/m5/ext/pystats/__init__.py  
b/src/python/m5/ext/pystats/__init__.py

index 04d7d11..942979a 100644
--- a/src/python/m5/ext/pystats/__init__.py
+++ b/src/python/m5/ext/pystats/__init__.py
@@ -24,7 +24,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-from .jsonserializable import JsonSerializable
+from .serializable_stat import SerializableStat
 from .group import Group
 from .simstat import SimStat
 from .statistic import Statistic
@@ -38,6 +38,6 @@
 "Statistic",
 "TimeConversion",
 "StorageType",
-"JsonSerializable",
+"SerializableStat",
 "JsonLoader",
 ]
diff --git a/src/python/m5/ext/pystats/group.py  
b/src/python/m5/ext/pystats/group.py

index c71f291..55ec4b2 100644
--- a/src/python/m5/ext/pystats/group.py
+++ b/src/python/m5/ext/pystats/group.py
@@ -36,12 +36,11 @@
 Union,
 )

-from .jsonserializable import JsonSerializable
+from .serializable_stat import SerializableStat
 from .statistic import Scalar, Statistic
 from .timeconversion import TimeConversion

-
-class Group(JsonSerializable):
+class Group(SerializableStat):
 """
 Used to create the heirarchical stats structure. A Group object  
contains a
 map of labeled  Groups, Statistics, Lists of Groups, or List of  
Statistics.
diff --git a/src/python/m5/ext/pystats/jsonserializable.py  
b/src/python/m5/ext/pystats/serializable_stat.py

similarity index 96%
rename from src/python/m5/ext/pystats/jsonserializable.py
rename to src/python/m5/ext/pystats/serializable_stat.py
index 63cb16f..3ad9b50 100644
--- a/src/python/m5/ext/pystats/jsonserializable.py
+++ b/src/python/m5/ext/pystats/serializable_stat.py
@@ -30,11 +30,10 @@

 from .storagetype import StorageType

-
-class JsonSerializable:
+class SerializableStat:
 """
-Classes which inherit from JsonSerializable can be translated into JSON
-using Python's json package.
+Classes which inherit from SerializableStat can be serialized as JSON
+output.

 Usage
 -
@@ -80,7 +79,7 @@
 A value which can be handled by the Python stdlib JSON package.
 """

-if isinstance(value, JsonSerializable):
+if isinstance(value, SerializableStat):
 return value.to_json()
 elif isinstance(value, (str, int, float)):
 return value
diff --git a/src/python/m5/ext/pystats/simstat.py  
b/src/python/m5/ext/pystats/simstat.py

index 5a48009..3cea133 100644
--- a/src/python/m5/ext/pystats/simstat.py
+++ b/src/python/m5/ext/pystats/simstat.py
@@ -27,13 +27,12 @@
 from datetime import datetime
 from typing import Dict, List, Optional, Union

-from .jsonserializable import JsonSerializable
+from .serializable_stat import SerializableStat
 from .group import Group
 from .statistic import Statistic
 from .timeconversion import TimeConversion

-
-class SimStat(JsonSerializable):
+class SimStat(SerializableStat):
 """
 Contains all the statistics for a given simulation.
 """
diff --git a/src/python/m5/ext/pystats/statistic.py  
b/src/python/m5/ext/pystats/statistic.py

index

[gem5-dev] [M] Change in gem5/gem5[develop]: python: Add AbstractStat for PyStats

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/63691?usp=email )


Change subject: python: Add AbstractStat for PyStats
..

python: Add AbstractStat for PyStats

Previously all PyStats inheritted from JsonSerializable. The
AbstractStat class has been added to give a cleaner, clearer Base class
for PyStats.

Change-Id: I7e1808c4b4dcd6110fd524ad3553a9dc19f72e24
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/63691
Reviewed-by: Bobby Bruce 
Maintainer: Bobby Bruce 
Tested-by: kokoro 
---
M src/python/SConscript
M src/python/m5/ext/pystats/__init__.py
A src/python/m5/ext/pystats/abstract_stat.py
M src/python/m5/ext/pystats/group.py
M src/python/m5/ext/pystats/simstat.py
M src/python/m5/ext/pystats/statistic.py
6 files changed, 63 insertions(+), 6 deletions(-)

Approvals:
  kokoro: Regressions pass
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved




diff --git a/src/python/SConscript b/src/python/SConscript
index f08752a..66e9842 100644
--- a/src/python/SConscript
+++ b/src/python/SConscript
@@ -284,6 +284,7 @@

 PySource('m5.ext.pystats', 'm5/ext/pystats/__init__.py')
 PySource('m5.ext.pystats', 'm5/ext/pystats/serializable_stat.py')
+PySource('m5.ext.pystats', 'm5/ext/pystats/abstract_stat.py')
 PySource('m5.ext.pystats', 'm5/ext/pystats/group.py')
 PySource('m5.ext.pystats', 'm5/ext/pystats/simstat.py')
 PySource('m5.ext.pystats', 'm5/ext/pystats/statistic.py')
diff --git a/src/python/m5/ext/pystats/__init__.py  
b/src/python/m5/ext/pystats/__init__.py

index 942979a..32cee43 100644
--- a/src/python/m5/ext/pystats/__init__.py
+++ b/src/python/m5/ext/pystats/__init__.py
@@ -24,6 +24,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

+from .abstract_stat import AbstractStat
 from .serializable_stat import SerializableStat
 from .group import Group
 from .simstat import SimStat
@@ -33,6 +34,7 @@
 from .jsonloader import JsonLoader

 __all__ = [
+"AbstractStat",
 "Group",
 "SimStat",
 "Statistic",
diff --git a/src/python/m5/ext/pystats/abstract_stat.py  
b/src/python/m5/ext/pystats/abstract_stat.py

new file mode 100644
index 000..511ee36
--- /dev/null
+++ b/src/python/m5/ext/pystats/abstract_stat.py
@@ -0,0 +1,37 @@
+# Copyright (c) 2022 The Regents of The University of California
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met: redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer;
+# redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution;
+# neither the name of the copyright holders nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+from .serializable_stat import SerializableStat
+
+
+class AbstractStat(SerializableStat):
+"""
+An abstract class which all PyStats inherit from.
+
+All PyStats are JsonSerializable.
+"""
+
+pass
diff --git a/src/python/m5/ext/pystats/group.py  
b/src/python/m5/ext/pystats/group.py

index 680a5d5..7fcd665 100644
--- a/src/python/m5/ext/pystats/group.py
+++ b/src/python/m5/ext/pystats/group.py
@@ -36,12 +36,12 @@
 Union,
 )

-from .serializable_stat import SerializableStat
+from .abstract_stat import AbstractStat
 from .statistic import Scalar, Statistic
 from .timeconversion import TimeConversion


-class Group(SerializableStat):
+class Group(AbstractStat):
 """
 Used to create the heirarchical stats structure. A Group object  
contains a
 map of labeled  Groups, Statistics, Lists of Groups, or List of  
Statistics.
diff --git a/src/python/m5/ext/pystats/simstat.py  
b/src/python/m5/ext/pystats/simstat.py

index bab47cc..c7c28f4 100644
---

[gem5-dev] [M] Change in gem5/gem5[develop]: tests, resources: CVE-2007-4559 Patch

2022-11-03 Thread Bobby Bruce (Gerrit) via gem5-dev 
Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/65271?usp=email )


Change subject: tests, resources: CVE-2007-4559 Patch
..

tests, resources: CVE-2007-4559 Patch

Hi, we are security researchers from the Advanced Research Center at  
Trellix.

We have began a campaign to patch a widespread bug named CVE-2007-4559.
CVE-2007-4559 is a 15 year old bug in the Python tarfile package. By using
extract() or extractall() on a tarfile object without sanitizing input,
a maliciously crafted .tar file could perform a directory path traversal
attack. We found at least one unsantized extractall() in your codebase
and are providing a patch for you via pull request. The patch essentially
checks to see if all tarfile members will be extracted safely and throws
an exception otherwise. We encourage you to use this patch or your own
solution to secure against CVE-2007-4559.

If you have further questions you may contact us through this
projects lead researcher Kasimir Schulz.

Change-Id: I891ac6652cfbd479aed51d64ef6d4e0fe740e06d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/65271
Reviewed-by: Bobby Bruce 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
Reviewed-by: Jason Lowe-Power 
---
M src/python/gem5/resources/downloader.py
M tests/gem5/fixture.py
2 files changed, 74 insertions(+), 2 deletions(-)

Approvals:
  Bobby Bruce: Looks good to me, approved
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/python/gem5/resources/downloader.py  
b/src/python/gem5/resources/downloader.py

index bc277ee..1fda8d8 100644
--- a/src/python/gem5/resources/downloader.py
+++ b/src/python/gem5/resources/downloader.py
@@ -501,5 +501,28 @@
 )
 unpack_to = download_dest[: -len(tar_extension)]
 with tarfile.open(download_dest) as f:
-f.extractall(unpack_to)
+
+def is_within_directory(directory, target):
+
+abs_directory = os.path.abspath(directory)
+abs_target = os.path.abspath(target)
+
+prefix = os.path.commonprefix([abs_directory,  
abs_target])

+
+return prefix == abs_directory
+
+def safe_extract(
+tar, path=".", members=None, *, numeric_owner=False
+):
+
+for member in tar.getmembers():
+member_path = os.path.join(path, member.name)
+if not is_within_directory(path, member_path):
+raise Exception(
+"Attempted Path Traversal in Tar File"
+)
+
+tar.extractall(path, members,  
numeric_owner=numeric_owner)

+
+safe_extract(f, unpack_to)
 os.remove(download_dest)
diff --git a/tests/gem5/fixture.py b/tests/gem5/fixture.py
index 5d6ae0c..65b5454 100644
--- a/tests/gem5/fixture.py
+++ b/tests/gem5/fixture.py
@@ -357,7 +357,28 @@
 import tarfile

 with tarfile.open(self.filename) as tf:
-tf.extractall(self.path)
+
+def is_within_directory(directory, target):
+
+abs_directory = os.path.abspath(directory)
+abs_target = os.path.abspath(target)
+
+prefix = os.path.commonprefix([abs_directory, abs_target])
+
+return prefix == abs_directory
+
+def safe_extract(
+tar, path=".", members=None, *, numeric_owner=False
+):
+
+for member in tar.getmembers():
+member_path = os.path.join(path, member.name)
+if not is_within_directory(path, member_path):
+raise Exception("Attempted Path Traversal in Tar  
File")

+
+tar.extractall(path, members, numeric_owner=numeric_owner)
+
+safe_extract(tf, self.path)

 def _setup(self, testitem):
 # Check to see if there is a file downloaded

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/65271?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I891ac6652cfbd479aed51d64ef6d4e0fe740e06d
Gerrit-Change-Number: 65271
Gerrit-PatchSet: 2
Gerrit-Owner: Melissa Jost 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [M] Change in gem5/gem5[develop]: tests, resources: CVE-2007-4559 Patch

2022-11-03 Thread Melissa Jost (Gerrit) via gem5-dev 
Melissa Jost has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/65271?usp=email )



Change subject: tests, resources: CVE-2007-4559 Patch
..

tests, resources: CVE-2007-4559 Patch

Hi, we are security researchers from the Advanced Research Center at  
Trellix.

We have began a campaign to patch a widespread bug named CVE-2007-4559.
CVE-2007-4559 is a 15 year old bug in the Python tarfile package. By using
extract() or extractall() on a tarfile object without sanitizing input,
a maliciously crafted .tar file could perform a directory path traversal
attack. We found at least one unsantized extractall() in your codebase
and are providing a patch for you via pull request. The patch essentially
checks to see if all tarfile members will be extracted safely and throws
an exception otherwise. We encourage you to use this patch or your own
solution to secure against CVE-2007-4559.

If you have further questions you may contact us through this
projects lead researcher Kasimir Schulz.

Change-Id: I891ac6652cfbd479aed51d64ef6d4e0fe740e06d
---
M src/python/gem5/resources/downloader.py
M tests/gem5/fixture.py
2 files changed, 69 insertions(+), 2 deletions(-)



diff --git a/src/python/gem5/resources/downloader.py  
b/src/python/gem5/resources/downloader.py

index bc277ee..1fda8d8 100644
--- a/src/python/gem5/resources/downloader.py
+++ b/src/python/gem5/resources/downloader.py
@@ -501,5 +501,28 @@
 )
 unpack_to = download_dest[: -len(tar_extension)]
 with tarfile.open(download_dest) as f:
-f.extractall(unpack_to)
+
+def is_within_directory(directory, target):
+
+abs_directory = os.path.abspath(directory)
+abs_target = os.path.abspath(target)
+
+prefix = os.path.commonprefix([abs_directory,  
abs_target])

+
+return prefix == abs_directory
+
+def safe_extract(
+tar, path=".", members=None, *, numeric_owner=False
+):
+
+for member in tar.getmembers():
+member_path = os.path.join(path, member.name)
+if not is_within_directory(path, member_path):
+raise Exception(
+"Attempted Path Traversal in Tar File"
+)
+
+tar.extractall(path, members,  
numeric_owner=numeric_owner)

+
+safe_extract(f, unpack_to)
 os.remove(download_dest)
diff --git a/tests/gem5/fixture.py b/tests/gem5/fixture.py
index 5d6ae0c..65b5454 100644
--- a/tests/gem5/fixture.py
+++ b/tests/gem5/fixture.py
@@ -357,7 +357,28 @@
 import tarfile

 with tarfile.open(self.filename) as tf:
-tf.extractall(self.path)
+
+def is_within_directory(directory, target):
+
+abs_directory = os.path.abspath(directory)
+abs_target = os.path.abspath(target)
+
+prefix = os.path.commonprefix([abs_directory, abs_target])
+
+return prefix == abs_directory
+
+def safe_extract(
+tar, path=".", members=None, *, numeric_owner=False
+):
+
+for member in tar.getmembers():
+member_path = os.path.join(path, member.name)
+if not is_within_directory(path, member_path):
+raise Exception("Attempted Path Traversal in Tar  
File")

+
+tar.extractall(path, members, numeric_owner=numeric_owner)
+
+safe_extract(tf, self.path)

 def _setup(self, testitem):
 # Check to see if there is a file downloaded

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/65271?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I891ac6652cfbd479aed51d64ef6d4e0fe740e06d
Gerrit-Change-Number: 65271
Gerrit-PatchSet: 1
Gerrit-Owner: Melissa Jost 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] Build failed in Jenkins: nightly #406

2022-11-03 Thread jenkins-no-reply--- via gem5-dev 
See 

Changes:

[shunhsingou] util: update termios to replace nl with cr-nl

[Giacomo Travaglini] arch-arm: Fix access permissions for GICv3 cpu registers

[Giacomo Travaglini] arch-arm: Fix GICv3 List register mapping

[Giacomo Travaglini] arch-arm: Remove ISA::haveGICv3CpuIfc method

[Bobby R. Bruce] python,stdlib: Add multiprocessing module

[yuhsingw] ext: upgrade to googletest 1.12.0

[shunhsingou] sim: allow specifying remote gdb port for each workload

[fcrh] mem: Fix SHM server path cleanup logic


--
[...truncated 415.56 KB...]
 [SO Param] m5.objects.PowerISA, PowerISA -> ALL/python/_m5/param_PowerISA.cc
 [SO Param] m5.objects.Compressors, PerfectCompressor -> 
ALL/python/_m5/param_PerfectCompressor.cc
 [SO Param] m5.objects.I82094AA, I82094AA -> ALL/python/_m5/param_I82094AA.cc
 [SO Param] m5.objects.GUPSGen, GUPSGen -> ALL/params/GUPSGen.hh
 [SO Param] m5.objects.Probe, ProbeListenerObject -> 
ALL/python/_m5/param_ProbeListenerObject.cc
 [SO Param] m5.objects.Directory_Controller, Directory_Controller -> 
ALL/python/_m5/param_Directory_Controller.cc
 [SO Param] m5.objects.ReplacementPolicies, WeightedLRURP -> 
ALL/python/_m5/param_WeightedLRURP.cc
 [ CXX] ALL/python/_m5/param_I82094AA.cc -> .o
 [ CXX] ALL/python/_m5/param_GUPSGen.cc -> .o
 [ CXX] ALL/cpu/testers/traffic_gen/gups_gen.cc -> .o
 [SO Param] m5.objects.RiscvTLB, RiscvTLB -> ALL/python/_m5/param_RiscvTLB.cc
 [ENUMDECL] m5.objects.IntelMP, X86IntelMPTriggerMode -> 
ALL/enums/X86IntelMPTriggerMode.hh
 [ CXX] ALL/python/_m5/param_RiscvTLB.cc -> .o
 [SO Param] m5.objects.Gic, ArmInterruptPin -> 
ALL/python/_m5/param_ArmInterruptPin.cc
 [SO Param] m5.objects.BranchPredictor, TAGE_SC_L_TAGE_8KB -> 
ALL/python/_m5/param_TAGE_SC_L_TAGE_8KB.cc
 [ CXX] ALL/python/_m5/param_ArmInterruptPin.cc -> .o
 [SO Param] m5.objects.I2C, I2CDevice -> ALL/params/I2CDevice.hh
 [SO Param] m5.objects.ElasticTrace, ElasticTrace -> ALL/params/ElasticTrace.hh
 [SO Param] m5.objects.DirectoryMemory, RubyDirectoryMemory -> 
ALL/python/_m5/param_RubyDirectoryMemory.cc
 [ CXX] ALL/python/_m5/param_I2CBus.cc -> .o
 [ CXX] ALL/cpu/o3/probe/elastic_trace.cc -> .o
 [ CXX] ALL/dev/i2c/bus.cc -> .o
 [SO Param] m5.objects.NoMali, CustomNoMaliGpu -> ALL/params/CustomNoMaliGpu.hh
 [ CXX] ALL/python/_m5/param_CustomNoMaliGpu.cc -> .o
 [ CXX] ALL/dev/arm/gpu_nomali.cc -> .o
 [SO Param] m5.objects.RealView, Pl050 -> ALL/python/_m5/param_Pl050.cc
 [SO Param] m5.objects.Cache, Cache -> ALL/params/Cache.hh
 [ CXX] ALL/python/_m5/param_Cache.cc -> .o
 [ CXX] ALL/mem/cache/cache.cc -> .o
 [SO Param] m5.objects.PS2, PS2Device -> ALL/params/PS2Device.hh
 [SO Param] m5.objects.ClockDomain, DerivedClockDomain -> 
ALL/python/_m5/param_DerivedClockDomain.cc
 [SO Param] m5.objects.TlmBridge, Gem5ToTlmBridge64 -> 
ALL/params/Gem5ToTlmBridge64.hh
 [ CXX] ALL/dev/x86/south_bridge.cc -> .o
 [ CXX] ALL/arch/x86/interrupts.cc -> .o
 [ CXX] ALL/python/_m5/param_Gem5ToTlmBridge64.cc -> .o
 [ CXX] ALL/systemc/tlm_bridge/gem5_to_tlm.cc -> .o
 [ CXX] ALL/python/_m5/param_SouthBridge.cc -> .o
 [ CXX] ALL/dev/x86/i8042.cc -> .o
 [ CXX] ALL/python/_m5/param_I8042.cc -> .o
 [ CXX] ALL/dev/x86/pc.cc -> .o
 [ CXX] ALL/python/_m5/param_Pl050.cc -> .o
 [ CXX] ALL/python/_m5/param_Pc.cc -> .o
 [ CXX] ALL/dev/arm/kmi.cc -> .o
 [SO Param] m5.objects.VirtIOBlock, VirtIOBlock -> 
ALL/python/_m5/param_VirtIOBlock.cc
 [ CXX] ALL/arch/arm/generated/generic_cpu_exec_2.cc -> .o
 [ CXX] ALL/python/_m5/param_VirtIOBlock.cc -> .o
 [ CXX] ALL/arch/arm/isa.cc -> .o
 [ CXX] ALL/arch/arm/generated/decoder.cc -> .o
 [ CXX] ALL/arch/arm/insts/data64.cc -> .o
 [ CXX] ALL/python/_m5/param_ArmTableWalker.cc -> .o
 [ CXX] ALL/arch/arm/generated/generic_cpu_exec_5.cc -> .o
 [ CXX] ALL/arch/arm/insts/sve_mem.cc -> .o
 [ CXX] ALL/arch/arm/decoder.cc -> .o
 [ CXX] ALL/arch/arm/regs/int.cc -> .o
 [ CXX] ALL/arch/arm/table_walker.cc -> .o
 [ CXX] ALL/arch/arm/insts/mem64.cc -> .o
 [ CXX] ALL/arch/arm/insts/branch.cc -> .o
 [ CXX] ALL/arch/arm/tlb.cc -> .o
 [ CXX] ALL/arch/arm/insts/pred_inst.cc -> .o
 [ CXX] ALL/arch/arm/generated/generic_cpu_exec_1.cc -> .o
 [ CXX] ALL/arch/arm/pmu.cc -> .o
 [ CXX] ALL/arch/arm/insts/sve.cc -> .o
 [ CXX] ALL/arch/arm/generated/generic_cpu_exec_4.cc -> .o
 [ CXX] ALL/python/_m5/param_ArmISA.cc -> .o
 [ CXX] ALL/arch/arm/insts/mem.cc -> .o
 [ CXX] ALL/arch/arm/mmu.cc -> .o
 [ CXX] ALL/arch/arm/tracers/tarmac_record_v8.cc -> .o
 [ CXX] ALL/arch/arm/stage2_lookup.cc -> .o
 [ CXX] ALL/arch/arm/insts/misc64.cc -> .o
 [ CXX] ALL/arch/arm/generated/inst-constrs-3.cc -> .o
 [ CXX] ALL/arch/arm/insts/tme64classic.cc -> .o
 [ CXX] ALL/arch/arm/utility.cc -> .o
 [ CXX]

[gem5-dev] [M] Change in gem5/gem5[develop]: mem: Fix SHM server path cleanup logic

2022-11-03 Thread Jui-min Lee (Gerrit) via gem5-dev 
Jui-min Lee has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/65153?usp=email )


Change subject: mem: Fix SHM server path cleanup logic
..

mem: Fix SHM server path cleanup logic

Previously, shared memory server remove old socket *before* filling the
target path into API's data structure. However, the target path might
get truncated hence the path we check against might not be the one we
will be using in the end.

In a case where the path specified by user is free while the truncated
path is in used, gem5 will get a mysterious EADDRINUSE.

We swap the two steps in the CL, so we'll be checking against the actual
path we use, instead of the path user request to use.

Change-Id: Ib34f8b00ea1d2f15dcd4e7b6d2d4a6d6ddc4e411
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/65153
Reviewed-by: Gabe Black 
Tested-by: kokoro 
Maintainer: Gabe Black 
---
M src/mem/shared_memory_server.cc
1 file changed, 59 insertions(+), 35 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/mem/shared_memory_server.cc  
b/src/mem/shared_memory_server.cc

index ae5043f..bee663b 100644
--- a/src/mem/shared_memory_server.cc
+++ b/src/mem/shared_memory_server.cc
@@ -56,47 +56,50 @@
   system(params.system), serverFd(-1)
 {
 fatal_if(system == nullptr, "Requires a system to share memory from!");
-// Ensure the unix socket path to use is not occupied. Also, if there's
-// actually anything to be removed, warn the user something might be  
off.

-if (unlink(unixSocketPath.c_str()) == 0) {
-warn(
-"The server path %s was occupied and will be replaced. Please "
-"make sure there is no other server using the same path.",
-unixSocketPath.c_str());
-}
 // Create a new unix socket.
 serverFd = ListenSocket::socketCloexec(AF_UNIX, SOCK_STREAM, 0);
-panic_if(serverFd < 0, "%s: cannot create unix socket: %s",  
name().c_str(),

+panic_if(serverFd < 0, "%s: cannot create unix socket: %s", name(),
  strerror(errno));
 // Bind to the specified path.
 sockaddr_un serv_addr = {};
 serv_addr.sun_family = AF_UNIX;
 strncpy(serv_addr.sun_path, unixSocketPath.c_str(),
 sizeof(serv_addr.sun_path) - 1);
-warn_if(strlen(serv_addr.sun_path) != unixSocketPath.size(),
-"%s: unix socket path truncated, expect '%s' but get '%s'",
-name().c_str(), unixSocketPath.c_str(), serv_addr.sun_path);
+// If the target path is truncated, warn the user that the actual path  
is

+// different and update the target path.
+if (strlen(serv_addr.sun_path) != unixSocketPath.size()) {
+warn("%s: unix socket path truncated, expect '%s' but get '%s'",
+ name(), unixSocketPath, serv_addr.sun_path);
+unixSocketPath = serv_addr.sun_path;
+}
+// Ensure the unix socket path to use is not occupied. Also, if there's
+// actually anything to be removed, warn the user something might be  
off.

+bool old_sock_removed = unlink(unixSocketPath.c_str()) == 0;
+warn_if(old_sock_removed,
+"%s: the server path %s was occupied and will be replaced.  
Please "

+"make sure there is no other server using the same path.",
+name(), unixSocketPath);
 int bind_retv = bind(serverFd, reinterpret_cast(_addr),
  sizeof(serv_addr));
-fatal_if(bind_retv != 0, "%s: cannot bind unix socket: %s",  
name().c_str(),

+fatal_if(bind_retv != 0, "%s: cannot bind unix socket: %s", name(),
  strerror(errno));
 // Start listening.
 int listen_retv = listen(serverFd, 1);
-fatal_if(listen_retv != 0, "%s: listen failed: %s", name().c_str(),
+fatal_if(listen_retv != 0, "%s: listen failed: %s", name(),
  strerror(errno));
 listenSocketEvent.reset(new ListenSocketEvent(serverFd, this));
 pollQueue.schedule(listenSocketEvent.get());
-inform("%s: listening at %s", name().c_str(), unixSocketPath.c_str());
+inform("%s: listening at %s", name(), unixSocketPath);
 }

 SharedMemoryServer::~SharedMemoryServer()
 {
 int unlink_retv = unlink(unixSocketPath.c_str());
-warn_if(unlink_retv != 0, "%s: cannot unlink unix socket: %s",
-name().c_str(), strerror(errno));
+warn_if(unlink_retv != 0, "%s: cannot unlink unix socket: %s", name(),
+strerror(errno));
 int close_retv = close(serverFd);
-warn_if(close_retv != 0, "%s: cannot close unix socket: %s",
-name().c_str(), strerror(errno));
+warn_if(close_retv != 0, "%s: cannot close unix socket: %s", name(),
+strerror(errno));
 }

 SharedMemoryServer::BaseShmPollEvent::BaseShmPollEvent(
@@ -121,7 +124,7 @@
 if (retv >= 0) {
 offset += retv;
 } else if (errno