Re: [gem5-dev] use of C++ exceptions

2017-05-10 Thread nathan binkert 
This is super historical.  We chose not to use exceptions because we were
worried that they were "slow".  That's largely just not true anymore.
IMHO, exceptions definitely have their place.  They're super useful when
dealing with rare error cases that need to fail a deep call tree.  They can
reduce the number of lines of code by a ton.  There are pitfalls and people
need to know what they're doing (i.e. don't throw in a destructor).

  Nate

On Wed, May 10, 2017 at 9:36 AM, Andreas Sandberg 
wrote:

> Having had a quick look at the code, I'd say that exceptions could
> definitely make sense. I would support limited use of exceptions where
> it makes sense to make the code flow less entangled. Initially, I would
> argue that we should keep exceptions local to SimObjects (interfaces
> like ports should never have to deal with exceptions) and only be used
> for recoverable errors (e.g., unexpected input that should result in a
> warning).
>
> Are there any technical reason why we aren't using exceptions?
>
> //Andreas
>
>
> On 10/05/2017 08:25, Gabe Black wrote:
>
>> This would be internal to the gdb code, ie if a read from the socket
>> fails,
>> it would detach and throw an exception which would unwind back out of all
>> the gdb stuff without having to add ifs all over the place. This bit of
>> code doesn't really have an external interface, so it wouldn't be visible
>> to the caller, assuming a stray exception didn't escape somehow. I think
>> it
>> would be a little nicer that way, but not so much that I'd want to argue
>> for it very strongly.
>>
>> Gabe
>>
>> On Wed, May 10, 2017 at 12:09 AM, Andreas Hansson <
>> andreas.hans...@arm.com>
>> wrote:
>>
>> Hi Gabe,
>>>
>>> I do not think adding exceptions will make things any less cluttered. It
>>> will simply move that complexity to any caller, will it not? I am not a
>>> fan of exceptions in general as it mucks with the control flow.
>>>
>>> Andreas
>>>
>>> On 10/05/2017, 07:31, "gem5-dev on behalf of Gabe Black"
>>>  wrote:
>>>
>>> Hi folks. I have a change to make the GDB stub in gem5 a bit less
 fragile:

 https://gem5-review.googlesource.com/#/c/3180/

 Unfortunately that involved adding a lot of error code checking which
 makes
 things a bit cluttered and ugly. I think it would be a lot nicer to use
 exceptions, but I remember those being a no-no. Are they currently
 against
 the rules, or could I use them to make that code a bit nicer?

 Gabe
 ___
 gem5-dev mailing list
 gem5-dev@gem5.org
 http://m5sim.org/mailman/listinfo/gem5-dev

>>> IMPORTANT NOTICE: The contents of this email and any attachments are
>>> confidential and may also be privileged. If you are not the intended
>>> recipient, please notify the sender immediately and do not disclose the
>>> contents to any other person, use it for any purpose, or store or copy
>>> the
>>> information in any medium. Thank you.
>>> ___
>>> gem5-dev mailing list
>>> gem5-dev@gem5.org
>>> http://m5sim.org/mailman/listinfo/gem5-dev
>>>
>> ___
>> gem5-dev mailing list
>> gem5-dev@gem5.org
>> http://m5sim.org/mailman/listinfo/gem5-dev
>>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
> ___
> gem5-dev mailing list
> gem5-dev@gem5.org
> http://m5sim.org/mailman/listinfo/gem5-dev
>
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Fixes for PyBind issues

2017-05-10 Thread Andreas Sandberg 

Hi Everyone,

Some of our internal users recently ran into various issues related to
the new PyBind wrappers. These are the issues I'm aware of:

  * Events implemented in Python aren't reference counted correctly.
There are cases where the Python side of an event gets deallocated, but
not the native object. The native object gets confused when the event fires.

 * The --debug-start and --debug-stop options broke because of API
changes in Python events.

  * Some uses of vectors of address ranges cause out-of-range memory
accesses in the Python bindings. These sometimes manifest themselves as
a std::bad_alloc exception. From what we have seen, these only occur for
debug builds.

I have posted fixes for all of the issues above on the pybind/fixes [1]
topic and a set of patches to upgrade to PyBind 2.1.1 [2].

//Andreas

[1]
https://gem5-review.googlesource.com/#/q/topic:pybind/fixes+(status:open+OR+status:merged)
[2]
https://gem5-review.googlesource.com/#/q/topic:pybind/v2.1.1+(status:open+OR+status:merged)

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: scons: Use the generalized switching headers on the GPU ISA.

2017-05-10 Thread Gabe Black (Gerrit) 
Gabe Black has submitted this change and it was merged. (  
https://gem5-review.googlesource.com/2984 )


Change subject: scons: Use the generalized switching headers on the GPU ISA.
..

scons: Use the generalized switching headers on the GPU ISA.

Now that the switching header implementation has been generalized, there's
no need to have two nearly identical implementations for the two different
groups of headers.

Change-Id: Ie7c24fcddbc672ac5ca2d69bfc35696f42c55580
Reviewed-on: https://gem5-review.googlesource.com/2984
Maintainer: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
Reviewed-by: Tony Gutierrez 
Reviewed-by: Jason Lowe-Power 
---
M SConstruct
M src/arch/SConscript
2 files changed, 4 insertions(+), 35 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, but someone else must approve
  Curtis Dunham: Looks good to me, approved
  Tony Gutierrez: Looks good to me, approved
  Andreas Sandberg: Looks good to me, approved



diff --git a/SConstruct b/SConstruct
index 1efd106..30f99c7 100755
--- a/SConstruct
+++ b/SConstruct
@@ -1331,8 +1331,6 @@
 #
 ###

-main['ALL_GPU_ISA_LIST'] = all_gpu_isa_list
-
 def build_switching_header(target, source, env):
 path = str(target[0])
 subdir = str(source[0])
@@ -1357,35 +1355,6 @@

 main.AddMethod(switching_headers, 'SwitchingHeaders')

-def make_gpu_switching_dir(dname, switch_headers, env):
-# Generate the header.  target[0] is the full path of the output
-# header to generate.  'source' is a dummy variable, since we get the
-# list of ISAs from env['ALL_ISA_LIST'].
-def gen_switch_hdr(target, source, env):
-fname = str(target[0])
-
-isa = env['TARGET_GPU_ISA'].lower()
-
-try:
-f = open(fname, 'w')
-print >>f, '#include "%s/%s/%s"' % (dname, isa,  
basename(fname))

-f.close()
-except IOError:
-print "Failed to create %s" % fname
-raise
-
-# Build SCons Action object. 'varlist' specifies env vars that this
-# action depends on; when env['ALL_ISA_LIST'] changes these actions
-# should get re-executed.
-switch_hdr_action = MakeAction(gen_switch_hdr,
-  Transform("GENERATE"),  
varlist=['ALL_ISA_GPU_LIST'])

-
-# Instantiate actions for each header
-for hdr in switch_headers:
-env.Command(hdr, [], switch_hdr_action)
-
-Export('make_gpu_switching_dir')
-
 # all-isas -> all-deps -> all-environs -> all_targets
 main.Alias('#all-isas', [])
 main.Alias('#all-deps', '#all-isas')
diff --git a/src/arch/SConscript b/src/arch/SConscript
index 891a5a2..ea94056 100644
--- a/src/arch/SConscript
+++ b/src/arch/SConscript
@@ -67,13 +67,13 @@
 env.subst('${TARGET_ISA}'))

 if env['BUILD_GPU']:
-gpu_isa_switch_hdrs = Split('''
+env.SwitchingHeaders(
+Split('''
 gpu_decoder.hh
 gpu_isa.hh
 gpu_types.hh
-''')
-
-make_gpu_switching_dir('arch', gpu_isa_switch_hdrs, env)
+'''),
+env.subst('${TARGET_GPU_ISA}'))

 #
 #

--
To view, visit https://gem5-review.googlesource.com/2984
To unsubscribe, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie7c24fcddbc672ac5ca2d69bfc35696f42c55580
Gerrit-Change-Number: 2984
Gerrit-PatchSet: 6
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Curtis Dunham 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Tony Gutierrez 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: scons: arch: Generalize the switching header code.

2017-05-10 Thread Gabe Black (Gerrit) 
Gabe Black has submitted this change and it was merged. (  
https://gem5-review.googlesource.com/2983 )


Change subject: scons: arch: Generalize the switching header code.
..

scons: arch: Generalize the switching header code.

Factor out the ISA ness of the switching header generating function. Also
turn it into a SCons builder which builds a single header, and a wrapping
method which uses the builder on a group of header files which all target
the same subdirectory.

Change-Id: I87705f97b6ebd9baebd4ebcfea19cc1218a64ad0
Reviewed-on: https://gem5-review.googlesource.com/2983
Reviewed-by: Curtis Dunham 
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
---
M SConstruct
M src/arch/SConscript
2 files changed, 29 insertions(+), 31 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, but someone else must approve; Looks  
good to me, approved

  Curtis Dunham: Looks good to me, approved



diff --git a/SConstruct b/SConstruct
index bf6a0c6..1efd106 100755
--- a/SConstruct
+++ b/SConstruct
@@ -1324,38 +1324,38 @@

 ###
 #
-# This function is used to set up a directory with switching headers
+# This builder and wrapper method are used to set up a directory with
+# switching headers. Those are headers which are in a generic location and
+# that include more specific headers from a directory chosen at build time
+# based on the current build settings.
 #
 ###

-main['ALL_ISA_LIST'] = all_isa_list
 main['ALL_GPU_ISA_LIST'] = all_gpu_isa_list
-def make_switching_dir(dname, switch_headers, env):
-# Generate the header.  target[0] is the full path of the output
-# header to generate.  'source' is a dummy variable, since we get the
-# list of ISAs from env['ALL_ISA_LIST'].
-def gen_switch_hdr(target, source, env):
-fname = str(target[0])
-isa = env['TARGET_ISA'].lower()
-try:
-f = open(fname, 'w')
-print >>f, '#include "%s/%s/%s"' % (dname, isa,  
basename(fname))

-f.close()
-except IOError:
-print "Failed to create %s" % fname
-raise

-# Build SCons Action object. 'varlist' specifies env vars that this
-# action depends on; when env['ALL_ISA_LIST'] changes these actions
-# should get re-executed.
-switch_hdr_action = MakeAction(gen_switch_hdr,
-  Transform("GENERATE"), varlist=['ALL_ISA_LIST'])
+def build_switching_header(target, source, env):
+path = str(target[0])
+subdir = str(source[0])
+dp, fp = os.path.split(path)
+dp = os.path.relpath(os.path.realpath(dp),
+ os.path.realpath(env['BUILDDIR']))
+with open(path, 'w') as hdr:
+print >>hdr, '#include "%s/%s/%s"' % (dp, subdir, fp)

-# Instantiate actions for each header
-for hdr in switch_headers:
-env.Command(hdr, [], switch_hdr_action)
+switching_header_action = MakeAction(build_switching_header,
+ Transform('GENERATE'))

-Export('make_switching_dir')
+switching_header_builder = Builder(action=switching_header_action,
+   source_factory=Value,
+   single_source=True)
+
+main.Append(BUILDERS = { 'SwitchingHeader': switching_header_builder })
+
+def switching_headers(self, headers, source):
+for header in headers:
+self.SwitchingHeader(header, source)
+
+main.AddMethod(switching_headers, 'SwitchingHeaders')

 def make_gpu_switching_dir(dname, switch_headers, env):
 # Generate the header.  target[0] is the full path of the output
diff --git a/src/arch/SConscript b/src/arch/SConscript
index 54d97a4..891a5a2 100644
--- a/src/arch/SConscript
+++ b/src/arch/SConscript
@@ -43,8 +43,8 @@
 #
 #

-# List of headers to generate
-isa_switch_hdrs = Split('''
+env.SwitchingHeaders(
+Split('''
 decoder.hh
 interrupts.hh
 isa.hh
@@ -63,10 +63,8 @@
 types.hh
 utility.hh
 vtophys.hh
-''')
-
-# Set up this directory to support switching headers
-make_switching_dir('arch', isa_switch_hdrs, env)
+'''),
+env.subst('${TARGET_ISA}'))

 if env['BUILD_GPU']:
 gpu_isa_switch_hdrs = Split('''

--
To view, visit https://gem5-review.googlesource.com/2983
To unsubscribe, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I87705f97b6ebd9baebd4ebcfea19cc1218a64ad0
Gerrit-Change-Number: 2983
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Curtis Dunham 
Gerrit-Reviewer: Gabe

Re: [gem5-dev] use of C++ exceptions

2017-05-10 Thread Andreas Sandberg 

Having had a quick look at the code, I'd say that exceptions could
definitely make sense. I would support limited use of exceptions where
it makes sense to make the code flow less entangled. Initially, I would
argue that we should keep exceptions local to SimObjects (interfaces
like ports should never have to deal with exceptions) and only be used
for recoverable errors (e.g., unexpected input that should result in a
warning).

Are there any technical reason why we aren't using exceptions?

//Andreas

On 10/05/2017 08:25, Gabe Black wrote:

This would be internal to the gdb code, ie if a read from the socket fails,
it would detach and throw an exception which would unwind back out of all
the gdb stuff without having to add ifs all over the place. This bit of
code doesn't really have an external interface, so it wouldn't be visible
to the caller, assuming a stray exception didn't escape somehow. I think it
would be a little nicer that way, but not so much that I'd want to argue
for it very strongly.

Gabe

On Wed, May 10, 2017 at 12:09 AM, Andreas Hansson 
wrote:


Hi Gabe,

I do not think adding exceptions will make things any less cluttered. It
will simply move that complexity to any caller, will it not? I am not a
fan of exceptions in general as it mucks with the control flow.

Andreas

On 10/05/2017, 07:31, "gem5-dev on behalf of Gabe Black"
 wrote:


Hi folks. I have a change to make the GDB stub in gem5 a bit less fragile:

https://gem5-review.googlesource.com/#/c/3180/

Unfortunately that involved adding a lot of error code checking which
makes
things a bit cluttered and ugly. I think it would be a lot nicer to use
exceptions, but I remember those being a no-no. Are they currently against
the rules, or could I use them to make that code a bit nicer?

Gabe
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient, please notify the sender immediately and do not disclose the
contents to any other person, use it for any purpose, or store or copy the
information in any medium. Thank you.
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev


IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: python: Prevent Python wrappers from deleting SimObjects

2017-05-10 Thread Andreas Sandberg (Gerrit) 

Hello Curtis Dunham,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/3224

to review the following change.


Change subject: python: Prevent Python wrappers from deleting SimObjects
..

python: Prevent Python wrappers from deleting SimObjects

The PyBind wrappers could potentially delete SimObjects if they don't
have any references. This is not desirable since there could be
pointers to such objects within the C++ world. This problem doesn't
normally occur since Python typically holds a pointer to the root node
as long as the simulator is running.

Prevent SimObject and Param deletion by using a PyBind-prescribed
unique_ptr with a dummy deleter as the pointer wrapper for the Python
world.

Change-Id: Ied14602c9ee69a083a69c5dae1b5fcf8efb4548a
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
---
M src/python/m5/SimObject.py
M src/python/pybind11/core.cc
2 files changed, 15 insertions(+), 8 deletions(-)



diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py
index b621a82..2243307 100644
--- a/src/python/m5/SimObject.py
+++ b/src/python/m5/SimObject.py
@@ -699,10 +699,13 @@
 ''')
 code.indent()
 if cls._base:
-code('py::class_<${cls}Params, ${{cls._base.type}}Params>(m, '  
\

- '"${cls}Params")')
+code('py::class_<${cls}Params, ${{cls._base.type}}Params, ' \
+ 'std::unique_ptr<${{cls}}Params, py::nodelete>>(' \
+ 'm, "${cls}Params")')
 else:
-code('py::class_<${cls}Params>(m, "${cls}Params")')
+code('py::class_<${cls}Params, ' \
+ 'std::unique_ptr<${cls}Params, py::nodelete>>(' \
+ 'm, "${cls}Params")')

 code.indent()
 if not hasattr(cls, 'abstract') or not cls.abstract:
@@ -727,10 +730,13 @@
 cls.cxx_bases
 if bases:
 base_str = ", ".join(bases)
-code('py::class_<${{cls.cxx_class}}, ${base_str}>(m, ' \
- '"${py_class_name}")')
+code('py::class_<${{cls.cxx_class}}, ${base_str}, ' \
+ 'std::unique_ptr<${{cls.cxx_class}}, py::nodelete>>(' \
+ 'm, "${py_class_name}")')
 else:
-code('py::class_<${{cls.cxx_class}}>(m, "${py_class_name}")')
+code('py::class_<${{cls.cxx_class}}, ' \
+ 'std::unique_ptr<${{cls.cxx_class}}, py::nodelete>>(' \
+ 'm, "${py_class_name}")')
 code.indent()
 for exp in cls.cxx_exports:
 exp.export(code, cls.cxx_class)
diff --git a/src/python/pybind11/core.cc b/src/python/pybind11/core.cc
index 7ad45b4..159b19f 100644
--- a/src/python/pybind11/core.cc
+++ b/src/python/pybind11/core.cc
@@ -132,7 +132,8 @@
 {
 py::module m = m_native.def_submodule("serialize");

-py::class_(m, "Serializable")
+py::class_>(
+m, "Serializable")
 ;

 py::class_(m, "CheckpointIn")
@@ -165,7 +166,7 @@
 .def("isSubset", ::isSubset)
 ;

-// We need to make vectors of AddrRange opaque to avoid a weird
+// We need to make vectors of AddrRange opaque to avoid weird
 // memory allocation issues in PyBind's STL wrappers.
 py::bind_vector(m, "AddrRangeVector");


--
To view, visit https://gem5-review.googlesource.com/3224
To unsubscribe, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied14602c9ee69a083a69c5dae1b5fcf8efb4548a
Gerrit-Change-Number: 3224
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg 
Gerrit-Reviewer: Curtis Dunham 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: ext: Upgrade PyBind11 to version 2.1.1

2017-05-10 Thread Andreas Sandberg (Gerrit) 

Hello Curtis Dunham,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/3225

to review the following change.


Change subject: ext: Upgrade PyBind11 to version 2.1.1
..

ext: Upgrade PyBind11 to version 2.1.1

Change-Id: I16870dec402d661295f9d013dc23e362b2b2c169
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
---
M ext/pybind11/.appveyor.yml
A ext/pybind11/.readthedocs.yml
M ext/pybind11/.travis.yml
M ext/pybind11/CMakeLists.txt
A ext/pybind11/ISSUE_TEMPLATE.md
M ext/pybind11/README.md
A ext/pybind11/docs/Doxyfile
M ext/pybind11/docs/advanced/cast/chrono.rst
M ext/pybind11/docs/advanced/cast/eigen.rst
M ext/pybind11/docs/advanced/cast/index.rst
M ext/pybind11/docs/advanced/cast/overview.rst
M ext/pybind11/docs/advanced/cast/stl.rst
A ext/pybind11/docs/advanced/cast/strings.rst
M ext/pybind11/docs/advanced/classes.rst
M ext/pybind11/docs/advanced/functions.rst
M ext/pybind11/docs/advanced/misc.rst
M ext/pybind11/docs/advanced/pycpp/numpy.rst
M ext/pybind11/docs/advanced/pycpp/object.rst
M ext/pybind11/docs/advanced/smart_ptrs.rst
M ext/pybind11/docs/basics.rst
M ext/pybind11/docs/changelog.rst
M ext/pybind11/docs/classes.rst
M ext/pybind11/docs/compiling.rst
M ext/pybind11/docs/conf.py
M ext/pybind11/docs/intro.rst
M ext/pybind11/docs/reference.rst
M ext/pybind11/docs/release.rst
A ext/pybind11/docs/requirements.txt
M ext/pybind11/include/pybind11/attr.h
M ext/pybind11/include/pybind11/cast.h
M ext/pybind11/include/pybind11/chrono.h
A ext/pybind11/include/pybind11/class_support.h
M ext/pybind11/include/pybind11/common.h
M ext/pybind11/include/pybind11/complex.h
M ext/pybind11/include/pybind11/eigen.h
M ext/pybind11/include/pybind11/eval.h
M ext/pybind11/include/pybind11/functional.h
M ext/pybind11/include/pybind11/numpy.h
M ext/pybind11/include/pybind11/pybind11.h
M ext/pybind11/include/pybind11/pytypes.h
M ext/pybind11/include/pybind11/stl.h
M ext/pybind11/include/pybind11/stl_bind.h
M ext/pybind11/pybind11/_version.py
M ext/pybind11/setup.py
M ext/pybind11/tests/CMakeLists.txt
M ext/pybind11/tests/conftest.py
M ext/pybind11/tests/constructor_stats.h
M ext/pybind11/tests/object.h
M ext/pybind11/tests/pybind11_tests.cpp
A ext/pybind11/tests/pytest.ini
M ext/pybind11/tests/test_alias_initialization.py
M ext/pybind11/tests/test_buffers.cpp
M ext/pybind11/tests/test_buffers.py
M ext/pybind11/tests/test_callbacks.cpp
M ext/pybind11/tests/test_callbacks.py
M ext/pybind11/tests/test_chrono.cpp
M ext/pybind11/tests/test_chrono.py
A ext/pybind11/tests/test_cmake_build/installed_function/CMakeLists.txt
A ext/pybind11/tests/test_cmake_build/installed_target/CMakeLists.txt
R ext/pybind11/tests/test_cmake_build/main.cpp
A ext/pybind11/tests/test_cmake_build/subdirectory_function/CMakeLists.txt
A ext/pybind11/tests/test_cmake_build/subdirectory_target/CMakeLists.txt
A ext/pybind11/tests/test_cmake_build/test.py
M ext/pybind11/tests/test_constants_and_functions.cpp
M ext/pybind11/tests/test_constants_and_functions.py
M ext/pybind11/tests/test_docstring_options.cpp
M ext/pybind11/tests/test_docstring_options.py
M ext/pybind11/tests/test_eigen.cpp
M ext/pybind11/tests/test_eigen.py
M ext/pybind11/tests/test_enum.py
M ext/pybind11/tests/test_inheritance.cpp
M ext/pybind11/tests/test_inheritance.py
D ext/pybind11/tests/test_installed_module/CMakeLists.txt
D ext/pybind11/tests/test_installed_module/test.py
D ext/pybind11/tests/test_installed_target/CMakeLists.txt
D ext/pybind11/tests/test_installed_target/main.cpp
D ext/pybind11/tests/test_installed_target/test.py
M ext/pybind11/tests/test_issues.cpp
M ext/pybind11/tests/test_issues.py
M ext/pybind11/tests/test_keep_alive.py
M ext/pybind11/tests/test_kwargs_and_defaults.cpp
M ext/pybind11/tests/test_kwargs_and_defaults.py
M ext/pybind11/tests/test_methods_and_attributes.cpp
M ext/pybind11/tests/test_methods_and_attributes.py
M ext/pybind11/tests/test_modules.py
M ext/pybind11/tests/test_multiple_inheritance.cpp
M ext/pybind11/tests/test_multiple_inheritance.py
M ext/pybind11/tests/test_numpy_array.cpp
M ext/pybind11/tests/test_numpy_array.py
M ext/pybind11/tests/test_numpy_dtypes.cpp
M ext/pybind11/tests/test_numpy_dtypes.py
M ext/pybind11/tests/test_numpy_vectorize.cpp
M ext/pybind11/tests/test_numpy_vectorize.py
M ext/pybind11/tests/test_opaque_types.py
M ext/pybind11/tests/test_operator_overloading.py
M ext/pybind11/tests/test_pickling.cpp
M ext/pybind11/tests/test_pickling.py
M ext/pybind11/tests/test_python_types.cpp
M ext/pybind11/tests/test_python_types.py
M ext/pybind11/tests/test_sequences_and_iterators.cpp
M ext/pybind11/tests/test_sequences_and_iterators.py
M ext/pybind11/tests/test_smart_ptr.cpp
M ext/pybind11/tests/test_smart_ptr.py
M ext/pybind11/tests/test_stl_binders.cpp
M ext/pybind11/tests/test_stl_binders.py
M ext/pybind11/tests/test_virtual_functions.py
M

[gem5-dev] Change in public/gem5[master]: python: Fix weird memory issue in wrapped AddrRange vectors

2017-05-10 Thread Andreas Sandberg (Gerrit) 

Hello Curtis Dunham,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/3223

to review the following change.


Change subject: python: Fix weird memory issue in wrapped AddrRange vectors
..

python: Fix weird memory issue in wrapped AddrRange vectors

There is a weird issue with the PyBind wrapper of
vector. Assigning new values to a param that is a vector of
AddrRange sometimes results in an out-of-bounds memory access.

We work around this issue by treating AddrRange vectors as opaque
types. This slightly changes the semantics of the wrapper since Python
now manipulates the real object rather than a copy that has been
converted to a list.

Change-Id: Ie027c06e7a7262214b43b19a76b24fe4b20426c5
Signed-off-by: Andreas Sandberg 
Reviewed-by: Sascha Bischoff 
Reviewed-by: Curtis Dunham 
Reviewed-by: Timothy Hayes 
---
M src/python/m5/SimObject.py
M src/python/pybind11/core.cc
A src/python/pybind11/core.hh
3 files changed, 66 insertions(+), 1 deletion(-)



diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py
index b5ad977..b621a82 100644
--- a/src/python/m5/SimObject.py
+++ b/src/python/m5/SimObject.py
@@ -681,6 +681,7 @@

 #include "sim/sim_object.hh"
 #include "params/$cls.hh"
+#include "python/pybind11/core.hh"
 #include "sim/init.hh"
 #include "${{cls.cxx_header}}"

@@ -1413,7 +1414,14 @@
 assert isinstance(value, list)
 vec = getattr(cc_params, param)
 assert not len(vec)
-setattr(cc_params, param, list(value))
+# Some types are exposed as opaque types. They support
+# the append operation unlike the automatically
+# wrapped types.
+if isinstance(vec, list):
+setattr(cc_params, param, list(value))
+else:
+for v in value:
+getattr(cc_params, param).append(v)
 else:
 setattr(cc_params, param, value)

diff --git a/src/python/pybind11/core.cc b/src/python/pybind11/core.cc
index a11ad7f..7ad45b4 100644
--- a/src/python/pybind11/core.cc
+++ b/src/python/pybind11/core.cc
@@ -46,6 +46,8 @@

 #include "pybind11/pybind11.h"

+#include "python/pybind11/core.hh"
+
 #include 

 #include "base/addr_range.hh"
@@ -163,6 +165,10 @@
 .def("isSubset", ::isSubset)
 ;

+// We need to make vectors of AddrRange opaque to avoid a weird
+// memory allocation issues in PyBind's STL wrappers.
+py::bind_vector(m, "AddrRangeVector");
+
 m.def("RangeEx", );
 m.def("RangeIn", );
 m.def("RangeSize", );
diff --git a/src/python/pybind11/core.hh b/src/python/pybind11/core.hh
new file mode 100644
index 000..81e7627
--- /dev/null
+++ b/src/python/pybind11/core.hh
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2017 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
+ * 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
+

[gem5-dev] Change in public/gem5[master]: sim: Add hooks to implement event reference counting

2017-05-10 Thread Andreas Sandberg (Gerrit) 

Hello Curtis Dunham,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/3221

to review the following change.


Change subject: sim: Add hooks to implement event reference counting
..

sim: Add hooks to implement event reference counting

We currently only support deleting an event if it is triggered and not
re-scheduled. This is fine for most native code. However, there are
cases where Python needs to count references to make sure that the
Python object stays live while the native object is live.

Generalise the mechanism used to implement AutoDelete to use a
reference manager that can be used to implement reference
counting. The reference manager has two calls:

  * RefManager::incref(T *o)
  * RefManager::decref(T *o)

These calls are used to increase and decrease the reference count of
an object. The default implementation in Event maintains backwards
compatibility with the existing AutoDelete feature by ignoring
incref() and deleting the event on decref() if it isn't scheduled
anymore.

Custom reference counting implementations can be implemented by
overriding the Event::getRefManager() method and returning a pointer
to a custom reference manager. Reference counting can then be enabled
by setting the Managed flag at Event creation.

Change-Id: I5637984c906a9d44c22780712cf1c521b8297149
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
---
M src/base/refcnt.hh
M src/sim/eventq.cc
M src/sim/eventq.hh
M src/sim/eventq_impl.hh
4 files changed, 63 insertions(+), 7 deletions(-)



diff --git a/src/base/refcnt.hh b/src/base/refcnt.hh
index baf08c6..f5da1fb 100644
--- a/src/base/refcnt.hh
+++ b/src/base/refcnt.hh
@@ -37,6 +37,17 @@
  * Classes for managing reference counted objects.
  */

+
+template
+class RefManager
+{
+  public:
+virtual ~RefManager() {};
+
+virtual void incref(T *o) const = 0;
+virtual void decref(T *o) const = 0;
+};
+
 /**
  * Derive from RefCounted if you want to enable reference counting of
  * this class.  If you want to use automatic reference counting, you
diff --git a/src/sim/eventq.cc b/src/sim/eventq.cc
index 7c2648c..3694a28 100644
--- a/src/sim/eventq.cc
+++ b/src/sim/eventq.cc
@@ -49,6 +49,19 @@

 Tick simQuantum = 0;

+
+class EventAutoDelete : public RefManager
+{
+  public:
+void incref(Event *e) const final {}
+void decref(Event *e) const final {
+if (!e->scheduled())
+delete e;
+}
+};
+
+static EventAutoDelete autoDeleter;
+
 //
 // Main Event Queues
 //
@@ -168,6 +181,12 @@
 return top;
 }

+const RefManager *
+Event::getRefManager() const
+{
+return 
+}
+
 void
 EventQueue::remove(Event *event)
 {
@@ -227,7 +246,7 @@

 event->process();
 if (event->isExitEvent()) {
-assert(!event->flags.isSet(Event::AutoDelete) ||
+assert(!event->flags.isSet(Event::Managed) ||
!event->flags.isSet(Event::IsMainQueue)); // would be  
silly

 return event;
 }
@@ -235,8 +254,7 @@
 event->flags.clear(Event::Squashed);
 }

-if (event->flags.isSet(Event::AutoDelete) && !event->scheduled())
-delete event;
+EventQueue::decref(event);

 return NULL;
 }
diff --git a/src/sim/eventq.hh b/src/sim/eventq.hh
index 95a36ca..4874aba 100644
--- a/src/sim/eventq.hh
+++ b/src/sim/eventq.hh
@@ -47,6 +47,7 @@
 #include 

 #include "base/flags.hh"
+#include "base/refcnt.hh"
 #include "base/types.hh"
 #include "debug/Event.hh"
 #include "sim/serialize.hh"
@@ -99,7 +100,8 @@
 static const FlagsType PublicWrite   = 0x001d; // public writable flags
 static const FlagsType Squashed  = 0x0001; // has been squashed
 static const FlagsType Scheduled = 0x0002; // has been scheduled
-static const FlagsType AutoDelete= 0x0004; // delete after dispatch
+static const FlagsType Managed   = 0x0004; // Use life cycle  
manager
+static const FlagsType AutoDelete= Managed; // delete after  
dispatch

 /**
  * This used to be AutoSerialize. This value can't be reused
  * without changing the checkpoint version since the flag field
@@ -282,6 +284,19 @@
 // This function isn't really useful if TRACING_ON is not defined
 virtual void trace(const char *action); //!< trace event activity

+/**
+ * Get a pointer to the reference manager that handles this event
+ *
+ * The reference manager implements generalized support for
+ * automatic event deletion. The default reference manager ignores
+ * RefManager::incref() calls and deletes an event on
+ * RefManager::decref() if the object isn't scheduled anymore.
+ *
+ * Objects that are exposed to managed languages will need to
+ * override this and implement proper reference counting.
+ */
+virtual const RefManager *getRefManager() const;
+

[gem5-dev] Change in public/gem5[master]: python: Add a helper function to create Python events

2017-05-10 Thread Andreas Sandberg (Gerrit) 

Hello Curtis Dunham,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/3220

to review the following change.


Change subject: python: Add a helper function to create Python events
..

python: Add a helper function to create Python events

Add a helper function, m5.event.create(), to create events from
Python. This function takes a callable Python object (e.g., a
function) as an argument and optionally a priority as a keyword
argument. This function was accidentally dropped from the public API
when switching to PyBind.

Change-Id: Icbd0e392d9506934ec2c9f541199aa35c1c2df8c
Signed-off-by: Andreas Sandberg 
Reviewed-by: Curtis Dunham 
---
M src/python/m5/event.py
1 file changed, 27 insertions(+), 1 deletion(-)



diff --git a/src/python/m5/event.py b/src/python/m5/event.py
index d1aff9e..20f81b9 100644
--- a/src/python/m5/event.py
+++ b/src/python/m5/event.py
@@ -48,6 +48,25 @@

 mainq = None

+class EventWrapper(Event):
+"""Helper class to wrap callable objects in an Event base class"""
+
+def __init__(self, func, **kwargs):
+super(EventWrapper, self).__init__(**kwargs)
+
+if not callable(func):
+raise RuntimeError("Can't wrap '%s', object is not callable" %  
\

+   str(func))
+
+self._func = func
+
+def __call__(self):
+self._func()
+
+def __str__(self):
+return "EventWrapper(%s)" % (str(self._func), )
+
+
 class ProgressEvent(Event):
 def __init__(self, eventq, period):
 super(ProgressEvent, self).__init__()
@@ -59,4 +78,11 @@
 print "Progress! Time now %fs" % (m5.curTick()/1e12)
 self.eventq.schedule(self, m5.curTick() + self.period)

-__all__ = [ 'Event', 'ProgressEvent', 'SimExit', 'mainq' ]
+
+def create(func, priority=Event.Default_Pri):
+"""Create an Event from a function"""
+
+return EventWrapper(func, priority=priority)
+
+__all__ = [ 'Event', 'EventWrapper', 'ProgressEvent', 'SimExit',
+'mainq', 'create' ]

--
To view, visit https://gem5-review.googlesource.com/3220
To unsubscribe, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icbd0e392d9506934ec2c9f541199aa35c1c2df8c
Gerrit-Change-Number: 3220
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg 
Gerrit-Reviewer: Curtis Dunham 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: ext: Include SystemC 2.3.1 into gem5

2017-05-10 Thread Matthias Jung (Gerrit) 

Hello Andreas Sandberg, Éder F. Zulian,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/2240

to look at the new patch set (#6).

Change subject: ext: Include SystemC 2.3.1 into gem5
..

ext: Include SystemC 2.3.1 into gem5

In the past it happened several times that some changes in gem5 broke the
SystemC coupling. Recently Accelera has changed the licence for SystemC
from their own licence to Apache2.0, which is compatible with gem5.
However, SystemC usually relies on the Boost library, but I was able to
exchange the boost calls by c++11 alternatives. The recent SystemC version
is placed into /ext and is integrated into gem5's build system. The goal is
to integrate some SystemC tests for the CI in some following patches.

Change-Id: I4b66ec806b5e3cffc1d7c85d3735ff4fa5b31fd0
---
M SConstruct
A ext/systemc/AUTHORS
A ext/systemc/ChangeLog
A ext/systemc/INSTALL
A ext/systemc/LICENSE
A ext/systemc/NEWS
A ext/systemc/NOTICE
A ext/systemc/README.gem5.md
A ext/systemc/README.md
A ext/systemc/README.sysc
A ext/systemc/RELEASENOTES
A ext/systemc/SConscript
A ext/systemc/src/README_TLM.txt
A ext/systemc/src/sysc/communication/sc_buffer.h
A ext/systemc/src/sysc/communication/sc_clock.cpp
A ext/systemc/src/sysc/communication/sc_clock.h
A ext/systemc/src/sysc/communication/sc_clock_ports.h
A ext/systemc/src/sysc/communication/sc_communication_ids.h
A ext/systemc/src/sysc/communication/sc_event_finder.cpp
A ext/systemc/src/sysc/communication/sc_event_finder.h
A ext/systemc/src/sysc/communication/sc_event_queue.cpp
A ext/systemc/src/sysc/communication/sc_event_queue.h
A ext/systemc/src/sysc/communication/sc_export.cpp
A ext/systemc/src/sysc/communication/sc_export.h
A ext/systemc/src/sysc/communication/sc_fifo.h
A ext/systemc/src/sysc/communication/sc_fifo_ifs.h
A ext/systemc/src/sysc/communication/sc_fifo_ports.h
A ext/systemc/src/sysc/communication/sc_host_mutex.h
A ext/systemc/src/sysc/communication/sc_interface.cpp
A ext/systemc/src/sysc/communication/sc_interface.h
A ext/systemc/src/sysc/communication/sc_mutex.cpp
A ext/systemc/src/sysc/communication/sc_mutex.h
A ext/systemc/src/sysc/communication/sc_mutex_if.h
A ext/systemc/src/sysc/communication/sc_port.cpp
A ext/systemc/src/sysc/communication/sc_port.h
A ext/systemc/src/sysc/communication/sc_prim_channel.cpp
A ext/systemc/src/sysc/communication/sc_prim_channel.h
A ext/systemc/src/sysc/communication/sc_semaphore.cpp
A ext/systemc/src/sysc/communication/sc_semaphore.h
A ext/systemc/src/sysc/communication/sc_semaphore_if.h
A ext/systemc/src/sysc/communication/sc_signal.cpp
A ext/systemc/src/sysc/communication/sc_signal.h
A ext/systemc/src/sysc/communication/sc_signal_ifs.h
A ext/systemc/src/sysc/communication/sc_signal_ports.cpp
A ext/systemc/src/sysc/communication/sc_signal_ports.h
A ext/systemc/src/sysc/communication/sc_signal_resolved.cpp
A ext/systemc/src/sysc/communication/sc_signal_resolved.h
A ext/systemc/src/sysc/communication/sc_signal_resolved_ports.cpp
A ext/systemc/src/sysc/communication/sc_signal_resolved_ports.h
A ext/systemc/src/sysc/communication/sc_signal_rv.h
A ext/systemc/src/sysc/communication/sc_signal_rv_ports.h
A ext/systemc/src/sysc/communication/sc_writer_policy.h
A ext/systemc/src/sysc/datatypes/bit/sc_bit.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_bit.h
A ext/systemc/src/sysc/datatypes/bit/sc_bit_ids.h
A ext/systemc/src/sysc/datatypes/bit/sc_bit_proxies.h
A ext/systemc/src/sysc/datatypes/bit/sc_bv.h
A ext/systemc/src/sysc/datatypes/bit/sc_bv_base.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_bv_base.h
A ext/systemc/src/sysc/datatypes/bit/sc_logic.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_logic.h
A ext/systemc/src/sysc/datatypes/bit/sc_lv.h
A ext/systemc/src/sysc/datatypes/bit/sc_lv_base.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_lv_base.h
A ext/systemc/src/sysc/datatypes/bit/sc_proxy.h
A ext/systemc/src/sysc/datatypes/fx/fx.h
A ext/systemc/src/sysc/datatypes/fx/sc_context.h
A ext/systemc/src/sysc/datatypes/fx/sc_fix.h
A ext/systemc/src/sysc/datatypes/fx/sc_fixed.h
A ext/systemc/src/sysc/datatypes/fx/sc_fx_ids.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxcast_switch.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxcast_switch.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxdefs.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxdefs.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum_observer.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum_observer.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxtype_params.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxtype_params.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxval.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxval.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxval_observer.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxval_observer.h
A ext/systemc/src/sysc/datatypes/fx/sc_ufix.h
A

[gem5-dev] Change in public/gem5[master]: ext: Include SystemC 2.3.1 into gem5

2017-05-10 Thread Matthias Jung (Gerrit) 

Hello Andreas Sandberg, Éder F. Zulian,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/2240

to look at the new patch set (#5).

Change subject: ext: Include SystemC 2.3.1 into gem5
..

ext: Include SystemC 2.3.1 into gem5

In the past it happened several times that some changes in gem5 broke the
SystemC coupling. Recently Accelera has changed the licence for SystemC from
their own licence to Apache2.0, which is compatible with gem5. However,  
SystemC
usually relies on the Boost library, but I was able to exchange the boost  
calls

by c++11 alternatives. The recent SystemC version is placed into /ext and is
integrated into gem5's build system. The goal is to integrate some SystemC
tests for the CI in some following patches.

Change-Id: I4b66ec806b5e3cffc1d7c85d3735ff4fa5b31fd0
---
M SConstruct
A ext/systemc/AUTHORS
A ext/systemc/ChangeLog
A ext/systemc/INSTALL
A ext/systemc/LICENSE
A ext/systemc/NEWS
A ext/systemc/NOTICE
A ext/systemc/README.gem5.md
A ext/systemc/README.md
A ext/systemc/README.sysc
A ext/systemc/RELEASENOTES
A ext/systemc/SConscript
A ext/systemc/src/README_TLM.txt
A ext/systemc/src/sysc/communication/sc_buffer.h
A ext/systemc/src/sysc/communication/sc_clock.cpp
A ext/systemc/src/sysc/communication/sc_clock.h
A ext/systemc/src/sysc/communication/sc_clock_ports.h
A ext/systemc/src/sysc/communication/sc_communication_ids.h
A ext/systemc/src/sysc/communication/sc_event_finder.cpp
A ext/systemc/src/sysc/communication/sc_event_finder.h
A ext/systemc/src/sysc/communication/sc_event_queue.cpp
A ext/systemc/src/sysc/communication/sc_event_queue.h
A ext/systemc/src/sysc/communication/sc_export.cpp
A ext/systemc/src/sysc/communication/sc_export.h
A ext/systemc/src/sysc/communication/sc_fifo.h
A ext/systemc/src/sysc/communication/sc_fifo_ifs.h
A ext/systemc/src/sysc/communication/sc_fifo_ports.h
A ext/systemc/src/sysc/communication/sc_host_mutex.h
A ext/systemc/src/sysc/communication/sc_interface.cpp
A ext/systemc/src/sysc/communication/sc_interface.h
A ext/systemc/src/sysc/communication/sc_mutex.cpp
A ext/systemc/src/sysc/communication/sc_mutex.h
A ext/systemc/src/sysc/communication/sc_mutex_if.h
A ext/systemc/src/sysc/communication/sc_port.cpp
A ext/systemc/src/sysc/communication/sc_port.h
A ext/systemc/src/sysc/communication/sc_prim_channel.cpp
A ext/systemc/src/sysc/communication/sc_prim_channel.h
A ext/systemc/src/sysc/communication/sc_semaphore.cpp
A ext/systemc/src/sysc/communication/sc_semaphore.h
A ext/systemc/src/sysc/communication/sc_semaphore_if.h
A ext/systemc/src/sysc/communication/sc_signal.cpp
A ext/systemc/src/sysc/communication/sc_signal.h
A ext/systemc/src/sysc/communication/sc_signal_ifs.h
A ext/systemc/src/sysc/communication/sc_signal_ports.cpp
A ext/systemc/src/sysc/communication/sc_signal_ports.h
A ext/systemc/src/sysc/communication/sc_signal_resolved.cpp
A ext/systemc/src/sysc/communication/sc_signal_resolved.h
A ext/systemc/src/sysc/communication/sc_signal_resolved_ports.cpp
A ext/systemc/src/sysc/communication/sc_signal_resolved_ports.h
A ext/systemc/src/sysc/communication/sc_signal_rv.h
A ext/systemc/src/sysc/communication/sc_signal_rv_ports.h
A ext/systemc/src/sysc/communication/sc_writer_policy.h
A ext/systemc/src/sysc/datatypes/bit/sc_bit.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_bit.h
A ext/systemc/src/sysc/datatypes/bit/sc_bit_ids.h
A ext/systemc/src/sysc/datatypes/bit/sc_bit_proxies.h
A ext/systemc/src/sysc/datatypes/bit/sc_bv.h
A ext/systemc/src/sysc/datatypes/bit/sc_bv_base.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_bv_base.h
A ext/systemc/src/sysc/datatypes/bit/sc_logic.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_logic.h
A ext/systemc/src/sysc/datatypes/bit/sc_lv.h
A ext/systemc/src/sysc/datatypes/bit/sc_lv_base.cpp
A ext/systemc/src/sysc/datatypes/bit/sc_lv_base.h
A ext/systemc/src/sysc/datatypes/bit/sc_proxy.h
A ext/systemc/src/sysc/datatypes/fx/fx.h
A ext/systemc/src/sysc/datatypes/fx/sc_context.h
A ext/systemc/src/sysc/datatypes/fx/sc_fix.h
A ext/systemc/src/sysc/datatypes/fx/sc_fixed.h
A ext/systemc/src/sysc/datatypes/fx/sc_fx_ids.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxcast_switch.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxcast_switch.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxdefs.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxdefs.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum_observer.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxnum_observer.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxtype_params.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxtype_params.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxval.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxval.h
A ext/systemc/src/sysc/datatypes/fx/sc_fxval_observer.cpp
A ext/systemc/src/sysc/datatypes/fx/sc_fxval_observer.h
A ext/systemc/src/sysc/datatypes/fx/sc_ufix.h
A

[gem5-dev] Change in public/gem5[master]: base: Make the VNC server more resilient.

2017-05-10 Thread Gabe Black (Gerrit) 
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/3200



Change subject: base: Make the VNC server more resilient.
..

base: Make the VNC server more resilient.

If the client does something bad, don't kill the whole simulation, just
complain, drop the client and keep going.

Change-Id: I824f2d121e2fe03cdf4323a25c192b68e0370acc
---
M src/base/vnc/vncserver.cc
M src/base/vnc/vncserver.hh
2 files changed, 78 insertions(+), 58 deletions(-)



diff --git a/src/base/vnc/vncserver.cc b/src/base/vnc/vncserver.cc
index 216fa2f..477284b 100644
--- a/src/base/vnc/vncserver.cc
+++ b/src/base/vnc/vncserver.cc
@@ -198,7 +198,10 @@
 panic("%s: cannot accept a connection if not listening!", name());

 int fd = listener.accept(true);
-fatal_if(fd < 0, "%s: failed to accept VNC connection!", name());
+if (fd < 0) {
+warn("%s: failed to accept VNC connection!", name());
+return;
+}

 if (dataFd != -1) {
 char message[] = "vnc server already attached!\n";
@@ -210,7 +213,7 @@
 dataFd = fd;

 // Send our version number to the client
-write((uint8_t*)vncVersion(), strlen(vncVersion()));
+write((uint8_t *)vncVersion(), strlen(vncVersion()));

 // read the client response
 dataEvent = new DataEvent(this, dataFd, POLLIN);
@@ -224,7 +227,6 @@
 VncServer::data()
 {
 // We have new data, see if we can handle it
-size_t len;
 DPRINTF(VNC, "Vnc client message recieved\n");

 switch (curState) {
@@ -237,8 +239,8 @@
   case WaitForClientInit:
 // Don't care about shared, just need to read it out of the socket
 uint8_t shared;
-len = read();
-assert(len == 1);
+if (read() != sizeof(shared))
+return;

 // Send our idea of the frame buffer
 sendServerInit();
@@ -246,12 +248,8 @@
 break;
   case NormalPhase:
 uint8_t message_type;
-len = read(_type);
-if (!len) {
-detach();
+if (read(_type) != sizeof(message_type))
 return;
-}
-assert(len == 1);

 switch (message_type) {
   case ClientSetPixelFormat:
@@ -273,8 +271,9 @@
 recvCutText();
 break;
   default:
-panic("Unimplemented message type recv from client: %d\n",
-  message_type);
+warn("Unimplemented message type recv from client: %d\n",
+ message_type);
+detach();
 break;
 }
 break;
@@ -297,10 +296,9 @@
 } while (ret == -1 && errno == EINTR);


-if (ret <= 0){
-DPRINTF(VNC, "Read failed.\n");
+if (ret != len) {
+DPRINTF(VNC, "Read failed %d.\n", ret);
 detach();
-return 0;
 }

 return ret;
@@ -309,10 +307,9 @@
 size_t
 VncServer::read1(uint8_t *buf, size_t len)
 {
-size_t read_len M5_VAR_USED;
-read_len = read(buf + 1, len - 1);
-assert(read_len == len - 1);
-return read_len;
+if (read(buf + 1, len - 1) != len - 1)
+return 0;
+return len - 1;
 }


@@ -320,7 +317,7 @@
 size_t
 VncServer::read(T* val)
 {
-return read((uint8_t*)val, sizeof(T));
+return read((uint8_t *)val, sizeof(T));
 }

 // write to socket
@@ -330,11 +327,12 @@
 if (dataFd < 0)
 panic("Vnc client not properly attached.\n");

-ssize_t ret;
-ret = atomic_write(dataFd, buf, len);
+ssize_t ret = atomic_write(dataFd, buf, len);

-if (ret < len)
+if (ret != len) {
+DPRINTF(VNC, "Write failed.\n");
 detach();
+}

 return ret;
 }
@@ -343,13 +341,13 @@
 size_t
 VncServer::write(T* val)
 {
-return write((uint8_t*)val, sizeof(T));
+return write((uint8_t *)val, sizeof(T));
 }

 size_t
 VncServer::write(const char* str)
 {
-return write((uint8_t*)str, strlen(str));
+return write((uint8_t *)str, strlen(str));
 }

 // detach a vnc client
@@ -377,7 +375,8 @@
 VncServer::sendError(const char* error_msg)
 {
uint32_t len = strlen(error_msg);
-   write();
+   if (write() != sizeof(len))
+   return;
write(error_msg);
 }

@@ -392,8 +391,11 @@
 // Null terminate the message so it's easier to work with
 version_string[12] = 0;

-len = read((uint8_t*)version_string, 12);
-assert(len == 12);
+if (read((uint8_t *)version_string, sizeof(version_string) - 1) !=
+sizeof(version_string) - 1) {
+warn("Failed to read protocol version.");
+return;
+}

 uint32_t major, minor;

@@ -402,6 +404,7 @@
 warn(" Malformed protocol version %s\n", version_string);
 sendError("Malformed protocol version\n");
 detach();
+return;
 }

 DPRINTF(VNC, "Client request protocol version %d.%d\n", major, minor);
@@ -412,16 +415,20 @@
 uint8_t err = AuthInvalid;
 write();
 detach();
+

[gem5-dev] Cron <m5test@zizzer> /z/m5/regression/do-regression quick

2017-05-10 Thread Cron Daemon 
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64f/simple-timing-ruby:
 CHANGED!
* build/RISCV/tests/opt/quick/se/00.hello/riscv/linux/simple-atomic: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64d/simple-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64d/simple-atomic: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64f/simple-timing: 
CHANGED!
* build/RISCV/tests/opt/quick/se/00.hello/riscv/linux/simple-timing-ruby: 
CHANGED!
* build/RISCV/tests/opt/quick/se/00.hello/riscv/linux/minor-timing: CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64a/minor-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64m/simple-timing-ruby:
 CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64a/simple-atomic: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64a/simple-timing-ruby:
 CHANGED!
* build/RISCV/tests/opt/quick/se/00.hello/riscv/linux/simple-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64a/simple-timing: 
CHANGED!
* build/RISCV/tests/opt/quick/se/00.hello/riscv/linux/o3-timing: CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64f/simple-atomic: 
CHANGED!
* build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64f/o3-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64m/simple-atomic: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64m/minor-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64d/minor-timing: 
CHANGED!
* build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64m/o3-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64f/minor-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64m/simple-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64d/simple-timing-ruby:
 CHANGED!
scons: *** [build/ALPHA/encumbered/eio/libexo.do] Error 1
scons: *** [build/ALPHA/encumbered/eio/eio.do] Error 1
scons: *** [build/ALPHA/encumbered/eio/libexo.fo] Error 1
scons: *** [build/ALPHA/encumbered/eio/eio.fo] Error 1
scons: *** [build/ALPHA/encumbered/eio/libexo.o] Error 1
scons: *** [build/ALPHA/encumbered/eio/eio.o] Error 1
* build/NULL/tests/opt/quick/se/70.tgen/null/none/tgen-simple-mem: passed.
* 
build/NULL_MOESI_hammer/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MOESI_hammer:
 passed.
* build/NULL/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby: passed.
* build/NULL/tests/opt/quick/se/50.memtest/null/none/memtest-filter: passed.
* build/NULL/tests/opt/quick/se/50.memtest/null/none/memtest: passed.
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-simple:
 passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/o3-timing: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-atomic: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing-ruby: 
passed.* build/NULL/tests/opt/quick/se/70.tgen/null/none/tgen-dram-ctrl: 
passed.
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-two-level:
 passed.
* 
build/NULL_MESI_Two_Level/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MESI_Two_Level:
 passed.
* 
build/NULL_MOESI_CMP_directory/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MOESI_CMP_directory:
 passed.
* build/NULL/tests/opt/quick/se/51.memcheck/null/none/memcheck: passed.
* 
build/NULL_MOESI_CMP_token/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MOESI_CMP_token:
 passed.
* build/POWER/tests/opt/quick/se/00.hello/power/linux/simple-atomic: passed.
* build/POWER/tests/opt/quick/se/00.hello/power/linux/o3-timing: passed.
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/simple-timing: 
passed.
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/simple-atomic-mp:
 passed.
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/o3-timing: passed.
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/simple-timing-mp:
 passed.
* 
build/SPARC/tests/opt/quick/se/03.learning-gem5/sparc/linux/learning-gem5-p1-two-level:
 passed.
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/o3-timing-mp:
 passed.
* 
build/SPARC/tests/opt/quick/se/03.learning-gem5/sparc/linux/learning-gem5-p1-simple:
 passed.
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-atomic: passed.
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-timing-ruby: 
passed.
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-timing: passed.
*

Re: [gem5-dev] use of C++ exceptions

2017-05-10 Thread Gabe Black 
This would be internal to the gdb code, ie if a read from the socket fails,
it would detach and throw an exception which would unwind back out of all
the gdb stuff without having to add ifs all over the place. This bit of
code doesn't really have an external interface, so it wouldn't be visible
to the caller, assuming a stray exception didn't escape somehow. I think it
would be a little nicer that way, but not so much that I'd want to argue
for it very strongly.

Gabe

On Wed, May 10, 2017 at 12:09 AM, Andreas Hansson 
wrote:

> Hi Gabe,
>
> I do not think adding exceptions will make things any less cluttered. It
> will simply move that complexity to any caller, will it not? I am not a
> fan of exceptions in general as it mucks with the control flow.
>
> Andreas
>
> On 10/05/2017, 07:31, "gem5-dev on behalf of Gabe Black"
>  wrote:
>
> >Hi folks. I have a change to make the GDB stub in gem5 a bit less fragile:
> >
> >https://gem5-review.googlesource.com/#/c/3180/
> >
> >Unfortunately that involved adding a lot of error code checking which
> >makes
> >things a bit cluttered and ugly. I think it would be a lot nicer to use
> >exceptions, but I remember those being a no-no. Are they currently against
> >the rules, or could I use them to make that code a bit nicer?
> >
> >Gabe
> >___
> >gem5-dev mailing list
> >gem5-dev@gem5.org
> >http://m5sim.org/mailman/listinfo/gem5-dev
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
> ___
> gem5-dev mailing list
> gem5-dev@gem5.org
> http://m5sim.org/mailman/listinfo/gem5-dev
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] use of C++ exceptions

2017-05-10 Thread Andreas Hansson 
Hi Gabe,

I do not think adding exceptions will make things any less cluttered. It
will simply move that complexity to any caller, will it not? I am not a
fan of exceptions in general as it mucks with the control flow.

Andreas

On 10/05/2017, 07:31, "gem5-dev on behalf of Gabe Black"
 wrote:

>Hi folks. I have a change to make the GDB stub in gem5 a bit less fragile:
>
>https://gem5-review.googlesource.com/#/c/3180/
>
>Unfortunately that involved adding a lot of error code checking which
>makes
>things a bit cluttered and ugly. I think it would be a lot nicer to use
>exceptions, but I remember those being a no-no. Are they currently against
>the rules, or could I use them to make that code a bit nicer?
>
>Gabe
>___
>gem5-dev mailing list
>gem5-dev@gem5.org
>http://m5sim.org/mailman/listinfo/gem5-dev

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] use of C++ exceptions

2017-05-10 Thread Gabe Black 
Hi folks. I have a change to make the GDB stub in gem5 a bit less fragile:

https://gem5-review.googlesource.com/#/c/3180/

Unfortunately that involved adding a lot of error code checking which makes
things a bit cluttered and ugly. I think it would be a lot nicer to use
exceptions, but I remember those being a no-no. Are they currently against
the rules, or could I use them to make that code a bit nicer?

Gabe
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in public/gem5[master]: misc: Make the remote GDB stub more resilient to bad connect...

2017-05-10 Thread Gabe Black (Gerrit) 
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/3180



Change subject: misc: Make the remote GDB stub more resilient to bad  
connections.

..

misc: Make the remote GDB stub more resilient to bad connections.

Currently, if the remote gdb stub fails to read a byte from an incoming
packet because the connection has been dropped, the read call will return
anyway and the calling code will have no way to know something bad
happened. It might reattempt the read over and over again waiting for some
particular byte, doomed to never make forward progress.

This change modifies the remote GDB code so that if a read or write call
fails, it will instead detach from the debugger and continue. Before this
change, When simulating a port scan, ie connecting to the debugger port
and then immediately dropping the connection using this command:

nc -v -n -z -w 1 127.0.0.1 7000

gem5 would enter the previously described death spiral. After it, gem5
detaches from the bad connection and resumes execution. Subsequently
attaching with gdb was successful.

This code is written in a C centric style, and would benefit from some
refactoring.

Change-Id: Ie3c0bb35b9cfe3671d0f731e3907548bae0d292f
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 144 insertions(+), 75 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 665dba8..7a0e3b1 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -125,6 +125,7 @@
 #include 

 #include 
+#include 
 #include 
 #include 

@@ -368,24 +369,32 @@
 //
 //

-uint8_t
-BaseRemoteGDB::getbyte()
+bool
+BaseRemoteGDB::getbyte(uint8_t )
 {
-uint8_t b;
-if (::read(fd, , 1) != 1)
-warn("could not read byte from debugger");
-return b;
+if (::read(fd, , sizeof(b)) == sizeof(b)) {
+return true;
+} else {
+warn("Couldn't read data from debugger, detaching.");
+detach();
+return false;
+}
 }

-void
+bool
 BaseRemoteGDB::putbyte(uint8_t b)
 {
-if (::write(fd, , 1) != 1)
-warn("could not write byte to debugger");
+if (::write(fd, , sizeof(b)) == sizeof(b)) {
+return true;
+} else {
+warn("Couldn't write data to the debugger, detaching.");
+detach();
+return false;
+}
 }

 // Send a packet to gdb
-void
+int
 BaseRemoteGDB::send(const char *bp)
 {
 const char *p;
@@ -396,20 +405,27 @@
 do {
 p = bp;
 //Start sending a packet
-putbyte(GDBStart);
+if (!putbyte(GDBStart))
+return -1;
 //Send the contents, and also keep a check sum.
 for (csum = 0; (c = *p); p++) {
-putbyte(c);
+if (!putbyte(c))
+return -1;
 csum += c;
 }
-//Send the ending character.
-putbyte(GDBEnd);
-//Sent the checksum.
-putbyte(i2digit(csum >> 4));
-putbyte(i2digit(csum));
-//Try transmitting over and over again until the other end doesn't  
send an

-//error back.
-} while ((c = getbyte() & 0x7f) == GDBBadP);
+if (//Send the ending character.
+!putbyte(GDBEnd) ||
+//Sent the checksum.
+!putbyte(i2digit(csum >> 4)) ||
+!putbyte(i2digit(csum))) {
+return -1;
+}
+//Try transmitting over and over again until the other end doesn't
+//send an error back.
+if (!getbyte(c))
+return -1;
+} while ((c & 0x7f) == GDBBadP);
+return 0;
 }

 // Receive a packet from gdb
@@ -417,19 +433,26 @@
 BaseRemoteGDB::recv(char *bp, int maxlen)
 {
 char *p;
-int c, csum;
+uint8_t c;
+int csum;
 int len;

 do {
 p = bp;
 csum = len = 0;
 //Find the beginning of a packet
-while ((c = getbyte()) != GDBStart)
-;
+do {
+if (!getbyte(c))
+return -1;
+} while (c != GDBStart);

 //Read until you find the end of the data in the packet, and keep
 //track of the check sum.
-while ((c = getbyte()) != GDBEnd && len < maxlen) {
+while (len < maxlen) {
+if (!getbyte(c))
+return -1;
+if (c == GDBEnd)
+break;
 c &= 0x7f;
 csum += c;
 *p++ = c;
@@ -442,29 +465,35 @@

 //If the command was too long, report an error.
 if (len >= maxlen) {
-putbyte(GDBBadP);
+if (!putbyte(GDBBadP))
+return -1;
 continue;
 }

 //Bring in the checksum. If the check sum matches, csum will be 0.
-csum -= digit2i(getbyte()) * 16;
-csum -= digit2i(getbyte());
+uint8_t csum1, csum2;
+if (!getbyte(csum1) || !getbyte(csum2))
+return -1;
+csum -= digit2i(csum1) *