-
-
Notifications
You must be signed in to change notification settings - Fork 412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix incorrect exception handling on aarch64 #4084
Conversation
@@ -216,7 +216,9 @@ static bool lsda_init(lsda_t* lsda, exception_context_t* context) | |||
return true; | |||
} | |||
|
|||
bool ponyint_lsda_scan(exception_context_t* context, uintptr_t* lp) | |||
bool ponyint_lsda_scan(uintptr_t* ttype_index __attribute__ ((unused)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the unused is required to keep gcc from complaining.
gcc has a warning for parameters that aren't used but are set. ttype_index
is an output parameter so it needs to be set.
This first comment on this issue should be used as the commit comment when squashing. |
CodeGenOpt::Level opt_level = | ||
opt->release ? CodeGenOpt::Aggressive : | ||
target_is_arm(opt->triple) ? CodeGenOpt::Default : CodeGenOpt::None; | ||
opt->release ? CodeGenOpt::Aggressive : CodeGenOpt::None; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was our workaround.
@@ -0,0 +1,14 @@ | |||
actor Main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither regression really does anything in CI at the moment because there's only a single runner run and it's in release mode, not debug mode, so these would get inlined away. I've opened a conversation in the ci stream on zulip to discuss how to change.
So, I've flipped the problem here now on aarch64, so more work needs to be done. In... actor Main
let _out: OutStream
new create(env: Env) =>
_out = env.out
foo("hello")
fun foo(s: String) =>
try
one("h")
error
else
one("e")
end
fun one(s: String) =>
_out.print(s) it used to be in |
Our exception handling on aarch64 was incorrect and could lead to valid Pony
programs segfaulting at runtime. This was the result of us never having done an
aarch64 port. A Pony user got us compiling on aarch64 and tests passed, but
there were situations where the incorrect exception handling would cause us to
crash.
In general, the crash was being hidden because the inliner would remove one of
the conditions needed to trigger the problem.
This commit was manually validated on a Raspbian 64-bit installation to verify
the fix.
Fixes #3874