Discussion:
[gem5-dev] Change in gem5/gem5[master]: base: Don't let exceptions leak from the to_number utility function.
Gabe Black (Gerrit)
2018-11-17 07:07:24 UTC
Permalink
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/14399


Change subject: base: Don't let exceptions leak from the to_number utility
function.
......................................................................

base: Don't let exceptions leak from the to_number utility function.

This function catches a couple types of exceptions the functions it
calls might throw, but if one that it doesn't catch is thrown, then
it will propogate that exception to its own callers, and not initialize
the value it was asked to convert.

This might be considered desirable behavior since it lets errors
propogate and avoids handling them in code that might not know the
context of when it's called. On the other hand, it upsets g++ since it
thinks that there might be an uninitialized value used elsewhere, even
though that value will only be uninitialized if an exception is
propogating, and the code that would use it is after a point where that
exception would have been caught and execution would have resumed.

To satisfy g++ and to also avoid silently hiding errors, this change
adds a catch all which will panic if an unexpected exception is raised.

Change-Id: Ie94dcef3a50f7902566328a3fa2eac59b3cf9aad
---
M src/base/str.hh
1 file changed, 4 insertions(+), 0 deletions(-)



diff --git a/src/base/str.hh b/src/base/str.hh
index d73058b..52ab977 100644
--- a/src/base/str.hh
+++ b/src/base/str.hh
@@ -39,6 +39,8 @@
#include <string>
#include <vector>

+#include "base/logging.hh"
+
inline void
eat_lead_white(std::string &s)
{
@@ -157,6 +159,8 @@
return false;
} catch (const std::invalid_argument&) {
return false;
+ } catch (...) {
+ panic("Unrecognized exception.\n");
}
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/14399
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: Ie94dcef3a50f7902566328a3fa2eac59b3cf9aad
Gerrit-Change-Number: 14399
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-MessageType: newchange
Gabe Black (Gerrit)
2018-11-19 21:24:15 UTC
Permalink
Gabe Black has submitted this change and it was merged. (
https://gem5-review.googlesource.com/c/public/gem5/+/14399 )

Change subject: base: Don't let exceptions leak from the to_number utility
function.
......................................................................

base: Don't let exceptions leak from the to_number utility function.

This function catches a couple types of exceptions the functions it
calls might throw, but if one that it doesn't catch is thrown, then
it will propogate that exception to its own callers, and not initialize
the value it was asked to convert.

This might be considered desirable behavior since it lets errors
propogate and avoids handling them in code that might not know the
context of when it's called. On the other hand, it upsets g++ since it
thinks that there might be an uninitialized value used elsewhere, even
though that value will only be uninitialized if an exception is
propogating, and the code that would use it is after a point where that
exception would have been caught and execution would have resumed.

To satisfy g++ and to also avoid silently hiding errors, this change
adds a catch all which will panic if an unexpected exception is raised.

Change-Id: Ie94dcef3a50f7902566328a3fa2eac59b3cf9aad
Reviewed-on: https://gem5-review.googlesource.com/c/14399
Reviewed-by: Jason Lowe-Power <***@lowepower.com>
Maintainer: Jason Lowe-Power <***@lowepower.com>
---
M src/base/str.hh
1 file changed, 4 insertions(+), 0 deletions(-)

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



diff --git a/src/base/str.hh b/src/base/str.hh
index d73058b..52ab977 100644
--- a/src/base/str.hh
+++ b/src/base/str.hh
@@ -39,6 +39,8 @@
#include <string>
#include <vector>

+#include "base/logging.hh"
+
inline void
eat_lead_white(std::string &s)
{
@@ -157,6 +159,8 @@
return false;
} catch (const std::invalid_argument&) {
return false;
+ } catch (...) {
+ panic("Unrecognized exception.\n");
}
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/14399
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: Ie94dcef3a50f7902566328a3fa2eac59b3cf9aad
Gerrit-Change-Number: 14399
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Bradford Beckmann <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gerrit-MessageType: merged
Loading...