Fix bare except:
s not catching anything
#354
No reviewers
Labels
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: M-Labs/artiq-zynq#354
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "jcoates/artiq-zynq:dwarf-target2-ttype"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Bare exceptions are currently broken on Kasli-SoC (https://nixbld.m-labs.hk/build/173443/nixlog/12), and do not catch any exceptions.
I believe this is because under the ARM EHABI, the type info pointer is encoded as a
R_ARM_TARGET2
value (see section 10.2), rather than a DWARF encoded pointer. This format is largely equivalent toDW_EH_PE_pcrel
. However, null pointers should not apply an offset, and should instead return 0 directly.1I have run all tests in
test_exceptions.py
with this patch, and then now all pass.except:
clauses which specify an exception continue to work with this change.It's not clear to me what the correct semantics of a 0-offset with
DW_EH_PE_pcrel
(or other relative offsets) would be. GCC skips applying offsets when the value is 0, but other implementations (e.g. LLVM or Rust's) do not. ↩︎Have you ran the remaining tests to check if anything else was broken?
@ -75,17 +75,23 @@ unsafe fn get_ttype_entry(
ttype_base: usize,
ttype: *const u8,
) -> Result<Option<*const u8>, ()> {
if encoding != DW_EH_PE_absptr {
Is this check necessary?
@ -89,0 +84,4 @@
// Read in a target2 value. See "Personality routine exception-handling table
// entries" in the ehabi32 spec, and the ARM_EHABI version of `get_shim_type_info`
// in LLVM's libcxxabi/src/cxa_personality.cpp.
let mut reader = DwarfReader::new(ttype.offset(-i));
I am trying to understand what this is doing differently from the
read_encoded_pointer_with_base
version, assuming the encoding isDW_EH_PE_absptr | DW_EH_PE_pcrel
? My assumption is that the pcrel hack is just to add the offset and the original offset, which you also do explicitly here (only difference being the use ofadd
vs+=
).1be126606d
to8b9bb38331
The difference is for the null pointer — in those cases, we shouldn't apply the offset, and keep it as null. After discussions with @dpn on Mattermost, it's actually fine to check that in
read_encoded_pointer_with_base
, so have reverted the previous commit and applied that change instead.@jcoates Noted, I see the distinction more clearly now. Assuming this passes all HITL tests, we can merge.
EDIT:
I ran some tests in office, seems one of the format tests was not yet adopted for zynq, but thats unrelated to this PR. The rest of the tests pass.
Was that
artiq.test.coredevice.test_exceptions.ExceptionFormatTest.test_nested_formatted_kernel_exception
? I'd definitely expect that one to pass — it does in my tests (ARTIQ d4bd4e587958a3ea249a7d4759d2ddf144d51a85).Apparently the test doesn't account for a device without a
device_map
, so unrelated to this PR. But to clarify, the test that fails istest_rtio_underflow
.